martanne / vis

A vi-like editor based on Plan 9's structural regular expressions
Other
4.19k stars 260 forks source link

`win:style` styles more terminal cells than expected #1147

Closed fischerling closed 8 months ago

fischerling commented 8 months ago

We noticed in https://github.com/fischerling/vis-lspc/pull/34 that the highlighting seems off around \t bytes.

This is reproducible by including something like

vis.events.subscribe(vis.events.WIN_HIGHLIGHT, function(win)
    win:style_define(43, 'back:red')
    win:style(43, 2, 2)
end)

in your visrc.lua file and opening a file containing only \t\tx. Instead of highlighting the cell containing x (the third byte in the file) the space after the first \t symbol up to and including the x is styled.

Example Screenshot. 20231012_19h16m02s_grim

fischerling commented 8 months ago

In view.c the position to sytle is found by iterating all cells on the proper line until we reach the position we want to style with the following while loop:

while (pos < start && col < width)
    pos += line->cells[col++].len;

In our example using a file containing \t\tx and after executing the loop pos will be 2 and col will be 5. However, there are more empty cells (cells with len == 0) following col 5 up until col 9, which we actually want to style.

The bug is fixed by skipping all "empty" cells after we found our starting position.

Briefly tested patch:

diff --git a/view.c b/view.c
index 5fa512b..a60466a 100644
--- a/view.c
+++ b/view.c
@@ -1429,6 +1429,10 @@ void view_style(View *view, enum UiStyle style_id, size_t start, size_t end) {
    while (pos < start && col < width)
        pos += line->cells[col++].len;

+   /* skip empty columns */
+   while (!line->cells[col].len && col < width)
+       col++;
+
    do {
        while (pos <= end && col < width) {
            pos += line->cells[col].len;
rnpnr commented 8 months ago

Hi, this patch seems correct. I have it applied locally and will likely apply it to master soon.

fischerling commented 8 months ago

Hi, this patch seems correct. I have it applied locally and will likely apply it to master soon.

What is the most convenient for you? Should I send it to the mailing list or open a PR here on Github?

rnpnr commented 8 months ago

What is the most convenient for you?

Either or is fine. Maybe more people will look at it if its a Github PR but its hard to say.