Bug: visual mode and linebreak [patch - add a failing test]

99 views
Skip to first unread message

Nazri Ramliy

unread,
Sep 5, 2014, 9:53:25 PM9/5/14
to vim_dev
Hi,

Attached is a patch that adds a failing test that should show the
buggy behavior on the following text:

-- 8< --
+REMOVE: This text must not be deleted
+REMOVE: aaaaaaa...repeat until linebreak kicks in...aaaaaaaaaaa....
+-- >8 --

With linebreak set and doing a character-wise visual selection on both
the "REMOVE: " characters (including the single whitespaces) will
select more than both of the "REMOVE: " characters.

Nazri
linebreak-visual-bug.patch

Bram Moolenaar

unread,
Sep 19, 2014, 10:52:04 AM9/19/14
to Nazri Ramliy, vim_dev
The test writes the ".ok" file, that's not the right way to do this.

Christian, did you already look into fixing this?

--
hundred-and-one symptoms of being an internet addict:
162. You go outside and look for a brightness knob to turn down the sun.

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

Christian Brabandt

unread,
Sep 19, 2014, 4:42:36 PM9/19/14
to vim_dev
On Fr, 19 Sep 2014, Bram Moolenaar wrote:

>
> Nazri Ramliy wrote:
>
> > Attached is a patch that adds a failing test that should show the
> > buggy behavior on the following text:
> >
> > -- 8< --
> > +REMOVE: This text must not be deleted
> > +REMOVE: aaaaaaa...repeat until linebreak kicks in...aaaaaaaaaaa....
> > +-- >8 --
> >
> > With linebreak set and doing a character-wise visual selection on both
> > the "REMOVE: " characters (including the single whitespaces) will
> > select more than both of the "REMOVE: " characters.
>
> The test writes the ".ok" file, that's not the right way to do this.
>
> Christian, did you already look into fixing this?

No, I thought this was just about a missing test. I'll look into it next
week.

Best,
Christian
--
Schön ist eigentliches alles, was man mit Liebe betrachtet.
-- Christian Morgenstern

Christian Brabandt

unread,
Sep 22, 2014, 5:28:15 PM9/22/14
to vim_dev
On Fr, 19 Sep 2014, Christian Brabandt wrote:

> On Fr, 19 Sep 2014, Bram Moolenaar wrote:
>
> >
> > Nazri Ramliy wrote:
> >
> > > Attached is a patch that adds a failing test that should show the
> > > buggy behavior on the following text:
> > >
> > > -- 8< --
> > > +REMOVE: This text must not be deleted
> > > +REMOVE: aaaaaaa...repeat until linebreak kicks in...aaaaaaaaaaa....
> > > +-- >8 --
> > >
> > > With linebreak set and doing a character-wise visual selection on both
> > > the "REMOVE: " characters (including the single whitespaces) will
> > > select more than both of the "REMOVE: " characters.
> >
> > The test writes the ".ok" file, that's not the right way to do this.
> >
> > Christian, did you already look into fixing this?
>
> No, I thought this was just about a missing test. I'll look into it next
> week.

This is ... complicated.

Here is a visual representation of what happens:
,-----------------,
| 1 BAR: xxxx |
| 2 BAR: |
| xxxxxxxxxxxxx |
`----------------´
(now visually block-select the BAR: (including the following
whitespace). You'll notice, that the visual selection extends till the
end of the window.

The problem is, when linebreak is set, win_lbr_chartabsize() will select
the complete whitespace until the end of the screen line, this is, so
that the linebreak feature works and the rest of the text is drawn on
the next line (and also moving around works as expected).

I have no idea how to fix it. I played around in function win_update()
where wp->w_old_cursor_lcol is set, but this has obviously problems when
moving around with the cursor.

Best,
Christian
--
Wie man sein Kind nicht nennen sollte:
Klaas Nost

Christian Brabandt

unread,
Sep 23, 2014, 3:56:51 PM9/23/14
to vim_dev
I forgot to mention, that virtual editing does seem to work, except that
this replaces the single space in the last line by the same amount of
spaces as the window is wide. This might be a bug there as well.

Perhaps it is easier, to temporarily reset the linebreak option? Here is
a patch, that does this and also prevents that the screen will be
highlighted incorrectly (and includes a slightly modified test case from
Nazri).

BTW: The patch also fixes an error for the test_listlbr patch, that
prevents it from being actually being executed (rather it will simply
copy the ok file), because the
exists("+conceal")
should actually be a
has("conceal")

(Oh man, I wondered quite a while, why the test would always pass...)

Best,
Christian
--
Stoffartige Hilfe, die sich die Poesie der letzten Zeit gibt
durch bedeutende Motive, Religion und Ritterwesen.
-- Goethe, Maximen und Reflektionen, Nr. 1310
linebreak_visual.patch.diff

Bram Moolenaar

unread,
Sep 27, 2014, 5:19:37 AM9/27/14
to Christian Brabandt, vim_dev
Thanks!

> (Oh man, I wondered quite a while, why the test would always pass...)

We need a test for the test :-).

--
hundred-and-one symptoms of being an internet addict:
186. You overstay in the office so you can have more time surfing the net.
Reply all
Reply to author
Forward
0 new messages