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

10 views
Skip to first unread message

Yegappan Lakshmanan

unread,
Sep 1, 2021, 9:54:10 PMSep 1
to vim/vim, Subscribed

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

Patch Links:


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]

unread,
Sep 1, 2021, 10:01:37 PMSep 1
to vim/vim, Subscribed

Codecov Report

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

Impacted file tree graph

@@             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é

unread,
Sep 2, 2021, 12:55:37 AMSep 2
to vim/vim, Subscribed

@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

unread,
Sep 2, 2021, 1:21:35 AMSep 2
to vim_dev, reply+ACY5DGCJVNEILOEEU5...@reply.github.com, vim/vim, Subscribed
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

unread,
Sep 2, 2021, 1:21:51 AMSep 2
to vim/vim, vim-dev ML, Your activity

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

unread,
Sep 2, 2021, 2:52:02 AMSep 2
to vim/vim, vim-dev ML, Comment

thanks!


You are receiving this because you commented.

Bram Moolenaar

unread,
Sep 2, 2021, 2:05:11 PMSep 2
to vim/vim, vim-dev ML, Comment

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

unread,
Sep 2, 2021, 2:05:51 PMSep 2
to vim/vim, vim-dev ML, Comment

Closed #8828 via 03d2579.


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

Yegappan Lakshmanan

unread,
Sep 3, 2021, 10:22:25 AMSep 3
to vim_dev, reply+ACY5DGBYSD4EEE5Z2O...@reply.github.com, vim/vim, vim-dev ML, Comment
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

unread,
Sep 3, 2021, 10:22:44 AMSep 3
to vim/vim, vim-dev ML, Your activity

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

unread,
Sep 3, 2021, 1:23:11 PMSep 3
to vim...@googlegroups.com, Christian Brabandt, Yegappan Lakshmanan, reply+ACY5DGBYSD4EEE5Z2O...@reply.github.com
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

unread,
Sep 3, 2021, 1:23:25 PMSep 3
to vim/vim, vim-dev ML, Your activity
/// 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

unread,
Sep 3, 2021, 2:14:41 PMSep 3
to vim...@googlegroups.com
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

unread,
Sep 22, 2021, 12:51:23 PMSep 22
to vim_dev
Hi Christian,
Should I open a Github issue to track this?

Regards,
Yegappan

> Best,
> Christian

Christian Brabandt

unread,
Sep 22, 2021, 12:59:26 PMSep 22
to vim_dev
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

unread,
Sep 23, 2021, 3:01:10 AMSep 23
to vim_dev
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

unread,
Sep 23, 2021, 3:40:24 AMSep 23
to vim_dev
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

unread,
Sep 23, 2021, 10:06:09 AMSep 23
to vim_dev
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
Reply all
Reply to author
Forward
0 new messages