liburl: minor api changes #15225
liburl: minor api changes #15225
Conversation
|
The renaming of functions should be accompanied by deprecations, with some guidance about the new version, that is, #[deprecated="use `Url::parse_str`"]
fn from_str(s: &str) { Url::parse_str(s) } |
| /// fragment: Some("quz".to_string()) }; | ||
| /// // https://username@example.com:8080/foo/bar?baz=qux#quz | ||
| /// let raw = "https://username@example.com:8080/foo/bar?baz=qux#quz"; | ||
| /// let url = Url::parse_str(raw).unwrap(); |
There was a problem hiding this comment.
Can we not have the .unwrap here? Something like
match Url::parse_str(raw) {
Ok(u) => println!("Parsed `{}`", u),
Err(e) => println!("Couldn't parse `{}`: {}", raw, e),
}(assuming that the error implements Show.)
|
@huonw Addressed your feedback |
| /// let url = encode("https://example.com/Rust (programming language)"); | ||
| /// println!("{}", url); // https://example.com/Rust%20(programming%20language) | ||
| /// ``` | ||
| pub fn encode<T: BytesContainer>(container: T) -> String { |
There was a problem hiding this comment.
I would personally prefer to halt the proliferation of BytesContainer and friends. It is indeed slightly more ergonomic, but BytesContainer was not intended for this purpose. Could these functions stay as taking &str?
There was a problem hiding this comment.
I'd favor a breaking change to &[u8] if BytesContainer is off the cards. &str can use as_bytes(), Vec<u8> can use as_slice(), etc.
For reference, while actually working on this there was someone on IRC frustrated by having to decode their bytes into utf8 before calling encode once again.
Alternatively, defining a 'local' trait impl'd for &str, String, Vec<u8>, &[u8] might be an option? Although I guess that's similar territory to BytesContainer.
There was a problem hiding this comment.
We've starting to see more pickup of BytesContainer in #15280 for example, so I think my personal preference may be overruled by ergonomics (this is fine as is)
url::from_str => url::Url::parse_str The FromStr trait still works, but its confusing to have a from_str free function that retuns a Result, while the regular from_str returns an Option, hence the rename. [breaking-change]
url::path_from_str => url::Path::parse_str The FromStr trait still works, but its confusing to have a path_from_str free function that retuns a Result, while the regular from_str style functions return an Option, hence the rename to indicate a Result. [breaking-change]
url.path - Now a Path instead of a String. To fix old code: url.path => url.path.path url.query => url.path.query url.fragment => url.path.fragment Not much point having the Path struct if it's not going to be used. [breaking-change]
Some signatures have changed from String to &str returns. To fix, call to_string() on the returned value. [breaking-change]
|
@alexcrichton |
See commits for info, a number of these are 'breaking', although liburl is marked experimental so I'm not sure that matters so much. First two commits will be impacted if #15138 is adopted, but it's a simple rename.
See commits for info, a number of these are 'breaking', although liburl is marked experimental so I'm not sure that matters so much.
First two commits will be impacted if #15138 is adopted, but it's a simple rename.