-
Notifications
You must be signed in to change notification settings - Fork 376
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
Comments
src/paint.rs:907
for a valid patch with syntax highlight and when line wrapping is enabledsrc/paint.rs:907
for a valid patch with syntax highlight and line wrapping
I was also unable to reproduce this, see #1724 |
This one also reproduces for me with the default configuration:
I've installed I'm on the latest Ubuntu, with Alacritty and tmux, but I doubt that it matters. |
I'm seeing the same error (0.17.0):
Also getting this one sometimes, not sure if it's related:
|
Yes, those are related. Does this happen with a public repo, or could you share a diff? With |
echo $COLUMNS
# 119 I was able to repro this on a public repo, here's the patch: To repro I opened the diff in a small tmux split and used this command: delta --no-gitconfig --side-by-side < bad.patch Can also repro this without Tmux, just Wezterm on mac: CleanShot.2024-08-03.at.23.52.19.mp4 |
@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. 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. |
Sorry, still can't reproduce this, using the 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
You might have to add @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 |
It panics for me with both 118 and 119 columns and with the
This is for version 0.17.0:
It could be worth noting that it fails even with the
And even with Have you considered, what a test that would include the terminal escape sequences would look like? |
I ran a bisection, and it pointed at this change: a8446c5 It was failing for 0.16.0, so I was bisecting between 0.15.0 and the tip of the |
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 |
Works fine with alacritty 0.14.0-dev (102b89a8, terminfo present) here, with or without tmux (3.3a-3) ... The dependencies will be hard to untangle, and I have a few more ideas: Does it also crash with If so, what about hiding the terminal completely from delta, i.e. Also, what do the syscalls which query the terminal report when running this command?
My `tty.strace` output when running in alacritty
|
I can reproduce locally as well
Exit code: 101 local environment
Different widths:
Hiding terminal: strace output |
Thank you, very interesting. And strange - but also... not reproducible here, with kitty 0.26.5 or 0.31.0 on Debian or Fedora. Looking at the strace output, I found that the diff between my These come from two locations, when querying the terminal width at set.rs:606 and when querying the terminal color scheme at theme.rs:131. The color scheme is newer and can easily be disabled by adding |
|
Works with |
Yeah unicode-width 0.1.13 contains a bunch of changes to how wide some characters are. See also #1718 |
Should be fixed in 0.18.2, thanks everyone, I was following completely wrong leads! |
With the following patch:
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
toa.txt
/b.txt
the panic goes away.Here is a complete output:
The text was updated successfully, but these errors were encountered: