Skip to content

Commit

Permalink
copy-tracking: implement copy tracking for external tools
Browse files Browse the repository at this point in the history
  • Loading branch information
fowles committed Aug 16, 2024
1 parent 7baf667 commit 2f2e5fb
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 26 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
38 changes: 27 additions & 11 deletions cli/src/diff_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
}
Expand Down Expand Up @@ -753,12 +753,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(
Expand All @@ -772,35 +775,48 @@ 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");
let right_wc_dir = temp_dir.path().join("right");
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,
Expand Down
29 changes: 15 additions & 14 deletions cli/tests/test_diff_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -1269,17 +1270,17 @@ 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
==
file3
--
file3
==
file1
--
file4
"###);

// diff with file patterns
Expand All @@ -1293,20 +1294,20 @@ fn test_diff_external_file_by_file_tool() {

insta::assert_snapshot!(
test_env.jj_cmd_success(&repo_path, &["log", "-p", config]), @r###"
@ rlvkpnrz [email protected] 2001-02-03 08:05:09 f12f62fb
@ rlvkpnrz [email protected] 2001-02-03 08:05:09 7b01704a
│ (no description set)
│ ==
│ file1
│ --
│ file1
│ ==
│ file2
│ --
│ file2
│ ==
│ file3
│ --
│ file3
│ ==
│ file1
│ --
│ file4
○ qpvuntsm [email protected] 2001-02-03 08:05:08 6e485984
│ (no description set)
│ ==
Expand All @@ -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 <[email protected]> (2001-02-03 08:05:09)
Committer: Test User <[email protected]> (2001-02-03 08:05:09)
(no description set)
==
file1
--
file1
==
file2
--
Expand All @@ -1341,6 +1338,10 @@ fn test_diff_external_file_by_file_tool() {
file3
--
file3
==
file1
--
file4
"###);
}

Expand Down

0 comments on commit 2f2e5fb

Please sign in to comment.