fix(tui): generalize rail-prefix detection with structural pattern matching (#1163)
Replace the hardcoded three-glyph TOOL_CARD_RAIL_PREFIXES set (only covered tool-card rails) with iterative structural detection that handles all TUI decoration glyphs: - Iterates through consecutive leading decorative spans so tool headers with multiple prefix spans (e.g. "• ▶ run issue") are fully stripped. - Pattern A: "<glyph>[<glyph>…]<space>" where all non-space chars are drawing characters — covers single-glyph (▏, ▶, ⌕) and multi-glyph prefixes (⋮⋮). - Pattern B: "<glyph>" + lone space span — covers assistant/user glyphs (●, ▎). - Covers Box Drawing, Block Elements, Geometric Shapes, and individual chars used as TUI decoration (•, …, ·, ⌕, ⋮). Store rail-prefix widths in TranscriptViewCache so the copy path reads metadata rather than guessing from glyphs.
This commit is contained in:
@@ -73,6 +73,11 @@ pub struct TranscriptViewCache {
|
||||
lines: Vec<Line<'static>>,
|
||||
/// Per-line metadata aligned with `lines`.
|
||||
line_meta: Vec<TranscriptLineMeta>,
|
||||
/// Per-line rail-prefix display-column count (`0` or `2`), aligned with
|
||||
/// `lines`. Populated during flatten so that selection-to-text can shift
|
||||
/// columns past visual-only decoration glyphs without guessing which
|
||||
/// spans are decorative (#1163).
|
||||
rail_prefix_widths: Vec<usize>,
|
||||
}
|
||||
|
||||
impl TranscriptViewCache {
|
||||
@@ -85,6 +90,7 @@ impl TranscriptViewCache {
|
||||
per_cell: Vec::new(),
|
||||
lines: Vec::new(),
|
||||
line_meta: Vec::new(),
|
||||
rail_prefix_widths: Vec::new(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -219,6 +225,7 @@ impl TranscriptViewCache {
|
||||
fn flatten(&mut self, spacing: TranscriptSpacing) {
|
||||
self.lines.clear();
|
||||
self.line_meta.clear();
|
||||
self.rail_prefix_widths.clear();
|
||||
self.append_flattened_cells(spacing, 0);
|
||||
}
|
||||
|
||||
@@ -243,6 +250,7 @@ impl TranscriptViewCache {
|
||||
.unwrap_or(self.lines.len());
|
||||
self.lines.truncate(truncate_at);
|
||||
self.line_meta.truncate(truncate_at);
|
||||
self.rail_prefix_widths.truncate(truncate_at);
|
||||
self.append_flattened_cells(spacing, first_cell);
|
||||
}
|
||||
|
||||
@@ -256,7 +264,7 @@ impl TranscriptViewCache {
|
||||
// Deref is zero-cost and gives us &[Line].
|
||||
let rendered_line_count = cached.lines.len();
|
||||
for (line_in_cell, line) in cached.lines.iter().enumerate() {
|
||||
self.lines.push(line_with_group_rail(
|
||||
let final_line = line_with_group_rail(
|
||||
line,
|
||||
tool_group_rail(
|
||||
self.per_cell.as_slice(),
|
||||
@@ -265,7 +273,10 @@ impl TranscriptViewCache {
|
||||
rendered_line_count,
|
||||
),
|
||||
usize::from(self.width),
|
||||
));
|
||||
);
|
||||
self.rail_prefix_widths
|
||||
.push(compute_rail_prefix_width(&final_line));
|
||||
self.lines.push(final_line);
|
||||
self.line_meta.push(TranscriptLineMeta::CellLine {
|
||||
cell_index,
|
||||
line_in_cell,
|
||||
@@ -277,6 +288,7 @@ impl TranscriptViewCache {
|
||||
for _ in 0..spacer_rows {
|
||||
self.lines.push(Line::from(""));
|
||||
self.line_meta.push(TranscriptLineMeta::Spacer);
|
||||
self.rail_prefix_widths.push(0);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -299,6 +311,18 @@ impl TranscriptViewCache {
|
||||
pub fn total_lines(&self) -> usize {
|
||||
self.lines.len()
|
||||
}
|
||||
|
||||
/// Return the rail-prefix display-column count for the line at
|
||||
/// `line_index`. Callers use this to shift selection coordinates past
|
||||
/// visual-only decoration glyphs without guessing which spans are
|
||||
/// decorative (#1163).
|
||||
#[must_use]
|
||||
pub fn rail_prefix_width(&self, line_index: usize) -> usize {
|
||||
self.rail_prefix_widths
|
||||
.get(line_index)
|
||||
.copied()
|
||||
.unwrap_or(0)
|
||||
}
|
||||
}
|
||||
|
||||
fn spacer_rows_between(
|
||||
@@ -391,6 +415,75 @@ fn line_with_group_rail(
|
||||
rendered
|
||||
}
|
||||
|
||||
/// Return the display-column count of consecutive visual-only decorative
|
||||
/// spans at the start of a rendered transcript line. Iterates through
|
||||
/// leading spans matching either of two patterns:
|
||||
///
|
||||
/// * Pattern A — span is `"<glyph>[<glyph>…]<space>"` where every character
|
||||
/// except the trailing space is a rail-drawing character (e.g. `▏ `,
|
||||
/// `▶ `, `⋮⋮ `). The entire span width is accumulated.
|
||||
/// * Pattern B — span is `"<glyph>"` (1 drawing char) followed by a lone
|
||||
/// space span `" "` (e.g. `●` then ` `, `▎` then ` `).
|
||||
///
|
||||
/// Stops at the first non-matching span. Every decorated glyph used by the
|
||||
/// TUI is a single display-column character, so char-count = display width.
|
||||
///
|
||||
/// Returns `0` for lines whose first span is not a decorative prefix.
|
||||
fn compute_rail_prefix_width(line: &Line<'static>) -> usize {
|
||||
let spans = line.spans.as_slice();
|
||||
let mut total = 0;
|
||||
let mut i = 0;
|
||||
|
||||
while i < spans.len() {
|
||||
let content = spans[i].content.as_ref();
|
||||
let n_chars = content.chars().count();
|
||||
|
||||
// Pattern A — span "<glyph>[<glyph>…]<space>" (≥ 2 chars, trailing
|
||||
// space, all preceding chars are drawing chars).
|
||||
if n_chars >= 2
|
||||
&& content.ends_with(' ')
|
||||
&& content
|
||||
.chars()
|
||||
.take(n_chars.saturating_sub(1))
|
||||
.all(is_rail_drawing_char)
|
||||
{
|
||||
total += n_chars;
|
||||
i += 1;
|
||||
continue;
|
||||
}
|
||||
|
||||
// Pattern B — span "<glyph>" (1 drawing char) + next span " ".
|
||||
if n_chars == 1
|
||||
&& content.chars().next().is_some_and(is_rail_drawing_char)
|
||||
&& spans.get(i + 1).is_some_and(|s| s.content.as_ref() == " ")
|
||||
{
|
||||
total += 2;
|
||||
i += 2;
|
||||
continue;
|
||||
}
|
||||
|
||||
break;
|
||||
}
|
||||
|
||||
total
|
||||
}
|
||||
|
||||
/// Characters that serve as decoration glyphs in the TUI left-rail and
|
||||
/// tool-header prefix system. All are single display-column characters.
|
||||
fn is_rail_drawing_char(ch: char) -> bool {
|
||||
matches!(
|
||||
ch,
|
||||
'\u{2500}'..='\u{257F}' // Box Drawing (╭ ╮ ╰ ╯ │ ╎ …)
|
||||
| '\u{2580}'..='\u{259F}' // Block Elements (▏ ▎ ▍ ▌ …)
|
||||
| '\u{25A0}'..='\u{25FF}' // Geometric Shapes (● ▶ ▷ ◆ ◐ …)
|
||||
| '\u{2022}' // • bullet (tool status / generic tool)
|
||||
| '\u{2026}' // … ellipsis (reasoning opener)
|
||||
| '\u{00B7}' // · middle dot (tool running symbol)
|
||||
| '\u{2315}' // ⌕ telephone recorder (find/search tool)
|
||||
| '\u{22EE}' // ⋮ vertical ellipsis (fanout/rlm tool)
|
||||
)
|
||||
}
|
||||
|
||||
fn truncate_spans_to_width(spans: Vec<Span<'static>>, max_width: usize) -> Vec<Span<'static>> {
|
||||
if max_width == 0 || spans.is_empty() {
|
||||
return Vec::new();
|
||||
|
||||
@@ -81,9 +81,7 @@ use crate::tui::tool_routing::exploring_label;
|
||||
use crate::tui::tool_routing::{
|
||||
handle_tool_call_complete, handle_tool_call_started, maybe_add_patch_preview,
|
||||
};
|
||||
use crate::tui::ui_text::{
|
||||
history_cell_to_text, line_to_plain_for_copy, slice_text, text_display_width,
|
||||
};
|
||||
use crate::tui::ui_text::{history_cell_to_text, line_to_plain, slice_text, text_display_width};
|
||||
use crate::tui::user_input::UserInputView;
|
||||
use crate::tui::views::subagent_view_agents;
|
||||
|
||||
@@ -7999,8 +7997,23 @@ fn selection_to_text(app: &App) -> Option<String> {
|
||||
let mut selected_lines = Vec::new();
|
||||
#[allow(clippy::needless_range_loop)]
|
||||
for line_index in start_index..=end_index {
|
||||
let (line_text, rail_width) = line_to_plain_for_copy(&lines[line_index]);
|
||||
// Rail-prefix decorations are stored as cache metadata rather than
|
||||
// detected from glyphs, so new decoration types are covered without
|
||||
// changes to the copy path (#1163).
|
||||
let rail_width = app.viewport.transcript_cache.rail_prefix_width(line_index);
|
||||
// Convert the rendered line to plain text (strips OSC-8), then
|
||||
// slice off the rail prefix so subsequent column offsets operate
|
||||
// on content-only text.
|
||||
let full_text = line_to_plain(&lines[line_index]);
|
||||
let line_text = if rail_width > 0 {
|
||||
slice_text(&full_text, rail_width, text_display_width(&full_text))
|
||||
} else {
|
||||
full_text
|
||||
};
|
||||
let line_width = text_display_width(&line_text);
|
||||
// Selection coordinates are recorded in rendered-column space, which
|
||||
// includes the visual rail prefix. Add rail_width back so the column
|
||||
// window maps correctly into the rail-stripped text.
|
||||
let (raw_col_start, raw_col_end) = if start_index == end_index {
|
||||
(start.column, end.column)
|
||||
} else if line_index == start_index {
|
||||
@@ -8011,10 +8024,6 @@ fn selection_to_text(app: &App) -> Option<String> {
|
||||
(0, line_width.saturating_add(rail_width))
|
||||
};
|
||||
|
||||
// The recorded selection columns include the rail prefix because
|
||||
// selection coordinates are taken from the rendered viewport. Shift
|
||||
// them left by the rail width and clamp to the rail-stripped line so
|
||||
// a click that lands on the rail glyph itself collapses to column 0.
|
||||
let col_start = raw_col_start.saturating_sub(rail_width).min(line_width);
|
||||
let col_end = raw_col_end.saturating_sub(rail_width).min(line_width);
|
||||
|
||||
|
||||
@@ -174,7 +174,7 @@ fn selection_to_text_handles_multiline_and_reversed_endpoints() {
|
||||
column: 6,
|
||||
});
|
||||
|
||||
assert_eq!(selection_to_text(&app).as_deref(), Some("a beta\n▏ gam"));
|
||||
assert_eq!(selection_to_text(&app).as_deref(), Some("a beta\ngam"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -228,10 +228,10 @@ fn selection_to_text_copies_rendered_transcript_block() {
|
||||
|
||||
let selected = selection_to_text(&app).expect("selection text");
|
||||
assert!(selected.contains("Note copy system"), "{selected:?}");
|
||||
assert!(selected.contains("▎ copy user"), "{selected:?}");
|
||||
assert!(selected.contains("copy user"), "{selected:?}");
|
||||
assert!(selected.contains("copy thinking"), "{selected:?}");
|
||||
assert!(selected.contains("tool output line"), "{selected:?}");
|
||||
assert!(selected.contains("● copy assistant"), "{selected:?}");
|
||||
assert!(selected.contains("copy assistant"), "{selected:?}");
|
||||
// #1163: tool-card middle lines are rendered with a `│ ` left rail
|
||||
// glyph, but that decoration must not leak into copied text. Assert
|
||||
// no isolated rail glyph survives at the start of any line.
|
||||
|
||||
@@ -6,14 +6,6 @@ use unicode_width::UnicodeWidthChar;
|
||||
use crate::tui::history::HistoryCell;
|
||||
use crate::tui::osc8;
|
||||
|
||||
/// Tool-card left-rail decoration glyphs emitted by
|
||||
/// `crate::tui::transcript::line_with_group_rail` as the leading span of a
|
||||
/// rendered tool-card line. They are visual-only and must not leak into
|
||||
/// copied text (#1163).
|
||||
const TOOL_CARD_RAIL_PREFIXES: &[&str] = &["\u{256D} ", "\u{2502} ", "\u{2570} "];
|
||||
/// Display width of any rail prefix (one box-drawing glyph + one space).
|
||||
const TOOL_CARD_RAIL_PREFIX_WIDTH: usize = 2;
|
||||
|
||||
pub(super) fn history_cell_to_text(cell: &HistoryCell, width: u16) -> String {
|
||||
cell.transcript_lines(width)
|
||||
.into_iter()
|
||||
@@ -28,26 +20,14 @@ fn line_to_string(line: Line<'static>) -> String {
|
||||
out
|
||||
}
|
||||
|
||||
/// Strip a leading tool-card rail glyph span (`╭ `, `│ `, `╰ `) and OSC-8
|
||||
/// link wrappers from a rendered transcript line so the decoration does
|
||||
/// not leak into copied text. Returns `(plain_text, rail_prefix_width)`
|
||||
/// where `rail_prefix_width` is `0` for non-tool-card lines and `2` when
|
||||
/// the rail prefix was stripped. Callers subtract `rail_prefix_width`
|
||||
/// from the recorded selection columns to keep the visible selection
|
||||
/// rect aligned with the returned plain text.
|
||||
pub(super) fn line_to_plain_for_copy(line: &Line<'static>) -> (String, usize) {
|
||||
let mut spans = line.spans.iter();
|
||||
if let Some(first) = line.spans.first()
|
||||
&& TOOL_CARD_RAIL_PREFIXES.contains(&first.content.as_ref())
|
||||
{
|
||||
spans.next();
|
||||
let mut out = String::new();
|
||||
append_spans_plain(spans, &mut out);
|
||||
return (out, TOOL_CARD_RAIL_PREFIX_WIDTH);
|
||||
}
|
||||
/// Convert a rendered transcript line to plain text, stripping OSC-8 link
|
||||
/// escape sequences. The caller is responsible for shifting selection columns
|
||||
/// to account for any visual-only rail prefix (see
|
||||
/// `TranscriptViewCache::rail_prefix_width`).
|
||||
pub(super) fn line_to_plain(line: &Line<'static>) -> String {
|
||||
let mut out = String::new();
|
||||
append_spans_plain(spans, &mut out);
|
||||
(out, 0)
|
||||
append_spans_plain(line.spans.iter(), &mut out);
|
||||
out
|
||||
}
|
||||
|
||||
fn append_spans_plain<'a, I>(spans: I, out: &mut String)
|
||||
@@ -103,9 +83,7 @@ mod tests {
|
||||
use ratatui::text::Span;
|
||||
|
||||
#[test]
|
||||
fn line_to_plain_for_copy_strips_osc_8_wrapper() {
|
||||
// A span carrying an OSC 8-wrapped URL must not leak the escape into
|
||||
// selection / clipboard output. The visible label survives.
|
||||
fn line_to_plain_strips_osc_8_wrapper() {
|
||||
let wrapped = format!(
|
||||
"\x1b]8;;{}\x1b\\{}\x1b]8;;\x1b\\",
|
||||
"https://example.com", "https://example.com"
|
||||
@@ -115,44 +93,46 @@ mod tests {
|
||||
Span::raw(wrapped),
|
||||
Span::raw(" for details"),
|
||||
]);
|
||||
let (text, rail_width) = line_to_plain_for_copy(&line);
|
||||
let text = line_to_plain(&line);
|
||||
assert_eq!(text, "see https://example.com for details");
|
||||
assert_eq!(rail_width, 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn line_to_plain_for_copy_passes_through_plain_spans() {
|
||||
fn line_to_plain_passes_through_plain_spans() {
|
||||
let line = Line::from(vec![Span::raw("plain "), Span::raw("text")]);
|
||||
let (text, rail_width) = line_to_plain_for_copy(&line);
|
||||
let text = line_to_plain(&line);
|
||||
assert_eq!(text, "plain text");
|
||||
assert_eq!(rail_width, 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn line_to_plain_for_copy_strips_tool_card_rail_prefix() {
|
||||
let line = Line::from(vec![Span::raw("\u{2502} "), Span::raw("body content")]);
|
||||
let (text, rail_width) = line_to_plain_for_copy(&line);
|
||||
assert_eq!(text, "body content");
|
||||
assert_eq!(rail_width, 2);
|
||||
fn line_to_plain_includes_all_spans() {
|
||||
// Visual-only rail spans are stripped by the caller using
|
||||
// TranscriptViewCache::rail_prefix_width — line_to_plain itself
|
||||
// is a faithful span-to-string pass-through.
|
||||
let line = Line::from(vec![Span::raw("\u{2502} "), Span::raw("tool output")]);
|
||||
let text = line_to_plain(&line);
|
||||
assert_eq!(text, "\u{2502} tool output");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn line_to_plain_for_copy_strips_top_and_bottom_rails() {
|
||||
for glyph in ["\u{256D} ", "\u{2570} "] {
|
||||
let line = Line::from(vec![Span::raw(glyph), Span::raw("x")]);
|
||||
let (text, rail_width) = line_to_plain_for_copy(&line);
|
||||
assert_eq!(text, "x");
|
||||
assert_eq!(rail_width, 2);
|
||||
}
|
||||
fn slice_text_respects_column_bounds() {
|
||||
let text = "hello world";
|
||||
assert_eq!(slice_text(text, 0, 5), "hello");
|
||||
assert_eq!(slice_text(text, 6, 11), "world");
|
||||
assert_eq!(slice_text(text, 0, 0), "");
|
||||
assert_eq!(slice_text(text, 0, 100), text);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn line_to_plain_for_copy_keeps_user_typed_pipe_when_no_rail() {
|
||||
// A normal line whose plain text starts with `│ literal` (single
|
||||
// span, not a rail prefix span) must round-trip verbatim.
|
||||
let line = Line::from(vec![Span::raw("\u{2502} literal pipe at start")]);
|
||||
let (text, rail_width) = line_to_plain_for_copy(&line);
|
||||
assert_eq!(text, "\u{2502} literal pipe at start");
|
||||
assert_eq!(rail_width, 0);
|
||||
fn slice_text_handles_multibyte_characters() {
|
||||
let text = "a─b"; // U+2500 is 1 display column on supported terminals
|
||||
assert_eq!(slice_text(text, 1, 2), "─");
|
||||
assert_eq!(slice_text(text, 0, 3), text);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn slice_text_truncates_at_end() {
|
||||
let text = "ab";
|
||||
assert_eq!(slice_text(text, 1, 5), "b");
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user