From 95fd77795717a9709ed8ac3fde485954f7f5b480 Mon Sep 17 00:00:00 2001 From: mo8it Date: Fri, 23 Feb 2024 17:20:35 +0100 Subject: [PATCH 1/6] Use next and avoid a redundant prefix strip --- helix-stdx/src/path.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/helix-stdx/src/path.rs b/helix-stdx/src/path.rs index 5746657c3c7b..6ebbfdd02a33 100644 --- a/helix-stdx/src/path.rs +++ b/helix-stdx/src/path.rs @@ -21,12 +21,12 @@ pub fn fold_home_dir(path: &Path) -> PathBuf { /// and only slash follows it. pub fn expand_tilde(path: impl AsRef) -> PathBuf { let path = path.as_ref(); - let mut components = path.components().peekable(); - if let Some(Component::Normal(c)) = components.peek() { - if c == &"~" { - if let Ok(home) = home_dir() { - // it's ok to unwrap, the path starts with `~` - return home.join(path.strip_prefix("~").unwrap()); + let mut components = path.components(); + if let Some(Component::Normal(c)) = components.next() { + if c == "~" { + if let Ok(mut buf) = home_dir() { + buf.push(components); + return buf; } } } From 781f9e2882ed01bbd1176b1fbeb994d24269ed8c Mon Sep 17 00:00:00 2001 From: mo8it Date: Fri, 23 Feb 2024 17:49:39 +0100 Subject: [PATCH 2/6] Avoid allocations Especially when `expand_tilde` is claled on a path that doesn't contain a tilde. --- helix-loader/src/lib.rs | 7 +++++-- helix-stdx/src/path.rs | 16 +++++++++------- helix-term/src/commands/typed.rs | 14 +++++++------- helix-term/src/ui/mod.rs | 6 +++--- 4 files changed, 24 insertions(+), 19 deletions(-) diff --git a/helix-loader/src/lib.rs b/helix-loader/src/lib.rs index f8fac67035e5..d683c9f80588 100644 --- a/helix-loader/src/lib.rs +++ b/helix-loader/src/lib.rs @@ -4,7 +4,10 @@ pub mod grammar; use helix_stdx::{env::current_working_dir, path}; use etcetera::base_strategy::{choose_base_strategy, BaseStrategy}; -use std::path::{Path, PathBuf}; +use std::{ + borrow::Cow, + path::{Path, PathBuf}, +}; pub const VERSION_AND_GIT_HASH: &str = env!("VERSION_AND_GIT_HASH"); @@ -53,7 +56,7 @@ fn prioritize_runtime_dirs() -> Vec { rt_dirs.push(conf_rt_dir); if let Ok(dir) = std::env::var("HELIX_RUNTIME") { - let dir = path::expand_tilde(dir); + let dir = path::expand_tilde(Cow::Borrowed(Path::new(&dir))); rt_dirs.push(path::normalize(dir)); } diff --git a/helix-stdx/src/path.rs b/helix-stdx/src/path.rs index 6ebbfdd02a33..1cec83804ac3 100644 --- a/helix-stdx/src/path.rs +++ b/helix-stdx/src/path.rs @@ -1,6 +1,9 @@ pub use etcetera::home_dir; -use std::path::{Component, Path, PathBuf}; +use std::{ + borrow::Cow, + path::{Component, Path, PathBuf}, +}; use crate::env::current_working_dir; @@ -19,19 +22,18 @@ pub fn fold_home_dir(path: &Path) -> PathBuf { /// Expands tilde `~` into users home directory if available, otherwise returns the path /// unchanged. The tilde will only be expanded when present as the first component of the path /// and only slash follows it. -pub fn expand_tilde(path: impl AsRef) -> PathBuf { - let path = path.as_ref(); +pub fn expand_tilde(path: Cow<'_, Path>) -> Cow<'_, Path> { let mut components = path.components(); if let Some(Component::Normal(c)) = components.next() { if c == "~" { if let Ok(mut buf) = home_dir() { buf.push(components); - return buf; + return Cow::Owned(buf); } } } - path.to_path_buf() + path } /// Normalize a path without resolving symlinks. @@ -109,9 +111,9 @@ pub fn normalize(path: impl AsRef) -> PathBuf { /// This function is used instead of [`std::fs::canonicalize`] because we don't want to verify /// here if the path exists, just normalize it's components. pub fn canonicalize(path: impl AsRef) -> PathBuf { - let path = expand_tilde(path); + let path = expand_tilde(Cow::Borrowed(path.as_ref())); let path = if path.is_relative() { - current_working_dir().join(path) + Cow::Owned(current_working_dir().join(path)) } else { path }; diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index b7ceeba59a18..44d8286949eb 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -110,14 +110,14 @@ fn open(cx: &mut compositor::Context, args: &[Cow], event: PromptEvent) -> ensure!(!args.is_empty(), "wrong argument count"); for arg in args { let (path, pos) = args::parse_file(arg); - let path = helix_stdx::path::expand_tilde(&path); + let path = helix_stdx::path::expand_tilde(Cow::Owned(path)); // If the path is a directory, open a file picker on that directory and update the status // message if let Ok(true) = std::fs::canonicalize(&path).map(|p| p.is_dir()) { let callback = async move { let call: job::Callback = job::Callback::EditorCompositor(Box::new( move |editor: &mut Editor, compositor: &mut Compositor| { - let picker = ui::file_picker(path, &editor.config()); + let picker = ui::file_picker(path.into_owned(), &editor.config()); compositor.push(Box::new(overlaid(picker))); }, )); @@ -1078,11 +1078,11 @@ fn change_current_directory( return Ok(()); } - let dir = helix_stdx::path::expand_tilde( - args.first() - .context("target directory not provided")? - .as_ref(), - ); + let dir = args + .first() + .context("target directory not provided")? + .as_ref(); + let dir = helix_stdx::path::expand_tilde(Cow::Borrowed(Path::new(dir))); helix_stdx::env::set_current_working_dir(dir)?; diff --git a/helix-term/src/ui/mod.rs b/helix-term/src/ui/mod.rs index d27e83553fb1..aff365e8493c 100644 --- a/helix-term/src/ui/mod.rs +++ b/helix-term/src/ui/mod.rs @@ -409,7 +409,7 @@ pub mod completers { use std::path::Path; let is_tilde = input == "~"; - let path = helix_stdx::path::expand_tilde(Path::new(input)); + let path = helix_stdx::path::expand_tilde(Cow::Borrowed(Path::new(input))); let (dir, file_name) = if input.ends_with(std::path::MAIN_SEPARATOR) { (path, None) @@ -428,9 +428,9 @@ pub mod completers { path } else { match path.parent() { - Some(path) if !path.as_os_str().is_empty() => path.to_path_buf(), + Some(path) if !path.as_os_str().is_empty() => Cow::Borrowed(path), // Path::new("h")'s parent is Some("")... - _ => helix_stdx::env::current_working_dir(), + _ => Cow::Owned(helix_stdx::env::current_working_dir()), } }; From b84376b9193529f65dfc48c07a57413751cc3bfa Mon Sep 17 00:00:00 2001 From: mo8it Date: Fri, 23 Feb 2024 17:51:34 +0100 Subject: [PATCH 3/6] Add a test --- helix-stdx/tests/path.rs | 136 +++++-------------------------- helix-stdx/tests/path_windows.rs | 126 ++++++++++++++++++++++++++++ 2 files changed, 145 insertions(+), 117 deletions(-) create mode 100644 helix-stdx/tests/path_windows.rs diff --git a/helix-stdx/tests/path.rs b/helix-stdx/tests/path.rs index cc3c15cba65c..ee9c64476992 100644 --- a/helix-stdx/tests/path.rs +++ b/helix-stdx/tests/path.rs @@ -1,124 +1,26 @@ -#![cfg(windows)] - use std::{ - env::set_current_dir, - error::Error, - path::{Component, Path, PathBuf}, + borrow::Cow, + ffi::OsStr, + path::{Component, Path}, }; use helix_stdx::path; -use tempfile::Builder; - -// Paths on Windows are almost always case-insensitive. -// Normalization should return the original path. -// E.g. mkdir `CaSe`, normalize(`case`) = `CaSe`. -#[test] -fn test_case_folding_windows() -> Result<(), Box> { - // tmp/root/case - let tmp_prefix = std::env::temp_dir(); - set_current_dir(&tmp_prefix)?; - - let root = Builder::new().prefix("root-").tempdir()?; - let case = Builder::new().prefix("CaSe-").tempdir_in(&root)?; - - let root_without_prefix = root.path().strip_prefix(&tmp_prefix)?; - - let lowercase_case = format!( - "case-{}", - case.path() - .file_name() - .unwrap() - .to_string_lossy() - .split_at(5) - .1 - ); - let test_path = root_without_prefix.join(lowercase_case); - assert_eq!( - path::normalize(&test_path), - case.path().strip_prefix(&tmp_prefix)? - ); - - Ok(()) -} #[test] -fn test_normalize_path() -> Result<(), Box> { - /* - tmp/root/ - ├── link -> dir1/orig_file - ├── dir1/ - │ └── orig_file - └── dir2/ - └── dir_link -> ../dir1/ - */ - - let tmp_prefix = std::env::temp_dir(); - set_current_dir(&tmp_prefix)?; - - // Create a tree structure as shown above - let root = Builder::new().prefix("root-").tempdir()?; - let dir1 = Builder::new().prefix("dir1-").tempdir_in(&root)?; - let orig_file = Builder::new().prefix("orig_file-").tempfile_in(&dir1)?; - let dir2 = Builder::new().prefix("dir2-").tempdir_in(&root)?; - - // Create path and delete existing file - let dir_link = Builder::new() - .prefix("dir_link-") - .tempfile_in(&dir2)? - .path() - .to_owned(); - let link = Builder::new() - .prefix("link-") - .tempfile_in(&root)? - .path() - .to_owned(); - - use std::os::windows; - windows::fs::symlink_dir(&dir1, &dir_link)?; - windows::fs::symlink_file(&orig_file, &link)?; - - // root/link - let path = link.strip_prefix(&tmp_prefix)?; - assert_eq!( - path::normalize(path), - path, - "input {:?} and symlink last component shouldn't be resolved", - path - ); - - // root/dir2/dir_link/orig_file/../.. - let path = dir_link - .strip_prefix(&tmp_prefix) - .unwrap() - .join(orig_file.path().file_name().unwrap()) - .join(Component::ParentDir) - .join(Component::ParentDir); - let expected = dir_link - .strip_prefix(&tmp_prefix) - .unwrap() - .join(Component::ParentDir); - assert_eq!( - path::normalize(&path), - expected, - "input {:?} and \"..\" should not erase the simlink that goes ahead", - &path - ); - - // root/link/.././../dir2/../ - let path = link - .strip_prefix(&tmp_prefix) - .unwrap() - .join(Component::ParentDir) - .join(Component::CurDir) - .join(Component::ParentDir) - .join(dir2.path().file_name().unwrap()) - .join(Component::ParentDir); - let expected = link - .strip_prefix(&tmp_prefix) - .unwrap() - .join(Component::ParentDir) - .join(Component::ParentDir); - assert_eq!(path::normalize(&path), expected, "input {:?}", &path); - - Ok(()) +fn expand_tilde() { + for path in ["~", "~/foo"] { + let expanded = path::expand_tilde(Cow::Borrowed(Path::new(path))); + + let tilde = Component::Normal(OsStr::new("~")); + + let mut component_count = 0; + for component in expanded.components() { + // No tilde left. + assert_ne!(component, tilde); + component_count += 1; + } + + // The path was at least expanded to something. + assert_ne!(component_count, 0); + } } diff --git a/helix-stdx/tests/path_windows.rs b/helix-stdx/tests/path_windows.rs new file mode 100644 index 000000000000..bdbae216c176 --- /dev/null +++ b/helix-stdx/tests/path_windows.rs @@ -0,0 +1,126 @@ +#![cfg(windows)] + +use std::{ + assert_eq, + borrow::Cow, + env::set_current_dir, + error::Error, + path::{Component, Path, PathBuf}, +}; + +use helix_stdx::path; +use tempfile::Builder; + +// Paths on Windows are almost always case-insensitive. +// Normalization should return the original path. +// E.g. mkdir `CaSe`, normalize(`case`) = `CaSe`. +#[test] +fn test_case_folding_windows() -> Result<(), Box> { + // tmp/root/case + let tmp_prefix = std::env::temp_dir(); + set_current_dir(&tmp_prefix)?; + + let root = Builder::new().prefix("root-").tempdir()?; + let case = Builder::new().prefix("CaSe-").tempdir_in(&root)?; + + let root_without_prefix = root.path().strip_prefix(&tmp_prefix)?; + + let lowercase_case = format!( + "case-{}", + case.path() + .file_name() + .unwrap() + .to_string_lossy() + .split_at(5) + .1 + ); + let test_path = root_without_prefix.join(lowercase_case); + assert_eq!( + path::normalize(&test_path), + case.path().strip_prefix(&tmp_prefix)? + ); + + Ok(()) +} + +#[test] +fn test_normalize_path() -> Result<(), Box> { + /* + tmp/root/ + ├── link -> dir1/orig_file + ├── dir1/ + │ └── orig_file + └── dir2/ + └── dir_link -> ../dir1/ + */ + + let tmp_prefix = std::env::temp_dir(); + set_current_dir(&tmp_prefix)?; + + // Create a tree structure as shown above + let root = Builder::new().prefix("root-").tempdir()?; + let dir1 = Builder::new().prefix("dir1-").tempdir_in(&root)?; + let orig_file = Builder::new().prefix("orig_file-").tempfile_in(&dir1)?; + let dir2 = Builder::new().prefix("dir2-").tempdir_in(&root)?; + + // Create path and delete existing file + let dir_link = Builder::new() + .prefix("dir_link-") + .tempfile_in(&dir2)? + .path() + .to_owned(); + let link = Builder::new() + .prefix("link-") + .tempfile_in(&root)? + .path() + .to_owned(); + + use std::os::windows; + windows::fs::symlink_dir(&dir1, &dir_link)?; + windows::fs::symlink_file(&orig_file, &link)?; + + // root/link + let path = link.strip_prefix(&tmp_prefix)?; + assert_eq!( + path::normalize(path), + path, + "input {:?} and symlink last component shouldn't be resolved", + path + ); + + // root/dir2/dir_link/orig_file/../.. + let path = dir_link + .strip_prefix(&tmp_prefix) + .unwrap() + .join(orig_file.path().file_name().unwrap()) + .join(Component::ParentDir) + .join(Component::ParentDir); + let expected = dir_link + .strip_prefix(&tmp_prefix) + .unwrap() + .join(Component::ParentDir); + assert_eq!( + path::normalize(&path), + expected, + "input {:?} and \"..\" should not erase the simlink that goes ahead", + &path + ); + + // root/link/.././../dir2/../ + let path = link + .strip_prefix(&tmp_prefix) + .unwrap() + .join(Component::ParentDir) + .join(Component::CurDir) + .join(Component::ParentDir) + .join(dir2.path().file_name().unwrap()) + .join(Component::ParentDir); + let expected = link + .strip_prefix(&tmp_prefix) + .unwrap() + .join(Component::ParentDir) + .join(Component::ParentDir); + assert_eq!(path::normalize(&path), expected, "input {:?}", &path); + + Ok(()) +} From 899c3a594cda11c5ef2bc5a0388d4af887a89e33 Mon Sep 17 00:00:00 2001 From: mo8it Date: Sat, 24 Feb 2024 01:14:14 +0100 Subject: [PATCH 4/6] =?UTF-8?q?Use=20Into>?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- helix-loader/src/lib.rs | 7 ++----- helix-stdx/src/path.rs | 8 ++++++-- helix-stdx/tests/path.rs | 3 +-- helix-term/src/commands/typed.rs | 4 ++-- helix-term/src/ui/mod.rs | 2 +- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/helix-loader/src/lib.rs b/helix-loader/src/lib.rs index d683c9f80588..93488e452e8f 100644 --- a/helix-loader/src/lib.rs +++ b/helix-loader/src/lib.rs @@ -4,10 +4,7 @@ pub mod grammar; use helix_stdx::{env::current_working_dir, path}; use etcetera::base_strategy::{choose_base_strategy, BaseStrategy}; -use std::{ - borrow::Cow, - path::{Path, PathBuf}, -}; +use std::path::{Path, PathBuf}; pub const VERSION_AND_GIT_HASH: &str = env!("VERSION_AND_GIT_HASH"); @@ -56,7 +53,7 @@ fn prioritize_runtime_dirs() -> Vec { rt_dirs.push(conf_rt_dir); if let Ok(dir) = std::env::var("HELIX_RUNTIME") { - let dir = path::expand_tilde(Cow::Borrowed(Path::new(&dir))); + let dir = path::expand_tilde(Path::new(&dir)); rt_dirs.push(path::normalize(dir)); } diff --git a/helix-stdx/src/path.rs b/helix-stdx/src/path.rs index 1cec83804ac3..f9de4cd2e392 100644 --- a/helix-stdx/src/path.rs +++ b/helix-stdx/src/path.rs @@ -22,7 +22,11 @@ pub fn fold_home_dir(path: &Path) -> PathBuf { /// Expands tilde `~` into users home directory if available, otherwise returns the path /// unchanged. The tilde will only be expanded when present as the first component of the path /// and only slash follows it. -pub fn expand_tilde(path: Cow<'_, Path>) -> Cow<'_, Path> { +pub fn expand_tilde<'a, P>(path: P) -> Cow<'a, Path> +where + P: Into>, +{ + let path = path.into(); let mut components = path.components(); if let Some(Component::Normal(c)) = components.next() { if c == "~" { @@ -111,7 +115,7 @@ pub fn normalize(path: impl AsRef) -> PathBuf { /// This function is used instead of [`std::fs::canonicalize`] because we don't want to verify /// here if the path exists, just normalize it's components. pub fn canonicalize(path: impl AsRef) -> PathBuf { - let path = expand_tilde(Cow::Borrowed(path.as_ref())); + let path = expand_tilde(path.as_ref()); let path = if path.is_relative() { Cow::Owned(current_working_dir().join(path)) } else { diff --git a/helix-stdx/tests/path.rs b/helix-stdx/tests/path.rs index ee9c64476992..bb2d2d126608 100644 --- a/helix-stdx/tests/path.rs +++ b/helix-stdx/tests/path.rs @@ -1,5 +1,4 @@ use std::{ - borrow::Cow, ffi::OsStr, path::{Component, Path}, }; @@ -9,7 +8,7 @@ use helix_stdx::path; #[test] fn expand_tilde() { for path in ["~", "~/foo"] { - let expanded = path::expand_tilde(Cow::Borrowed(Path::new(path))); + let expanded = path::expand_tilde(Path::new(path)); let tilde = Component::Normal(OsStr::new("~")); diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 44d8286949eb..3d7ea3fc8b36 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -110,7 +110,7 @@ fn open(cx: &mut compositor::Context, args: &[Cow], event: PromptEvent) -> ensure!(!args.is_empty(), "wrong argument count"); for arg in args { let (path, pos) = args::parse_file(arg); - let path = helix_stdx::path::expand_tilde(Cow::Owned(path)); + let path = helix_stdx::path::expand_tilde(path); // If the path is a directory, open a file picker on that directory and update the status // message if let Ok(true) = std::fs::canonicalize(&path).map(|p| p.is_dir()) { @@ -1082,7 +1082,7 @@ fn change_current_directory( .first() .context("target directory not provided")? .as_ref(); - let dir = helix_stdx::path::expand_tilde(Cow::Borrowed(Path::new(dir))); + let dir = helix_stdx::path::expand_tilde(Path::new(dir)); helix_stdx::env::set_current_working_dir(dir)?; diff --git a/helix-term/src/ui/mod.rs b/helix-term/src/ui/mod.rs index aff365e8493c..0873116cb464 100644 --- a/helix-term/src/ui/mod.rs +++ b/helix-term/src/ui/mod.rs @@ -409,7 +409,7 @@ pub mod completers { use std::path::Path; let is_tilde = input == "~"; - let path = helix_stdx::path::expand_tilde(Cow::Borrowed(Path::new(input))); + let path = helix_stdx::path::expand_tilde(Path::new(input)); let (dir, file_name) = if input.ends_with(std::path::MAIN_SEPARATOR) { (path, None) From b076a93556dadb5d2babf5e0f65fbde61a78b70f Mon Sep 17 00:00:00 2001 From: mo8it Date: Sat, 24 Feb 2024 01:17:02 +0100 Subject: [PATCH 5/6] Put the expand_tilde test at the end of the file --- helix-stdx/src/path.rs | 29 +++++++ helix-stdx/tests/path.rs | 137 +++++++++++++++++++++++++++---- helix-stdx/tests/path_windows.rs | 126 ---------------------------- 3 files changed, 148 insertions(+), 144 deletions(-) delete mode 100644 helix-stdx/tests/path_windows.rs diff --git a/helix-stdx/src/path.rs b/helix-stdx/src/path.rs index f9de4cd2e392..1dc4d0b2448b 100644 --- a/helix-stdx/src/path.rs +++ b/helix-stdx/src/path.rs @@ -189,3 +189,32 @@ pub fn get_truncated_path(path: impl AsRef) -> PathBuf { ret.push(file); ret } + +#[cfg(test)] +mod tests { + use std::{ + ffi::OsStr, + path::{Component, Path}, + }; + + use crate::path; + + #[test] + fn expand_tilde() { + for path in ["~", "~/foo"] { + let expanded = path::expand_tilde(Path::new(path)); + + let tilde = Component::Normal(OsStr::new("~")); + + let mut component_count = 0; + for component in expanded.components() { + // No tilde left. + assert_ne!(component, tilde); + component_count += 1; + } + + // The path was at least expanded to something. + assert_ne!(component_count, 0); + } + } +} diff --git a/helix-stdx/tests/path.rs b/helix-stdx/tests/path.rs index bb2d2d126608..bdbae216c176 100644 --- a/helix-stdx/tests/path.rs +++ b/helix-stdx/tests/path.rs @@ -1,25 +1,126 @@ +#![cfg(windows)] + use std::{ - ffi::OsStr, - path::{Component, Path}, + assert_eq, + borrow::Cow, + env::set_current_dir, + error::Error, + path::{Component, Path, PathBuf}, }; use helix_stdx::path; +use tempfile::Builder; + +// Paths on Windows are almost always case-insensitive. +// Normalization should return the original path. +// E.g. mkdir `CaSe`, normalize(`case`) = `CaSe`. +#[test] +fn test_case_folding_windows() -> Result<(), Box> { + // tmp/root/case + let tmp_prefix = std::env::temp_dir(); + set_current_dir(&tmp_prefix)?; + + let root = Builder::new().prefix("root-").tempdir()?; + let case = Builder::new().prefix("CaSe-").tempdir_in(&root)?; + + let root_without_prefix = root.path().strip_prefix(&tmp_prefix)?; + + let lowercase_case = format!( + "case-{}", + case.path() + .file_name() + .unwrap() + .to_string_lossy() + .split_at(5) + .1 + ); + let test_path = root_without_prefix.join(lowercase_case); + assert_eq!( + path::normalize(&test_path), + case.path().strip_prefix(&tmp_prefix)? + ); + + Ok(()) +} #[test] -fn expand_tilde() { - for path in ["~", "~/foo"] { - let expanded = path::expand_tilde(Path::new(path)); - - let tilde = Component::Normal(OsStr::new("~")); - - let mut component_count = 0; - for component in expanded.components() { - // No tilde left. - assert_ne!(component, tilde); - component_count += 1; - } - - // The path was at least expanded to something. - assert_ne!(component_count, 0); - } +fn test_normalize_path() -> Result<(), Box> { + /* + tmp/root/ + ├── link -> dir1/orig_file + ├── dir1/ + │ └── orig_file + └── dir2/ + └── dir_link -> ../dir1/ + */ + + let tmp_prefix = std::env::temp_dir(); + set_current_dir(&tmp_prefix)?; + + // Create a tree structure as shown above + let root = Builder::new().prefix("root-").tempdir()?; + let dir1 = Builder::new().prefix("dir1-").tempdir_in(&root)?; + let orig_file = Builder::new().prefix("orig_file-").tempfile_in(&dir1)?; + let dir2 = Builder::new().prefix("dir2-").tempdir_in(&root)?; + + // Create path and delete existing file + let dir_link = Builder::new() + .prefix("dir_link-") + .tempfile_in(&dir2)? + .path() + .to_owned(); + let link = Builder::new() + .prefix("link-") + .tempfile_in(&root)? + .path() + .to_owned(); + + use std::os::windows; + windows::fs::symlink_dir(&dir1, &dir_link)?; + windows::fs::symlink_file(&orig_file, &link)?; + + // root/link + let path = link.strip_prefix(&tmp_prefix)?; + assert_eq!( + path::normalize(path), + path, + "input {:?} and symlink last component shouldn't be resolved", + path + ); + + // root/dir2/dir_link/orig_file/../.. + let path = dir_link + .strip_prefix(&tmp_prefix) + .unwrap() + .join(orig_file.path().file_name().unwrap()) + .join(Component::ParentDir) + .join(Component::ParentDir); + let expected = dir_link + .strip_prefix(&tmp_prefix) + .unwrap() + .join(Component::ParentDir); + assert_eq!( + path::normalize(&path), + expected, + "input {:?} and \"..\" should not erase the simlink that goes ahead", + &path + ); + + // root/link/.././../dir2/../ + let path = link + .strip_prefix(&tmp_prefix) + .unwrap() + .join(Component::ParentDir) + .join(Component::CurDir) + .join(Component::ParentDir) + .join(dir2.path().file_name().unwrap()) + .join(Component::ParentDir); + let expected = link + .strip_prefix(&tmp_prefix) + .unwrap() + .join(Component::ParentDir) + .join(Component::ParentDir); + assert_eq!(path::normalize(&path), expected, "input {:?}", &path); + + Ok(()) } diff --git a/helix-stdx/tests/path_windows.rs b/helix-stdx/tests/path_windows.rs deleted file mode 100644 index bdbae216c176..000000000000 --- a/helix-stdx/tests/path_windows.rs +++ /dev/null @@ -1,126 +0,0 @@ -#![cfg(windows)] - -use std::{ - assert_eq, - borrow::Cow, - env::set_current_dir, - error::Error, - path::{Component, Path, PathBuf}, -}; - -use helix_stdx::path; -use tempfile::Builder; - -// Paths on Windows are almost always case-insensitive. -// Normalization should return the original path. -// E.g. mkdir `CaSe`, normalize(`case`) = `CaSe`. -#[test] -fn test_case_folding_windows() -> Result<(), Box> { - // tmp/root/case - let tmp_prefix = std::env::temp_dir(); - set_current_dir(&tmp_prefix)?; - - let root = Builder::new().prefix("root-").tempdir()?; - let case = Builder::new().prefix("CaSe-").tempdir_in(&root)?; - - let root_without_prefix = root.path().strip_prefix(&tmp_prefix)?; - - let lowercase_case = format!( - "case-{}", - case.path() - .file_name() - .unwrap() - .to_string_lossy() - .split_at(5) - .1 - ); - let test_path = root_without_prefix.join(lowercase_case); - assert_eq!( - path::normalize(&test_path), - case.path().strip_prefix(&tmp_prefix)? - ); - - Ok(()) -} - -#[test] -fn test_normalize_path() -> Result<(), Box> { - /* - tmp/root/ - ├── link -> dir1/orig_file - ├── dir1/ - │ └── orig_file - └── dir2/ - └── dir_link -> ../dir1/ - */ - - let tmp_prefix = std::env::temp_dir(); - set_current_dir(&tmp_prefix)?; - - // Create a tree structure as shown above - let root = Builder::new().prefix("root-").tempdir()?; - let dir1 = Builder::new().prefix("dir1-").tempdir_in(&root)?; - let orig_file = Builder::new().prefix("orig_file-").tempfile_in(&dir1)?; - let dir2 = Builder::new().prefix("dir2-").tempdir_in(&root)?; - - // Create path and delete existing file - let dir_link = Builder::new() - .prefix("dir_link-") - .tempfile_in(&dir2)? - .path() - .to_owned(); - let link = Builder::new() - .prefix("link-") - .tempfile_in(&root)? - .path() - .to_owned(); - - use std::os::windows; - windows::fs::symlink_dir(&dir1, &dir_link)?; - windows::fs::symlink_file(&orig_file, &link)?; - - // root/link - let path = link.strip_prefix(&tmp_prefix)?; - assert_eq!( - path::normalize(path), - path, - "input {:?} and symlink last component shouldn't be resolved", - path - ); - - // root/dir2/dir_link/orig_file/../.. - let path = dir_link - .strip_prefix(&tmp_prefix) - .unwrap() - .join(orig_file.path().file_name().unwrap()) - .join(Component::ParentDir) - .join(Component::ParentDir); - let expected = dir_link - .strip_prefix(&tmp_prefix) - .unwrap() - .join(Component::ParentDir); - assert_eq!( - path::normalize(&path), - expected, - "input {:?} and \"..\" should not erase the simlink that goes ahead", - &path - ); - - // root/link/.././../dir2/../ - let path = link - .strip_prefix(&tmp_prefix) - .unwrap() - .join(Component::ParentDir) - .join(Component::CurDir) - .join(Component::ParentDir) - .join(dir2.path().file_name().unwrap()) - .join(Component::ParentDir); - let expected = link - .strip_prefix(&tmp_prefix) - .unwrap() - .join(Component::ParentDir) - .join(Component::ParentDir); - assert_eq!(path::normalize(&path), expected, "input {:?}", &path); - - Ok(()) -} From fe9466ca227fbb3f2436618bd520cba55fe938e2 Mon Sep 17 00:00:00 2001 From: mo8it Date: Sat, 24 Feb 2024 01:19:09 +0100 Subject: [PATCH 6/6] Remove unused importsw --- helix-stdx/tests/path.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/helix-stdx/tests/path.rs b/helix-stdx/tests/path.rs index bdbae216c176..cc3c15cba65c 100644 --- a/helix-stdx/tests/path.rs +++ b/helix-stdx/tests/path.rs @@ -1,8 +1,6 @@ #![cfg(windows)] use std::{ - assert_eq, - borrow::Cow, env::set_current_dir, error::Error, path::{Component, Path, PathBuf},