diff --git a/codex-rs/tui/src/history_cell/messages.rs b/codex-rs/tui/src/history_cell/messages.rs index 28015dfee8..c026b5dcae 100644 --- a/codex-rs/tui/src/history_cell/messages.rs +++ b/codex-rs/tui/src/history_cell/messages.rs @@ -2,6 +2,10 @@ use super::markdown_render_cache::MarkdownRenderCache; use super::*; +use crate::terminal_hyperlinks::annotate_web_urls_in_line; +use crate::terminal_hyperlinks::remap_wrapped_line; +use crate::wrapping::url_preserving_wrap_options; +use crate::wrapping::word_wrap_line; use std::borrow::Cow; #[derive(Debug)] @@ -147,18 +151,12 @@ fn remote_image_display_line(style: Style, index: usize) -> Line<'static> { Line::from(local_image_label_text(index)).style(style) } -fn trim_trailing_blank_lines(mut lines: Vec>) -> Vec> { - while lines - .last() - .is_some_and(|line| line.spans.iter().all(|span| span.content.trim().is_empty())) - { - lines.pop(); - } - lines -} - impl HistoryCell for UserHistoryCell { fn display_lines(&self, width: u16) -> Vec> { + visible_lines(self.display_hyperlink_lines(width)) + } + + fn display_hyperlink_lines(&self, width: u16) -> Vec { let message = sanitize_user_text((&self.message).into()); let text_elements = if message.as_ref() == self.message { self.text_elements.as_slice() @@ -177,7 +175,7 @@ impl HistoryCell for UserHistoryCell { let wrapped_remote_images = if self.remote_image_urls.is_empty() { None } else { - Some(adaptive_wrap_lines( + Some(plain_hyperlink_lines(adaptive_wrap_lines( self.remote_image_urls .iter() .enumerate() @@ -186,36 +184,61 @@ impl HistoryCell for UserHistoryCell { }), RtOptions::new(usize::from(wrap_width)) .wrap_algorithm(textwrap::WrapAlgorithm::FirstFit), - )) + ))) }; let wrapped_message = if message.is_empty() && text_elements.is_empty() { None - } else if text_elements.is_empty() { - let message_without_trailing_newlines = message.trim_end_matches(['\r', '\n']); - let wrapped = adaptive_wrap_lines( - message_without_trailing_newlines - .split('\n') - .map(|line| Line::from(line).style(style)), - // Wrap algorithm matches textarea.rs. - RtOptions::new(usize::from(wrap_width)) - .wrap_algorithm(textwrap::WrapAlgorithm::FirstFit), - ); - let wrapped = trim_trailing_blank_lines(wrapped); - (!wrapped.is_empty()).then_some(wrapped) } else { - let raw_lines = build_user_message_lines_with_elements( - message.as_ref(), - text_elements, - style, - element_style, - ); - let wrapped = adaptive_wrap_lines( - raw_lines, - RtOptions::new(usize::from(wrap_width)) - .wrap_algorithm(textwrap::WrapAlgorithm::FirstFit), - ); - let wrapped = trim_trailing_blank_lines(wrapped); + 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.as_ref(), + 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) }; @@ -223,31 +246,35 @@ impl HistoryCell for UserHistoryCell { return Vec::new(); } - let mut lines: Vec> = vec![Line::from("").style(style)]; + let mut lines = vec![HyperlinkLine::new(Line::from("").style(style))]; if let Some(wrapped_remote_images) = wrapped_remote_images { - lines.extend(prefix_lines( + lines.extend(prefix_hyperlink_lines( wrapped_remote_images, " ".into(), " ".into(), )); if wrapped_message.is_some() { - lines.push(Line::from("").style(style)); + lines.push(HyperlinkLine::new(Line::from("").style(style))); } } if let Some(wrapped_message) = wrapped_message { - lines.extend(prefix_lines( + lines.extend(prefix_hyperlink_lines( wrapped_message, "› ".bold().dim(), " ".into(), )); } - lines.push(Line::from("").style(style)); + lines.push(HyperlinkLine::new(Line::from("").style(style))); lines } + fn transcript_hyperlink_lines(&self, width: u16) -> Vec { + self.display_hyperlink_lines(width) + } + fn raw_lines(&self) -> Vec> { let message = sanitize_user_text((&self.message).into()); let mut lines = raw_lines_from_source(message.as_ref().trim_end_matches(['\r', '\n'])); diff --git a/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__user_history_cell_wraps_long_urls_inside_the_message_gutter.snap b/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__user_history_cell_wraps_long_urls_inside_the_message_gutter.snap new file mode 100644 index 0000000000..2d8a2fac39 --- /dev/null +++ b/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__user_history_cell_wraps_long_urls_inside_the_message_gutter.snap @@ -0,0 +1,14 @@ +--- +source: tui/src/history_cell/tests.rs +expression: "render_lines(&cell.display_lines(width)).join(\"\\n\")" +--- + +› Skip tests. + + I just reprocessed + https://example.test/forwarded/threads/10930?page=1&search=&f + ilter=all&queue=customer_support_unprocessed&sort=latest_desc + &forwardedScope=all + can you check where we are with it? + + [Image #1] diff --git a/codex-rs/tui/src/history_cell/tests.rs b/codex-rs/tui/src/history_cell/tests.rs index 2970aadd20..47403c1710 100644 --- a/codex-rs/tui/src/history_cell/tests.rs +++ b/codex-rs/tui/src/history_cell/tests.rs @@ -2225,6 +2225,61 @@ fn user_history_cell_wraps_and_prefixes_each_line_snapshot() { insta::assert_snapshot!(rendered); } +#[test] +fn user_history_cell_wraps_long_urls_inside_the_message_gutter() { + let url = "https://example.test/forwarded/threads/10930?page=1&search=&filter=all&queue=customer_support_unprocessed&sort=latest_desc&forwardedScope=all"; + let message = format!( + "Skip tests.\n\nI just reprocessed\n{url}\ncan you check where we are with it?\n\n[Image #1]" + ); + let image_start = message.find("[Image #1]").unwrap(); + let cell = UserHistoryCell { + message, + text_elements: vec![TextElement::new( + (image_start..image_start + "[Image #1]".len()).into(), + Some("[Image #1]".to_string()), + )], + local_image_paths: Vec::new(), + remote_image_urls: Vec::new(), + }; + let width = 64; + let hyperlink_lines = cell.display_hyperlink_lines(width); + + assert!( + hyperlink_lines + .iter() + .all(|line| line.width() <= usize::from(width)), + "every user-message row must fit its viewport: {hyperlink_lines:?}" + ); + + let linked_rows = hyperlink_lines + .iter() + .filter(|line| !line.hyperlinks.is_empty()) + .collect::>(); + assert!(linked_rows.len() > 1, "expected the long URL to wrap"); + assert!( + linked_rows.iter().all(|line| { + line.line + .spans + .first() + .is_some_and(|span| span.content == " ") + }), + "wrapped URL rows must retain the user-message gutter: {linked_rows:?}" + ); + assert!( + linked_rows.iter().all(|line| { + line.hyperlinks + .iter() + .all(|hyperlink| hyperlink.destination == url) + }), + "each wrapped URL fragment must preserve the complete clickable destination" + ); + + insta::assert_snapshot!( + "user_history_cell_wraps_long_urls_inside_the_message_gutter", + render_lines(&cell.display_lines(width)).join("\n") + ); +} + #[test] fn user_history_cell_renders_remote_image_urls() { let cell = UserHistoryCell { @@ -2364,7 +2419,7 @@ fn render_uses_wrapping_for_long_url_like_line() { .map(|y| { (0..area.width) .map(|x| { - let symbol = buf[(x, y)].symbol(); + let symbol = crate::terminal_hyperlinks::strip_osc8(buf[(x, y)].symbol()); if symbol.is_empty() { ' ' } else { @@ -2375,10 +2430,22 @@ fn render_uses_wrapping_for_long_url_like_line() { }) .collect::>(); let rendered_blob = rendered.join("\n"); + let rendered_url = rendered + .iter() + .filter(|row| !row.trim().is_empty()) + .enumerate() + .map(|(index, row)| { + if index == 0 { + row.strip_prefix("› ").unwrap().trim() + } else { + row.trim() + } + }) + .collect::(); - assert!( - rendered_blob.contains("session_id=abc123"), - "expected URL tail to be visible after wrapping, got:\n{rendered_blob}" + assert_eq!( + rendered_url, url, + "wrapped URL must preserve every character" ); let non_empty_rows = rendered.iter().filter(|row| !row.trim().is_empty()).count() as u16; diff --git a/codex-rs/tui/src/insert_history.rs b/codex-rs/tui/src/insert_history.rs index 28682b6160..56a49053d2 100644 --- a/codex-rs/tui/src/insert_history.rs +++ b/codex-rs/tui/src/insert_history.rs @@ -803,6 +803,94 @@ mod tests { ); } + #[test] + fn vt100_user_message_url_wrap_preserves_gutter_and_background() { + use crate::history_cell::HistoryCell; + use crate::history_cell::UserHistoryCell; + + let width = 36; + let height = 12; + let backend = VT100Backend::new(width, height); + let mut term = crate::custom_terminal::Terminal::with_options(backend).expect("terminal"); + term.set_viewport_area(Rect::new( + /*x*/ 0, + /*y*/ height - 1, + /*width*/ width, + /*height*/ 1, + )); + + let url = "https://example.test/forwarded/threads/10930?page=1&queue=customer_support_unprocessed&forwardedScope=all"; + let cell = UserHistoryCell { + message: url.to_string(), + text_elements: Vec::new(), + local_image_paths: Vec::new(), + remote_image_urls: Vec::new(), + }; + let lines = cell + .display_hyperlink_lines(width) + .into_iter() + .map(|line| line.style(ratatui::style::Style::default().bg(Color::Blue))) + .collect::>(); + + insert_history_hyperlink_lines_with_mode_and_wrap_policy( + &mut term, + &lines, + InsertHistoryMode::Standard, + HistoryLineWrapPolicy::PreWrap, + ) + .expect("insert wrapped user message"); + + let screen = term.backend().vt100().screen(); + let rows = screen.rows(/*start*/ 0, width).collect::>(); + let message_rows = rows + .iter() + .enumerate() + .filter(|(_, row)| !row.trim().is_empty()) + .collect::>(); + + assert!(message_rows.len() > 1, "expected wrapped URL: {rows:?}"); + assert!( + message_rows[0].1.starts_with("› "), + "the first user-message row must retain its prompt: {rows:?}" + ); + assert!( + message_rows + .iter() + .skip(/*n*/ 1) + .all(|(_, row)| row.starts_with(" ")), + "all wrapped URL rows must preserve the message gutter: {rows:?}" + ); + assert_eq!( + message_rows + .iter() + .enumerate() + .map(|(index, (_, row))| { + if index == 0 { + row.strip_prefix("› ").unwrap().trim() + } else { + row.trim() + } + }) + .collect::(), + url + ); + for (row, _) in message_rows { + assert_ne!( + screen.cell(row as u16, /*col*/ 0).unwrap().bgcolor(), + vt100::Color::Default, + "wrapped user-message gutter lost its background on row {row}" + ); + assert_ne!( + screen + .cell(row as u16, /*col*/ width - 1) + .unwrap() + .bgcolor(), + vt100::Color::Default, + "wrapped user-message row lost its background after the URL on row {row}" + ); + } + } + #[test] fn vt100_prefixed_mixed_url_line_wraps_suffix_words_together() { let width: u16 = 24;