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

Support kitty KeyRelease and KeyRepeat events #500

Closed
pdevine opened this issue Oct 28, 2021 · 17 comments
Closed

Support kitty KeyRelease and KeyRepeat events #500

pdevine opened this issue Oct 28, 2021 · 17 comments

Comments

@pdevine
Copy link

pdevine commented Oct 28, 2021

Kitty supports enhanced key events such as KeyRelease and KeyRepeat as part of its enhanced keyboard handling protocol. This is extremely useful for making interactive applications and games in the terminal.

The protocol itself is simple and works by first testing to see if the terminal supports the feature via:

CSI ? u

and then turning on the keyboard enhancement feature with something like:

CSI = 10 ; 1 u

10 here denotes 0b1010 which turns on reporting of all enhanced event types and each key as an escape code. Each key will generate both a KeyPress and a KeyRelease event, and if you hold down a key it will generate a KeyRepeat event.

These come in the form:

CSI key-code ; modifier : 1 # key press
CSI key-code ; modifier : 2 # key repeat
CSI key-code ; modifier : 3 # key release

I'd love to see this get implemented in tcell and in other terminals other than just kitty. The protocol is well thought out and would be really useful for tcell derived applications and games.

@gdamore
Copy link
Owner

gdamore commented Nov 15, 2021

I'm a little concerned about this -- mostly because as we start to depend on escape sequences that are implemented only by a small minority of terminals, it drives applications towards developing less and less portable applications.

I'd be a bit more enthusiastic about this if it had adoption beyond just one emulator.

@kovidgoyal
Copy link

Both iterm2 https://gitlab.com/gnachman/iterm2/-/issues/10017
and foot kovidgoyal/kitty#3248 (comment)
are going to implement it.

And it is supported in notcurses dankamongmen/notcurses#2131

@gdamore
Copy link
Owner

gdamore commented Nov 15, 2021

The fact that tickets exist for this does not reflect actual intent or a promise to implement.

I'm not sure that I want to follow notcurses example with the rationale that if they implement it so should we.

Features like this substantially complicate supporting the library, and really we want to make sure that doing so is benefit to users beyond just a few niche users (Kitty is still niche IMO).

I'll leave this ticket open, but my priority is going to be supporting features that are beneficial to the mainstream.

@kovidgoyal
Copy link

kovidgoyal commented Nov 15, 2021 via email

@pdevine
Copy link
Author

pdevine commented Nov 15, 2021

I agree with @kovidgoyal 's points here completely. It seems crazy to me that we've struggled with something as basic as Key Release events in the terminal for decade after decade without any kind of workable solution. I've tried just about every hack to try and cobble together something workable, but until reading Kovid's proposal nothing came close to being reasonable.

Having or not having tcell onboard isn't going to be the end of the world, at least for me. When I wrote go-asciisprite I had originally written it with termbox, but converted it to use tcell with the compat layer since I didn't want to go through a complete rewrite. The compat layer is missing a bunch of stuff already (like mouse events and broken colour terminal support) which I've been slowly adding back anyway, so I'm sure it's possible to find some workarounds here. If you do decide to put support in tcell it would certainly help though, because the terminfo code is already pretty confusing, just like Kovid was describing.

@gdamore
Copy link
Owner

gdamore commented Apr 16, 2022

The history of the last 40 years is because historically terminals simply couldn't report this. If you have an X window app, or a pure Windows app, then of course you get all the data, on the assumption that you're on a fast enough local connection to support this kind of usage.

There is a pretty fundamental disconnect because key strokes on terminal lines are reflective of "logical" key events -- meaning you get a logical "letter A" rather than a keyboard scan code (which typically has a very different value, depending on whether you're using USB or PS/2, and may be subject to different layouts for different locales.)

Requests like this are slowly requesting to move more towards the underlying physical layer, but it does start to call into question, why are we trying to make this work in these contexts? Wouldn't it be better to just use a proper GUI application?

Anyway, I'm leaving this ticket open still, because I want to see that this is reached a level of de-facto standard that it can be broadly useful. I don't want to make a hack that only a small percentage of users can use, especially where the hack requires either a lot of extra complexity in the application or library code, or breaks the model completely.

My fear is that getting these key release events (and presumably corresponding key press events) is a significant enough change that we would want to make it opt-in, and applications are going to have to accept that a most terminals won't support it.

Adoption by vte, xterm, or the vanilla Wndows or macOS terminals would give me more confidence. iTerm, kitty, and foot, even when combined together, represent at best a small minority. In fact iTerm2 is probably several orders of magnitude more popular than the other two.

@gdamore
Copy link
Owner

gdamore commented Oct 14, 2022

Status update: foot implemented this. So right now it's kitty and foot. There doesn't appear to be any movement on iTerm2. If anyone has further updated information I'd like to hear it.

@dnkl
Copy link
Contributor

dnkl commented Oct 14, 2022

WezTerm, when told so.

Source: https://sw.kovidgoyal.net/kitty/keyboard-protocol/

@gdamore
Copy link
Owner

gdamore commented Jan 9, 2023

FYI, I am considering the CSI-u portion of this proposal. That would bring richer keyboard to various terminals, and allow more modifiers, etc.

I am still not sold on bringing in separate key-press and key-release events. Applications that needed that would wind up having to code differently, and they'd have to be able to discriminate between having this capability and not having it, and falling back to some kind of default handing for when it isn't present.

Conversely, CSI-u is pretty simple for us to support, and represents no real risks to application developers.

@Joyje
Copy link

Joyje commented Apr 1, 2023

It seems like even vim implemented this. I think it's time guys.

@gdamore
Copy link
Owner

gdamore commented Apr 1, 2023

I don't think VIM implemented separate handling for key press and key release events -- it would be astonishing to me if that were true.

I'm onboard with adding support for more accurate and complete key reporting, but separating key press and release events not so much -- the difference in model is really application breaking -- I can't think of any use for discriminating these two things apart from games. And really if you need that kind of fidelity why are you using a terminal emulator?

(It's still really niche to have terminals that support the separate event types, while I think the CSI-u protocol is gaining traction.)

@gdamore
Copy link
Owner

gdamore commented Apr 1, 2023

WezTerm, when told so.

Source: https://sw.kovidgoyal.net/kitty/keyboard-protocol/

It's not clear to me that wezTerm supports the separate key press and release events. It does seem to have some of the richer reporting for different types of keys though. (Basically the non-model-breaking CSI-u protocol.)

@kovidgoyal
Copy link

I dont understand why you are getting hung up on key release events. The protocol has them as optional. Indeed the protocol is designed so that every single enhancement over legacy is optional. If you dont want to support something, dont. Though users of your library are eventually going to ask for it, so if I were you I would get it out of the way once and for all. https://sw.kovidgoyal.net/kitty/keyboard-protocol/#progressive-enhancement

And all of the advanced features use CSI-u encoding. THe escape code encoding is largely irrelevant.

@gdamore
Copy link
Owner

gdamore commented Apr 1, 2023

Right... the reason is that THIS TICKET is specifically asking for KeyRelease and KeyRepeat events. The issue is not that we couldn't enable them... but that the model for applications will be quite a bit different ... this is a request for a new API with a new set of events, which are mutually exclusive to the legacy set. It greatly complicates lives for application developers because they will need to equipped to cope with both situations. Further, the folks who write libraries above mine (many widget libraries out there for example) are simply not going to be able to deal with the different model.

I have no problem adopting the improved model that helps disambiguate for example Tab vs. Ctrl-I. I love that this is an option -- I just haven't had enough time to code it up.

But changing the operational model to support key press vs. release events is simply not something I'm prepared to do -- it makes life harder for my consumers, and it's potentially significantly API breaking. Given all that, I'm just not inclined to go down that particular rabbit hole -- especially since this capability is quite definitely not widely adopted or used. The only way to usefully use that capability is to either abandon support for pretty much all of the common terminals, or write code that can fallback to reasonable behavior when that isn't supported -- and thats a fairly large burden.

I suppose that one possibility does exist -- we could detect when the underlying terminal doesn't support key release events, and synthetically turn key events into individual key press and release events. I'm not actually sure that helps application authors though.

To be honest the separate key press vs. release events does feel like a solution looking for a problem, at least in the context of terminal applications.

@kovidgoyal
Copy link

I dont know what sort of APIs you have. But adding capabilities,
especially when they are optional does not typically complicate the life
of existing consumers of an API.

Existing applications that use your library can go on doing what they
have always been doing. New applications that want release events can
opt in to the new event types. I would recommend you dont try to
synthesize events. An application that opts into release events will
want actual release events, synthetic ones would likely do more harm
than good. Instead have a way for applications to query for
availability, they can decide how they want to handle fallback
themselves.

At least one of your users has a use case for release events, so
there's at least one problem its an actual solution for.

@Joyje
Copy link

Joyje commented Apr 2, 2023

I don't think VIM implemented separate handling for key press and key release events -- it would be astonishing to me if that were true.

I don't know much about this new protocol and all the details. All of this is new knowledge to me. I just know that vim can differentiate between Ctrl-i and Tab, which means vim at least implemented some part of this new protocol.

I'm onboard with adding support for more accurate and complete key reporting, but separating key press and release events not so much

I would appreciate this so much! It seems like this is all that's needed for my use case anyways. I don't care much for the other part of the protocol (the key press and release events).

Right... the reason is that THIS TICKET is specifically asking for KeyRelease and KeyRepeat events.

You are absolutely right! Maybe I should have created a new issue instead of bumping this thread, but at least it is related to the same protocol if I understand this correctly. It seemed convenient to post a comment here. I would really love to see tcell live up to the same standard as vim. It would improve the quality of every single project having tcell in it's dependency.

I have no problem adopting the improved model that helps disambiguate for example Tab vs. Ctrl-I. I love that this is an option -- I just haven't had enough time to code it up.

Thanks a bunch! I hope you can find the time to code it, alternatively let some other contributor do it for us all. Again thanks if you intend to do this, and please keep us updated!

@gdamore
Copy link
Owner

gdamore commented Dec 10, 2023

I'm closing this ticket, in favor of #671. The main thing is that I have no interest in implementing the key release or key repeat events, because of the semantic implications for the API and the badness it means for an application. I'm not prepared to opt-in to that.

Conversely, what #671 indicates is that we should use modern reporting to be able to discriminate some of these keys that today make it impossible to treat e.g. Ctrl-I and Tab differently.

I expect I will probably implement #671 in the next release.

@gdamore gdamore closed this as completed Dec 10, 2023
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

No branches or pull requests

5 participants