mirror of
https://github.com/openai/codex.git
synced 2026-10-20 17:38:45 +00:00
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
This commit is contained in:
@@ -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,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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<Arc<dyn HistoryCell>> = 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;
|
||||
|
||||
+28
@@ -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,
|
||||
]
|
||||
}
|
||||
@@ -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<usize>,
|
||||
/// Source offset at the logical line end; absent on soft wraps and synthetic rows.
|
||||
line_end: Option<usize>,
|
||||
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<usize>,
|
||||
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<String> {
|
||||
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<Text
|
||||
TextRow {
|
||||
line: wrapped,
|
||||
source: range,
|
||||
line_end: None,
|
||||
content_width,
|
||||
first_column: first_column + prefix_columns,
|
||||
prefix_columns,
|
||||
|
||||
@@ -111,6 +111,37 @@ fn selection_highlights_wide_graphemes_without_trailing_padding() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn selected_newlines_respect_wrapping_and_synthetic_rows() {
|
||||
for (width, expected) in [(8, vec![(2, 2), (6, 5), (2, 6)]), (4, vec![(2, 2), (2, 7)])] {
|
||||
let layout = TextLayout::new(
|
||||
vec!["".into(), "alpha beta ".into(), "".into(), "界".into()],
|
||||
width,
|
||||
)
|
||||
.with_disclosure_control_at("details".into(), /*source_offset*/ 0)
|
||||
.with_leading_separator();
|
||||
let area = Rect::new(/*x*/ 2, /*y*/ 1, width, /*height*/ 9);
|
||||
let mut buffer = Buffer::empty(area);
|
||||
layout.render(area, &mut buffer, /*start_row*/ 0);
|
||||
for range in [0..1, 13..15] {
|
||||
layout.highlight_selection(
|
||||
range,
|
||||
/*next*/ None,
|
||||
area,
|
||||
&mut buffer,
|
||||
/*start_row*/ 0,
|
||||
);
|
||||
}
|
||||
assert_eq!(
|
||||
(area.top()..area.bottom())
|
||||
.flat_map(|y| (area.left()..area.right()).map(move |x| (x, y)))
|
||||
.filter(|&(x, y)| buffer[(x, y)].modifier.contains(Modifier::REVERSED))
|
||||
.collect::<Vec<_>>(),
|
||||
expected,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn offsets_can_reach_rows_after_u16_limit() {
|
||||
let layout = TextLayout::new(vec!["x".repeat(/*n*/ 65_538).into()], /*width*/ 1);
|
||||
|
||||
Reference in New Issue
Block a user