diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a61564..3dacb63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Preview ownership uses shared snapshots.** The TUI and search index share one catalog allocation. Live and trash previews borrow snippets instead of copying their content each frame. +- **Preview tabs use four-cell stops.** Fragment, README, and note previews + expand tabs after syntax highlighting. Wrapping and mouse selections match + the visible columns. Full-content copy preserves source tabs. ### Deprecated diff --git a/ROADMAP.md b/ROADMAP.md index f260d8b..be2f351 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -19,17 +19,6 @@ the rejected options and the settled shape should it ever be built, plus one separable read-only idea (keyboard selection and yank, closing the gap where only mouse users can copy part of a fragment). -### Tab expansion in the preview - -- [ ] Expand tabs before syntax highlighting using an explicit tab-stop policy; - ratatui filters the control character itself, so tabs currently collapse to one - visual column. Independent of the grapheme-cluster measurement that shipped - alongside it. - -Implementation handoff, with the verified surface inventory, the settled -decisions, and corrections to the details above: -`docs/plans/preview-ownership-and-tabs.md`. - ### Search index follow-up - [ ] Decide whether `MemoryIndex` needs an inverted index over titles and tags. @@ -267,7 +256,11 @@ there is no popularity threshold for new packages. - **Shared preview ownership** — `App` and `MemoryIndex` share one `Arc`. Live and trash previews borrow snippets or clone `Arc`s, so frames do not copy snippet content. - (`docs/plans/preview-ownership-and-tabs.md`, Track 1) + (`docs/plans/completed/preview-ownership-and-tabs.md`, Track 1) +- **Tab expansion in previews** — Tabs in fragment, README, and note previews + expand to four-cell stops after syntax highlighting. Display-cell widths keep + CJK wrapping and mouse selection aligned. Full-content copy preserves source + tabs. (`docs/plans/completed/preview-ownership-and-tabs.md`, Track 2) ### Library portability diff --git a/src/tui/highlight.rs b/src/tui/highlight.rs index de5535b..51318ed 100644 --- a/src/tui/highlight.rs +++ b/src/tui/highlight.rs @@ -5,13 +5,17 @@ use syntect::easy::HighlightLines; use syntect::highlighting::{FontStyle, Theme}; use syntect::parsing::SyntaxSet; use syntect::util::LinesWithEndings; +use unicode_segmentation::UnicodeSegmentation; use crate::domain::Fragment; use crate::error::{Result, SnipError}; use crate::render::find_syntax; +use super::selection::cluster_width; use super::theme::TuiTheme; +const PREVIEW_TAB_WIDTH: u16 = 4; + pub struct Highlighter { syntaxes: SyntaxSet, theme: Theme, @@ -29,6 +33,10 @@ impl Highlighter { Ok(()) } + /// Tabs are expanded after highlighting, so syntect always parses the + /// original bytes. No bundled syntax is known to need that — a Makefile + /// recipe highlights identically either way — but the order costs nothing + /// and keeps a tab-sensitive syntax from silently breaking later. pub fn fragment(&self, fragment: &Fragment) -> Result> { let syntax = find_syntax(&self.syntaxes, &fragment.language, &fragment.file); let mut highlighter = HighlightLines::new(syntax, &self.theme); @@ -64,7 +72,9 @@ impl Highlighter { ) }) .collect::>(); - lines.push(Line::from(spans)); + let mut line = Line::from(spans); + expand_preview_tabs(&mut line); + lines.push(line); } if lines.is_empty() { lines.push(Line::default()); @@ -122,9 +132,38 @@ pub fn markdown(markdown: &str, theme: TuiTheme) -> Text<'static> { while lines.len() > 1 && lines.last().is_some_and(|line| line.spans.is_empty()) { lines.pop(); } + for line in &mut lines { + expand_preview_tabs(line); + } Text::from(lines) } +fn expand_preview_tabs(line: &mut Line<'static>) { + let mut column = 0_u16; + for span in &mut line.spans { + if span.content.contains('\t') { + let mut expanded = String::with_capacity(span.content.len()); + for cluster in span.content.graphemes(true) { + if cluster == "\t" { + let padding = PREVIEW_TAB_WIDTH - (column % PREVIEW_TAB_WIDTH); + expanded.extend(std::iter::repeat_n(' ', usize::from(padding))); + column = column.saturating_add(padding); + } else { + expanded.push_str(cluster); + column = column.saturating_add(cluster_width(cluster)); + } + } + span.content = expanded.into(); + } else { + column = span + .content + .graphemes(true) + .map(cluster_width) + .fold(column, u16::saturating_add); + } + } +} + fn push_span(lines: &mut Vec>, value: &str, style: Style) { let mut parts = value.split('\n').peekable(); while let Some(part) = parts.next() { @@ -148,6 +187,30 @@ mod tests { use std::path::PathBuf; use uuid::Uuid; + fn fragment(language: &str, file: &str, content: &str) -> Fragment { + Fragment { + manifest: FragmentManifest { + id: Uuid::new_v4(), + title: "Test".to_owned(), + language: language.to_owned(), + file: file.to_owned(), + note: None, + source_language: None, + extra: toml::Table::new(), + }, + content: content.to_owned(), + note_content: None, + absolute_path: PathBuf::from(file), + } + } + + fn line_text(line: &Line<'_>) -> String { + line.spans + .iter() + .map(|span| span.content.as_ref()) + .collect() + } + #[test] fn markdown_maps_emphasis_and_rust_gets_non_default_styles() { let source = crate::theme::load("dark-default").unwrap(); @@ -161,20 +224,11 @@ mod tests { .any(|span| span.style != Style::default()) ); - let fragment = Fragment { - manifest: FragmentManifest { - id: Uuid::new_v4(), - title: "Rust".to_owned(), - language: "rust".to_owned(), - file: "fragments/main.rs".to_owned(), - note: None, - source_language: None, - extra: toml::Table::new(), - }, - content: "fn main() { let answer = 42; }\n".to_owned(), - note_content: None, - absolute_path: PathBuf::from("main.rs"), - }; + let fragment = fragment( + "rust", + "fragments/main.rs", + "fn main() { let answer = 42; }\n", + ); let highlighted = Highlighter::new(&source) .unwrap() .fragment(&fragment) @@ -187,4 +241,49 @@ mod tests { .any(|span| span.style != Style::default()) ); } + + #[test] + fn tabs_expand_to_four_cell_stops() { + for (input, expected) in [ + ("\tfoo", " foo"), + ("a\tb", "a b"), + ("abcd\te", "abcd e"), + ("中\tx", "中 x"), + ] { + let mut line = Line::raw(input.to_owned()); + expand_preview_tabs(&mut line); + assert_eq!(line_text(&line), expected, "{input:?}"); + } + } + + #[test] + fn tab_stops_carry_across_span_boundaries() { + let red = Style::default().fg(Color::Red); + let blue = Style::default().fg(Color::Blue); + let mut line = Line::from(vec![Span::styled("a", red), Span::styled("\tb", blue)]); + + expand_preview_tabs(&mut line); + + assert_eq!(line_text(&line), "a b"); + assert_eq!(line.spans[0].style, red); + assert_eq!(line.spans[1].style, blue); + } + + #[test] + fn makefile_recipe_expands_its_leading_tab_and_stays_highlighted() { + let source = crate::theme::load("dark-default").unwrap(); + let highlighted = Highlighter::new(&source) + .unwrap() + .fragment(&fragment("makefile", "Makefile", "all:\n\t@echo ok\n")) + .unwrap(); + let recipe = &highlighted.lines[1]; + + assert_eq!(line_text(recipe), " @echo ok"); + assert!( + recipe + .spans + .iter() + .any(|span| span.style != Style::default()) + ); + } } diff --git a/src/tui/preview/cache.rs b/src/tui/preview/cache.rs index 52578cd..de60e73 100644 --- a/src/tui/preview/cache.rs +++ b/src/tui/preview/cache.rs @@ -163,6 +163,15 @@ mod tests { .collect() } + fn assert_no_tabs(text: &Text<'static>) { + assert!( + text.lines + .iter() + .flat_map(|line| &line.spans) + .all(|span| !span.content.contains('\t')) + ); + } + #[test] fn fragment_document_omits_readme() { let (highlighter, theme) = highlighter(); @@ -205,6 +214,53 @@ mod tests { } } + #[test] + fn built_fragment_and_markdown_documents_expand_tabs() { + let (highlighter, theme) = highlighter(); + let mut snippet = snippet(Some("```\n\treadme\n```\n")); + snippet.loaded_fragments[0].content = "\tfragment\n".to_owned(); + snippet.loaded_fragments[0].note_content = Some("```\n\tnote\n```\n".to_owned()); + + let fragment = build(&snippet, PreviewTarget::Fragment(0), &highlighter, theme).unwrap(); + let PreviewDocument::Fragment { note, body } = fragment else { + panic!("a fragment target must build a fragment document"); + }; + assert_no_tabs(&body); + assert_no_tabs(note.as_ref().expect("fixture has a note")); + assert!(plain(&body).contains(" fragment")); + assert!(plain(note.as_ref().unwrap()).contains(" note")); + + let readme = build(&snippet, PreviewTarget::Readme, &highlighter, theme).unwrap(); + let PreviewDocument::Readme(readme) = readme else { + panic!("a README target must build a README document"); + }; + assert_no_tabs(&readme); + assert!(plain(&readme).contains(" readme")); + } + + #[test] + fn cjk_before_a_tab_wraps_at_the_expanded_display_column() { + let (highlighter, theme) = highlighter(); + let mut snippet = snippet(None); + snippet.loaded_fragments[0].content = "中\tx\n".to_owned(); + snippet.loaded_fragments[0].note_content = None; + let document = build(&snippet, PreviewTarget::Fragment(0), &highlighter, theme).unwrap(); + let wrapped = wrap_preview(compose_preview(document, false, theme, 4), 4, false); + let lines = wrapped + .text + .lines + .iter() + .map(|line| { + line.spans + .iter() + .map(|span| span.content.as_ref()) + .collect::() + }) + .collect::>(); + + assert_eq!(lines, ["中 ", "x"]); + } + #[test] fn cache_misses_across_targets() { let (highlighter, theme) = highlighter(); diff --git a/tests/tui.rs b/tests/tui.rs index d7f1f50..52ad46f 100644 --- a/tests/tui.rs +++ b/tests/tui.rs @@ -1633,6 +1633,48 @@ fn preview_drag_selection_copies_text_without_line_number_gutter() { assert_eq!(buffer.cell((x + 3, y)).unwrap().bg, app.theme.selection_bg); } +#[test] +fn preview_expands_tabs_but_copy_content_preserves_source_tabs() { + let temporary = tempfile::tempdir().unwrap(); + let library = Library::init(&temporary.path().join("Tabs.sniplib"), None).unwrap(); + create_snippet( + &library, + &CreateOptions { + title: "Tabs".to_owned(), + language: "makefile".to_owned(), + content: "\tfoo\n".to_owned(), + ..CreateOptions::default() + }, + ) + .unwrap(); + let mut app = App::new(library, &AppConfig::default()).unwrap(); + let backend = TestBackend::new(100, 20); + let mut terminal = Terminal::new(backend).unwrap(); + terminal + .draw(|frame| snip::tui::ui::draw(frame, &mut app)) + .unwrap(); + + let x = app.layout.preview_content.x; + let y = app.layout.preview_content.y; + assert!(row_text_from(terminal.backend().buffer(), y, x).starts_with("1│ foo")); + + let _ = app.handle_mouse(mouse(MouseEventKind::Down(MouseButton::Left), x, y)); + let _ = app.handle_mouse(mouse(MouseEventKind::Drag(MouseButton::Left), x + 10, y)); + let effects = app.handle_mouse(mouse(MouseEventKind::Up(MouseButton::Left), x + 10, y)); + let Effect::CopyToClipboard { text, label } = &effects[0] else { + panic!("expected automatic clipboard effect"); + }; + assert_eq!(text, " foo"); + assert_eq!(label, "selection"); + + let effects = app.run_command(CommandId::CopyContent); + let Effect::CopyToClipboard { text, label } = &effects[0] else { + panic!("expected content clipboard effect"); + }; + assert_eq!(text, "\tfoo\n"); + assert_eq!(label, "fragment"); +} + #[test] fn help_overlay_accepts_mouse_wheel_scrolling() { let (_temporary, library, _first_id, _second_id) = fixture();