Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Panic in src/paint.rs:907 for a valid patch with syntax highlight and line wrapping #1726

Open
ilya-bobyr opened this issue Jun 22, 2024 · 10 comments

Comments

@ilya-bobyr
Copy link

With the following patch:

--- a.rs
+++ b.rs
@@ -10,3 +10,3 @@
  a
  The ';' character on this line must be exactly in the very last position on the right ----------> ;
  b

I see an assertion failure, if the ; happens to be exactly in the very last position on the screen.
It also needs to be a complete patch, when there are two sides that need to be output.
It does not matter what is on the left side, or that it has not actual updates in it.
Syntax also matters. If I change a.rs/b.rs to a.txt/b.txt the panic goes away.
Here is a complete output:

❯ cat broken.patch | RUST_BACKTRACE=1 delta --wrap-max-lines=2
thread 'main' panicked at /home/ilya/.cargo/registry/src/index.crates.io-6f17d22bba15001f/git-delta-0.17.0/src/paint.rs:907:17:
String mismatch encountered while superimposing style sections: ';' vs '↴'
stack backtrace:
   0: rust_begin_unwind
   1: core::panicking::panic_fmt
   2: delta::paint::superimpose_style_sections::superimpose_style_sections
   3: delta::paint::Painter::paint_line
   4: delta::features::side_by_side::paint_zero_lines_side_by_side
   5: delta::paint::Painter::paint_zero_line
   6: delta::handlers::hunk::<impl delta::delta::StateMachine>::handle_hunk_line
   7: delta::delta::delta
   8: delta::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

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

──────┐
• 10: │
──────┘
│ 10 │ a                                                                                                   │ 10 │ a
@ilya-bobyr ilya-bobyr changed the title 🐛 Panic in src/paint.rs:907 for a valid patch with syntax highlight and when line wrapping is enabled 🐛 Panic in src/paint.rs:907 for a valid patch with syntax highlight and line wrapping Jun 22, 2024
@th1000s
Copy link
Collaborator

th1000s commented Jun 23, 2024

I was also unable to reproduce this, see #1724 ☹️

@ilya-bobyr
Copy link
Author

This one also reproduces for me with the default configuration:

❯ vim broken.patch
# Paste patch from GitHub, save and exit

❯ 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/paint.rs:907:17:
String mismatch encountered while superimposing style sections: ';' vs '↴'
stack backtrace:
   0: rust_begin_unwind
   1: core::panicking::panic_fmt
   2: delta::paint::superimpose_style_sections::superimpose_style_sections
   3: delta::paint::Painter::paint_line
   4: delta::features::side_by_side::paint_zero_lines_side_by_side
   5: delta::paint::Painter::paint_zero_line
   6: delta::handlers::hunk::<impl delta::delta::StateMachine>::handle_hunk_line
   7: delta::delta::delta
   8: delta::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

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

────┐
10: │
────┘
│ 10 │ a                                                                                                  │ 10 │ a

❯ echo $COLUMNS
212

I've installed delta with cargo install delta. So, I think, my binary is matching the 0.17.0 release.
I also started noticing this issue only after a recent update.

I'm on the latest Ubuntu, with Alacritty and tmux, but I doubt that it matters.

@dkarter
Copy link

dkarter commented Jul 21, 2024

I'm seeing the same error (0.17.0):

thread 'main' panicked at /Users/dorian/.asdf/installs/rust/1.79.0/registry/src/index.crates.io-6f17d22bba15001f/git-delta-0.17.0/src/paint.rs:907:17:
String mismatch encountered while superimposing style sections: '{' vs '↴'
stack backtrace:
   0: _rust_begin_unwind
   1: core::panicking::panic_fmt
   2: delta::paint::superimpose_style_sections::superimpose_style_sections
   3: delta::paint::Painter::paint_line
   4: delta::features::side_by_side::paint_zero_lines_side_by_side
   5: delta::paint::Painter::paint_zero_line
   6: delta::handlers::hunk::<impl delta::delta::StateMachine>::handle_hunk_line
   7: delta::delta::delta
   8: delta::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

Also getting this one sometimes, not sure if it's related:

assertion `left == right` failed: syntax and diff wrapping differs [*l*] (-)
  left: (0, 1)
 right: (0, 2)
stack backtrace:
   0:        0x102540b7c - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h1f3776e0b5c7517d
   1:        0x10255e6c8 - core::fmt::write::heedef092c8c0962e
   2:        0x10253d9fc - std::io::Write::write_fmt::h7178e8e2ea928914
   3:        0x1025409d4 - std::sys_common::backtrace::print::h417292deb95532ed
   4:        0x102541cd0 - std::panicking::default_hook::{{closure}}::h0cb68f1228c4613a
   5:        0x1025419c4 - std::panicking::default_hook::h24535936bc1f51de
   6:        0x102542588 - std::panicking::rust_panic_with_hook::h5db4d2345b297bed
   7:        0x102541fb8 - std::panicking::begin_panic_handler::{{closure}}::h3fd558f09a0d5492
   8:        0x102541004 - std::sys_common::backtrace::__rust_end_short_backtrace::hfc76eebe1ce501b2
   9:        0x102541d28 - _rust_begin_unwind
  10:        0x102577a18 - core::panicking::panic_fmt::hc2b459a5bd3dce66
  11:        0x102577d18 - core::panicking::assert_failed_inner::h80507f91aa687ac5
  12:        0x1025687a8 - core::panicking::assert_failed::hba476d343ab37030
  13:        0x1023751b4 - delta::wrapping::wrap_minusplus_block::wrap_syntax_and_diff::h98dd41239ad26134
  14:        0x1023731f8 - delta::wrapping::wrap_minusplus_block::h05dcfdb9e5436ffe
  15:        0x1023870c8 - delta::paint::paint_minus_and_plus_lines::h9cf8ef46819f3e8d
  16:        0x102384464 - delta::paint::Painter::paint_buffered_minus_and_plus_lines::h77ecfbe33c5b90a9
  17:        0x102396854 - delta::handlers::hunk::<impl delta::delta::StateMachine>::handle_hunk_line::he000af041af4e9b4
  18:        0x1023900a0 - delta::delta::delta::h15f78429f4d360eb
  19:        0x10238ab5c - delta::main::he0a1c629ca3fe9eb
  20:        0x102318384 - std::sys_common::backtrace::__rust_begin_short_backtrace::h3ecb28f718c15367
  21:        0x102399bc0 - std::rt::lang_start::{{closure}}::h33cde0c0d94d42c6
  22:        0x1025369d8 - std::rt::lang_start_internal::hecc68fef83c8f44d
  23:        0x10238c2d8 - _main

@th1000s
Copy link
Collaborator

th1000s commented Jul 21, 2024

Yes, those are related. Does this happen with a public repo, or could you share a diff? With --side-by-side the current terminal width (echo $COLUMNS) is also import, ideally it can be triggered with HOME=/ delta --no-gitconfig --side-by-side < bad.patch

@dkarter
Copy link

dkarter commented Jul 26, 2024

@th1000s

echo $COLUMNS
# 119

I was able to repro this on a public repo, here's the patch:
bad.patch

To repro I opened the diff in a small tmux split and used this command:

delta --no-gitconfig --side-by-side < bad.patch

CleanShot 2024-07-26 at 16 59 25@2x

Can also repro this without Tmux, just Wezterm on mac:

CleanShot.2024-08-03.at.23.52.19.mp4

@ilya-bobyr
Copy link
Author

@dkarter Try with the patch that is included in the very first message in this issue:

--- a.rs
+++ b.rs
@@ -10,3 +10,3 @@
  a
  The ';' character on this line must be exactly in the very last position on the right ----------> ;
  b

It is a minimal example that failed for me.
You may want to play with the length of the line, make sure that the ; is on the edge of the terminal.

It fails for me with

RUST_BACKTRACE=1 HOME=/ delta --no-gitconfig --side-by-side <broken.patch

Considering that Thomas was not able to reproduce the issue with my patch, I wonder if the terminal configuration also matters here.
I did not try it out yet.
But I was thinking of trying it out with a few different terminals.

@th1000s
Copy link
Collaborator

th1000s commented Jul 31, 2024

Sorry, still can't reproduce this, using the cargo install git-delta binary, on Debian 12 using xterm or Konsole, also tried inside tmux or screen.

Using the delta-0.17.0-x86_64-unknown-linux-gnu or delta-0.17.0-x86_64-unknown-linux-musl binaries from the releases also worked fine here, so it probably isn't something picked up at build time from the local system.

To isolate env vars, can you try calling delta like this, which tells env to filter all environment vars and only set the provided ones:

env - TERM=xterm COLORTERM=truecolor "$(which delta)" --no-gitconfig --side-by-side < crashing.patch

You might have to add PATH=/usr/bin, also try leaving out TERM and/or COLORTERM

@ilya-bobyr, does adapting the patch to work with 119/118 $COLUMS still crash? See the following:

--- a.rs
+++ b.rs
@@ -10,3 +10,3 @@
  a
  The ';' character on last position --------------> ;
  b

@ilya-bobyr
Copy link
Author

ilya-bobyr commented Aug 2, 2024

It panics for me with both 118 and 119 columns and with the env restricting the environment as suggested.
But not with 117 or 120 columns.

❯ env - TERM=xterm COLORTERM=truecolor RUST_BACKTRACE=1 "$(which 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/paint.rs:907:17:
String mismatch encountered while superimposing style sections: ';' vs '↴'
stack backtrace:
   0: rust_begin_unwind
   1: core::panicking::panic_fmt
   2: delta::paint::superimpose_style_sections::superimpose_style_sections
   3: delta::paint::Painter::paint_line
   4: delta::features::side_by_side::paint_zero_lines_side_by_side
   5: delta::paint::Painter::paint_zero_line
   6: delta::handlers::hunk::<impl delta::delta::StateMachine>::handle_hunk_line
   7: delta::delta::delta
   8: delta::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

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

────┐
10: │
────┘
│ 10 │ a                                                   │ 10 │ a

❯ echo $COLUMNS
118

❯ env - TERM=xterm COLORTERM=truecolor "$(which 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/paint.rs:907:17:
String mismatch encountered while superimposing style sections: ';' vs '↴'
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

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

────┐
10: │
────┘
│ 10 │ a                                                   │ 10 │ a

This is for version 0.17.0:

❯ delta --version
delta 0.17.0

It could be worth noting that it fails even with the TERM unset (or set to dumb):

❯ env - "$(which delta)" --no-gitconfig --side-by-side <broken.patch
WARNING: terminal is not fully functional
thread 'main' panicked at /home/ilya/.cargo/registry/src/index.crates.io-6f17d22bba15001f/git-delta-0.17.0/src/paint.rs:907:17:
String mismatch encountered while superimposing style sections: ';' vs '↴'
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Press RETURN to continue

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

────┐
10: │
────┘
│ 10 │ a                                                   │ 10 │ a

And even with TERM unset, if I run the above command with 117 columns, the output is still colored.
I could not find a terminfo entry that disables all the coloring sequences.


Have you considered, what a test that would include the terminal escape sequences would look like?
I found tests that check line wrapping, but it seems to be running with the syntax highlight disabled, as the expectation is just plain text: main...ilya-bobyr:delta:wrap-panic

@ilya-bobyr
Copy link
Author

I ran a bisection, and it pointed at this change: a8446c5
Unfortunately, it is a dependency bump, so not something that is immediately explainable.

It was failing for 0.16.0, so I was bisecting between 0.15.0 and the tip of the master branch.

@dkarter
Copy link

dkarter commented Aug 4, 2024

Can confirm things are working in 0.15, but not in 0.16 or 0.17, sounds like we need to "bisect" those dependency updates until we can find the culprit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants