From d6174a8798828a41380527ca24b898a4ce47f862 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 25 Aug 2026 21:50:36 +0000 Subject: [PATCH] Preserve composer hyperlinks across wrapped lines (#40720) ## What changed - Detect visible HTTP(S) URLs in the composer and attach the complete OSC 8 destination to every wrapped fragment, including when part of the URL is scrolled offscreen. - Cache hyperlink metadata with the textarea wrap state while leaving masked input and destinations larger than 8 KiB unannotated. - Make shared hyperlink mapping grapheme-aware, recognize Unicode whitespace boundaries, and scan URLs and trailing delimiters incrementally for large inputs. ## Testing - Add coverage for wrapped and scrolled URLs, joined emoji, Unicode whitespace, preserved highlighting, masked input, cache invalidation, oversized destinations, and drafts containing many URLs. GitOrigin-RevId: 40facdf2fe79438c91c82bff2d849c342f1ab28a --- codex-rs/tui/src/bottom_pane/textarea.rs | 19 +- .../src/bottom_pane/textarea/hyperlinks.rs | 98 +++++ .../bottom_pane/textarea/hyperlinks_tests.rs | 354 ++++++++++++++++++ ...__tests__joined_emoji_url_cell_ranges.snap | 16 + ...aximum_length_url_without_annotations.snap | 8 + ...__unicode_whitespace_url_destinations.snap | 96 +++++ ...extarea_url_keeps_original_appearance.snap | 7 + codex-rs/tui/src/terminal_hyperlinks.rs | 112 +++--- codex-rs/tui/src/terminal_hyperlinks_tests.rs | 66 ++++ 9 files changed, 726 insertions(+), 50 deletions(-) create mode 100644 codex-rs/tui/src/bottom_pane/textarea/hyperlinks.rs create mode 100644 codex-rs/tui/src/bottom_pane/textarea/hyperlinks_tests.rs create mode 100644 codex-rs/tui/src/bottom_pane/textarea/snapshots/codex_tui__bottom_pane__textarea__hyperlinks__tests__joined_emoji_url_cell_ranges.snap create mode 100644 codex-rs/tui/src/bottom_pane/textarea/snapshots/codex_tui__bottom_pane__textarea__hyperlinks__tests__maximum_length_url_without_annotations.snap create mode 100644 codex-rs/tui/src/bottom_pane/textarea/snapshots/codex_tui__bottom_pane__textarea__hyperlinks__tests__unicode_whitespace_url_destinations.snap create mode 100644 codex-rs/tui/src/bottom_pane/textarea/snapshots/codex_tui__bottom_pane__textarea__hyperlinks__tests__wrapped_textarea_url_keeps_original_appearance.snap create mode 100644 codex-rs/tui/src/terminal_hyperlinks_tests.rs diff --git a/codex-rs/tui/src/bottom_pane/textarea.rs b/codex-rs/tui/src/bottom_pane/textarea.rs index ba9ff40024..3749001040 100644 --- a/codex-rs/tui/src/bottom_pane/textarea.rs +++ b/codex-rs/tui/src/bottom_pane/textarea.rs @@ -13,6 +13,8 @@ //! Wrapping also reserves a visible insertion point: full logical lines get continuation rows, //! and trailing spaces wrap instead of moving the cursor outside the textarea. At soft word //! breaks, interior separators hang off the preceding row without changing the editable text. +//! Visible web URLs carry their complete terminal hyperlink destination across wrapped rows; +//! masked rendering never exposes hyperlink destinations. use crate::key_hint::KeyBindingListExt; use crate::key_hint::is_altgr; @@ -38,12 +40,14 @@ use ratatui::text::Span; use ratatui::widgets::StatefulWidgetRef; use ratatui::widgets::WidgetRef; use std::borrow::Cow; +use std::cell::OnceCell; use std::cell::Ref; use std::cell::RefCell; use std::ops::Range; use std::sync::Arc; use unicode_segmentation::UnicodeSegmentation; +mod hyperlinks; mod vim; mod vim_commands; mod wrapping; @@ -147,6 +151,7 @@ pub(crate) struct TextArea { struct WrapCache { width: u16, lines: Vec>, + hyperlinks: OnceCell, } #[derive(Debug, Default, Clone, Copy)] @@ -1994,7 +1999,11 @@ impl TextArea { if needs_recalc { let display_text = text_for_display(&self.text); let lines = wrapping::wrapped_lines(display_text.as_ref(), width); - *cache = Some(WrapCache { width, lines }); + *cache = Some(WrapCache { + width, + lines, + hyperlinks: OnceCell::new(), + }); } } @@ -2107,7 +2116,7 @@ impl TextArea { highlights: &[(Range, Style)], ) { let element_style = base_style.fg(Color::Cyan); - for (row, idx) in range.enumerate() { + for (row, idx) in range.clone().enumerate() { let r = &lines[idx]; let y = area.y + row as u16; let visible = wrapping::visible_prefix(&self.text[r.start..r.end - 1], area.width); @@ -2149,6 +2158,12 @@ impl TextArea { ); } } + if let Some(wrap_cache) = self.wrap_cache.borrow().as_ref() { + wrap_cache + .hyperlinks + .get_or_init(|| hyperlinks::HyperlinkCache::new(&self.text, lines)) + .mark(buf, area, &self.text, lines, range); + } } /// Renders width-preserving mask glyphs without writing outside the textarea viewport. diff --git a/codex-rs/tui/src/bottom_pane/textarea/hyperlinks.rs b/codex-rs/tui/src/bottom_pane/textarea/hyperlinks.rs new file mode 100644 index 0000000000..afd1c87820 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/textarea/hyperlinks.rs @@ -0,0 +1,98 @@ +//! Preserve complete web destinations on visible, independently wrapped textarea rows. + +use super::text_for_display; +use super::wrapping::visible_prefix; +use crate::terminal_hyperlinks::HyperlinkLine; +use crate::terminal_hyperlinks::TerminalHyperlink; +use crate::terminal_hyperlinks::mark_buffer_hyperlinks; +use crate::terminal_hyperlinks::web_links_in_text; +use crate::width::display_width; +use ratatui::buffer::Buffer; +use ratatui::layout::Rect; +use ratatui::text::Line; +use std::ops::Range; + +/// Destinations and row offsets retained for one lazily initialized textarea wrap state. +#[derive(Clone, Debug, Default)] +pub(super) struct HyperlinkCache { + hyperlinks: Vec, + source_columns: Vec, +} + +impl HyperlinkCache { + /// Scan the draft once and precompute row offsets for constant-time scrolled lookup. + pub(super) fn new(text: &str, lines: &[Range]) -> Self { + let display_text = text_for_display(text); + let hyperlinks = web_links_in_text(display_text.as_ref()); + if hyperlinks.is_empty() { + return Self::default(); + } + + let mut source_column = 0; + let mut previous_start = 0; + let source_columns = lines + .iter() + .map(|line| { + source_column += display_width(&display_text[previous_start..line.start]); + previous_start = line.start; + source_column + }) + .collect(); + + Self { + hyperlinks, + source_columns, + } + } + + /// Attach complete destinations after styling without revisiting offscreen draft text. + pub(super) fn mark( + &self, + buf: &mut Buffer, + area: Rect, + text: &str, + lines: &[Range], + visible_rows: Range, + ) { + if area.width == 0 || area.height == 0 || self.hyperlinks.is_empty() { + return; + } + + for (row, index) in visible_rows.enumerate() { + let line_range = &lines[index]; + let source_column = self.source_columns[index]; + let raw_visible = visible_prefix( + &text[line_range.start..line_range.end.saturating_sub(1)], + area.width, + ); + let visible = text_for_display(raw_visible); + let line_end = source_column + display_width(visible.as_ref()); + let first_hyperlink = self + .hyperlinks + .partition_point(|hyperlink| hyperlink.columns.end <= source_column); + + let mut annotated = HyperlinkLine::new(Line::from(visible.into_owned())); + for hyperlink in self.hyperlinks[first_hyperlink..] + .iter() + .take_while(|hyperlink| hyperlink.columns.start < line_end) + { + let start = hyperlink.columns.start.max(source_column) - source_column; + let end = hyperlink.columns.end.min(line_end) - source_column; + annotated.hyperlinks.push(TerminalHyperlink::web( + start..end, + hyperlink.destination.clone(), + )); + } + + if !annotated.hyperlinks.is_empty() { + let row_area = + Rect::new(area.x, area.y + row as u16, area.width, /*height*/ 1); + mark_buffer_hyperlinks(buf, row_area, &[annotated], /*scroll_rows*/ 0); + } + } + } +} + +#[cfg(test)] +#[path = "hyperlinks_tests.rs"] +mod tests; diff --git a/codex-rs/tui/src/bottom_pane/textarea/hyperlinks_tests.rs b/codex-rs/tui/src/bottom_pane/textarea/hyperlinks_tests.rs new file mode 100644 index 0000000000..e9b8c67ba8 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/textarea/hyperlinks_tests.rs @@ -0,0 +1,354 @@ +use super::super::TextArea; +use super::super::TextAreaState; +use crate::app_event::AppEvent; +use crate::app_event_sender::AppEventSender; +use crate::bottom_pane::ChatComposer; +use crate::render::renderable::Renderable; +use crate::terminal_hyperlinks::strip_osc8; +use crate::width::display_width; +use codex_protocol::user_input::MAX_USER_INPUT_TEXT_CHARS; +use pretty_assertions::assert_eq; +use ratatui::buffer::Buffer; +use ratatui::layout::Rect; +use ratatui::style::Color; +use ratatui::style::Style; +use ratatui::widgets::StatefulWidgetRef; +use tokio::sync::mpsc::unbounded_channel; + +type LinkedCell = (u16, u16, String, String); + +fn linked_cells(buf: &Buffer) -> Vec { + buf.area + .positions() + .filter_map(|position| { + let symbol = buf[position].symbol(); + let (destination, _) = symbol.strip_prefix("\x1b]8;;")?.split_once('\x07')?; + Some(( + position.x, + position.y, + strip_osc8(symbol), + destination.to_string(), + )) + }) + .collect() +} + +fn render(text: &str, width: u16, height: u16) -> (TextArea, TextAreaState, Buffer) { + let mut textarea = TextArea::new(); + textarea.insert_str(text); + let area = Rect::new(/*x*/ 0, /*y*/ 0, width, height); + let mut buf = Buffer::empty(area); + let mut state = TextAreaState::default(); + StatefulWidgetRef::render_ref(&(&textarea), area, &mut buf, &mut state); + (textarea, state, buf) +} + +fn linked_text(cells: &[LinkedCell]) -> String { + cells.iter().map(|(_, _, text, _)| text.as_str()).collect() +} + +fn hyperlink_snapshot(buf: &Buffer) -> String { + let mut snapshot = (0..buf.area.height) + .map(|row| { + let text = (0..buf.area.width) + .map(|column| strip_osc8(buf[(column, row)].symbol())) + .collect::(); + format!("|{text}|\n") + }) + .collect::(); + let linked = linked_cells(buf); + snapshot.push_str("Hyperlinks:\n"); + if linked.is_empty() { + snapshot.push_str(" none\n"); + } + for cells in linked.chunk_by(|left, right| { + left.1 == right.1 + && left.3 == right.3 + && usize::from(left.0) + display_width(&left.2) == usize::from(right.0) + }) { + let (column, row, _, destination) = &cells[0]; + let last = cells.last().unwrap(); + let end = usize::from(last.0) + display_width(&last.2); + snapshot.push_str(&format!( + " row {row}, columns {column}..{end} -> {destination}\n" + )); + } + snapshot +} + +fn assert_destination(buf: &Buffer, destination: &str) -> Vec { + let linked = linked_cells(buf); + assert!(!linked.is_empty()); + assert!(linked.iter().all(|(_, _, _, target)| target == destination)); + linked +} + +#[test] +fn wrapped_url_fragments_keep_the_complete_destination() { + let url = "https://github.com/openai/codex/pull/20252"; + let (_, _, buf) = render( + &format!("Fix CI on {url}"), + /*width*/ 26, + /*height*/ 5, + ); + let area = buf.area; + let linked = assert_destination(&buf, url); + assert_eq!(linked_text(&linked), url); + assert!(linked.windows(2).any(|pair| pair[0].1 != pair[1].1)); + assert!(!buf[(0, 0)].symbol().contains("\x1b]8;;")); + + let visible = (0..area.height) + .map(|row| { + (0..area.width) + .map(|column| strip_osc8(buf[(column, row)].symbol())) + .collect::() + .trim_end() + .to_string() + }) + .filter(|line| !line.is_empty()) + .collect::>() + .join("\n"); + insta::assert_snapshot!("wrapped_textarea_url_keeps_original_appearance", visible); +} + +#[test] +fn composer_wrapped_url_fragments_keep_the_complete_destination() { + let url = "https://github.com/openai/codex/pull/20252"; + let (sender, _receiver) = unbounded_channel::(); + let mut composer = ChatComposer::new( + /*has_input_focus*/ true, + AppEventSender::new(sender), + /*enhanced_keys_supported*/ true, + "Ask Codex to do anything".to_string(), + /*disable_paste_burst*/ false, + ); + composer.set_text_content(format!("Fix CI on {url}"), Vec::new(), Vec::new()); + let area = Rect::new( + /*x*/ 0, /*y*/ 0, /*width*/ 34, /*height*/ 8, + ); + let mut buf = Buffer::empty(area); + + composer.render(area, &mut buf); + + let linked = assert_destination(&buf, url); + assert_eq!(linked_text(&linked), url); + assert!(linked.windows(2).any(|pair| pair[0].1 != pair[1].1)); +} + +#[test] +fn scrolled_url_fragments_keep_the_offscreen_destination() { + let url = "https://example.test/alpha/beta/gamma/delta/epsilon"; + let (_, state, buf) = render(url, /*width*/ 16, /*height*/ 2); + let visible_url = linked_text(&assert_destination(&buf, url)); + assert!(state.scroll > 0); + assert_ne!(visible_url, url); + assert!(url.ends_with(&visible_url)); +} + +#[test] +fn long_drafts_reuse_hyperlink_detection_across_cursor_redraws() { + let text = "x".repeat(MAX_USER_INPUT_TEXT_CHARS); + let (mut textarea, mut state, mut buf) = render(&text, /*width*/ 80, /*height*/ 2); + let area = buf.area; + + let first_cache = { + let cache = textarea.wrap_cache.borrow(); + let hyperlinks = cache.as_ref().unwrap().hyperlinks.get().unwrap(); + assert!(hyperlinks.hyperlinks.is_empty()); + hyperlinks as *const super::HyperlinkCache + }; + + textarea.set_cursor(text.len() - 1); + StatefulWidgetRef::render_ref(&(&textarea), area, &mut buf, &mut state); + + let cache = textarea.wrap_cache.borrow(); + let second_cache = cache.as_ref().unwrap().hyperlinks.get().unwrap(); + assert!(std::ptr::eq(first_cache, second_cache)); +} + +#[test] +fn maximum_length_urls_render_without_osc8_annotations() { + let prefix = "https://example.test/"; + let text = format!( + "{prefix}{}", + "a".repeat(MAX_USER_INPUT_TEXT_CHARS - prefix.len()) + ); + let (textarea, _, buf) = render(&text, /*width*/ 80, /*height*/ 2); + + assert_eq!(textarea.text(), text); + assert_eq!(linked_cells(&buf), Vec::new()); + assert!(buf.content.iter().all(|cell| cell.symbol().len() == 1)); + insta::assert_snapshot!( + "maximum_length_url_without_annotations", + hyperlink_snapshot(&buf) + ); +} + +#[test] +fn many_urls_render_with_the_complete_destination() { + let destination = "https://x.io"; + let text = format!("{destination} ").repeat(/*n*/ 50_000); + let (textarea, state, buf) = render(&text, /*width*/ 80, /*height*/ 2); + + assert!(state.scroll > 0); + assert_destination(&buf, destination); + assert_eq!( + textarea + .wrap_cache + .borrow() + .as_ref() + .unwrap() + .hyperlinks + .get() + .unwrap() + .hyperlinks + .len(), + 50_000 + ); +} + +#[test] +fn joined_emoji_preserve_complete_url_cell_ranges() { + let mut snapshots = Vec::new(); + for url in ["https://example.test/path", "https://example.test/👩‍💻/path"] { + for width in [64, 24, 16] { + let (_, _, buf) = render(&format!("👩‍💻 {url} tail"), width, /*height*/ 6); + let linked = assert_destination(&buf, url); + assert_eq!(linked_text(&linked), url); + for row in 0..buf.area.height { + let cells = linked + .iter() + .filter(|(_, y, _, _)| *y == row) + .cloned() + .collect::>(); + if let (Some(first), Some(last)) = (cells.first(), cells.last()) { + snapshots.push(format!( + "width {width}, row {row}, columns {}..={}: {}", + first.0, + last.0, + linked_text(&cells) + )); + } + } + } + } + insta::assert_snapshot!("joined_emoji_url_cell_ranges", snapshots.join("\n")); +} + +#[test] +fn unicode_whitespace_separates_url_destinations() { + let first = "https://one.test/a"; + let second = "https://two.test/b"; + let mut snapshots = Vec::new(); + for separator in ['\u{2003}', '\u{a0}', '\u{2007}', '\u{202f}', '\u{2028}'] { + for width in [48, 20] { + let (_, _, buf) = render( + &format!("{first}{separator}{second}"), + width, + /*height*/ 4, + ); + let linked = linked_cells(&buf); + for destination in [first, second] { + let cells = linked + .iter() + .filter(|(_, _, _, target)| target == destination) + .cloned() + .collect::>(); + assert_eq!(linked_text(&cells), destination); + } + assert_eq!(linked.len(), first.len() + second.len()); + snapshots.push(format!( + "separator U+{:04X}, width {width}\n{}", + separator as u32, + hyperlink_snapshot(&buf) + )); + } + } + insta::assert_snapshot!("unicode_whitespace_url_destinations", snapshots.join("\n")); +} + +#[test] +fn hyperlink_cache_is_invalidated_when_text_changes() { + let first = "https://one.test/before"; + let second = "https://two.test/after"; + let (mut textarea, mut state, mut buf) = render(first, /*width*/ 40, /*height*/ 1); + let area = buf.area; + + textarea.set_text_clearing_elements(second); + assert!(textarea.wrap_cache.borrow().is_none()); + + buf = Buffer::empty(area); + StatefulWidgetRef::render_ref(&(&textarea), area, &mut buf, &mut state); + assert_eq!(linked_text(&assert_destination(&buf, second)), second); +} + +#[test] +fn distinct_urls_respect_punctuation_wide_prefixes_and_tabs() { + let first = "https://one.test/a"; + let second = "https://two.test/b"; + let (_, _, buf) = render( + &format!("界\t({first}),\n{second}!"), + /*width*/ 48, + /*height*/ 2, + ); + let linked = linked_cells(&buf); + for (row, column, destination) in [(0, 4, first), (1, 0, second)] { + let cells = linked + .iter() + .filter(|(_, cell_row, _, _)| *cell_row == row) + .cloned() + .collect::>(); + assert_eq!(cells.first().map(|cell| cell.0), Some(column)); + assert!(cells.iter().all(|cell| cell.3 == destination)); + assert_eq!(linked_text(&cells), destination); + } +} + +#[test] +fn url_hyperlinks_preserve_existing_highlight_styles() { + let url = "https://example.test/highlight"; + let mut textarea = TextArea::new(); + textarea.insert_str(url); + let area = Rect::new( + /*x*/ 0, /*y*/ 0, /*width*/ 40, /*height*/ 1, + ); + let mut buf = Buffer::empty(area); + let mut state = TextAreaState::default(); + let highlight = Style::default().fg(Color::Magenta); + + textarea.render_ref_styled_with_highlights( + area, + &mut buf, + &mut state, + Style::default(), + &[(0..url.len(), highlight)], + ); + + let linked = assert_destination(&buf, url); + assert_eq!(linked.len(), url.len()); + assert!( + linked + .iter() + .all(|(column, row, _, _)| { buf[(*column, *row)].fg == Color::Magenta }) + ); +} + +#[test] +fn masked_url_input_never_exposes_hyperlink_destinations() { + let url = "https://example.test/secret"; + let mut textarea = TextArea::new(); + textarea.insert_str(url); + let area = Rect::new( + /*x*/ 0, /*y*/ 0, /*width*/ 40, /*height*/ 1, + ); + let mut buf = Buffer::empty(area); + let mut state = TextAreaState::default(); + + textarea.render_ref_masked(area, &mut buf, &mut state, '*'); + + assert_eq!(linked_cells(&buf), Vec::new()); + assert!( + area.positions() + .all(|position| !buf[position].symbol().contains(url)) + ); +} diff --git a/codex-rs/tui/src/bottom_pane/textarea/snapshots/codex_tui__bottom_pane__textarea__hyperlinks__tests__joined_emoji_url_cell_ranges.snap b/codex-rs/tui/src/bottom_pane/textarea/snapshots/codex_tui__bottom_pane__textarea__hyperlinks__tests__joined_emoji_url_cell_ranges.snap new file mode 100644 index 0000000000..426ed7d246 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/textarea/snapshots/codex_tui__bottom_pane__textarea__hyperlinks__tests__joined_emoji_url_cell_ranges.snap @@ -0,0 +1,16 @@ +--- +source: tui/src/bottom_pane/textarea/hyperlinks_tests.rs +expression: "snapshots.join(\"\\n\")" +--- +width 64, row 0, columns 3..=27: https://example.test/path +width 24, row 0, columns 3..=23: https://example.test/ +width 24, row 1, columns 0..=3: path +width 16, row 0, columns 3..=10: https:// +width 16, row 1, columns 0..=12: example.test/ +width 16, row 2, columns 0..=3: path +width 64, row 0, columns 3..=30: https://example.test/👩‍💻/path +width 24, row 0, columns 3..=23: https://example.test/ +width 24, row 1, columns 0..=6: 👩‍💻/path +width 16, row 0, columns 3..=10: https:// +width 16, row 1, columns 0..=15: example.test/👩‍💻/ +width 16, row 2, columns 0..=3: path diff --git a/codex-rs/tui/src/bottom_pane/textarea/snapshots/codex_tui__bottom_pane__textarea__hyperlinks__tests__maximum_length_url_without_annotations.snap b/codex-rs/tui/src/bottom_pane/textarea/snapshots/codex_tui__bottom_pane__textarea__hyperlinks__tests__maximum_length_url_without_annotations.snap new file mode 100644 index 0000000000..e9237679bb --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/textarea/snapshots/codex_tui__bottom_pane__textarea__hyperlinks__tests__maximum_length_url_without_annotations.snap @@ -0,0 +1,8 @@ +--- +source: tui/src/bottom_pane/textarea/hyperlinks_tests.rs +expression: hyperlink_snapshot(&buf) +--- +|aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa| +|aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | +Hyperlinks: + none diff --git a/codex-rs/tui/src/bottom_pane/textarea/snapshots/codex_tui__bottom_pane__textarea__hyperlinks__tests__unicode_whitespace_url_destinations.snap b/codex-rs/tui/src/bottom_pane/textarea/snapshots/codex_tui__bottom_pane__textarea__hyperlinks__tests__unicode_whitespace_url_destinations.snap new file mode 100644 index 0000000000..7f3b2c5261 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/textarea/snapshots/codex_tui__bottom_pane__textarea__hyperlinks__tests__unicode_whitespace_url_destinations.snap @@ -0,0 +1,96 @@ +--- +source: tui/src/bottom_pane/textarea/hyperlinks_tests.rs +expression: "snapshots.join(\"\\n\")" +--- +separator U+2003, width 48 +|https://one.test/a https://two.test/b | +| | +| | +| | +Hyperlinks: + row 0, columns 0..18 -> https://one.test/a + row 0, columns 19..37 -> https://two.test/b + +separator U+2003, width 20 +|https://one.test/a  | +|https://two.test/b | +| | +| | +Hyperlinks: + row 0, columns 0..18 -> https://one.test/a + row 1, columns 0..18 -> https://two.test/b + +separator U+00A0, width 48 +|https://one.test/a https://two.test/b | +| | +| | +| | +Hyperlinks: + row 0, columns 0..18 -> https://one.test/a + row 0, columns 19..37 -> https://two.test/b + +separator U+00A0, width 20 +|https://one.test/ | +|a https://two.test/b| +| | +| | +Hyperlinks: + row 0, columns 0..17 -> https://one.test/a + row 1, columns 0..1 -> https://one.test/a + row 1, columns 2..20 -> https://two.test/b + +separator U+2007, width 48 +|https://one.test/a https://two.test/b | +| | +| | +| | +Hyperlinks: + row 0, columns 0..18 -> https://one.test/a + row 0, columns 19..37 -> https://two.test/b + +separator U+2007, width 20 +|https://one.test/ | +|a https://two.test/b| +| | +| | +Hyperlinks: + row 0, columns 0..17 -> https://one.test/a + row 1, columns 0..1 -> https://one.test/a + row 1, columns 2..20 -> https://two.test/b + +separator U+202F, width 48 +|https://one.test/a https://two.test/b | +| | +| | +| | +Hyperlinks: + row 0, columns 0..18 -> https://one.test/a + row 0, columns 19..37 -> https://two.test/b + +separator U+202F, width 20 +|https://one.test/ | +|a https://two.test/b| +| | +| | +Hyperlinks: + row 0, columns 0..17 -> https://one.test/a + row 1, columns 0..1 -> https://one.test/a + row 1, columns 2..20 -> https://two.test/b + +separator U+2028, width 48 +|https://one.test/a
https://two.test/b | +| | +| | +| | +Hyperlinks: + row 0, columns 0..18 -> https://one.test/a + row 0, columns 19..37 -> https://two.test/b + +separator U+2028, width 20 +|https://one.test/a
 | +|https://two.test/b | +| | +| | +Hyperlinks: + row 0, columns 0..18 -> https://one.test/a + row 1, columns 0..18 -> https://two.test/b diff --git a/codex-rs/tui/src/bottom_pane/textarea/snapshots/codex_tui__bottom_pane__textarea__hyperlinks__tests__wrapped_textarea_url_keeps_original_appearance.snap b/codex-rs/tui/src/bottom_pane/textarea/snapshots/codex_tui__bottom_pane__textarea__hyperlinks__tests__wrapped_textarea_url_keeps_original_appearance.snap new file mode 100644 index 0000000000..ab2fd30116 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/textarea/snapshots/codex_tui__bottom_pane__textarea__hyperlinks__tests__wrapped_textarea_url_keeps_original_appearance.snap @@ -0,0 +1,7 @@ +--- +source: tui/src/bottom_pane/textarea/hyperlinks_tests.rs +expression: visible +--- +Fix CI on https:// +github.com/openai/codex/ +pull/20252 diff --git a/codex-rs/tui/src/terminal_hyperlinks.rs b/codex-rs/tui/src/terminal_hyperlinks.rs index 78bee452db..c154ef6480 100644 --- a/codex-rs/tui/src/terminal_hyperlinks.rs +++ b/codex-rs/tui/src/terminal_hyperlinks.rs @@ -22,16 +22,19 @@ use ratatui::text::Text; use ratatui::widgets::Paragraph; use ratatui::widgets::Widget; use ratatui::widgets::Wrap; +use unicode_segmentation::UnicodeSegmentation; use url::Url; use crate::line_truncation::line_width; use crate::render::line_utils::line_to_borrowed; use crate::render::line_utils::line_to_static; -use crate::width::char_width; 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; + #[derive(Clone, Debug, Eq, PartialEq)] pub(crate) struct TerminalHyperlink { pub(crate) columns: Range, @@ -235,10 +238,7 @@ pub(crate) fn remap_wrapped_line( if index > 0 { let trimmed = source_text[source_byte..].trim_start_matches(char::is_whitespace); let skipped = source_text[source_byte..].len() - trimmed.len(); - source_column += source_text[source_byte..source_byte + skipped] - .chars() - .map(char_cell_width) - .sum::(); + source_column += display_width(&source_text[source_byte..source_byte + skipped]); source_byte += skipped; } @@ -249,8 +249,8 @@ pub(crate) fn remap_wrapped_line( }; let mapped = &rendered[rendered_start..]; let mut output_column = display_width(&rendered[..rendered_start]); - for ch in mapped.chars() { - let width = char_cell_width(ch); + for grapheme in mapped.graphemes(/*is_extended*/ true) { + let width = display_width(grapheme); while source .hyperlinks .get(link_index) @@ -282,7 +282,7 @@ fn line_text(line: &Line<'_>) -> String { fn longest_suffix_matching_prefix(rendered: &str, source: &str) -> Option { rendered - .char_indices() + .grapheme_indices(/*is_extended*/ true) .map(|(index, _)| index) .chain(std::iter::once(rendered.len())) .find(|index| source.starts_with(&rendered[*index..]) && *index < rendered.len()) @@ -306,7 +306,9 @@ fn push_link_range(line: &mut HyperlinkLine, range: Range, link: &Termina pub(crate) fn web_links_in_text(text: &str) -> Vec { let mut links = Vec::new(); let mut search_from = 0usize; - for raw_token in text.split_ascii_whitespace() { + let mut source_byte = 0usize; + let mut source_column = 0usize; + for raw_token in text.split_whitespace() { let Some(relative_start) = text[search_from..].find(raw_token) else { continue; }; @@ -323,9 +325,12 @@ pub(crate) fn web_links_in_text(text: &str) -> Vec { let Some(destination) = web_destination(candidate) else { continue; }; - let start = display_width(&text[..raw_start + trimmed_start]); - let end = start + display_width(candidate); - links.push(TerminalHyperlink::web(start..end, destination)); + let candidate_start = raw_start + trimmed_start; + // Measure disjoint prefixes so scanning a draft with many URLs stays linear. + source_column += display_width(&text[source_byte..candidate_start]); + source_byte = candidate_start; + let end = source_column + display_width(candidate); + links.push(TerminalHyperlink::web(source_column..end, destination)); } links } @@ -338,15 +343,41 @@ fn is_leading_punctuation(ch: char) -> bool { } fn trailing_url_end(candidate: &str) -> usize { + // Count delimiter balances once rather than rescanning for every trailing closer. + let mut balances = [0isize; 4]; + for ch in candidate.chars() { + match ch { + '(' => balances[0] += 1, + ')' => balances[0] -= 1, + '[' => balances[1] += 1, + ']' => balances[1] -= 1, + '{' => balances[2] += 1, + '}' => balances[2] -= 1, + '<' => balances[3] += 1, + '>' => balances[3] -= 1, + _ => {} + } + } let mut end = candidate.len(); while end > 0 { let remaining = &candidate[..end]; let Some(ch) = remaining.chars().next_back() else { break; }; - let trim = matches!(ch, ',' | '.' | ';' | '!' | '\'' | '"') - || matches!(ch, ')' | ']' | '}' | '>') - && has_unmatched_closing_delimiter(remaining, ch); + let balance = match ch { + ')' => Some(&mut balances[0]), + ']' => Some(&mut balances[1]), + '}' => Some(&mut balances[2]), + '>' => Some(&mut balances[3]), + _ => None, + }; + let trim = if let Some(balance) = balance { + let unmatched = *balance < 0; + *balance += 1; + unmatched + } else { + matches!(ch, ',' | '.' | ';' | '!' | '\'' | '"') + }; if !trim { break; } @@ -355,20 +386,8 @@ fn trailing_url_end(candidate: &str) -> usize { end } -fn has_unmatched_closing_delimiter(candidate: &str, closing: char) -> bool { - let opening = match closing { - ')' => '(', - ']' => '[', - '}' => '{', - '>' => '<', - _ => return false, - }; - candidate.chars().filter(|ch| *ch == closing).count() - > candidate.chars().filter(|ch| *ch == opening).count() -} - pub(crate) fn web_destination(destination: &str) -> Option { - let safe_destination = sanitized_destination(destination); + let safe_destination = sanitized_destination(destination)?; let parsed = Url::parse(&safe_destination).ok()?; matches!(parsed.scheme(), "http" | "https") .then(|| parsed.host_str()) @@ -377,13 +396,16 @@ pub(crate) fn web_destination(destination: &str) -> Option { } fn trusted_file_destination(destination: &str) -> Option { - let safe_destination = sanitized_destination(destination); + let safe_destination = sanitized_destination(destination)?; let parsed = Url::parse(&safe_destination).ok()?; (parsed.scheme() == "file" && parsed.to_file_path().is_ok()).then_some(safe_destination) } -fn sanitized_destination(destination: &str) -> String { - destination.chars().filter(|ch| !ch.is_control()).collect() +fn sanitized_destination(destination: &str) -> Option { + if destination.len() > MAX_HYPERLINK_DESTINATION_BYTES { + return None; + } + Some(destination.chars().filter(|ch| !ch.is_control()).collect()) } pub(crate) fn osc8_hyperlink(destination: &str, text: &str) -> String { @@ -437,8 +459,8 @@ pub(crate) fn decorate_spans(line: &HyperlinkLine) -> Vec> { let mut active_link_index = None; let mut active_destination: Option = None; for span in &line.line.spans { - for ch in span.content.chars() { - let width = char_cell_width(ch); + for grapheme in span.content.graphemes(/*is_extended*/ true) { + let width = display_width(grapheme); while line .hyperlinks .get(link_index) @@ -465,7 +487,7 @@ pub(crate) fn decorate_spans(line: &HyperlinkLine) -> Vec> { } active_link_index = selected_link_index; } - push_styled_content(&mut out, ch.encode_utf8(&mut [0; 4]), span.style); + push_styled_content(&mut out, grapheme, span.style); column += width; } } @@ -491,13 +513,6 @@ fn append_to_last_span(out: &mut [Span<'static>], content: &str) { } } -fn char_cell_width(ch: char) -> usize { - if ch.is_control() { - return 0; - } - char_width(ch) -} - pub(crate) fn mark_buffer_hyperlinks( buf: &mut Buffer, area: Rect, @@ -556,7 +571,9 @@ pub(crate) fn mark_buffer_hyperlinks( continue; } for link in &rendered.hyperlinks { - let destination = link.terminal_destination(); + let Some(destination) = link.terminal_destination() else { + continue; + }; let mut trailing_columns = 0usize; for column in link.columns.clone() { if trailing_columns > 0 { @@ -570,12 +587,7 @@ pub(crate) fn mark_buffer_hyperlinks( continue; } trailing_columns = usize::from(cell.cell_width()).saturating_sub(1); - let symbol = destination.as_ref().map_or_else( - || cell.symbol().to_string(), - |destination| { - format!("\x1b]8;;{destination}\x07{}\x1b]8;;\x07", cell.symbol()) - }, - ); + let symbol = format!("\x1b]8;;{destination}\x07{}\x1b]8;;\x07", cell.symbol()); let width = NonZeroU16::new(cell.cell_width()).unwrap_or(NonZeroU16::MIN); cell.set_symbol(&symbol) .set_diff_option(CellDiffOption::ForcedWidth(width)); @@ -621,6 +633,10 @@ fn mark_matching_cells( } } +#[cfg(test)] +#[path = "terminal_hyperlinks_tests.rs"] +mod regression_tests; + #[cfg(test)] mod tests { use super::*; diff --git a/codex-rs/tui/src/terminal_hyperlinks_tests.rs b/codex-rs/tui/src/terminal_hyperlinks_tests.rs new file mode 100644 index 0000000000..8648631cef --- /dev/null +++ b/codex-rs/tui/src/terminal_hyperlinks_tests.rs @@ -0,0 +1,66 @@ +use super::*; +use pretty_assertions::assert_eq; + +#[test] +fn oversized_destinations_remain_plain_text() { + let prefix = "https://example.com/"; + let destination = format!( + "{prefix}{}", + "a".repeat(MAX_HYPERLINK_DESTINATION_BYTES - prefix.len()) + ); + assert_eq!(web_destination(&destination), Some(destination.clone())); + let oversized = format!("{destination}é"); + assert_eq!(web_destination(&oversized), None); + assert_eq!(osc8_hyperlink(&oversized, "visible"), "visible"); + + // Explicit Markdown links can reach the cell marker without bare-URL detection. + let line = HyperlinkLine { + line: Line::from("visible"), + hyperlinks: vec![TerminalHyperlink::web(/*columns*/ 0..7, oversized)], + }; + let mut buf = Buffer::with_lines(["visible"]); + let expected = buf.clone(); + let area = buf.area; + mark_buffer_hyperlinks(&mut buf, area, &[line], /*scroll_rows*/ 0); + assert_eq!(buf, expected); +} + +#[test] +fn discovers_many_web_urls_with_incremental_columns() { + let destination = "https://x.io"; + let token = format!("👩‍💻 ({destination}).\u{2003}"); + let text = token.repeat(/*n*/ 50_000); + let expected = (0..50_000) + .map(|index| { + let start = index * display_width(&token) + 4; + TerminalHyperlink::web(start..start + destination.len(), destination.to_string()) + }) + .collect::>(); + assert_eq!(web_links_in_text(&text), expected); +} + +#[test] +fn trims_long_runs_of_unmatched_closing_delimiters() { + let destination = "https://example.com/a_(b)[c]{d}"; + let text = format!("{destination}{}", ")]}>".repeat(/*n*/ 50_000)); + assert_eq!( + web_links_in_text(&text), + vec![TerminalHyperlink::web( + /*columns*/ 0..destination.len(), + destination.to_string(), + )] + ); +} + +#[test] +fn decorated_hyperlinks_preserve_joined_emoji_columns() { + let destination = "https://example.test/👩‍💻/path"; + let line = annotate_web_urls_in_line(Line::from(format!("👩‍💻 {destination} tail"))); + assert_eq!( + decorate_spans(&line), + vec![Span::from(format!( + "👩‍💻 {} tail", + osc8_hyperlink(destination, destination) + ))] + ); +}