Optimization of tilde expansion

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<PathBuf> {
     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));
     }
 

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<Path>) -> 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());
+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 Cow::Owned(buf);
             }
         }
     }
 
-    path.to_path_buf()
+    path
 }
 
 /// Normalize a path without resolving symlinks.
@@ -109,9 +111,9 @@ pub fn normalize(path: impl AsRef<Path>) -> 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<Path>) -> 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
     };

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<dyn Error>> {
-    // 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<dyn Error>> {
-    /*
-    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);
+    }
 }

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<dyn Error>> {
+    // 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<dyn Error>> {
+    /*
+    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(())
+}

helix-term/src/commands/typed.rs

@@ -110,14 +110,14 @@ fn open(cx: &mut compositor::Context, args: &[Cow<str>], 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)?;
 

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()),
                 }
             };