Patch 8.1.2052

[Bram Moolenaar]

Patch 8.1.2052
Problem: Using "x" before a closed fold may delete that fold.
Solution: Do not translate 'x' do "dl". (Christian Brabandt, closes #4927)
Files: src/normal.c, src/testdir/test_fold.vim


*** ../vim-8.1.2051/src/normal.c 2019-09-09 20:04:04.738340561 +0200
--- src/normal.c 2019-09-17 22:38:03.254221383 +0200
***************
*** 7381,7388 ****

if (!checkclearopq(cap->oap))
{
! /* In Vi "2D" doesn't delete the next line. Can't translate it
! * either, because "2." should also not use the count. */
if (cap->cmdchar == 'D' && vim_strchr(p_cpo, CPO_HASH) != NULL)
{
cap->oap->start = curwin->w_cursor;
--- 7381,7388 ----

if (!checkclearopq(cap->oap))
{
! // In Vi "2D" doesn't delete the next line. Can't translate it
! // either, because "2." should also not use the count.
if (cap->cmdchar == 'D' && vim_strchr(p_cpo, CPO_HASH) != NULL)
{
cap->oap->start = curwin->w_cursor;
***************
*** 7400,7406 ****
{
if (cap->count0)
stuffnumReadbuff(cap->count0);
! stuffReadbuff(ar[(int)(vim_strchr(str, cap->cmdchar) - str)]);
}
}
cap->opcount = 0;
--- 7400,7412 ----
{
if (cap->count0)
stuffnumReadbuff(cap->count0);
! // If on an empty line and using 'x' and "l" is included in the
! // whichwrap option, do not delete the next line.
! if (cap->cmdchar == 'x' && vim_strchr(p_ww, 'l') != NULL
! && gchar_cursor() == NUL)
! stuffReadbuff((char_u *)"dd");
! else
! stuffReadbuff(ar[(int)(vim_strchr(str, cap->cmdchar) - str)]);
}
}
cap->opcount = 0;
*** ../vim-8.1.2051/src/testdir/test_fold.vim 2019-08-24 20:49:58.825320302 +0200
--- src/testdir/test_fold.vim 2019-09-17 22:36:13.970638225 +0200
***************
*** 757,759 ****
--- 757,771 ----
bwipe!
bwipe!
endfunc
+
+ func Test_fold_delete_with_marker_and_whichwrap()
+ new
+ let content1 = ['']
+ let content2 = ['folded line 1 "{{{1', ' test', ' test2', ' test3', '', 'folded line 2 "{{{1', ' test', ' test2', ' test3']
+ call setline(1, content1 + content2)
+ set fdm=marker ww+=l
+ normal! x
+ call assert_equal(content2, getline(1, '$'))
+ set fdm& ww&
+ bwipe!
+ endfunc
*** ../vim-8.1.2051/src/version.c 2019-09-17 21:27:46.453881351 +0200
--- src/version.c 2019-09-17 22:39:12.113961089 +0200
***************
*** 759,760 ****
--- 759,762 ----
{ /* Add new patch number below this line */
+ /**/
+ 2052,
/**/

--
Facepalm statement #2: "If there is a country without immigrants I'm going to
move there"

/// 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]

On Di, 17 Sep 2019, Bram Moolenaar wrote:

> Patch 8.1.2052
> Problem: Using "x" before a closed fold may delete that fold.
> Solution: Do not translate 'x' do "dl". (Christian Brabandt, closes #4927)
> Files: src/normal.c, src/testdir/test_fold.vim

Quoting from the related issue:

> Thanks. But the line should still be deleted, thus in this case the
> translated operation is "dd".

I am not sure I agree.

I did intentionally not delete the line. My idea was, that `x` on an
empty line (or after a newline [e.g. with `:set
virtualedit=onemore`]) should be a no-op and the `whichwrap` option
should not apply to it, since the user did not type `dl`.

And because there is no character after the cursor I think that `x`
should not do anything (just like `x` on an empty line with a default
whichwrap setting).

In fact, the new behaviour will now delete a line, e.g. with

:set ww+=l virtualedit=onemore list listchars=eol:$

one a line like this and the cursor on the End-of-line marker ($):

abcd$

pressing x on this will now delete the line and this is rather unexpected.

In fact, if we are going to translate `x` to `dd` in that case, we
probably should do the same for `X` when `h` is in whichwrap.

But I still think this is wrong.

What do you think? I can make a patch either way.

Best,
Christian
--
Eine fortschrittliche Frau fortgeschrittenen Alters kann keine Macht
der Welt im Zaum halten.
-- Dorothy L. Sayers

[Bram Moolenaar]

Christian wrote:

> On Di, 17 Sep 2019, Bram Moolenaar wrote:
>
> > Patch 8.1.2052
> > Problem: Using "x" before a closed fold may delete that fold.
> > Solution: Do not translate 'x' do "dl". (Christian Brabandt, closes #4927)
> > Files: src/normal.c, src/testdir/test_fold.vim
>
> Quoting from the related issue:
>
> > Thanks. But the line should still be deleted, thus in this case the
> > translated operation is "dd".
>
> I am not sure I agree.
>
> I did intentionally not delete the line. My idea was, that `x` on an
> empty line (or after a newline [e.g. with `:set
> virtualedit=onemore`]) should be a no-op and the `whichwrap` option
> should not apply to it, since the user did not type `dl`.

I thought we only needed to avoid deleting the following fold and not
change anything else. But now I read this comment in the help for
'whichwrap':

When 'l' is included and it is used after an operator at the end of a
line then it will not move to the next line. This makes "dl", "cl",
"yl" etc. work normally.

This would apply since "x" is equivalent to "dl". However, if you try
"dl" on an empty line, it does delete the line break. It's like the
cursor is on top of the line break, so that gets deleted.

> And because there is no character after the cursor I think that `x`
> should not do anything (just like `x` on an empty line with a default
> whichwrap setting).
>
> In fact, the new behaviour will now delete a line, e.g. with
>
> :set ww+=l virtualedit=onemore list listchars=eol:$
>
> one a line like this and the cursor on the End-of-line marker ($):
>
> abcd$
>
> pressing x on this will now delete the line and this is rather unexpected.
>
> In fact, if we are going to translate `x` to `dd` in that case, we
> probably should do the same for `X` when `h` is in whichwrap.
>
> But I still think this is wrong.
>
> What do you think? I can make a patch either way.

Let's be practical: We fixed something because a user complained and it
was clear it had to be fixed. For "x" deleting a line we did not get
complaints. And, depending on how you look at it, if "l" crosses line
boundaries, it makes sense that "x" does too. Think of editing a
paragraph with wrapped lines, then repeating "x" should not get stuck
when the line gets empty.

--
Facepalm statement #5: "Petrol getting more expensive? Not for me, I'm always
tanking for 20 dollars"

[Christian Brabandt]

On Mi, 18 Sep 2019, Bram Moolenaar wrote:

> Let's be practical: We fixed something because a user complained and it
> was clear it had to be fixed. For "x" deleting a line we did not get
> complaints. And, depending on how you look at it, if "l" crosses line
> boundaries, it makes sense that "x" does too. Think of editing a
> paragraph with wrapped lines, then repeating "x" should not get stuck
> when the line gets empty.

okay fine. I am still worried, that we are possibly deleting a line
content, that is not wanted (e.g. when virtualedit is active and the
cursor is on a NUL).

So perhaps it would be better that 'x' in that case works like 'gJ'
(which does effectively only delete the line break).

And for consistency, we should to translate X to either 'dd' or "-gJ"
when 'h' is in whichwrap, right?

Best,
Christian
--
Aber kein Genuß ist vorübergehend: denn der Eindruck, den er
zurückläßt, ist bleibend.
-- Johann Wolfgang von Goethe (Lehrjahre)

[Christian Brabandt]

On Mi, 18 Sep 2019, Christian Brabandt wrote:
> On Mi, 18 Sep 2019, Bram Moolenaar wrote:
>
> > Let's be practical: We fixed something because a user complained and it
> > was clear it had to be fixed. For "x" deleting a line we did not get
> > complaints. And, depending on how you look at it, if "l" crosses line
> > boundaries, it makes sense that "x" does too. Think of editing a
> > paragraph with wrapped lines, then repeating "x" should not get stuck
> > when the line gets empty.
>
> okay fine. I am still worried, that we are possibly deleting a line
> content, that is not wanted (e.g. when virtualedit is active and the
> cursor is on a NUL).
>
> So perhaps it would be better that 'x' in that case works like 'gJ'
> (which does effectively only delete the line break).
>
> And for consistency, we should to translate X to either 'dd' or "-gJ"
> when 'h' is in whichwrap, right?

Sorry, I am wrong, X does already the sane thing on an empty line even
when 'h' is in the whichwrap option. So I am just wondering if perhaps x
should be translated to 'gJ' so it does the right thing, even when the
cursor is after the end of line.

Best,
Christian
--
Das Individuelle entscheidet überall. Wie wenig kann jeder vom besten
Helden brauchen! - Der Dichter gibt überall nur sittliche Momente, die
jeder anwende!
-- Jean Paul

[Tony Mechelynck]

On Wed, Sep 18, 2019 at 1:46 PM Christian Brabandt <cbl...@256bit.org> wrote:
>
>
> On Mi, 18 Sep 2019, Bram Moolenaar wrote:
>
> > Let's be practical: We fixed something because a user complained and it
> > was clear it had to be fixed. For "x" deleting a line we did not get
> > complaints. And, depending on how you look at it, if "l" crosses line
> > boundaries, it makes sense that "x" does too. Think of editing a
> > paragraph with wrapped lines, then repeating "x" should not get stuck
> > when the line gets empty.
>
> okay fine. I am still worried, that we are possibly deleting a line
> content, that is not wanted (e.g. when virtualedit is active and the
> cursor is on a NUL).
>
> So perhaps it would be better that 'x' in that case works like 'gJ'
> (which does effectively only delete the line break).
>
> And for consistency, we should to translate X to either 'dd' or "-gJ"
> when 'h' is in whichwrap, right?
>
> Best,
> Christian

When the cursor is on an end-of-line (even without 'list', which I
use) I would expect that x, or dl, would join the current line with
the next one. Not that it'd be a no-op.

Best regards,
Tony.

[Christian Brabandt]

On Mi, 18 Sep 2019, Tony Mechelynck wrote:

> When the cursor is on an end-of-line (even without 'list', which I
> use) I would expect that x, or dl, would join the current line with
> the next one. Not that it'd be a no-op.

So `dd` would be wrong?

Best,
Christian
--
Alle wollen zurück zur Natur - nur nicht zu Fuß.

[Andy Wokula]

Am 17.09.2019 um 22:45 schrieb Bram Moolenaar:
> Patch 8.1.2052
> Problem: Using "x" before a closed fold may delete that fold.
> Solution: Do not translate 'x' do "dl". (Christian Brabandt, closes #4927)
> Files: src/normal.c, src/testdir/test_fold.vim

So it's ok when `dl' deletes that fold?

And `x' is documented to behave like `dl'.

--
Andy

[Christian Brabandt]

No the patch fixes it and deletes only the line on which the cursor is.

But I think it creates another problem.

Take a file like this:

,----[ file.txt ]
| abc
| def
| ghi
`----

Now start Vim like this:

vim --clean -c ':set ww+=l virtualedit=onemore' -c 'norm! 1ggl' file

The cursor is now after the content of line 1 ('_' being the cursor, ga
returns 'NUL' on that position):

,----[ file.txt ]
| abc_
| def
| ghi
`----

Now press 'x' and you are left with:

,----[ file.txt ]
| def
| ghi
`----

And I think it is wrong in that 'x' deleted the first line (e.g.
characters before the cursor position).

I think it should probably be:

,----
| abcdef
| ghi
`----

That is what this patch would do:

diff --git a/src/normal.c b/src/normal.c
index e83c4c0be..6afebd1ab 100644
--- a/src/normal.c
+++ b/src/normal.c
@@ -7404,7 +7404,7 @@ nv_optrans(cmdarg_T *cap)

// whichwrap option, do not delete the next line.

if (cap->cmdchar == 'x' && vim_strchr(p_ww, 'l') != NULL

&& gchar_cursor() == NUL)
- stuffReadbuff((char_u *)"dd");
+ stuffReadbuff((char_u *)"gJ");
else

stuffReadbuff(ar[(int)(vim_strchr(str, cap->cmdchar) - str)]);
}

Best,
Christian
--
Wußten Sie, daß Sie wenn Sie einen Polizisten tätlich angreifen bis
zu zwei Jahre Haft bekommen können? Wenn Sie aber das Polizeiauto
beschädigen, gibt es bis zu fünf Jahren.
-- Hagen Rether

[Andy Wokula]

And `x' is documented to behave like `dl'. So it's ok to add a special case for `x'?

Why not fix `dl'?

--
Andy

[Christian Brabandt]

On Mi, 18 Sep 2019, 'Andy Wokula' via vim_dev wrote:
> So it's ok when `dl' deletes that fold?

No I don't think so. Do you think so because x is expected to work like
'dl' and `l` is expected to move to the next line?

> And `x' is documented to behave like `dl'. So it's ok to add a special case for `x'?
>
> Why not fix `dl'?

,----[ :h 'ww' ]

| When 'l' is included and it is used after an operator at the end of a
| line then it will not move to the next line. This makes "dl", "cl",
| "yl" etc. work normally.

`----

Yes, the documentation states, it won't move to the next line, but in
fact dl on an empty line does move to the next line and removes it
(including deleting a closed fold). So perhaps we should fix the 'dl'
behaviour on an empty line and revert the fix above.

Best,
Christian
--
Als Humboldt den Chimborasso bestieg, war die Luft so dünn, daß er nicht mehr ohne Brille
lesen konnte.
-- Johann Georg August Galletti

[Christian Brabandt]

On Mi, 18 Sep 2019, Bram Moolenaar wrote:

> I thought we only needed to avoid deleting the following fold and not
> change anything else. But now I read this comment in the help for
> 'whichwrap':
>
> When 'l' is included and it is used after an operator at the end of a
> line then it will not move to the next line. This makes "dl", "cl",
> "yl" etc. work normally.
>
> This would apply since "x" is equivalent to "dl". However, if you try
> "dl" on an empty line, it does delete the line break. It's like the
> cursor is on top of the line break, so that gets deleted.

So how about adjusting the code to do what is actually in the
documentation, so even when the cursor is on the linebreak, do not move
over to the next line. This should also fix the problem that lead to
this patch initially. I think this patch does it:

diff --git a/src/normal.c b/src/normal.c
index f91217ec1..06242b89f 100644
--- a/src/normal.c
+++ b/src/normal.c
@@ -5985,8 +5985,7 @@ nv_right(cmdarg_T *cap)
* Set cap->oap->inclusive when last char in the line is
* included, move to next line after that */
if ( cap->oap->op_type != OP_NOP
- && !cap->oap->inclusive
- && !LINEEMPTY(curwin->w_cursor.lnum))
+ && !cap->oap->inclusive)
cap->oap->inclusive = TRUE;
else
{
@@ -7418,13 +7417,7 @@ nv_optrans(cmdarg_T *cap)

{
if (cap->count0)
stuffnumReadbuff(cap->count0);

- // If on an empty line and using 'x' and "l" is included in the
- // whichwrap option, do not delete the next line.
- if (cap->cmdchar == 'x' && vim_strchr(p_ww, 'l') != NULL
- && gchar_cursor() == NUL)

- stuffReadbuff((char_u *)"dd");

- else
- stuffReadbuff(ar[(int)(vim_strchr(str, cap->cmdchar) - str)]);
+ stuffReadbuff(ar[(int)(vim_strchr(str, cap->cmdchar) - str)]);
}
}
cap->opcount = 0;
diff --git a/src/testdir/test_fold.vim b/src/testdir/test_fold.vim
index 824a4f22f..d89f91298 100644
--- a/src/testdir/test_fold.vim
+++ b/src/testdir/test_fold.vim
@@ -765,7 +765,7 @@ func Test_fold_delete_with_marker_and_whichwrap()

call setline(1, content1 + content2)

set fdm=marker ww+=l
normal! x
- call assert_equal(content2, getline(1, '$'))
+ call assert_equal(content1 + content2, getline(1, '$'))
set fdm& ww&
bwipe!
endfunc


Best
Christian
--
Das ist das Teuflischste, was eine menschliche Hand tun kann.
Darum zahlen wir mit den schrecklichen Dingen, die in der Welt
geschehen.
-- Mutter Teresa

[Bram Moolenaar]

Christian wrote:

> On Mi, 18 Sep 2019, Bram Moolenaar wrote:
>
> > I thought we only needed to avoid deleting the following fold and not
> > change anything else. But now I read this comment in the help for
> > 'whichwrap':
> >
> > When 'l' is included and it is used after an operator at the end of a
> > line then it will not move to the next line. This makes "dl", "cl",
> > "yl" etc. work normally.
> >
> > This would apply since "x" is equivalent to "dl". However, if you try
> > "dl" on an empty line, it does delete the line break. It's like the
> > cursor is on top of the line break, so that gets deleted.
>
> So how about adjusting the code to do what is actually in the
> documentation, so even when the cursor is on the linebreak, do not move
> over to the next line. This should also fix the problem that lead to
> this patch initially. I think this patch does it:

This breaks that using "x" or "dl" on an empty line deletes the line
break. All these solutions also don't work when using a count.

I think it should be fixed where the fold is included. I'll make a
patch, please check for any remaining problems.

--
GALAHAD: No. Look, I can tackle this lot single-handed!
GIRLS: Yes, yes, let him Tackle us single-handed!
"Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD