[vim/vim] Indent: bogus characters inserted in visual-block append

[Vitor Antunes]

Assume that you have two lines of code:

```
var_a = something()
b = something()
```

If you use `c-vj$A;` to append a `;` character at the end of each line and if `;` is part of `'indentkeys'` and also if `'indentexpr'` function only indents the first line, then the bottom line will include the last N characters of the original first line after pressing `<esc>`.

Example `'indentexpr'` for the above code:

```Vim
function! MyIndentFunction()
if v:lnum == 1
return &sw
else
return 0
endif
endfunction
```

Note: This was originally posted in Github under https://github.com/vim/vim/issues/1269


Thanks in advance,
Vitor

[Christian Brabandt]

Hi Vitor!

On Di, 06 Dez 2016, Vitor Antunes wrote:

> Note: This was originally posted in Github under
> https://github.com/vim/vim/issues/1269

Yes we have seen this problem. Please do not double poste.

BTW: I cannot reproduce the problem.


Best,
Christian
--
"Wie haben andere Linux Benutzer ihr `erstes Mal' mit Linux erlebt??"
"Wir haben danach gemeinsam eine Gitanes geraucht und nochmal über
alles geredet."
-- P.Vollmann und Stefanie Teufel in dcolm

[Vitor Antunes]

On Tuesday, 6 December 2016 14:21:18 UTC, Christian Brabandt wrote:
> Hi Vitor!
>
> On Di, 06 Dez 2016, Vitor Antunes wrote:
>
> > Note: This was originally posted in Github under
> > https://github.com/vim/vim/issues/1269
>
> Yes we have seen this problem. Please do not double poste.
>
> BTW: I cannot reproduce the problem.

Hi Christian,

I apologize for double posting.
May I suggest that you add a label after reviewing (or looking at) issues in Github? I received no feedback and for that reason assumed developers didn't give too much attention to Github. My bad... :)

Do you want to discuss this issue further in the mailing list, or on Github?

Thanks,
Vitor

[Christian Brabandt]

Hi Vitor!

On Di, 06 Dez 2016, Vitor Antunes wrote:

> I apologize for double posting.
> May I suggest that you add a label after reviewing (or looking at) issues in Github? I received no feedback and for that reason assumed developers didn't give too much attention to Github. My bad... :)
>
> Do you want to discuss this issue further in the mailing list, or on Github?

Since you are already here, I prefer discussion here.


Best,
Christian
--
Wenn ich gestern abend so müde gewesen wäre, wie ich heute morgen bin,
dann wäre ich heute morgen so munter, wie ich gestern abend war.
-- Hans Leitinger

[Vitor Antunes]

On Tuesday, 6 December 2016 16:08:21 UTC, Christian Brabandt wrote:
> Since you are already here, I prefer discussion here.

Since you weren't able to replicate, I did some more testing. It appears that this only happens when 'expandtab' is set.

Could you please try on your side?

Thanks,
Vitor

[Vitor Antunes]

BTW, when this option is not enable the problem does not appear, but the ';' is not appended to the second line as I would expect it to.

[Christian Brabandt]

Hi Vitor!

Thanks, now I can confirm.


Best,
Christian
--
Man fragt den andern um Rat, nicht, weil man nicht weiß, was man tun
soll, sondern weil man es weiß, aber nicht gern tut - der andere soll
dann einer guten oder bösen Neigung den Ausschlag [geben].
-- Jean Paul

[Vitor Antunes]

Hi Christian,

On Tuesday, 6 December 2016 16:20:06 UTC, Christian Brabandt wrote:
> Thanks, now I can confirm.
>
> Best,
> Christian

Although without the purpose of creating pressure, but doing so anyway...
Any updates in this regard?

Thanks!
Vitor

[Christian Brabandt]

Hi Vitor!

On Mi, 22 Feb 2017, Vitor Antunes wrote:

> On Tuesday, 6 December 2016 16:20:06 UTC, Christian Brabandt wrote:
> > Thanks, now I can confirm.
> >
> > Best,
> > Christian
>
> Although without the purpose of creating pressure, but doing so anyway...
> Any updates in this regard?

;)

Would you mind, posting a complete minimal example? I don't get it
reproduced again... (or was this fixed?)

Please include all relevant settings like tabstop, shiftwidth,
expandtab, etc...

However, I cannot promise anything. I remember having looked into it,
but the problem was tricky.

Best,
Christian
--

[Tony Mechelynck]

On Wed, Feb 22, 2017 at 1:19 PM, Christian Brabandt <cbl...@256bit.org> wrote:
> Hi Vitor!
>
> On Mi, 22 Feb 2017, Vitor Antunes wrote:
>
>> On Tuesday, 6 December 2016 16:20:06 UTC, Christian Brabandt wrote:
>> > Thanks, now I can confirm.
>> >
>> > Best,
>> > Christian
>>
>> Although without the purpose of creating pressure, but doing so anyway...
>> Any updates in this regard?
>
> ;)
>
> Would you mind, posting a complete minimal example? I don't get it
> reproduced again... (or was this fixed?)
>
> Please include all relevant settings like tabstop, shiftwidth,
> expandtab, etc...

Also, the 'paste' setting might be significant:
:set paste?
And for a boolean option like this one, the question mark is important
(":set paste" with no question mark sets the option to TRUE).

>
> However, I cannot promise anything. I remember having looked into it,
> but the problem was tricky.
>
> Best,
> Christian

Best regards,
Tony.

[Vitor Antunes]

Hi Christian,

I've attached a small test case. Please run it as following: ./run.sh
I was able to replicate this issue using the latest master.

Best regards,
Vitor

[Vitor Antunes]

Hi Tony,

On Wednesday, 22 February 2017 12:36:17 UTC, Tony Mechelynck wrote:
> Also, the 'paste' setting might be significant:
> :set paste?
> And for a boolean option like this one, the question mark is important
> (":set paste" with no question mark sets the option to TRUE).

If you look at the test case I attached yesterday, you will see that 'paste' is not set.

Thanks!
Vitor

[Christian Brabandt]

Would you rather not have the first line indented or abort if the indent
changes the line?

Best,
Christian
--
Alles fließt. (Panta rhei.)
-- Heraklit von Ephesus (540-480 v. Chr.)

[Vitor Antunes]

Hi Christian,

On Thursday, 23 February 2017 21:38:39 UTC, Christian Brabandt wrote:
> Would you rather not have the first line indented or abort if the indent
> changes the line?

Well, I would rather have the result in the expected.txt file, but taking
your question into account I can assume that's not so easy.

Typically, when doing column insertion every action appears "suspended"
until insertion mode is exited. Would it be possible to queue the call
to 'indentexpr' until that time? And maybe then apply it to all
previously selected lines?

Aborting is too much. For example, when doing column editing and text goes
over 'textwidth' a newline is inserted and what was being edited is simply
lost. This is not ideal behavior, but it doesn't abort. In my opinion,
coherency should be kept in this case.
So, if nothing else is possible I would say that it is best not to indent
at all.

Cheers,
Vitor

[Christian Brabandt]

Yeah, it is not that easy. However I might have found a way (but see
below.

The way block inserting works, is like Vim is figuring out, what text
has been appended or prepended to a line and applies that change to the
reset of the line. That does not mean it will reevaluate the indent
expressions or c indenting or expand abbreviations. It simply copies,
what the **result** of your editing of the first line was and adds it
to the rest of the block.

The bug you have found looks therefore to Vim like you have actually
entered "something();" to the end of the first line since Vim did not
consider that the line was shifted because of the indenting function.

That means, we cannot make Vim work like you prefer and have all indents
reapplied as the text is changed per block line.

However here is a first play patch, that at least makes Vim aware that
indenting could have taken place and tries to adjust to it. Please check
it out. It seems to work for my test cases so far.

If it works okay, I'll add some test cases.

Best,
Christian
--
Demnächst sollen auch die ersten Landesmeisterschaften im
Beamtendreikampf ausgerichtet werden: Knicken, Lochen, Abheften.

[Vitor Antunes]

Hi Christian,

On Friday, 24 February 2017 14:24:13 UTC, Christian Brabandt wrote:
> That means, we cannot make Vim work like you prefer and have all indents
> reapplied as the text is changed per block line.

That's a pitty, as it would be the best approach.
On the other hand, I don't know if it wouldn't make sense to disable indent
and wrap altogether when appending in a visual block. It appears that only
"strange" things happen when any of those features is activated.

> However here is a first play patch, that at least makes Vim aware that
> indenting could have taken place and tries to adjust to it. Please check
> it out. It seems to work for my test cases so far.
>
> If it works okay, I'll add some test cases.

The end result is much better now. But I did some extra testing and noticed
that if the two lines start indented by 2*&sw (which means the first line will be de-indented), then the second line will not have the ; appended.

Other than that, at least from my point of view, the new behavior is more
acceptable than the original.

Thanks for your help!
Vitor

[Vitor Antunes]

Hi Christian,

Hope everything's alright with you.
And since we're talking, any updates in this regard? ;P

Kind regards,
Vitor

[Christian Brabandt]

Hi Vitor!

On Mo, 06 Mär 2017, Vitor Antunes wrote:
> > The end result is much better now. But I did some extra testing and
> > noticed
> > that if the two lines start indented by 2*&sw (which means the first line
> > will be de-indented), then the second line will not have the ; appended.
> >
> > Other than that, at least from my point of view, the new behavior is more
> > acceptable than the original.
>
> Hope everything's alright with you.
> And since we're talking, any updates in this regard? ;P

What you see is like removing characters when doing block wise editing.
This is not supported as far as i know

Best,
Christian
--
Ist der Server überlastet, wird der SysOp eingeknastet.

[Vitor Antunes]

Hi Chistian,

On Monday, 6 March 2017 20:18:33 UTC, Christian Brabandt wrote:
> Hi Vitor!
>
> On Mo, 06 Mär 2017, Vitor Antunes wrote:
> > Hope everything's alright with you.
> > And since we're talking, any updates in this regard? ;P
>
> What you see is like removing characters when doing block wise editing.
> This is not supported as far as i know

Well, at least the original issue would be fixed. When do you think
you will have your change merged?

Thanks,
Vitor

[Christian Brabandt]

On Mo, 06 Mär 2017, Vitor Antunes wrote:

> Well, at least the original issue would be fixed. When do you think
> you will have your change merged?

I am attaching an updated patch including some tests. Don't know when
Bram comes to including this.

Best,
Christian
--
Es gibt Personen, denen ich wohl will und wünsche, ihnen besser
wollen zu können.
-- Goethe, Maximen und Reflektionen, Nr. 381

[Bram Moolenaar]

Christian Brabandt wrote:

> On Mo, 06 Mär 2017, Vitor Antunes wrote:
>
> > Well, at least the original issue would be fixed. When do you think
> > you will have your change merged?
>
> I am attaching an updated patch including some tests. Don't know when
> Bram comes to including this.

Thanks for looking into this.


> /*
> + * Return the number of whitespace bytes in front of a given line
> + */
> + int
> +get_whitespace_line_start(linenr_T lnum)
> +{
> + char_u *l1, *l2;
> +
> + l1 = l2 = ml_get(lnum);
> + while (vim_iswhite(*l2))
> + ++l2;

This can use skipwhite(). There are several other places where this
computation is done, e.g. in misc1.c, open_line():

p = ml_get_curline();
ai_col = (colnr_T)(skipwhite(p) - p);

Similar one a bit further down.

Would be good to put this function close to skipwhite() to make it
easeri to find.

> + /* if indent kicked in, the firstline might have changed
> + * but only do that, if the indent actually increased */
> + ind_post = get_whitespace_line_start(curwin->w_cursor.lnum);
> + if (curbuf->b_op_start.col > ind_pre && ind_post > ind_pre)
> + {
> + bd.textcol += ind_post - ind_pre;
> + bd.start_vcol += ind_post - ind_pre;
> + }

Why only for when the indent increases?


--
From "know your smileys":
:'-D Laughing so much that they're crying

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

Hi Bram!

On Do, 09 Mär 2017, Bram Moolenaar wrote:

> This can use skipwhite(). There are several other places where this
> computation is done, e.g. in misc1.c, open_line():
>
> p = ml_get_curline();
> ai_col = (colnr_T)(skipwhite(p) - p);
>
> Similar one a bit further down.
>
> Would be good to put this function close to skipwhite() to make it
> easeri to find.

Attached patch does that.

>
> > + /* if indent kicked in, the firstline might have changed
> > + * but only do that, if the indent actually increased */
> > + ind_post = get_whitespace_line_start(curwin->w_cursor.lnum);
> > + if (curbuf->b_op_start.col > ind_pre && ind_post > ind_pre)
> > + {
> > + bd.textcol += ind_post - ind_pre;
> > + bd.start_vcol += ind_post - ind_pre;
> > + }
>
> Why only for when the indent increases?

To be on the safe side. Currently blockwise editing only allows for
actually adding text. This expectation isn't documented yet, so I
documented it.

Best,
Christian
--
Ohne Spekulation gibt es keine neue Beobachtung.
-- Charles Darwin

[Vitor Antunes]

Hi Bram and Christian,

As far as I can tell, this patch was still not merged. Is there anything I can help with in order to have this into Git HEAD?

Thanks in advance and best regards,
Vitor

[Vitor Antunes]

Hi Bram,

Is there anything still missing in Christian's patches?

Thanks,
Vitor

[Bram Moolenaar]

Vitor Antunes wrote:

> On Friday, 12 May 2017 15:37:25 UTC+1, Vitor Antunes wrote:
> > On Tuesday, 14 March 2017 10:26:15 UTC, Christian Brabandt wrote:
> > > Hi Bram!
> > >
> > > On Do, 09 M=C3=A4r 2017, Bram Moolenaar wrote:
> > >
> > > > This can use skipwhite(). There are several other places where this
> > > > computation is done, e.g. in misc1.c, open_line():
> > > >

> > > > p =3D ml_get_curline();
> > > > ai_col =3D (colnr_T)(skipwhite(p) - p);

> > > >
> > > > Similar one a bit further down.
> > > >
> > > > Would be good to put this function close to skipwhite() to make it
> > > > easeri to find.
> > >
> > > Attached patch does that.
> > >
> > > >
> > > > > + /* if indent kicked in, the firstline might have changed
> > > > > + * but only do that, if the indent actually increased */

> > > > > + ind_post =3D get_whitespace_line_start(curwin->w_cursor.lnum);

> > > > > + if (curbuf->b_op_start.col > ind_pre && ind_post > ind_pre)
> > > > > + {

> > > > > + bd.textcol +=3D ind_post - ind_pre;
> > > > > + bd.start_vcol +=3D ind_post - ind_pre;

> > > > > + }
> > > >
> > > > Why only for when the indent increases?
> > >
> > > To be on the safe side. Currently blockwise editing only allows for
> > > actually adding text. This expectation isn't documented yet, so I
> > > documented it.
> > >
> > > Best,
> > > Christian
> >
> > Hi Bram and Christian,
> >
> > As far as I can tell, this patch was still not merged. Is there
> > anything I can help with in order to have this into Git HEAD?
> >
> > Thanks in advance and best regards,
> > Vitor
>
> Is there anything still missing in Christian's patches?

I only see a few lines of code here. Is there a patch (with a test)?

--
If someone questions your market projections, simply point out that your
target market is "People who are nuts" and "People who will buy any damn
thing". Nobody is going to tell you there aren't enough of those people
to go around.
(Scott Adams - The Dilbert principle)

[Christian Brabandt]

On Di, 22 Aug 2017, Bram Moolenaar wrote:

> I only see a few lines of code here. Is there a patch (with a test)?

It is here:
https://groups.google.com/d/msg/vim_dev/uMsaEfGexoI/8sUyU_aFDAAJ

Best,
Christian
--
Man hat den Epikur, der ein armer Hund war wie ich, sehr
missverstanden, wenn er das Höchste in die Schmerzlosigkeit legte.
-- Goethe, Maximen und Reflektionen, Nr. 1290

[Vitor Antunes]

On Wednesday, 23 August 2017 05:55:52 UTC+1, Christian Brabandt wrote:
> On Di, 22 Aug 2017, Bram Moolenaar wrote:
>
> > I only see a few lines of code here. Is there a patch (with a test)?
>
> It is here:
> https://groups.google.com/d/msg/vim_dev/uMsaEfGexoI/8sUyU_aFDAAJ

Hi Bram and Christian,

Just noticed that this was included in latest vim, in patch 8.0.1041.

Thanks for your help solving this issue!

Cheers,
Vitor