Skip to content

Commit

Permalink
diff: do not add excessive number of context lines to last unified-di…
Browse files Browse the repository at this point in the history
…ff hunk

The last hunk could be truncated instead, but the .peekable() version is easier
to follow. If we truncated lines, we would have to adjust line ranges
accordingly.
  • Loading branch information
yuja committed Jul 2, 2024
1 parent ec709e7 commit f9a15ba
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 12 deletions.
16 changes: 8 additions & 8 deletions cli/src/diff_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -771,15 +771,20 @@ fn unified_diff_hunks<'content>(
lines: vec![],
};
let diff = Diff::for_tokenizer(&[left_content, right_content], diff::find_line_ranges);
for hunk in diff.hunks() {
let mut diff_hunks = diff.hunks().peekable();
while let Some(hunk) = diff_hunks.next() {
match hunk {
DiffHunk::Matching(content) => {
let mut lines = content.split_inclusive(|b| *b == b'\n').fuse();
if !current_hunk.lines.is_empty() {
// The previous hunk line should be either removed/added.
current_hunk.extend_context_lines(lines.by_ref().take(num_context_lines));
}
let before_lines = lines.by_ref().rev().take(num_context_lines).collect_vec();
let before_lines = if diff_hunks.peek().is_some() {
lines.by_ref().rev().take(num_context_lines).collect()
} else {
vec![] // No more hunks
};
let num_skip_lines = lines.count();
if num_skip_lines > 0 {
let left_start = current_hunk.left_line_range.end + num_skip_lines;
Expand All @@ -804,12 +809,7 @@ fn unified_diff_hunks<'content>(
}
}
}
// The last unified hunk might contain redundant "before" context lines.
if !current_hunk
.lines
.iter()
.all(|(diff_type, _line)| *diff_type == DiffLineType::Context)
{
if !current_hunk.lines.is_empty() {
hunks.push(current_hunk);
}
hunks
Expand Down
5 changes: 1 addition & 4 deletions cli/tests/test_diff_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -952,14 +952,13 @@ fn test_diff_leading_trailing_context() {

// N=5 <= 2 * num_context_lines: The last hunk wouldn't be split if
// trailing diff existed.
// FIXME: trailing context lines should be trimmed
let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--git", "--context=3"]);
insta::assert_snapshot!(stdout, @r###"
diff --git a/file1 b/file1
index 1bf57dee4a...69b3e1865c 100644
--- a/file1
+++ b/file1
@@ -3,10 +3,10 @@
@@ -3,8 +3,8 @@
3
4
5
Expand All @@ -969,8 +968,6 @@ fn test_diff_leading_trailing_context() {
7
8
9
10
11
"###);

// N=5 > 2 * num_context_lines: The last hunk should be split no matter
Expand Down

0 comments on commit f9a15ba

Please sign in to comment.