From f07aaf920b14d7a746e435add34b8bbd37da5da6 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Mon, 21 Sep 2026 16:17:38 +0000 Subject: [PATCH] Highlight selected newlines in the transcript view (#47068) ## Why Transcript selections could include a newline without showing it in the highlight, making `code` and `code\n` look identical. ## What changed Highlight one trailing cell for selected hard line breaks when space permits, including newline separators between transcript entries. Track logical line endings so soft wraps and synthetic rows do not receive newline highlights. ## Testing Add selection snapshots for hard breaks within and between entries, plus layout coverage for empty lines, wrapping, trailing whitespace, and synthetic rows. Update the app selection test to assert the leading newline highlight. GitOrigin-RevId: d6f332c472598a3f79557b7ebb41730c6acc7820 --- .../tui/src/app/tests/transcript_selection.rs | 7 ++-- codex-rs/tui/src/transcript_view/selection.rs | 18 +++++++-- .../src/transcript_view/selection_tests.rs | 38 +++++++++++++++++++ ...ard_breaks_within_and_between_entries.snap | 28 ++++++++++++++ codex-rs/tui/src/transcript_view/text.rs | 36 ++++++++++++++++++ .../tui/src/transcript_view/text_tests.rs | 31 +++++++++++++++ 6 files changed, 151 insertions(+), 7 deletions(-) create mode 100644 codex-rs/tui/src/transcript_view/snapshots/codex_tui__transcript_view__selection__tests__selection_highlights_hard_breaks_within_and_between_entries.snap diff --git a/codex-rs/tui/src/app/tests/transcript_selection.rs b/codex-rs/tui/src/app/tests/transcript_selection.rs index 7e32a8f0e8..735a7db672 100644 --- a/codex-rs/tui/src/app/tests/transcript_selection.rs +++ b/codex-rs/tui/src/app/tests/transcript_selection.rs @@ -62,7 +62,7 @@ async fn text_selection_temporarily_replaces_the_rendered_backtrack_highlight() .await?; let (_, highlighted) = render(&mut app); assert_eq!(highlighted, ""); - // The display-only blank before a user message may be the selection's first source row. + // The selection starts on the leading blank source row before the user message. for _ in 0..2 { app.handle_tui_event( &mut tui, @@ -72,9 +72,8 @@ async fn text_selection_temporarily_replaces_the_rendered_backtrack_highlight() .await?; } let (_, highlighted) = render(&mut app); - assert!(!highlighted.is_empty()); - assert!(prompt.starts_with(&highlighted)); - assert!(highlighted.len() < prompt.len()); + // The selected leading newline paints one blank cell before the first prompt character. + assert_eq!(highlighted, " S"); app.handle_tui_event( &mut tui, diff --git a/codex-rs/tui/src/transcript_view/selection.rs b/codex-rs/tui/src/transcript_view/selection.rs index 1cc62623bd..78cfd5f1fd 100644 --- a/codex-rs/tui/src/transcript_view/selection.rs +++ b/codex-rs/tui/src/transcript_view/selection.rs @@ -332,9 +332,21 @@ impl TranscriptView { self.area.width, /*height*/ 1, ); - visible - .layout - .highlight(begin..finish, area, buf, visible.row); + let next = (visible.index + 1..=end.index) + .filter_map(|index| { + selection + .snapshot + .pinned + .get(&self.entry_key(&selection.snapshot.cells, index)) + }) + .find(|layout| layout.row_count() > 0); + visible.layout.highlight_selection( + begin..finish, + next.map(AsRef::as_ref), + area, + buf, + visible.row, + ); } } diff --git a/codex-rs/tui/src/transcript_view/selection_tests.rs b/codex-rs/tui/src/transcript_view/selection_tests.rs index 435966a390..2358f5ef6e 100644 --- a/codex-rs/tui/src/transcript_view/selection_tests.rs +++ b/codex-rs/tui/src/transcript_view/selection_tests.rs @@ -18,6 +18,44 @@ use crossterm::event::MouseEventKind; use pretty_assertions::assert_eq; use std::time::Instant; +#[test] +fn selection_highlights_hard_breaks_within_and_between_entries() { + let mut frames = Vec::new(); + for (lines, end_row) in [ + (vec![vec!["code", "next"]], 1), + (vec![vec!["code"], vec!["next"]], 2), + ] { + let cells: Vec> = lines + .into_iter() + .map(|lines| { + Arc::new(PlainHistoryCell::new( + lines.into_iter().map(Line::from).collect(), + )) as _ + }) + .collect(); + let area = Rect::new( + /*x*/ 0, /*y*/ 0, /*width*/ 8, /*height*/ 3, + ); + let mut view = TranscriptView::default(); + let mut buffer = Buffer::empty(area); + view.render(area, &mut buffer, &cells); + view.begin_selection(&cells, /*column*/ 0, /*row*/ 0, /*clicks*/ 1); + view.extend_selection(/*column*/ 7, /*row*/ 0); + view.render(area, &mut buffer, &cells); + assert_eq!(view.selected_text(&cells).as_deref(), Some("code")); + assert!( + !buffer[(4, 0)] + .modifier + .contains(ratatui::style::Modifier::REVERSED) + ); + view.extend_selection(/*column*/ 0, end_row); + view.render(area, &mut buffer, &cells); + assert_eq!(view.selected_text(&cells).as_deref(), Some("code\n")); + frames.push(format!("{buffer:?}")); + } + insta::assert_snapshot!(frames.join("\n")); +} + #[test] fn copy_shortcuts_clear_selection_only_after_confirmed_delivery() { use crate::clipboard_copy::CopyStatus; diff --git a/codex-rs/tui/src/transcript_view/snapshots/codex_tui__transcript_view__selection__tests__selection_highlights_hard_breaks_within_and_between_entries.snap b/codex-rs/tui/src/transcript_view/snapshots/codex_tui__transcript_view__selection__tests__selection_highlights_hard_breaks_within_and_between_entries.snap new file mode 100644 index 0000000000..3b9a004252 --- /dev/null +++ b/codex-rs/tui/src/transcript_view/snapshots/codex_tui__transcript_view__selection__tests__selection_highlights_hard_breaks_within_and_between_entries.snap @@ -0,0 +1,28 @@ +--- +source: tui/src/transcript_view/selection_tests.rs +expression: "frames.join(\"\\n\")" +--- +Buffer { + area: Rect { x: 0, y: 0, width: 8, height: 3 }, + content: [ + "code ", + "next ", + " ", + ], + styles: [ + x: 0, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: REVERSED, + x: 5, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + ] +} +Buffer { + area: Rect { x: 0, y: 0, width: 8, height: 3 }, + content: [ + "code ", + " ", + "next ", + ], + styles: [ + x: 0, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: REVERSED, + x: 5, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + ] +} diff --git a/codex-rs/tui/src/transcript_view/text.rs b/codex-rs/tui/src/transcript_view/text.rs index fe87555760..b1aa7f4552 100644 --- a/codex-rs/tui/src/transcript_view/text.rs +++ b/codex-rs/tui/src/transcript_view/text.rs @@ -4,6 +4,7 @@ //! lines introduce hard breaks. Display wrapping and synthetic controls never enter `text`. //! Layout offsets use `usize`; terminal coordinates are narrowed only for visible rows. //! Disclosure controls follow their activity's source text without changing its indentation. +//! Selected hard breaks highlight one trailing cell when space permits. use std::borrow::Cow; use std::ops::Range; @@ -59,6 +60,8 @@ struct DisclosureControl { struct TextRow { line: HyperlinkLine, source: Range, + /// Source offset at the logical line end; absent on soft wraps and synthetic rows. + line_end: Option, content_width: u16, first_column: usize, prefix_columns: usize, @@ -121,6 +124,7 @@ impl TextLayout { TextRow { line: HyperlinkLine::from(line), source: source_offset..source_offset, + line_end: None, content_width: self.width, first_column: indent, prefix_columns: indent, @@ -144,6 +148,7 @@ impl TextLayout { TextRow { line: HyperlinkLine::from(""), source: 0..0, + line_end: None, content_width: self.width, first_column: 0, prefix_columns: 0, @@ -282,6 +287,33 @@ impl TextLayout { } } + /// Add a trailing cell for selected hard breaks, including a copied separator to `next`. + pub(super) fn highlight_selection( + &self, + range: Range, + next: Option<&Self>, + area: Rect, + buf: &mut Buffer, + start_row: usize, + ) { + self.highlight(range.clone(), area, buf, start_row); + for (screen_row, row) in self.visible_rows(area, start_row) { + let Some(end) = row.line_end else { continue }; + let selected = range.contains(&end) + || (end == self.text.len() + && next.is_some_and(|next| self.separator_after(next).contains('\n'))); + let column = row.first_column + + row.tabs.column_for_offset( + display_width(&self.text[row.source.clone()]), + row.source.len(), + ); + if selected && column < usize::from(self.width) { + buf[(area.x + column as u16, area.y + screen_row)] + .set_style(Style::default().add_modifier(Modifier::REVERSED)); + } + } + } + /// 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)?; @@ -321,6 +353,9 @@ impl TextLayout { .map(|span| span.content.as_ref()), ); rows.extend(layout_line(line, start, width)); + if let Some(row) = rows.last_mut() { + row.line_end = Some(text.len()); + } } Self { logical, @@ -405,6 +440,7 @@ fn layout_line(logical: &LogicalLine, text_start: usize, width: u16) -> Vec>(), + expected, + ); + } +} + #[test] fn offsets_can_reach_rows_after_u16_limit() { let layout = TextLayout::new(vec!["x".repeat(/*n*/ 65_538).into()], /*width*/ 1);