fix(column)!: ensure 'statuscolumn' works with virtual and wrapped lines

[luukvbaal]

fix(column)!: ensure 'statuscolumn' works with virtual and wrapped lines

Problem: The 'statuscolumn' was not re-evaluated for wrapped lines, when preceded by virtual/filler lines. There was also no way to distinguish virtual and wrapped lines in the status column.

Solution: Make sure to rebuild the statuscolumn, and replace variable v:wrap with v:virtnum. v:virtnum is negative when drawing virtual lines, zero when drawing the actual buffer line, and positive when drawing the wrapped part of a buffer line.

Also resolve a memory leak when returning from win_line() by breaking instead of returning.

fix(column): avoid drawing columns for virt_lines_leftcol

Problem: The default fold column, as well as the 'statuscolumn', were drawn unnecessarily/unexpectedly for virtual lines placed with virt_lines_leftcol set.

Solution: Skip the column states if a virtual line with virt_lines_leftcol set will be drawn.

[luukvbaal]

I guess this actually revealed an issue, only tested with diff-mode filler lines before. Now we also rebuild the statuscolumn for the first wrapped line after filler lines.

[lewis6991]

It's hard to tell from the test, but statuscolumn should not be rendered at all for virtual lines.

[luukvbaal]

It's hard to tell from the test, but statuscolumn should not be rendered at all for virtual lines.

Hmm why? Isn't the point of statuscolumn to let the user decide how their "gutter" will look? Seems arbitrary to exclude virtual lines from this, e.g. with a column border:

I would be more in favor of introducing a v:virtline (or something) variable if it needs to be distinguishable from v:wrap.

[lewis6991]

Hmm ok maybe, but we need to make sure virt_lines_leftcol is properly handled.

[luukvbaal]

Oof, seems to work actually. However it is drawn over the statuscolumn, is that okay?

Or should statuscolumn not be drawn for virt_lines_leftcol lines?

[lewis6991]

Oof, seems to work actually. However it is drawn over the statuscolumn, is that okay?

Not really.

Or should statuscolumn not be drawn for virt_lines_leftcol lines?

Yes

[luukvbaal]

Oof, seems to work actually. However it is drawn over the statuscolumn, is that okay?

Not really.

Or should statuscolumn not be drawn for virt_lines_leftcol lines?

Yes

The same is true for the default fold column BTW, should that be considered a bug as well then? This is some virt_lines_leftcol lines without 'statuscolumn' set:

[luukvbaal]

Should virtual lines have l:wrap set, even though they precede the real line rather than being its continuation? It produces correct behavior in terms of %l/%s not displaying anything on those lines by default, but it seems like there should be a separate v: value to indicate it so users can detect the difference.

I would be more in favor of introducing a v:virtline (or something) variable if it needs to be distinguishable from v:wrap.

Proposal

Rename v:wrap to v:virtline(?) and set it's value to:

• -1 for filler_lines above the actual buffer line.
• 0 for the actual buffer line
• 1 for the wrapped part of lines

User can check that v:virtline == / < / > 0. Seems better than adding a new variable.

Alternatively we could set it to row - startrow - filler_lines(v:virtnum) to have it indicate the distance to the actual buffer line. Not sure that that would be useful but I guess it wouldn't hurt.

[luukvbaal]

Do we need newst.txt updates for breaking changes on unreleased features?

[zeertzjq]

This change is probably to small to mention as these variables aren't mentioned either.

[luukvbaal]

e627053 fixes this issue #21768 (comment). It might be better discussed in a separate PR but it depends on this so I added it here.

[lewis6991]

LGTM

[phgz]

Alternatively we could set it to row - startrow - filler_lines(v:virtnum) to have it indicate the distance to the actual buffer line. Not sure that that would be useful but I guess it wouldn't hurt.

This could be nice to have actually. I am trying to do a different action depending on the relative wrapped line position to the actual buffer line. Also, it would not be breaking change.

[luukvbaal]

I would be okay with this. It would mean 'statuscolumn' is re-evaluated for each line in the window, currently it is only re-evaluated once for filler lines above the buffer line, once for the buffer line itself, and once for the wrapped lines below. So there would be a performance hit as well, but not compared to buffers where 'wrap' is disabled anyways. So not something we have to worry about I think.