From 00cce0af2f118445e1a6eeb72698fe9df054e1cf Mon Sep 17 00:00:00 2001 From: rhysd Date: Fri, 1 Jul 2022 23:14:19 +0900 Subject: [PATCH 01/11] use `Cow<'_, str>` for `to_slash` and `to_slash_lossy` --- src/lib.rs | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 8e7a2d3..6566adc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -54,6 +54,7 @@ #![forbid(unsafe_code)] #![warn(clippy::dbg_macro, clippy::print_stdout)] +use std::borrow::Cow; use std::ffi::OsStr; use std::path::{Path, PathBuf}; @@ -68,8 +69,8 @@ use std::path::{Path, PathBuf}; /// ); /// ``` pub trait PathExt { - fn to_slash(&self) -> Option; - fn to_slash_lossy(&self) -> String; + fn to_slash(&self) -> Option>; + fn to_slash_lossy(&self) -> Cow<'_, str>; } impl PathExt for Path { @@ -93,8 +94,8 @@ impl PathExt for Path { /// assert_eq!(s.to_slash_lossy(), "foo/bar/piyo.txt".to_string()); /// ``` #[cfg(not(target_os = "windows"))] - fn to_slash_lossy(&self) -> String { - self.to_string_lossy().to_string() + fn to_slash_lossy(&self) -> Cow<'_, str> { + self.to_string_lossy() } /// Convert the file path into slash path as UTF-8 string. @@ -117,7 +118,7 @@ impl PathExt for Path { /// assert_eq!(s.to_slash_lossy(), "foo/bar/piyo.txt".to_string()); /// ``` #[cfg(target_os = "windows")] - fn to_slash_lossy(&self) -> String { + fn to_slash_lossy(&self) -> Cow<'_, str> { use std::path; let mut buf = String::new(); @@ -149,7 +150,7 @@ impl PathExt for Path { buf.pop(); // Pop last '/' } - buf + Cow::Owned(buf) } /// Convert the file path into slash path as UTF-8 string. @@ -172,8 +173,8 @@ impl PathExt for Path { /// assert_eq!(s.to_slash(), Some("foo/bar/piyo.txt".to_string())); /// ``` #[cfg(not(target_os = "windows"))] - fn to_slash(&self) -> Option { - self.to_str().map(str::to_string) + fn to_slash(&self) -> Option> { + self.to_str().map(Cow::Borrowed) } /// Convert the file path into slash path as UTF-8 string. @@ -196,7 +197,7 @@ impl PathExt for Path { /// assert_eq!(s.to_slash(), Some("foo/bar/piyo.txt".to_string())); /// ``` #[cfg(target_os = "windows")] - fn to_slash(&self) -> Option { + fn to_slash(&self) -> Option> { use std::path; let mut buf = String::new(); @@ -231,7 +232,7 @@ impl PathExt for Path { buf.pop(); // Pop last '/' } - Some(buf) + Some(Cow::Owned(buf)) } } @@ -250,8 +251,8 @@ pub trait PathBufExt { fn from_slash_lossy>(s: S) -> Self; fn from_backslash>(s: S) -> Self; fn from_backslash_lossy>(s: S) -> Self; - fn to_slash(&self) -> Option; - fn to_slash_lossy(&self) -> String; + fn to_slash(&self) -> Option>; + fn to_slash_lossy(&self) -> Cow<'_, str>; } impl PathBufExt for PathBuf { @@ -435,7 +436,7 @@ impl PathBufExt for PathBuf { /// /// assert_eq!(s.to_slash_lossy(), "foo/bar/piyo.txt".to_string()); /// ``` - fn to_slash_lossy(&self) -> String { + fn to_slash_lossy(&self) -> Cow<'_, str> { self.as_path().to_slash_lossy() } @@ -457,7 +458,7 @@ impl PathBufExt for PathBuf { /// /// assert_eq!(s.to_slash(), Some("foo/bar/piyo.txt".to_string())); /// ``` - fn to_slash(&self) -> Option { + fn to_slash(&self) -> Option> { self.as_path().to_slash() } } From 4e9f638fb2429e99f4c8ddb97542b4b4a3db1747 Mon Sep 17 00:00:00 2001 From: rhysd Date: Sat, 2 Jul 2022 12:00:14 +0900 Subject: [PATCH 02/11] fix tests for checking returned `Cow` values --- src/lib.rs | 85 ++++++++++++++++++++++++++--------------------------- src/test.rs | 13 ++++++-- 2 files changed, 52 insertions(+), 46 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 6566adc..f9d59df 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -22,33 +22,33 @@ //! #[cfg(target_os = "windows")] //! { //! assert_eq!( -//! Path::new(r"foo\bar\piyo.txt").to_slash(), -//! Some("foo/bar/piyo.txt".to_string()), +//! Path::new(r"foo\bar\piyo.txt").to_slash().unwrap(), +//! "foo/bar/piyo.txt", //! ); //! assert_eq!( -//! Path::new(r"C:\foo\bar\piyo.txt").to_slash(), -//! Some("C:/foo/bar/piyo.txt".to_string()), +//! Path::new(r"C:\foo\bar\piyo.txt").to_slash().unwrap(), +//! "C:/foo/bar/piyo.txt", //! ); //! //! let p = PathBuf::from_slash("foo/bar/piyo.txt"); //! assert_eq!(p, PathBuf::from(r"foo\bar\piyo.txt")); -//! assert_eq!(p.to_slash(), Some("foo/bar/piyo.txt".to_string())); +//! assert_eq!(p.to_slash().unwrap(), "foo/bar/piyo.txt"); //! } //! //! #[cfg(not(target_os = "windows"))] //! { //! assert_eq!( -//! Path::new("foo/bar/piyo.txt").to_slash(), -//! Some("foo/bar/piyo.txt".to_string()), +//! Path::new("foo/bar/piyo.txt").to_slash().unwrap(), +//! "foo/bar/piyo.txt", //! ); //! assert_eq!( -//! Path::new("/foo/bar/piyo.txt").to_slash(), -//! Some("/foo/bar/piyo.txt".to_string()), +//! Path::new("/foo/bar/piyo.txt").to_slash().unwrap(), +//! "/foo/bar/piyo.txt", //! ); //! //! let p = PathBuf::from_slash("foo/bar/piyo.txt"); -//! assert_eq!(p, PathBuf::from(r"foo/bar/piyo.txt")); -//! assert_eq!(p.to_slash(), Some("foo/bar/piyo.txt".to_string())); +//! assert_eq!(p, PathBuf::from("foo/bar/piyo.txt")); +//! assert_eq!(p.to_slash().unwrap(), "foo/bar/piyo.txt"); //! } //! ``` #![forbid(unsafe_code)] @@ -61,11 +61,13 @@ use std::path::{Path, PathBuf}; /// Trait to extend [`std::path::Path`]. /// /// ``` +/// # use std::path::Path; +/// # use std::borrow::Cow; /// use path_slash::PathExt; /// /// assert_eq!( -/// std::path::Path::new("foo").to_slash(), -/// Some("foo".to_string()), +/// Path::new("foo").to_slash(), +/// Some(Cow::Borrowed("foo")), /// ); /// ``` pub trait PathExt { @@ -79,10 +81,8 @@ impl PathExt for Path { /// Any file path separators in the file path is replaced with '/'. /// Any non-Unicode sequences are replaced with U+FFFD. /// - /// On non-Windows OS, it is equivalent to `to_string_lossy().to_string()` - /// /// ``` - /// use std::path::Path; + /// # use std::path::Path; /// use path_slash::PathExt; /// /// #[cfg(target_os = "windows")] @@ -91,7 +91,7 @@ impl PathExt for Path { /// #[cfg(not(target_os = "windows"))] /// let s = Path::new("foo/bar/piyo.txt"); /// - /// assert_eq!(s.to_slash_lossy(), "foo/bar/piyo.txt".to_string()); + /// assert_eq!(s.to_slash_lossy(), "foo/bar/piyo.txt"); /// ``` #[cfg(not(target_os = "windows"))] fn to_slash_lossy(&self) -> Cow<'_, str> { @@ -103,10 +103,8 @@ impl PathExt for Path { /// Any file path separators in the file path is replaced with '/'. /// Any non-Unicode sequences are replaced with U+FFFD. /// - /// On non-Windows OS, it is equivalent to `.to_string_lossy().to_string()`. - /// /// ``` - /// use std::path::Path; + /// # use std::path::Path; /// use path_slash::PathExt; /// /// #[cfg(target_os = "windows")] @@ -115,7 +113,7 @@ impl PathExt for Path { /// #[cfg(not(target_os = "windows"))] /// let s = Path::new("foo/bar/piyo.txt"); /// - /// assert_eq!(s.to_slash_lossy(), "foo/bar/piyo.txt".to_string()); + /// assert_eq!(s.to_slash_lossy(), "foo/bar/piyo.txt"); /// ``` #[cfg(target_os = "windows")] fn to_slash_lossy(&self) -> Cow<'_, str> { @@ -158,10 +156,9 @@ impl PathExt for Path { /// Any file path separators in the file path is replaced with '/'. /// When the path contains non-Unicode sequence, this method returns None. /// - /// On non-Windows OS, it is equivalent to `.to_str().map(str::to_string)` - /// /// ``` - /// use std::path::Path; + /// # use std::path::Path; + /// # use std::borrow::Cow; /// use path_slash::PathExt; /// /// #[cfg(target_os = "windows")] @@ -170,7 +167,7 @@ impl PathExt for Path { /// #[cfg(not(target_os = "windows"))] /// let s = Path::new("foo/bar/piyo.txt"); /// - /// assert_eq!(s.to_slash(), Some("foo/bar/piyo.txt".to_string())); + /// assert_eq!(s.to_slash(), Some(Cow::Borrowed("foo/bar/piyo.txt"))); /// ``` #[cfg(not(target_os = "windows"))] fn to_slash(&self) -> Option> { @@ -185,7 +182,8 @@ impl PathExt for Path { /// On non-Windows OS, it is equivalent to `.to_str().map(str::to_string)` /// /// ``` - /// use std::path::Path; + /// # use std::path::Path; + /// # use std::borrow::Cow; /// use path_slash::PathExt; /// /// #[cfg(target_os = "windows")] @@ -194,7 +192,7 @@ impl PathExt for Path { /// #[cfg(not(target_os = "windows"))] /// let s = Path::new("foo/bar/piyo.txt"); /// - /// assert_eq!(s.to_slash(), Some("foo/bar/piyo.txt".to_string())); + /// assert_eq!(s.to_slash(), Some(Cow::Borrowed("foo/bar/piyo.txt"))); /// ``` #[cfg(target_os = "windows")] fn to_slash(&self) -> Option> { @@ -239,11 +237,12 @@ impl PathExt for Path { /// Trait to extend [`std::path::PathBuf`]. /// /// ``` +/// # use std::path::PathBuf; /// use path_slash::PathBufExt; /// /// assert_eq!( -/// std::path::PathBuf::from_slash("foo/bar/piyo.txt").to_slash(), -/// Some("foo/bar/piyo.txt".to_string()), +/// PathBuf::from_slash("foo/bar/piyo.txt").to_slash().unwrap(), +/// "foo/bar/piyo.txt", /// ); /// ``` pub trait PathBufExt { @@ -265,7 +264,7 @@ impl PathBufExt for PathBuf { /// On non-Windows OS, it is simply equivalent to [`std::path::PathBuf::from`]. /// /// ``` - /// use std::path::PathBuf; + /// # use std::path::PathBuf; /// use path_slash::PathBufExt; /// /// let p = PathBuf::from_slash("foo/bar/piyo.txt"); @@ -290,7 +289,7 @@ impl PathBufExt for PathBuf { /// On non-Windows OS, it is simply equivalent to [`std::path::PathBuf::from`]. /// /// ``` - /// use std::path::PathBuf; + /// # use std::path::PathBuf; /// use path_slash::PathBufExt; /// /// let p = PathBuf::from_slash("foo/bar/piyo.txt"); @@ -313,6 +312,7 @@ impl PathBufExt for PathBuf { c => c, }) .collect::(); + PathBuf::from(s) } @@ -371,8 +371,8 @@ impl PathBufExt for PathBuf { /// loss while conversion. /// /// ``` + /// # use std::path::PathBuf; /// use std::ffi::OsStr; - /// use std::path::PathBuf; /// use path_slash::PathBufExt; /// /// let s: &OsStr = "foo/bar/piyo.txt".as_ref(); @@ -400,8 +400,8 @@ impl PathBufExt for PathBuf { /// loss while conversion. /// /// ``` + /// # use std::path::PathBuf; /// use std::ffi::OsStr; - /// use std::path::PathBuf; /// use path_slash::PathBufExt; /// /// let s: &OsStr = "foo/bar/piyo.txt".as_ref(); @@ -423,18 +423,17 @@ impl PathBufExt for PathBuf { /// Any file path separators in the file path is replaced with '/'. /// Any non-Unicode sequences are replaced with U+FFFD. /// - /// On non-Windows OS, it is equivalent to `to_string_lossy().to_string()` - /// /// ``` + /// # use std::path::PathBuf; /// use path_slash::PathBufExt; /// /// #[cfg(target_os = "windows")] - /// let s = std::path::PathBuf::from(r"foo\bar\piyo.txt"); + /// let s = PathBuf::from(r"foo\bar\piyo.txt"); /// /// #[cfg(not(target_os = "windows"))] - /// let s = std::path::PathBuf::from("foo/bar/piyo.txt"); + /// let s = PathBuf::from("foo/bar/piyo.txt"); /// - /// assert_eq!(s.to_slash_lossy(), "foo/bar/piyo.txt".to_string()); + /// assert_eq!(s.to_slash_lossy(), "foo/bar/piyo.txt"); /// ``` fn to_slash_lossy(&self) -> Cow<'_, str> { self.as_path().to_slash_lossy() @@ -445,18 +444,18 @@ impl PathBufExt for PathBuf { /// Any file path separators in the file path is replaced with '/'. /// When the path contains non-Unicode sequence, this method returns None. /// - /// On non-Windows OS, it is equivalent to `.to_str().map(std::to_string())` - /// /// ``` + /// # use std::path::PathBuf; + /// # use std::borrow::Cow; /// use path_slash::PathBufExt; /// /// #[cfg(target_os = "windows")] - /// let s = std::path::PathBuf::from(r"foo\bar\piyo.txt"); + /// let s = PathBuf::from(r"foo\bar\piyo.txt"); /// /// #[cfg(not(target_os = "windows"))] - /// let s = std::path::PathBuf::from("foo/bar/piyo.txt"); + /// let s = PathBuf::from("foo/bar/piyo.txt"); /// - /// assert_eq!(s.to_slash(), Some("foo/bar/piyo.txt".to_string())); + /// assert_eq!(s.to_slash(), Some(Cow::Borrowed("foo/bar/piyo.txt"))); /// ``` fn to_slash(&self) -> Option> { self.as_path().to_slash() diff --git a/src/test.rs b/src/test.rs index a64b946..015b083 100644 --- a/src/test.rs +++ b/src/test.rs @@ -1,5 +1,6 @@ use super::{PathBufExt as _, PathExt as _}; use lazy_static::lazy_static; +use std::borrow::Cow; use std::ffi::OsStr; use std::path; use std::path::PathBuf; @@ -114,14 +115,17 @@ lazy_static! { #[test] fn to_slash_path() { for (input, expected) in TO_SLASH_TESTS.iter() { - assert_eq!(input.as_path().to_slash(), Some(expected.clone())); + assert_eq!( + input.as_path().to_slash(), + Some(Cow::Borrowed(expected.as_str())) + ); } } #[test] fn to_slash_pathbuf() { for (input, expected) in TO_SLASH_TESTS.iter() { - assert_eq!(input.to_slash(), Some(expected.clone())); + assert_eq!(input.to_slash(), Some(Cow::Borrowed(expected.as_str()))); } } @@ -142,7 +146,10 @@ fn to_slash_lossy_pathbuf() { #[test] fn from_slash_to_slash() { for (_, path) in TO_SLASH_TESTS.iter() { - assert_eq!(PathBuf::from_slash(path).to_slash(), Some(path.clone())); + assert_eq!( + PathBuf::from_slash(path).to_slash(), + Some(Cow::Borrowed(path.as_str())) + ); } } From 36090db4fbecc7518af19489d99ec0d1147a198a Mon Sep 17 00:00:00 2001 From: rhysd Date: Sat, 2 Jul 2022 12:07:25 +0900 Subject: [PATCH 03/11] refactoring using `?` --- src/lib.rs | 40 ++++++++++------------------------------ 1 file changed, 10 insertions(+), 30 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index f9d59df..61d5ea0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -56,7 +56,7 @@ use std::borrow::Cow; use std::ffi::OsStr; -use std::path::{Path, PathBuf}; +use std::path::{Path, PathBuf, MAIN_SEPARATOR}; /// Trait to extend [`std::path::Path`]. /// @@ -127,18 +127,11 @@ impl PathExt for Path { path::Component::CurDir => buf.push('.'), path::Component::ParentDir => buf.push_str(".."), path::Component::Prefix(ref prefix) => { - let s = prefix.as_os_str(); - match s.to_str() { - Some(ref s) => buf.push_str(s), - None => buf.push_str(&s.to_string_lossy()), - } + buf.push_str(previs.as_os_str().to_string_lossy()); // C:\foo is [Prefix, RootDir, Normal]. Avoid C:// continue; } - path::Component::Normal(ref s) => match s.to_str() { - Some(ref s) => buf.push_str(s), - None => buf.push_str(&s.to_string_lossy()), - }, + path::Component::Normal(ref s) => buf.push_str(s.to_string_lossy()), } buf.push('/'); has_trailing_slash = true; @@ -206,21 +199,11 @@ impl PathExt for Path { path::Component::CurDir => buf.push('.'), path::Component::ParentDir => buf.push_str(".."), path::Component::Prefix(ref prefix) => { - if let Some(s) = prefix.as_os_str().to_str() { - buf.push_str(s); - // C:\foo is [Prefix, RootDir, Normal]. Avoid C:// - continue; - } else { - return None; - } - } - path::Component::Normal(ref s) => { - if let Some(s) = s.to_str() { - buf.push_str(s); - } else { - return None; - } + buf.push_str(prefix.as_os_str().to_str()?); + // C:\foo is [Prefix, RootDir, Normal]. Avoid C:// + continue; } + path::Component::Normal(ref s) => buf.push_str(s.to_str()?), } buf.push('/'); has_trailing_slash = true; @@ -302,13 +285,11 @@ impl PathBufExt for PathBuf { /// ``` #[cfg(target_os = "windows")] fn from_slash>(s: S) -> Self { - use std::path; - let s = s .as_ref() .chars() .map(|c| match c { - '/' => path::MAIN_SEPARATOR, + '/' => MAIN_SEPARATOR, c => c, }) .collect::(); @@ -322,16 +303,15 @@ impl PathBufExt for PathBuf { /// The replacements only happen on non-Windows. #[cfg(not(target_os = "windows"))] fn from_backslash>(s: S) -> Self { - use std::path; - let s = s .as_ref() .chars() .map(|c| match c { - '\\' => path::MAIN_SEPARATOR, + '\\' => MAIN_SEPARATOR, c => c, }) .collect::(); + PathBuf::from(s) } From a6d8c10d4d67cb907ca40286831cc03099d91a9b Mon Sep 17 00:00:00 2001 From: rhysd Date: Sat, 2 Jul 2022 12:15:25 +0900 Subject: [PATCH 04/11] fix test failures on Windows --- src/lib.rs | 10 +++++----- src/test.rs | 30 +++++++++++++++--------------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 61d5ea0..2b958d6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -126,12 +126,12 @@ impl PathExt for Path { path::Component::RootDir => { /* empty */ } path::Component::CurDir => buf.push('.'), path::Component::ParentDir => buf.push_str(".."), - path::Component::Prefix(ref prefix) => { - buf.push_str(previs.as_os_str().to_string_lossy()); + path::Component::Prefix(prefix) => { + buf.push_str(&prefix.as_os_str().to_string_lossy()); // C:\foo is [Prefix, RootDir, Normal]. Avoid C:// continue; } - path::Component::Normal(ref s) => buf.push_str(s.to_string_lossy()), + path::Component::Normal(s) => buf.push_str(&s.to_string_lossy()), } buf.push('/'); has_trailing_slash = true; @@ -198,12 +198,12 @@ impl PathExt for Path { path::Component::RootDir => { /* empty */ } path::Component::CurDir => buf.push('.'), path::Component::ParentDir => buf.push_str(".."), - path::Component::Prefix(ref prefix) => { + path::Component::Prefix(prefix) => { buf.push_str(prefix.as_os_str().to_str()?); // C:\foo is [Prefix, RootDir, Normal]. Avoid C:// continue; } - path::Component::Normal(ref s) => buf.push_str(s.to_str()?), + path::Component::Normal(s) => buf.push_str(s.to_str()?), } buf.push('/'); has_trailing_slash = true; diff --git a/src/test.rs b/src/test.rs index 015b083..c9a7169 100644 --- a/src/test.rs +++ b/src/test.rs @@ -161,8 +161,8 @@ mod windows { fn with_driver_letter_to_slash() { let path = PathBuf::from_slash("C:/foo/bar"); assert_eq!(path, PathBuf::from(r"C:\foo\bar")); - let slash = path.to_slash(); - assert_eq!(slash, Some("C:/foo/bar".to_string())); + let slash = path.to_slash().unwrap(); + assert_eq!(slash, "C:/foo/bar"); } #[test] @@ -170,7 +170,7 @@ mod windows { let path = PathBuf::from_slash("C:/foo/bar"); assert_eq!(path, PathBuf::from(r"C:\foo\bar")); let slash = path.to_slash_lossy(); - assert_eq!(slash, "C:/foo/bar".to_string()); + assert_eq!(slash, "C:/foo/bar"); } #[test] @@ -178,7 +178,7 @@ mod windows { let path = PathBuf::from_slash("C:"); assert_eq!(path, PathBuf::from(r"C:")); let slash = path.to_slash().unwrap(); - assert_eq!(slash, "C:".to_string()); + assert_eq!(slash, "C:"); } #[test] @@ -186,7 +186,7 @@ mod windows { let path = PathBuf::from_slash("C:"); assert_eq!(path, PathBuf::from(r"C:")); let slash = path.to_slash_lossy(); - assert_eq!(slash, "C:".to_string()); + assert_eq!(slash, "C:"); } #[test] @@ -194,7 +194,7 @@ mod windows { let path = PathBuf::from_slash(r"\\?\C:/foo/bar"); assert_eq!(path, PathBuf::from(r"\\?\C:\foo\bar")); let slash = path.to_slash().unwrap(); - assert_eq!(slash, r"\\?\C:/foo/bar".to_string()); + assert_eq!(slash, r"\\?\C:/foo/bar"); } #[test] @@ -202,7 +202,7 @@ mod windows { let path = PathBuf::from_slash(r"\\?\C:/foo/bar"); assert_eq!(path, PathBuf::from(r"\\?\C:\foo\bar")); let slash = path.to_slash_lossy(); - assert_eq!(slash, r"\\?\C:/foo/bar".to_string()); + assert_eq!(slash, r"\\?\C:/foo/bar"); } #[test] @@ -210,7 +210,7 @@ mod windows { let path = PathBuf::from_slash(r"\\server\share/foo/bar"); assert_eq!(path, PathBuf::from(r"\\server\share\foo\bar")); let slash = path.to_slash().unwrap(); - assert_eq!(slash, r"\\server\share/foo/bar".to_string()); + assert_eq!(slash, r"\\server\share/foo/bar"); } #[test] @@ -218,7 +218,7 @@ mod windows { let path = PathBuf::from_slash(r"\\server\share/foo/bar"); assert_eq!(path, PathBuf::from(r"\\server\share\foo\bar")); let slash = path.to_slash_lossy(); - assert_eq!(slash, r"\\server\share/foo/bar".to_string()); + assert_eq!(slash, r"\\server\share/foo/bar"); } #[test] @@ -226,7 +226,7 @@ mod windows { let path = PathBuf::from_slash(r"\\server\share"); assert_eq!(path, PathBuf::from(r"\\server\share")); let slash = path.to_slash().unwrap(); - assert_eq!(slash, r"\\server\share".to_string()); + assert_eq!(slash, r"\\server\share"); } #[test] @@ -234,7 +234,7 @@ mod windows { let path = PathBuf::from_slash(r"\\server\share"); assert_eq!(path, PathBuf::from(r"\\server\share")); let slash = path.to_slash_lossy(); - assert_eq!(slash, r"\\server\share".to_string()); + assert_eq!(slash, r"\\server\share"); } #[test] @@ -242,7 +242,7 @@ mod windows { let path = PathBuf::from_slash(r"\\?\UNC\server\share/foo/bar"); assert_eq!(path, PathBuf::from(r"\\?\UNC\server\share\foo\bar")); let slash = path.to_slash().unwrap(); - assert_eq!(slash, r"\\?\UNC\server\share/foo/bar".to_string()); + assert_eq!(slash, r"\\?\UNC\server\share/foo/bar"); } #[test] @@ -250,7 +250,7 @@ mod windows { let path = PathBuf::from_slash(r"\\?\UNC\server\share/foo/bar"); assert_eq!(path, PathBuf::from(r"\\?\UNC\server\share\foo\bar")); let slash = path.to_slash_lossy(); - assert_eq!(slash, r"\\?\UNC\server\share/foo/bar".to_string()); + assert_eq!(slash, r"\\?\UNC\server\share/foo/bar"); } #[test] @@ -258,7 +258,7 @@ mod windows { let path = PathBuf::from_slash(r"\\?\UNC\server\share"); assert_eq!(path, PathBuf::from(r"\\?\UNC\server\share")); let slash = path.to_slash().unwrap(); - assert_eq!(slash, r"\\?\UNC\server\share".to_string()); + assert_eq!(slash, r"\\?\UNC\server\share"); } #[test] @@ -266,6 +266,6 @@ mod windows { let path = PathBuf::from_slash(r"\\?\UNC\server\share"); assert_eq!(path, PathBuf::from(r"\\?\UNC\server\share")); let slash = path.to_slash_lossy(); - assert_eq!(slash, r"\\?\UNC\server\share".to_string()); + assert_eq!(slash, r"\\?\UNC\server\share"); } } From d92629430c88620820551cd723f203e5cb1f9755 Mon Sep 17 00:00:00 2001 From: rhysd Date: Sat, 2 Jul 2022 12:48:07 +0900 Subject: [PATCH 05/11] implement `CowExt` to convert slash paths to `Cow` --- src/lib.rs | 132 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 132 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index 2b958d6..3e95489 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -442,5 +442,137 @@ impl PathBufExt for PathBuf { } } +/// Trait to extend [`std::borrow::Cow`]. +/// +/// ``` +/// # use std::path::Path; +/// # use std::borrow::Cow; +/// use path_slash::{CowExt, PathExt}; +/// +/// assert_eq!( +/// Cow::from_slash("foo/bar/piyo.txt").to_slash_lossy(), +/// "foo/bar/piyo.txt", +/// ); +/// ``` +pub trait CowExt<'a> { + fn from_slash(s: &'a str) -> Self; + fn from_slash_lossy(s: &'a OsStr) -> Self; + fn from_backslash(s: &'a str) -> Self; + fn from_backslash_lossy(s: &'a OsStr) -> Self; +} + +impl<'a> CowExt<'a> for Cow<'a, Path> { + /// Convert the slash path (path separated with '/') to [`std::path::PathBuf`]. + /// + /// Any '/' in the slash path is replaced with the file path separator. + /// The replacements only happen on Windows since the file path separators on other OSes are the + /// same as '/'. + /// + /// On non-Windows OS, it is simply equivalent to [`std::path::PathBuf::from`]. + #[cfg(not(target_os = "windows"))] + fn from_slash(s: &'a str) -> Self { + Cow::Borrowed(Path::new(s)) + } + + /// Convert the slash path (path separated with '/') to [`std::path::PathBuf`]. + /// + /// Any '/' in the slash path is replaced with the file path separator. + /// The replacements only happen on Windows since the file path separators on other OSes are the + /// same as '/'. + /// + /// On non-Windows OS, it is simply equivalent to [`std::path::PathBuf::from`]. + #[cfg(target_os = "windows")] + fn from_slash(s: &'a str) -> Self { + let s = s + .chars() + .map(|c| match c { + '/' => MAIN_SEPARATOR, + c => c, + }) + .collect::(); + + Cow::Owned(PathBuf::from(s)) + } + + /// Convert the backslash path (path separated with '\\') to [`std::path::PathBuf`]. + /// + /// Any '\\' in the slash path is replaced with the file path separator. + /// The replacements only happen on non-Windows. + #[cfg(not(target_os = "windows"))] + fn from_backslash(s: &'a str) -> Self { + let s = s + .chars() + .map(|c| match c { + '\\' => MAIN_SEPARATOR, + c => c, + }) + .collect::(); + + Cow::Owned(PathBuf::from(s)) + } + + /// Convert the backslash path (path separated with '\\') to [`std::path::PathBuf`]. + /// + /// Any '\\' in the slash path is replaced with the file path separator. + /// The replacements only happen on non-Windows. + #[cfg(target_os = "windows")] + fn from_backslash(s: &'a str) -> Self { + Cow::Borrowed(Path::new(s)) + } + + /// Convert the backslash path (path separated with '\\') to [`std::path::PathBuf`]. + /// + /// Any '\\' in the slash path is replaced with the file path separator. + #[cfg(not(target_os = "windows"))] + fn from_backslash_lossy(s: &'a OsStr) -> Self { + Cow::Owned(s.to_string_lossy().replace('\\', "/").into()) + } + + /// Convert the backslash path (path separated with '\\') to [`std::path::PathBuf`]. + /// + /// Any '\\' in the slash path is replaced with the file path separator. + #[cfg(target_os = "windows")] + fn from_backslash_lossy(s: &'a OsStr) -> Self { + Cow::Borrowed(Path::new(s)) + } + + /// Convert the slash path (path separated with '/') to [`std::path::PathBuf`]. + /// + /// Any '/' in the slash path is replaced with the file path separator. + /// The replacements only happen on Windows since the file path separators on other OSes are the + /// same as '/'. + /// + /// On Windows, any non-Unicode sequences are replaced with U+FFFD while the conversion. + /// On non-Windows OS, it is simply equivalent to [`std::path::PathBuf::from`] and there is no + /// loss while conversion. + #[cfg(not(target_os = "windows"))] + fn from_slash_lossy(s: &'a OsStr) -> Self { + Cow::Borrowed(Path::new(s)) + } + + /// Convert the slash path (path separated with '/') to [`std::path::PathBuf`]. + /// + /// Any '/' in the slash path is replaced with the file path separator. + /// The replacements only happen on Windows since the file path separators on other OSes are the + /// same as '/'. + /// + /// On Windows, any non-Unicode sequences are replaced with U+FFFD while the conversion. + /// On non-Windows OS, it is simply equivalent to [`std::path::PathBuf::from`] and there is no + /// loss while conversion. + #[cfg(target_os = "windows")] + fn from_slash_lossy(s: &'a OsStr) -> Self { + let s = s + .to_string_lossy() + .chars() + .map(|c| match c { + '/' => MAIN_SEPARATOR, + c => c, + }) + .collect::(); + + Cow::Owned(PathBuf::from(s)) + } +} + #[cfg(test)] mod test; From 486eb02701df32a1e1dbbc6a5180a349efcd9c8e Mon Sep 17 00:00:00 2001 From: rhysd Date: Sat, 2 Jul 2022 15:08:24 +0900 Subject: [PATCH 06/11] reduce heap allocations on converting to `Cow` --- src/lib.rs | 116 +++++++++++++++++++++++++---------------------------- 1 file changed, 55 insertions(+), 61 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 3e95489..2c550b7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -58,6 +58,37 @@ use std::borrow::Cow; use std::ffi::OsStr; use std::path::{Path, PathBuf, MAIN_SEPARATOR}; +fn str_to_path(s: &str, sep: char) -> Cow<'_, Path> { + let mut buf = String::new(); + + for (i, c) in s.char_indices() { + if c == sep { + if buf.is_empty() { + buf.reserve(s.len()); + buf.push_str(&s[..i]); + } + buf.push(MAIN_SEPARATOR); + } else if !buf.is_empty() { + buf.push(c); + } + } + + if buf.is_empty() { + Cow::Borrowed(Path::new(s)) + } else { + Cow::Owned(PathBuf::from(buf)) + } +} + +fn str_to_pathbuf>(s: S, sep: char) -> PathBuf { + let s = s + .as_ref() + .chars() + .map(|c| if c == sep { MAIN_SEPARATOR } else { c }) + .collect::(); + PathBuf::from(s) +} + /// Trait to extend [`std::path::Path`]. /// /// ``` @@ -117,21 +148,21 @@ impl PathExt for Path { /// ``` #[cfg(target_os = "windows")] fn to_slash_lossy(&self) -> Cow<'_, str> { - use std::path; + use std::path::Component; let mut buf = String::new(); let mut has_trailing_slash = false; for c in self.components() { match c { - path::Component::RootDir => { /* empty */ } - path::Component::CurDir => buf.push('.'), - path::Component::ParentDir => buf.push_str(".."), - path::Component::Prefix(prefix) => { + Component::RootDir => { /* empty */ } + Component::CurDir => buf.push('.'), + Component::ParentDir => buf.push_str(".."), + Component::Prefix(prefix) => { buf.push_str(&prefix.as_os_str().to_string_lossy()); // C:\foo is [Prefix, RootDir, Normal]. Avoid C:// continue; } - path::Component::Normal(s) => buf.push_str(&s.to_string_lossy()), + Component::Normal(s) => buf.push_str(&s.to_string_lossy()), } buf.push('/'); has_trailing_slash = true; @@ -189,21 +220,21 @@ impl PathExt for Path { /// ``` #[cfg(target_os = "windows")] fn to_slash(&self) -> Option> { - use std::path; + use std::path::Component; let mut buf = String::new(); let mut has_trailing_slash = false; for c in self.components() { match c { - path::Component::RootDir => { /* empty */ } - path::Component::CurDir => buf.push('.'), - path::Component::ParentDir => buf.push_str(".."), - path::Component::Prefix(prefix) => { + Component::RootDir => { /* empty */ } + Component::CurDir => buf.push('.'), + Component::ParentDir => buf.push_str(".."), + Component::Prefix(prefix) => { buf.push_str(prefix.as_os_str().to_str()?); // C:\foo is [Prefix, RootDir, Normal]. Avoid C:// continue; } - path::Component::Normal(s) => buf.push_str(s.to_str()?), + Component::Normal(s) => buf.push_str(s.to_str()?), } buf.push('/'); has_trailing_slash = true; @@ -285,16 +316,7 @@ impl PathBufExt for PathBuf { /// ``` #[cfg(target_os = "windows")] fn from_slash>(s: S) -> Self { - let s = s - .as_ref() - .chars() - .map(|c| match c { - '/' => MAIN_SEPARATOR, - c => c, - }) - .collect::(); - - PathBuf::from(s) + str_to_pathbuf(s, '/') } /// Convert the backslash path (path separated with '\\') to [`std::path::PathBuf`]. @@ -303,16 +325,7 @@ impl PathBufExt for PathBuf { /// The replacements only happen on non-Windows. #[cfg(not(target_os = "windows"))] fn from_backslash>(s: S) -> Self { - let s = s - .as_ref() - .chars() - .map(|c| match c { - '\\' => MAIN_SEPARATOR, - c => c, - }) - .collect::(); - - PathBuf::from(s) + str_to_pathbuf(s, '\\') } /// Convert the backslash path (path separated with '\\') to [`std::path::PathBuf`]. @@ -483,15 +496,7 @@ impl<'a> CowExt<'a> for Cow<'a, Path> { /// On non-Windows OS, it is simply equivalent to [`std::path::PathBuf::from`]. #[cfg(target_os = "windows")] fn from_slash(s: &'a str) -> Self { - let s = s - .chars() - .map(|c| match c { - '/' => MAIN_SEPARATOR, - c => c, - }) - .collect::(); - - Cow::Owned(PathBuf::from(s)) + str_to_path(s, '/') } /// Convert the backslash path (path separated with '\\') to [`std::path::PathBuf`]. @@ -500,15 +505,7 @@ impl<'a> CowExt<'a> for Cow<'a, Path> { /// The replacements only happen on non-Windows. #[cfg(not(target_os = "windows"))] fn from_backslash(s: &'a str) -> Self { - let s = s - .chars() - .map(|c| match c { - '\\' => MAIN_SEPARATOR, - c => c, - }) - .collect::(); - - Cow::Owned(PathBuf::from(s)) + str_to_path(s, '\\') } /// Convert the backslash path (path separated with '\\') to [`std::path::PathBuf`]. @@ -525,7 +522,10 @@ impl<'a> CowExt<'a> for Cow<'a, Path> { /// Any '\\' in the slash path is replaced with the file path separator. #[cfg(not(target_os = "windows"))] fn from_backslash_lossy(s: &'a OsStr) -> Self { - Cow::Owned(s.to_string_lossy().replace('\\', "/").into()) + match s.to_string_lossy() { + Cow::Borrowed(s) => str_to_path(s, '\\'), + Cow::Owned(s) => Cow::Owned(str_to_pathbuf(&s, '\\')), + } } /// Convert the backslash path (path separated with '\\') to [`std::path::PathBuf`]. @@ -561,16 +561,10 @@ impl<'a> CowExt<'a> for Cow<'a, Path> { /// loss while conversion. #[cfg(target_os = "windows")] fn from_slash_lossy(s: &'a OsStr) -> Self { - let s = s - .to_string_lossy() - .chars() - .map(|c| match c { - '/' => MAIN_SEPARATOR, - c => c, - }) - .collect::(); - - Cow::Owned(PathBuf::from(s)) + match s.to_string_lossy() { + Cow::Borrowed(s) => str_to_path(s, '/'), + Cow::Owned(s) => Cow::Owned(str_to_pathbuf(&s, '/')), + } } } From f0ec925d37f58b201e3b47aa6309dadffce14236 Mon Sep 17 00:00:00 2001 From: rhysd Date: Sat, 2 Jul 2022 15:16:13 +0900 Subject: [PATCH 07/11] add tests for methods in `CowExt` --- src/test.rs | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/src/test.rs b/src/test.rs index c9a7169..e2c3ad4 100644 --- a/src/test.rs +++ b/src/test.rs @@ -1,4 +1,4 @@ -use super::{PathBufExt as _, PathExt as _}; +use super::{CowExt as _, PathBufExt as _, PathExt as _}; use lazy_static::lazy_static; use std::borrow::Cow; use std::ffi::OsStr; @@ -77,6 +77,38 @@ fn from_backslash_lossy() { } } +#[test] +fn cow_from_slash() { + for (input, expected) in FROM_SLASH_TESTS.iter() { + assert_eq!(&Cow::from_slash(input), expected); + } +} + +#[test] +fn cow_from_slash_lossy() { + for (input, expected) in FROM_SLASH_TESTS.iter() { + let input: &OsStr = input.as_ref(); + assert_eq!(&Cow::from_slash_lossy(input), expected); + } +} + +#[test] +fn cow_from_backslash() { + for (input, expected) in FROM_SLASH_TESTS.iter() { + let input = input.replace('/', r"\"); + assert_eq!(&Cow::from_backslash(&input), expected); + } +} + +#[test] +fn cow_from_backslash_lossy() { + for (input, expected) in FROM_SLASH_TESTS.iter() { + let input = input.replace('/', r"\"); + let input: &OsStr = input.as_ref(); + assert_eq!(&Cow::from_backslash_lossy(input), expected); + } +} + lazy_static! { static ref TO_SLASH_TESTS: Vec<(PathBuf, String)> = { [ From 95223f57d61baea3b1a9676a19eff9b0915a39b6 Mon Sep 17 00:00:00 2001 From: rhysd Date: Sat, 2 Jul 2022 17:24:52 +0900 Subject: [PATCH 08/11] fix `CowExt` documents --- src/lib.rs | 283 +++++++++++++++++++++++++++++++++-------------------- 1 file changed, 179 insertions(+), 104 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 2c550b7..08f93d8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,10 +1,10 @@ -//! A library for converting file paths to and from "slash paths." +//! A library for converting file paths to and from "slash paths". //! //! A "slash path" is a path whose components are always separated by `/` and never `\`. //! //! On Unix-like OSes, the path separator is `/`. So any conversion is not necessary. -//! But on Windows, the file path separator is `\`, and needs to be replaced with `/`. Of course, `\`s used -//! for escaping characters should not be replaced. +//! But on Windows, the file path separator is `\`, and needs to be replaced with `/` for converting +//! the paths to "slash paths". Of course, `\`s used for escaping characters should not be replaced. //! //! For example, a file path `foo\bar\piyo.txt` can be converted to/from a slash path `foo/bar/piyo.txt`. //! @@ -13,42 +13,49 @@ //! //! ```rust //! use std::path::{Path, PathBuf}; +//! use std::borrow::Cow; //! //! // Trait for extending std::path::Path -//! use path_slash::PathExt; +//! use path_slash::PathExt as _; //! // Trait for extending std::path::PathBuf -//! use path_slash::PathBufExt; +//! use path_slash::PathBufExt as _; +//! // Trait for extending std::borrow::Cow +//! use path_slash::CowExt as _; //! //! #[cfg(target_os = "windows")] //! { +//! // Convert from `Path` //! assert_eq!( //! Path::new(r"foo\bar\piyo.txt").to_slash().unwrap(), //! "foo/bar/piyo.txt", //! ); -//! assert_eq!( -//! Path::new(r"C:\foo\bar\piyo.txt").to_slash().unwrap(), -//! "C:/foo/bar/piyo.txt", -//! ); //! +//! // Convert to/from PathBuf //! let p = PathBuf::from_slash("foo/bar/piyo.txt"); //! assert_eq!(p, PathBuf::from(r"foo\bar\piyo.txt")); //! assert_eq!(p.to_slash().unwrap(), "foo/bar/piyo.txt"); +//! +//! // Convert to Cow<'_, Path> +//! let p = Cow::from_slash("foo/bar/piyo.txt"); +//! assert_eq!(p, Cow::Owned(PathBuf::from(r"foo\bar\piyo.txt"))); //! } //! //! #[cfg(not(target_os = "windows"))] //! { +//! // Convert from `Path` //! assert_eq!( //! Path::new("foo/bar/piyo.txt").to_slash().unwrap(), //! "foo/bar/piyo.txt", //! ); -//! assert_eq!( -//! Path::new("/foo/bar/piyo.txt").to_slash().unwrap(), -//! "/foo/bar/piyo.txt", -//! ); //! +//! // Convert to/from PathBuf //! let p = PathBuf::from_slash("foo/bar/piyo.txt"); //! assert_eq!(p, PathBuf::from("foo/bar/piyo.txt")); //! assert_eq!(p.to_slash().unwrap(), "foo/bar/piyo.txt"); +//! +//! // Convert to Cow<'_, Path> +//! let p = Cow::from_slash("foo/bar/piyo.txt"); +//! assert_eq!(p, Cow::Borrowed(Path::new("foo/bar/piyo.txt"))); //! } //! ``` #![forbid(unsafe_code)] @@ -94,7 +101,7 @@ fn str_to_pathbuf>(s: S, sep: char) -> PathBuf { /// ``` /// # use std::path::Path; /// # use std::borrow::Cow; -/// use path_slash::PathExt; +/// use path_slash::PathExt as _; /// /// assert_eq!( /// Path::new("foo").to_slash(), @@ -114,7 +121,7 @@ impl PathExt for Path { /// /// ``` /// # use std::path::Path; - /// use path_slash::PathExt; + /// use path_slash::PathExt as _; /// /// #[cfg(target_os = "windows")] /// let s = Path::new(r"foo\bar\piyo.txt"); @@ -136,7 +143,7 @@ impl PathExt for Path { /// /// ``` /// # use std::path::Path; - /// use path_slash::PathExt; + /// use path_slash::PathExt as _; /// /// #[cfg(target_os = "windows")] /// let s = Path::new(r"foo\bar\piyo.txt"); @@ -183,7 +190,7 @@ impl PathExt for Path { /// ``` /// # use std::path::Path; /// # use std::borrow::Cow; - /// use path_slash::PathExt; + /// use path_slash::PathExt as _; /// /// #[cfg(target_os = "windows")] /// let s = Path::new(r"foo\bar\piyo.txt"); @@ -208,7 +215,7 @@ impl PathExt for Path { /// ``` /// # use std::path::Path; /// # use std::borrow::Cow; - /// use path_slash::PathExt; + /// use path_slash::PathExt as _; /// /// #[cfg(target_os = "windows")] /// let s = Path::new(r"foo\bar\piyo.txt"); @@ -319,41 +326,7 @@ impl PathBufExt for PathBuf { str_to_pathbuf(s, '/') } - /// Convert the backslash path (path separated with '\\') to [`std::path::PathBuf`]. - /// - /// Any '\\' in the slash path is replaced with the file path separator. - /// The replacements only happen on non-Windows. - #[cfg(not(target_os = "windows"))] - fn from_backslash>(s: S) -> Self { - str_to_pathbuf(s, '\\') - } - - /// Convert the backslash path (path separated with '\\') to [`std::path::PathBuf`]. - /// - /// Any '\\' in the slash path is replaced with the file path separator. - /// The replacements only happen on non-Windows. - #[cfg(target_os = "windows")] - fn from_backslash>(s: S) -> Self { - PathBuf::from(s.as_ref()) - } - - /// Convert the backslash path (path separated with '\\') to [`std::path::PathBuf`]. - /// - /// Any '\\' in the slash path is replaced with the file path separator. - #[cfg(not(target_os = "windows"))] - fn from_backslash_lossy>(s: S) -> Self { - s.as_ref().to_string_lossy().replace('\\', "/").into() - } - - /// Convert the backslash path (path separated with '\\') to [`std::path::PathBuf`]. - /// - /// Any '\\' in the slash path is replaced with the file path separator. - #[cfg(target_os = "windows")] - fn from_backslash_lossy>(s: S) -> Self { - PathBuf::from(s.as_ref()) - } - - /// Convert the slash path (path separated with '/') to [`std::path::PathBuf`]. + /// Convert the [`OsStr`] slash path (path separated with '/') to [`std::path::PathBuf`]. /// /// Any '/' in the slash path is replaced with the file path separator. /// The replacements only happen on Windows since the file path separators on other OSes are the @@ -382,7 +355,7 @@ impl PathBufExt for PathBuf { PathBuf::from(s.as_ref()) } - /// Convert the slash path (path separated with '/') to [`std::path::PathBuf`]. + /// Convert the [`OsStr`] slash path (path separated with '/') to [`std::path::PathBuf`]. /// /// Any '/' in the slash path is replaced with the file path separator. /// The replacements only happen on Windows since the file path separators on other OSes are the @@ -411,6 +384,40 @@ impl PathBufExt for PathBuf { Self::from_slash(&s.as_ref().to_string_lossy()) } + /// Convert the backslash path (path separated with '\\') to [`std::path::PathBuf`]. + /// + /// Any '\\' in the slash path is replaced with the file path separator. + /// The replacements only happen on non-Windows. + #[cfg(not(target_os = "windows"))] + fn from_backslash>(s: S) -> Self { + str_to_pathbuf(s, '\\') + } + + /// Convert the backslash path (path separated with '\\') to [`std::path::PathBuf`]. + /// + /// Any '\\' in the slash path is replaced with the file path separator. + /// The replacements only happen on non-Windows. + #[cfg(target_os = "windows")] + fn from_backslash>(s: S) -> Self { + PathBuf::from(s.as_ref()) + } + + /// Convert the [`OsStr`] backslash path (path separated with '\\') to [`std::path::PathBuf`]. + /// + /// Any '\\' in the slash path is replaced with the file path separator. + #[cfg(not(target_os = "windows"))] + fn from_backslash_lossy>(s: S) -> Self { + s.as_ref().to_string_lossy().replace('\\', "/").into() + } + + /// Convert the [`OsStr`] backslash path (path separated with '\\') to [`std::path::PathBuf`]. + /// + /// Any '\\' in the slash path is replaced with the file path separator. + #[cfg(target_os = "windows")] + fn from_backslash_lossy>(s: S) -> Self { + PathBuf::from(s.as_ref()) + } + /// Convert the file path into slash path as UTF-8 string. /// /// Any file path separators in the file path is replaced with '/'. @@ -460,7 +467,7 @@ impl PathBufExt for PathBuf { /// ``` /// # use std::path::Path; /// # use std::borrow::Cow; -/// use path_slash::{CowExt, PathExt}; +/// use path_slash::{CowExt as _, PathExt as _}; /// /// assert_eq!( /// Cow::from_slash("foo/bar/piyo.txt").to_slash_lossy(), @@ -475,51 +482,149 @@ pub trait CowExt<'a> { } impl<'a> CowExt<'a> for Cow<'a, Path> { - /// Convert the slash path (path separated with '/') to [`std::path::PathBuf`]. + /// Convert the slash path (path separated with '/') to [`Cow`]. /// /// Any '/' in the slash path is replaced with the file path separator. - /// The replacements only happen on Windows since the file path separators on other OSes are the - /// same as '/'. + /// Heap allocation may only happen on Windows since the file path separators on other OSes are + /// the same as '/'. /// - /// On non-Windows OS, it is simply equivalent to [`std::path::PathBuf::from`]. + /// ``` + /// # use std::borrow::Cow; + /// # use std::path::Path; + /// use path_slash::CowExt; + /// + /// #[cfg(not(target_os = "windows"))] + /// assert_eq!( + /// Cow::from_slash("foo/bar/piyo.txt"), + /// Path::new("foo/bar/piyo.txt"), + /// ); + /// + /// #[cfg(target_os = "windows")] + /// assert_eq!( + /// Cow::from_slash("foo/bar/piyo.txt"), + /// Path::new(r"foo\\bar\\piyo.txt"), + /// ); + /// ``` #[cfg(not(target_os = "windows"))] fn from_slash(s: &'a str) -> Self { Cow::Borrowed(Path::new(s)) } - /// Convert the slash path (path separated with '/') to [`std::path::PathBuf`]. + /// Convert the slash path (path separated with '/') to [`Cow`]. /// /// Any '/' in the slash path is replaced with the file path separator. - /// The replacements only happen on Windows since the file path separators on other OSes are the - /// same as '/'. + /// Heap allocation may only happen on Windows since the file path separators on other OSes are + /// the same as '/'. /// - /// On non-Windows OS, it is simply equivalent to [`std::path::PathBuf::from`]. + /// ``` + /// # use std::borrow::Cow; + /// # use std::path::Path; + /// use path_slash::CowExt; + /// + /// #[cfg(not(target_os = "windows"))] + /// assert_eq!( + /// Cow::from_slash("foo/bar/piyo.txt"), + /// Path::new("foo/bar/piyo.txt"), + /// ); + /// + /// #[cfg(target_os = "windows")] + /// assert_eq!( + /// Cow::from_slash("foo/bar/piyo.txt"), + /// Path::new(r"foo\\bar\\piyo.txt"), + /// ); + /// ``` #[cfg(target_os = "windows")] fn from_slash(s: &'a str) -> Self { str_to_path(s, '/') } - /// Convert the backslash path (path separated with '\\') to [`std::path::PathBuf`]. + /// Convert the [`OsStr`] slash path (path separated with '/') to [`Cow`]. /// - /// Any '\\' in the slash path is replaced with the file path separator. - /// The replacements only happen on non-Windows. + /// Any '/' in the slash path is replaced with the file path separator. + /// Heap allocation may only happen on Windows since the file path separators on other OSes are + /// the same as '/'. + /// + /// On Windows, any non-Unicode sequences are replaced with U+FFFD while the conversion. + /// On non-Windows OS, there is no loss while conversion. + #[cfg(not(target_os = "windows"))] + fn from_slash_lossy(s: &'a OsStr) -> Self { + Cow::Borrowed(Path::new(s)) + } + + /// Convert the [`OsStr`] slash path (path separated with '/') to [`Cow`]. + /// + /// Any '/' in the slash path is replaced with the file path separator. + /// Heap allocation may only happen on Windows since the file path separators on other OSes are + /// the same as '/'. + /// + /// On Windows, any non-Unicode sequences are replaced with U+FFFD while the conversion. + /// On non-Windows OS, there is no loss while conversion. + #[cfg(target_os = "windows")] + fn from_slash_lossy(s: &'a OsStr) -> Self { + match s.to_string_lossy() { + Cow::Borrowed(s) => str_to_path(s, '/'), + Cow::Owned(s) => Cow::Owned(str_to_pathbuf(&s, '/')), + } + } + + /// Convert the backslash path (path separated with '\\') to [`Cow`]. + /// + /// Any '\\' in the slash path is replaced with the file path separator. Heap allocation may + /// only happen on non-Windows. + /// + /// ``` + /// # use std::borrow::Cow; + /// # use std::path::Path; + /// use path_slash::CowExt; + /// + /// #[cfg(not(target_os = "windows"))] + /// assert_eq!( + /// Cow::from_backslash(r"foo\\bar\\piyo.txt"), + /// Path::new("foo/bar/piyo.txt"), + /// ); + /// + /// #[cfg(target_os = "windows")] + /// assert_eq!( + /// Cow::from_backslash(r"foo\\bar\\piyo.txt"), + /// Path::new(r"foo\\bar\\piyo.txt"), + /// ); + /// ``` #[cfg(not(target_os = "windows"))] fn from_backslash(s: &'a str) -> Self { str_to_path(s, '\\') } - /// Convert the backslash path (path separated with '\\') to [`std::path::PathBuf`]. + /// Convert the backslash path (path separated with '\\') to [`Cow`]. /// - /// Any '\\' in the slash path is replaced with the file path separator. - /// The replacements only happen on non-Windows. + /// Any '\\' in the slash path is replaced with the file path separator. Heap allocation may + /// only happen on non-Windows. + /// + /// ``` + /// # use std::borrow::Cow; + /// # use std::path::Path; + /// use path_slash::CowExt; + /// + /// #[cfg(not(target_os = "windows"))] + /// assert_eq!( + /// Cow::from_backslash(r"foo\\bar\\piyo.txt"), + /// Path::new("foo/bar/piyo.txt"), + /// ); + /// + /// #[cfg(target_os = "windows")] + /// assert_eq!( + /// Cow::from_backslash(r"foo\\bar\\piyo.txt"), + /// Path::new(r"foo\\bar\\piyo.txt"), + /// ); + /// ``` #[cfg(target_os = "windows")] fn from_backslash(s: &'a str) -> Self { Cow::Borrowed(Path::new(s)) } - /// Convert the backslash path (path separated with '\\') to [`std::path::PathBuf`]. + /// Convert the [`OsStr`] backslash path (path separated with '\\') to [`Cow`]. /// - /// Any '\\' in the slash path is replaced with the file path separator. + /// Any '\\' in the slash path is replaced with the file path separator. Heap allocation may + /// only happen on non-Windows. #[cfg(not(target_os = "windows"))] fn from_backslash_lossy(s: &'a OsStr) -> Self { match s.to_string_lossy() { @@ -528,44 +633,14 @@ impl<'a> CowExt<'a> for Cow<'a, Path> { } } - /// Convert the backslash path (path separated with '\\') to [`std::path::PathBuf`]. + /// Convert the [`OsStr`] backslash path (path separated with '\\') to [`Cow`]. /// - /// Any '\\' in the slash path is replaced with the file path separator. + /// Any '\\' in the slash path is replaced with the file path separator. Heap allocation may + /// only happen on non-Windows. #[cfg(target_os = "windows")] fn from_backslash_lossy(s: &'a OsStr) -> Self { Cow::Borrowed(Path::new(s)) } - - /// Convert the slash path (path separated with '/') to [`std::path::PathBuf`]. - /// - /// Any '/' in the slash path is replaced with the file path separator. - /// The replacements only happen on Windows since the file path separators on other OSes are the - /// same as '/'. - /// - /// On Windows, any non-Unicode sequences are replaced with U+FFFD while the conversion. - /// On non-Windows OS, it is simply equivalent to [`std::path::PathBuf::from`] and there is no - /// loss while conversion. - #[cfg(not(target_os = "windows"))] - fn from_slash_lossy(s: &'a OsStr) -> Self { - Cow::Borrowed(Path::new(s)) - } - - /// Convert the slash path (path separated with '/') to [`std::path::PathBuf`]. - /// - /// Any '/' in the slash path is replaced with the file path separator. - /// The replacements only happen on Windows since the file path separators on other OSes are the - /// same as '/'. - /// - /// On Windows, any non-Unicode sequences are replaced with U+FFFD while the conversion. - /// On non-Windows OS, it is simply equivalent to [`std::path::PathBuf::from`] and there is no - /// loss while conversion. - #[cfg(target_os = "windows")] - fn from_slash_lossy(s: &'a OsStr) -> Self { - match s.to_string_lossy() { - Cow::Borrowed(s) => str_to_path(s, '/'), - Cow::Owned(s) => Cow::Owned(str_to_pathbuf(&s, '/')), - } - } } #[cfg(test)] From 9c041e9a765ea3470fc6abc6407fa494931c2865 Mon Sep 17 00:00:00 2001 From: rhysd Date: Sun, 3 Jul 2022 01:09:50 +0900 Subject: [PATCH 09/11] fix readme document --- README.md | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 660c218..a5e443e 100644 --- a/README.md +++ b/README.md @@ -16,19 +16,24 @@ and [`path/filepath.ToSlash`](https://golang.org/pkg/path/filepath/#ToSlash). ## Usage -`path_slash::PathExt` and `path_slash::PathBufExt` traits are defined. By using them, `std::path::Path` -and `std::path::PathBuf` gains some methods and associated functions +`path_slash::PathExt` and `path_slash::PathBufExt` traits are defined. By using them, `std::path::Path`, +`std::path::PathBuf` and `Cow<'_, Path>` gain some methods and associated functions. - `PathExt` - - `Path::to_slash(&self) -> Option` - - `Path::to_slash_lossy(&self) -> String` + - `Path::to_slash(&self) -> Option>` + - `Path::to_slash_lossy(&self) -> Cow<'_, Path>` - `PathBufExt` - `PathBuf::from_slash>(s: S) -> PathBuf` - `PathBuf::from_slash_lossy>(s: S) -> PathBuf` - `PathBuf::from_backslash>(s: S) -> PathBuf` - `PathBuf::from_backslash_lossy>(s: S) -> PathBuf` - - `PathBuf::to_slash(&self) -> Option` - - `PathBuf::to_slash_lossy(&self) -> String` + - `PathBuf::to_slash(&self) -> Option>` + - `PathBuf::to_slash_lossy(&self) -> Cow<'_, Path>` +- `CowExt` + - `fn from_slash(s: &str) -> Self` + - `fn from_slash_lossy(s: &OsStr) -> Self` + - `fn from_backslash(s: &str) -> Self` + - `fn from_backslash_lossy(s: &OsStr) -> Self` ```rust fn example_path_ext() { @@ -40,10 +45,6 @@ fn example_path_ext() { Path::new(r"foo\bar\piyo.txt").to_slash(), Some("foo/bar/piyo.txt".to_string()), ); - assert_eq!( - Path::new(r"C:\foo\bar\piyo.txt").to_slash(), - Some("C:/foo/bar/piyo.txt".to_string()), - ); } fn example_pathbuf_ext() { @@ -53,7 +54,18 @@ fn example_pathbuf_ext() { // On Windows let p = PathBuf::from_slash("foo/bar/piyo.txt"); assert_eq!(p, PathBuf::from(r"foo\bar\piyo.txt")); - assert_eq!(p.to_slash(), Some("foo/bar/piyo.txt".to_string())); + assert_eq!(p.to_slash().unwrap(), "foo/bar/piyo.txt".to_string()); +} + +fn example_cow_ext() { + // Trait for extending std::borrow::Cow<'_, Path> + use path_slash::CowExt as _; + + let p = Cow::from_slash("foo/bar/piyo.txt"); + // On Windows + assert_eq!(p, Cow::Owned(PathBuf::from(r"foo\bar\piyo.txt"))); + // On non-Windows + assert_eq!(p, Cow::Borrowed(Path::new("foo/bar/piyo.txt"))); } ``` From 39cb6255acb5b7acfd67a725f59adb84c87003d0 Mon Sep 17 00:00:00 2001 From: rhysd Date: Sun, 3 Jul 2022 01:17:18 +0900 Subject: [PATCH 10/11] remove duplicate doc comments --- src/lib.rs | 157 +---------------------------------------------------- 1 file changed, 1 insertion(+), 156 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 08f93d8..1beeb6c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -135,24 +135,6 @@ impl PathExt for Path { fn to_slash_lossy(&self) -> Cow<'_, str> { self.to_string_lossy() } - - /// Convert the file path into slash path as UTF-8 string. - /// - /// Any file path separators in the file path is replaced with '/'. - /// Any non-Unicode sequences are replaced with U+FFFD. - /// - /// ``` - /// # use std::path::Path; - /// use path_slash::PathExt as _; - /// - /// #[cfg(target_os = "windows")] - /// let s = Path::new(r"foo\bar\piyo.txt"); - /// - /// #[cfg(not(target_os = "windows"))] - /// let s = Path::new("foo/bar/piyo.txt"); - /// - /// assert_eq!(s.to_slash_lossy(), "foo/bar/piyo.txt"); - /// ``` #[cfg(target_os = "windows")] fn to_slash_lossy(&self) -> Cow<'_, str> { use std::path::Component; @@ -204,27 +186,6 @@ impl PathExt for Path { fn to_slash(&self) -> Option> { self.to_str().map(Cow::Borrowed) } - - /// Convert the file path into slash path as UTF-8 string. - /// - /// Any file path separators in the file path is replaced with '/'. - /// When the path contains non-Unicode sequence, this method returns None. - /// - /// On non-Windows OS, it is equivalent to `.to_str().map(str::to_string)` - /// - /// ``` - /// # use std::path::Path; - /// # use std::borrow::Cow; - /// use path_slash::PathExt as _; - /// - /// #[cfg(target_os = "windows")] - /// let s = Path::new(r"foo\bar\piyo.txt"); - /// - /// #[cfg(not(target_os = "windows"))] - /// let s = Path::new("foo/bar/piyo.txt"); - /// - /// assert_eq!(s.to_slash(), Some(Cow::Borrowed("foo/bar/piyo.txt"))); - /// ``` #[cfg(target_os = "windows")] fn to_slash(&self) -> Option> { use std::path::Component; @@ -300,27 +261,6 @@ impl PathBufExt for PathBuf { fn from_slash>(s: S) -> Self { PathBuf::from(s.as_ref()) } - - /// Convert the slash path (path separated with '/') to [`std::path::PathBuf`]. - /// - /// Any '/' in the slash path is replaced with the file path separator. - /// The replacements only happen on Windows since the file path separators on other OSes are the - /// same as '/'. - /// - /// On non-Windows OS, it is simply equivalent to [`std::path::PathBuf::from`]. - /// - /// ``` - /// # use std::path::PathBuf; - /// use path_slash::PathBufExt; - /// - /// let p = PathBuf::from_slash("foo/bar/piyo.txt"); - /// - /// #[cfg(target_os = "windows")] - /// assert_eq!(p, PathBuf::from(r"foo\bar\piyo.txt")); - /// - /// #[cfg(not(target_os = "windows"))] - /// assert_eq!(p, PathBuf::from("foo/bar/piyo.txt")); - /// ``` #[cfg(target_os = "windows")] fn from_slash>(s: S) -> Self { str_to_pathbuf(s, '/') @@ -338,7 +278,7 @@ impl PathBufExt for PathBuf { /// /// ``` /// # use std::path::PathBuf; - /// use std::ffi::OsStr; + /// # use std::ffi::OsStr; /// use path_slash::PathBufExt; /// /// let s: &OsStr = "foo/bar/piyo.txt".as_ref(); @@ -354,31 +294,6 @@ impl PathBufExt for PathBuf { fn from_slash_lossy>(s: S) -> Self { PathBuf::from(s.as_ref()) } - - /// Convert the [`OsStr`] slash path (path separated with '/') to [`std::path::PathBuf`]. - /// - /// Any '/' in the slash path is replaced with the file path separator. - /// The replacements only happen on Windows since the file path separators on other OSes are the - /// same as '/'. - /// - /// On Windows, any non-Unicode sequences are replaced with U+FFFD while the conversion. - /// On non-Windows OS, it is simply equivalent to [`std::path::PathBuf::from`] and there is no - /// loss while conversion. - /// - /// ``` - /// # use std::path::PathBuf; - /// use std::ffi::OsStr; - /// use path_slash::PathBufExt; - /// - /// let s: &OsStr = "foo/bar/piyo.txt".as_ref(); - /// let p = PathBuf::from_slash_lossy(s); - /// - /// #[cfg(target_os = "windows")] - /// assert_eq!(p, PathBuf::from(r"foo\bar\piyo.txt")); - /// - /// #[cfg(not(target_os = "windows"))] - /// assert_eq!(p, PathBuf::from("foo/bar/piyo.txt")); - /// ``` #[cfg(target_os = "windows")] fn from_slash_lossy>(s: S) -> Self { Self::from_slash(&s.as_ref().to_string_lossy()) @@ -392,11 +307,6 @@ impl PathBufExt for PathBuf { fn from_backslash>(s: S) -> Self { str_to_pathbuf(s, '\\') } - - /// Convert the backslash path (path separated with '\\') to [`std::path::PathBuf`]. - /// - /// Any '\\' in the slash path is replaced with the file path separator. - /// The replacements only happen on non-Windows. #[cfg(target_os = "windows")] fn from_backslash>(s: S) -> Self { PathBuf::from(s.as_ref()) @@ -409,10 +319,6 @@ impl PathBufExt for PathBuf { fn from_backslash_lossy>(s: S) -> Self { s.as_ref().to_string_lossy().replace('\\', "/").into() } - - /// Convert the [`OsStr`] backslash path (path separated with '\\') to [`std::path::PathBuf`]. - /// - /// Any '\\' in the slash path is replaced with the file path separator. #[cfg(target_os = "windows")] fn from_backslash_lossy>(s: S) -> Self { PathBuf::from(s.as_ref()) @@ -509,30 +415,6 @@ impl<'a> CowExt<'a> for Cow<'a, Path> { fn from_slash(s: &'a str) -> Self { Cow::Borrowed(Path::new(s)) } - - /// Convert the slash path (path separated with '/') to [`Cow`]. - /// - /// Any '/' in the slash path is replaced with the file path separator. - /// Heap allocation may only happen on Windows since the file path separators on other OSes are - /// the same as '/'. - /// - /// ``` - /// # use std::borrow::Cow; - /// # use std::path::Path; - /// use path_slash::CowExt; - /// - /// #[cfg(not(target_os = "windows"))] - /// assert_eq!( - /// Cow::from_slash("foo/bar/piyo.txt"), - /// Path::new("foo/bar/piyo.txt"), - /// ); - /// - /// #[cfg(target_os = "windows")] - /// assert_eq!( - /// Cow::from_slash("foo/bar/piyo.txt"), - /// Path::new(r"foo\\bar\\piyo.txt"), - /// ); - /// ``` #[cfg(target_os = "windows")] fn from_slash(s: &'a str) -> Self { str_to_path(s, '/') @@ -550,15 +432,6 @@ impl<'a> CowExt<'a> for Cow<'a, Path> { fn from_slash_lossy(s: &'a OsStr) -> Self { Cow::Borrowed(Path::new(s)) } - - /// Convert the [`OsStr`] slash path (path separated with '/') to [`Cow`]. - /// - /// Any '/' in the slash path is replaced with the file path separator. - /// Heap allocation may only happen on Windows since the file path separators on other OSes are - /// the same as '/'. - /// - /// On Windows, any non-Unicode sequences are replaced with U+FFFD while the conversion. - /// On non-Windows OS, there is no loss while conversion. #[cfg(target_os = "windows")] fn from_slash_lossy(s: &'a OsStr) -> Self { match s.to_string_lossy() { @@ -593,29 +466,6 @@ impl<'a> CowExt<'a> for Cow<'a, Path> { fn from_backslash(s: &'a str) -> Self { str_to_path(s, '\\') } - - /// Convert the backslash path (path separated with '\\') to [`Cow`]. - /// - /// Any '\\' in the slash path is replaced with the file path separator. Heap allocation may - /// only happen on non-Windows. - /// - /// ``` - /// # use std::borrow::Cow; - /// # use std::path::Path; - /// use path_slash::CowExt; - /// - /// #[cfg(not(target_os = "windows"))] - /// assert_eq!( - /// Cow::from_backslash(r"foo\\bar\\piyo.txt"), - /// Path::new("foo/bar/piyo.txt"), - /// ); - /// - /// #[cfg(target_os = "windows")] - /// assert_eq!( - /// Cow::from_backslash(r"foo\\bar\\piyo.txt"), - /// Path::new(r"foo\\bar\\piyo.txt"), - /// ); - /// ``` #[cfg(target_os = "windows")] fn from_backslash(s: &'a str) -> Self { Cow::Borrowed(Path::new(s)) @@ -632,11 +482,6 @@ impl<'a> CowExt<'a> for Cow<'a, Path> { Cow::Owned(s) => Cow::Owned(str_to_pathbuf(&s, '\\')), } } - - /// Convert the [`OsStr`] backslash path (path separated with '\\') to [`Cow`]. - /// - /// Any '\\' in the slash path is replaced with the file path separator. Heap allocation may - /// only happen on non-Windows. #[cfg(target_os = "windows")] fn from_backslash_lossy(s: &'a OsStr) -> Self { Cow::Borrowed(Path::new(s)) From 182b0fe4c9e6a69540578d92c7f8cd3ae7911604 Mon Sep 17 00:00:00 2001 From: rhysd Date: Sun, 3 Jul 2022 01:19:54 +0900 Subject: [PATCH 11/11] refactor with removing redundant flags --- src/lib.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 1beeb6c..b6387bc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -140,7 +140,6 @@ impl PathExt for Path { use std::path::Component; let mut buf = String::new(); - let mut has_trailing_slash = false; for c in self.components() { match c { Component::RootDir => { /* empty */ } @@ -154,10 +153,9 @@ impl PathExt for Path { Component::Normal(s) => buf.push_str(&s.to_string_lossy()), } buf.push('/'); - has_trailing_slash = true; } - if buf != "/" && has_trailing_slash { + if buf != "/" && buf.ends_with('/') { buf.pop(); // Pop last '/' } @@ -191,7 +189,6 @@ impl PathExt for Path { use std::path::Component; let mut buf = String::new(); - let mut has_trailing_slash = false; for c in self.components() { match c { Component::RootDir => { /* empty */ } @@ -205,10 +202,9 @@ impl PathExt for Path { Component::Normal(s) => buf.push_str(s.to_str()?), } buf.push('/'); - has_trailing_slash = true; } - if buf != "/" && has_trailing_slash { + if buf != "/" && buf.ends_with('/') { buf.pop(); // Pop last '/' }