diff: fix unified diff to emit one-lower start line number for empty …

…hunks

Both Mercurial and Git (xdiff) have a special case for empty hunks.

https://repo.mercurial-scm.org/hg/rev/2b1ec74c961f

I also changed the internal line numbers to start from 0 so we wouldn't have
to think about whether "N - 1" would underflow.

Fixes #5049

CHANGELOG.md

@@ -23,6 +23,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 * The `$NO_COLOR` environment variable must now be non-empty to be respected.
 
+* Fixed incompatible rendering of empty hunks in git/unified diffs.
+  [#5049](https://github.com/martinvonz/jj/issues/5049)
+
 ## [0.24.0] - 2024-12-04
 
 ### Release highlights

cli/src/diff_util.rs

@@ -1322,8 +1322,8 @@ fn unified_diff_hunks<'content>(
 ) -> Vec<UnifiedDiffHunk<'content>> {
     let mut hunks = vec![];
     let mut current_hunk = UnifiedDiffHunk {
-        left_line_range: 1..1,
-        right_line_range: 1..1,
+        left_line_range: 0..0,
+        right_line_range: 0..0,
         lines: vec![],
     };
     let diff = diff_by_line([left_content, right_content], &options.line_diff);
@@ -1438,13 +1438,28 @@ fn show_unified_diff_hunks(
     right_content: &[u8],
     options: &UnifiedDiffOptions,
 ) -> io::Result<()> {
+    // "If the chunk size is 0, the first number is one lower than one would
+    // expect." - https://www.artima.com/weblogs/viewpost.jsp?thread=164293
+    //
+    // The POSIX spec also states that "the ending line number of an empty range
+    // shall be the number of the preceding line, or 0 if the range is at the
+    // start of the file."
+    // - https://pubs.opengroup.org/onlinepubs/9799919799/utilities/diff.html
+    fn to_line_number(range: Range<usize>) -> usize {
+        if range.is_empty() {
+            range.start
+        } else {
+            range.start + 1
+        }
+    }
+
     for hunk in unified_diff_hunks(left_content, right_content, options) {
         writeln!(
             formatter.labeled("hunk_header"),
             "@@ -{},{} +{},{} @@",
-            hunk.left_line_range.start,
+            to_line_number(hunk.left_line_range.clone()),
             hunk.left_line_range.len(),
-            hunk.right_line_range.start,
+            to_line_number(hunk.right_line_range.clone()),
             hunk.right_line_range.len()
         )?;
         for (line_type, tokens) in &hunk.lines {

cli/tests/test_absorb_command.rs

@@ -100,7 +100,7 @@ fn test_absorb_simple() {
     │  index e69de29bb2..ed237b5112 100644
     │  --- a/file1
     │  +++ b/file1
-    │  @@ -1,0 +1,3 @@
+    │  @@ -0,0 +1,3 @@
     │  +1X
     │  +1A
     │  +1b
@@ -195,7 +195,7 @@ fn test_absorb_replace_single_line_hunk() {
        index 0000000000..0000000000
        --- /dev/null
        +++ b/file1
-       @@ -1,0 +1,10 @@
+       @@ -0,0 +1,10 @@
        +<<<<<<< Conflict 1 of 1
        +%%%%%%% Changes from base to side #1
        +-2a
@@ -271,7 +271,7 @@ fn test_absorb_merge() {
     │ │  index 0000000000..44442d2d7b
     │ │  --- /dev/null
     │ │  +++ b/file2
-    │ │  @@ -1,0 +1,1 @@
+    │ │  @@ -0,0 +1,1 @@
     │ │  +3A
     │ ○  zsuskuln 71d1ee56 2
     │ │  diff --git a/file1 b/file1
@@ -297,7 +297,7 @@ fn test_absorb_merge() {
        index 0000000000..eb6e8821f1
        --- /dev/null
        +++ b/file1
-       @@ -1,0 +1,1 @@
+       @@ -0,0 +1,1 @@
        +0a
     ");
 }
@@ -396,7 +396,7 @@ fn test_absorb_file_mode() {
        index 0000000000..268de3f3ec
        --- /dev/null
        +++ b/file1
-       @@ -1,0 +1,1 @@
+       @@ -0,0 +1,1 @@
        +1A
     ");
 }
@@ -488,7 +488,7 @@ fn test_absorb_from_into() {
     │  index 0000000000..faf62af049
     │  --- /dev/null
     │  +++ b/file1
-    │  @@ -1,0 +1,7 @@
+    │  @@ -0,0 +1,7 @@
     │  +1a
     │  +X
     │  +2a
@@ -544,14 +544,14 @@ fn test_absorb_paths() {
        index 0000000000..268de3f3ec
        --- /dev/null
        +++ b/file1
-       @@ -1,0 +1,1 @@
+       @@ -0,0 +1,1 @@
        +1A
        diff --git a/file2 b/file2
        new file mode 100644
        index 0000000000..a8994dc188
        --- /dev/null
        +++ b/file2
-       @@ -1,0 +1,1 @@
+       @@ -0,0 +1,1 @@
        +1a
     ");
 }
@@ -619,7 +619,7 @@ fn test_absorb_immutable() {
        index 0000000000..8c5268f893
        --- /dev/null
        +++ b/file1
-       @@ -1,0 +1,2 @@
+       @@ -0,0 +1,2 @@
        +1a
        +1b
     ");

cli/tests/test_diff_command.rs

@@ -87,15 +87,15 @@ fn test_diff_basic() {
     "###);
 
     let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--git", "file1"]);
-    insta::assert_snapshot!(stdout, @r###"
+    insta::assert_snapshot!(stdout, @r"
     diff --git a/file1 b/file1
     deleted file mode 100644
     index 257cc5642c..0000000000
     --- a/file1
     +++ /dev/null
-    @@ -1,1 +1,0 @@
+    @@ -1,1 +0,0 @@
     -foo
-    "###);
+    ");
 
     let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--git"]);
     insta::assert_snapshot!(stdout, @r###"
@@ -118,23 +118,23 @@ fn test_diff_basic() {
     "###);
 
     let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--git", "--context=0"]);
-    insta::assert_snapshot!(stdout, @r###"
+    insta::assert_snapshot!(stdout, @r"
     diff --git a/file2 b/file2
     index 94ebaf9001..1ffc51b472 100644
     --- a/file2
     +++ b/file2
     @@ -2,1 +2,1 @@
     -2
     +5
-    @@ -4,1 +4,0 @@
+    @@ -4,1 +3,0 @@
     -4
     diff --git a/file1 b/file3
     rename from file1
     rename to file3
     diff --git a/file2 b/file4
     copy from file2
     copy to file4
-    "###);
+    ");
 
     let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--git", "--color=debug"]);
     insta::assert_snapshot!(stdout, @r###"
@@ -313,7 +313,7 @@ fn test_diff_file_mode() {
     "###);
 
     let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "-r@--", "--git"]);
-    insta::assert_snapshot!(stdout, @r###"
+    insta::assert_snapshot!(stdout, @r"
     diff --git a/file1 b/file1
     new file mode 100755
     index 0000000000..e69de29bb2
@@ -322,28 +322,28 @@ fn test_diff_file_mode() {
     index 0000000000..d00491fd7e
     --- /dev/null
     +++ b/file2
-    @@ -1,0 +1,1 @@
+    @@ -0,0 +1,1 @@
     +1
     diff --git a/file3 b/file3
     new file mode 100644
     index 0000000000..d00491fd7e
     --- /dev/null
     +++ b/file3
-    @@ -1,0 +1,1 @@
+    @@ -0,0 +1,1 @@
     +1
     diff --git a/file4 b/file4
     new file mode 100644
     index 0000000000..e69de29bb2
-    "###);
+    ");
     let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "-r@-", "--git"]);
-    insta::assert_snapshot!(stdout, @r###"
+    insta::assert_snapshot!(stdout, @r"
     diff --git a/file1 b/file1
     old mode 100755
     new mode 100644
     index e69de29bb2..0cfbf08886
     --- a/file1
     +++ b/file1
-    @@ -1,0 +1,1 @@
+    @@ -0,0 +1,1 @@
     +2
     diff --git a/file2 b/file2
     old mode 100755
@@ -360,34 +360,34 @@ fn test_diff_file_mode() {
     diff --git a/file4 b/file4
     old mode 100644
     new mode 100755
-    "###);
+    ");
     let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "-r@", "--git"]);
-    insta::assert_snapshot!(stdout, @r###"
+    insta::assert_snapshot!(stdout, @r"
     diff --git a/file1 b/file1
     deleted file mode 100644
     index 0cfbf08886..0000000000
     --- a/file1
     +++ /dev/null
-    @@ -1,1 +1,0 @@
+    @@ -1,1 +0,0 @@
     -2
     diff --git a/file2 b/file2
     deleted file mode 100644
     index d00491fd7e..0000000000
     --- a/file2
     +++ /dev/null
-    @@ -1,1 +1,0 @@
+    @@ -1,1 +0,0 @@
     -1
     diff --git a/file3 b/file3
     deleted file mode 100755
     index 0cfbf08886..0000000000
     --- a/file3
     +++ /dev/null
-    @@ -1,1 +1,0 @@
+    @@ -1,1 +0,0 @@
     -2
     diff --git a/file4 b/file4
     deleted file mode 100755
     index e69de29bb2..0000000000
-    "###);
+    ");
 }
 
 #[test]
@@ -686,18 +686,18 @@ fn test_diff_hunks() {
     "###);
 
     let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--git"]);
-    insta::assert_snapshot!(stdout, @r###"
+    insta::assert_snapshot!(stdout, @r"
     diff --git a/file1 b/file1
     index e69de29bb2..257cc5642c 100644
     --- a/file1
     +++ b/file1
-    @@ -1,0 +1,1 @@
+    @@ -0,0 +1,1 @@
     +foo
     diff --git a/file2 b/file2
     index 257cc5642c..e69de29bb2 100644
     --- a/file2
     +++ b/file2
-    @@ -1,1 +1,0 @@
+    @@ -1,1 +0,0 @@
     -foo
     diff --git a/file3 b/file3
     index 221a95a095..a543ef3892 100644
@@ -708,21 +708,21 @@ fn test_diff_hunks() {
     -baz qux blah blah
     +bar
     +baz quux blah blah
-    "###);
+    ");
 
     let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--git", "--color=debug"]);
-    insta::assert_snapshot!(stdout, @r###"
+    insta::assert_snapshot!(stdout, @r"
     <<diff file_header::diff --git a/file1 b/file1>>
     <<diff file_header::index e69de29bb2..257cc5642c 100644>>
     <<diff file_header::--- a/file1>>
     <<diff file_header::+++ b/file1>>
-    [38;5;6m<<diff hunk_header::@@ -1,0 +1,1 @@>>[39m
+    [38;5;6m<<diff hunk_header::@@ -0,0 +1,1 @@>>[39m
     <<diff added::+>><<diff added token::foo>>
     <<diff file_header::diff --git a/file2 b/file2>>
     <<diff file_header::index 257cc5642c..e69de29bb2 100644>>
     <<diff file_header::--- a/file2>>
     <<diff file_header::+++ b/file2>>
-    [38;5;6m<<diff hunk_header::@@ -1,1 +1,0 @@>>[39m
+    [38;5;6m<<diff hunk_header::@@ -1,1 +0,0 @@>>[39m
     <<diff removed::->><<diff removed token::foo>>
     <<diff file_header::diff --git a/file3 b/file3>>
     <<diff file_header::index 221a95a095..a543ef3892 100644>>
@@ -733,7 +733,7 @@ fn test_diff_hunks() {
     <<diff removed::-baz >><<diff removed token::qux>><<diff removed:: blah blah>>
     <<diff added::+>><<diff added token::bar>>
     <<diff added::+baz >><<diff added token::quux>><<diff added:: blah blah>>
-    "###);
+    ");
 }
 
 #[test]
@@ -1805,14 +1805,14 @@ context = 0
             "--reversed",
         ],
     );
-    insta::assert_snapshot!(stdout, @r#"
+    insta::assert_snapshot!(stdout, @r"
     === First commit
     diff --git a/file1 b/file1
     new file mode 100644
     index 0000000000..0fec236860
     --- /dev/null
     +++ b/file1
-    @@ -1,0 +1,5 @@
+    @@ -0,0 +1,5 @@
     +a
     +b
     +c
@@ -1827,7 +1827,7 @@ context = 0
     @@ -3,1 +3,1 @@
     -c
     +C
-    "#);
+    ");
 }
 
 #[test]

cli/tests/test_diffedit_command.rs

@@ -675,13 +675,13 @@ fn test_diffedit_old_restore_interactive_tests() {
     Added 0 files, modified 1 files, removed 0 files
     "###);
     let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--git"]);
-    insta::assert_snapshot!(stdout, @r###"
+    insta::assert_snapshot!(stdout, @r"
     diff --git a/file1 b/file1
     deleted file mode 100644
     index 7898192261..0000000000
     --- a/file1
     +++ /dev/null
-    @@ -1,1 +1,0 @@
+    @@ -1,1 +0,0 @@
     -a
     diff --git a/file2 b/file2
     index 7898192261..6178079822 100644
@@ -695,9 +695,9 @@ fn test_diffedit_old_restore_interactive_tests() {
     index 0000000000..c21c9352f7
     --- /dev/null
     +++ b/file3
-    @@ -1,0 +1,1 @@
+    @@ -0,0 +1,1 @@
     +unrelated
-    "###);
+    ");
 }
 
 #[test]

cli/tests/test_evolog_command.rs

@@ -104,7 +104,7 @@ fn test_evolog_with_or_without_diff() {
 
     // Test `--git` format, and that it implies `-p`
     let stdout = test_env.jj_cmd_success(&repo_path, &["evolog", "--no-graph", "--git"]);
-    insta::assert_snapshot!(stdout, @r###"
+    insta::assert_snapshot!(stdout, @r"
     rlvkpnrz [email protected] 2001-02-03 08:05:10 66b42ad3
     my description
     diff --git a/file1 b/file1
@@ -136,11 +136,11 @@ fn test_evolog_with_or_without_diff() {
     index 0000000000..257cc5642c
     --- /dev/null
     +++ b/file2
-    @@ -1,0 +1,1 @@
+    @@ -0,0 +1,1 @@
     +foo
     rlvkpnrz hidden [email protected] 2001-02-03 08:05:08 2b023b5f
     (empty) my description
-    "###);
+    ");
 }
 
 #[test]