From f3f07753ca3db9321a8074d4448afeeb6e293245 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sun, 20 Sep 2026 17:49:12 +0000 Subject: [PATCH] Extract shared text selection helpers for the TUI (#46857) ## What changed Move selection units, word and logical-line boundaries, click counting, and copy-key detection into `text_selection`, and update the transcript view to use them. Clamp selection offsets to valid UTF-8 boundaries and exclude key-release events from copy-key detection. Make `TranscriptView::cancel_search` accessible throughout the crate. ## Testing Update existing text-layout tests to exercise the shared selection ranges for wrapped lines, combining characters, and empty logical lines. GitOrigin-RevId: 6a8b44b4aa97fe2965a5e330121dc22fddc335b5 --- codex-rs/tui/src/lib.rs | 1 + codex-rs/tui/src/text_selection.rs | 76 +++++++++++++++++++ codex-rs/tui/src/transcript_view/input.rs | 23 +----- codex-rs/tui/src/transcript_view/search.rs | 2 +- codex-rs/tui/src/transcript_view/selection.rs | 28 +------ codex-rs/tui/src/transcript_view/text.rs | 20 ----- .../tui/src/transcript_view/text_tests.rs | 12 ++- 7 files changed, 93 insertions(+), 69 deletions(-) create mode 100644 codex-rs/tui/src/text_selection.rs diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 4e2490e5fb..3161e6e2c4 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -216,6 +216,7 @@ mod terminal_probe; mod terminal_title; mod terminal_visualization_instructions; mod text_formatting; +mod text_selection; mod theme_picker; mod thread_color; mod thread_transcript; diff --git a/codex-rs/tui/src/text_selection.rs b/codex-rs/tui/src/text_selection.rs new file mode 100644 index 0000000000..cdd3491109 --- /dev/null +++ b/codex-rs/tui/src/text_selection.rs @@ -0,0 +1,76 @@ +//! Shared selection gestures and source-text boundaries; each view owns its hit testing and state. + +use crossterm::event::KeyCode; +use crossterm::event::KeyEvent; +use crossterm::event::KeyEventKind; +use crossterm::event::KeyModifiers; +use std::ops::Range; +use std::time::Duration; +use std::time::Instant; +use unicode_segmentation::UnicodeSegmentation; + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum SelectionUnit { + Character, + Word, + Line, +} + +impl SelectionUnit { + pub(crate) fn from_clicks(clicks: u8) -> Self { + match clicks { + 2 => Self::Word, + 3 => Self::Line, + _ => Self::Character, + } + } + + pub(crate) fn range(self, text: &str, offset: usize) -> Range { + let offset = text.floor_char_boundary(offset.min(text.len())); + match self { + Self::Character => offset..offset, + Self::Word => text + .split_word_bound_indices() + .map(|(start, word)| start..start + word.len()) + .find(|range| range.contains(&offset)) + .unwrap_or(text.len()..text.len()), + // Logical lines include their hard newline, regardless of visual wrapping. + Self::Line => { + let start = text[..offset] + .rfind('\n') + .map_or(/*default*/ 0, |newline| newline + 1); + let end = text[offset..] + .find('\n') + .map_or(text.len(), |newline| offset + newline + 1); + start..end + } + } + } +} + +pub(crate) fn click_count( + last_click: &mut Option<(Instant, u16, u16, u8)>, + column: u16, + row: u16, +) -> u8 { + let now = Instant::now(); + let clicks = last_click + .filter(|(at, x, y, _)| { + now.duration_since(*at) < Duration::from_millis(/*millis*/ 400) + && *x == column + && *y == row + }) + .map_or(/*default*/ 1, |(_, _, _, clicks)| clicks % 3 + 1); + *last_click = Some((now, column, row, clicks)); + clicks +} + +pub(crate) fn is_copy_key(key: KeyEvent) -> bool { + // Kitty reports Cmd+C as Super+C, including over SSH. Crossterm may report + // Ctrl+Shift+C as uppercase C with only Control set. + let (code, modifiers) = crate::key_hint::normalize_key_parts(key.code, key.modifiers); + key.kind != KeyEventKind::Release + && code == KeyCode::Char('c') + && (matches!(modifiers, KeyModifiers::CONTROL | KeyModifiers::SUPER) + || modifiers == (KeyModifiers::CONTROL | KeyModifiers::SHIFT)) +} diff --git a/codex-rs/tui/src/transcript_view/input.rs b/codex-rs/tui/src/transcript_view/input.rs index 834cb57768..4a9351c288 100644 --- a/codex-rs/tui/src/transcript_view/input.rs +++ b/codex-rs/tui/src/transcript_view/input.rs @@ -9,8 +9,6 @@ use crossterm::event::MouseButton; use crossterm::event::MouseEvent; use crossterm::event::MouseEventKind; use ratatui::layout::Position as ScreenPosition; -use std::time::Duration; -use std::time::Instant; use super::*; @@ -230,14 +228,7 @@ impl TranscriptView { key: KeyEvent, cells: &[Arc], ) -> Option { - // Kitty keyboard reporting delivers macOS Cmd+C as Super+C, including over SSH to - // non-macOS hosts. Ghostty forwards it when there is no terminal-native selection. - // Crossterm can encode Ctrl+Shift+C as uppercase C with only Control set. - let (code, modifiers) = crate::key_hint::normalize_key_parts(key.code, key.modifiers); - if (matches!(modifiers, KeyModifiers::CONTROL | KeyModifiers::SUPER) - && code == KeyCode::Char('c')) - || (modifiers == (KeyModifiers::CONTROL | KeyModifiers::SHIFT) - && code == KeyCode::Char('c')) + if crate::text_selection::is_copy_key(key) || (key.modifiers == KeyModifiers::NONE && key.code == KeyCode::Enter) { return Some( @@ -316,16 +307,8 @@ impl TranscriptView { .link_at(visible.row, event.column.saturating_sub(self.area.x)) .map(ViewAction::OpenLink); } - let now = Instant::now(); - let clicks = self - .last_click - .filter(|(at, column, row, _)| { - now.duration_since(*at) < Duration::from_millis(/*millis*/ 400) - && *column == event.column - && *row == event.row - }) - .map_or(/*default*/ 1, |(_, _, _, clicks)| clicks % 3 + 1); - self.last_click = Some((now, event.column, event.row, clicks)); + let clicks = + crate::text_selection::click_count(&mut self.last_click, event.column, event.row); if clicks >= 2 && visible .layout diff --git a/codex-rs/tui/src/transcript_view/search.rs b/codex-rs/tui/src/transcript_view/search.rs index 3c9503482a..047f821923 100644 --- a/codex-rs/tui/src/transcript_view/search.rs +++ b/codex-rs/tui/src/transcript_view/search.rs @@ -215,7 +215,7 @@ impl TranscriptView { } /// Restore the pre-search presentation before an explicit return or cancellation. - pub(super) fn cancel_search(&mut self) { + pub(crate) fn cancel_search(&mut self) { if !self.search.active { return; } diff --git a/codex-rs/tui/src/transcript_view/selection.rs b/codex-rs/tui/src/transcript_view/selection.rs index 1cd6d3698e..7dd259716c 100644 --- a/codex-rs/tui/src/transcript_view/selection.rs +++ b/codex-rs/tui/src/transcript_view/selection.rs @@ -6,6 +6,7 @@ use ratatui::layout::Position as ScreenPosition; use unicode_segmentation::UnicodeSegmentation; use super::*; +use crate::text_selection::SelectionUnit; pub(super) struct Selection { pub(super) snapshot: ViewSnapshot, @@ -20,23 +21,6 @@ pub(super) struct Selection { preferred_column: Option, } -#[derive(Clone, Copy)] -enum SelectionUnit { - Character, - Word, - Line, -} - -impl SelectionUnit { - fn range(self, layout: &TextLayout, offset: usize) -> std::ops::Range { - match self { - Self::Character => offset..offset, - Self::Word => layout.word_range(offset), - Self::Line => layout.line_range(offset), - } - } -} - impl TranscriptView { pub(super) fn begin_selection( &mut self, @@ -49,12 +33,8 @@ impl TranscriptView { return; }; self.cancel_beginning(); - let unit = match clicks { - 2 => SelectionUnit::Word, - 3 => SelectionUnit::Line, - _ => SelectionUnit::Character, - }; - let range = unit.range(&layout, anchor.offset); + let unit = SelectionUnit::from_clicks(clicks); + let range = unit.range(layout.text(), anchor.offset); let snapshot = self.capture_snapshot(cells); self.held_reading = None; let start = Anchor { @@ -130,7 +110,7 @@ impl TranscriptView { return; }; let cells = Arc::clone(&selection.snapshot.cells); - let range = selection.unit.range(&layout, end.offset); + let range = selection.unit.range(layout.text(), end.offset); let origin = selection.origin; let backwards = (self.resolve(&cells, end), end.offset) < (self.resolve(&cells, origin.0), origin.0.offset); diff --git a/codex-rs/tui/src/transcript_view/text.rs b/codex-rs/tui/src/transcript_view/text.rs index fb2ac357fb..fe87555760 100644 --- a/codex-rs/tui/src/transcript_view/text.rs +++ b/codex-rs/tui/src/transcript_view/text.rs @@ -282,14 +282,6 @@ impl TextLayout { } } - pub(super) fn word_range(&self, offset: usize) -> Range { - self.text - .split_word_bound_indices() - .map(|(start, word)| start..start + word.len()) - .find(|range| range.contains(&offset)) - .unwrap_or(self.text.len()..self.text.len()) - } - /// Resolve the displayed link using the same destination policy as terminal OSC-8 output. pub(super) fn link_at(&self, row: usize, column: u16) -> Option { let row = self.rows.get(row)?; @@ -301,18 +293,6 @@ impl TextLayout { .terminal_destination() } - /// Select a logical line, including its terminating hard newline when present. - pub(super) fn line_range(&self, offset: usize) -> Range { - let offset = self.text.floor_char_boundary(offset.min(self.text.len())); - let start = self.text[..offset] - .rfind('\n') - .map_or(/*default*/ 0, |newline| newline + 1); - let end = self.text[offset..] - .find('\n') - .map_or(self.text.len(), |newline| offset + newline + 1); - start..end - } - fn visible_rows(&self, area: Rect, start_row: usize) -> impl Iterator { let height = if area.width == 0 { 0 } else { area.height }; debug_assert!(area.width == 0 || area.width == self.width); diff --git a/codex-rs/tui/src/transcript_view/text_tests.rs b/codex-rs/tui/src/transcript_view/text_tests.rs index 3ecd5bc177..b62e4e599a 100644 --- a/codex-rs/tui/src/transcript_view/text_tests.rs +++ b/codex-rs/tui/src/transcript_view/text_tests.rs @@ -4,6 +4,7 @@ use super::TextLayout; use crate::terminal_hyperlinks::HyperlinkLine; use crate::terminal_hyperlinks::adaptive_wrap_hyperlink_lines; use crate::terminal_hyperlinks::prefix_hyperlink_lines; +use crate::text_selection::SelectionUnit; use crate::wrapping::RtOptions; use pretty_assertions::assert_eq; use ratatui::buffer::Buffer; @@ -28,7 +29,7 @@ fn copying_wrapped_text_preserves_only_hard_newlines() { (11, 1), ); assert_eq!( - &layout.text()[layout.line_range(/*offset*/ 12)], + &layout.text()[SelectionUnit::Line.range(layout.text(), /*offset*/ 12)], "alpha beta gamma\n" ); @@ -69,7 +70,10 @@ fn hit_testing_uses_graphemes_and_does_not_select_padding() { .collect::>(), vec![0, 1, 1, 4, 7, 7, 18, 18, 18, 18, 18, 18], ); - assert_eq!(&layout.text()[layout.word_range(/*offset*/ 4)], "e\u{301}"); + assert_eq!( + &layout.text()[SelectionUnit::Word.range(layout.text(), /*offset*/ 4)], + "e\u{301}" + ); } #[test] @@ -151,8 +155,8 @@ fn empty_logical_lines_keep_their_hard_breaks() { ( layout.text(), layout.row_count(), - layout.line_range(/*offset*/ 0), - layout.line_range(/*offset*/ 5) + SelectionUnit::Line.range(layout.text(), /*offset*/ 0), + SelectionUnit::Line.range(layout.text(), /*offset*/ 5) ), ("\ntext\n", 3, 0..1, 1..6), );