Unnamed repository; edit this file 'description' to name the repository.
tui: Refactor Config type handling in backends
The `Config` can be passed when creating the backend (for example
`CrosstermBackend::new`) and is already updated in the
`Backend::reconfigure` callback. Recreating the tui `Config` during
`claim` and `restore` is unnecessary and causes a clone of the editor's
Config which is a fairly large type. This change drops the `Config`
parameter from those callbacks and updates the callers. Instead it is
passed to `CrosstermBackend` which then owns it.
I've also moved the override from the `editor.undercurl` key onto the
tui `Config` type - I believe it was just an oversight that this was not
done originally. And I've updated the `From<EditorConfig> for Config`
to take a reference to the editor's `Config` to avoid the unnecessary
clone during `CrosstermBackend::new` and `Backend::reconfigure`.
| -rw-r--r-- | helix-term/src/application.rs | 19 | ||||
| -rw-r--r-- | helix-tui/src/backend/crossterm.rs | 26 | ||||
| -rw-r--r-- | helix-tui/src/backend/mod.rs | 4 | ||||
| -rw-r--r-- | helix-tui/src/backend/test.rs | 4 | ||||
| -rw-r--r-- | helix-tui/src/terminal.rs | 14 |
5 files changed, 31 insertions, 36 deletions
diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index cf09aac0..de661f30 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -104,7 +104,7 @@ impl Application { let theme_loader = theme::Loader::new(&theme_parent_dirs); #[cfg(not(feature = "integration"))] - let backend = CrosstermBackend::new(stdout(), &config.editor); + let backend = CrosstermBackend::new(stdout(), (&config.editor).into()); #[cfg(feature = "integration")] let backend = TestBackend::new(120, 150); @@ -367,7 +367,7 @@ impl Application { ConfigEvent::Update(editor_config) => { let mut app_config = (*self.config.load().clone()).clone(); app_config.editor = *editor_config; - if let Err(err) = self.terminal.reconfigure(app_config.editor.clone().into()) { + if let Err(err) = self.terminal.reconfigure((&app_config.editor).into()) { self.editor.set_error(err.to_string()); }; self.config.store(Arc::new(app_config)); @@ -412,8 +412,7 @@ impl Application { document.replace_diagnostics(diagnostics, &[], None); } - self.terminal - .reconfigure(default_config.editor.clone().into())?; + self.terminal.reconfigure((&default_config.editor).into())?; // Store new config self.config.store(Arc::new(default_config)); Ok(()) @@ -503,7 +502,7 @@ impl Application { // https://github.com/neovim/neovim/issues/12322 // https://github.com/neovim/neovim/pull/13084 for retries in 1..=10 { - match self.claim_term().await { + match self.terminal.claim() { Ok(()) => break, Err(err) if retries == 10 => panic!("Failed to claim terminal: {}", err), Err(_) => continue, @@ -1099,26 +1098,20 @@ impl Application { lsp::ShowDocumentResult { success: true } } - async fn claim_term(&mut self) -> std::io::Result<()> { - let terminal_config = self.config.load().editor.clone().into(); - self.terminal.claim(terminal_config) - } - 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) + self.terminal.restore() } pub async fn run<S>(&mut self, input_stream: &mut S) -> Result<i32, Error> where S: Stream<Item = std::io::Result<crossterm::event::Event>> + Unpin, { - self.claim_term().await?; + self.terminal.claim()?; // Exit the alternate screen and disable raw mode before panicking let hook = std::panic::take_hook(); diff --git a/helix-tui/src/backend/crossterm.rs b/helix-tui/src/backend/crossterm.rs index 28e20419..d04d00af 100644 --- a/helix-tui/src/backend/crossterm.rs +++ b/helix-tui/src/backend/crossterm.rs @@ -14,10 +14,7 @@ use crossterm::{ terminal::{self, Clear, ClearType}, Command, }; -use helix_view::{ - editor::Config as EditorConfig, - graphics::{Color, CursorKind, Modifier, Rect, UnderlineStyle}, -}; +use helix_view::graphics::{Color, CursorKind, Modifier, Rect, UnderlineStyle}; use once_cell::sync::OnceCell; use std::{ fmt, @@ -74,17 +71,17 @@ impl Capabilities { /// on the $TERM environment variable. If detection fails, returns /// a default value where no capability is supported, or just undercurl /// if config.undercurl is set. - pub fn from_env_or_default(config: &EditorConfig) -> Self { + pub fn from_env_or_default(config: &Config) -> Self { match termini::TermInfo::from_env() { Err(_) => Capabilities { - has_extended_underlines: config.undercurl, + has_extended_underlines: config.force_enable_extended_underlines, ..Capabilities::default() }, Ok(t) => Capabilities { // Smulx, VTE: https://unix.stackexchange.com/a/696253/246284 // Su (used by kitty): https://sw.kovidgoyal.net/kitty/underlines // WezTerm supports underlines but a lot of distros don't properly install its terminfo - has_extended_underlines: config.undercurl + has_extended_underlines: config.force_enable_extended_underlines || t.extended_cap("Smulx").is_some() || t.extended_cap("Su").is_some() || vte_version() >= Some(5102) @@ -98,6 +95,7 @@ impl Capabilities { /// Terminal backend supporting a wide variety of terminals pub struct CrosstermBackend<W: Write> { buffer: W, + config: Config, capabilities: Capabilities, supports_keyboard_enhancement_protocol: OnceCell<bool>, mouse_capture_enabled: bool, @@ -108,14 +106,15 @@ impl<W> CrosstermBackend<W> where W: Write, { - pub fn new(buffer: W, config: &EditorConfig) -> CrosstermBackend<W> { + pub fn new(buffer: W, config: Config) -> CrosstermBackend<W> { // helix is not usable without colors, but crossterm will disable // them by default if NO_COLOR is set in the environment. Override // this behaviour. crossterm::style::force_color_output(true); CrosstermBackend { buffer, - capabilities: Capabilities::from_env_or_default(config), + capabilities: Capabilities::from_env_or_default(&config), + config, supports_keyboard_enhancement_protocol: OnceCell::new(), mouse_capture_enabled: false, supports_bracketed_paste: true, @@ -157,7 +156,7 @@ impl<W> Backend for CrosstermBackend<W> where W: Write, { - fn claim(&mut self, config: Config) -> io::Result<()> { + fn claim(&mut self) -> io::Result<()> { terminal::enable_raw_mode()?; execute!( self.buffer, @@ -173,7 +172,7 @@ where Ok(_) => (), }; execute!(self.buffer, terminal::Clear(terminal::ClearType::All))?; - if config.enable_mouse_capture { + if self.config.enable_mouse_capture { execute!(self.buffer, EnableMouseCapture)?; self.mouse_capture_enabled = true; } @@ -198,15 +197,16 @@ where } self.mouse_capture_enabled = config.enable_mouse_capture; } + self.config = config; Ok(()) } - fn restore(&mut self, config: Config) -> io::Result<()> { + fn restore(&mut self) -> io::Result<()> { // reset cursor shape self.buffer .write_all(self.capabilities.reset_cursor_command.as_bytes())?; - if config.enable_mouse_capture { + if self.config.enable_mouse_capture { execute!(self.buffer, DisableMouseCapture)?; } if self.supports_keyboard_enhancement_protocol() { diff --git a/helix-tui/src/backend/mod.rs b/helix-tui/src/backend/mod.rs index d8b0555c..1423bc0a 100644 --- a/helix-tui/src/backend/mod.rs +++ b/helix-tui/src/backend/mod.rs @@ -17,11 +17,11 @@ pub use self::test::TestBackend; /// Representation of a terminal backend. pub trait Backend { /// Claims the terminal for TUI use. - fn claim(&mut self, config: Config) -> Result<(), io::Error>; + fn claim(&mut self) -> Result<(), io::Error>; /// Update terminal configuration. fn reconfigure(&mut self, config: Config) -> Result<(), io::Error>; /// Restores the terminal to a normal state, undoes `claim` - fn restore(&mut self, config: Config) -> Result<(), io::Error>; + fn restore(&mut self) -> Result<(), io::Error>; /// Forcibly resets the terminal, ignoring errors and configuration fn force_restore() -> Result<(), io::Error>; /// Draws styled text to the terminal diff --git a/helix-tui/src/backend/test.rs b/helix-tui/src/backend/test.rs index 771cc309..8cd3a2fd 100644 --- a/helix-tui/src/backend/test.rs +++ b/helix-tui/src/backend/test.rs @@ -107,7 +107,7 @@ impl TestBackend { } impl Backend for TestBackend { - fn claim(&mut self, _config: Config) -> Result<(), io::Error> { + fn claim(&mut self) -> Result<(), io::Error> { Ok(()) } @@ -115,7 +115,7 @@ impl Backend for TestBackend { Ok(()) } - fn restore(&mut self, _config: Config) -> Result<(), io::Error> { + fn restore(&mut self) -> Result<(), io::Error> { Ok(()) } diff --git a/helix-tui/src/terminal.rs b/helix-tui/src/terminal.rs index 969b285c..9dcad81d 100644 --- a/helix-tui/src/terminal.rs +++ b/helix-tui/src/terminal.rs @@ -24,12 +24,14 @@ pub struct Viewport { #[derive(Debug)] pub struct Config { pub enable_mouse_capture: bool, + pub force_enable_extended_underlines: bool, } -impl From<EditorConfig> for Config { - fn from(config: EditorConfig) -> Self { +impl From<&EditorConfig> for Config { + fn from(config: &EditorConfig) -> Self { Self { enable_mouse_capture: config.mouse, + force_enable_extended_underlines: config.undercurl, } } } @@ -102,16 +104,16 @@ where }) } - pub fn claim(&mut self, config: Config) -> io::Result<()> { - self.backend.claim(config) + pub fn claim(&mut self) -> io::Result<()> { + self.backend.claim() } pub fn reconfigure(&mut self, config: Config) -> io::Result<()> { self.backend.reconfigure(config) } - pub fn restore(&mut self, config: Config) -> io::Result<()> { - self.backend.restore(config) + pub fn restore(&mut self) -> io::Result<()> { + self.backend.restore() } // /// Get a Frame object which provides a consistent view into the terminal state for rendering. |