From eec3a6eadcda3b12b1c9bc32d45aad102beca55f Mon Sep 17 00:00:00 2001 From: TitiAlone Date: Mon, 3 Jul 2017 19:31:15 +0200 Subject: [PATCH 1/2] Resolved the issue 363 with some doc --- src/path_segments.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/path_segments.rs b/src/path_segments.rs index f5b7d51f9..504932c2b 100644 --- a/src/path_segments.rs +++ b/src/path_segments.rs @@ -214,4 +214,30 @@ impl<'a> PathSegmentsMut<'a> { }); self } + + /// Returns the URL reference the PathSegmentsMut was constructed with. + /// + /// Example: + /// + /// ```rust + /// use url::Url; + /// # use std::error::Error; + /// + /// # fn run() -> Result<(), Box> { + /// let mut url = Url::parse("https://github.com/servo/rust-url/issues/363")?; + /// url.path_segments_mut() + /// .map_err(|_| "cannot be base")? + /// .clear() + /// .finish() + /// .query_pairs_mut() + /// // ... + /// .finish(); + /// assert_eq!(url.as_str(), "https://github.com/"); + /// # Ok(()) + /// # } + /// # run().unwrap(); + /// ``` + pub fn finish(&mut self) -> &mut ::Url { + self.url + } } From d359ea04560633107e4f5fbb81e82ef0cf678d4f Mon Sep 17 00:00:00 2001 From: TitiAlone Date: Fri, 14 Jul 2017 14:47:41 +0200 Subject: [PATCH 2/2] Permanently resolve the issue and changed docs and tests. --- src/path_segments.rs | 26 ++++++++++++-------------- tests/unit.rs | 22 +++++++++++++++++++--- 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/src/path_segments.rs b/src/path_segments.rs index 504932c2b..3f02cfe45 100644 --- a/src/path_segments.rs +++ b/src/path_segments.rs @@ -63,7 +63,7 @@ impl<'a> Drop for PathSegmentsMut<'a> { impl<'a> PathSegmentsMut<'a> { /// Remove all segments in the path, leaving the minimal `url.path() == "/"`. /// - /// Returns `&mut Self` so that method calls can be chained. + /// Returns `mut Self` so that method calls can be chained. /// /// Example: /// @@ -80,7 +80,7 @@ impl<'a> PathSegmentsMut<'a> { /// # } /// # run().unwrap(); /// ``` - pub fn clear(&mut self) -> &mut Self { + pub fn clear(mut self) -> Self { self.url.serialization.truncate(self.after_first_slash); self } @@ -91,7 +91,7 @@ impl<'a> PathSegmentsMut<'a> { /// In other words, remove one path trailing slash, if any, /// unless it is also the initial slash (so this does nothing if `url.path() == "/")`. /// - /// Returns `&mut Self` so that method calls can be chained. + /// Returns `mut Self` so that method calls can be chained. /// /// Example: /// @@ -113,7 +113,7 @@ impl<'a> PathSegmentsMut<'a> { /// # } /// # run().unwrap(); /// ``` - pub fn pop_if_empty(&mut self) -> &mut Self { + pub fn pop_if_empty(mut self) -> Self { if self.url.serialization[self.after_first_slash..].ends_with('/') { self.url.serialization.pop(); } @@ -124,8 +124,8 @@ impl<'a> PathSegmentsMut<'a> { /// /// If the path only has one segment, make it empty such that `url.path() == "/"`. /// - /// Returns `&mut Self` so that method calls can be chained. - pub fn pop(&mut self) -> &mut Self { + /// Returns `mut Self` so that method calls can be chained. + pub fn pop(mut self) -> Self { let last_slash = self.url.serialization[self.after_first_slash..].rfind('/').unwrap_or(0); self.url.serialization.truncate(self.after_first_slash + last_slash); self @@ -135,8 +135,9 @@ impl<'a> PathSegmentsMut<'a> { /// /// See the documentation for `.extend()`. /// - /// Returns `&mut Self` so that method calls can be chained. - pub fn push(&mut self, segment: &str) -> &mut Self { + /// Returns `mut Self` so that method calls can be chained. + #[allow(unused_mut)] + pub fn push(mut self, segment: &str) -> Self { self.extend(Some(segment)) } @@ -156,7 +157,7 @@ impl<'a> PathSegmentsMut<'a> { /// /// To obtain a behavior similar to `Url::join`, call `.pop()` unconditionally first. /// - /// Returns `&mut Self` so that method calls can be chained. + /// Returns `mut Self` so that method calls can be chained. /// /// Example: /// @@ -193,7 +194,7 @@ impl<'a> PathSegmentsMut<'a> { /// # } /// # run().unwrap(); /// ``` - pub fn extend(&mut self, segments: I) -> &mut Self + pub fn extend(mut self, segments: I) -> Self where I: IntoIterator, I::Item: AsRef { let scheme_type = SchemeType::from(self.url.scheme()); let path_start = self.url.path_start as usize; @@ -228,16 +229,13 @@ impl<'a> PathSegmentsMut<'a> { /// url.path_segments_mut() /// .map_err(|_| "cannot be base")? /// .clear() - /// .finish() - /// .query_pairs_mut() - /// // ... /// .finish(); /// assert_eq!(url.as_str(), "https://github.com/"); /// # Ok(()) /// # } /// # run().unwrap(); /// ``` - pub fn finish(&mut self) -> &mut ::Url { + pub fn finish(self) -> &'a mut ::Url { self.url } } diff --git a/tests/unit.rs b/tests/unit.rs index 3f65edd8d..37545dde1 100644 --- a/tests/unit.rs +++ b/tests/unit.rs @@ -331,7 +331,7 @@ fn issue_241() { fn append_trailing_slash() { let mut url: Url = "http://localhost:6767/foo/bar?a=b".parse().unwrap(); url.check_invariants().unwrap(); - url.path_segments_mut().unwrap().push(""); + url.path_segments_mut().unwrap().push("").finish(); url.check_invariants().unwrap(); assert_eq!(url.to_string(), "http://localhost:6767/foo/bar/?a=b"); } @@ -343,7 +343,7 @@ fn extend_query_pairs_then_mutate() { url.query_pairs_mut().extend_pairs(vec![ ("auth", "my-token") ].into_iter()); url.check_invariants().unwrap(); assert_eq!(url.to_string(), "http://localhost:6767/foo/bar?auth=my-token"); - url.path_segments_mut().unwrap().push("some_other_path"); + url.path_segments_mut().unwrap().push("some_other_path").finish(); url.check_invariants().unwrap(); assert_eq!(url.to_string(), "http://localhost:6767/foo/bar/some_other_path?auth=my-token"); } @@ -353,7 +353,7 @@ fn extend_query_pairs_then_mutate() { fn append_empty_segment_then_mutate() { let mut url: Url = "http://localhost:6767/foo/bar?a=b".parse().unwrap(); url.check_invariants().unwrap(); - url.path_segments_mut().unwrap().push("").pop(); + url.path_segments_mut().unwrap().push("").pop().finish(); url.check_invariants().unwrap(); assert_eq!(url.to_string(), "http://localhost:6767/foo/bar?a=b"); } @@ -452,6 +452,22 @@ fn test_origin_hash() { assert_ne!(hash(&opaque_origin), hash(&other_opaque_origin)); } +#[test] +// https://github.com/servo/rust-url/issues/363 +fn chain_path_segments_query_pair() { + let mut url = Url::parse("https://github.com/servo/rust-url/issues/363").unwrap(); + + url.path_segments_mut() + .expect("path_segments_mut") + .clear() + .finish() + .query_pairs_mut() + .finish(); + + /// The ? at the end is added by query_pairs_mut + assert_eq!(url.as_str(), "https://github.com/?"); +} + #[test] fn test_windows_unc_path() { if !cfg!(windows) {