From e4a4fc568ff3fb31cf05ca05b6aa4328b2ea151e Mon Sep 17 00:00:00 2001 From: Myriad-Dreamin <35292584+Myriad-Dreamin@users.noreply.github.com> Date: Wed, 26 Mar 2025 12:46:33 +0800 Subject: [PATCH] fix: correct rename on unix platforms caused by pathdiff#8 (#1587) * fix: correct rename on unix platforms caused by pathdiff#8 * fix: ensure all calls to pathdiff * fix: names * fix: file path on windows --- Cargo.lock | 5 +---- crates/tinymist-project/Cargo.toml | 1 - crates/tinymist-query/Cargo.toml | 1 - .../src/analysis/completion/path.rs | 2 +- .../snaps/test@import_package.typ.snap | 2 +- .../snaps/test@import_package_self.typ.snap | 2 +- .../rename/snaps/test@module_path.typ.snap | 2 +- .../snaps/test@module_path_alias.typ.snap | 2 +- .../snaps/test@module_path_non_cano.typ.snap | 4 ++-- .../snaps/test@module_path_star.typ.snap | 2 +- .../rename/snaps/test@resources.typ.snap | 2 +- crates/tinymist-query/src/rename.rs | 7 ++++--- crates/tinymist-query/src/tests.rs | 10 +++++---- .../tinymist-query/src/will_rename_files.rs | 2 +- crates/tinymist-std/Cargo.toml | 1 + crates/tinymist-std/src/path.rs | 21 ++++++++++++++++++- crates/tinymist-task/Cargo.toml | 1 - crates/tinymist-task/src/primitives.rs | 4 ++-- crates/tinymist/Cargo.toml | 1 - 19 files changed, 44 insertions(+), 28 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 12765f0b..4979ce25 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4044,7 +4044,6 @@ dependencies = [ "open", "parking_lot", "paste", - "pathdiff", "rayon", "reflexo", "reflexo-typst", @@ -4203,7 +4202,6 @@ dependencies = [ "log", "notify", "parking_lot", - "pathdiff", "rayon", "rpds", "semver", @@ -4243,7 +4241,6 @@ dependencies = [ "lsp-types", "once_cell", "parking_lot", - "pathdiff", "percent-encoding", "rayon", "regex", @@ -4307,6 +4304,7 @@ dependencies = [ "lsp-types", "parking_lot", "path-clean", + "pathdiff", "rkyv", "rustc-hash 2.1.1", "same-file", @@ -4337,7 +4335,6 @@ dependencies = [ "log", "notify", "parking_lot", - "pathdiff", "rayon", "rpds", "semver", diff --git a/crates/tinymist-project/Cargo.toml b/crates/tinymist-project/Cargo.toml index af5cce14..0a33257a 100644 --- a/crates/tinymist-project/Cargo.toml +++ b/crates/tinymist-project/Cargo.toml @@ -21,7 +21,6 @@ dirs.workspace = true ecow.workspace = true log.workspace = true parking_lot.workspace = true -pathdiff.workspace = true tokio = { workspace = true, features = ["sync"] } rayon.workspace = true rpds.workspace = true diff --git a/crates/tinymist-query/Cargo.toml b/crates/tinymist-query/Cargo.toml index 3b23e1b7..3096622f 100644 --- a/crates/tinymist-query/Cargo.toml +++ b/crates/tinymist-query/Cargo.toml @@ -45,7 +45,6 @@ lsp-types.workspace = true if_chain.workspace = true percent-encoding.workspace = true unscanny.workspace = true -pathdiff.workspace = true ttf-parser.workspace = true rust_iso639.workspace = true rust_iso3166.workspace = true diff --git a/crates/tinymist-query/src/analysis/completion/path.rs b/crates/tinymist-query/src/analysis/completion/path.rs index c99202d7..6a622804 100644 --- a/crates/tinymist-query/src/analysis/completion/path.rs +++ b/crates/tinymist-query/src/analysis/completion/path.rs @@ -78,7 +78,7 @@ impl CompletionPair<'_, '_, '_> { .parent() .unwrap_or(Path::new("/")); let path = path.vpath().as_rooted_path(); - let w = pathdiff::diff_paths(path, base)?; + let w = tinymist_std::path::diff(path, base)?; unix_slash(&w).into() }; crate::log_debug_ct!("compl_label: {label:?}"); diff --git a/crates/tinymist-query/src/fixtures/goto_definition/snaps/test@import_package.typ.snap b/crates/tinymist-query/src/fixtures/goto_definition/snaps/test@import_package.typ.snap index 1ce1bc50..6ba2a6c4 100644 --- a/crates/tinymist-query/src/fixtures/goto_definition/snaps/test@import_package.typ.snap +++ b/crates/tinymist-query/src/fixtures/goto_definition/snaps/test@import_package.typ.snap @@ -9,6 +9,6 @@ snapshot_kind: text "originSelectionRange": "1:20:1:27", "targetRange": "0:0:0:0", "targetSelectionRange": "0:0:0:0", - "targetUri": "lib.typ" + "targetUri": "-/lib.typ" } ] diff --git a/crates/tinymist-query/src/fixtures/goto_definition/snaps/test@import_package_self.typ.snap b/crates/tinymist-query/src/fixtures/goto_definition/snaps/test@import_package_self.typ.snap index a6072910..2093d890 100644 --- a/crates/tinymist-query/src/fixtures/goto_definition/snaps/test@import_package_self.typ.snap +++ b/crates/tinymist-query/src/fixtures/goto_definition/snaps/test@import_package_self.typ.snap @@ -9,6 +9,6 @@ snapshot_kind: text "originSelectionRange": "1:20:1:27", "targetRange": "0:0:0:0", "targetSelectionRange": "0:0:0:0", - "targetUri": "lib.typ" + "targetUri": "-/lib.typ" } ] diff --git a/crates/tinymist-query/src/fixtures/rename/snaps/test@module_path.typ.snap b/crates/tinymist-query/src/fixtures/rename/snaps/test@module_path.typ.snap index 48774544..779e8280 100644 --- a/crates/tinymist-query/src/fixtures/rename/snaps/test@module_path.typ.snap +++ b/crates/tinymist-query/src/fixtures/rename/snaps/test@module_path.typ.snap @@ -19,7 +19,7 @@ input_file: crates/tinymist-query/src/fixtures/rename/module_path.typ }, { "kind": "rename", - "newUri": "variable.typ/../new_name.typ", + "newUri": "new_name.typ", "oldUri": "variable.typ" } ] diff --git a/crates/tinymist-query/src/fixtures/rename/snaps/test@module_path_alias.typ.snap b/crates/tinymist-query/src/fixtures/rename/snaps/test@module_path_alias.typ.snap index e2cddaea..00541c7b 100644 --- a/crates/tinymist-query/src/fixtures/rename/snaps/test@module_path_alias.typ.snap +++ b/crates/tinymist-query/src/fixtures/rename/snaps/test@module_path_alias.typ.snap @@ -19,7 +19,7 @@ input_file: crates/tinymist-query/src/fixtures/rename/module_path_alias.typ }, { "kind": "rename", - "newUri": "variable.typ/../new_name.typ", + "newUri": "new_name.typ", "oldUri": "variable.typ" } ] diff --git a/crates/tinymist-query/src/fixtures/rename/snaps/test@module_path_non_cano.typ.snap b/crates/tinymist-query/src/fixtures/rename/snaps/test@module_path_non_cano.typ.snap index a8e81bfc..a629f7af 100644 --- a/crates/tinymist-query/src/fixtures/rename/snaps/test@module_path_non_cano.typ.snap +++ b/crates/tinymist-query/src/fixtures/rename/snaps/test@module_path_non_cano.typ.snap @@ -8,7 +8,7 @@ input_file: crates/tinymist-query/src/fixtures/rename/module_path_non_cano.typ { "edits": [ { - "newText": "\"../../new_name.typ\" as variable", + "newText": "\"new_name.typ\" as variable", "range": "0:29:0:51" } ], @@ -19,7 +19,7 @@ input_file: crates/tinymist-query/src/fixtures/rename/module_path_non_cano.typ }, { "kind": "rename", - "newUri": "variable.typ/../../../new_name.typ", + "newUri": "new_name.typ", "oldUri": "variable.typ" } ] diff --git a/crates/tinymist-query/src/fixtures/rename/snaps/test@module_path_star.typ.snap b/crates/tinymist-query/src/fixtures/rename/snaps/test@module_path_star.typ.snap index daa7ce70..9a3aec58 100644 --- a/crates/tinymist-query/src/fixtures/rename/snaps/test@module_path_star.typ.snap +++ b/crates/tinymist-query/src/fixtures/rename/snaps/test@module_path_star.typ.snap @@ -19,7 +19,7 @@ input_file: crates/tinymist-query/src/fixtures/rename/module_path_star.typ }, { "kind": "rename", - "newUri": "variable.typ/../new_name.typ", + "newUri": "new_name.typ", "oldUri": "variable.typ" } ] diff --git a/crates/tinymist-query/src/fixtures/rename/snaps/test@resources.typ.snap b/crates/tinymist-query/src/fixtures/rename/snaps/test@resources.typ.snap index 2ffcd5b7..adba4b14 100644 --- a/crates/tinymist-query/src/fixtures/rename/snaps/test@resources.typ.snap +++ b/crates/tinymist-query/src/fixtures/rename/snaps/test@resources.typ.snap @@ -23,7 +23,7 @@ input_file: crates/tinymist-query/src/fixtures/rename/resources.typ }, { "kind": "rename", - "newUri": "lib.typ/../new_name.typ", + "newUri": "new_name.typ", "oldUri": "lib.typ" } ] diff --git a/crates/tinymist-query/src/rename.rs b/crates/tinymist-query/src/rename.rs index 00437c66..0b10ac17 100644 --- a/crates/tinymist-query/src/rename.rs +++ b/crates/tinymist-query/src/rename.rs @@ -60,14 +60,15 @@ impl StatefulRequest for RenameRequest { // todo: rename in untitled files let old_path = ctx.path_for_id(def_fid).ok()?.to_err().ok()?; + let new_path = Path::new(new_path_str.as_str()); let rename_loc = Path::new(ref_path_str.as_str()); - let diff = pathdiff::diff_paths(Path::new(&new_path_str), rename_loc)?; + let diff = tinymist_std::path::diff(new_path, rename_loc)?; if diff.is_absolute() { - log::info!("bad rename: absolute path, base: {rename_loc:?}, new: {new_path_str}, diff: {diff:?}"); + log::info!("bad rename: absolute path, base: {rename_loc:?}, new: {new_path:?}, diff: {diff:?}"); return None; } - let new_path = old_path.join(&diff); + let new_path = old_path.join(&diff).clean(); let old_uri = path_to_url(&old_path).ok()?; let new_uri = path_to_url(&new_path).ok()?; diff --git a/crates/tinymist-query/src/tests.rs b/crates/tinymist-query/src/tests.rs index f138ac67..5a1eafcf 100644 --- a/crates/tinymist-query/src/tests.rs +++ b/crates/tinymist-query/src/tests.rs @@ -465,10 +465,12 @@ pub(crate) fn file_path(uri: &str) -> String { } else { PathBuf::from("/root") }; - let uri = uri.replace("file://", ""); - let abs_path = Path::new(&uri).strip_prefix(root).map(|s| s.as_os_str()); - let rel_path = abs_path.unwrap_or_else(|_| Path::new(&uri).file_name().unwrap()); - unix_slash(Path::new(rel_path.to_str().unwrap())) + let uri = lsp_types::Url::parse(uri).unwrap().to_file_path().unwrap(); + let abs_path = Path::new(&uri).strip_prefix(root).map(|p| p.to_owned()); + let rel_path = + abs_path.unwrap_or_else(|_| Path::new("-").join(Path::new(&uri).iter().last().unwrap())); + + unix_slash(&rel_path) } pub struct HashRepr(pub T); diff --git a/crates/tinymist-query/src/will_rename_files.rs b/crates/tinymist-query/src/will_rename_files.rs index 09bf93e6..365cd057 100644 --- a/crates/tinymist-query/src/will_rename_files.rs +++ b/crates/tinymist-query/src/will_rename_files.rs @@ -21,7 +21,7 @@ impl StatefulRequest for WillRenameFilesRequest { self.paths .into_iter() .map(|(left, right)| { - let diff = pathdiff::diff_paths(&right, &left)?; + let diff = tinymist_std::path::diff(&right, &left)?; log::info!("did rename diff: {diff:?}"); if diff.is_absolute() { log::info!( diff --git a/crates/tinymist-std/Cargo.toml b/crates/tinymist-std/Cargo.toml index 55fe34bd..f196c0d0 100644 --- a/crates/tinymist-std/Cargo.toml +++ b/crates/tinymist-std/Cargo.toml @@ -21,6 +21,7 @@ fxhash.workspace = true log.workspace = true tinymist-analysis.workspace = true path-clean.workspace = true +pathdiff.workspace = true parking_lot.workspace = true rustc-hash.workspace = true serde = { workspace = true, features = ["derive"] } diff --git a/crates/tinymist-std/src/path.rs b/crates/tinymist-std/src/path.rs index a6707e7e..424a2f92 100644 --- a/crates/tinymist-std/src/path.rs +++ b/crates/tinymist-std/src/path.rs @@ -1,6 +1,7 @@ //! Path utilities. -use std::path::{Component, Path}; +use std::borrow::Cow; +use std::path::{Component, Path, PathBuf}; pub use path_clean::PathClean; @@ -48,6 +49,24 @@ pub fn unix_slash(root: &Path) -> String { /// Get the path cleaned as a platform-style string. pub use path_clean::clean; +/// Construct a relative path from a provided base directory path to the +/// provided path. +pub fn diff(fr: &Path, to: &Path) -> Option { + // Because of , we have to clean the path + // before diff. + fn clean_for_diff(p: &Path) -> Cow<'_, Path> { + if p.components() + .any(|c| matches!(c, Component::ParentDir | Component::CurDir)) + { + Cow::Owned(p.clean()) + } else { + Cow::Borrowed(p) + } + } + + pathdiff::diff_paths(clean_for_diff(fr).as_ref(), clean_for_diff(to).as_ref()) +} + #[cfg(test)] mod test { use std::path::{Path, PathBuf}; diff --git a/crates/tinymist-task/Cargo.toml b/crates/tinymist-task/Cargo.toml index f4e81a2e..e25c26ba 100644 --- a/crates/tinymist-task/Cargo.toml +++ b/crates/tinymist-task/Cargo.toml @@ -21,7 +21,6 @@ dirs.workspace = true ecow.workspace = true log.workspace = true parking_lot.workspace = true -pathdiff.workspace = true tokio = { workspace = true, features = ["sync"] } rayon.workspace = true rpds.workspace = true diff --git a/crates/tinymist-task/src/primitives.rs b/crates/tinymist-task/src/primitives.rs index 1ee3ebc6..a101bddb 100644 --- a/crates/tinymist-task/src/primitives.rs +++ b/crates/tinymist-task/src/primitives.rs @@ -405,7 +405,7 @@ impl ResourcePath { inp.to_path_buf() } else { let cwd = std::env::current_dir().unwrap(); - pathdiff::diff_paths(inp, &cwd).unwrap() + tinymist_std::path::diff(inp, &cwd).unwrap() }; let rel = unix_slash(&rel); ResourcePath("file".into(), rel.to_string()) @@ -431,7 +431,7 @@ impl ResourcePath { if self.0 == "file" { let path = Path::new(&self.1); if path.is_absolute() { - Some(pathdiff::diff_paths(path, base).unwrap_or_else(|| path.to_owned())) + Some(tinymist_std::path::diff(path, base).unwrap_or_else(|| path.to_owned())) } else { Some(path.to_owned()) } diff --git a/crates/tinymist/Cargo.toml b/crates/tinymist/Cargo.toml index 0e299784..ed316c68 100644 --- a/crates/tinymist/Cargo.toml +++ b/crates/tinymist/Cargo.toml @@ -44,7 +44,6 @@ lsp-types.workspace = true log.workspace = true once_cell.workspace = true open.workspace = true -pathdiff.workspace = true parking_lot.workspace = true paste.workspace = true rayon.workspace = true