copy-tracking: improve --summary and add --stat

- add support for copy tracking to `diff --stat`
- switch `--summary` to match git's output more closely
- rework `show_diff_summary` signature to be more consistent

CHANGELOG.md

@@ -15,7 +15,7 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 ### New features
 
 * The following diff formats now include information about copies and moves:
-  `--color-words`, `--summary`
+  `--color-words`, `--stat`, `--summary`
 
 ### Fixed bugs
 

cli/Cargo.toml

@@ -93,6 +93,7 @@ toml_edit = { workspace = true }
 tracing = { workspace = true }
 tracing-chrome = { workspace = true }
 tracing-subscriber = { workspace = true }
+unicode-segmentation = "1.11.0"
 unicode-width = { workspace = true }
 
 [target.'cfg(unix)'.dependencies]

cli/src/commit_templater.rs

@@ -1394,14 +1394,17 @@ fn builtin_tree_diff_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, T
             let path_converter = language.path_converter;
             let template = self_property
                 .map(move |diff| {
+                    let from_tree = diff.from_tree.clone();
                     let to_tree = diff.to_tree.clone();
-                    diff.into_formatted(move |formatter, _store, tree_diff| {
+                    let matcher = diff.matcher.clone();
+                    diff.into_formatted(move |formatter, _store, _tree_diff| {
                         diff_util::show_diff_summary(
                             formatter,
-                            tree_diff,
                             path_converter,
-                            &Default::default(),
+                            &from_tree,
                             &to_tree,
+                            matcher.as_ref(),
+                            &Default::default(),
                         )
                     })
                 })

cli/src/diff_util.rs

@@ -280,12 +280,17 @@ impl<'a> DiffRenderer<'a> {
         for format in &self.formats {
             match format {
                 DiffFormat::Summary => {
-                    let tree_diff = from_tree.diff_stream(to_tree, matcher, copy_records);
-                    show_diff_summary(formatter, tree_diff, path_converter, copy_records, to_tree)?;
+                    show_diff_summary(
+                        formatter,
+                        path_converter,
+                        from_tree,
+                        to_tree,
+                        matcher,
+                        copy_records,
+                    )?;
                 }
                 DiffFormat::Stat => {
-                    let no_copy_tracking = Default::default();
-                    let tree_diff = from_tree.diff_stream(to_tree, matcher, &no_copy_tracking);
+                    let tree_diff = from_tree.diff_stream(to_tree, matcher, copy_records);
                     show_diff_stat(formatter, store, tree_diff, path_converter, width)?;
                 }
                 DiffFormat::Types => {
@@ -1146,18 +1151,26 @@ pub fn show_git_diff(
     .block_on()
 }
 
-// TODO rework this signature to pass both from_tree and to_tree explicitly
 #[instrument(skip_all)]
 pub fn show_diff_summary(
     formatter: &mut dyn Formatter,
-    mut tree_diff: TreeDiffStream,
     path_converter: &RepoPathUiConverter,
-    copy_records: &CopyRecords,
+    from_tree: &MergedTree,
     to_tree: &MergedTree,
+    matcher: &dyn Matcher,
+    copy_records: &CopyRecords,
 ) -> io::Result<()> {
+    let mut tree_diff = from_tree.diff_stream(to_tree, matcher, copy_records);
+
     let copied_sources: HashSet<&RepoPath> = copy_records
         .iter()
-        .map(|record| record.source.as_ref())
+        .filter_map(|record| {
+            if matcher.matches(&record.target) {
+                Some(record.source.as_ref())
+            } else {
+                None
+            }
+        })
         .collect();
 
     async {
@@ -1171,16 +1184,11 @@ pub fn show_diff_summary(
             let after_ui_path = path_converter.format_file_path(&after_path);
             if before_path != after_path {
                 let before_ui_path = path_converter.format_file_path(&before_path);
+                let path = text_util::render_copied_path(&before_ui_path, &after_ui_path);
                 if to_tree.path_value(&before_path).unwrap().is_absent() {
-                    writeln!(
-                        formatter.labeled("renamed"),
-                        "R {before_ui_path} => {after_ui_path}"
-                    )?
+                    writeln!(formatter.labeled("renamed"), "R {path}")?
                 } else {
-                    writeln!(
-                        formatter.labeled("copied"),
-                        "C {before_ui_path} => {after_ui_path}"
-                    )?
+                    writeln!(formatter.labeled("copied"), "C {path}")?
                 }
             } else {
                 let path = after_ui_path;
@@ -1206,6 +1214,7 @@ struct DiffStat {
     path: String,
     added: usize,
     removed: usize,
+    is_deletion: bool,
 }
 
 fn get_diff_stat(
@@ -1233,6 +1242,7 @@ fn get_diff_stat(
         path,
         added,
         removed,
+        is_deletion: right_content.contents.is_empty(),
     }
 }
 
@@ -1244,21 +1254,28 @@ pub fn show_diff_stat(
     display_width: usize,
 ) -> Result<(), DiffRenderError> {
     let mut stats: Vec<DiffStat> = vec![];
+    let mut unresolved_renames = HashSet::<String>::new();
     let mut max_path_width = 0;
     let mut max_diffs = 0;
 
     let mut diff_stream = materialized_diff_stream(store, tree_diff);
     async {
         while let Some(MaterializedTreeDiffEntry {
-            source: _, // TODO handle copy tracking
-            target: repo_path,
+            source: left_path,
+            target: right_path,
             value: diff,
         }) = diff_stream.next().await
         {
             let (left, right) = diff?;
-            let path = path_converter.format_file_path(&repo_path);
-            let left_content = diff_content(&repo_path, left)?;
-            let right_content = diff_content(&repo_path, right)?;
+            let left_content = diff_content(&left_path, left)?;
+            let right_content = diff_content(&right_path, right)?;
+
+            let left_ui_path = path_converter.format_file_path(&left_path);
+            let right_ui_path = path_converter.format_file_path(&right_path);
+            if left_ui_path != right_ui_path {
+                unresolved_renames.insert(left_ui_path.clone());
+            }
+            let path = text_util::render_copied_path(&left_ui_path, &right_ui_path);
             max_path_width = max(max_path_width, path.width());
             let stat = get_diff_stat(path, &left_content, &right_content);
             max_diffs = max(max_diffs, stat.added + stat.removed);
@@ -1283,10 +1300,15 @@ pub fn show_diff_stat(
 
     let mut total_added = 0;
     let mut total_removed = 0;
-    let total_files = stats.len();
+    let mut total_files = 0;
     for stat in &stats {
+        if stat.is_deletion && unresolved_renames.contains(&stat.path) {
+            continue;
+        }
+
         total_added += stat.added;
         total_removed += stat.removed;
+        total_files += 1;
         let bar_added = (stat.added as f64 * factor).ceil() as usize;
         let bar_removed = (stat.removed as f64 * factor).ceil() as usize;
         // replace start of path with ellipsis if the path is too long

cli/src/text_util.rs

@@ -15,6 +15,8 @@
 use std::borrow::Cow;
 use std::{cmp, io};
 
+use itertools::Itertools;
+use unicode_segmentation::UnicodeSegmentation;
 use unicode_width::UnicodeWidthChar as _;
 
 use crate::formatter::{FormatRecorder, Formatter};
@@ -35,6 +37,33 @@ pub fn split_email(email: &str) -> (&str, Option<&str>) {
     }
 }
 
+pub fn render_copied_path(source: &str, target: &str) -> String {
+    if source == target {
+        return target.into();
+    }
+    let prefix = UnicodeSegmentation::split_word_bound_indices(source)
+        .zip(UnicodeSegmentation::split_word_bound_indices(target))
+        .take_while_inclusive(|((_, source_word), (_, target_word))| source_word == target_word)
+        .last()
+        .map(|((source_index, _), (target_index, _))| (source_index, target_index))
+        .unwrap_or((0, 0));
+    let suffix = UnicodeSegmentation::split_word_bound_indices(source)
+        .rev()
+        .zip(UnicodeSegmentation::split_word_bound_indices(target).rev())
+        .take_while_inclusive(|((_, source_word), (_, target_word))| source_word == target_word)
+        .last()
+        .map(|((source_index, _), (target_index, _))| (source_index, target_index))
+        .unwrap_or((source.len(), target.len()));
+
+    format!(
+        "{}{{{} => {}}}{}",
+        &source[0..prefix.0],
+        &source[prefix.0..suffix.0.max(prefix.0)],
+        &target[prefix.1..suffix.1.max(prefix.1)],
+        &source[suffix.0..]
+    )
+}
+
 /// Shortens `text` to `max_width` by removing leading characters. `ellipsis` is
 /// added if the `text` gets truncated.
 ///
@@ -629,4 +658,30 @@ mod tests {
             "foo\n",
         );
     }
+
+    #[test]
+    fn test_render_copied_path() {
+        assert_eq!(
+            render_copied_path("one/two/three", "one/two/three"),
+            "one/two/three"
+        );
+        assert_eq!(
+            render_copied_path("two/three", "four/three"),
+            "{two => four}/three"
+        );
+        assert_eq!(
+            render_copied_path("one/two/three", "one/four/three"),
+            "one/{two => four}/three"
+        );
+        assert_eq!(
+            render_copied_path("one/two/three", "one/three"),
+            "one/{two => }/three"
+        );
+        assert_eq!(
+            render_copied_path("one/two", "one/four"),
+            "one/{two => four}"
+        );
+        assert_eq!(render_copied_path("two", "four"), "{two => four}");
+        assert_eq!(render_copied_path("file1", "file2"), "{file1 => file2}");
+    }
 }

cli/tests/test_diff_command.rs

@@ -65,7 +65,7 @@ fn test_diff_basic() {
     let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "-s"]);
     insta::assert_snapshot!(stdout, @r###"
     M file2
-    R file1 => file3
+    R {file1 => file3}
     "###);
 
     let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--types"]);
@@ -158,7 +158,7 @@ fn test_diff_basic() {
     let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "-s", "--git"]);
     insta::assert_snapshot!(stdout, @r###"
     M file2
-    R file1 => file3
+    R {file1 => file3}
     diff --git a/file1 b/file1
     deleted file mode 100644
     index 257cc5642c..0000000000
@@ -186,15 +186,15 @@ fn test_diff_basic() {
 
     let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--stat"]);
     insta::assert_snapshot!(stdout, @r###"
-    file1 | 1 -
-    file2 | 3 ++-
-    file3 | 1 +
-    3 files changed, 3 insertions(+), 2 deletions(-)
+    file2            | 3 ++-
+    {file1 => file3} | 0
+    2 files changed, 2 insertions(+), 1 deletion(-)
     "###);
 
     // Filter by glob pattern
     let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "-s", "glob:file[12]"]);
     insta::assert_snapshot!(stdout, @r###"
+    D file1
     M file2
     "###);
 
@@ -213,7 +213,7 @@ fn test_diff_basic() {
     );
     insta::assert_snapshot!(stdout.replace('\\', "/"), @r###"
     M repo/file2
-    R repo/file1 => repo/file3
+    R repo/{file1 => file3}
     "###);
     insta::assert_snapshot!(stderr.replace('\\', "/"), @r###"
     Warning: No matching entries for paths: repo/x, repo/y/z