[bug] cursor incorrectly positioned

[Dominique Pellé]

Hi

I see a bug with Vim-7.4.460. Steps to reproduce:

$ vim -u NONE -c 'set showbreak=: columns=95' bug-vim.txt

... where "bug-vim.txt" is the attached file. Then in
command mode, press A to append text at the end
of the line.

Observe that cursor is positioned before the end of
line (bug!). Appending characters appends at the end
of line, but cursor remains at the wrong position
before the end of line.

Bug only happens with columns=95 with this file, which
corresponds to having a tab displayed at the beginning
of a wrapped line.

Bug happens in the terminal as well as in gvim (gtk2).

It's not a new bug since I also see the bug in Vim-7.052
which comes with Ubuntu-14.04. I tried a much older
version (vim-7.1.126) which also had the same bug.
I did not try older versions.

Regards
Dominique

[Christian Brabandt]

Hi Dominique!

On Mi, 24 Dez 2014, Dominique Pellé wrote:

> Hi
>
> I see a bug with Vim-7.4.460. Steps to reproduce:
>
> $ vim -u NONE -c 'set showbreak=: columns=95' bug-vim.txt
>
> ... where "bug-vim.txt" is the attached file. Then in
> command mode, press A to append text at the end
> of the line.
>
> Observe that cursor is positioned before the end of
> line (bug!). Appending characters appends at the end
> of line, but cursor remains at the wrong position
> before the end of line.
>
> Bug only happens with columns=95 with this file, which
> corresponds to having a tab displayed at the beginning
> of a wrapped line.

Can you try the attached patch?

Kind regards,
Christian
--
Wie man sein Kind nicht nennen sollte:
Bred R. Zaun

[Dominique Pellé]

Hi Christian

You're quick! ... but too quick :-)

Your patch contains changes in getchar.c and ui.c
which seem unrelated to this issue. Right?

Part of your patch in search.c is also already present
in Vim-5.4.560 (it was part of patch 7.4.547).

The change in screen.c looks like the relevant
piece. But it does not work. It changes the
behavior, but the cursor still appears before
the end of line. Did it work for you?

I hope to find time later to try fixing it. This
piece of code in screen.c is a bit hairy though.

Regards
Dominique

[Christian Brabandt]

Hi Dominique!

On Do, 25 Dez 2014, Dominique Pellé wrote:

> You're quick! ... but too quick :-)
>
> Your patch contains changes in getchar.c and ui.c
> which seem unrelated to this issue. Right?

Yes, some other changes I am working on. I shouldn't post patches, right
before I am going to sleep...

> The change in screen.c looks like the relevant
> piece. But it does not work. It changes the
> behavior, but the cursor still appears before
> the end of line. Did it work for you?

Yes, it appeared to work for me. Here is a better version, that works
for me and also handles correctly multibyte chars for the showbreak
setting (a problem, that I have previously caused and needed to be fixed
by 7.4.478):

diff --git a/src/screen.c b/src/screen.c
--- a/src/screen.c
+++ b/src/screen.c
@@ -4516,9 +4516,14 @@ win_line(wp, lnum, startrow, endrow, noc
if (c == TAB && (!wp->w_p_list || lcs_tab1))
{
int tab_len = 0;
+ long vcol_adjusted = vcol; /* removed showbreak length */
+#ifdef FEAT_LINEBREAK
+ if (*p_sbr != NUL && vcol > W_WIDTH(wp))
+ vcol_adjusted = vcol - MB_CHARLEN(p_sbr);
+#endif
/* tab amount depends on current column */
tab_len = (int)wp->w_buffer->b_p_ts
- - vcol % (int)wp->w_buffer->b_p_ts - 1;
+ - vcol_adjusted % (int)wp->w_buffer->b_p_ts - 1;
#ifdef FEAT_LINEBREAK
if (!wp->w_p_lbr || !wp->w_p_list)
#endif

Please check that this works for you as well. If this works, I'll create
a test case for this as well.


Best,
Christian
--
Nicht jeder, der fest im Sattel sitzt, kann auch reiten.
-- Kim Snolden

[Dominique Pellé]

Hi Christian

This 2nd patch fixes the case I described. Thanks.
However, a similar bug then happens with 92 columns
after your patch:

$ vim -f -g -u NONE -c 'set showbreak=: columns=92' bug-vim.txt

When pressing A in command mode to append text, the cursor
is after the end of line. It was working fine with 92 columns
prior to your patch. So it's still not right.

Regards
Dominique

[Christian Brabandt]

Am 2014-12-25 14:07, schrieb Dominique Pellé:
(sorry for the PM)

> This 2nd patch fixes the case I described. Thanks.
> However, a similar bug then happens with 92 columns
> after your patch:
>
> $ vim -f -g -u NONE -c 'set showbreak=: columns=92' bug-vim.txt
>
> When pressing A in command mode to append text, the cursor
> is after the end of line. It was working fine with 92 columns
> prior to your patch. So it's still not right.

We are getting there.

So this only happens, if the TAB is the first char after the showbreak
value
has been drawn, right?

So try this patch.

Kind regards,
Christian

[Christian Brabandt]

Sorry, again missing proper multibyte handling.

So please try again.

Best,
Christian

[Christian Brabandt]

Hi Dominique!

On Fr, 26 Dez 2014, Dominique Pellé wrote:

> On Fri, Dec 26, 2014 at 2:37 PM, Christian Brabandt <cbl...@256bit.org> wrote:
> > Hi Dominique!
> >

> > On Fr, 26 Dez 2014, Dominique Pellé wrote:
> >
> >> Sorry for the noise. My latest patch does not fix it.
> >> I had been testing vim small, which does not have
> >> showbreak so it did not show the bug <sigh>
> >> The bug is still present in vim huge with my patch
> >> or with your latest patch, at least when doing:
> >>
> >> $ vim -u NONE -c 'set nu cpo= sbr=: co=49' bug-vim.txt
> >>
> >> And press A in command mode to append
> >> char at the end of the line and see that cursor is
> >> incorrectly positionned.
> >
> > Oh, that happens because the last TAB char at the previous line is drawn
> > too long. Here is an updated patch.
>
> It's not an easy bug to fix :-(

Yes, the screen drawing code is complex and I think you have now found
another problem, that is actually not caused by that but by the
win_lbr_chartabsize function.

> It's still broken with your latest patch using...

I come to the conclusion, this is a different problem, that is
independent of the problem you reported initially. The problem with the
screen size of 49 is actually, that Vim does not consider, the
numberwidth option and thinks the second last tab would not wrap, while
it does.

The screen drawing code is independent of this, and does take this into
account (and therefore my previous patch was wrong, because the screen
drawing code does in fact take the length of the 'sbr' into account and
adjusts the length of a tab correctly, and therefore, my last attempt
fixed this special case, but the general case was broken, as you pointed
out in your last message)

Therefore I think we have observed two different problems that need to
be fixed. Part one is the wrong drawing of the length of a tab as
pointed in your initial mail (patch sbr_buf_new.diff), part two is the
wrong calculation of the column position and therefore not taking into
account that wrapping will actually happen. This is fixed by the patch
win_lbr_chartabsize_numberwidth.diff. That function is unfortunately
called a lot and I hope, this patch does not brake at other parts of the
code).

>
> $ vim -u NONE -c 'set nu cpo= sbr=: co=90' bug-vim.txt
>
> It's also broken with:
>
> $ vim -u NONE -c 'set sbr=: co=84' bug-vim.txt
>
> Then press A in command mode and cursor gets beyond
> the end of line.

That is the general case and should now again work as expected again.

I have checked will all your 4 provided test cases and it looks good so
far. However you have successfully found many different problems with
different screen sizes so you might want to check, that this works as
well.

I'll copy vim-dev. Once you verified, that it works for you correctly,
I'll try to create some test cases that catch at least those two
problems.

Best,
Christian
--
Die höchste Liebe glaubt und fordert höchste Vollkommenheit, daher ist
sie ihrem Ende am nächsten.
-- Jean Paul

[Dominique Pellé]

Hi Christian

I installed your 2 latest patches. But I still see the bug :-(

A good way to test manually is to press A or $ to move
cursor the the end of line, and then resize the terminal
with the mouse to various horizontal sizes and check that
the cursor remains at the end of line for various sizes.

I see the bug at least with:

$ vim -u NONE -c 'set nu cpo= sbr=: co=88' bug-vim.txt
or

$ vim -u NONE -c 'set nu cpo= sbr=: co=49' bug-vim.txt

Then press A to append at end of line and observe the
cursor at the wrong location.

Regards
Dominique

[Christian Brabandt]

Hi Dominique!

Hm, I could fix those, but now some tests are failing. I will look into
those issue more closely within the next week.

Best,
Christian
--
Wenn es heute 0 Grad ist und morgen doppelt so kalt werden soll,
wie kalt wird es morgen?

[Christian Brabandt]

Hi vim-dev!

I have checked with Dominique some more in private and here are two
updated patches. They fix some wrong cursor positioning with long
(wrapped) screen lines.

those patches are also available at:
https://github.com/chrisbra/vim-mq-patches/blob/master/win_lbr_chartabsize_numberwidth
https://github.com/chrisbra/vim-mq-patches/blob/master/sbr_bug

Best,
Christian
--
Made in Germany - da ist der Wurm drin.
-- Heinz Erhard