Switch to using idna package

[mahmoud]

As discussed in issue #19, Python's builtin idna support is outdated and broken. Thankfully the idna package exists. This PR switches to using that, instead of Python's builtin idna codec.

[codecov-io]

Codecov Report

Merging #56 into master will decrease coverage by 0.84%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #56      +/-   ##
==========================================
- Coverage   97.94%   97.09%   -0.85%     
==========================================
  Files           8        8              
  Lines        1408     1413       +5     
  Branches      164      166       +2     
==========================================
- Hits         1379     1372       -7     
- Misses         14       22       +8     
- Partials       15       19       +4

Impacted Files	Coverage Δ
hyperlink/test/test_url.py	99.8% <100%> (ø)	⬆️
hyperlink/_url.py	94.96% <100%> (-1.15%)	⬇️
hyperlink/test/test_parse.py	85.71% <0%> (-14.29%)	⬇️
hyperlink/test/test_decoded_url.py	100% <0%> (ø)	⬆️

---

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 a23a1a4...99e9c02. Read the comment docs.

[glyph]

Thinking out loud my anxieties here, rather than a code review:

• I seem to remember that the built-in idna encoding manages to encode some things which are officially "invalid" yet recognized by browsers. Am I misremembering?
• Do we care about "zero dependencies"? Years of being burned by packaging have left their mark; perhaps I shouldn't care, and if you can depend on hyperlink you can depend on its downstreams transitively.
• Why the ceiling pin in install_requires? Is that just for semver compatibility? Is there a process for noticing that the pin should float a version higher? I'd be inclined to just leave it out, to avoid conflicting with other software that might need more recent versions.

[mahmoud]

@glyph, replies in order:

• I haven't reported/encountered anything like that. More worrisome is that idna encoding certain characters can result in totally-invalid URLs (slashes in the host, like ℅ does not encode as a domain name (Python's built-in idna encoding is insufficient) #19). On the positive side, Cory Benfield says they use the idna package for everything. It could be there are old (pre-idna2k8) URLs lying around which might fail to decode, I can make it fallback. Note that I had to kill a doctest but it was clearly an example URL someone typed out, not linked to/used.
• I don't care all that much about no deps. Lots of hyper projects have dependencies. Pretty sure the Twisted tls extra pulls in idna anyways.
• The ceiling on install_requires was copied from requests and is basically semver at this point. I'm fine cutting that, too. The Twisted tls extra uses idna!=2.3,>=0.6.

[wsanchez]

permission ➜ premissive

[wsanchez]

I would remove the <2.7 ceiling, unless there's a known issue; otherwise you are unnecessarily (prematurely, anyway) restricting client use of a newer idna package.

[wsanchez]

Looks great, thanks!

[mahmoud]

@wsanchez, all fixed up! The codecov integration on this PR is all froze up, but the report itself says the diff coverage is 100%, so I'm going to merge momentarily. Thanks!

[glyph]

Approving on @wsanchez's behalf since he's for some reason not an "approved" approver, and also since the answers to my questions were fairly satisfactory. (What is the mechanism for adding him to that list?)

[glyph]

Come on codecov do your thing.

[glyph]

(Please convince codecov that these lines are all covered somehow, then land.)

[mahmoud]

@glyph yeah it's not gonna update, but the report on the site confirms 100% coverage, see screenshot above.

[wsanchez]

@glyph I think I'm not an approved approved because I'm not a committer.