From d4be3b203098b2f130e18504f421d60fbc9147df Mon Sep 17 00:00:00 2001 From: Innes Anderson-Morrison Date: Sat, 15 Feb 2025 17:28:29 +0000 Subject: [PATCH 1/4] feat: caching computed token styling in the Tui UI --- src/config.rs | 27 ++++++++-------- src/term.rs | 22 +++++++++++++ src/ui/mod.rs | 1 + src/ui/tui.rs | 88 +++++++++++++++++++++++++++++++++++---------------- 4 files changed, 97 insertions(+), 41 deletions(-) diff --git a/src/config.rs b/src/config.rs index 8e3e01d..5b2baca 100644 --- a/src/config.rs +++ b/src/config.rs @@ -9,7 +9,7 @@ use crate::{ util::parent_dir_containing, }; use serde::{de, Deserialize, Deserializer}; -use std::{collections::HashMap, env, fs, io, path::Path}; +use std::{collections::HashMap, env, fs, io, iter::successors, path::Path}; use tracing::{error, warn}; pub const DEFAULT_CONFIG: &str = include_str!("../data/config.toml"); @@ -181,20 +181,21 @@ impl Default for ColorScheme { } impl ColorScheme { - /// Determine UI [Style]s to be applied for a given syntax tag. + /// Determine UI [Styles] to be applied for a given syntax tag. /// - /// If the full tag does not have associated styling but its dotted prefix does (e.g. - /// "function.macro" -> "function") then the styling of the prefix is used. Otherwise default - /// styling will be used ([TK_DEFAULT]). + /// If the full tag does not have associated styling but its dotted prefix does then the + /// styling of the prefix is used, otherwise default styling will be used ([TK_DEFAULT]). + /// + /// For key "foo.bar.baz" this will return the first value found out of the following keyset: + /// - "foo.bar.baz" + /// - "foo.bar" + /// - "foo" + /// - [TK_DEFAULT] pub fn styles_for(&self, tag: &str) -> &Styles { - match self.syntax.get(tag) { - Some(styles) => styles, - None => tag - .split_once('.') - .and_then(|(prefix, _)| self.syntax.get(prefix)) - .or(self.syntax.get(TK_DEFAULT)) - .expect("to have default styles"), - } + successors(Some(tag), |s| Some(s.rsplit_once('.')?.0)) + .find_map(|k| self.syntax.get(k)) + .or(self.syntax.get(TK_DEFAULT)) + .expect("to have default styles") } } diff --git a/src/term.rs b/src/term.rs index 51c22da..ea127d6 100644 --- a/src/term.rs +++ b/src/term.rs @@ -113,6 +113,28 @@ pub struct Styles { pub underline: bool, } +impl fmt::Display for Styles { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if let Some(fg) = self.fg { + write!(f, "{}", Style::Fg(fg))?; + } + if let Some(bg) = self.bg { + write!(f, "{}", Style::Bg(bg))?; + } + if self.bold { + write!(f, "{}", Style::Bold)?; + } + if self.italic { + write!(f, "{}", Style::Italic)?; + } + if self.underline { + write!(f, "{}", Style::Underline)?; + } + + Ok(()) + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Style { Fg(Color), diff --git a/src/ui/mod.rs b/src/ui/mod.rs index 49aba2f..99f3f0a 100644 --- a/src/ui/mod.rs +++ b/src/ui/mod.rs @@ -67,6 +67,7 @@ pub(crate) enum StateChange { StatusMessage { msg: String }, } +#[allow(clippy::large_enum_variant)] #[derive(Debug)] pub(crate) enum Ui { Headless, diff --git a/src/ui/tui.rs b/src/ui/tui.rs index 13b4f2c..620e9f8 100644 --- a/src/ui/tui.rs +++ b/src/ui/tui.rs @@ -20,10 +20,13 @@ use crate::{ ziplist, ORIGINAL_TERMIOS, VERSION, }; use std::{ + cell::RefCell, char, cmp::{min, Ordering}, + collections::HashMap, io::{stdin, stdout, Read, Stdout, Write}, panic, + rc::Rc, sync::mpsc::Sender, thread::{spawn, JoinHandle}, time::Instant, @@ -47,11 +50,15 @@ pub struct Tui { screen_cols: usize, status_message: String, last_status: Instant, + // Box elements for rendering window borders vstr: String, xstr: String, tstr: String, hvh: String, vh: String, + // Cache of the ANSI escape code strings required for each fully qualified tree-sitter + // highlighting tag. See render_line for details on how the cache is used. + style_cache: Rc>>, } impl Default for Tui { @@ -79,13 +86,14 @@ impl Tui { xstr: String::new(), hvh: String::new(), vh: String::new(), + style_cache: Default::default(), }; - tui.update_box_elements(); + tui.update_cached_elements(); tui } - fn update_box_elements(&mut self) { + fn update_cached_elements(&mut self) { let cs = &config_handle!().colorscheme; let vstr = box_draw_str(VLINE, cs); let hstr = box_draw_str(HLINE, cs); @@ -94,6 +102,7 @@ impl Tui { self.hvh = format!("{hstr}{vstr}{hstr}"); self.vh = format!("{vstr}{hstr}"); self.vstr = vstr; + self.style_cache.borrow_mut().clear(); } fn render_banner(&self, screen_rows: usize, cs: &ColorScheme) -> Vec { @@ -202,17 +211,18 @@ impl Tui { } else { cs.bg }; - let styles = Styles { + let style_str = Styles { fg: Some(cs.fg), bg: Some(bg), ..Default::default() - }; + } + .to_string(); let mut rline = String::new(); let mut cols = 0; render_slice( slice, - &styles, + &style_str, self.screen_cols, tabstop, &mut 0, @@ -288,7 +298,7 @@ impl UserInterface for Tui { fn state_change(&mut self, change: StateChange) { match change { - StateChange::ConfigUpdated => self.update_box_elements(), + StateChange::ConfigUpdated => self.update_cached_elements(), StateChange::StatusMessage { msg } => { self.status_message = msg; self.last_status = Instant::now(); @@ -402,7 +412,15 @@ impl<'a> WinsIter<'a> { .iter() .map(|(is_focus, col)| { let rng = if is_focus { load_exec_range } else { None }; - ColIter::new(col, layout, rng, screen_rows, tabstop, cs) + ColIter::new( + col, + layout, + rng, + screen_rows, + tabstop, + cs, + tui.style_cache.clone(), + ) }) .collect(); let buf = Vec::with_capacity(col_iters.len()); @@ -442,6 +460,7 @@ struct ColIter<'a> { current: Option>, layout: &'a Layout, cs: &'a ColorScheme, + style_cache: Rc>>, load_exec_range: Option<(bool, Range)>, screen_rows: usize, tabstop: usize, @@ -457,12 +476,14 @@ impl<'a> ColIter<'a> { screen_rows: usize, tabstop: usize, cs: &'a ColorScheme, + style_cache: Rc>>, ) -> Self { ColIter { inner: col.wins.iter(), current: None, layout, cs, + style_cache, load_exec_range, screen_rows, tabstop, @@ -493,6 +514,7 @@ impl<'a> ColIter<'a> { gb: &b.txt, w, cs: self.cs, + style_cache: self.style_cache.clone(), }) } } @@ -530,6 +552,7 @@ struct WinIter<'a> { gb: &'a GapBuffer, w: &'a Window, cs: &'a ColorScheme, + style_cache: Rc>>, } impl Iterator for WinIter<'_> { @@ -574,7 +597,8 @@ impl Iterator for WinIter<'_> { self.w.view.col_off, self.n_cols - padding, self.tabstop, - self.cs + self.cs, + &mut self.style_cache ), width = self.w_lnum ) @@ -621,7 +645,7 @@ fn render_pending(keys: &[Input]) -> String { #[inline] fn render_slice( slice: Slice<'_>, - styles: &Styles, + style_str: &str, max_cols: usize, tabstop: usize, to_skip: &mut usize, @@ -650,21 +674,7 @@ fn render_slice( } } - if let Some(fg) = styles.fg { - buf.push_str(&Style::Fg(fg).to_string()); - } - if let Some(bg) = styles.bg { - buf.push_str(&Style::Bg(bg).to_string()); - } - if styles.bold { - buf.push_str(&Style::Bold.to_string()); - } - if styles.italic { - buf.push_str(&Style::Italic.to_string()); - } - if styles.underline { - buf.push_str(&Style::Underline.to_string()); - } + buf.push_str(style_str); if let Some(n) = spaces { buf.extend(std::iter::repeat_n(' ', n)); @@ -706,17 +716,39 @@ fn render_line( max_cols: usize, tabstop: usize, cs: &ColorScheme, + style_cache: &mut Rc>>, ) -> String { let mut buf = String::new(); let mut to_skip = col_off; let mut cols = 0; for tk in it { - let styles = cs.styles_for(tk.tag()); - let slice = tk.as_slice(gb); + // In the common case we have the styles for each tag already cached, so we take the hit on + // allocating the cache key and looking it up a second time when we insert into the cache. + // This allows us to avoid having to allocate the key on every lookup in order to make use + // of the entry API. + // + // The cache styles are also stored against the original tag rather so they can be looked + // up directly each time they are used, rather than need to to traverse the fallback path + // as done in ColorScheme::styles_for. + // + // We always assume that it safe to borrow the style_cache mutably at this point as we are + // only expecting this function to be called as part of a render pass where the clones of + // the style_cache Rc are held in different iterators that are processed sequentially in a + // single thread. + let mut guard = style_cache.borrow_mut(); + let style_str = match guard.get(tk.tag) { + Some(s) => s, + None => { + let s = cs.styles_for(tk.tag).to_string(); + guard.insert(tk.tag.to_string(), s); + guard.get(tk.tag).unwrap() + } + }; + render_slice( - slice, - styles, + tk.as_slice(gb), + style_str, max_cols, tabstop, &mut to_skip, From 1bfa2084965b90a4055b75f3b4ce87355c5a538b Mon Sep 17 00:00:00 2001 From: Innes Anderson-Morrison Date: Sun, 16 Feb 2025 09:31:55 +0000 Subject: [PATCH 2/4] fix: handling of skipped tokens due to horizontal scrolling --- src/term.rs | 1 + src/ui/layout.rs | 4 +- src/ui/tui.rs | 114 +++++++++++++++++++++++++++-------------------- 3 files changed, 68 insertions(+), 51 deletions(-) diff --git a/src/term.rs b/src/term.rs index ea127d6..f69f5b8 100644 --- a/src/term.rs +++ b/src/term.rs @@ -20,6 +20,7 @@ const ENABLE_MOUSE_SUPPORT: &str = "\x1b[?1000h\x1b[?1002h\x1b[?1015h\x1b[?1006h const DISABLE_MOUSE_SUPPORT: &str = "\x1b[?1006l\x1b[?1015l\x1b[?1002l\x1b[?1000l"; const ENABLE_ALTERNATE_SCREEN: &str = "\x1b[?1049h"; const DISABLE_ALTERNATE_SCREEN: &str = "\x1b[?1049l"; +pub const RESET_STYLE: &str = "\x1b[m"; /// Used for storing and checking whether or not we've received a signal that our window /// size has changed. diff --git a/src/ui/layout.rs b/src/ui/layout.rs index a0ea798..7907c76 100644 --- a/src/ui/layout.rs +++ b/src/ui/layout.rs @@ -789,7 +789,7 @@ impl View { } if y >= self.row_off + screen_rows { - self.row_off = y - screen_rows + 1; + self.row_off = y + 1 - screen_rows; } if self.rx < self.col_off { @@ -797,7 +797,7 @@ impl View { } if self.rx >= self.col_off + screen_cols - w_sgncol { - self.col_off = self.rx - screen_cols + w_sgncol + 1; + self.col_off = self.rx + w_sgncol + 1 - screen_cols; } } diff --git a/src/ui/tui.rs b/src/ui/tui.rs index 620e9f8..405d77d 100644 --- a/src/ui/tui.rs +++ b/src/ui/tui.rs @@ -1,6 +1,6 @@ //! A terminal UI for ad use crate::{ - buffer::{Buffer, GapBuffer, Slice}, + buffer::{Buffer, Chars, GapBuffer}, config::ColorScheme, config_handle, die, dot::Range, @@ -11,8 +11,9 @@ use crate::{ term::{ clear_screen, enable_alternate_screen, enable_mouse_support, enable_raw_mode, get_termios, get_termsize, register_signal_handler, win_size_changed, CurShape, Cursor, Style, Styles, + RESET_STYLE, }, - ts::{LineIter, TokenIter}, + ts::{LineIter, RangeToken}, ui::{ layout::{Column, Window}, Layout, StateChange, UserInterface, @@ -25,6 +26,7 @@ use std::{ cmp::{min, Ordering}, collections::HashMap, io::{stdin, stdout, Read, Stdout, Write}, + iter::{repeat_n, Peekable}, panic, rc::Rc, sync::mpsc::Sender, @@ -211,21 +213,21 @@ impl Tui { } else { cs.bg }; - let style_str = Styles { + + let mut cols = 0; + let mut chars = slice.chars().peekable(); + let mut rline = Styles { fg: Some(cs.fg), bg: Some(bg), ..Default::default() } .to_string(); - let mut rline = String::new(); - let mut cols = 0; - render_slice( - slice, - &style_str, + render_chars( + &mut chars, + None, self.screen_cols, tabstop, - &mut 0, &mut cols, &mut rline, ); @@ -643,41 +645,46 @@ fn render_pending(keys: &[Input]) -> String { } #[inline] -fn render_slice( - slice: Slice<'_>, - style_str: &str, - max_cols: usize, +fn skip_token_chars( + chars: &mut Peekable>, tabstop: usize, to_skip: &mut usize, - cols: &mut usize, - buf: &mut String, -) { - let mut chars = slice.chars().peekable(); - let mut spaces = None; +) -> Option { + for ch in chars.by_ref() { + let w = if ch == '\t' { + tabstop + } else { + UnicodeWidthChar::width(ch).unwrap_or(1) + }; - if *to_skip > 0 { - for ch in chars.by_ref() { - let w = if ch == '\t' { - tabstop - } else { - UnicodeWidthChar::width(ch).unwrap_or(1) - }; + match (*to_skip).cmp(&w) { + Ordering::Less => { + *to_skip = 0; + return Some(w - *to_skip); + } - match (*to_skip).cmp(&w) { - Ordering::Less => { - spaces = Some(w - *to_skip); - break; - } - Ordering::Equal => break, - Ordering::Greater => *to_skip -= w, + Ordering::Equal => { + *to_skip = 0; + break; } + + Ordering::Greater => *to_skip -= w, } } - buf.push_str(style_str); + None +} +fn render_chars( + chars: &mut Peekable>, + spaces: Option, + max_cols: usize, + tabstop: usize, + cols: &mut usize, + buf: &mut String, +) { if let Some(n) = spaces { - buf.extend(std::iter::repeat_n(' ', n)); + buf.extend(repeat_n(' ', n)); *cols = n; } @@ -696,7 +703,7 @@ fn render_slice( // Tab is just a control character that moves the cursor rather than // replacing the previous buffer content so we need to explicitly // insert spaces instead. - buf.extend(std::iter::repeat_n(' ', tabstop)); + buf.extend(repeat_n(' ', tabstop)); } else { buf.push(ch); } @@ -706,12 +713,12 @@ fn render_slice( } } - buf.push_str(&Style::Reset.to_string()); + buf.push_str(RESET_STYLE); } -fn render_line( - gb: &GapBuffer, - it: TokenIter<'_>, +fn render_line<'a>( + gb: &'a GapBuffer, + it: impl Iterator>, col_off: usize, max_cols: usize, tabstop: usize, @@ -723,6 +730,18 @@ fn render_line( let mut cols = 0; for tk in it { + let slice = tk.as_slice(gb); + let mut chars = slice.chars().peekable(); + let spaces = if to_skip > 0 { + let spaces = skip_token_chars(&mut chars, tabstop, &mut to_skip); + if to_skip > 0 { + continue; + } + spaces + } else { + None + }; + // In the common case we have the styles for each tag already cached, so we take the hit on // allocating the cache key and looking it up a second time when we insert into the cache. // This allows us to avoid having to allocate the key on every lookup in order to make use @@ -746,20 +765,17 @@ fn render_line( } }; - render_slice( - tk.as_slice(gb), - style_str, - max_cols, - tabstop, - &mut to_skip, - &mut cols, - &mut buf, - ); + buf.push_str(style_str); + render_chars(&mut chars, spaces, max_cols, tabstop, &mut cols, &mut buf); + + if cols == max_cols { + break; + } } if cols < max_cols { buf.push_str(&Style::Bg(cs.bg).to_string()); - buf.extend(std::iter::repeat_n(' ', max_cols - cols)); + buf.extend(repeat_n(' ', max_cols - cols)); } buf From ae9de090d8f1b52faf7dfd7c0701f49d8a024758 Mon Sep 17 00:00:00 2001 From: Innes Anderson-Morrison Date: Sun, 16 Feb 2025 10:26:02 +0000 Subject: [PATCH 3/4] testing the new render_line logic (still need unicode tests) --- src/ts.rs | 4 ++-- src/ui/tui.rs | 61 +++++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 61 insertions(+), 4 deletions(-) diff --git a/src/ts.rs b/src/ts.rs index e05e43a..f4918d6 100644 --- a/src/ts.rs +++ b/src/ts.rs @@ -398,8 +398,8 @@ impl Tokenizer { /// Byte offsets within a Buffer #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] pub(crate) struct ByteRange { - from: usize, - to: usize, + pub(crate) from: usize, + pub(crate) to: usize, } impl ByteRange { diff --git a/src/ui/tui.rs b/src/ui/tui.rs index 405d77d..59d537d 100644 --- a/src/ui/tui.rs +++ b/src/ui/tui.rs @@ -659,8 +659,9 @@ fn skip_token_chars( match (*to_skip).cmp(&w) { Ordering::Less => { + let spaces = Some(w - *to_skip); *to_skip = 0; - return Some(w - *to_skip); + return spaces; } Ordering::Equal => { @@ -734,7 +735,7 @@ fn render_line<'a>( let mut chars = slice.chars().peekable(); let spaces = if to_skip > 0 { let spaces = skip_token_chars(&mut chars, tabstop, &mut to_skip); - if to_skip > 0 { + if to_skip > 0 || (chars.peek().is_none() && spaces.is_none()) { continue; } spaces @@ -877,6 +878,7 @@ fn try_read_input(stdin: &mut impl Read) -> Option { #[cfg(test)] mod tests { use super::*; + use crate::ts::{ByteRange, TK_DEFAULT}; use simple_test_case::test_case; use std::{char::REPLACEMENT_CHARACTER, io}; @@ -895,4 +897,59 @@ mod tests { assert_eq!(&chars, expected); } + + fn rt(tag: &'static str, from: usize, to: usize) -> RangeToken<'static> { + RangeToken { + tag, + r: ByteRange { from, to }, + } + } + + // The !| characters here are the dummy style strings in the style_cache + // The $# characters are replaced with RESET_STYLE and bg color respectively + #[test_case(0, 14, "foo\tbar baz", "!foo$| $!bar$| $!baz$# "; "full line padded to max cols")] + #[test_case(0, 12, "foo\tbar baz", "!foo$| $!bar$| $!baz$"; "full line")] + #[test_case(1, 11, "foo\tbar baz", "!oo$| $!bar$| $!baz$"; "skipping first character")] + #[test_case(3, 9, "foo\tbar baz", "| $!bar$| $!baz$"; "skipping first token")] + #[test_case(4, 8, "foo\tbar baz", "| $!bar$| $!baz$"; "skipping part way through a tab")] + #[test] + fn render_line_correctly_skips_tokens( + col_off: usize, + max_cols: usize, + s: &str, + expected_template: &str, + ) { + let gb = GapBuffer::from(s); + let range_tokens = vec![ + rt("a", 0, 3), + rt(TK_DEFAULT, 3, 4), + rt("a", 4, 7), + rt(TK_DEFAULT, 7, 8), + rt("a", 8, 11), + ]; + + let cs = ColorScheme::default(); + let style_cache: HashMap = [ + ("a".to_owned(), "!".to_owned()), + (TK_DEFAULT.to_owned(), "|".to_owned()), + ] + .into_iter() + .collect(); + + let s = render_line( + &gb, + range_tokens.into_iter(), + col_off, + max_cols, + 2, + &cs, + &mut Rc::new(RefCell::new(style_cache)), + ); + + let expected = expected_template + .replace("$", RESET_STYLE) + .replace("#", &Style::Bg(cs.bg).to_string()); + + assert_eq!(s, expected); + } } From d9782381ffe72462cd5086afe7f35432407fd982 Mon Sep 17 00:00:00 2001 From: Innes Anderson-Morrison Date: Sun, 16 Feb 2025 11:29:56 +0000 Subject: [PATCH 4/4] unicode tests for the new rendering logic --- src/ui/tui.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/ui/tui.rs b/src/ui/tui.rs index 59d537d..0b1aeb3 100644 --- a/src/ui/tui.rs +++ b/src/ui/tui.rs @@ -912,6 +912,12 @@ mod tests { #[test_case(1, 11, "foo\tbar baz", "!oo$| $!bar$| $!baz$"; "skipping first character")] #[test_case(3, 9, "foo\tbar baz", "| $!bar$| $!baz$"; "skipping first token")] #[test_case(4, 8, "foo\tbar baz", "| $!bar$| $!baz$"; "skipping part way through a tab")] + #[test_case(0, 10, "世\t界 foo", "!世$| $!界$| $!foo$"; "unicode full line")] + #[test_case(0, 12, "世\t界 foo", "!世$| $!界$| $!foo$# "; "unicode full line padded to max cols")] + // In the case where we skip part way through a unicode character we still apply the tag + // styling to the spaces we insert to pad to the correct offset rather than replacing it + // with default styling instead + #[test_case(1, 9, "世\t界 foo", "! $| $!界$| $!foo$"; "unicode skipping first column of multibyte char")] #[test] fn render_line_correctly_skips_tokens( col_off: usize,