Skip to content

Commit

Permalink
Optimization of tilde expansion (helix-editor#9709)
Browse files Browse the repository at this point in the history
* Use next and avoid a redundant prefix strip

* Avoid allocations

Especially when `expand_tilde` is claled on a path
that doesn't contain a tilde.

* Add a test

* Use Into<Cow<…>>

* Put the expand_tilde test at the end of the file

* Remove unused importsw
  • Loading branch information
mo8it authored and Schuyler Mortimer committed Jul 10, 2024
1 parent 5e4f6d1 commit e8c53fd
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 22 deletions.
2 changes: 1 addition & 1 deletion helix-loader/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,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(Path::new(&dir));
rt_dirs.push(path::normalize(dir));
}

Expand Down
59 changes: 47 additions & 12 deletions helix-stdx/src/path.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -19,19 +22,22 @@ 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<'a, P>(path: P) -> Cow<'a, Path>
where
P: Into<Cow<'a, Path>>,
{
let path = path.into();
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.
Expand Down Expand Up @@ -109,9 +115,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(path.as_ref());
let path = if path.is_relative() {
current_working_dir().join(path)
Cow::Owned(current_working_dir().join(path))
} else {
path
};
Expand Down Expand Up @@ -183,3 +189,32 @@ pub fn get_truncated_path(path: impl AsRef<Path>) -> 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);
}
}
}
14 changes: 7 additions & 7 deletions helix-term/src/commands/typed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(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)));
},
));
Expand Down Expand Up @@ -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(Path::new(dir));

helix_stdx::env::set_current_working_dir(dir)?;

Expand Down
4 changes: 2 additions & 2 deletions helix-term/src/ui/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -429,9 +429,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()),
}
};

Expand Down

0 comments on commit e8c53fd

Please sign in to comment.