From d875b39cb65e83977fc4b10e197ac1c21cb0d063 Mon Sep 17 00:00:00 2001 From: Matt Kulukundis Date: Thu, 15 Aug 2024 11:22:34 -0400 Subject: [PATCH] copy-tracking: implement copy tracking for external tools --- CHANGELOG.md | 3 ++- cli/src/diff_util.rs | 38 ++++++++++++++++++++++++---------- cli/tests/test_diff_command.rs | 29 +++++++++++++------------- 3 files changed, 44 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 53c0450f72..c00e5e8557 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,8 @@ 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`, `--git`, `--stat`, `--summary`, `--types` + `--color-words`, `--git`, `--stat`, `--summary`, `--types`, and external diff + tools in file-by-file mode. * 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/diff_util.rs b/cli/src/diff_util.rs index 3f3f435fa0..80ee9d9285 100644 --- a/cli/src/diff_util.rs +++ b/cli/src/diff_util.rs @@ -324,15 +324,15 @@ impl<'a> DiffRenderer<'a> { DiffFormat::Tool(tool) => { match tool.diff_invocation_mode { DiffToolMode::FileByFile => { - 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_file_by_file_diff( ui, formatter, store, tree_diff, path_converter, + matcher, + copy_records, tool, ) } @@ -760,12 +760,15 @@ pub fn show_color_words_diff( .block_on() } +#[allow(clippy::too_many_arguments)] pub fn show_file_by_file_diff( ui: &Ui, formatter: &mut dyn Formatter, store: &Store, tree_diff: TreeDiffStream, path_converter: &RepoPathUiConverter, + matcher: &dyn Matcher, + copy_records: &CopyRecords, tool: &ExternalMergeTool, ) -> Result<(), DiffRenderError> { fn create_file( @@ -779,6 +782,7 @@ pub fn show_file_by_file_diff( std::fs::write(&fs_path, content.contents)?; Ok(fs_path) } + let copied_sources = collect_copied_sources(copy_records, matcher); let temp_dir = new_utf8_temp_dir("jj-diff-")?; let left_wc_dir = temp_dir.path().join("left"); @@ -786,28 +790,40 @@ pub fn show_file_by_file_diff( let mut diff_stream = materialized_diff_stream(store, tree_diff); 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 ui_path = path_converter.format_file_path(&path); let (left_value, right_value) = diff?; + if right_value.is_absent() && copied_sources.contains(left_path.as_ref()) { + continue; + } + + let left_ui_path = path_converter.format_file_path(&left_path); + let right_ui_path = path_converter.format_file_path(&right_path); match (&left_value, &right_value) { - (_, MaterializedTreeValue::AccessDenied(source)) - | (MaterializedTreeValue::AccessDenied(source), _) => { + (_, MaterializedTreeValue::AccessDenied(source)) => { + write!( + formatter.labeled("access-denied"), + "Access denied to {right_ui_path}:" + )?; + writeln!(formatter, " {source}")?; + continue; + } + (MaterializedTreeValue::AccessDenied(source), _) => { write!( formatter.labeled("access-denied"), - "Access denied to {ui_path}:" + "Access denied to {left_ui_path}:" )?; writeln!(formatter, " {source}")?; continue; } _ => {} } - let left_path = create_file(&path, &left_wc_dir, left_value)?; - let right_path = create_file(&path, &right_wc_dir, right_value)?; + let left_path = create_file(&left_path, &left_wc_dir, left_value)?; + let right_path = create_file(&right_path, &right_wc_dir, right_value)?; invoke_external_diff( ui, diff --git a/cli/tests/test_diff_command.rs b/cli/tests/test_diff_command.rs index a1745cf344..59b233fc2f 100644 --- a/cli/tests/test_diff_command.rs +++ b/cli/tests/test_diff_command.rs @@ -1253,6 +1253,7 @@ fn test_diff_external_file_by_file_tool() { std::fs::remove_file(repo_path.join("file1")).unwrap(); std::fs::write(repo_path.join("file2"), "file2\nfile2\n").unwrap(); std::fs::write(repo_path.join("file3"), "file3\n").unwrap(); + std::fs::write(repo_path.join("file4"), "file1\n").unwrap(); let edit_script = test_env.set_up_fake_diff_editor(); std::fs::write( @@ -1269,10 +1270,6 @@ fn test_diff_external_file_by_file_tool() { insta::assert_snapshot!( test_env.jj_cmd_success(&repo_path, &["diff", config]), @r###" == - file1 - -- - file1 - == file2 -- file2 @@ -1280,6 +1277,10 @@ fn test_diff_external_file_by_file_tool() { file3 -- file3 + == + file1 + -- + file4 "###); // diff with file patterns @@ -1293,13 +1294,9 @@ fn test_diff_external_file_by_file_tool() { insta::assert_snapshot!( test_env.jj_cmd_success(&repo_path, &["log", "-p", config]), @r###" - @ rlvkpnrz test.user@example.com 2001-02-03 08:05:09 f12f62fb + @ rlvkpnrz test.user@example.com 2001-02-03 08:05:09 7b01704a │ (no description set) │ == - │ file1 - │ -- - │ file1 - │ == │ file2 │ -- │ file2 @@ -1307,6 +1304,10 @@ fn test_diff_external_file_by_file_tool() { │ file3 │ -- │ file3 + │ == + │ file1 + │ -- + │ file4 ○ qpvuntsm test.user@example.com 2001-02-03 08:05:08 6e485984 │ (no description set) │ == @@ -1322,17 +1323,13 @@ fn test_diff_external_file_by_file_tool() { insta::assert_snapshot!( test_env.jj_cmd_success(&repo_path, &["show", config]), @r###" - Commit ID: f12f62fb1d73629c1b44abd4d5bbb500d7f8b86c + Commit ID: 7b01704a670bc77d11ed117d362855cff1d4513b Change ID: rlvkpnrzqnoowoytxnquwvuryrwnrmlp Author: Test User (2001-02-03 08:05:09) Committer: Test User (2001-02-03 08:05:09) (no description set) - == - file1 - -- - file1 == file2 -- @@ -1341,6 +1338,10 @@ fn test_diff_external_file_by_file_tool() { file3 -- file3 + == + file1 + -- + file4 "###); }