Use terminal cursor, redux

[tassmjau]

Make vis use the terminal/tty's cursor instead of hiding it and drawing it using cell attributes.

This PR is based on the idea in #953, since that PR has been inactive for some quite time, and I want this functionality, I opened this one. Credit goes to @gotroyb127 for the original proposal.

Some underlying constructs that patch either relied on or had to work around has changed, which allows this patch to be simpler in some places.
For example: we no longer need to accumulate dx and dy offsets across windows to figure out which coordinates are the selected window, since they're now available on vis->win.

My initial attempt at a rebase was packed with conflicts to the point where it was simpler to take what was needed line by line and implement the rest.

There were some discussion in #953 about having optional styling for the primary cursor. In this proposal I got rid of STYLE_CURSOR_PRIMARY all together, since that's how I prefer it. If that's a deal breaker I can dig into it.

There were also an outstanding issue about the cursor not working as expected for auto-complete which is addressed here.

[micampe]

if you're interested in this feature I have it as a plugin instead of a patch: modecursor.lua

by default it doesn't set the shape of the normal mode cursor but you can do it in visrc like this:

local cursor = require('modecursor')
cursor.shapes[vis.modes.NORMAL] = 'block'

it uses a bit of a hack to hide vis' cursor but it works ok.
note that I only tested this with vis HEAD on my machine.

@tassmjau I don't mean to hijack your pr, this code is obviously a better solution, my hack is just a workaround while this is being worked on.

[tassmjau]

Thanks @micampe I had a look at your plugin, it looks nice!

My main motivation for this patch is getting the cursor position to behave nice when switching between vis and tmux's copy-mode. Ie I want the tmux cursor to start off at the line/column where the cursor is in vis, and not at the bottom where the hidden cursor resides.

Maybe the more pragmatic way of achieving that is to sync the position of the "real cursor" (even though it's hidden), so that when switching from the cell-drawn cursor out to tmux's copy-mode it works as I would expect(?). Personally I prefer this, but if such a patch is more likely to get merged it could likely be done.

[rnpnr]

I am going to run this for a while before I give any review. I just remember I kept running into issues with the original version and I don't want to miss anything that I have forgotten about since then.

[MaxGyver83]

For me, the only reason to use #953 was to have a bar cursor in insert mode.

@micampe : Thanks for your plugin. It works well on master but on this branch it gives me this error when I enter insert mode:

/tmp/vis/modecursor.lua:77: bad argument #1 to 'style_define' (number expected, got nil)

[micampe]

@micampe : Thanks for your plugin. It works well on master but on this branch it gives me this error when I enter insert mode:

this patch removes STYLE_CURSOR_PRIMARY. Delete the whole if vis.API block and it should work.

[rnpnr]

Having tested this for a while I did not find any issues with the core functionality. However it still has the annoying behaviour of breaking styling when the cursor is on top of a \t character and it makes it difficult to distinguish which parentheses of a pair the cursor is currently on.

Because of the parentheses issue I have been repeatedly making the mistake of inserting text in the wrong location since applying this patch.

The tab issue is quite important even if only for editing vis' source code. Personally the only reason my tabwidth is set to 2 instead of 1 is so that I can see that I inserted a tab and not a space allowing others to view the text with whatever spacing they prefer. With this patch I can't see the highlighting anymore.

I'm pretty sure both of these issues can be fixed with very minor changes to the code but that would need to be done before I would consider merging this.

[tassmjau]

Thanks for trying it out @rnpnr,
a couple of questions just to make sure I understand the issues you're seeing...

By breaking the styling when being on a \t character, do you mean that the cursor no longer stretches out across the full display width of the tab?
If breaking that behavior is unacceptable, I'm not sure we can use the native cursor of the terminal, since the latter is one character wide regardless of tab width.
If you have any pointers with regards to that I'd be happy to take a look though.

The matching cursor styling has not been an issue for me. Could it be a theme issue?
Below is how it looks for me with:

lexers.STYLE_CURSOR_MATCHING = 'fore:yellow,reverse,bold'

vis_cursor_1.mp4

[rnpnr]

1. [...]

Yes, and I believe it should be fixable by storing the correct style in the extra cells in view_expand_tab(). If the drawing code does not render the style because the cell is empty then that should be fixed. For the record if you have multiple cursors and all of them start on a \t only the primary one is missing the proper highlighting.

2. [...]

Right, I forgot about STYLE_CURSOR_MATCHING. It's a little confusing because previously the matching brace seemed to use STYLE_SELECTION but I think that separating out STYLE_CURSOR_MATCHING is the best course of action.

[rnpnr]

I believe it should be fixable by storing the correct style in the extra cells in view_expand_tab().

Sorry this may be a little misleading. The selection is styled after view_expand_tab(). You may need to step through the window_draw_selections() code to see how it determines to draw the tab highlighting. I'm not certain on a quick glance.

[tassmjau]

I found where I broke it, and reverted that part:

@@ -276,8 +276,7 @@ static void window_draw_selections(Win *win) {
                        break;
                window_draw_cursor(win, s);
        }
-       if (win->vis->mode->visual)
-               window_draw_selection(win, sel);
+       window_draw_selection(win, sel);
        window_draw_cursor(win, sel);
        for (Selection *s = view_selections_next(sel); s; s = view_selections_n
                window_draw_selection(win, s);

Like you say, filling the tab is a side-effect of window_draw_selection, I completely missed that in my first patch. Before you gave me that last pointer I had something else going, which would have kept window_draw_selection to be used only in visual mode:

@@ -263,7 +263,14 @@ static void window_draw_cursor(Win *win, Selection *cur) {
        if (cur != primary)
                vis_ui_window_style_set(&win->vis->ui, line->cells + cur->col, UI_STYLE_CURSOR);
        window_draw_cursor_matching(win, cur);
-       return;
+       char byte;
+       if (!text_byte_get(win->file->text, cur->pos, &byte) || byte != '\t')
+               return;
+       for (int col = cur->col; col < line->width; col++) {
+               if (col > cur->col && line->cells[col].file_byte_count)
+                       break;
+               vis_ui_window_style_set(&win->vis->ui, line->cells + col, UI_STYLE_SELECTION);
+       }
 }

It would reduce the cycles spent on drawing the cursor in normal mode, but not changing this behavior in the first place is probably better :)

[tassmjau]

I was trying this with a "bar" style cursor and that makes it apparent that the "selection" block is always rendered behind the cursor:

vis_draw_non_primary_selection.mp4

The second approach from my previous message doesn't produce this artifact, since it only renders that block when being on top of a \t:

vis_dont_draw_non_primary_selection.mp4

I don't have any strong feelings about it since I normally use a block cursor anyway, but thought it was a difference worth mentioning.

[micampe]

@tassmjau personally I use a bar cursor in insert mode and block otherwise (and I think that's the reasonable option); in normal mode the selection should be visible, like in your first video.

[tassmjau]

@micampe Unfortunately, that approach brought some issuses to terminals that doesn't draw the cursor as something other than the reverse of what's currently beneath it. Like xterm does. If you haven't explicitly set a cursor style, the cursor more or less becomes invisible. See 8c6ca22