Skip to content

Commit 9cc912a

Browse files
committed
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`.
1 parent fe1393c commit 9cc912a

File tree

5 files changed

+31
-36
lines changed

5 files changed

+31
-36
lines changed

helix-term/src/application.rs

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ impl Application {
104104
let theme_loader = theme::Loader::new(&theme_parent_dirs);
105105

106106
#[cfg(not(feature = "integration"))]
107-
let backend = CrosstermBackend::new(stdout(), &config.editor);
107+
let backend = CrosstermBackend::new(stdout(), (&config.editor).into());
108108

109109
#[cfg(feature = "integration")]
110110
let backend = TestBackend::new(120, 150);
@@ -367,7 +367,7 @@ impl Application {
367367
ConfigEvent::Update(editor_config) => {
368368
let mut app_config = (*self.config.load().clone()).clone();
369369
app_config.editor = *editor_config;
370-
if let Err(err) = self.terminal.reconfigure(app_config.editor.clone().into()) {
370+
if let Err(err) = self.terminal.reconfigure((&app_config.editor).into()) {
371371
self.editor.set_error(err.to_string());
372372
};
373373
self.config.store(Arc::new(app_config));
@@ -412,8 +412,7 @@ impl Application {
412412
document.replace_diagnostics(diagnostics, &[], None);
413413
}
414414

415-
self.terminal
416-
.reconfigure(default_config.editor.clone().into())?;
415+
self.terminal.reconfigure((&default_config.editor).into())?;
417416
// Store new config
418417
self.config.store(Arc::new(default_config));
419418
Ok(())
@@ -503,7 +502,7 @@ impl Application {
503502
// https://github.com/neovim/neovim/issues/12322
504503
// https://github.com/neovim/neovim/pull/13084
505504
for retries in 1..=10 {
506-
match self.claim_term().await {
505+
match self.terminal.claim() {
507506
Ok(()) => break,
508507
Err(err) if retries == 10 => panic!("Failed to claim terminal: {}", err),
509508
Err(_) => continue,
@@ -1099,26 +1098,20 @@ impl Application {
10991098
lsp::ShowDocumentResult { success: true }
11001099
}
11011100

1102-
async fn claim_term(&mut self) -> std::io::Result<()> {
1103-
let terminal_config = self.config.load().editor.clone().into();
1104-
self.terminal.claim(terminal_config)
1105-
}
1106-
11071101
fn restore_term(&mut self) -> std::io::Result<()> {
1108-
let terminal_config = self.config.load().editor.clone().into();
11091102
use helix_view::graphics::CursorKind;
11101103
self.terminal
11111104
.backend_mut()
11121105
.show_cursor(CursorKind::Block)
11131106
.ok();
1114-
self.terminal.restore(terminal_config)
1107+
self.terminal.restore()
11151108
}
11161109

11171110
pub async fn run<S>(&mut self, input_stream: &mut S) -> Result<i32, Error>
11181111
where
11191112
S: Stream<Item = std::io::Result<crossterm::event::Event>> + Unpin,
11201113
{
1121-
self.claim_term().await?;
1114+
self.terminal.claim()?;
11221115

11231116
// Exit the alternate screen and disable raw mode before panicking
11241117
let hook = std::panic::take_hook();

helix-tui/src/backend/crossterm.rs

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,7 @@ use crossterm::{
1414
terminal::{self, Clear, ClearType},
1515
Command,
1616
};
17-
use helix_view::{
18-
editor::Config as EditorConfig,
19-
graphics::{Color, CursorKind, Modifier, Rect, UnderlineStyle},
20-
};
17+
use helix_view::graphics::{Color, CursorKind, Modifier, Rect, UnderlineStyle};
2118
use once_cell::sync::OnceCell;
2219
use std::{
2320
fmt,
@@ -74,17 +71,17 @@ impl Capabilities {
7471
/// on the $TERM environment variable. If detection fails, returns
7572
/// a default value where no capability is supported, or just undercurl
7673
/// if config.undercurl is set.
77-
pub fn from_env_or_default(config: &EditorConfig) -> Self {
74+
pub fn from_env_or_default(config: &Config) -> Self {
7875
match termini::TermInfo::from_env() {
7976
Err(_) => Capabilities {
80-
has_extended_underlines: config.undercurl,
77+
has_extended_underlines: config.force_enable_extended_underlines,
8178
..Capabilities::default()
8279
},
8380
Ok(t) => Capabilities {
8481
// Smulx, VTE: https://unix.stackexchange.com/a/696253/246284
8582
// Su (used by kitty): https://sw.kovidgoyal.net/kitty/underlines
8683
// WezTerm supports underlines but a lot of distros don't properly install its terminfo
87-
has_extended_underlines: config.undercurl
84+
has_extended_underlines: config.force_enable_extended_underlines
8885
|| t.extended_cap("Smulx").is_some()
8986
|| t.extended_cap("Su").is_some()
9087
|| vte_version() >= Some(5102)
@@ -98,6 +95,7 @@ impl Capabilities {
9895
/// Terminal backend supporting a wide variety of terminals
9996
pub struct CrosstermBackend<W: Write> {
10097
buffer: W,
98+
config: Config,
10199
capabilities: Capabilities,
102100
supports_keyboard_enhancement_protocol: OnceCell<bool>,
103101
mouse_capture_enabled: bool,
@@ -108,14 +106,15 @@ impl<W> CrosstermBackend<W>
108106
where
109107
W: Write,
110108
{
111-
pub fn new(buffer: W, config: &EditorConfig) -> CrosstermBackend<W> {
109+
pub fn new(buffer: W, config: Config) -> CrosstermBackend<W> {
112110
// helix is not usable without colors, but crossterm will disable
113111
// them by default if NO_COLOR is set in the environment. Override
114112
// this behaviour.
115113
crossterm::style::force_color_output(true);
116114
CrosstermBackend {
117115
buffer,
118-
capabilities: Capabilities::from_env_or_default(config),
116+
capabilities: Capabilities::from_env_or_default(&config),
117+
config,
119118
supports_keyboard_enhancement_protocol: OnceCell::new(),
120119
mouse_capture_enabled: false,
121120
supports_bracketed_paste: true,
@@ -157,7 +156,7 @@ impl<W> Backend for CrosstermBackend<W>
157156
where
158157
W: Write,
159158
{
160-
fn claim(&mut self, config: Config) -> io::Result<()> {
159+
fn claim(&mut self) -> io::Result<()> {
161160
terminal::enable_raw_mode()?;
162161
execute!(
163162
self.buffer,
@@ -173,7 +172,7 @@ where
173172
Ok(_) => (),
174173
};
175174
execute!(self.buffer, terminal::Clear(terminal::ClearType::All))?;
176-
if config.enable_mouse_capture {
175+
if self.config.enable_mouse_capture {
177176
execute!(self.buffer, EnableMouseCapture)?;
178177
self.mouse_capture_enabled = true;
179178
}
@@ -198,15 +197,16 @@ where
198197
}
199198
self.mouse_capture_enabled = config.enable_mouse_capture;
200199
}
200+
self.config = config;
201201

202202
Ok(())
203203
}
204204

205-
fn restore(&mut self, config: Config) -> io::Result<()> {
205+
fn restore(&mut self) -> io::Result<()> {
206206
// reset cursor shape
207207
self.buffer
208208
.write_all(self.capabilities.reset_cursor_command.as_bytes())?;
209-
if config.enable_mouse_capture {
209+
if self.config.enable_mouse_capture {
210210
execute!(self.buffer, DisableMouseCapture)?;
211211
}
212212
if self.supports_keyboard_enhancement_protocol() {

helix-tui/src/backend/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,11 @@ pub use self::test::TestBackend;
1717
/// Representation of a terminal backend.
1818
pub trait Backend {
1919
/// Claims the terminal for TUI use.
20-
fn claim(&mut self, config: Config) -> Result<(), io::Error>;
20+
fn claim(&mut self) -> Result<(), io::Error>;
2121
/// Update terminal configuration.
2222
fn reconfigure(&mut self, config: Config) -> Result<(), io::Error>;
2323
/// Restores the terminal to a normal state, undoes `claim`
24-
fn restore(&mut self, config: Config) -> Result<(), io::Error>;
24+
fn restore(&mut self) -> Result<(), io::Error>;
2525
/// Forcibly resets the terminal, ignoring errors and configuration
2626
fn force_restore() -> Result<(), io::Error>;
2727
/// Draws styled text to the terminal

helix-tui/src/backend/test.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,15 +107,15 @@ impl TestBackend {
107107
}
108108

109109
impl Backend for TestBackend {
110-
fn claim(&mut self, _config: Config) -> Result<(), io::Error> {
110+
fn claim(&mut self) -> Result<(), io::Error> {
111111
Ok(())
112112
}
113113

114114
fn reconfigure(&mut self, _config: Config) -> Result<(), io::Error> {
115115
Ok(())
116116
}
117117

118-
fn restore(&mut self, _config: Config) -> Result<(), io::Error> {
118+
fn restore(&mut self) -> Result<(), io::Error> {
119119
Ok(())
120120
}
121121

helix-tui/src/terminal.rs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,14 @@ pub struct Viewport {
2424
#[derive(Debug)]
2525
pub struct Config {
2626
pub enable_mouse_capture: bool,
27+
pub force_enable_extended_underlines: bool,
2728
}
2829

29-
impl From<EditorConfig> for Config {
30-
fn from(config: EditorConfig) -> Self {
30+
impl From<&EditorConfig> for Config {
31+
fn from(config: &EditorConfig) -> Self {
3132
Self {
3233
enable_mouse_capture: config.mouse,
34+
force_enable_extended_underlines: config.undercurl,
3335
}
3436
}
3537
}
@@ -102,16 +104,16 @@ where
102104
})
103105
}
104106

105-
pub fn claim(&mut self, config: Config) -> io::Result<()> {
106-
self.backend.claim(config)
107+
pub fn claim(&mut self) -> io::Result<()> {
108+
self.backend.claim()
107109
}
108110

109111
pub fn reconfigure(&mut self, config: Config) -> io::Result<()> {
110112
self.backend.reconfigure(config)
111113
}
112114

113-
pub fn restore(&mut self, config: Config) -> io::Result<()> {
114-
self.backend.restore(config)
115+
pub fn restore(&mut self) -> io::Result<()> {
116+
self.backend.restore()
115117
}
116118

117119
// /// Get a Frame object which provides a consistent view into the terminal state for rendering.

0 commit comments

Comments
 (0)