Skip to content

Conversation

@mahmoud
Copy link
Member

@mahmoud mahmoud commented Feb 24, 2018

Per #61. This PR adds a new argument to _percent_decode(), its variants, and URL.normalize().

This option (disabled by default, and enabled for normalize), exists to turn unmatched % characters to their encoded equivalent, correcting an otherwise improperly encoded URL. Major browsers don't do this by default, so it's currently limited to the explicit normalize step.

… variants, and URL.normalize(). This option (disabled by default, and enabled for normalize), exists to turn unmatched % characters to their encoded equivalent, correcting an otherwise improperly encoded URL. Major browsers don't do this by default, so it's currently limited to the explicit normalize step. Fixes #61.
@codecov-io
Copy link

codecov-io commented Feb 24, 2018

Codecov Report

Merging #62 into master will increase coverage by 0.14%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #62      +/-   ##
==========================================
+ Coverage   97.97%   98.11%   +0.14%     
==========================================
  Files           8        8              
  Lines        1431     1433       +2     
  Branches      166      165       -1     
==========================================
+ Hits         1402     1406       +4     
+ Misses         14       13       -1     
+ Partials       15       14       -1
Impacted Files Coverage Δ
hyperlink/_url.py 96.42% <100%> (+0.28%) ⬆️
hyperlink/test/test_url.py 99.8% <100%> (ø) ⬆️

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 28c908b...4cbf71f. Read the comment docs.

Copy link
Collaborator

@glyph glyph left a comment

Choose a reason for hiding this comment

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

encode_stray_percents seems somewhat wordy, especially in a portion of the API which has been optimized for brevity.

Could we just call it "percents"?

Otherwise I like this change.

@mahmoud
Copy link
Member Author

mahmoud commented Feb 26, 2018

@glyph, you got it, normalize() now takes a "percents" argument.

@mahmoud mahmoud merged commit e3b496f into master Feb 26, 2018
@mahmoud mahmoud deleted the i61_normalize_stray_percents branch April 8, 2019 07:08
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.

4 participants