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

@@ -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",

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

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

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:?}");

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"
  }
 ]

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"
  }
 ]

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"
   }
  ]

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"
   }
  ]

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"
   }
  ]

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"
   }
  ]

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"
   }
  ]

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()?;

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 uri = lsp_types::Url::parse(uri).unwrap().to_file_path().unwrap();
-    let abs_path = Path::new(&uri).strip_prefix(root).map(|s| s.as_os_str());
+    let abs_path = Path::new(&uri).strip_prefix(root).map(|p| p.to_owned());
-    let rel_path = abs_path.unwrap_or_else(|_| Path::new(&uri).file_name().unwrap());
+    let rel_path =
-    unix_slash(Path::new(rel_path.to_str().unwrap()))
+        abs_path.unwrap_or_else(|_| Path::new("-").join(Path::new(&uri).iter().last().unwrap()));
+
+    unix_slash(&rel_path)
 }
 
 pub struct HashRepr<T>(pub T);

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!(

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"] }

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<PathBuf> {
+    // Because of <https://github.com/Manishearth/pathdiff/issues/8>, 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};

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

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

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