From 02ebcff988dedb278c35064826a98a3f8df4aa3a Mon Sep 17 00:00:00 2001 From: jazzfool Date: Mon, 13 Mar 2023 15:31:59 +1100 Subject: [PATCH 1/3] attempt ancestor canonicalization first, use in get_relative_path --- Cargo.lock | 1 + helix-core/Cargo.toml | 1 + helix-core/src/path.rs | 32 ++++++++++++++++++++++++++++---- 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index de985bca11eb..71bf32dbeed0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1076,6 +1076,7 @@ dependencies = [ "arc-swap", "bitflags", "chrono", + "dunce", "encoding_rs", "etcetera", "hashbrown 0.13.2", diff --git a/helix-core/Cargo.toml b/helix-core/Cargo.toml index 62ec87b485ca..8f107f4b0710 100644 --- a/helix-core/Cargo.toml +++ b/helix-core/Cargo.toml @@ -32,6 +32,7 @@ regex = "1" bitflags = "1.3" ahash = "0.8.3" hashbrown = { version = "0.13.2", features = ["raw"] } +dunce = "1.0" log = "0.4" serde = { version = "1.0", features = ["derive"] } diff --git a/helix-core/src/path.rs b/helix-core/src/path.rs index d59a6baad604..64d8c4608c42 100644 --- a/helix-core/src/path.rs +++ b/helix-core/src/path.rs @@ -40,6 +40,24 @@ pub fn expand_tilde(path: &Path) -> PathBuf { /// needs to improve on. /// Copied from cargo: pub fn get_normalized_path(path: &Path) -> PathBuf { + // normalization strategy is to canonicalize first ancestor path that exists (i.e., canonicalize as much as possible), + // then run handrolled normalization on the non-existent remainder + let (base, path) = path + .ancestors() + .find(|path| path.exists()) + .and_then(|path| Some((path, dunce::canonicalize(path).ok()?))) + .map(|(base, canonicalized)| { + ( + canonicalized, + path.strip_prefix(base).unwrap(/* base is an ancestor of path */).into(), + ) + }) + .unwrap_or_else(|| (PathBuf::new(), PathBuf::from(path))); + + if path.as_os_str().is_empty() { + return base; + } + let mut components = path.components().peekable(); let mut ret = if let Some(c @ Component::Prefix(..)) = components.peek().cloned() { components.next(); @@ -63,7 +81,7 @@ pub fn get_normalized_path(path: &Path) -> PathBuf { } } } - ret + base.join(ret) } /// Returns the canonical, absolute form of a path with all intermediate components normalized. @@ -82,13 +100,19 @@ pub fn get_canonicalized_path(path: &Path) -> std::io::Result { } pub fn get_relative_path(path: &Path) -> PathBuf { + let path = PathBuf::from(path); let path = if path.is_absolute() { - let cwdir = std::env::current_dir().expect("couldn't determine current directory"); - path.strip_prefix(cwdir).unwrap_or(path) + let cwdir = std::env::current_dir() + .map(|path| get_normalized_path(&path)) + .expect("couldn't determine current directory"); + get_normalized_path(&path) + .strip_prefix(cwdir) + .map(PathBuf::from) + .unwrap_or(path) } else { path }; - fold_home_dir(path) + fold_home_dir(&path) } /// Returns a truncated filepath where the basepart of the path is reduced to the first From cead3640ee9d52e9db021e1c98b7b71fad4b89bc Mon Sep 17 00:00:00 2001 From: jazzfool Date: Thu, 16 Mar 2023 17:46:36 +1100 Subject: [PATCH 2/3] use ancestor prefix strictly if canonicalization succeeds --- helix-core/src/path.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/helix-core/src/path.rs b/helix-core/src/path.rs index 64d8c4608c42..efa46c46e98f 100644 --- a/helix-core/src/path.rs +++ b/helix-core/src/path.rs @@ -44,13 +44,10 @@ pub fn get_normalized_path(path: &Path) -> PathBuf { // then run handrolled normalization on the non-existent remainder let (base, path) = path .ancestors() - .find(|path| path.exists()) - .and_then(|path| Some((path, dunce::canonicalize(path).ok()?))) - .map(|(base, canonicalized)| { - ( - canonicalized, - path.strip_prefix(base).unwrap(/* base is an ancestor of path */).into(), - ) + .find_map(|base| { + let canonicalized_base = dunce::canonicalize(base).ok()?; + let remainder = path.strip_prefix(base).ok()?.into(); + Some((canonicalized_base, remainder)) }) .unwrap_or_else(|| (PathBuf::new(), PathBuf::from(path))); From 0a5fd2abbdbd5a848a064d8c70d67143dbcf91c2 Mon Sep 17 00:00:00 2001 From: jazzfool Date: Thu, 16 Mar 2023 18:16:02 +1100 Subject: [PATCH 3/3] fix path normalization in tests --- helix-term/tests/test/commands.rs | 4 ++-- helix-term/tests/test/splits.rs | 8 +++++--- helix-term/tests/test/write.rs | 8 ++++---- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/helix-term/tests/test/commands.rs b/helix-term/tests/test/commands.rs index e8d16bfaf2be..19a321c1e51e 100644 --- a/helix-term/tests/test/commands.rs +++ b/helix-term/tests/test/commands.rs @@ -1,6 +1,6 @@ use std::ops::RangeInclusive; -use helix_core::diagnostic::Severity; +use helix_core::{diagnostic::Severity, path::get_normalized_path}; use helix_term::application::Application; use super::*; @@ -20,7 +20,7 @@ async fn test_write_quit_fail() -> anyhow::Result<()> { assert_eq!(1, docs.len()); let doc = docs.pop().unwrap(); - assert_eq!(Some(file.path()), doc.path().map(PathBuf::as_path)); + assert_eq!(Some(&get_normalized_path(file.path())), doc.path()); assert_eq!(&Severity::Error, app.editor.get_status().unwrap().1); }), false, diff --git a/helix-term/tests/test/splits.rs b/helix-term/tests/test/splits.rs index 96ced21a57cd..1d70f24a6dac 100644 --- a/helix-term/tests/test/splits.rs +++ b/helix-term/tests/test/splits.rs @@ -1,5 +1,7 @@ use super::*; +use helix_core::path::get_normalized_path; + #[tokio::test(flavor = "multi_thread")] async fn test_split_write_quit_all() -> anyhow::Result<()> { let mut file1 = tempfile::NamedTempFile::new()?; @@ -25,21 +27,21 @@ async fn test_split_write_quit_all() -> anyhow::Result<()> { let doc1 = docs .iter() - .find(|doc| doc.path().unwrap() == file1.path()) + .find(|doc| doc.path().unwrap() == &get_normalized_path(file1.path())) .unwrap(); assert_eq!("hello1", doc1.text().to_string()); let doc2 = docs .iter() - .find(|doc| doc.path().unwrap() == file2.path()) + .find(|doc| doc.path().unwrap() == &get_normalized_path(file2.path())) .unwrap(); assert_eq!("hello2", doc2.text().to_string()); let doc3 = docs .iter() - .find(|doc| doc.path().unwrap() == file3.path()) + .find(|doc| doc.path().unwrap() == &get_normalized_path(file3.path())) .unwrap(); assert_eq!("hello3", doc3.text().to_string()); diff --git a/helix-term/tests/test/write.rs b/helix-term/tests/test/write.rs index 81459b2fe846..cbdcfdab96b0 100644 --- a/helix-term/tests/test/write.rs +++ b/helix-term/tests/test/write.rs @@ -3,7 +3,7 @@ use std::{ ops::RangeInclusive, }; -use helix_core::diagnostic::Severity; +use helix_core::{diagnostic::Severity, path::get_normalized_path}; use helix_view::doc; use super::*; @@ -179,7 +179,7 @@ async fn test_write_scratch_to_new_path() -> anyhow::Result<()> { assert_eq!(1, docs.len()); let doc = docs.pop().unwrap(); - assert_eq!(Some(&file.path().to_path_buf()), doc.path()); + assert_eq!(Some(&get_normalized_path(file.path())), doc.path()); }), false, ) @@ -251,7 +251,7 @@ async fn test_write_new_path() -> anyhow::Result<()> { Some(&|app| { let doc = doc!(app.editor); assert!(!app.editor.is_err()); - assert_eq!(file1.path(), doc.path().unwrap()); + assert_eq!(&get_normalized_path(file1.path()), doc.path().unwrap()); }), ), ( @@ -259,7 +259,7 @@ async fn test_write_new_path() -> anyhow::Result<()> { Some(&|app| { let doc = doc!(app.editor); assert!(!app.editor.is_err()); - assert_eq!(file2.path(), doc.path().unwrap()); + assert_eq!(&get_normalized_path(file2.path()), doc.path().unwrap()); assert!(app.editor.document_by_path(file1.path()).is_none()); }), ),