Update to rust-url 1.0 #42

SimonSapin · 2016-03-02T15:52:05Z

Do not merge yet: rust-url 1.0 is not published yet and may still get breaking changes. The goal of this PR for now is to demonstrate API usage.

Depends on servo/rust-url#176

Version 1.0, rewrite the data structure and API **Do not merge yet**, I’d like to get feedback on API design. This is a large rewrite of the data structures and API that aims to eventually become version 1.0.0. Motivations are: * I designed rust-url when I was relatively new to Rust, coming from Python where memory allocation and ownership are not concerns. The URL Standard also writes algorithms to optimize for human comprehension, not computer resources. As a result, each component of the `Url` struct is a `String` or a `Vec<String>`. * At the same time, I tried to use enums to enforce some invariants in the type system. For example, “non-relative” like `data:` don’t have a host or port number. In practice I think this makes the API a pain to use more than it’s worth. * The spec [has changed](whatwg/url@b266a43) a lot (notably around dealing with protocols/schemes outside the small list it knows about), and I’m not sure the API can be adapted backward-compatibly anyway. I think the API can be more "rusty", and a bunch of long-standing issues be fixed along the way. The big idea is that the `Url` struct now contains a single `String` of the URL’s serialization (the only heap memory allocation), plus some indices into it (and some other meta-data). The indices allow accessor methods for URL components to return `&str` in O(1) time. The fields of `Url` are now private, as they need to maintain a number of invariants. Getting invariants wrong is not a memory-safety hazard (e.g. string indexing is still checked) but can cause non-sensical results. Rather than multiple ad-hoc methods like `serialize_authority` and `serialize_without_fragment`, `Url` implements indexing with ranges of `Position`, which is an enum indicating a position between URL components. For example, `serialize_without_fragment` is now `&url[..Position::AfterQuery]`, takes O(1) time, and returns a `&str` slice. While we’re making breaking changes, this is an opportunity to clean up APIs that didn’t turn out so great: * Drop `UrlParser`. The builder pattern works well for `std::process::Command`, but for URLs in practice almost only `base_url` is ever used. v0.5.2 already has [`base_url.join(str)`](https://github.com/servo/rust-url/blob/v0.5.2/src/lib.rs#L867-L874), which stays the same. * Drop all the `lossy_percent_decode_*` methods. They don’t seem to be used at all. Maybe accessors could return a `PercentEncodedStr` wrapper that dereferences to `&str` and has a `percent_decode()` method? * In the `percent_encoding` module, there was “duplicated” APIs that returned a new `String` or `Vec<u8>` for convenience, or pushed to an existing `&mut String` or `&mut Vec<u8>` for flexibility. This was a bit awkward, so I’ve changed it to return iterators of `char` or `u8` which can be used with `.extend(…)` or `.collect()`. I’m not sure this an improvement and might revert it. Feedback appreciated. This new design is great for accessing components, but not so much for modifying them. We need to make changes in the middle of the string, and with percent-encoding we don’t know the encoded size of the new thing until we’ve encoded it. Then we need to fix up indices. As a result, the `set_*` methods have lots of fiddly code where it’s easy to make subtle mistakes. This code should be well-tested before this PR is merged, but it’s not at all yet. To see what the new API looks like more easily than by reading the code, you can find the output of `cargo doc` at https://simonsapin.github.io/rust-url-1.0-docs/url/ Too see what the API usage is like in practice, I’ve updated some crates that use rust-url for this PR: * https://github.com/alexcrichton/git2-rs/pull/115 * rust-lang/cargo#2428 * rwf2/cookie-rs#42 * hyperium/hyper#740 * https://github.com/cyderize/rust-websocket/pull/70 * servo/servo#9840 Since this PR isn’t merged yet, to do the same in your crates you’ll need a clone of the git repository at the right branch: ``` git clone https://github.com/servo/rust-url --branch 1.0 ``` … and a Cargo [path override](http://doc.crates.io/guide.html#overriding-dependencies) in the directory of your crate: `.cargo/config` ``` path = [ "../rust-url", ] ``` Since we’re making breaking changes, this is the opportunity to make *more* (or different) breaking changes before version 1.0 is published and we commit to this new API. Please comment here to say what are your pain points with the 0.5.x API, what could be improved, what I’ve done in this PR that doesn’t seem like a good idea, etc. Thanks!

Update to rust-url 1.0 **Do not merge yet:** rust-url 1.0 is not published yet and may still get breaking changes. The goal of this PR for now is to demonstrate API usage. Depends on: * servo/rust-url#176 * rwf2/cookie-rs#42 * hyperium/hyper#740 * https://github.com/cyderize/rust-websocket/pull/70  [<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9840)

SimonSapin · 2016-04-21T21:16:42Z

https://crates.io/crates/url/1.0.0 is (finally!) published, so this is ready to land. Could you also publish it on crates.io?

alexcrichton · 2016-04-21T22:05:22Z

Nice!

SimonSapin · 2016-04-22T07:05:54Z

It realized that this crates only uses percent-encoding. Do you think it would be useful to extract that from the rest of url into a separate crate?

alexcrichton · 2016-04-22T16:25:52Z

Perhaps? I'd be fine just leaving the dependency on url, not a burning desire of me to slim that down at all

Update to rust-url 1.0 **Do not merge yet:** rust-url 1.0 is not published yet and may still get breaking changes. The goal of this PR for now is to demonstrate API usage. Depends on: * <s>https://github.com/servo/rust-url/pull/176</s> * <s>https://github.com/alexcrichton/cookie-rs/pull/42</s> * <s>https://github.com/hyperium/hyper/pull/740</s> * https://github.com/cyderize/rust-websocket/pull/70 * mozilla/webdriver-rust#28  [<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9840)

…sajeffrey **Do not merge yet:** rust-url 1.0 is not published yet and may still get breaking changes. The goal of this PR for now is to demonstrate API usage. Depends on: * <s>https://github.com/servo/rust-url/pull/176</s> * <s>https://github.com/alexcrichton/cookie-rs/pull/42</s> * <s>https://github.com/hyperium/hyper/pull/740</s> * https://github.com/cyderize/rust-websocket/pull/70 * mozilla/webdriver-rust#28 Source-Repo: https://github.com/servo/servo Source-Revision: 84ab7e9fe8f4a6528995eff3eb6e814cb724c364 UltraBlame original commit: 6da38378a33a0b292803ee9f46fd03ed7ecd4968

This was referenced Mar 2, 2016

Update to rust-url 1.0 hyperium/hyper#740

Merged

Update to rust-url 1.0 websockets-rs/rust-websocket#70

Merged

Update to rust-url 1.0 servo/servo#9840

Merged

Version 1.0, rewrite the data structure and API servo/rust-url#176

Merged

SimonSapin force-pushed the url-1.0 branch from 1b728a0 to 4006a87 Compare April 20, 2016 22:06

Update to rust-url 1.0

d5adcad

SimonSapin force-pushed the url-1.0 branch from 4006a87 to d5adcad Compare April 21, 2016 21:14

alexcrichton merged commit eeaf3ec into rwf2:master Apr 21, 2016

SimonSapin deleted the url-1.0 branch April 22, 2016 07:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to rust-url 1.0 #42

Update to rust-url 1.0 #42

SimonSapin commented Mar 2, 2016 •

edited

SimonSapin commented Apr 21, 2016

alexcrichton commented Apr 21, 2016

SimonSapin commented Apr 22, 2016

alexcrichton commented Apr 22, 2016

Update to rust-url 1.0 #42

Update to rust-url 1.0 #42

Conversation

SimonSapin commented Mar 2, 2016 • edited

SimonSapin commented Apr 21, 2016

alexcrichton commented Apr 21, 2016

SimonSapin commented Apr 22, 2016

alexcrichton commented Apr 22, 2016

SimonSapin commented Mar 2, 2016 •

edited