search

dandavison / delta

A syntax-highlighting pager for git, diff, grep, and blame output
https://dandavison.github.io/delta/
MIT License
21.32k stars 358 forks source link

πŸ› Panic in `src/wrapping.rs:469` for a correct patch with syntax highlight and line wrapping #1724

Open ilya-bobyr opened 1 week ago

ilya-bobyr commented 1 week ago

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 commented 1 week ago

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 commented 2 days ago

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 = user@example.com

[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 commented 1 day ago

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 commented 23 hours ago

[...] 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 commented 20 hours ago

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.