🐛 Panic in `src/wrapping.rs:469` for a correct patch with syntax highlight and line wrapping

[ilya-bobyr]

With the following patch:

diff --git a.rs b.rs
--- a.rs
+++ b.rs
@@ -127,6 +128,16 @@ f() {
         a
+        "------------------------------------------------------------------------------------------\
         z
 }

I see an assertion failure if my terminal is narrow enough, such that delta needs to wrap the diff line. Note that if I remove the " at the beginning or the \ at the end, then no panic happens, and the patch is presented correctly. Even if the line wrap needs to happen.

The output that I see is as follows:

❯ cat broken.patch | RUST_BACKTRACE=1 delta
thread 'main' panicked at /home/ilya/.cargo/registry/src/index.crates.io-6f17d22bba15001f/git-delta-0.17.0/src/wrapping.rs:469:9:
assertion `left == right` failed: syntax and diff wrapping differs (-) [*r*]
  left: (0, 1)
 right: (0, 2)
stack backtrace:
   0: rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::panicking::assert_failed_inner
   3: core::panicking::assert_failed
   4: delta::wrapping::wrap_minusplus_block::wrap_syntax_and_diff
   5: delta::wrapping::wrap_minusplus_block
   6: delta::paint::paint_minus_and_plus_lines
   7: delta::paint::Painter::paint_buffered_minus_and_plus_lines
   8: delta::handlers::hunk::<impl delta::delta::StateMachine>::handle_hunk_line
   9: delta::delta::delta
  10: delta::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

Δ a.rs ⟶   b.rs
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

─────────────┐
• 128: f() { │
─────────────┘
│ 127│        a                                                                                           │ 128│        a

[th1000s]

Thank you for your bug reports!

Unfortunately, I was unable to reproduce this by resizing my terminal to various sizes, using the current main or v0.17 of delta.

Can you try with HOME=/ delta --no-gitconfig --side-by-side < broken.patch (and maybe more options)? Once you have reproduced it like that, the terminal type and width (echo $COLUMNS) would also be interesting. Very unlikely, but cargo install --locked delta might also help.

[jan-swiecki]

I get the same panic:

:thread 'main' panicked at /home/jswiecki/.cargo/registry/src/index.crates.io-6f17d22bba15001f/git-delta-0.17.0/src/wrapping.rs:469:9:
assertion `left == right` failed: syntax and diff wrapping differs [*l*] (-)
  left: (22, 24)
 right: (22, 23)
stack backtrace:
   0: rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::panicking::assert_failed_inner
   3: core::panicking::assert_failed
   4: delta::wrapping::wrap_minusplus_block::wrap_syntax_and_diff
   5: delta::wrapping::wrap_minusplus_block
   6: delta::paint::paint_minus_and_plus_lines
   7: delta::paint::Painter::paint_buffered_minus_and_plus_lines
   8: delta::handlers::hunk::<impl delta::delta::StateMachine>::handle_hunk_line
   9: delta::delta::delta
  10: delta::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

It fails with git show @~1 but, interestingly, doesn't fail with git show @~1 | delta.

Unfortunately I cannot share the diff (confidential source), but I managed to narrow down which config options, when disabled, prevent the panic:

$ cat .gitconfig
[user]
	name = John Doe
	email = [email protected]

[core]
	pager = delta

[delta]
	side-by-side = true

	# without this output lines are truncated and we might miss something in the diff
	wrap-max-lines = 1000

[diff]
	colorMoved = default

When I remove wrap-max-lines or colorMoved the error disapears.

Full backtrace:

:thread 'main' panicked at /home/jswiecki/.cargo/registry/src/index.crates.io-6f17d22bba15001f/git-delta-0.17.0/src/wrapping.rs:469:9:
assertion `left == right` failed: syntax and diff wrapping differs [*l*] (-)
  left: (22, 24)
 right: (22, 23)
stack backtrace:
   0:     0x55a2612d3245 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h1e1a1972118942ad
   1:     0x55a2612fb61b - core::fmt::write::hc090a2ffd6b28c4a
   2:     0x55a2612d02af - std::io::Write::write_fmt::h8898bac6ff039a23
   3:     0x55a2612d301e - std::sys_common::backtrace::print::ha96650907276675e
   4:     0x55a2612d4479 - std::panicking::default_hook::{{closure}}::h215c2a0a8346e0e0
   5:     0x55a2612d41bd - std::panicking::default_hook::h207342be97478370
   6:     0x55a2612d4913 - std::panicking::rust_panic_with_hook::hac8bdceee1e4fe2c
   7:     0x55a2612d47f4 - std::panicking::begin_panic_handler::{{closure}}::h00d785e82757ce3c
   8:     0x55a2612d3709 - std::sys_common::backtrace::__rust_end_short_backtrace::h1628d957bcd06996
   9:     0x55a2612d4527 - rust_begin_unwind
  10:     0x55a260f89333 - core::panicking::panic_fmt::hdc63834ffaaefae5
  11:     0x55a260f8974f - core::panicking::assert_failed_inner::hda4754f94c1c1cb1
  12:     0x55a260f77f7f - core::panicking::assert_failed::h30d7be808a745fb4
  13:     0x55a261010c62 - delta::wrapping::wrap_minusplus_block::wrap_syntax_and_diff::hb2cda4881c64c2c8
  14:     0x55a26100e1b5 - delta::wrapping::wrap_minusplus_block::h6ed1e7130ce762d2
  15:     0x55a261081a36 - delta::paint::paint_minus_and_plus_lines::h543350accf6f1aeb
  16:     0x55a26107d1ab - delta::paint::Painter::paint_buffered_minus_and_plus_lines::h5b491f18c2ae8207
  17:     0x55a2610569ef - delta::handlers::hunk::<impl delta::delta::StateMachine>::handle_hunk_line::h9e7c53f7ad59f73c
  18:     0x55a26104eb42 - delta::delta::delta::h45726ce1673a6576
  19:     0x55a261031b77 - delta::main::h010575b1197248a0
  20:     0x55a260fb0c83 - std::sys_common::backtrace::__rust_begin_short_backtrace::h265a032561e7bfb3
  21:     0x55a261046ccd - std::rt::lang_start::{{closure}}::h7c3de75061e14dd4
  22:     0x55a2612c8690 - std::rt::lang_start_internal::h3ed4fe7b2f419135
  23:     0x55a2610334b5 - main
  24:     0x7ff0ef423a90 - __libc_start_call_main
                               at ./csu/../sysdeps/nptl/libc_start_call_main.h:58:16
  25:     0x7ff0ef423b49 - __libc_start_main_impl
                               at ./csu/../csu/libc-start.c:360:3
  26:     0x55a260f899e5 - _start
  27:                0x0 - <unknown>

[ilya-bobyr]

Sorry for a slow response. It crashes with the default configuration as well.

Here:

❯ RUST_BACKTRACE=1 HOME=/ delta --no-gitconfig --side-by-side < broken.patch
thread 'main' panicked at /home/ilya/.cargo/registry/src/index.crates.io-6f17d22bba15001f/git-delta-0.17.0/src/wrapping.rs:469:9:
assertion `left == right` failed: syntax and diff wrapping differs (-) [*r*]
  left: (0, 1)
 right: (0, 2)
stack backtrace:
   0: rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::panicking::assert_failed_inner
   3: core::panicking::assert_failed
   4: delta::wrapping::wrap_minusplus_block::wrap_syntax_and_diff
   5: delta::wrapping::wrap_minusplus_block
   6: delta::paint::paint_minus_and_plus_lines
   7: delta::paint::Painter::paint_buffered_minus_and_plus_lines
   8: delta::handlers::hunk::<impl delta::delta::StateMachine>::handle_hunk_line
   9: delta::delta::delta
  10: delta::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

a.rs ⟶   b.rs
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

───────────┐
128: f() { │
───────────┘
│ 127│        a                                                                                           │ 128│        a

❯ echo $COLUMNS
212

I noticed that if I set wrap-max-lines to 0 to disable line wrapping, then it does not crash. Also, it only crashes when the diff is highlighted in a certain way. Changing a.rs to a.txt in the patch makes the issue go away. And, notice, that the \ needs to hit the very last character in the terminal. At least in my case.

I'll try to reproduce with the master version as well. Maybe there is a unit test example I can try to copy and modify to reproduce it in the source tree directly?

[ilya-bobyr]

[...] And, notice, that the \ needs to hit the very last character in the terminal. At least in my case.

Mixed up with another bug. In this case, I think, what is necessary is for the ---- line to be longer than half the terminal width, for the line wrapping to be applied.

And the " at the beginning is also important. If I remove the " the panic does not happen. So it is the Rust syntax combined with line wrapping.

[ilya-bobyr]

Here is my first attempt at creating a unit test that would reproduce this issue:

WIP: A line wrapping test that is panicking

I do not understand how to enable syntax highlight in the test. The expected text contains no syntax highlight sequences. And the test is passing. I think the bug requires syntax highlight and seems to be related to incorrect character width calculations.

I was wondering if maybe it has something to do with a Unicode character used for the line continuation, but that seems to work OK.

[th1000s]

Yes, those are probably two different triggers, diff.colorMoved = default means git itself colors in moved lines, this is usually lost when dumping a diff to a file. @jan-swiecki - can you try to create a minimal reproducer?

And I can't trigger it with the bad.patch input, tried column width 210-214.

% git checkout 0.17.0 && cargo build --release >& /dev/null && cat -A bad.patch && sha1sum bad.patch && \
    HOME=/ target/release/delta --no-gitconfig --side-by-side < bad.patch && echo SUCCESS at $COLUMNS
HEAD is now at 13c8219 Bump version
diff --git a.rs b.rs$
--- a.rs$
+++ b.rs$
@@ -127,6 +128,16 @@ f() {$
         a$
+        "------------------------------------------------------------------------------------------\$
         z$
 }$
61dd3d44d6605e7ba5d552ca4b2bb426c5d10b88  bad.patch
[..]
SUCCESS at 212

[domWalters]

I am also getting this issue.

• Delta v0.18.1.
• Ubuntu 20.04.
• Alacritty 0.13.1.

Command:

env - TERM=xterm COLORTERM=truecolor "$(which delta)" --no-gitconfig --side-by-side --width=60 < /tmp/diff.diff

The diff will also fail with --width=61 but not --width=59 or --width=62.

Contents of /tmp/diff.diff (this is a heavily manual edit of a proprietary commit, as such it may not even be a valid patch anymore):

commit 6f45d481f266936ebe48db0445c8872e51f38af6
Author:
Date:

    feat: feat

diff --git a/a.cc b/a.cc
index 57eca873..5b0da8b1 100644
--- a/a.cc
+++ b/a.cc
@@ -2,18 +3,18 @@ a

+    void aaaaadbbdcccc()

@@ -240,3 +240,3 @@ protected:

-        aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa;  // TODO: check that this can come before a path change
+        bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb;

Error:

thread 'main' panicked at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/git-delta-0.18.1/src/wrapping.rs:469:9:
assertion `left == right` failed: syntax and diff wrapping differs (-) [*r*]
  left: (0, 1)
 right: (0, 2)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Depending on window size, the error in #1726 can happen instead.

---

Some further things I've noted:

• If I change the file name in the diff to a-cc instead of a.cc, I do not get this error. This seems particularly weird.
• When I use this command without --width and manually resize the terminal, there is a kind of hysteresis where I shrink the terminal below a size that causes the error, but then have to resize it far above the trigger point to stop the issue.

[th1000s]

If you obtained the binary via cargo install then please try cargo install --locked git-delta via shaicoleman in #1726. It seems a dependency only works as expected when using the exact version that delta specifies. A normal cargo install does not do that.

[domWalters]

If you obtained the binary via cargo install then please try cargo install --locked git-delta via shaicoleman in #1726. It seems a dependency only works as expected when using the exact version that delta specifies. A normal cargo install does not do that.

Yes, this worked. Thanks!

[th1000s]

Should be fixed in 0.18.2 and work without --locked now.