Multi-line replace ranges end up cutting off visible lines from the buffer

[wz1000]

Steps

Create a multi-line replace-range. I used the following command:

define-command fold %{
  declare-option range-specs hidden
  execute-keys <a-i>i
  set-option window hidden %val{timestamp} "%val{selection_desc}|...\n"
  add-highlighter window/ replace-ranges hidden
}

See https://asciinema.org/a/LToZQAJwLsPiFxAA68Qa439HE to see this in action

Outcome

The bottom n lines visible in the window are blank, where n is the number of lines currently on the screen
hidden by the replace ranges.

Expected

There should be no blank lines.

[sucrecacao]

Any update on that issue?

I think it is the only one block the development of folding inside kakoune.

Folding would unlock a lot of great feature!

[syntonym]

Hey all,

I don't have any experience with the kakoune codebase (or c++ for that matter), but wanted to look into this bug. My hypothesis was that kakoune buffers exactly M lines, where M is the number of lines needed to fill the screen with lines. The content of the screen is calculated by Window::update_display_buffer, which updates the DisplayBuffer which contains the content being written to the window. The DisplayBuffer takes its content from the Buffer, which represents one kakoune buffer. The number of lines to buffer into the DisplayBuffer is given by DisplaySetup, which is calculated by the function Window::compute_display_setup. The DisplaySetup struct contains five values: window_pos which is the window position (start) relative to the Buffer, window_range which is the number of lines and columns to buffer, the cursor_pos which is the position of the cursor (I think relative to the Buffer, not to the DisplayBuffer), the scroll_offset which is how many lines and columns should be visible around the cursor, and finally the bool full_lines which I don't really know what it does.

Highlighters can modify the DisplaySetup (here). Initially I thought that the ReplaceRangesHighlighter does not account for lines it removes, but it does. After the ReplaceRangesHighlighter accounts for the number of lines it removes, the WrapHighlighter (which wraps text-lines that are too long to display in one output-line into multiple output-lines) recalculates the number of lines in the buffer window_range in such a way that it does not take removed lines into account (here). If I understand that code correctly, it calculates the number of lines needed from the Buffer by starting at 0 lines, taking one line from the buffer, potentially splitting it into multiple output-lines and stopping when the window length is exhausted. At this point only the DisplaySetup is calculated (positions and ranges), so no modification has occured and how many lines the ReplaceRangesHighlighter would remove is not available.

This means turning of the WrapHighlighter is a workaround for this specific bug.

One way to potentially fix this bug is to turn around in which order the highlighters modify the DisplaySetup. This would mean that first the effect of Wrapping is taken into account and then the effect of Ranges Replaced, by modyfing window.cc line 243 in the following way:

for (auto pass : { HighlightPass::Move, HighlightPass::Wrap })

to

for (auto pass : { HighlightPass::Wrap, HighlightPass::Move })

this is also the order in which the actuall highlighting is executed in line 166:

for (auto pass : { HighlightPass::Wrap, HighlightPass::Move, HighlightPass::Colorize })

Potentially there are other incompatibilities between the ReplaceRangesHighlighter and the WrapHighlighter, so this might not be enough. Also I'm not sure if there is a reason for first having the Move HighlightPass and then the Wrap highlight pass. I could imagine thatother highlighter expect to modify the DisplaySetup after the WrapHighlighter modified it. I have tested this approach with the default highlighters and it seemed to work.

Another solution would be to change the WrapHighlighter to be less invasive about the number of buffered lines setup.window_range. Instead of starting at 0 and counting each line until the window is full, it could remove one count from setup.window_range.line for each additional output-line it produces.

Best,
syntonym

[Screwtapello]

Thanks for doing this investigation!

This means turning off the WrapHighlighter is a workaround for this specific bug.

This helps, but does not completely work around the bug.

I have a test plugin I wrote that replaces the saved selection (^ register) with ---- so I can easily test folding by selecting different patterns of text with Z. With WrapHighlighter disabled, I can select any number of lines of text, plus one more partial line (up to but not including the final newline character) and replacement works nicely - no "undrawn" lines appear at the bottom of the screen. However, if the last line of the selection is complete (i.e. including the final newline character) then I get one "undrawn" line at the bottom of the screen.

I haven't tried messing with the order of highlighters.

[syntonym]

Changing the order of Move and Wrap Highlighters messes up the column calculation and leads to weird visual artifacts when the wrap highlighter is activated. But the wrap highlighter can be changed to take the length of the buffer needed from the DisplaySetup instead of directly from the screen, by changing line highlighters.cc:L804: from

const auto win_height = context.context.window().dimensions().line;

to

const auto win_height = setup.window_range.line;

This still has problems when the removed range includes long lines which are line wrapped. I don't think there is a way to implement ReplaceRangesHighlighter and WrapHighlighter independently from each other: The WrapHighlighter needs to know which lines are removed. Removed Lines are then ignored when calculating how many lines are added due to wrapping. The ReplaceRangesHighlighter needs to know how many "visual-lines" one buffer-line is representing to properly calculate how many more lines are needed.

However, if the last line of the selection is complete (i.e. including the final newline character) then I get one "undrawn" line at the bottom of the screen.

Because the newline is part of the previous line it needs to be accounted additionally. One can check whether the last character in the replaced range is a newline and increase the number of removed_lines accordingly.

Can the replacement of the ReplaceRangesHighlighter be multiline? I don't think that's currently accounted for. Additionally the WrapHighlighter cannot calculate how many lines are added due to wrapping if it doesn't know the replacement. So I don't think calculating how many more/less lines are needed due to replacing/wrapping can be calculated independently of the replacement at all.

[nonumeros]

indentation is the key . as soon as the selection is not indented and using the above example from the op, that selection jumps to the beginning of line (file, whatever in use)

e.g, with number-lines marker set, if no indentation is selected it jumps to the beginning of line so it would go from the current line (say line n to line 1 of the file)

any piece of code or text or whatever as long as itss' indented gets folding , or else doesn't.

That piece of code must be indented