Do not use "..." between diff chunks when it only replaces 1 line of the diff

The number of lines in the diff output is unchanged.

This makes diffs a little more readable when the "..." would otherwise hide a
single line of code that helps in understanding the surrounding context lines.

This change mostly rearranges the loop that consumes the diff lines, so it can
buffer up to num_context_lines*2+1 lines instead of just num_context_lines.
There's a bit of extra code to handle times when a "..." replaces the last line
of a diff.

Note that `jj diff --git` is unchanged, and will still output `@@` lines that
replace a single line of context.
This commit is contained in:
Danny Hooper 2022-12-05 18:44:00 -06:00
parent 15d40ffa54
commit 6787e17254
3 changed files with 139 additions and 17 deletions

View File

@ -21,7 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
Thanks to the people who made this release happen!
* Martin von Zweigbergk (@martinvonz)
* Danny Hooper (hooper@google.com)
## [0.6.1] - 2022-12-05

View File

@ -1296,6 +1296,7 @@ fn show_color_words_diff_hunks(
right: &[u8],
formatter: &mut dyn Formatter,
) -> io::Result<()> {
const SKIPPED_CONTEXT_LINE: &[u8] = b" ...\n";
let num_context_lines = 3;
let mut context = VecDeque::new();
// Have we printed "..." for any skipped context?
@ -1305,23 +1306,24 @@ fn show_color_words_diff_hunks(
for diff_line in files::diff(left, right) {
if diff_line.is_unmodified() {
context.push_back(diff_line.clone());
if context.len() > num_context_lines {
if context_before {
let mut start_skipping_context = false;
if context_before {
if skipped_context && context.len() > num_context_lines {
context.pop_front();
} else {
context.pop_back();
} else if !skipped_context && context.len() > num_context_lines + 1 {
start_skipping_context = true;
}
if !context_before {
for line in &context {
show_color_words_diff_line(formatter, line)?;
}
context.clear();
context_before = true;
}
if !skipped_context {
formatter.write_bytes(b" ...\n")?;
skipped_context = true;
} else if context.len() > num_context_lines * 2 + 1 {
for line in context.drain(..num_context_lines) {
show_color_words_diff_line(formatter, &line)?;
}
start_skipping_context = true;
}
if start_skipping_context {
context.drain(..2);
formatter.write_bytes(SKIPPED_CONTEXT_LINE)?;
skipped_context = true;
context_before = true;
}
} else {
for line in &context {
@ -1334,9 +1336,17 @@ fn show_color_words_diff_hunks(
}
}
if !context_before {
if context.len() > num_context_lines + 1 {
context.truncate(num_context_lines);
skipped_context = true;
context_before = true;
}
for line in &context {
show_color_words_diff_line(formatter, line)?;
}
if context_before {
formatter.write_bytes(SKIPPED_CONTEXT_LINE)?;
}
}
// If the last diff line doesn't end with newline, add it.

View File

@ -270,7 +270,7 @@ fn test_color_words_diff_missing_newline() {
...
=== Modify middle line
Modified regular file file1:
...
1 1: A
2 2: b
3 3: c
4 4: d
@ -278,7 +278,7 @@ fn test_color_words_diff_missing_newline() {
6 6: f
7 7: g
8 8: h
...
9 9: i
=== Modify last line
Modified regular file file1:
...
@ -313,3 +313,115 @@ fn test_color_words_diff_missing_newline() {
9 : I
"###);
}
#[test]
fn test_diff_skipped_context() {
let test_env = TestEnvironment::default();
test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]);
let repo_path = test_env.env_root().join("repo");
std::fs::write(repo_path.join("file1"), "a\nb\nc\nd\ne\nf\ng\nh\ni\nj").unwrap();
test_env.jj_cmd_success(&repo_path, &["describe", "-m", "=== Left side of diffs"]);
test_env.jj_cmd_success(&repo_path, &["new", "@", "-m", "=== Must skip 2 lines"]);
std::fs::write(repo_path.join("file1"), "A\nb\nc\nd\ne\nf\ng\nh\ni\nJ").unwrap();
test_env.jj_cmd_success(&repo_path, &["new", "@-", "-m", "=== Don't skip 1 line"]);
std::fs::write(repo_path.join("file1"), "A\nb\nc\nd\ne\nf\ng\nh\nI\nj").unwrap();
test_env.jj_cmd_success(&repo_path, &["new", "@-", "-m", "=== No gap to skip"]);
std::fs::write(repo_path.join("file1"), "a\nB\nc\nd\ne\nf\ng\nh\nI\nj").unwrap();
test_env.jj_cmd_success(&repo_path, &["new", "@-", "-m", "=== No gap to skip"]);
std::fs::write(repo_path.join("file1"), "a\nb\nC\nd\ne\nf\ng\nh\nI\nj").unwrap();
test_env.jj_cmd_success(&repo_path, &["new", "@-", "-m", "=== 1 line at start"]);
std::fs::write(repo_path.join("file1"), "a\nb\nc\nd\nE\nf\ng\nh\ni\nj").unwrap();
test_env.jj_cmd_success(&repo_path, &["new", "@-", "-m", "=== 1 line at end"]);
std::fs::write(repo_path.join("file1"), "a\nb\nc\nd\ne\nF\ng\nh\ni\nj").unwrap();
let stdout = test_env.jj_cmd_success(
&repo_path,
&["log", "-Tdescription", "-p", "--no-graph", "--reversed"],
);
insta::assert_snapshot!(stdout, @r###"
(no description set)
=== Left side of diffs
Added regular file file1:
1: a
2: b
3: c
4: d
5: e
6: f
7: g
8: h
9: i
10: j
=== Must skip 2 lines
Modified regular file file1:
1 1: aA
2 2: b
3 3: c
4 4: d
...
7 7: g
8 8: h
9 9: i
10 10: jJ
=== Don't skip 1 line
Modified regular file file1:
1 1: aA
2 2: b
3 3: c
4 4: d
5 5: e
6 6: f
7 7: g
8 8: h
9 9: iI
10 10: j
=== No gap to skip
Modified regular file file1:
1 1: a
2 2: bB
3 3: c
4 4: d
5 5: e
6 6: f
7 7: g
8 8: h
9 9: iI
10 10: j
=== No gap to skip
Modified regular file file1:
1 1: a
2 2: b
3 3: cC
4 4: d
5 5: e
6 6: f
7 7: g
8 8: h
9 9: iI
10 10: j
=== 1 line at start
Modified regular file file1:
1 1: a
2 2: b
3 3: c
4 4: d
5 5: eE
6 6: f
7 7: g
8 8: h
...
=== 1 line at end
Modified regular file file1:
...
3 3: c
4 4: d
5 5: e
6 6: fF
7 7: g
8 8: h
9 9: i
10 10: j
"###);
}