Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Uniformize UTF-8 naming #1115

Merged
merged 5 commits into from
Aug 5, 2021
Merged

Uniformize UTF-8 naming #1115

merged 5 commits into from
Aug 5, 2021

Conversation

BoboTiG
Copy link
Contributor

@BoboTiG BoboTiG commented Jul 28, 2021

Replace utf8 -> utf-8 everywhere.
It should have no impact, utf8 is an alias of utf-8 [1].

[1] https://github.com/python/cpython/blob/ee03bad25e83b00ba5fc2a0265b48c6286e6b3f7/Lib/encodings/aliases.py#L534


Full story: I kind of need such uniformization for #1110 where I am adding tests. And I have to deal with our naming (utf8) and the one from requests or charset_normalizer (utf-8).
Note that this is a one-time small clean-up, and I can live without that patch being merged.

Replace `utf8` -> `utf-8` everywhere.
It should have no impact, `utf8` is an alias of `utf-8` [1].

[1] https://github.com/python/cpython/blob/ee03bad25e83b00ba5fc2a0265b48c6286e6b3f7/Lib/encodings/aliases.py#L534
@BoboTiG BoboTiG requested a review from jkbrzt July 28, 2021 08:56
@codecov-commenter
Copy link

codecov-commenter commented Jul 28, 2021

Codecov Report

Merging #1115 (f0ccf9f) into master (04d05a8) will increase coverage by 0.49%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1115      +/-   ##
==========================================
+ Coverage   96.35%   96.84%   +0.49%     
==========================================
  Files          64       65       +1     
  Lines        4137     4180      +43     
==========================================
+ Hits         3986     4048      +62     
+ Misses        151      132      -19     
Impacted Files Coverage Δ
httpie/cli/requestitems.py 91.17% <ø> (ø)
tests/test_cli.py 100.00% <ø> (ø)
tests/test_downloads.py 100.00% <ø> (ø)
httpie/cli/argparser.py 96.35% <100.00%> (ø)
httpie/client.py 100.00% <100.00%> (ø)
httpie/config.py 90.00% <100.00%> (+0.12%) ⬆️
httpie/constants.py 100.00% <100.00%> (ø)
httpie/context.py 87.17% <100.00%> (+0.16%) ⬆️
httpie/models.py 92.00% <100.00%> (+0.10%) ⬆️
httpie/output/streams.py 99.03% <100.00%> (+8.74%) ⬆️
... and 19 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 04d05a8...f0ccf9f. Read the comment docs.

@BoboTiG
Copy link
Contributor Author

BoboTiG commented Jul 29, 2021

It is finally not needed for #1110 but I am still for such clean-up, if you are OK with that @jakubroztocil :)

Copy link
Member

@jkbrzt jkbrzt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove the argument from .encode('utf-8') and .decode('utf-8') since it’s the default.

I’d also introduce a UTF8 = 'UTF-8' constant and use it instead of the string to avoid inconsistencies and to have a central place where we can talk about the encoding.

Let's be explicit over implicit. And prevent future warnings from PEP-597 [1].

[1] https://www.python.org/dev/peps/pep-0597/#using-the-default-encoding-is-a-common-mistake
@BoboTiG
Copy link
Contributor Author

BoboTiG commented Jul 31, 2021

Actually I did the exact opposite :)

I think it is a good thing to explicitly set the encoding everywhere to prevent surprises. And the PEP-597 will eventually throw warnings in a near future.

@BoboTiG
Copy link
Contributor Author

BoboTiG commented Jul 31, 2021

/packit copr-build

@BoboTiG
Copy link
Contributor Author

BoboTiG commented Aug 2, 2021

/packit copr-build

@jkbrzt
Copy link
Member

jkbrzt commented Aug 4, 2021

Actually I did the exact opposite :)

I think it is a good thing to explicitly set the encoding everywhere to prevent surprises. And the PEP-597 will eventually throw warnings in a near future.

I believe the PEP only talks about open()’s encoding. For encode()/decode(), utf-8 is the argument’s default value:

str.encode(encoding="utf-8", errors="strict")
bytes.decode(encoding="utf-8", errors="strict"

https://docs.python.org/3/library/stdtypes.html#str.encode
https://docs.python.org/3/library/stdtypes.html#bytes.decode

@BoboTiG
Copy link
Contributor Author

BoboTiG commented Aug 4, 2021

Oh how did miss that?!

I'll remove all encoding occurrences for those functions then 👍

@BoboTiG BoboTiG requested a review from jkbrzt August 5, 2021 08:53
@jkbrzt jkbrzt merged commit c6cbc7d into httpie:master Aug 5, 2021
@BoboTiG BoboTiG deleted the impr-utf-8-naming-uniformization branch August 5, 2021 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants