From 07589d40d19deb32e21d780cfb57beb34f526721 Mon Sep 17 00:00:00 2001 From: Matt Kulukundis Date: Wed, 14 Aug 2024 10:20:17 -0400 Subject: [PATCH] copy-tracking: add support for `diff --git` --- CHANGELOG.md | 2 +- cli/src/commit_templater.rs | 18 +++++++-- cli/src/diff_util.rs | 57 ++++++++++++++++++++++------ cli/tests/test_diff_command.rs | 67 +++++++++------------------------ cli/tests/test_show_command.rs | 68 ++++++---------------------------- 5 files changed, 90 insertions(+), 122 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f5aad64ef96..53c0450f724 100644 --- a/CHANGELOG.md +++ b/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`, `--stat`, `--summary`, `--types` + `--color-words`, `--git`, `--stat`, `--summary`, `--types` * A tilde (`~`) at the start of the path will now be expanded to the user's home directory when configuring a `signing.key` for SSH commit signing. diff --git a/cli/src/commit_templater.rs b/cli/src/commit_templater.rs index b821d16ff34..d7c0a03b88a 100644 --- a/cli/src/commit_templater.rs +++ b/cli/src/commit_templater.rs @@ -1366,8 +1366,20 @@ fn builtin_tree_diff_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, T let template = (self_property, context_property) .map(|(diff, context)| { let context = context.unwrap_or(diff_util::DEFAULT_CONTEXT_LINES); - diff.into_formatted(move |formatter, store, tree_diff| { - diff_util::show_git_diff(formatter, store, tree_diff, context) + // TODO: don't pass separate copies of from_tree/to_tree/matcher + let from_tree = diff.from_tree.clone(); + let to_tree = diff.to_tree.clone(); + let matcher = diff.matcher.clone(); + diff.into_formatted(move |formatter, store, _tree_diff| { + diff_util::show_git_diff( + formatter, + store, + &from_tree, + &to_tree, + matcher.as_ref(), + &Default::default(), // TODO: real copy tracking + context, + ) }) }) .into_template(); @@ -1405,7 +1417,7 @@ fn builtin_tree_diff_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, T &from_tree, &to_tree, matcher.as_ref(), - &Default::default(), + &Default::default(), // TODO: real copy tracking ) }) }) diff --git a/cli/src/diff_util.rs b/cli/src/diff_util.rs index a7b55214461..1b08db8d2ed 100644 --- a/cli/src/diff_util.rs +++ b/cli/src/diff_util.rs @@ -308,9 +308,15 @@ impl<'a> DiffRenderer<'a> { show_names(formatter, tree_diff, path_converter)?; } DiffFormat::Git { context } => { - let no_copy_tracking = Default::default(); - let tree_diff = from_tree.diff_stream(to_tree, matcher, &no_copy_tracking); - show_git_diff(formatter, store, tree_diff, *context)?; + show_git_diff( + formatter, + store, + from_tree, + to_tree, + matcher, + copy_records, + *context, + )?; } DiffFormat::ColorWords { context } => { let tree_diff = from_tree.diff_stream(to_tree, matcher, copy_records); @@ -1098,23 +1104,40 @@ fn show_unified_diff_hunks( pub fn show_git_diff( formatter: &mut dyn Formatter, store: &Store, - tree_diff: TreeDiffStream, + from_tree: &MergedTree, + to_tree: &MergedTree, + matcher: &dyn Matcher, + copy_records: &CopyRecords, num_context_lines: usize, ) -> Result<(), DiffRenderError> { + let tree_diff = from_tree.diff_stream(to_tree, matcher, copy_records); let mut diff_stream = materialized_diff_stream(store, tree_diff); + let copied_sources = collect_copied_sources(copy_records, matcher); + async { while let Some(MaterializedTreeDiffEntry { - source: _, // TODO handle copy tracking - target: path, + source: left_path, + target: right_path, value: diff, }) = diff_stream.next().await { - let path_string = path.as_internal_file_string(); + let left_path_string = left_path.as_internal_file_string(); + let right_path_string = right_path.as_internal_file_string(); let (left_value, right_value) = diff?; - let left_part = git_diff_part(&path, left_value)?; - let right_part = git_diff_part(&path, right_value)?; + + let left_part = git_diff_part(&left_path, left_value)?; + let right_part = git_diff_part(&right_path, right_value)?; + + // Skip the "delete" entry when there is a rename. + if right_part.mode.is_none() && copied_sources.contains(left_path.as_ref()) { + continue; + } + formatter.with_label("file_header", |formatter| { - writeln!(formatter, "diff --git a/{path_string} b/{path_string}")?; + writeln!( + formatter, + "diff --git a/{left_path_string} b/{right_path_string}" + )?; let left_hash = &left_part.hash; let right_hash = &right_part.hash; match (left_part.mode, right_part.mode) { @@ -1127,6 +1150,16 @@ pub fn show_git_diff( writeln!(formatter, "index {left_hash}..{right_hash}")?; } (Some(left_mode), Some(right_mode)) => { + if left_path != right_path { + let operation = if to_tree.path_value(&left_path).unwrap().is_absent() { + "rename" + } else { + "copy" + }; + // TODO: include similarity index? + writeln!(formatter, "{operation} from {left_path_string}")?; + writeln!(formatter, "{operation} to {right_path_string}")?; + } if left_mode != right_mode { writeln!(formatter, "old mode {left_mode}")?; writeln!(formatter, "new mode {right_mode}")?; @@ -1147,11 +1180,11 @@ pub fn show_git_diff( } let left_path = match left_part.mode { - Some(_) => format!("a/{path_string}"), + Some(_) => format!("a/{left_path_string}"), None => "/dev/null".to_owned(), }; let right_path = match right_part.mode { - Some(_) => format!("b/{path_string}"), + Some(_) => format!("b/{right_path_string}"), None => "/dev/null".to_owned(), }; if left_part.content.is_binary || right_part.content.is_binary { diff --git a/cli/tests/test_diff_command.rs b/cli/tests/test_diff_command.rs index 5489a7b1772..15a87376731 100644 --- a/cli/tests/test_diff_command.rs +++ b/cli/tests/test_diff_command.rs @@ -80,7 +80,7 @@ fn test_diff_basic() { FF file2 "###); - let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--git"]); + let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--git", "file1"]); insta::assert_snapshot!(stdout, @r###" diff --git a/file1 b/file1 deleted file mode 100644 @@ -89,6 +89,10 @@ fn test_diff_basic() { +++ /dev/null @@ -1,1 +1,0 @@ -foo + "###); + + let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--git"]); + insta::assert_snapshot!(stdout, @r###" diff --git a/file2 b/file2 index 523a4a9de8..485b56a572 100644 --- a/file2 @@ -98,24 +102,13 @@ fn test_diff_basic() { -baz qux +bar +baz quux - diff --git a/file3 b/file3 - new file mode 100644 - index 0000000000..257cc5642c - --- /dev/null - +++ b/file3 - @@ -1,0 +1,1 @@ - +foo + diff --git a/file1 b/file3 + rename from file1 + rename to file3 "###); let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--git", "--context=0"]); insta::assert_snapshot!(stdout, @r###" - diff --git a/file1 b/file1 - deleted file mode 100644 - index 257cc5642c..0000000000 - --- a/file1 - +++ /dev/null - @@ -1,1 +1,0 @@ - -foo diff --git a/file2 b/file2 index 523a4a9de8..485b56a572 100644 --- a/file2 @@ -124,24 +117,13 @@ fn test_diff_basic() { -baz qux +bar +baz quux - diff --git a/file3 b/file3 - new file mode 100644 - index 0000000000..257cc5642c - --- /dev/null - +++ b/file3 - @@ -1,0 +1,1 @@ - +foo + diff --git a/file1 b/file3 + rename from file1 + rename to file3 "###); let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--git", "--color=debug"]); insta::assert_snapshot!(stdout, @r###" - <> - <> - <> - <> - <> - <> - <><> <> <> <> @@ -151,26 +133,15 @@ 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} - diff --git a/file1 b/file1 - deleted file mode 100644 - index 257cc5642c..0000000000 - --- a/file1 - +++ /dev/null - @@ -1,1 +1,0 @@ - -foo diff --git a/file2 b/file2 index 523a4a9de8..485b56a572 100644 --- a/file2 @@ -180,13 +151,9 @@ fn test_diff_basic() { -baz qux +bar +baz quux - diff --git a/file3 b/file3 - new file mode 100644 - index 0000000000..257cc5642c - --- /dev/null - +++ b/file3 - @@ -1,0 +1,1 @@ - +foo + diff --git a/file1 b/file3 + rename from file1 + rename to file3 "###); let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--stat"]); diff --git a/cli/tests/test_show_command.rs b/cli/tests/test_show_command.rs index 4a1d9cdbc9d..0500cd486ff 100644 --- a/cli/tests/test_show_command.rs +++ b/cli/tests/test_show_command.rs @@ -136,13 +136,6 @@ fn test_show_basic() { (no description set) - diff --git a/file1 b/file1 - deleted file mode 100644 - index 257cc5642c..0000000000 - --- a/file1 - +++ /dev/null - @@ -1,1 +1,0 @@ - -foo diff --git a/file2 b/file2 index 523a4a9de8..485b56a572 100644 --- a/file2 @@ -152,13 +145,9 @@ fn test_show_basic() { -baz qux +bar +baz quux - diff --git a/file3 b/file3 - new file mode 100644 - index 0000000000..257cc5642c - --- /dev/null - +++ b/file3 - @@ -1,0 +1,1 @@ - +foo + diff --git a/file1 b/file3 + rename from file1 + rename to file3 "###); let stdout = test_env.jj_cmd_success(&repo_path, &["show", "--git", "--context=0"]); @@ -170,13 +159,6 @@ fn test_show_basic() { (no description set) - diff --git a/file1 b/file1 - deleted file mode 100644 - index 257cc5642c..0000000000 - --- a/file1 - +++ /dev/null - @@ -1,1 +1,0 @@ - -foo diff --git a/file2 b/file2 index 523a4a9de8..485b56a572 100644 --- a/file2 @@ -185,13 +167,9 @@ fn test_show_basic() { -baz qux +bar +baz quux - diff --git a/file3 b/file3 - new file mode 100644 - index 0000000000..257cc5642c - --- /dev/null - +++ b/file3 - @@ -1,0 +1,1 @@ - +foo + diff --git a/file1 b/file3 + rename from file1 + rename to file3 "###); let stdout = test_env.jj_cmd_success(&repo_path, &["show", "--git", "--color=debug"]); @@ -203,13 +181,6 @@ fn test_show_basic() { <> - <> - <> - <> - <> - <> - <> - <><> <> <> <> @@ -219,13 +190,9 @@ fn test_show_basic() { <><><> <><> <><><> - <> - <> - <> - <> - <> - <> - <><> + <> + <> + <> "###); let stdout = test_env.jj_cmd_success(&repo_path, &["show", "-s", "--git"]); @@ -239,13 +206,6 @@ fn test_show_basic() { M file2 R {file1 => file3} - diff --git a/file1 b/file1 - deleted file mode 100644 - index 257cc5642c..0000000000 - --- a/file1 - +++ /dev/null - @@ -1,1 +1,0 @@ - -foo diff --git a/file2 b/file2 index 523a4a9de8..485b56a572 100644 --- a/file2 @@ -255,13 +215,9 @@ fn test_show_basic() { -baz qux +bar +baz quux - diff --git a/file3 b/file3 - new file mode 100644 - index 0000000000..257cc5642c - --- /dev/null - +++ b/file3 - @@ -1,0 +1,1 @@ - +foo + diff --git a/file1 b/file3 + rename from file1 + rename to file3 "###); let stdout = test_env.jj_cmd_success(&repo_path, &["show", "--stat"]);