Skip to content

Commit

Permalink
copy-tracking: add support for diff --git
Browse files Browse the repository at this point in the history
  • Loading branch information
fowles committed Aug 15, 2024
1 parent 5694f23 commit 95e8dd5
Show file tree
Hide file tree
Showing 9 changed files with 177 additions and 172 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
18 changes: 15 additions & 3 deletions cli/src/commit_templater.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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
)
})
})
Expand Down
59 changes: 46 additions & 13 deletions cli/src/diff_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,9 +307,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);
Expand Down Expand Up @@ -1097,23 +1103,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) {
Expand All @@ -1126,6 +1149,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)?.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}")?;
Expand All @@ -1138,19 +1171,19 @@ pub fn show_git_diff(
}
(None, None) => panic!("either left or right part should be present"),
}
io::Result::Ok(())
Ok::<(), DiffRenderError>(())
})?;

if left_part.content.contents == right_part.content.contents {
continue; // no content hunks
}

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 {
Expand Down
18 changes: 9 additions & 9 deletions cli/tests/test_acls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ fn test_diff() {
let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--summary"]);
insta::assert_snapshot!(stdout.replace('\\', "/"), @r###"
M a-first
A added-secret
C {a-first => added-secret}
D deleted-secret
M dir/secret
M modified-secret
Expand All @@ -61,21 +61,21 @@ fn test_diff() {
let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--types"]);
insta::assert_snapshot!(stdout.replace('\\', "/"), @r###"
FF a-first
-F added-secret
FF {a-first => added-secret}
F- deleted-secret
FF dir/secret
FF modified-secret
FF z-last
"###);
let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--stat"]);
insta::assert_snapshot!(stdout.replace('\\', "/"), @r###"
a-first | 2 +-
added-secret | 1 +
deleted-secret | 1 -
dir/secret | 0
modified-secret | 0
z-last | 2 +-
6 files changed, 3 insertions(+), 3 deletions(-)
a-first | 2 +-
{a-first => added-secret} | 2 +-
deleted-secret | 1 -
dir/secret | 0
modified-secret | 0
z-last | 2 +-
6 files changed, 3 insertions(+), 4 deletions(-)
"###);
let assert = test_env
.jj_cmd(&repo_path, &["diff", "--git"])
Expand Down
Loading

0 comments on commit 95e8dd5

Please sign in to comment.