Skip to content

Conversation

yan12125
Copy link

Fixes #7932

See the "Analysis & Possible solution" in the relevant issue for why it's broken.

@jamessan
Copy link
Member

Cc @leonerd

@marvim marvim added the RFC label Jan 31, 2018
@leonerd
Copy link
Contributor

leonerd commented Jan 31, 2018

OK.. is there a specific question for me?

@jamessan
Copy link
Member

Just wanted to see if this looked right to you or if there was something else we should be considering.

@leonerd
Copy link
Contributor

leonerd commented Jan 31, 2018

It's all a bit of a mess, but it's going to be a bit of a mess anyway. A proper solution would be libtickit, really because that asks for mode changes and then knows if they've happened. Anything that happens to work here is probably fine, taking the view that it's a temporary measure at this point anyway.

@justinmk
Copy link
Member

justinmk commented Jan 31, 2018

In tui.c we always enable SGR mouse-mode.

But what is the effect of returning NULL from tui_tk_ti_getstr()?

  • In libtermkey's driver-ti.c:load_terminfo() the terminfo value is skipped if we force it to NULL.
  • I assume that causes driver-ti.c:peekkey() to return TERMKEY_RES_NONE
  • Later termkey.c:peekkey() iterates through all drivers, it finds TERMKEY_RES_NONE for the ti driver and proceeds to handle the mouse input as SGR via the CSI driver.

// terminfo driver (driver-ti.c), which is wrong. Don't use it if it's SGR
// encoding. XXX how about other encodings (UTF-8, URXVT...)?
if (value != NULL && strequal("\033[<", value)) {
return NULL;
Copy link
Member

@justinmk justinmk Jan 31, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we return NULL always? Why do we care what value is?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, forgot to mention another tricky thing in libtermkey: the CSI driver can handle SGR and URXVR protocols but not the X10 protocol.

https://github.com/neovim/libtermkey/blob/master/driver-csi.c#L162

neovim already forces the SGR protocol (1006), so it's safe to skip the value check. I put it here so that neovim won't break in the future if it wants a different mouse encoding protocol.

(According to the author of ncurses, some terminals do not implement 1006 protocol correctly [1])

[1] https://raw.githubusercontent.com/ThomasDickey/ncurses-snapshots/master/misc/terminfo.src

justinmk pushed a commit that referenced this pull request Jan 31, 2018
Fixes #7932

Nvim (tui.c) always enables SGR mouse (TUIData.unibi_ext.enable_mouse).
But if libtermkey sees key_mouse (kmous) in terminfo its terminfo driver
(driver-ti.c) will be activated, which by accident only supports X10
protocol. The libtermkey CSI driver (driver-csi.c), in contrast,
supports SGR.

We can force libtermkey to ignore the terminfo key_mouse entry by
returning NULL in the tui_tk_ti_getstr hook. That forces the CSI driver.

What is the effect of returning NULL from `tui_tk_ti_getstr()`?
- libtermkey `driver-ti.c:load_terminfo()` skips the entry.
- `termkey.c:peekkey()` iterates through all drivers, it finds
  `TERMKEY_RES_NONE` for the ti driver and falls back to the CSI driver.
@justinmk
Copy link
Member

Merged. Thanks for looking into this!

I attempted a different explanation in 3a5721e , and removed the value check.

@justinmk justinmk closed this Jan 31, 2018
@justinmk justinmk removed the RFC label Jan 31, 2018
@yan12125 yan12125 deleted the libtermkey-terminfo-no-sgr branch February 1, 2018 03:33
justinmk added a commit that referenced this pull request Jun 11, 2018
FEATURES:
3cc7ebf #7234 built-in VimL expression parser
6a7c904 #4419 implement <Cmd> key to invoke command in any mode
b836328 #7679 'startup: treat stdin as text instead of commands'
58b210e :digraphs : highlight with hl-SpecialKey #2690
7a13611 #8276 'startup: Let `-s -` read from stdin'
1e71978 events: VimSuspend, VimResume #8280
1e7d5e8 #6272 'stdpath()'
f96d99a #8247 server: introduce --listen
e8c39f7 #8226 insert-mode: interpret unmapped META as ESC
98e7112 msg: do not scroll entire screen (#8088)
f72630b #8055 let negative 'writedelay' show all redraws
5d2dd2e win: has("wsl") on Windows Subsystem for Linux #7330
a4f6cec cmdline: CmdlineEnter and CmdlineLeave autocommands (#7422)
207b7ca #6844 channels: support buffered output and bytes sockets/stdio

API:
f85cbea #7917 API: buffer updates
418abfc #6743 API: list information about all channels/jobs.
36b2e3f #8375 API: nvim_get_commands
273d2cd #8329 API: Make nvim_set_option() update `:verbose set …`
8d40b36 #8371 API: more reliable/descriptive VimL errors
ebb1acb #8353 API: nvim_call_dict_function
9f994bb #8004 API: nvim_list_uis
3405704 #7520 API/UI: forward option updates to UIs
911b1e4 #7821 API: improve nvim_command_output

WINDOWS OS:
9cefd83 #8084, #8516 build/win: support MSVC
ee4e1fd win: Fix reading content from stdin (#8267)

TUI:
ffb8904 #8309 TUI: add support for mouse release events in urxvt
8d5a46e #8081 TUI: implement "standout" attribute
6071637 TUI: support TERM=konsole-256color
67848c0 #7653 TUI: report TUI info with -V3 ('verbose' >= 3)
3d0ee17 TUI/rxvt: enable focus-reporting
d109f56 #7640 TUI: 'term' option: reflect effective terminal behavior

FIXES:
ed6a113 #8273 'job-control: avoid kill-timer race'
4e02f1a #8107 'jobs: separate process-group'
451c48a terminal: flush vterm output buffer on pty output #8486
5d6732f :checkhealth fixes #8335
53f11dc #8218 'Fix errors reported by PVS'
d05712f inccommand: pause :terminal redraws (#8307)
51af911 inccommand: do not execute trailing commands #8256
84359a4 terminal: resize to the max dimensions (#8249)
d49c1dd #8228 Make vim_fgets() return the same values as in Vim
60e96a4 screen: winhl=Normal:Background should not override syntax (#8093)
0c59ac1 #5908 'shada: Also save numbered marks'
ba87a2c cscope: ignore EINTR while reading the prompt (#8079)
b1412dc #7971 ':terminal Enter/Leave should not increment jumplist'
3a5721e TUI: libtermkey: force CSI driver for mouse input #7948
6ff13d7 #7720 TUI: faster startup
1c6e956 #7862 TUI: fix resize-related segfaults
a58c909 #7676 TUI: always hide cursor when flushing, never flush buffers during unibilium output
303e1df #7624 TUI: disable BCE almost always
249bdb0 #7761 mark: Make sure that jumplist item will not have zero lnum
6f41ce0 #7704 macOS: Set $LANG based on the system locale
a043899 #7633 'Retry fgets on EINTR'

CHANGES:
ad60927 #8304 default to 'nofsync'
f3f1970 #8035 defaults: 'fillchars'
a6052c7 #7984 defaults: sidescroll=1
b69fa86 #7888 defaults: enable cscopeverbose
7c4bb23 defaults: do :filetype stuff unless explicitly "off"
2aa308c #5658 'Apply :lmap in macros'
8ce6393 terminal: Leave 'relativenumber' alone (#8360)
e46534b #4486 refactor: Remove maxmem, maxmemtot options
131aad9 win: defaults: 'shellcmdflag', 'shellxquote' #7343
c57d315 #8031 jobwait(): return -2 on interrupt also with timeout
6452831 clipboard: macOS: fallback to tmux if pbcopy is broken #7940
300d365 #7919 Make 'langnoremap' apply directly after a map
ada1956 #7880 'lua/executor: Remove lightuserdata'

INTERNAL:
de0a954 #7806 internal statistics for list impl
dee78a4 #7708 rewrite internal list impl
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants