{"action":"closed","number":176,"pull_request":{"url":"https://api.github.com/repos/servo/rust-url/pulls/176","id":61419709,"html_url":"https://github.com/servo/rust-url/pull/176","diff_url":"https://github.com/servo/rust-url/pull/176.diff","patch_url":"https://github.com/servo/rust-url/pull/176.patch","issue_url":"https://api.github.com/repos/servo/rust-url/issues/176","number":176,"state":"closed","locked":false,"title":"Version 1.0, rewrite the data structure and API","user":{"login":"SimonSapin","id":291359,"avatar_url":"https://avatars.githubusercontent.com/u/291359?v=3","gravatar_id":"","url":"https://api.github.com/users/SimonSapin","html_url":"https://github.com/SimonSapin","followers_url":"https://api.github.com/users/SimonSapin/followers","following_url":"https://api.github.com/users/SimonSapin/following{/other_user}","gists_url":"https://api.github.com/users/SimonSapin/gists{/gist_id}","starred_url":"https://api.github.com/users/SimonSapin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/SimonSapin/subscriptions","organizations_url":"https://api.github.com/users/SimonSapin/orgs","repos_url":"https://api.github.com/users/SimonSapin/repos","events_url":"https://api.github.com/users/SimonSapin/events{/privacy}","received_events_url":"https://api.github.com/users/SimonSapin/received_events","type":"User","site_admin":false},"body":"**Do not merge yet**, I’d like to get feedback on API design.\r\n\r\n## What’s changing\r\n\r\nThis is a large rewrite of the data structures and API that aims to eventually become version 1.0.0. Motivations are:\r\n\r\n* 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>`.\r\n* 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.\r\n* The spec [has changed](https://github.com/whatwg/url/commit/b266a43fc9df0e8607074bd4d336a517e2010009) 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.\r\n\r\nI 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.\r\n\r\nThe 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.\r\n\r\nRather 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.\r\n\r\nWhile we’re making breaking changes, this is an opportunity to clean up APIs that didn’t turn out so great:\r\n\r\n* 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.\r\n* 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?\r\n* 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.\r\n\r\n## Downsides\r\n\r\nThis 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.\r\n\r\n## Docs\r\n\r\nTo 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/\r\n\r\n## Trying it out\r\n\r\nToo see what the API usage is like in practice, I’ve updated some crates that use rust-url for this PR:\r\n\r\n* https://github.com/alexcrichton/git2-rs/pull/115\r\n* https://github.com/rust-lang/cargo/pull/2428\r\n* https://github.com/alexcrichton/cookie-rs/pull/42\r\n* https://github.com/hyperium/hyper/pull/740\r\n* https://github.com/cyderize/rust-websocket/pull/70\r\n* https://github.com/servo/servo/pull/9840\r\n\r\nSince 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:\r\n\r\n\r\n```\r\ngit clone https://github.com/servo/rust-url --branch 1.0\r\n```\r\n\r\n… and a Cargo [path override](http://doc.crates.io/guide.html#overriding-dependencies) in the directory of your crate:\r\n\r\n`.cargo/config`\r\n```\r\npath = [\r\n    \"../rust-url\",\r\n]\r\n```\r\n\r\n## Feedback, please\r\n\r\nSince 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.\r\n\r\nPlease 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.\r\n\r\nThanks!\r\n\r\n<!-- Reviewable:start -->\r\n<!--  (Simon) Hidden for now \r\n[<img src=\"https://reviewable.io/review_button.svg\" height=\"40\" alt=\"Review on Reviewable\"/>](https://reviewable.io/reviews/servo/rust-url/176)\r\n-->\r\n<!-- Reviewable:end -->\r\n","created_at":"2016-03-02T15:47:11Z","updated_at":"2016-04-20T21:46:17Z","closed_at":"2016-04-20T21:46:17Z","merged_at":"2016-04-20T21:46:17Z","merge_commit_sha":"4a59d935fccd69366f46b64cd2928f894180b740","assignee":{"login":"asajeffrey","id":403333,"avatar_url":"https://avatars.githubusercontent.com/u/403333?v=3","gravatar_id":"","url":"https://api.github.com/users/asajeffrey","html_url":"https://github.com/asajeffrey","followers_url":"https://api.github.com/users/asajeffrey/followers","following_url":"https://api.github.com/users/asajeffrey/following{/other_user}","gists_url":"https://api.github.com/users/asajeffrey/gists{/gist_id}","starred_url":"https://api.github.com/users/asajeffrey/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/asajeffrey/subscriptions","organizations_url":"https://api.github.com/users/asajeffrey/orgs","repos_url":"https://api.github.com/users/asajeffrey/repos","events_url":"https://api.github.com/users/asajeffrey/events{/privacy}","received_events_url":"https://api.github.com/users/asajeffrey/received_events","type":"User","site_admin":false},"milestone":null,"commits_url":"https://api.github.com/repos/servo/rust-url/pulls/176/commits","review_comments_url":"https://api.github.com/repos/servo/rust-url/pulls/176/comments","review_comment_url":"https://api.github.com/repos/servo/rust-url/pulls/comments{/number}","comments_url":"https://api.github.com/repos/servo/rust-url/issues/176/comments","statuses_url":"https://api.github.com/repos/servo/rust-url/statuses/4a59d935fccd69366f46b64cd2928f894180b740","head":{"label":"servo:1.0","ref":"1.0","sha":"4a59d935fccd69366f46b64cd2928f894180b740","user":{"login":"servo","id":2566135,"avatar_url":"https://avatars.githubusercontent.com/u/2566135?v=3","gravatar_id":"","url":"https://api.github.com/users/servo","html_url":"https://github.com/servo","followers_url":"https://api.github.com/users/servo/followers","following_url":"https://api.github.com/users/servo/following{/other_user}","gists_url":"https://api.github.com/users/servo/gists{/gist_id}","starred_url":"https://api.github.com/users/servo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/servo/subscriptions","organizations_url":"https://api.github.com/users/servo/orgs","repos_url":"https://api.github.com/users/servo/repos","events_url":"https://api.github.com/users/servo/events{/privacy}","received_events_url":"https://api.github.com/users/servo/received_events","type":"Organization","site_admin":false},"repo":{"id":14934771,"name":"rust-url","full_name":"servo/rust-url","owner":{"login":"servo","id":2566135,"avatar_url":"https://avatars.githubusercontent.com/u/2566135?v=3","gravatar_id":"","url":"https://api.github.com/users/servo","html_url":"https://github.com/servo","followers_url":"https://api.github.com/users/servo/followers","following_url":"https://api.github.com/users/servo/following{/other_user}","gists_url":"https://api.github.com/users/servo/gists{/gist_id}","starred_url":"https://api.github.com/users/servo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/servo/subscriptions","organizations_url":"https://api.github.com/users/servo/orgs","repos_url":"https://api.github.com/users/servo/repos","events_url":"https://api.github.com/users/servo/events{/privacy}","received_events_url":"https://api.github.com/users/servo/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/servo/rust-url","description":"URL parser for Rust","fork":false,"url":"https://api.github.com/repos/servo/rust-url","forks_url":"https://api.github.com/repos/servo/rust-url/forks","keys_url":"https://api.github.com/repos/servo/rust-url/keys{/key_id}","collaborators_url":"https://api.github.com/repos/servo/rust-url/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/servo/rust-url/teams","hooks_url":"https://api.github.com/repos/servo/rust-url/hooks","issue_events_url":"https://api.github.com/repos/servo/rust-url/issues/events{/number}","events_url":"https://api.github.com/repos/servo/rust-url/events","assignees_url":"https://api.github.com/repos/servo/rust-url/assignees{/user}","branches_url":"https://api.github.com/repos/servo/rust-url/branches{/branch}","tags_url":"https://api.github.com/repos/servo/rust-url/tags","blobs_url":"https://api.github.com/repos/servo/rust-url/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/servo/rust-url/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/servo/rust-url/git/refs{/sha}","trees_url":"https://api.github.com/repos/servo/rust-url/git/trees{/sha}","statuses_url":"https://api.github.com/repos/servo/rust-url/statuses/{sha}","languages_url":"https://api.github.com/repos/servo/rust-url/languages","stargazers_url":"https://api.github.com/repos/servo/rust-url/stargazers","contributors_url":"https://api.github.com/repos/servo/rust-url/contributors","subscribers_url":"https://api.github.com/repos/servo/rust-url/subscribers","subscription_url":"https://api.github.com/repos/servo/rust-url/subscription","commits_url":"https://api.github.com/repos/servo/rust-url/commits{/sha}","git_commits_url":"https://api.github.com/repos/servo/rust-url/git/commits{/sha}","comments_url":"https://api.github.com/repos/servo/rust-url/comments{/number}","issue_comment_url":"https://api.github.com/repos/servo/rust-url/issues/comments{/number}","contents_url":"https://api.github.com/repos/servo/rust-url/contents/{+path}","compare_url":"https://api.github.com/repos/servo/rust-url/compare/{base}...{head}","merges_url":"https://api.github.com/repos/servo/rust-url/merges","archive_url":"https://api.github.com/repos/servo/rust-url/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/servo/rust-url/downloads","issues_url":"https://api.github.com/repos/servo/rust-url/issues{/number}","pulls_url":"https://api.github.com/repos/servo/rust-url/pulls{/number}","milestones_url":"https://api.github.com/repos/servo/rust-url/milestones{/number}","notifications_url":"https://api.github.com/repos/servo/rust-url/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/servo/rust-url/labels{/name}","releases_url":"https://api.github.com/repos/servo/rust-url/releases{/id}","deployments_url":"https://api.github.com/repos/servo/rust-url/deployments","created_at":"2013-12-04T20:19:38Z","updated_at":"2016-04-18T08:28:08Z","pushed_at":"2016-04-20T21:46:16Z","git_url":"git://github.com/servo/rust-url.git","ssh_url":"git@github.com:servo/rust-url.git","clone_url":"https://github.com/servo/rust-url.git","svn_url":"https://github.com/servo/rust-url","homepage":"http://servo.github.io/rust-url/url/index.html","size":3184,"stargazers_count":140,"watchers_count":140,"language":"Rust","has_issues":true,"has_downloads":true,"has_wiki":false,"has_pages":true,"forks_count":65,"mirror_url":null,"open_issues_count":25,"forks":65,"open_issues":25,"watchers":140,"default_branch":"master"}},"base":{"label":"servo:master","ref":"master","sha":"be00f8f007ef09891328b20b7beaf41a043240c0","user":{"login":"servo","id":2566135,"avatar_url":"https://avatars.githubusercontent.com/u/2566135?v=3","gravatar_id":"","url":"https://api.github.com/users/servo","html_url":"https://github.com/servo","followers_url":"https://api.github.com/users/servo/followers","following_url":"https://api.github.com/users/servo/following{/other_user}","gists_url":"https://api.github.com/users/servo/gists{/gist_id}","starred_url":"https://api.github.com/users/servo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/servo/subscriptions","organizations_url":"https://api.github.com/users/servo/orgs","repos_url":"https://api.github.com/users/servo/repos","events_url":"https://api.github.com/users/servo/events{/privacy}","received_events_url":"https://api.github.com/users/servo/received_events","type":"Organization","site_admin":false},"repo":{"id":14934771,"name":"rust-url","full_name":"servo/rust-url","owner":{"login":"servo","id":2566135,"avatar_url":"https://avatars.githubusercontent.com/u/2566135?v=3","gravatar_id":"","url":"https://api.github.com/users/servo","html_url":"https://github.com/servo","followers_url":"https://api.github.com/users/servo/followers","following_url":"https://api.github.com/users/servo/following{/other_user}","gists_url":"https://api.github.com/users/servo/gists{/gist_id}","starred_url":"https://api.github.com/users/servo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/servo/subscriptions","organizations_url":"https://api.github.com/users/servo/orgs","repos_url":"https://api.github.com/users/servo/repos","events_url":"https://api.github.com/users/servo/events{/privacy}","received_events_url":"https://api.github.com/users/servo/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/servo/rust-url","description":"URL parser for Rust","fork":false,"url":"https://api.github.com/repos/servo/rust-url","forks_url":"https://api.github.com/repos/servo/rust-url/forks","keys_url":"https://api.github.com/repos/servo/rust-url/keys{/key_id}","collaborators_url":"https://api.github.com/repos/servo/rust-url/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/servo/rust-url/teams","hooks_url":"https://api.github.com/repos/servo/rust-url/hooks","issue_events_url":"https://api.github.com/repos/servo/rust-url/issues/events{/number}","events_url":"https://api.github.com/repos/servo/rust-url/events","assignees_url":"https://api.github.com/repos/servo/rust-url/assignees{/user}","branches_url":"https://api.github.com/repos/servo/rust-url/branches{/branch}","tags_url":"https://api.github.com/repos/servo/rust-url/tags","blobs_url":"https://api.github.com/repos/servo/rust-url/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/servo/rust-url/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/servo/rust-url/git/refs{/sha}","trees_url":"https://api.github.com/repos/servo/rust-url/git/trees{/sha}","statuses_url":"https://api.github.com/repos/servo/rust-url/statuses/{sha}","languages_url":"https://api.github.com/repos/servo/rust-url/languages","stargazers_url":"https://api.github.com/repos/servo/rust-url/stargazers","contributors_url":"https://api.github.com/repos/servo/rust-url/contributors","subscribers_url":"https://api.github.com/repos/servo/rust-url/subscribers","subscription_url":"https://api.github.com/repos/servo/rust-url/subscription","commits_url":"https://api.github.com/repos/servo/rust-url/commits{/sha}","git_commits_url":"https://api.github.com/repos/servo/rust-url/git/commits{/sha}","comments_url":"https://api.github.com/repos/servo/rust-url/comments{/number}","issue_comment_url":"https://api.github.com/repos/servo/rust-url/issues/comments{/number}","contents_url":"https://api.github.com/repos/servo/rust-url/contents/{+path}","compare_url":"https://api.github.com/repos/servo/rust-url/compare/{base}...{head}","merges_url":"https://api.github.com/repos/servo/rust-url/merges","archive_url":"https://api.github.com/repos/servo/rust-url/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/servo/rust-url/downloads","issues_url":"https://api.github.com/repos/servo/rust-url/issues{/number}","pulls_url":"https://api.github.com/repos/servo/rust-url/pulls{/number}","milestones_url":"https://api.github.com/repos/servo/rust-url/milestones{/number}","notifications_url":"https://api.github.com/repos/servo/rust-url/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/servo/rust-url/labels{/name}","releases_url":"https://api.github.com/repos/servo/rust-url/releases{/id}","deployments_url":"https://api.github.com/repos/servo/rust-url/deployments","created_at":"2013-12-04T20:19:38Z","updated_at":"2016-04-18T08:28:08Z","pushed_at":"2016-04-20T21:46:16Z","git_url":"git://github.com/servo/rust-url.git","ssh_url":"git@github.com:servo/rust-url.git","clone_url":"https://github.com/servo/rust-url.git","svn_url":"https://github.com/servo/rust-url","homepage":"http://servo.github.io/rust-url/url/index.html","size":3184,"stargazers_count":140,"watchers_count":140,"language":"Rust","has_issues":true,"has_downloads":true,"has_wiki":false,"has_pages":true,"forks_count":65,"mirror_url":null,"open_issues_count":25,"forks":65,"open_issues":25,"watchers":140,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/servo/rust-url/pulls/176"},"html":{"href":"https://github.com/servo/rust-url/pull/176"},"issue":{"href":"https://api.github.com/repos/servo/rust-url/issues/176"},"comments":{"href":"https://api.github.com/repos/servo/rust-url/issues/176/comments"},"review_comments":{"href":"https://api.github.com/repos/servo/rust-url/pulls/176/comments"},"review_comment":{"href":"https://api.github.com/repos/servo/rust-url/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/servo/rust-url/pulls/176/commits"},"statuses":{"href":"https://api.github.com/repos/servo/rust-url/statuses/4a59d935fccd69366f46b64cd2928f894180b740"}},"merged":true,"mergeable":null,"mergeable_state":"unknown","merged_by":{"login":"bors-servo","id":4368172,"avatar_url":"https://avatars.githubusercontent.com/u/4368172?v=3","gravatar_id":"","url":"https://api.github.com/users/bors-servo","html_url":"https://github.com/bors-servo","followers_url":"https://api.github.com/users/bors-servo/followers","following_url":"https://api.github.com/users/bors-servo/following{/other_user}","gists_url":"https://api.github.com/users/bors-servo/gists{/gist_id}","starred_url":"https://api.github.com/users/bors-servo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bors-servo/subscriptions","organizations_url":"https://api.github.com/users/bors-servo/orgs","repos_url":"https://api.github.com/users/bors-servo/repos","events_url":"https://api.github.com/users/bors-servo/events{/privacy}","received_events_url":"https://api.github.com/users/bors-servo/received_events","type":"User","site_admin":false},"comments":94,"review_comments":0,"commits":89,"additions":9345,"deletions":3259,"changed_files":40}}