From fcaa035438e45eacea079a400dd9b7ad55106e81 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 19 Sep 2026 20:09:58 +0000 Subject: [PATCH] Preserve logical source text and styles across TUI wrapping (#46708) ## Why Display wrapping removes whitespace and adds gutters, so rendered rows alone cannot reliably describe the original logical text. Streaming previews also need to detect source changes that leave visible rows unchanged. ## What changed - Retain shared logical text, styles, exact byte ranges, and display margins alongside wrapped rows; remap hyperlinks using those ranges. - Preserve source metadata through prompt, Markdown, and prefixed history rendering, including spoken artifact annotations. Clear it when split-flap animation replaces the text. - Include source metadata when comparing streaming previews and agent or plan tails. - Keep compound graphemes intact, preserve explicit span styles, and reuse first-fit wrapping results when safe. - Use the lighter history prompt background for submitted messages. ## Testing Add regression coverage for source ranges and styles across widths and gutters, distinct repeated lines, source-only streaming updates, compound graphemes, and equivalence between reused and generic wrapping results. Extend Markdown, spoken artifact, and animation tests to check retained source metadata. GitOrigin-RevId: 9e60746642fc9a8f91c1abfe7c77eefc1fe421ab --- .../tui/src/chatwidget/realtime_split_flap.rs | 2 + .../chatwidget/realtime_split_flap_tests.rs | 11 +- codex-rs/tui/src/history_cell/base.rs | 19 +- codex-rs/tui/src/history_cell/base_tests.rs | 95 ++++++++++ codex-rs/tui/src/history_cell/messages.rs | 138 ++++++++------ .../tui/src/history_cell/messages_tests.rs | 15 ++ codex-rs/tui/src/history_cell/mod.rs | 2 - codex-rs/tui/src/history_cell/plans.rs | 10 +- ...els_follow_the_painted_prompt_surface.snap | 25 +-- .../tui/src/history_cell/spoken_artifacts.rs | 65 ++++++- codex-rs/tui/src/history_cell/tests.rs | 8 +- codex-rs/tui/src/insert_history.rs | 1 + codex-rs/tui/src/markdown_render.rs | 22 ++- .../src/markdown_render/web_links_tests.rs | 1 + codex-rs/tui/src/multi_agents.rs | 1 + .../src/streaming/controller_preview_tests.rs | 32 ++++ codex-rs/tui/src/streaming/prose_preview.rs | 3 +- codex-rs/tui/src/style.rs | 1 - codex-rs/tui/src/terminal_hyperlinks.rs | 127 ++++++++++++- .../tui/src/terminal_hyperlinks/source.rs | 114 ++++++++++++ codex-rs/tui/src/terminal_hyperlinks_tests.rs | 1 + codex-rs/tui/src/wrapping.rs | 172 +++++++++++++++--- codex-rs/tui/src/wrapping_reuse_tests.rs | 114 ++++++++++++ 23 files changed, 856 insertions(+), 123 deletions(-) create mode 100644 codex-rs/tui/src/history_cell/base_tests.rs create mode 100644 codex-rs/tui/src/terminal_hyperlinks/source.rs create mode 100644 codex-rs/tui/src/wrapping_reuse_tests.rs diff --git a/codex-rs/tui/src/chatwidget/realtime_split_flap.rs b/codex-rs/tui/src/chatwidget/realtime_split_flap.rs index f9635c52cc..50f0b8c57e 100644 --- a/codex-rs/tui/src/chatwidget/realtime_split_flap.rs +++ b/codex-rs/tui/src/chatwidget/realtime_split_flap.rs @@ -164,6 +164,8 @@ impl SplitFlapTranscriptCell { continue; } + // Animated glyphs and board padding no longer map to authored source bytes. + line.source = None; let original_spans = std::mem::take(&mut line.line.spans); line.line.style = line.line.style.patch(board_style); for span in original_spans { diff --git a/codex-rs/tui/src/chatwidget/realtime_split_flap_tests.rs b/codex-rs/tui/src/chatwidget/realtime_split_flap_tests.rs index bece1513be..5007f5382f 100644 --- a/codex-rs/tui/src/chatwidget/realtime_split_flap_tests.rs +++ b/codex-rs/tui/src/chatwidget/realtime_split_flap_tests.rs @@ -90,11 +90,12 @@ fn split_flap_frames_assemble_a_transcript_from_dark_tiles() { #[test] fn split_flap_paints_the_entire_transcript_row_black() { let cell = cell("GATE", MotionMode::Animated); - let lines = cell.animate_lines( - cell.inner.display_hyperlink_lines(/*width*/ 12), - /*width*/ 12, - Duration::ZERO, - ); + let mut source_lines = cell.inner.display_hyperlink_lines(/*width*/ 12); + source_lines[0].source = Some(crate::terminal_hyperlinks::LogicalLineSource::from_line( + &source_lines[0].line, + )); + let lines = cell.animate_lines(source_lines, /*width*/ 12, Duration::ZERO); + assert!(lines[0].source.is_none()); let area = Rect::new( /*x*/ 0, /*y*/ 0, /*width*/ 12, /*height*/ 1, ); diff --git a/codex-rs/tui/src/history_cell/base.rs b/codex-rs/tui/src/history_cell/base.rs index 44db928e47..15ab8638f5 100644 --- a/codex-rs/tui/src/history_cell/base.rs +++ b/codex-rs/tui/src/history_cell/base.rs @@ -80,13 +80,24 @@ impl PrefixedWrappedHistoryCell { impl HistoryCell for PrefixedWrappedHistoryCell { fn display_lines(&self, width: u16) -> Vec> { + visible_lines(self.display_hyperlink_lines(width)) + } + + fn display_hyperlink_lines(&self, width: u16) -> Vec { if width == 0 { return Vec::new(); } - let opts = RtOptions::new(width.max(1) as usize) + let opts = RtOptions::new(usize::from(width)) .initial_indent(self.initial_prefix.clone()) .subsequent_indent(self.subsequent_prefix.clone()); - adaptive_wrap_lines(&self.text, opts) + crate::terminal_hyperlinks::adaptive_wrap_hyperlink_lines( + &plain_hyperlink_lines(self.text.lines.clone()), + opts, + ) + } + + fn transcript_hyperlink_lines(&self, width: u16) -> Vec { + self.display_hyperlink_lines(width) } fn raw_lines(&self) -> Vec> { @@ -173,3 +184,7 @@ impl HistoryCell for CompositeHistoryCell { false } } + +#[cfg(test)] +#[path = "base_tests.rs"] +mod tests; diff --git a/codex-rs/tui/src/history_cell/base_tests.rs b/codex-rs/tui/src/history_cell/base_tests.rs new file mode 100644 index 0000000000..fbcbb7cbad --- /dev/null +++ b/codex-rs/tui/src/history_cell/base_tests.rs @@ -0,0 +1,95 @@ +//! Prefixed rows preserve authored whitespace and exclude display gutters from source text. + +use super::*; +use pretty_assertions::assert_eq; +use std::sync::Arc; + +#[test] +fn prefixed_wrapping_retains_one_source_line_across_widths_and_gutters() { + let source = " let result = compute(alpha, beta, gamma); print(result);"; + let styled = Line::from(vec![ + " let result = ".into(), + "compute".red().bold(), + "(alpha, beta, gamma); print(result);".into(), + ]) + .cyan() + .italic(); + let cell = PrefixedWrappedHistoryCell::new(styled, "βœ” ".green(), " "); + for width in [16, 24, 48] { + let lines = cell.transcript_hyperlink_lines(width); + let first = lines + .first() + .expect("wrapped source") + .source + .as_ref() + .expect("source range"); + assert_eq!(first.text.as_ref(), source); + assert_eq!( + first.styled_range(0..source.len()), + Line::from(vec![ + " let result = ".cyan().italic(), + "compute".red().bold().italic(), + "(alpha, beta, gamma); print(result);".cyan().italic(), + ]) + ); + assert_eq!(first.range.start, 0); + let restyled = lines[0].clone().style(Style::new().on_blue()); + assert_eq!( + restyled + .source + .as_ref() + .unwrap() + .styled_range(0..source.len()), + Line::from(vec![ + " let result = ".cyan().italic().on_blue(), + "compute".red().bold().italic().on_blue(), + "(alpha, beta, gamma); print(result);" + .cyan() + .italic() + .on_blue(), + ]) + ); + for line in &lines { + let origin = line.source.as_ref().expect("source range"); + let visible = line.line.to_string(); + assert!(Arc::ptr_eq(&origin.text, &first.text)); + assert_eq!( + &visible[origin.prefix_bytes..origin.prefix_bytes + origin.range.len()], + &source[origin.range.clone()], + ); + assert!(line.line.width() <= usize::from(width)); + } + assert_eq!( + lines + .last() + .expect("last row") + .source + .as_ref() + .expect("source") + .range + .end, + source.len() + ); + } +} + +#[test] +fn prefixed_wrapping_keeps_hard_lines_distinct_even_when_their_text_repeats() { + let cell = PrefixedWrappedHistoryCell::new( + Text::from(vec![ + Line::from("repeated text"), + Line::from("repeated text"), + ]), + "βœ” ", + " ", + ); + let lines = cell.transcript_hyperlink_lines(/*width*/ 40); + let first = lines[0].source.as_ref().expect("first source"); + let second = lines[1].source.as_ref().expect("second source"); + assert_eq!( + (first.text.as_ref(), second.text.as_ref()), + ("repeated text", "repeated text") + ); + assert!(!Arc::ptr_eq(&first.text, &second.text)); + assert!(cell.display_hyperlink_lines(/*width*/ 0).is_empty()); +} diff --git a/codex-rs/tui/src/history_cell/messages.rs b/codex-rs/tui/src/history_cell/messages.rs index c5c3002c02..555aaf4929 100644 --- a/codex-rs/tui/src/history_cell/messages.rs +++ b/codex-rs/tui/src/history_cell/messages.rs @@ -4,10 +4,12 @@ use super::markdown_render_cache::MarkdownRenderCache; use super::*; use crate::style::accent_color_on; +use crate::style::history_prompt_style; use crate::terminal_hyperlinks::annotate_web_urls_in_line; -use crate::terminal_hyperlinks::remap_wrapped_line; +use crate::terminal_hyperlinks::lines_with_sources_eq; +use crate::terminal_hyperlinks::remap_source_wrapped_line; use crate::wrapping::url_preserving_wrap_options; -use crate::wrapping::word_wrap_line; +use crate::wrapping::word_wrap_line_with_source; use std::borrow::Cow; #[derive(Debug)] @@ -185,9 +187,9 @@ impl HistoryCell for UserHistoryCell { .saturating_sub( LIVE_PREFIX_COLS + 1, /* keep a one-column right margin for wrapping */ ) - .max(1); + .max(/*other*/ 1); - let style = user_message_style(); + let style = history_prompt_style(); let element_style = style.fg(accent_color_on(style.bg)); let wrapped_remote_images = if self.remote_image_urls.is_empty() { @@ -202,60 +204,7 @@ impl HistoryCell for UserHistoryCell { } .filter(|lines| !lines.is_empty()); - let wrapped_message = if message.is_empty() && text_elements.is_empty() { - None - } else { - let wrap_options = RtOptions::new(usize::from(wrap_width)) - .wrap_algorithm(textwrap::WrapAlgorithm::FirstFit); - let mut wrapped = if text_elements.is_empty() { - let message_without_trailing_newlines = message.trim_end_matches(['\r', '\n']); - adaptive_wrap_lines( - message_without_trailing_newlines - .split('\n') - .map(|line| Line::from(line).style(style)), - wrap_options, - ) - } else { - adaptive_wrap_lines( - build_user_message_lines_with_elements( - message, - text_elements, - style, - element_style, - ), - wrap_options, - ) - } - .into_iter() - .flat_map(|line| { - if line.width() <= usize::from(wrap_width) { - return vec![HyperlinkLine::new(line)]; - } - - // Terminal autowrap loses the message gutter and background. Explicitly split - // oversized URL tokens while retaining their complete OSC-8 destination. - let line = annotate_web_urls_in_line(line); - let forced_lines = word_wrap_line( - &line.line, - url_preserving_wrap_options(RtOptions::new(usize::from(wrap_width))) - .break_words(/*break_words*/ true), - ) - .iter() - .map(line_to_static) - .collect(); - remap_wrapped_line(&line, forced_lines) - }) - .collect::>(); - while wrapped.last().is_some_and(|line| { - line.line - .spans - .iter() - .all(|span| span.content.trim().is_empty()) - }) { - wrapped.pop(); - } - (!wrapped.is_empty()).then_some(wrapped) - }; + let wrapped_message = wrap_user_message(message, text_elements, style, wrap_width); if wrapped_remote_images.is_none() && wrapped_message.is_none() { return Vec::new(); @@ -287,6 +236,9 @@ impl HistoryCell for UserHistoryCell { } lines.push(HyperlinkLine::new(Line::from("").style(style))); + for source in lines.iter_mut().filter_map(|line| line.source.as_mut()) { + source.right_reserve = 1; + } lines } @@ -308,6 +260,61 @@ impl HistoryCell for UserHistoryCell { } } +/// Wrap a prompt body before the user-message gutter and background padding are added. +fn wrap_user_message( + message: &str, + text_elements: &[TextElement], + style: Style, + wrap_width: u16, +) -> Option> { + let element_style = style.fg(accent_color_on(style.bg)); + if message.is_empty() && text_elements.is_empty() { + return None; + } + + let wrap_options = + RtOptions::new(usize::from(wrap_width)).wrap_algorithm(textwrap::WrapAlgorithm::FirstFit); + let logical_lines = if text_elements.is_empty() { + let message_without_trailing_newlines = message.trim_end_matches(['\r', '\n']); + message_without_trailing_newlines + .split('\n') + .map(|line| Line::from(line.to_owned()).style(style)) + .collect() + } else { + build_user_message_lines_with_elements(message, text_elements, style, element_style) + }; + let mut wrapped = crate::terminal_hyperlinks::adaptive_wrap_hyperlink_lines( + &plain_hyperlink_lines(logical_lines), + wrap_options, + ) + .into_iter() + .flat_map(|mut line| { + if line.width() <= usize::from(wrap_width) { + return vec![line]; + } + + // Terminal autowrap loses the message gutter and background. Explicitly split + // oversized URL tokens while retaining their complete OSC-8 destination. + line.hyperlinks = annotate_web_urls_in_line(line.line.clone()).hyperlinks; + let forced_lines = word_wrap_line_with_source( + &line.line, + url_preserving_wrap_options(RtOptions::new(usize::from(wrap_width))) + .break_words(/*break_words*/ true), + ); + remap_source_wrapped_line(&line, forced_lines) + }) + .collect::>(); + while wrapped.last().is_some_and(|line| { + line.line + .spans + .iter() + .all(|span| span.content.trim().is_empty()) + }) { + wrapped.pop(); + } + (!wrapped.is_empty()).then_some(wrapped) +} + #[derive(Debug)] pub(crate) struct ReasoningSummaryCell { _header: String, @@ -522,6 +529,10 @@ fn normalize_whitespace_only_hyperlink_lines(mut lines: Vec) -> V { line.line = Line::default().style(line.line.style); line.hyperlinks.clear(); + if let Some(source) = &mut line.source { + source.prefix_bytes = 0; + source.range.end = source.range.start; + } } } lines @@ -594,13 +605,20 @@ mod tests; /// /// During streaming, lines that have not yet been committed to scrollback because they belong to /// an in-progress table are displayed via this cell in the `active_cell` slot. It is replaced on -/// deltas that change the visible tail and cleared when the stream finalizes. -#[derive(Debug, Eq, PartialEq)] +/// deltas that change the visible tail or retained source, and cleared when the stream finalizes. +#[derive(Debug, Eq)] pub(crate) struct StreamingAgentTailCell { lines: Vec, is_first_line: bool, } +impl PartialEq for StreamingAgentTailCell { + fn eq(&self, other: &Self) -> bool { + self.is_first_line == other.is_first_line + && lines_with_sources_eq(&self.lines, &other.lines) + } +} + impl StreamingAgentTailCell { pub(crate) fn new(lines: Vec, is_first_line: bool) -> Self { Self { diff --git a/codex-rs/tui/src/history_cell/messages_tests.rs b/codex-rs/tui/src/history_cell/messages_tests.rs index deabffe9ad..8db0126863 100644 --- a/codex-rs/tui/src/history_cell/messages_tests.rs +++ b/codex-rs/tui/src/history_cell/messages_tests.rs @@ -234,6 +234,21 @@ fn spoken_artifacts_link_only_real_workspace_files_and_preserve_existing_urls() span.content == "src/lib.rs:42" && span.style.add_modifier.contains(Modifier::UNDERLINED) })); assert_eq!(spoken.raw_lines(), vec![Line::from(markdown)]); + for width in [90, 24] { + for line in spoken.display_hyperlink_lines(width) { + let source = line.source.expect("spoken source"); + assert!( + source + .styled_range(0..source.text.len()) + .spans + .iter() + .any(|span| { + span.content == "src/lib.rs:42" + && span.style.add_modifier.contains(Modifier::UNDERLINED) + }) + ); + } + } insta::assert_snapshot!( format!( "{}\n{:?} -> /src/lib.rs\n{:?} -> https://example.com", diff --git a/codex-rs/tui/src/history_cell/mod.rs b/codex-rs/tui/src/history_cell/mod.rs index be50cc6042..97b7a856d6 100644 --- a/codex-rs/tui/src/history_cell/mod.rs +++ b/codex-rs/tui/src/history_cell/mod.rs @@ -33,7 +33,6 @@ use crate::render::line_utils::push_owned_lines; use crate::render::renderable::Renderable; use crate::session_state::ThreadSessionState; use crate::style::proposed_plan_style; -use crate::style::user_message_style; use crate::terminal_hyperlinks::HyperlinkLine; use crate::terminal_hyperlinks::HyperlinkParagraph; use crate::terminal_hyperlinks::plain_hyperlink_lines; @@ -83,7 +82,6 @@ use codex_utils_absolute_path::AbsolutePathBuf; #[cfg(test)] use codex_utils_cli::format_env_display; use ratatui::prelude::*; -use ratatui::style::Color; use ratatui::style::Modifier; use ratatui::style::Style; use ratatui::style::Styled; diff --git a/codex-rs/tui/src/history_cell/plans.rs b/codex-rs/tui/src/history_cell/plans.rs index 4847d30ca4..e7545b37a3 100644 --- a/codex-rs/tui/src/history_cell/plans.rs +++ b/codex-rs/tui/src/history_cell/plans.rs @@ -2,18 +2,26 @@ use super::markdown_render_cache::MarkdownRenderCache; use super::*; +use crate::terminal_hyperlinks::lines_with_sources_eq; /// Transient active-cell representation of the mutable tail of a proposed-plan stream. /// /// The controller prepares the full styled plan lines because plan tails need the same header, /// padding, and background treatment as committed `ProposedPlanStreamCell`s while remaining /// preview-only during streaming. -#[derive(Debug, Eq, PartialEq)] +#[derive(Debug, Eq)] pub(crate) struct StreamingPlanTailCell { lines: Vec, is_stream_continuation: bool, } +impl PartialEq for StreamingPlanTailCell { + fn eq(&self, other: &Self) -> bool { + self.is_stream_continuation == other.is_stream_continuation + && lines_with_sources_eq(&self.lines, &other.lines) + } +} + impl StreamingPlanTailCell { pub(crate) fn new(lines: Vec, is_stream_continuation: bool) -> Self { Self { diff --git a/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__user_image_labels_follow_the_painted_prompt_surface.snap b/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__user_image_labels_follow_the_painted_prompt_surface.snap index ce43b56f63..2968505509 100644 --- a/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__user_image_labels_follow_the_painted_prompt_surface.snap +++ b/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__user_image_labels_follow_the_painted_prompt_surface.snap @@ -1,5 +1,6 @@ --- source: tui/src/history_cell/tests.rs +assertion_line: 2425 expression: "snapshots.join(\"\\n\")" --- (255, 255, 255): Buffer { @@ -13,11 +14,11 @@ expression: "snapshots.join(\"\\n\")" ], styles: [ x: 0, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, - x: 0, y: 1, fg: Rgb(28, 100, 200), bg: Rgb(244, 244, 244), underline: Reset, modifier: NONE, + x: 0, y: 1, fg: Rgb(28, 100, 200), bg: Rgb(249, 249, 249), underline: Reset, modifier: NONE, x: 12, y: 1, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, - x: 0, y: 3, fg: Reset, bg: Rgb(244, 244, 244), underline: Reset, modifier: BOLD | DIM, - x: 2, y: 3, fg: Rgb(28, 100, 200), bg: Rgb(244, 244, 244), underline: Reset, modifier: NONE, - x: 12, y: 3, fg: Reset, bg: Rgb(244, 244, 244), underline: Reset, modifier: NONE, + x: 0, y: 3, fg: Reset, bg: Rgb(249, 249, 249), underline: Reset, modifier: BOLD | DIM, + x: 2, y: 3, fg: Rgb(28, 100, 200), bg: Rgb(249, 249, 249), underline: Reset, modifier: NONE, + x: 12, y: 3, fg: Reset, bg: Rgb(249, 249, 249), underline: Reset, modifier: NONE, x: 27, y: 3, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, ] } @@ -32,11 +33,11 @@ expression: "snapshots.join(\"\\n\")" ], styles: [ x: 0, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, - x: 0, y: 1, fg: Rgb(99, 168, 248), bg: Rgb(46, 48, 57), underline: Reset, modifier: NONE, + x: 0, y: 1, fg: Rgb(99, 168, 248), bg: Rgb(55, 57, 66), underline: Reset, modifier: NONE, x: 12, y: 1, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, - x: 0, y: 3, fg: Reset, bg: Rgb(46, 48, 57), underline: Reset, modifier: BOLD | DIM, - x: 2, y: 3, fg: Rgb(99, 168, 248), bg: Rgb(46, 48, 57), underline: Reset, modifier: NONE, - x: 12, y: 3, fg: Reset, bg: Rgb(46, 48, 57), underline: Reset, modifier: NONE, + x: 0, y: 3, fg: Reset, bg: Rgb(55, 57, 66), underline: Reset, modifier: BOLD | DIM, + x: 2, y: 3, fg: Rgb(99, 168, 248), bg: Rgb(55, 57, 66), underline: Reset, modifier: NONE, + x: 12, y: 3, fg: Reset, bg: Rgb(55, 57, 66), underline: Reset, modifier: NONE, x: 27, y: 3, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, ] } @@ -51,11 +52,11 @@ expression: "snapshots.join(\"\\n\")" ], styles: [ x: 0, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, - x: 0, y: 1, fg: Rgb(243, 248, 254), bg: Rgb(114, 114, 114), underline: Reset, modifier: NONE, + x: 0, y: 1, fg: Rgb(5, 9, 14), bg: Rgb(120, 120, 120), underline: Reset, modifier: NONE, x: 12, y: 1, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, - x: 0, y: 3, fg: Reset, bg: Rgb(114, 114, 114), underline: Reset, modifier: BOLD | DIM, - x: 2, y: 3, fg: Rgb(243, 248, 254), bg: Rgb(114, 114, 114), underline: Reset, modifier: NONE, - x: 12, y: 3, fg: Reset, bg: Rgb(114, 114, 114), underline: Reset, modifier: NONE, + x: 0, y: 3, fg: Reset, bg: Rgb(120, 120, 120), underline: Reset, modifier: BOLD | DIM, + x: 2, y: 3, fg: Rgb(5, 9, 14), bg: Rgb(120, 120, 120), underline: Reset, modifier: NONE, + x: 12, y: 3, fg: Reset, bg: Rgb(120, 120, 120), underline: Reset, modifier: NONE, x: 27, y: 3, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, ] } diff --git a/codex-rs/tui/src/history_cell/spoken_artifacts.rs b/codex-rs/tui/src/history_cell/spoken_artifacts.rs index ae13971b9d..c44cb43c7f 100644 --- a/codex-rs/tui/src/history_cell/spoken_artifacts.rs +++ b/codex-rs/tui/src/history_cell/spoken_artifacts.rs @@ -7,12 +7,14 @@ use crate::width::display_width; use ratatui::style::Modifier; use ratatui::text::Span; use std::path::Path; +use std::sync::Arc; const MAX_ARTIFACT_CANDIDATES: usize = 16; pub(super) fn annotate_spoken_artifacts(lines: &mut [HyperlinkLine], cwd: &Path) { let mut inspected = 0; - for line in lines { + let mut source_patches = Vec::new(); + 'lines: for line in &mut *lines { let text = line .line .spans @@ -46,7 +48,7 @@ pub(super) fn annotate_spoken_artifacts(lines: &mut [HyperlinkLine], cwd: &Path) } inspected += 1; if inspected > MAX_ARTIFACT_CANDIDATES { - return; + break 'lines; } let Some(file) = TrustedWorkspaceFile::validate(cwd, path) else { continue; @@ -61,6 +63,17 @@ pub(super) fn annotate_spoken_artifacts(lines: &mut [HyperlinkLine], cwd: &Path) } line.hyperlinks .push(TerminalHyperlink::trusted_workspace_file(columns, file)); + if let Some(source) = &line.source { + let start = byte_start.max(source.prefix_bytes); + let end = byte_end.min(source.prefix_bytes + source.range.len()); + if start < end { + source_patches.push(( + Arc::clone(&source.text), + source.range.start + start - source.prefix_bytes + ..source.range.start + end - source.prefix_bytes, + )); + } + } let mut offset = 0; for span in std::mem::take(&mut line.line.spans) { @@ -91,4 +104,52 @@ pub(super) fn annotate_spoken_artifacts(lines: &mut [HyperlinkLine], cwd: &Path) } line.hyperlinks.sort_by_key(|link| link.columns.start); } + + // Every wrapped fragment must retain all annotations on its shared logical line. + while let Some((text, range)) = source_patches.pop() { + let mut ranges = vec![range]; + source_patches.retain(|(other, range)| { + if Arc::ptr_eq(&text, other) { + ranges.push(range.clone()); + false + } else { + true + } + }); + let mut sources = lines + .iter_mut() + .filter_map(|line| line.source.as_mut()) + .filter(|source| Arc::ptr_eq(&source.text, &text)); + let Some(first) = sources.next() else { + continue; + }; + let mut styles = Vec::new(); + for (span, style) in first.styles.iter() { + let mut boundaries = vec![span.start, span.end]; + boundaries.extend( + ranges + .iter() + .flat_map(|range| [range.start, range.end]) + .filter(|offset| span.start < *offset && *offset < span.end), + ); + boundaries.sort_unstable(); + boundaries.dedup(); + for pair in boundaries.windows(/*size*/ 2) { + let range = pair[0]..pair[1]; + let style = if ranges + .iter() + .any(|patch| patch.start < range.end && range.start < patch.end) + { + style.add_modifier(Modifier::UNDERLINED) + } else { + *style + }; + styles.push((range, style)); + } + } + first.styles = styles.into(); + for source in sources { + source.styles = Arc::clone(&first.styles); + } + } } diff --git a/codex-rs/tui/src/history_cell/tests.rs b/codex-rs/tui/src/history_cell/tests.rs index 8f3958011b..5cdf6eb781 100644 --- a/codex-rs/tui/src/history_cell/tests.rs +++ b/codex-rs/tui/src/history_cell/tests.rs @@ -120,13 +120,17 @@ fn streaming_agent_tail_blank_line_uses_one_viewport_row() { let cell = StreamingAgentTailCell::new( vec![ HyperlinkLine::from("first"), - HyperlinkLine::from(""), + HyperlinkLine::from(" "), HyperlinkLine::from("second"), ], /*is_first_line*/ false, ); - let lines = cell.display_lines(/*width*/ 80); + let rendered = cell.display_hyperlink_lines(/*width*/ 80); + let source = rendered[1].source.as_ref().expect("blank row source"); + assert_eq!((source.prefix_bytes, source.range.clone()), (0, 0..0)); + assert_eq!(source.text.as_ref(), " "); + let lines = visible_lines(rendered); insta::assert_snapshot!(render_lines(&lines).join("\n"), @" first second"); diff --git a/codex-rs/tui/src/insert_history.rs b/codex-rs/tui/src/insert_history.rs index 73027d7581..e5ad5a277a 100644 --- a/codex-rs/tui/src/insert_history.rs +++ b/codex-rs/tui/src/insert_history.rs @@ -332,6 +332,7 @@ fn write_history_line( }) .collect(); let merged_line = HyperlinkLine { + source: None, line: Line::from(merged_spans), hyperlinks: line.hyperlinks.clone(), }; diff --git a/codex-rs/tui/src/markdown_render.rs b/codex-rs/tui/src/markdown_render.rs index 4cdd619a55..4decb1eb01 100644 --- a/codex-rs/tui/src/markdown_render.rs +++ b/codex-rs/tui/src/markdown_render.rs @@ -2001,6 +2001,11 @@ where } } else { let mut spans = self.current_initial_indent.clone(); + let mut source = + crate::terminal_hyperlinks::LogicalLineSource::from_line(&line.line); + source.prefix_bytes = spans.iter().map(|span| span.content.len()).sum(); + source.continuation_indent = self.current_subsequent_indent.clone().into(); + line.source = Some(source); let shift = Self::spans_display_width(&spans); spans.append(&mut line.line.spans); for hyperlink in &mut line.hyperlinks { @@ -2180,6 +2185,7 @@ mod markdown_render_tests { mod tests { use super::*; use pretty_assertions::assert_eq; + use ratatui::style::Stylize; use ratatui::text::Text; fn lines_to_strings(text: &Text<'_>) -> Vec { @@ -2257,9 +2263,19 @@ mod tests { #[test] fn wraps_blockquotes() { - let markdown = "> block quote with content that should wrap nicely"; - let rendered = render_markdown_text_with_width(markdown, Some(22)); - let lines = lines_to_strings(&rendered); + let markdown = "> block quote with **content** that should wrap nicely"; + let rendered = + render_markdown_lines_with_width_and_cwd(markdown, Some(22), /*cwd*/ None); + let source = rendered[0].source.as_ref().expect("blockquote source"); + assert_eq!( + source.styled_range(0..source.text.len()), + Line::from(vec![ + "block quote with ".green(), + "content".green().bold(), + " that should wrap nicely".green(), + ]) + ); + let lines: Vec<_> = rendered.iter().map(|line| line.line.to_string()).collect(); assert_eq!( lines, vec![ diff --git a/codex-rs/tui/src/markdown_render/web_links_tests.rs b/codex-rs/tui/src/markdown_render/web_links_tests.rs index d503f77417..608f77f406 100644 --- a/codex-rs/tui/src/markdown_render/web_links_tests.rs +++ b/codex-rs/tui/src/markdown_render/web_links_tests.rs @@ -79,6 +79,7 @@ fn supporting_terminals_render_only_the_styled_label_and_keep_its_target() { assert_eq!( render(markdown, /*width*/ 80, display), vec![HyperlinkLine { + source: None, line: Line::from(label), hyperlinks: vec![TerminalHyperlink::web( 0..label_width, diff --git a/codex-rs/tui/src/multi_agents.rs b/codex-rs/tui/src/multi_agents.rs index cfa05f74dd..1604be9b29 100644 --- a/codex-rs/tui/src/multi_agents.rs +++ b/codex-rs/tui/src/multi_agents.rs @@ -318,6 +318,7 @@ pub(crate) fn sub_agent_activity_history_cell(item: &ThreadItem) -> Option String { match kind { SubAgentActivityKind::Started => format!("Started `{agent_path}`"), diff --git a/codex-rs/tui/src/streaming/controller_preview_tests.rs b/codex-rs/tui/src/streaming/controller_preview_tests.rs index 60baca07e9..19d4820f94 100644 --- a/codex-rs/tui/src/streaming/controller_preview_tests.rs +++ b/codex-rs/tui/src/streaming/controller_preview_tests.rs @@ -2,6 +2,38 @@ use super::*; use crate::terminal_hyperlinks::visible_lines; use pretty_assertions::assert_eq; +#[test] +fn source_only_changes_refresh_the_preview_and_active_tail() { + let cwd = std::env::temp_dir(); + let mut controller = StreamController::new(Some(5), &cwd, HistoryRenderMode::Rich); + controller.push("hello"); + let before = controller.current_tail_lines(); + + // An entity preserves trailing whitespace that Markdown otherwise trims at EOF. + assert!(controller.push(" ")); + let after = controller.current_tail_lines(); + assert_eq!(visible_lines(before.clone()), visible_lines(after.clone())); + assert_eq!( + after[0].source, + render_source( + "hello ", + Some(5), + &cwd, + HistoryRenderMode::Rich, + /*inline_visualization_context*/ None, + )[0] + .source, + ); + assert_ne!( + history_cell::StreamingAgentTailCell::new(before.clone(), /*is_first_line*/ true), + history_cell::StreamingAgentTailCell::new(after.clone(), /*is_first_line*/ true), + ); + assert_ne!( + history_cell::StreamingPlanTailCell::new(before, /*is_stream_continuation*/ false), + history_cell::StreamingPlanTailCell::new(after, /*is_stream_continuation*/ false), + ); +} + #[test] fn unterminated_prose_reflows_and_finishes_without_duplication() { let cwd = std::env::temp_dir(); diff --git a/codex-rs/tui/src/streaming/prose_preview.rs b/codex-rs/tui/src/streaming/prose_preview.rs index 94a848a330..b8b4cdc1cf 100644 --- a/codex-rs/tui/src/streaming/prose_preview.rs +++ b/codex-rs/tui/src/streaming/prose_preview.rs @@ -6,6 +6,7 @@ use super::render::render_source; use crate::history_cell::HistoryRenderMode; use crate::inline_visualization::InlineVisualizationContext; use crate::terminal_hyperlinks::HyperlinkLine; +use crate::terminal_hyperlinks::lines_with_sources_eq; use ratatui::text::Line; use std::path::Path; @@ -67,7 +68,7 @@ impl ProsePreview { if start > 0 { lines.insert(0, HyperlinkLine::new(Line::from("…"))); } - if self.lines == lines { + if lines_with_sources_eq(&self.lines, &lines) { return false; } self.lines = lines; diff --git a/codex-rs/tui/src/style.rs b/codex-rs/tui/src/style.rs index 32e64ca6b8..dee9f16ed6 100644 --- a/codex-rs/tui/src/style.rs +++ b/codex-rs/tui/src/style.rs @@ -62,7 +62,6 @@ pub fn user_message_style() -> Style { } /// Submitted prompts use a lighter fill than the editable composer in either theme. -#[allow(dead_code, reason = "Used by later layers of the TUI refresh stack.")] pub(crate) fn history_prompt_style() -> Style { let Some(background) = default_bg() else { return Style::default(); diff --git a/codex-rs/tui/src/terminal_hyperlinks.rs b/codex-rs/tui/src/terminal_hyperlinks.rs index 390d50d309..7fa6a83045 100644 --- a/codex-rs/tui/src/terminal_hyperlinks.rs +++ b/codex-rs/tui/src/terminal_hyperlinks.rs @@ -4,8 +4,10 @@ //! when text reaches a terminal buffer or scrollback writer so OSC 8 bytes never affect geometry. mod paragraph; +mod source; pub(crate) use paragraph::HyperlinkParagraph; +pub(crate) use source::LogicalLineSource; use std::num::NonZeroU16; use std::ops::Range; @@ -31,7 +33,6 @@ use crate::render::line_utils::line_to_borrowed; use crate::render::line_utils::line_to_static; use crate::width::display_width; use crate::wrapping::RtOptions; -use crate::wrapping::adaptive_wrap_line; // Destinations are repeated in every linked buffer cell. Leave oversized URLs as plain text. const MAX_HYPERLINK_DESTINATION_BYTES: usize = 8 * 1024; @@ -120,7 +121,7 @@ impl TerminalHyperlink { } } - fn with_columns(&self, columns: Range) -> Self { + pub(crate) fn with_columns(&self, columns: Range) -> Self { Self { columns, destination: self.destination.clone(), @@ -136,10 +137,40 @@ impl TerminalHyperlink { } } -#[derive(Clone, Debug, Default, Eq, PartialEq)] +#[derive(Clone, Default)] pub(crate) struct HyperlinkLine { pub(crate) line: Line<'static>, pub(crate) hyperlinks: Vec, + pub(crate) source: Option, +} + +// Source provenance is shared layout metadata; omit it from visual diagnostics to avoid +// repeating an entire logical line for every wrapped fragment. +impl std::fmt::Debug for HyperlinkLine { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + formatter + .debug_struct("HyperlinkLine") + .field("line", &self.line) + .field("hyperlinks", &self.hyperlinks) + .finish() + } +} + +impl PartialEq for HyperlinkLine { + fn eq(&self, other: &Self) -> bool { + self.line == other.line && self.hyperlinks == other.hyperlinks + } +} + +impl Eq for HyperlinkLine {} + +/// Cache equality also tracks source whitespace hidden by display wrapping. +pub(crate) fn lines_with_sources_eq(left: &[HyperlinkLine], right: &[HyperlinkLine]) -> bool { + left == right + && left + .iter() + .map(|line| &line.source) + .eq(right.iter().map(|line| &line.source)) } impl HyperlinkLine { @@ -147,6 +178,7 @@ impl HyperlinkLine { Self { line, hyperlinks: Vec::new(), + source: None, } } @@ -155,6 +187,7 @@ impl HyperlinkLine { } pub(crate) fn push_span(&mut self, span: Span<'static>, destination: Option<&str>) { + self.source = None; let start = self.width(); let end = start + display_width(span.content.as_ref()); self.line.push_span(span); @@ -168,6 +201,9 @@ impl HyperlinkLine { pub(crate) fn style(mut self, style: ratatui::style::Style) -> Self { self.line = self.line.style(style); + if let Some(source) = &mut self.source { + source.line_style = style; + } self } } @@ -220,6 +256,16 @@ pub(crate) fn prefix_hyperlink_lines( subsequent_prefix.clone() }; let shift = display_width(prefix.content.as_ref()); + let mut source = line + .source + .take() + .unwrap_or_else(|| LogicalLineSource::from_line(&line.line)); + source.prefix_bytes += prefix.content.len(); + source + .continuation_indent + .spans + .insert(/*index*/ 0, subsequent_prefix.clone()); + line.source = Some(source); let mut spans = Vec::with_capacity(line.line.spans.len() + 1); spans.push(prefix); spans.extend(line.line.spans); @@ -245,12 +291,14 @@ pub(crate) fn adaptive_wrap_hyperlink_lines( .clone() .initial_indent(options.subsequent_indent.clone()) }; - out.extend(remap_wrapped_line( - line, - adaptive_wrap_line(&line.line, options) - .into_iter() - .map(|wrapped| line_to_static(&wrapped)) - .collect(), + let mut source = line.clone(); + source + .source + .get_or_insert_with(|| LogicalLineSource::from_line(&line.line)) + .continuation_indent = options.subsequent_indent.clone(); + out.extend(remap_source_wrapped_line( + &source, + crate::wrapping::adaptive_wrap_line_with_source(&line.line, options), )); } out @@ -271,11 +319,67 @@ pub(crate) fn annotate_web_urls_in_line(line: Line<'static>) -> HyperlinkLine { out } +/// Project annotations from the exact source slices used by the existing wrapping algorithm. +pub(crate) fn remap_source_wrapped_line( + source: &HyperlinkLine, + wrapped: Vec>, +) -> Vec { + let text = line_text(&source.line); + let mut logical = source + .source + .clone() + .unwrap_or_else(|| LogicalLineSource::from_line(&source.line)); + // Wrapping bakes the row style into each span; mirror it once for all source fragments. + if logical.line_style != ratatui::style::Style::default() { + logical.styles = logical + .styles + .iter() + .map(|(range, style)| (range.clone(), logical.line_style.patch(*style))) + .collect::>() + .into(); + } + let mut source_byte = 0; + let mut source_column = 0; + wrapped + .into_iter() + .map(|wrapped| { + let line = line_to_static(&wrapped.line); + let displayed = line_text(&line); + let prefix_columns = display_width(&displayed[..wrapped.prefix_bytes]); + source_column += display_width(&text[source_byte..wrapped.range.start]); + let start = source_column; + let end = start + display_width(&text[wrapped.range.clone()]); + source_byte = wrapped.range.end; + source_column = end; + let hyperlinks = source + .hyperlinks + .iter() + .filter_map(|link| { + let first = link.columns.start.max(start); + let last = link.columns.end.min(end); + (first < last).then(|| { + link.with_columns( + prefix_columns + first - start..prefix_columns + last - start, + ) + }) + }) + .collect(); + HyperlinkLine { + line, + hyperlinks, + source: Some(logical.wrapped(wrapped.range, wrapped.prefix_bytes)), + } + }) + .collect() +} + /// Re-attach source hyperlink ranges after visible-text wrapping has split a line. /// /// Link text is matched in display order so a URL split across table rows retains the complete /// destination on every rendered fragment. Whitespace inserted or removed at line boundaries is /// ignored while matching; hyperlink destinations themselves are never reconstructed from output. +/// This legacy projection does not infer logical source provenance. New wrapping callers should +/// pass authoritative ranges to [`remap_source_wrapped_line`]. pub(crate) fn remap_wrapped_line( source: &HyperlinkLine, wrapped: Vec>, @@ -284,7 +388,6 @@ pub(crate) fn remap_wrapped_line( if source.hyperlinks.is_empty() { return out; } - let source_text = line_text(&source.line); let mut source_byte = 0usize; let mut source_column = 0usize; @@ -758,6 +861,7 @@ mod tests { fn decorates_a_contiguous_web_link_with_one_osc8_pair() { let destination = "https://example.com/a/very/long/path"; let line = HyperlinkLine { + source: None, line: Line::from(destination), hyperlinks: vec![TerminalHyperlink::web( /*columns*/ 0..usize::from(destination.cell_width()), @@ -823,6 +927,7 @@ mod tests { wrapped, vec![ HyperlinkLine { + source: None, line: Line::from(" alpha πŸ˜€here"), hyperlinks: vec![TerminalHyperlink::web( /*columns*/ 10..14, @@ -830,6 +935,7 @@ mod tests { )], }, HyperlinkLine { + source: None, line: Line::from(" middle there end"), hyperlinks: vec![TerminalHyperlink::web( /*columns*/ 11..16, @@ -1091,6 +1197,7 @@ mod tests { ); link.retarget_to_trusted_file(&file_url); let line = HyperlinkLine { + source: None, line: Line::from("view"), hyperlinks: vec![link], }; diff --git a/codex-rs/tui/src/terminal_hyperlinks/source.rs b/codex-rs/tui/src/terminal_hyperlinks/source.rs new file mode 100644 index 0000000000..cb1a343c52 --- /dev/null +++ b/codex-rs/tui/src/terminal_hyperlinks/source.rs @@ -0,0 +1,114 @@ +//! Logical text and margins retained when display wrapping removes whitespace or adds a gutter. + +use ratatui::style::Style; +use ratatui::text::Line; +use ratatui::text::Span; +use std::ops::Range; +use std::sync::Arc; + +/// Wrapping policy of an existing display renderer. +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] +pub(crate) enum LineWrapPolicy { + #[default] + Word, +} + +/// A displayed line's contiguous fragment of one original logical line. +/// +/// Wrapped fragments share `text`. `prefix_bytes` counts synthetic display bytes before +/// the fragment, so selection never has to infer whether a gutter belongs to the source. +#[derive(Clone, Debug, Eq, PartialEq)] +pub(crate) struct LogicalLineSource { + pub(crate) text: Arc, + /// Span styles share the text's byte coordinates, including whitespace omitted by wrapping. + pub(crate) styles: Arc<[(Range, Style)]>, + /// Row style, applied beneath explicit span styles just like ratatui's `Line`. + pub(crate) line_style: Style, + /// A later uniform span-style patch, such as the dim/italic reasoning treatment. + pub(crate) span_style: Style, + pub(crate) range: Range, + pub(crate) prefix_bytes: usize, + pub(crate) wrap_policy: LineWrapPolicy, + pub(crate) continuation_indent: Line<'static>, + /// Blank columns after wrapped content; they remain outside the copied source text. + pub(crate) right_reserve: u16, +} + +impl LogicalLineSource { + pub(crate) fn new(text: String) -> Self { + let end = text.len(); + Self { + text: text.into(), + styles: vec![(0..end, Style::default())].into(), + line_style: Style::default(), + span_style: Style::default(), + range: 0..end, + prefix_bytes: 0, + wrap_policy: LineWrapPolicy::Word, + continuation_indent: Line::default(), + right_reserve: 0, + } + } + + pub(crate) fn from_line(line: &Line<'_>) -> Self { + let mut source = Self::new( + line.spans + .iter() + .map(|span| span.content.as_ref()) + .collect(), + ); + let mut offset = 0; + source.line_style = line.style; + source.styles = line + .spans + .iter() + .map(|span| { + let start = offset; + offset += span.content.len(); + (start..offset, span.style) + }) + .collect::>() + .into(); + source + } + + #[allow( + dead_code, + reason = "Used by the activity presentation layer of this stack." + )] + pub(crate) fn styled_range(&self, range: Range) -> Line<'static> { + Line::from( + self.styles + .iter() + .filter_map(|(style_range, style)| { + let start = style_range.start.max(range.start); + let end = style_range.end.min(range.end); + (start < end).then(|| { + Span::styled( + self.text[start..end].to_owned(), + self.line_style.patch(*style).patch(self.span_style), + ) + }) + }) + .collect::>(), + ) + } + + pub(super) fn wrapped(&self, displayed: Range, prefix_bytes: usize) -> Self { + let source_start = self.prefix_bytes; + let source_end = source_start + self.range.len(); + let start = displayed.start.clamp(source_start, source_end); + let end = displayed.end.clamp(source_start, source_end); + Self { + text: Arc::clone(&self.text), + styles: Arc::clone(&self.styles), + line_style: self.line_style, + span_style: self.span_style, + range: self.range.start + start - source_start..self.range.start + end - source_start, + prefix_bytes: prefix_bytes + start.saturating_sub(displayed.start).min(displayed.len()), + wrap_policy: self.wrap_policy, + continuation_indent: self.continuation_indent.clone(), + right_reserve: self.right_reserve, + } + } +} diff --git a/codex-rs/tui/src/terminal_hyperlinks_tests.rs b/codex-rs/tui/src/terminal_hyperlinks_tests.rs index 8648631cef..ead3f3662b 100644 --- a/codex-rs/tui/src/terminal_hyperlinks_tests.rs +++ b/codex-rs/tui/src/terminal_hyperlinks_tests.rs @@ -15,6 +15,7 @@ fn oversized_destinations_remain_plain_text() { // Explicit Markdown links can reach the cell marker without bare-URL detection. let line = HyperlinkLine { + source: None, line: Line::from("visible"), hyperlinks: vec![TerminalHyperlink::web(/*columns*/ 0..7, oversized)], }; diff --git a/codex-rs/tui/src/wrapping.rs b/codex-rs/tui/src/wrapping.rs index d65c6b88aa..a1d35ff5a4 100644 --- a/codex-rs/tui/src/wrapping.rs +++ b/codex-rs/tui/src/wrapping.rs @@ -46,12 +46,16 @@ struct ProjectedText { grapheme_boundaries: Vec, } -/// Replaces halfwidth sound-mark graphemes with equally wide, textwrap-safe placeholders. +/// Replaces compound graphemes with equally wide, textwrap-safe placeholders. /// /// Source boundaries recover original byte offsets, while grapheme boundaries keep placeholders /// indivisible and preserve leading whitespace as a wrapping opportunity. -fn project_halfwidth_sound_marks(text: &str) -> Option { - if !text.contains(['\u{FF9E}', '\u{FF9F}']) { +fn project_complex_graphemes(text: &str) -> Option { + if !text.contains(['\u{FF9E}', '\u{FF9F}']) + && !text + .graphemes(/*is_extended*/ true) + .any(|grapheme| grapheme.chars().count() > 1) + { return None; } @@ -59,7 +63,7 @@ fn project_halfwidth_sound_marks(text: &str) -> Option { let mut source_boundaries = vec![(0, 0)]; let mut grapheme_boundaries = vec![0]; for (source_start, grapheme) in text.grapheme_indices(/*is_extended*/ true) { - if grapheme.contains(['\u{FF9E}', '\u{FF9F}']) { + if grapheme.chars().count() > 1 || grapheme.contains(['\u{FF9E}', '\u{FF9F}']) { let source_end = source_start + grapheme.len(); let content_start = grapheme .find(|ch: char| !ch.is_whitespace()) @@ -72,6 +76,10 @@ fn project_halfwidth_sound_marks(text: &str) -> Option { } let width = display_width(content); + if width == 0 && !content.is_empty() { + projected.push_str(content); + source_boundaries.push((projected.len(), source_end)); + } let projected_start = projected.len(); for _ in 0..width / 2 { if projected.len() > projected_start { @@ -233,7 +241,7 @@ where O: Into>, { let opts = width_or_options.into(); - if let Some(projected) = project_halfwidth_sound_marks(text) { + if let Some(projected) = project_complex_graphemes(text) { return wrap_projected_ranges(&projected, &opts, /*include_trailing_spaces*/ true); } let mut lines: Vec> = Vec::new(); @@ -279,7 +287,7 @@ where O: Into>, { let opts = width_or_options.into(); - if let Some(projected) = project_halfwidth_sound_marks(text) { + if let Some(projected) = project_complex_graphemes(text) { return wrap_projected_ranges(&projected, &opts, /*include_trailing_spaces*/ false); } let mut lines: Vec> = Vec::new(); @@ -701,6 +709,24 @@ pub(crate) fn url_preserving_wrap_options<'a>(opts: RtOptions<'a>) -> RtOptions< /// while a genuinely overlong non-URL token can still split if needed. #[must_use] pub(crate) fn adaptive_wrap_line<'a>(line: &'a Line<'a>, base: RtOptions<'a>) -> Vec> { + adaptive_wrap_line_with_source(line, base) + .into_iter() + .map(|wrapped| wrapped.line) + .collect() +} + +/// A display row and the exact source fragment used to build it. +pub(crate) struct WrappedLine<'a> { + pub(crate) line: Line<'a>, + pub(crate) range: Range, + pub(crate) prefix_bytes: usize, +} + +/// Preserve wrapping's source ranges for selection and hyperlink projection. +pub(crate) fn adaptive_wrap_line_with_source<'a>( + line: &'a Line<'a>, + base: RtOptions<'a>, +) -> Vec> { let (flat, span_bounds) = flatten_line(line); let mut saw_url = false; let mut saw_non_url = false; @@ -854,6 +880,20 @@ impl<'a> RtOptions<'a> { #[must_use] pub(crate) fn word_wrap_line<'a, O>(line: &'a Line<'a>, width_or_options: O) -> Vec> +where + O: Into>, +{ + word_wrap_line_with_source(line, width_or_options) + .into_iter() + .map(|wrapped| wrapped.line) + .collect() +} + +/// Standard wrapping with the same source ranges used to slice styled spans. +pub(crate) fn word_wrap_line_with_source<'a, O>( + line: &'a Line<'a>, + width_or_options: O, +) -> Vec> where O: Into>, { @@ -866,7 +906,7 @@ fn word_wrap_flattened_line<'a>( flat: &str, span_bounds: &[(Range, ratatui::style::Style)], rt_opts: RtOptions<'a>, -) -> Vec> { +) -> Vec> { let opts = Options::new(rt_opts.width) .line_ending(rt_opts.line_ending) .break_words(rt_opts.break_words) @@ -874,7 +914,7 @@ fn word_wrap_flattened_line<'a>( .word_separator(rt_opts.word_separator) .word_splitter(rt_opts.word_splitter); - let mut out: Vec> = Vec::new(); + let mut out: Vec> = Vec::new(); // Compute first line range with reduced width due to initial indent. let initial_width_available = opts @@ -883,7 +923,16 @@ fn word_wrap_flattened_line<'a>( .max(1); let initial_wrapped = wrap_ranges_trim(flat, opts.clone().width(initial_width_available)); let Some(first_line_range) = initial_wrapped.first() else { - return vec![rt_opts.initial_indent.clone()]; + return vec![WrappedLine { + line: rt_opts.initial_indent.clone().style(line.style), + range: 0..0, + prefix_bytes: rt_opts + .initial_indent + .spans + .iter() + .map(|span| span.content.len()) + .sum(), + }]; }; // Build first wrapped line with initial indent. @@ -895,14 +944,22 @@ fn word_wrap_flattened_line<'a>( &mut sliced .spans .into_iter() - .map(|s| s.patch_style(line.style)) + .map(|span| Span::styled(span.content, line.style.patch(span.style))) .collect(), ); first_line.spans = spans; - out.push(first_line); + out.push(WrappedLine { + line: first_line, + range: first_line_range.clone(), + prefix_bytes: rt_opts + .initial_indent + .spans + .iter() + .map(|span| span.content.len()) + .sum(), + }); } - // Wrap the remainder using subsequent indent width and map back to original indices. let base = first_line_range.end; let skip_leading_spaces = flat[base..].chars().take_while(|c| *c == ' ').count(); let base = base + skip_leading_spaces; @@ -910,24 +967,55 @@ fn word_wrap_flattened_line<'a>( .width .saturating_sub(line_width(&rt_opts.subsequent_indent)) .max(1); - let remaining_wrapped = wrap_ranges_trim(&flat[base..], opts.width(subsequent_width_available)); - for r in &remaining_wrapped { - if r.is_empty() { + // First-fit decisions do not depend on later rows. Reuse the full first pass when + // both indents leave the same width and the remainder starts after a complete word. + // Splitting inside a word can change hyphenation when the remainder is tokenized again. + // Custom tokenizers can also repartition the remainder, so they retain the second pass. + let remaining_wrapped = if initial_width_available == subsequent_width_available + && matches!(rt_opts.wrap_algorithm, textwrap::WrapAlgorithm::FirstFit) + && matches!( + rt_opts.word_separator, + WordSeparator::AsciiSpace | WordSeparator::UnicodeBreakProperties + ) + && (skip_leading_spaces > 0 || base == flat.len()) + // The projected and plain text wrappers interpret control characters differently. + && !flat.as_bytes().iter().any(u8::is_ascii_control) + && initial_wrapped + .get(/*index*/ 1) + .map_or(base == flat.len(), |range| range.start == base) + { + initial_wrapped.into_iter().skip(/*n*/ 1).collect() + } else { + wrap_ranges_trim(&flat[base..], opts.width(subsequent_width_available)) + .into_iter() + .map(|range| (range.start + base)..(range.end + base)) + .collect::>() + }; + for offset_range in remaining_wrapped { + if offset_range.is_empty() { continue; } let mut subsequent_line = rt_opts.subsequent_indent.clone().style(line.style); - let offset_range = (r.start + base)..(r.end + base); let sliced = slice_line_spans(line, span_bounds, &offset_range); let mut spans = subsequent_line.spans; spans.append( &mut sliced .spans .into_iter() - .map(|s| s.patch_style(line.style)) + .map(|span| Span::styled(span.content, line.style.patch(span.style))) .collect(), ); subsequent_line.spans = spans; - out.push(subsequent_line); + out.push(WrappedLine { + line: subsequent_line, + range: offset_range, + prefix_bytes: rt_opts + .subsequent_indent + .spans + .iter() + .map(|span| span.content.len()) + .sum(), + }); } out @@ -950,7 +1038,7 @@ fn mixed_url_wrap_line<'a>( flat: &str, span_bounds: &[(Range, ratatui::style::Style)], rt_opts: RtOptions<'a>, -) -> Vec> { +) -> Vec> { let initial_width_available = rt_opts .width .saturating_sub(line_width(&rt_opts.initial_indent)) @@ -969,20 +1057,38 @@ fn mixed_url_wrap_line<'a>( rt_opts.subsequent_indent.clone() } .style(line.style); + let prefix_bytes = wrapped_line + .spans + .iter() + .map(|span| span.content.len()) + .sum(); let sliced = slice_line_spans(line, span_bounds, range); let mut spans = wrapped_line.spans; spans.extend( sliced .spans .into_iter() - .map(|span| span.patch_style(line.style)), + .map(|span| Span::styled(span.content, line.style.patch(span.style))), ); wrapped_line.spans = spans; - out.push(wrapped_line); + out.push(WrappedLine { + line: wrapped_line, + range: range.clone(), + prefix_bytes, + }); } if out.is_empty() { - vec![rt_opts.initial_indent.clone()] + vec![WrappedLine { + line: rt_opts.initial_indent.clone().style(line.style), + range: 0..0, + prefix_bytes: rt_opts + .initial_indent + .spans + .iter() + .map(|span| span.content.len()) + .sum(), + }] } else { out } @@ -1269,6 +1375,10 @@ fn slice_line_spans<'a>( } } +#[cfg(test)] +#[path = "wrapping_reuse_tests.rs"] +mod reuse_tests; + #[cfg(test)] mod tests { use super::*; @@ -1293,6 +1403,24 @@ mod tests { assert_eq!(concat_line(&out[0]), "hello"); } + #[test] + fn narrow_wrap_ranges_preserve_compound_graphemes() { + assert_eq!( + wrap_ranges_trim("πŸ‘©β€πŸ’»e\u{301}x", /*width_or_options*/ 2), + vec![0..11, 11..15], + ); + for text in ["\u{301}\u{302}", " \u{301}\u{302}"] { + assert_eq!( + wrap_ranges_trim(text, /*width_or_options*/ 2), + vec![0..text.len()] + ); + } + assert_eq!( + wrap_ranges("\u{301}\u{302}", /*width_or_options*/ 2), + vec![0..5], + ); + } + #[test] fn simple_unstyled_wrap_narrow_width() { let line = Line::from("hello world"); diff --git a/codex-rs/tui/src/wrapping_reuse_tests.rs b/codex-rs/tui/src/wrapping_reuse_tests.rs new file mode 100644 index 0000000000..0dcd15c8a0 --- /dev/null +++ b/codex-rs/tui/src/wrapping_reuse_tests.rs @@ -0,0 +1,114 @@ +//! Compare reused first-fit rows with the generic wrapping path, including source provenance. + +use super::*; +use pretty_assertions::assert_eq; +use ratatui::style::Stylize; + +#[test] +fn custom_word_separator_reprocesses_the_remainder() { + let separator = WordSeparator::Custom(|text| { + let Some(space) = text.find(' ') else { + return Box::new(std::iter::once(Word::from(text))); + }; + Box::new([Word::from(&text[..=space]), Word::from(&text[space + 1..])].into_iter()) + }); + let line = Line::from("one two three"); + let rows = + word_wrap_line_with_source(&line, RtOptions::new(/*width*/ 5).word_separator(separator)) + .into_iter() + .map(|row| (row.line, row.range)) + .collect::>(); + assert_eq!( + rows, + vec![ + (Line::from("one"), 0..3), + (Line::from("two"), 4..7), + (Line::from("three"), 8..13), + ] + ); +} + +#[test] +fn first_fit_reuse_preserves_styled_rows_and_source_ranges() { + // A custom first-fit function exercises the unchanged generic path without copying + // its implementation into the test. Compare complete rows, styles and byte ranges. + let generic_first_fit = textwrap::WrapAlgorithm::Custom(|words, widths| { + textwrap::WrapAlgorithm::FirstFit.wrap(words, widths) + }); + // Cover distinct split, width, indentation and separator boundaries without a random matrix. + for (text, width, initial, subsequent, break_words, separator) in [ + ("", 0, "", "", true, WordSeparator::AsciiSpace), + ( + "hello world", + 0, + "> ", + " ", + true, + WordSeparator::AsciiSpace, + ), + ( + "cafe\u{301} δΈ­ζ–‡ πŸ‘©πŸ½β€πŸ’» πŸ‡§πŸ‡· 「゙", + 2, + "", + "", + true, + WordSeparator::UnicodeBreakProperties, + ), + ( + "a-very-long-hyphenated-word", + 5, + "η•Œ", + "ab", + true, + WordSeparator::AsciiSpace, + ), + ( + "https://example.com/a/b tail", + 17, + "> ", + " ", + false, + WordSeparator::AsciiSpace, + ), + ( + "alpha beta\u{a0}gamma\u{200b}delta", + 5, + "", + " ", + true, + WordSeparator::UnicodeBreakProperties, + ), + ( + "one\n two\r\nthree\tfour", + 5, + "> ", + " ", + true, + WordSeparator::AsciiSpace, + ), + ( + "\u{1b}[31mred\u{1b}[0m tail", + 80, + "", + "", + false, + WordSeparator::AsciiSpace, + ), + ] { + let line = Line::from(vec![text.bold(), " suffix".cyan()]).italic(); + let options = RtOptions::new(width) + .initial_indent(Line::from(initial.red())) + .subsequent_indent(Line::from(subsequent.green())) + .break_words(break_words) + .word_separator(separator); + let actual = word_wrap_line_with_source(&line, options.clone()) + .into_iter() + .map(|row| (row.line, row.range, row.prefix_bytes)) + .collect::>(); + let expected = word_wrap_line_with_source(&line, options.wrap_algorithm(generic_first_fit)) + .into_iter() + .map(|row| (row.line, row.range, row.prefix_bytes)) + .collect::>(); + assert_eq!(actual, expected, "text={text:?}, width={width}"); + } +}