-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
tui: Only query keyboard enhancement support once #6160
tui: Only query keyboard enhancement support once #6160
Conversation
We query the host terminal emulator to check whether it supports the keyboard enhancement protocol after entering raw mode on startup. We only need to check this value once though, so we can store the result in a OnceCell to prevent repeated queries. The query for keyboard enhancement support times out and fails when suspending (C-z) and resuming (`fg`) which can cause delays and leaves sequences in the terminal after Helix suspends or exits. Caching in the OnceCell prevents this behavior.
if matches!(terminal::supports_keyboard_enhancement(), Ok(true)) { | ||
execute!(stdout, PopKeyboardEnhancementFlags)?; | ||
} | ||
// Ignore errors on disabling, this might trigger on windows if we call | ||
// disable without calling enable previously | ||
let _ = execute!(stdout, DisableMouseCapture); | ||
let _ = execute!(stdout, PopKeyboardEnhancementFlags); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally we would check terminal.supports_keyboard_enhancement_protocol()
here but it's complicated to pass in values from Application because this function is set as a panic handler. We might be able to pass in just the boolean for whether keyboard enhancement is supported. I will check on some different terminals to see if it works to just send this PopKeyboardEnhancementFlags
always though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I gave this a test with Kitty, WezTerm (with enhanced keyboard protocol disabled), Alacritty and the default login terminal for my linux machine and it seems to not make a difference - the terminal ignores the escape sequence when it doesn't recognize it
@@ -62,6 +64,7 @@ where | |||
CrosstermBackend { | |||
buffer, | |||
capabilities: Capabilities::from_env_or_default(), | |||
supports_keyboard_enhancement_protocol: OnceCell::new(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This value is not included in Capabilities because the tty needs to be in raw mode to perform the query and the backend is created before raw mode is enabled
#6149 would alternatively be solved by #6170 but we will want similar behavior in the future for checking synchronized output support as well (#731) EDIT: it looks like #6149 can also happen when resuming the process with |
Superseded by #6194 which should make it easier to add other queries in the future |
We query the host terminal emulator to check whether it supports the keyboard enhancement protocol after entering raw mode on startup. We only need to check this value once though, so we can store the result in a OnceCell to prevent repeated queries. The query for keyboard enhancement support times out and fails when suspending (C-z) and resuming (
fg
) which can cause delays and leaves sequences in the terminal after Helix suspends or exits. Caching in the OnceCell prevents this behavior.Fixes #6149