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

add and test URL.__str__, which enables URLs to be passed directly to… #55

Merged
merged 3 commits into from
Jan 6, 2018

Conversation

mahmoud
Copy link
Member

@mahmoud mahmoud commented Dec 31, 2017

This URL.__str__() enables URLs to be passed directly to libaries which stringify (like requests), as brought up by issue #49.

I've opted to keep it simple and have Python 2's __str__ (Py3's __bytes__()) return the UTF-8 encoded version of URL.to_text(). This means that when printed on terminals configured to display UTF-8, the user will get a copy-and-pastable URL.

I also considered making __str__ return the ASCII-encoded URL.to_url().to_text(), but this would have the property of obscuring special characters and possibly falsely-equating URIs and IRIs, contrary to hyperlink's longstanding design.

… libaries which stringify (like requests). fixes #49.
@mahmoud mahmoud requested a review from glyph December 31, 2017 22:56
@codecov-io
Copy link

codecov-io commented Dec 31, 2017

Codecov Report

Merging #55 into master will increase coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #55      +/-   ##
=========================================
+ Coverage   97.76%   97.8%   +0.03%     
=========================================
  Files           6       6              
  Lines        1118    1137      +19     
  Branches      135     137       +2     
=========================================
+ Hits         1093    1112      +19     
  Misses         13      13              
  Partials       12      12
Impacted Files Coverage Δ
hyperlink/test/test_url.py 99.8% <100%> (ø) ⬆️
hyperlink/_url.py 95.72% <100%> (+0.05%) ⬆️

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 5810649...04694a0. Read the comment docs.

@glyph
Copy link
Collaborator

glyph commented Dec 31, 2017

Rather than adding the "some libraries are bad at IRIs" in a private docstring that I guarantee you no one will ever read, if the goal here is requests compatibility, perhaps we should apply the as_url() transformation unconditionally here? A higher-fidelity representation is always available via to_text if desired.

@mahmoud
Copy link
Member Author

mahmoud commented Dec 31, 2017

The "some libraries are bad at IRIs" is just a hint of a glimmer of something I remember, combined with general suspicion. I'm fine with the always as_url() if you are, btw.

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.

Please file a follow-up ticket to add documentation showing that this is an intentional integration point with Requests.

(And we should probably make sure it works with Treq too, although I'm not sure if it does right now.)

@mahmoud mahmoud merged commit 17dc8d3 into master Jan 6, 2018
@glyph glyph deleted the i49_better_str branch January 6, 2018 23:33
@markrwilliams
Copy link
Member

@glyph twisted/treq#212

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