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

Improved F1 through F4 handling #736

Merged
merged 6 commits into from Dec 28, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/event/sys/unix/parse.rs
Expand Up @@ -170,6 +170,13 @@ pub(crate) fn parse_csi(buffer: &[u8]) -> Result<Option<InternalEvent>> {
b'I' => Some(Event::FocusGained),
b'O' => Some(Event::FocusLost),
b';' => return parse_csi_modifier_key_code(buffer),
// P, Q, and S for compatibility with Kitty keyboard protocol,
// as the 1 in 'CSI 1 P' etc. must be omitted if there are no
// modifiers pressed:
// https://sw.kovidgoyal.net/kitty/keyboard-protocol/#legacy-functional-keys
b'P' => Some(Event::Key(KeyCode::F(1).into())),
Copy link
Member

Choose a reason for hiding this comment

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

Wonder if this could cause compatibility issues with nonkitty sequences. Are we sure this pattern never occurs when kitty is not enabled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There aren't any conflicts in the xterm documentation for sequences sent from the terminal starting with CSI and immediately terminated by P, Q, or S.

The closest is CSI ps P with ps having a default value of 1 (so it can be omitted, shortening the sequence to CSI P), but that is a sequence sent to the terminal from the program, not the other way around.

Copy link
Member

Choose a reason for hiding this comment

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

Alright, then it is fine. If in future it becomes an issue we likely can split it out and use this pr changes to check if kitty is enabled and check for those sequences.

b'Q' => Some(Event::Key(KeyCode::F(2).into())),
b'S' => Some(Event::Key(KeyCode::F(4).into())),
b'0'..=b'9' => {
// Numbered escape code.
if buffer.len() == 3 {
Expand Down