Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Add ncurses_cursor ui_options. #1393

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@ekie
Copy link
Contributor

ekie commented May 23, 2017

Let the ncurses_ui interface handle the cursor style.

Add ncurses_cursor ui_options.
Let the ncurses_ui interface handle the cursor style.
@lenormf

This comment has been minimized.

Copy link
Contributor

lenormf commented May 23, 2017

This doesn't really make sense, the "cursor" is always a one character wide selection, and whatever the terminal decides to render the hardware cursor as shouldn't be taken into account by the editor. We should let the terminal handle this.

@ekie

This comment has been minimized.

Copy link
Contributor Author

ekie commented May 23, 2017

well the terminal will still handle the hardware cursor this just offers a way to set the visual representation which seems to be common for console based ui. moreover this assures consistent user experience with a default setting. there was some confusion when 91ed57c broke the colorschemes and failed to highlight the cursor.

@blastrock

This comment has been minimized.

Copy link
Contributor

blastrock commented Jul 28, 2017

+1 on this

Even if there is always a one-char wide selection, when we press 'a', we clearly see the selection before the cursor and a cursor which is one-char wide after that selection. The cursor in this case is not part of the selection.

I think it makes a lot of sense to change the cursor shape to reflect that you are in insert mode. It is far easier to see than the "insert" that appears in the status bar. Also, as a user coming from neovim with NVIM_TUI_ENABLE_CURSOR_SHAPE, I find that it is a bit of a regression.

@blastrock

This comment has been minimized.

Copy link
Contributor

blastrock commented Aug 27, 2017

I actually tried to make this pull request work. The problem is that the cursor is rendered by the Window class, it just highlights the cursor block with a different color. The NCursesUI class disables the terminal cursor rendering in its constructor.

Cursors must be rendered by the Window class because when there are multiple ones, the terminal will not be able to handle them. So I made a compromise and made the Window class render all cursors except the main one and let it be rendered by NCursesUI.

I am not particularly happy with the code, so I am leaving what I have done here in case anyone has a better idea: https://github.com/blastrock/kakoune/commit/ae1b92469148c37c57846c307dd4dd7a9bb47052

@mawww

This comment has been minimized.

Copy link
Owner

mawww commented Jun 19, 2018

I dont plan to merge this, we have experimented with visible ncurses cursor some time ago, and it broke many terminals. Kakoune will keep full control of cursor display for the time being.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.