From 3ae79f27feb157bf0253935ffd04d320c41132d5 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Sun, 5 Mar 2023 12:06:12 -0600 Subject: [PATCH 1/3] Move terminal claim/restore code to helix-tui This moves the `Application::claim_term` and `helix-term::application::restore_term` functions into the helix-tui crate. How the terminal should be claimed and restored is a TUI concern and is implemented differently through different TUI backends. This cleans out a lot of crossterm and TUI code in Application and makes it easier to modify claim/restore based on information we query from the terminal host. The child commit will take advantage of this to cache the check for whether the host terminal supports the keyboard enhancement protocol. Without this change, caching that information takes much more code which is not easily reusable for anything else. The code to restore the terminal is somewhat duplicated by this patch: we want to restore the terminal in cases of panics. Panic handler hooks must live for `'static` and the Application's terminal does not. --- Cargo.lock | 1 + helix-term/src/application.rs | 98 +++++++----------------------- helix-tui/Cargo.toml | 1 + helix-tui/src/backend/crossterm.rs | 70 ++++++++++++++++++++- helix-tui/src/backend/mod.rs | 5 +- helix-tui/src/backend/test.rs | 13 ++++ helix-tui/src/terminal.rs | 22 +++++++ 7 files changed, 131 insertions(+), 79 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 090b5f027492..349a246537c9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1216,6 +1216,7 @@ dependencies = [ "crossterm", "helix-core", "helix-view", + "log", "serde", "termini", "unicode-segmentation", diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index df6d9da6cd32..2487a02f831d 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -30,22 +30,14 @@ use crate::{ use log::{debug, error, warn}; use std::{ - io::{stdin, stdout, Write}, + io::{stdin, stdout}, sync::Arc, time::{Duration, Instant}, }; use anyhow::{Context, Error}; -use crossterm::{ - event::{ - DisableBracketedPaste, DisableFocusChange, DisableMouseCapture, EnableBracketedPaste, - EnableFocusChange, EnableMouseCapture, Event as CrosstermEvent, KeyboardEnhancementFlags, - PopKeyboardEnhancementFlags, PushKeyboardEnhancementFlags, - }, - execute, terminal, - tty::IsTty, -}; +use crossterm::{event::Event as CrosstermEvent, tty::IsTty}; #[cfg(not(windows))] use { signal_hook::{consts::signal, low_level}, @@ -63,10 +55,12 @@ use tui::backend::CrosstermBackend; use tui::backend::TestBackend; #[cfg(not(feature = "integration"))] -type Terminal = tui::terminal::Terminal>; +type TerminalBackend = CrosstermBackend; #[cfg(feature = "integration")] -type Terminal = tui::terminal::Terminal; +type TerminalBackend = TestBackend; + +type Terminal = tui::terminal::Terminal; pub struct Application { compositor: Compositor, @@ -108,26 +102,6 @@ fn setup_integration_logging() { .apply(); } -fn restore_term() -> Result<(), Error> { - let mut stdout = stdout(); - // reset cursor shape - write!(stdout, "\x1B[0 q")?; - 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); - execute!( - stdout, - DisableBracketedPaste, - DisableFocusChange, - terminal::LeaveAlternateScreen - )?; - terminal::disable_raw_mode()?; - Ok(()) -} - impl Application { pub fn new( args: Args, @@ -472,13 +446,7 @@ impl Application { pub async fn handle_signals(&mut self, signal: i32) { match signal { signal::SIGTSTP => { - // restore cursor - use helix_view::graphics::CursorKind; - self.terminal - .backend_mut() - .show_cursor(CursorKind::Block) - .ok(); - restore_term().unwrap(); + self.restore_term().unwrap(); low_level::emulate_default_handler(signal::SIGTSTP).unwrap(); } signal::SIGCONT => { @@ -1054,37 +1022,19 @@ impl Application { } } - async fn claim_term(&mut self) -> Result<(), Error> { - use helix_view::graphics::CursorKind; - terminal::enable_raw_mode()?; - if self.terminal.cursor_kind() == CursorKind::Hidden { - self.terminal.backend_mut().hide_cursor().ok(); - } - let mut stdout = stdout(); - execute!( - stdout, - terminal::EnterAlternateScreen, - EnableBracketedPaste, - EnableFocusChange - )?; - execute!(stdout, terminal::Clear(terminal::ClearType::All))?; - if self.config.load().editor.mouse { - execute!(stdout, EnableMouseCapture)?; - } - if matches!(terminal::supports_keyboard_enhancement(), Ok(true)) { - log::debug!("The enhanced keyboard protocol is supported on this terminal"); - execute!( - stdout, - PushKeyboardEnhancementFlags( - KeyboardEnhancementFlags::DISAMBIGUATE_ESCAPE_CODES - | KeyboardEnhancementFlags::REPORT_ALTERNATE_KEYS - ) - )?; - } else { - log::debug!("The enhanced keyboard protocol is not supported on this terminal"); - } + async fn claim_term(&mut self) -> std::io::Result<()> { + let terminal_config = self.config.load().editor.clone().into(); + self.terminal.claim(terminal_config) + } - Ok(()) + fn restore_term(&mut self) -> std::io::Result<()> { + let terminal_config = self.config.load().editor.clone().into(); + use helix_view::graphics::CursorKind; + self.terminal + .backend_mut() + .show_cursor(CursorKind::Block) + .ok(); + self.terminal.restore(terminal_config) } pub async fn run(&mut self, input_stream: &mut S) -> Result @@ -1099,7 +1049,7 @@ impl Application { // We can't handle errors properly inside this closure. And it's // probably not a good idea to `unwrap()` inside a panic handler. // So we just ignore the `Result`. - let _ = restore_term(); + let _ = TerminalBackend::force_restore(); hook(info); })); @@ -1107,13 +1057,7 @@ impl Application { let close_errs = self.close().await; - // restore cursor - use helix_view::graphics::CursorKind; - self.terminal - .backend_mut() - .show_cursor(CursorKind::Block) - .ok(); - restore_term()?; + self.restore_term()?; for err in close_errs { self.editor.exit_code = 1; diff --git a/helix-tui/Cargo.toml b/helix-tui/Cargo.toml index ccd016f50b9c..3ca7e044e215 100644 --- a/helix-tui/Cargo.toml +++ b/helix-tui/Cargo.toml @@ -22,5 +22,6 @@ unicode-segmentation = "1.10" crossterm = { version = "0.26", optional = true } termini = "0.1" serde = { version = "1", "optional" = true, features = ["derive"]} +log = "~0.4" helix-view = { version = "0.6", path = "../helix-view", features = ["term"] } helix-core = { version = "0.6", path = "../helix-core" } diff --git a/helix-tui/src/backend/crossterm.rs b/helix-tui/src/backend/crossterm.rs index 5305640cb15a..e81c1e008c4d 100644 --- a/helix-tui/src/backend/crossterm.rs +++ b/helix-tui/src/backend/crossterm.rs @@ -1,6 +1,11 @@ -use crate::{backend::Backend, buffer::Cell}; +use crate::{backend::Backend, buffer::Cell, terminal::Config}; use crossterm::{ cursor::{Hide, MoveTo, SetCursorStyle, Show}, + event::{ + DisableBracketedPaste, DisableFocusChange, DisableMouseCapture, EnableBracketedPaste, + EnableFocusChange, EnableMouseCapture, KeyboardEnhancementFlags, + PopKeyboardEnhancementFlags, PushKeyboardEnhancementFlags, + }, execute, queue, style::{ Attribute as CAttribute, Color as CColor, Print, SetAttribute, SetBackgroundColor, @@ -83,6 +88,69 @@ impl Backend for CrosstermBackend where W: Write, { + fn claim(&mut self, config: Config) -> io::Result<()> { + terminal::enable_raw_mode()?; + execute!( + self.buffer, + terminal::EnterAlternateScreen, + EnableBracketedPaste, + EnableFocusChange + )?; + execute!(self.buffer, terminal::Clear(terminal::ClearType::All))?; + if config.enable_mouse_capture { + execute!(self.buffer, EnableMouseCapture)?; + } + if matches!(terminal::supports_keyboard_enhancement(), Ok(true)) { + log::debug!("The enhanced keyboard protocol is supported on this terminal"); + execute!( + self.buffer, + PushKeyboardEnhancementFlags( + KeyboardEnhancementFlags::DISAMBIGUATE_ESCAPE_CODES + | KeyboardEnhancementFlags::REPORT_ALTERNATE_KEYS + ) + )?; + } else { + log::debug!("The enhanced keyboard protocol is not supported on this terminal"); + } + Ok(()) + } + + fn restore(&mut self, config: Config) -> io::Result<()> { + // reset cursor shape + write!(self.buffer, "\x1B[0 q")?; + if config.enable_mouse_capture { + execute!(self.buffer, DisableMouseCapture)?; + } + if matches!(terminal::supports_keyboard_enhancement(), Ok(true)) { + execute!(self.buffer, PopKeyboardEnhancementFlags)?; + } + execute!( + self.buffer, + DisableBracketedPaste, + DisableFocusChange, + terminal::LeaveAlternateScreen + )?; + terminal::disable_raw_mode() + } + + fn force_restore() -> io::Result<()> { + let mut stdout = io::stdout(); + + // reset cursor shape + write!(stdout, "\x1B[0 q")?; + // 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); + execute!( + stdout, + DisableBracketedPaste, + DisableFocusChange, + terminal::LeaveAlternateScreen + )?; + terminal::disable_raw_mode() + } + fn draw<'a, I>(&mut self, content: I) -> io::Result<()> where I: Iterator, diff --git a/helix-tui/src/backend/mod.rs b/helix-tui/src/backend/mod.rs index c6c11019de8c..6d7c3894258b 100644 --- a/helix-tui/src/backend/mod.rs +++ b/helix-tui/src/backend/mod.rs @@ -1,6 +1,6 @@ use std::io; -use crate::buffer::Cell; +use crate::{buffer::Cell, terminal::Config}; use helix_view::graphics::{CursorKind, Rect}; @@ -13,6 +13,9 @@ mod test; pub use self::test::TestBackend; pub trait Backend { + fn claim(&mut self, config: Config) -> Result<(), io::Error>; + fn restore(&mut self, config: Config) -> Result<(), io::Error>; + fn force_restore() -> Result<(), io::Error>; fn draw<'a, I>(&mut self, content: I) -> Result<(), io::Error> where I: Iterator; diff --git a/helix-tui/src/backend/test.rs b/helix-tui/src/backend/test.rs index 52474148e7bc..ff133ff3e846 100644 --- a/helix-tui/src/backend/test.rs +++ b/helix-tui/src/backend/test.rs @@ -1,6 +1,7 @@ use crate::{ backend::Backend, buffer::{Buffer, Cell}, + terminal::Config, }; use helix_core::unicode::width::UnicodeWidthStr; use helix_view::graphics::{CursorKind, Rect}; @@ -106,6 +107,18 @@ impl TestBackend { } impl Backend for TestBackend { + fn claim(&mut self, _config: Config) -> Result<(), io::Error> { + Ok(()) + } + + fn restore(&mut self, _config: Config) -> Result<(), io::Error> { + Ok(()) + } + + fn force_restore() -> Result<(), io::Error> { + Ok(()) + } + fn draw<'a, I>(&mut self, content: I) -> Result<(), io::Error> where I: Iterator, diff --git a/helix-tui/src/terminal.rs b/helix-tui/src/terminal.rs index 22e9232f3f86..802a8c1d9f53 100644 --- a/helix-tui/src/terminal.rs +++ b/helix-tui/src/terminal.rs @@ -1,4 +1,5 @@ use crate::{backend::Backend, buffer::Buffer}; +use helix_view::editor::Config as EditorConfig; use helix_view::graphics::{CursorKind, Rect}; use std::io; @@ -16,6 +17,19 @@ pub struct Viewport { resize_behavior: ResizeBehavior, } +#[derive(Debug)] +pub struct Config { + pub enable_mouse_capture: bool, +} + +impl From for Config { + fn from(config: EditorConfig) -> Self { + Self { + enable_mouse_capture: config.mouse, + } + } +} + impl Viewport { /// UNSTABLE pub fn fixed(area: Rect) -> Viewport { @@ -98,6 +112,14 @@ where }) } + pub fn claim(&mut self, config: Config) -> io::Result<()> { + self.backend.claim(config) + } + + pub fn restore(&mut self, config: Config) -> io::Result<()> { + self.backend.restore(config) + } + // /// Get a Frame object which provides a consistent view into the terminal state for rendering. // pub fn get_frame(&mut self) -> Frame { // Frame { From 51625cb4b03c8c81901aaf99455bc0afea35a288 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Sun, 5 Mar 2023 12:13:11 -0600 Subject: [PATCH 2/3] tui: Cache the keyboard enhancement check Wether the host terminal supports keyboard enhancement can be cached for the lifetime of a Helix session. Caching this lookup prevents a potential lockup within crossterm's event reading system where the query for the keyboard enhancement support waits on the next keyboard event, which can happen if the crossterm event stream is checked by `tokio::select!` in another thread. --- Cargo.lock | 1 + helix-tui/Cargo.toml | 1 + helix-tui/src/backend/crossterm.rs | 14 ++++++++++++-- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 349a246537c9..50c827c8d25c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1217,6 +1217,7 @@ dependencies = [ "helix-core", "helix-view", "log", + "once_cell", "serde", "termini", "unicode-segmentation", diff --git a/helix-tui/Cargo.toml b/helix-tui/Cargo.toml index 3ca7e044e215..8a6d5367d82e 100644 --- a/helix-tui/Cargo.toml +++ b/helix-tui/Cargo.toml @@ -22,6 +22,7 @@ unicode-segmentation = "1.10" crossterm = { version = "0.26", optional = true } termini = "0.1" serde = { version = "1", "optional" = true, features = ["derive"]} +once_cell = "1.17" log = "~0.4" helix-view = { version = "0.6", path = "../helix-view", features = ["term"] } helix-core = { version = "0.6", path = "../helix-core" } diff --git a/helix-tui/src/backend/crossterm.rs b/helix-tui/src/backend/crossterm.rs index e81c1e008c4d..7c512ce3e430 100644 --- a/helix-tui/src/backend/crossterm.rs +++ b/helix-tui/src/backend/crossterm.rs @@ -15,6 +15,7 @@ use crossterm::{ Command, }; use helix_view::graphics::{Color, CursorKind, Modifier, Rect, UnderlineStyle}; +use once_cell::sync::OnceCell; use std::{ fmt, io::{self, Write}, @@ -57,6 +58,7 @@ impl Capabilities { pub struct CrosstermBackend { buffer: W, capabilities: Capabilities, + supports_keyboard_enhancement_protocol: OnceCell, } impl CrosstermBackend @@ -67,8 +69,16 @@ where CrosstermBackend { buffer, capabilities: Capabilities::from_env_or_default(), + supports_keyboard_enhancement_protocol: OnceCell::new(), } } + + #[inline] + fn supports_keyboard_enhancement_protocol(&self) -> io::Result { + self.supports_keyboard_enhancement_protocol + .get_or_try_init(terminal::supports_keyboard_enhancement) + .copied() + } } impl Write for CrosstermBackend @@ -100,7 +110,7 @@ where if config.enable_mouse_capture { execute!(self.buffer, EnableMouseCapture)?; } - if matches!(terminal::supports_keyboard_enhancement(), Ok(true)) { + if self.supports_keyboard_enhancement_protocol()? { log::debug!("The enhanced keyboard protocol is supported on this terminal"); execute!( self.buffer, @@ -121,7 +131,7 @@ where if config.enable_mouse_capture { execute!(self.buffer, DisableMouseCapture)?; } - if matches!(terminal::supports_keyboard_enhancement(), Ok(true)) { + if self.supports_keyboard_enhancement_protocol()? { execute!(self.buffer, PopKeyboardEnhancementFlags)?; } execute!( From eceb50437084e23c691760ee637138dce0e5ca08 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Tue, 7 Mar 2023 10:33:46 -0600 Subject: [PATCH 3/3] tui: Log keyboard enhancement query time In my testing this takes around 3-4ms in terminals that support the enhanced keyboard protocol (Kitty, WezTerm) and a few hundred microseconds in terminals that don't (st, Alacritty). --- helix-tui/src/backend/crossterm.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/helix-tui/src/backend/crossterm.rs b/helix-tui/src/backend/crossterm.rs index 7c512ce3e430..fba1f029c244 100644 --- a/helix-tui/src/backend/crossterm.rs +++ b/helix-tui/src/backend/crossterm.rs @@ -76,7 +76,18 @@ where #[inline] fn supports_keyboard_enhancement_protocol(&self) -> io::Result { self.supports_keyboard_enhancement_protocol - .get_or_try_init(terminal::supports_keyboard_enhancement) + .get_or_try_init(|| { + use std::time::Instant; + + let now = Instant::now(); + let support = terminal::supports_keyboard_enhancement(); + log::debug!( + "The keyboard enhancement protocol is {}supported in this terminal (checked in {:?})", + if matches!(support, Ok(true)) { "" } else { "not " }, + Instant::now().duration_since(now) + ); + support + }) .copied() } } @@ -111,7 +122,6 @@ where execute!(self.buffer, EnableMouseCapture)?; } if self.supports_keyboard_enhancement_protocol()? { - log::debug!("The enhanced keyboard protocol is supported on this terminal"); execute!( self.buffer, PushKeyboardEnhancementFlags( @@ -119,8 +129,6 @@ where | KeyboardEnhancementFlags::REPORT_ALTERNATE_KEYS ) )?; - } else { - log::debug!("The enhanced keyboard protocol is not supported on this terminal"); } Ok(()) }