[vim/vim] Add a test for 8.2.3391: Resizing a window crashes with showbreak and breakindentopt (#8828)

[Yegappan Lakshmanan]

---

You can view, comment on, or merge this pull request online at:

  https://github.com/vim/vim/pull/8828

Commit Summary

• Add asserts

File Changes

• M src/testdir/test_breakindent.vim (18)

Patch Links:

• https://github.com/vim/vim/pull/8828.patch
• https://github.com/vim/vim/pull/8828.diff

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.

[codecov[bot]]

Codecov Report

Merging #8828 (756dc08) into master (04626c2) will decrease coverage by 87.68%.
The diff coverage is n/a.

@@             Coverage Diff             @@

##           master    #8828       +/-   ##

===========================================

- Coverage   90.14%    2.45%   -87.69%     

===========================================

  Files         151      149        -2     

  Lines      170814   165556     -5258     

===========================================

- Hits       153985     4072   -149913     

- Misses      16829   161484   +144655     

Flag	Coverage Δ
huge-clang-none	?
huge-gcc-none	?
huge-gcc-testgui	?
huge-gcc-unittests	2.45% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/float.c	0.00% <0.00%> (-99.22%)	⬇️
src/gui_gtk_f.c	0.00% <0.00%> (-97.54%)	⬇️
src/crypt_zip.c	0.00% <0.00%> (-97.06%)	⬇️
src/match.c	0.00% <0.00%> (-96.98%)	⬇️
src/sha256.c	0.00% <0.00%> (-96.94%)	⬇️
src/evalbuffer.c	0.00% <0.00%> (-96.92%)	⬇️
src/textprop.c	0.00% <0.00%> (-96.85%)	⬇️
src/debugger.c	0.00% <0.00%> (-96.62%)	⬇️
src/libvterm/src/rect.h	0.00% <0.00%> (-96.56%)	⬇️
src/cmdhist.c	0.00% <0.00%> (-96.49%)	⬇️
... and 137 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 04626c2...756dc08. Read the comment docs.

[Dominique Pellé]

@dpelle commented on this pull request.

---

In src/testdir/test_breakindent.vim:

> @@ -831,4 +831,22 @@ func Test_breakindent20_list()
   call s:close_windows('set breakindent& briopt& linebreak& list& listchars& showbreak&')
 endfunc
 
+" The following used to crash Vim. This is fixed by 8.2.3391.
+" This is a regression introduced by 8.2.2903.

Just confirming that the test caused vim-8.2.3390 to crash and latest vim-8.2.3394 does not crash on Linux x86_64.
Thanks.

[Yegappan Lakshmanan]

Hi,

On Wed, Sep 1, 2021 at 9:55 PM Dominique Pellé <vim-dev...@256bit.org> wrote:

@dpelle commented on this pull request.

---

In src/testdir/test_breakindent.vim:

> @@ -831,4 +831,22 @@ func Test_breakindent20_list()
   call s:close_windows('set breakindent& briopt& linebreak& list& listchars& showbreak&')
 endfunc
 
+" The following used to crash Vim. This is fixed by 8.2.3391.
+" This is a regression introduced by 8.2.2903.

Just confirming that the test caused vim-8.2.3390 to crash and latest vim-8.2.3394 does not crash on Linux x86_64.
Thanks.

Yes. The test crashes Vim 8.2.3390 and doesn't crash with 8.2.3394.

- Yegappan

 

[vim-dev ML]

Hi,

On Wed, Sep 1, 2021 at 9:55 PM Dominique Pellé ***@***.***>
wrote:

> ***@***.**** commented on this pull request.
> ------------------------------
>
> In src/testdir/test_breakindent.vim
> <https://github.com/vim/vim/pull/8828#discussion_r700743830>:

>
> > @@ -831,4 +831,22 @@ func Test_breakindent20_list()
> call s:close_windows('set breakindent& briopt& linebreak& list& listchars& showbreak&')
> endfunc
>
> +" The following used to crash Vim. This is fixed by 8.2.3391.
> +" This is a regression introduced by 8.2.2903.
>
> Just confirming that the test caused vim-8.2.3390 to crash and latest
> vim-8.2.3394 does not crash on Linux x86_64.
> Thanks.
>
>
>
Yes. The test crashes Vim 8.2.3390 and doesn't crash with 8.2.3394.

- Yegappan

[Christian Brabandt]

thanks!

—
You are receiving this because you commented.

[Bram Moolenaar]

Thanks for creating this test.
There is another check for n_extra becoming negative, and if I comment it out no test fails.
Perhaps you can manage to write a test for that one too?
This is in line 1215 of src/drawline.c

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub.

[Bram Moolenaar]

Closed #8828 via 03d2579.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub.

[Yegappan Lakshmanan]

Hi Bram,

On Thu, Sep 2, 2021 at 11:05 AM Bram Moolenaar <vim-dev...@256bit.org> wrote:

Thanks for creating this test.
There is another check for n_extra becoming negative, and if I comment it out no test fails.
Perhaps you can manage to write a test for that one too?
This is in line 1215 of src/drawline.c

I created a test that exercises this line. But I ran into some bugs.

If you source the following script:

-------------------------------------------------------------------------------------

10new
20vnew
call setline(1, repeat('aa ', 200))
setlocal breakindent breakindentopt=shift:4 linebreak showbreak=>>
setlocal number
set cpo+=n
normal 12gj
redraw!
normal $
redraw!

 -------------------------------------------------------------------------------------

You will see that the cursor is positioned a few characters before the end

of the line and it cannot be moved to the end of the line. Also, if you move

the cursor to the left by pressing "h" quite a few times (to cause the screen

to scroll down), you will see the cursor can be positioned on the

'showbreak' characters.

- Yegappan

[vim-dev ML]

Hi Bram,

On Thu, Sep 2, 2021 at 11:05 AM Bram Moolenaar ***@***.***>

—
You are receiving this because you are subscribed to this thread.

Reply to this email directly, view it on GitHub.

[Bram Moolenaar]

It looks like this happens because the " >>" in the first line is not
counted, thus when w_skipcol is non-zero. It's also strange that there
is a space before ">>". Perhaps Christian knows what needs to change to
fix this.

--
The budget process was invented by an alien race of sadistic beings who
resemble large cats.
(Scott Adams - The Dilbert principle)

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// \\\
\\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

[vim-dev ML]

/// Bram Moolenaar -- ***@***.*** -- http://www.Moolenaar.net \\\

/// \\\
\\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

—
You are receiving this because you are subscribed to this thread.

Reply to this email directly, view it on GitHub.

[Christian Brabandt]

I'll have a look.

Best,
Christian
--
Wer am wenigsten des Morgen bedarf, der geht am heitersten dem Morgen
entgegen.
-- Epikur von Samos (341-271 v.Chr.)

[Yegappan Lakshmanan]

Hi Christian,

Should I open a Github issue to track this?

Regards,
Yegappan

> Best,
> Christian

[Christian Brabandt]

Hi Yegappan,
you can. I just don't have much spare time to debug such screen-logic.

Best,
Christian
--
Warum ließ sich einst ein Mathematiker scheiden?
Seine Frau war unberechenbar!

[Christian Brabandt]

I found that time yesterday. I think the following patch should fix it.
However I am not completely sure that this is the best way to do it. I
also need some more time to thoroughly test it. I will do this later and
submit a proper PR with your testcase.

diff --git a/src/charset.c b/src/charset.c
index fcaeedf75..953319af5 100644
--- a/src/charset.c
+++ b/src/charset.c
@@ -1090,6 +1090,16 @@ win_lbr_chartabsize(
colnr_T sbrlen = 0;
int numberwidth = win_col_off(wp);

+ // long line that does not fit in the window
+ // need to adjust for leading sbr value
+ if (wp->w_skipcol &&
+ wp->w_p_wrap &&
+ col >= wp->w_skipcol &&
+ *sbr != NUL &&
+ col <= wp->w_skipcol + wp->w_width
+ - win_col_off(wp) - MB_CHARLEN(sbr))
+ col -= (colnr_T)MB_CHARLEN(sbr);
+
numberextra = numberwidth;
col += numberextra + mb_added;
if (col >= (colnr_T)wp->w_width)


Best,
Christian

[Bram Moolenaar]

Can't say whether this is the right fix, just a small nit: I prefer putting the && at the start of the line.

--
--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

---
You received this message because you are subscribed to the Google Groups "vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+u...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/vim_dev/20210923070105.GB3729843%40256bit.org.

[Yegappan Lakshmanan]

Hi Christian,

Thanks for the patch. When I tested the patch, I saw a couple of problems.

After executing the above script, if I press 0 and then $ to go to the end of
the line, the cursor is positioned a few characters before the end of the line.
I am not able to move the cursor to the right by pressing 'l'. But if
I press 'h'
once then the cursor is positioned at the end of the line.

Also, if I press 'h' several times to cause the buffer to scroll down,
after hitting
the beginning of the first screen line, the cursor jumps to the second character
in the second screen line. If I press 'h' now, then the cursor jumps
to the fourth
character in the second screen line.

Regards,
Yegappan