Merge pull request #756 from sourcefrog/similar-diff

Use similar rather than difflib to get better diffs
crate-ci · Feb 27, 2024 · 8812b27 · 8812b27
2 parents 3905071 + a03450a
commit 8812b27
Show file tree

Hide file tree

Showing 6 changed files with 29 additions and 65 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -75,7 +75,6 @@ indexmap = "2.2"
 time = { version = "0.3", features = ["formatting", "macros"] }
 dirs-next = "2.0"
 ignore = "0.4"
-difflib = "0.4"
 itertools = "0.12"
 once_cell = "1.19.0"
 clap = { version = "4.4.18", features = ["derive", "wrap_help"] }
@@ -88,6 +87,7 @@ trycmd = "0.14.21"
 anyhow = "1.0.79"
 concolor-control = { version = "0.0.7", features = ["auto"] }
 git-conventional = "0.12.4"
+similar = "2.4"
 
 [dev-dependencies]
 assert_fs = "1.1"

diff --git a/src/diff.rs b/src/diff.rs
@@ -0,0 +1,14 @@
+use std::path::Path;
+
+use similar::TextDiff;
+
+pub(crate) fn unified_diff(old: &str, new: &str, path: &Path, new_description: &str) -> String {
+    let diff = TextDiff::from_lines(old, new);
+    let path = path.display();
+    diff.unified_diff()
+        .header(
+            &format!("{path}\toriginal"),
+            &format!("{path}\t{new_description}"),
+        )
+        .to_string()
+}
diff --git a/src/lib.rs b/src/lib.rs
@@ -7,6 +7,7 @@
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
 
 pub mod config;
+mod diff;
 pub mod error;
 pub mod ops;
 pub mod shell;

diff --git a/src/ops/cargo.rs b/src/ops/cargo.rs
@@ -180,22 +180,9 @@ pub fn set_workspace_version(
 
     if dry_run {
         if manifest != original_manifest {
-            let display_path = manifest_path.display().to_string();
-            let old_lines: Vec<_> = original_manifest
-                .lines()
-                .map(|s| format!("{}\n", s))
-                .collect();
-            let new_lines: Vec<_> = manifest.lines().map(|s| format!("{}\n", s)).collect();
-            let diff = difflib::unified_diff(
-                &old_lines,
-                &new_lines,
-                display_path.as_str(),
-                display_path.as_str(),
-                "original",
-                "updated",
-                0,
-            );
-            log::debug!("change:\n{}", itertools::join(diff.into_iter(), ""));
+            let diff =
+                crate::diff::unified_diff(&original_manifest, &manifest, manifest_path, "updated");
+            log::debug!("change:\n{diff}");
         }
     } else {
         atomic_write(manifest_path, &manifest)?;
@@ -291,22 +278,9 @@ pub fn set_package_version(manifest_path: &Path, version: &str, dry_run: bool) -
 
     if dry_run {
         if manifest != original_manifest {
-            let display_path = manifest_path.display().to_string();
-            let old_lines: Vec<_> = original_manifest
-                .lines()
-                .map(|s| format!("{}\n", s))
-                .collect();
-            let new_lines: Vec<_> = manifest.lines().map(|s| format!("{}\n", s)).collect();
-            let diff = difflib::unified_diff(
-                &old_lines,
-                &new_lines,
-                display_path.as_str(),
-                display_path.as_str(),
-                "original",
-                "updated",
-                0,
-            );
-            log::debug!("change:\n{}", itertools::join(diff.into_iter(), ""));
+            let diff =
+                crate::diff::unified_diff(&original_manifest, &manifest, manifest_path, "updated");
+            log::debug!("change:\n{diff}");
         }
     } else {
         atomic_write(manifest_path, &manifest)?;
@@ -340,22 +314,9 @@ pub fn upgrade_dependency_req(
     let manifest = manifest.to_string();
     if manifest != original_manifest {
         if dry_run {
-            let display_path = manifest_path.display().to_string();
-            let old_lines: Vec<_> = original_manifest
-                .lines()
-                .map(|s| format!("{}\n", s))
-                .collect();
-            let new_lines: Vec<_> = manifest.lines().map(|s| format!("{}\n", s)).collect();
-            let diff = difflib::unified_diff(
-                &old_lines,
-                &new_lines,
-                display_path.as_str(),
-                display_path.as_str(),
-                "original",
-                "updated",
-                0,
-            );
-            log::debug!("change:\n{}", itertools::join(diff.into_iter(), ""));
+            let diff =
+                crate::diff::unified_diff(&original_manifest, &manifest, manifest_path, "updated");
+            log::debug!("change:\n{diff}");
         } else {
             atomic_write(manifest_path, &manifest)?;
         }

diff --git a/src/ops/replace.rs b/src/ops/replace.rs
@@ -118,25 +118,13 @@ pub fn do_file_replacements(
 
         if data != replaced {
             if dry_run {
-                let display_path = path.display().to_string();
-                let data_lines: Vec<_> = data.lines().map(|s| format!("{}\n", s)).collect();
-                let replaced_lines: Vec<_> = replaced.lines().map(|s| format!("{}\n", s)).collect();
-                let diff = difflib::unified_diff(
-                    &data_lines,
-                    &replaced_lines,
-                    display_path.as_str(),
-                    display_path.as_str(),
-                    "original",
-                    "replaced",
-                    0,
-                );
                 if noisy {
                     let _ = crate::ops::shell::status(
                         "Replacing",
                         format!(
                             "in {}\n{}",
                             path.display(),
-                            itertools::join(diff.into_iter(), "")
+                            crate::diff::unified_diff(&data, &replaced, &path, "replaced")
                         ),
                     );
                 } else {