-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Rewrite extra::url, based on the right spec #10707
Comments
I'm interested in this |
@stibbons I have a work-in-progress implementation. I’ll publish it later today so you can pick it up if you want. |
Here it is: https://github.com/SimonSapin/rust-url IDNA and the actual URL parser are not there yet, but everything else (host/domain/ipv6, The host parser returns an error on non-ASCII domains. This is good enough for now, IDNA support can be added later. @jgraham also made an in-progress implementation: https://gist.github.com/SimonSapin/7794849 |
Also, be aware that the URL spec itself is still moving and has a number of open bugs: |
I added Punycode encoding and decoding (with tests) to https://github.com/SimonSapin/rust-url but not Nameprep, which is the other big part of IDNA. |
rust-url is now (IMHO) ready to replace It is not "done" yet (some bugs remain, including in the spec!), but at this point it has more than feature-parity and is probably correct in more corner cases. Given the current tendency to move things out of So, whenever the Rust team decides it’s appropriate, I suggest removing |
I would generally be in favor of a |
In that case, I’d like to iterate a bit on rust-url (port rust-http and Servo to it, …) before it moves into the distribution. Also, rust-url depends on rust-encoding, which I guess would have to move as well. |
I’d say rust-url is now in "beta" state. I have branches of rust-http and Servo using it: It builds with Cargo. It depends on rust-encoding (which also builds with Cargo), but for something non-essential that could be deactivated if avoiding the additional dependency helps. I’d like people to start using it and give feedback. I think the best way to do that is to get rid of the old liburl. I’ll go through the RFC process, but before that, @alexcrichton what would you prefer?
[*] In any case, I’d like to keep a separate rust-url repository that I can update in Servo independently of language upgrades. |
+1 on removing liburl. Thanks @SimonSapin |
Perhaps we can deprecate all of liburl for one release cycle. |
This seems like a great place to let cargo shine. |
@brson Deprecating sounds reasonable. That means emitting warnings by default, right? |
@SimonSapin, awesome work! I agree with @cmr and @brson that I think this is where cargo shines, and we can probably just remove liburl entirely without re-adding rust-url. We can also try to grow more official support for it in terms of documentation and automation (not present currently). We can mark liburl as |
The replacement is [rust-url](https://github.com/servo/rust-url), which can be used with Cargo. Fix rust-lang#15874 Fix rust-lang#10707 Close rust-lang#10706 Close rust-lang#10705 Close rust-lang#8486
…t, r=Manishearth check for `..` pattern in `redundant_pattern_matching` The `redundant_pattern_matching` lint currently checks for `if let Some(_) = ...`, but not for `if let Some(..) = ...`. This PR makes sure to also check for the `..` pattern in tuple structs. It also found one such instance in clippy itself so that shows it's worth checking for this pattern as well 😅 changelog: [`redundant_pattern_matching`]: check for `..` pattern in tuple structs
The common fix for #8486, #10705, and #10706 is to rewrite the module based on the spec at http://url.spec.whatwg.org/ and the tests at https://github.com/w3c/web-platform-tests/tree/master/url
(Note: the tests assume the URLUtils API, designed for JavaScript in web browsers. We do not necessarily want the same API for Rust outside of testing code.)
The text was updated successfully, but these errors were encountered: