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

Do not panic on superimposition mismatch in release builds #1450

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

th1000s
Copy link
Contributor

@th1000s th1000s commented Jun 19, 2023

Prints: !!<delta bug 1450> .... !please report! which should be eye-catching enough.

Lines which would trigger a panic and early exit with the message "String mismatch encountered while superimposing style sections" now try to print the line as well as possible plus a brief error message.

In debug builds and when running from the target/ dir (such as when core.pager = "target/release/delta" is set) or when the env var DELTA_NO_WORKAROUNDS is set this mismatch panics as before.

@dandavison
Copy link
Owner

Good call -- I agree that the nature of delta is such that it is generally more helpful to just keep going.

@dandavison
Copy link
Owner

(This is in Draft state so I'll wait for you to take an action here.)

Lines which would trigger a panic and early exit with the message
"String mismatch encountered while superimposing style sections"
now try to print the line as well as possible plus a brief error message.

In debug builds and when running from the target/ dir (such as when
core.pager = "target/release/delta" is set) or when the env var
DELTA_NO_WORKAROUNDS is set this mismatch panics as before.
@th1000s
Copy link
Contributor Author

th1000s commented Jun 21, 2023

So, if this error shows up in the side-by-side view it can get truncated, all I can do is trim the message a bit. Well, still better than crashing...

Remind me, why is the superimposition done so late, couldn't it be done after Painter::update_diff_style_sections()? This is e.g. why wrap has to be called for the syntax and the diff part separately.

@dandavison
Copy link
Owner

Remind me, why is the superimposition done so late, couldn't it be done after Painter::update_diff_style_sections()? This is e.g. why wrap has to be called for the syntax and the diff part separately.

Interesting, I hadn't noticed that this was a bit ugly and that there seems to be the possibility that you point out. I looked at the code and I agree it looks superficially at least as if it could be done after Painter::update_diff_style_sections()

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

Successfully merging this pull request may close these issues.

None yet

2 participants