feat(tui): query terminal for CSI u support

[gpanders]

On startup query the terminal for CSI u support and enable it using the escape sequence from kitty's progressive enhancement protocol.

See also #16570 for a broader implementation of the kitty keyboard protocol.

[zeertzjq]

A difference I noticed: in xterm Alt-Shift-0 is <M-S-)>, while in kitty Alt-Shift-0 is <M-S-0>. In terminal emulators without support for extended keys it is <M-)>.

[jamessan]

A difference I noticed: in xterm Alt-Shift-0 is <M-S-)>, while in kitty Alt-Shift-0 is <M-S-0>.

Does xterm behave like that even without this PR? That sounds like a bug.

[zeertzjq]

This PR should not affect the behavior in xterm, only the behavior in kitty.

[gpanders]

Bikeshedding over naming is welcome. I chose "extended keys" as the catch all term for modifyOtherKeys, CSI u, and kitty keyboard protocol (this is a term borrowed from tmux). That said, I don't think it's exactly a very standard term, so I'm open to other suggestions.

[dnkl]

A difference I noticed: in xterm Alt-Shift-0 is <M-S-)>

This is a pretty well known issue with XTerm's extended keyboard protocol - it always sends the modified (translated) symbol. The most basic example: Shift+a, sends CSI 27;2;65~, where 65 is upper case A. That is, it says you pressed shift+A, not shift+a. This is true for all combinations I have tested so far. While it's not a big issue with a vs A, it is an issue with other keys.

I.e. I for one really hope this gets merged :)

[justinmk]

is kitty protocol different than extkeys? Or can we call it extkeys generically?

If there is a delta, we will need to explain it in docs.

[gpanders]

This might be more information than you bargained for, but:

modifyOtherKeys is the name of the Xterm extension to disambiguate control sequence modifiers. This mode can be programmatically enabled via the XTMODKEYS sequence \x1b[>4;2m.

"CSI u" is the colloquial name given to Leonerd's encoding scheme, which is distinct from Xterm's modifyOtherKeys. libtermkey (which Neovim uses) supports both.

Kitty's keyboard protocol is yet another encoding scheme which is based off of Leonerd's CSI u. Kitty's protocol supports progressive enhancement so you can use different "levels" of the protocol; the base level (level 1, which this PR enables) is identical to CSI u.

However, Leonerd's CSI u proposal does not provide any way of querying the terminal for support nor programmatically enabling or disabling that mode. Both of these are supported by kitty's keyboard protocol.

Finally, tmux uses the term "extended keys" to refer to any encoding scheme that disambiguates control sequences (either modifyOtherKeys or CSI u). I've adopted that terminology to refer to the same thing.

So here "extended keys" (extkeys) means any encoding scheme that enables us to distinguish between e.g. <Tab> and <C-I>. modifyOtherKeys is the Xterm scheme and kitty/CSI u is the one proposed by Leonerd.

The terminology is kind of confusing and there isn't a super well established standard. As I said I'm open to suggestions on wording, but for now I will use the terms as outlined above.

I'll also add a section to the docs outlining all of this.

[justinmk]

However, Leonerd's CSI u proposal does not provide any way of querying the terminal for support nor programmatically enabling or disabling that mode. Both of these are supported by kitty's keyboard protocol.

Do we actually need that? Why not always try to enable it?

Finally, tmux uses the term "extended keys" to refer to any encoding scheme that disambiguates control sequences (either modifyOtherKeys or CSI u). I've adopted that terminology to refer to the same thing.

👍 good choice

[gpanders]

Do we actually need that? Why not always try to enable it?

Well we'd need to know if the terminal understood our attempt to enable it so that if it doesn't we can fall back to the modifyOtherKeys sequence. Since that would require querying the terminal anyway, we might as well just ask up front if it supports it.

[kovidgoyal]

Kitty's keyboard protocol is yet another encoding scheme which is based off of Leonerd's CSI u. Kitty's protocol supports progressive enhancement so you can use different "levels" of the protocol; the base level (level 1, which this PR enables) is identical to CSI u.

However, Leonerd's CSI u proposal does not provide any way of querying the terminal for support nor programmatically enabling or disabling that mode. Both of these are supported by kitty's keyboard protocol.

There are a whole bunch of problems with fixterms that are addressed by kitty's protocol, see https://sw.kovidgoyal.net/kitty/keyboard-protocol/#fixterms-bugs

[justinmk]

LGTM except I'd suggest to drop the terminfo check.

[gpanders]

Functional test is failing:

[  FAILED  ] test/functional/core/main_spec.lua @ 51: Command-line option -s does not crash after reading from stdin in non-headless mode
test/functional/core/main_spec.lua:60: Row 7 did not match.
Expected:
  |^                                        |
  |{1:~                                       }|
  |{1:~                                       }|
  |{1:~                                       }|
  |{1:~                                       }|
  |{2:[No Name]             0,0-1          All}|
  |*                                        |
  |                                        |
Actual:
  |^                                        |
  |{1:~                                       }|
  |{1:~                                       }|
  |{1:~                                       }|
  |{1:~                                       }|
  |{2:[No Name]             0,0-1          All}|
  |*                             2c         |
  |                                        |

The 2c is from the response to the primary device attribute request. I can't reproduce this locally in any terminal emulator, so I guess there's something unique to the CI environment that is causing these bytes to not be interpreted by Neovim and draw straight to the terminal. Not sure what the best way to address this is.

[gpanders]

No luck with ttimeoutlen=9999 :(

[gpanders]

I am guessing that this particular test fails because it reads from stdin with the -s flag and it is interpreting 2c from the primary device attribute response as the beginning of a normal command.

I don't have any ideas on how to fix this, so hopefully someone else has something. I wonder if this is a real bug that people could encounter "in the wild" or if it's unique to the CI environment.

We could try deferring writing the get_extkeys sequence until after Neovim is finished reading stdin when the -s flag is used?

[gpanders]

I have a working hypothesis on what the problem is. The test in core/main_spec.lua is using -s - to read normal commands from stdin, but it's not actually piping anything on stdin, it's just connecting stdin to the PTY generated with termopen. In tui/input.c we check for when stdin is not a tty and use stderr as the input stream instead:

https://github.com/gpanders/neovim/blob/d5b0a03971ef5b53a4fe29fae33e282dcc516408/src/nvim/tui/input.c#L62-L64

But this isn't happening because in the test stdin is a tty.

I tried adding stdin = "null" to the termopen opts in main_spec.lua but this didn't work. But I'm not convinced that means my theory is wrong, I think it could be that the stdin option is currently ignored when pty = true (as is the case for termopen).

Update:

I think it could be that the stdin option is currently ignored when pty = true (as is the case for termopen).

This is not the case, it looks like the "stdin" option is handled correctly even for PTYs. But PTYs do not allocate a separate stderr stream, which implies that the snippet above still evaluates to false.

[gpanders]

Well this fixes the test so it looks like that is in fact the issue:

diff --git a/test/functional/core/main_spec.lua b/test/functional/core/main_spec.lua
index cf6d849c0..fe8feb306 100644
--- a/test/functional/core/main_spec.lua
+++ b/test/functional/core/main_spec.lua
@@ -52,11 +52,7 @@ describe('Command-line option', function()
       if helpers.pending_win32(pending) then return end
       local screen = Screen.new(40, 8)
       screen:attach()
-      funcs.termopen({
-        nvim_prog_abs(), '-u', 'NONE', '-i', 'NONE',
-         '--cmd', 'set noswapfile shortmess+=IFW fileformats=unix',
-         '-s', '-'
-      }, { stdin = "null" })
+      funcs.termopen('echo "" | ' .. nvim_prog_abs() .. ' -u NONE -i NONE --cmd "set noswapfile shortmess+=IFW fileformats=unix" -s -')
       screen:expect([[
         ^                                        |
         {1:~                                       }|

But it's a pretty ugly fix.

[github-actions]

Successfully created backport PR #18264 for release-0.7.