avoid negative repeat_count fixes #1760

[lrfurtado]

No description provided.

[th1000s]

Nice find, interesting that this unsigned underflow was never triggered before.

Can you provide a minimal example diff which triggers this so it can be called with cat diff.txt | delta --no-gitconfig --width=X? I assume the current terminal width is a contributing factor, so --width should make it independent of that. Then a test with insta's assert_snapshot! can be written to ensure that never happens again.

[lrfurtado]

commit XXXXXXXXXXXXXXXXXXXXXXXXX
Author: LUCIANO FURTADO <[email protected]>
Date:   Tue Jun 25 14:07:31 2024 -0500

    Ref: https://github.com/dandavison/delta/issues/1760

diff --git a/foobar b/foobar
index 185a33cd5b..123f893935 100755
--- a/foobar
+++ b/foobar
@@ -14,6 +14,11 @@ function UNDERFLOW_EXEC() {
    XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX XX  
 }

+
+
+echo "=== VALIDATE UNDERFLOW"
+/usr/local/bin/check_underflow
+
 # this is an example of the underflow error
 echo "=== UNDERFLOW_EXEC"
 UNDERFLOW_EXEC -i foobar.yaml

[lrfurtado]

@th1000s are my latest changes what you had in mind ?

[th1000s]

Yes, however when keeping the test and reverting your fix, then running cargo test test_long_line_underflow -- --nocapture does not trigger the crash. But this BgFillMethod::Spaces code path is quite hard to it, I could not do it after trying a few parameters.

Can you reproduce this when piping your example input into the delta (debug build) binary? And can you reproduce this when specifying --no-gitconfig and then adding the settings from your git config manually? And then, what is your terminal width: echo $COLUMNS.

[lrfurtado]

found something interesting no crash with --no-gitconfig

It's something on my gitconfig + the diff on the test that causes causes the crash.

The part of my gitconfig related to delta is:

[lrfurtado]

The options causing the problem seems to be

[lrfurtado]

without the saturating_sub the test now crashes

[lrfurtado]

@th1000s I think it looks better now

[lrfurtado]

@th1000s I think it looks better now

Found something interesting @th1000s --width 111 is not fully insulating the unitest from terminal sizes differences, if I increase or decrease the size of the font on my terminal. The tests start breaking.

[th1000s]

Nice, thank you for the investigation, looks like styling zero hunks is not tested enough. And because of this other bug you found (but is now nicely reproducible) let's just add #[ignore] to the #[test].

And when you set the expected output in assert_snapshot!(..) try setting your terminal to 111 so it is as it should be. Also, cargo fmt will take care of the code formatting.

[th1000s]

You can leave out almost all of these +- lines, as you found out the context before the changed lines was the cause.

[lrfurtado]

any concerns leaving as is ?

[th1000s]

Well, it makes the minimal reproducer diff larger than it has to be, and the text sort of indicates that it's these lines. As you have to rebase anyhow to make it build with rust 1.80, maybe you can also trim the added lines down to one or two.