From 594283af5c0a4c99cdede91b0d7951524b2535fb Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 6 Oct 2026 15:03:05 +0000 Subject: [PATCH] Make URLs in asynchronous question titles clickable (#51391) ## What changed Render web URLs in TUI asynchronous question titles as terminal hyperlinks, preserving the full destination when a URL wraps across lines or is partially clipped. Exclude trailing question punctuation such as `)?` from detected URLs while preserving balanced parentheses and query strings. ## Testing Add a rendering regression test and snapshot for a wrapped URL, checking complete hyperlink destinations at full and reduced heights. Extend URL detection tests for trailing question punctuation and balanced parentheses in query strings. GitOrigin-RevId: 6c9a807466d7b7e3a23bc6e86aae8410ae378161 --- .../src/bottom_pane/async_questions/layout.rs | 4 +- .../src/bottom_pane/async_questions/mod.rs | 19 ++++++-- .../src/bottom_pane/async_questions/render.rs | 10 ++-- ...uestions__tests__question_wrapped_url.snap | 16 +++++++ .../async_questions/state_tests.rs | 40 ++++++++++++++++ codex-rs/tui/src/terminal_hyperlinks.rs | 47 +++++++++++++------ 6 files changed, 112 insertions(+), 24 deletions(-) create mode 100644 codex-rs/tui/src/bottom_pane/async_questions/snapshots/codex_tui__bottom_pane__async_questions__tests__question_wrapped_url.snap diff --git a/codex-rs/tui/src/bottom_pane/async_questions/layout.rs b/codex-rs/tui/src/bottom_pane/async_questions/layout.rs index 6316ec939e..e52ca2617a 100644 --- a/codex-rs/tui/src/bottom_pane/async_questions/layout.rs +++ b/codex-rs/tui/src/bottom_pane/async_questions/layout.rs @@ -3,13 +3,15 @@ use ratatui::layout::Rect; +use crate::terminal_hyperlinks::HyperlinkLine; + use super::AsyncQuestions; use super::DESIRED_SPACERS_BETWEEN_SECTIONS; pub(super) struct LayoutSections { pub(super) progress_area: Rect, pub(super) question_area: Rect, - pub(super) question_lines: Vec, + pub(super) question_lines: Vec, pub(super) options_area: Rect, pub(super) notes_area: Rect, pub(super) footer_lines: u16, diff --git a/codex-rs/tui/src/bottom_pane/async_questions/mod.rs b/codex-rs/tui/src/bottom_pane/async_questions/mod.rs index 824fc2d7d3..7753a3a677 100644 --- a/codex-rs/tui/src/bottom_pane/async_questions/mod.rs +++ b/codex-rs/tui/src/bottom_pane/async_questions/mod.rs @@ -16,6 +16,11 @@ use crate::key_hint::KeyBindingListExt; use crate::keymap::KeymapContext; use crate::keymap::ListAction; use crate::keymap::RuntimeKeymap; +use crate::terminal_hyperlinks::HyperlinkLine; +use crate::terminal_hyperlinks::annotate_web_urls_in_line; +use crate::terminal_hyperlinks::remap_source_wrapped_line; +use crate::wrapping::WrappedLine; +use crate::wrapping::wrap_ranges_trim; use codex_protocol::items::AsyncUserInputQuestion; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; @@ -150,13 +155,19 @@ impl AsyncQuestions { .and_then(|answer| answer.options_state.selected_idx) } - pub(super) fn wrapped_question_lines(&self, width: u16) -> Vec { + pub(super) fn wrapped_question_lines(&self, width: u16) -> Vec { self.current_question() .map(|q| { - textwrap::wrap(&q.title, width.max(1) as usize) + let source = annotate_web_urls_in_line(q.title.clone().into()); + let wrapped = wrap_ranges_trim(&q.title, usize::from(width.max(1))) .into_iter() - .map(|line| line.to_string()) - .collect::>() + .map(|range| WrappedLine { + line: (&q.title[range.clone()]).into(), + range, + prefix_bytes: 0, + }) + .collect(); + remap_source_wrapped_line(&source, wrapped) }) .unwrap_or_default() } diff --git a/codex-rs/tui/src/bottom_pane/async_questions/render.rs b/codex-rs/tui/src/bottom_pane/async_questions/render.rs index e13c8c64ea..f83dff8c10 100644 --- a/codex-rs/tui/src/bottom_pane/async_questions/render.rs +++ b/codex-rs/tui/src/bottom_pane/async_questions/render.rs @@ -12,6 +12,7 @@ use crate::bottom_pane::selection_popup_common::menu_surface_inset; use crate::bottom_pane::selection_popup_common::menu_surface_padding_height; use crate::bottom_pane::selection_popup_common::render_menu_surface; use crate::render::renderable::Renderable; +use crate::terminal_hyperlinks::HyperlinkParagraph; use super::AsyncQuestions; use super::TIP_SEPARATOR; @@ -49,10 +50,11 @@ impl Renderable for AsyncQuestions { } let sections = self.layout_sections(content_area); Paragraph::new(self.progress_prefix_text().dim()).render(sections.progress_area, buf); - Paragraph::new(sections.question_lines.join("\n")) - .style(crate::style::accent_style()) - .bold() - .render(sections.question_area, buf); + HyperlinkParagraph::new( + §ions.question_lines, + crate::style::accent_style().bold(), + ) + .render(sections.question_area, buf); let option_rows = self.option_rows(); diff --git a/codex-rs/tui/src/bottom_pane/async_questions/snapshots/codex_tui__bottom_pane__async_questions__tests__question_wrapped_url.snap b/codex-rs/tui/src/bottom_pane/async_questions/snapshots/codex_tui__bottom_pane__async_questions__tests__question_wrapped_url.snap new file mode 100644 index 0000000000..4435c91add --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/async_questions/snapshots/codex_tui__bottom_pane__async_questions__tests__question_wrapped_url.snap @@ -0,0 +1,16 @@ +--- +source: tui/src/bottom_pane/async_questions/state_tests.rs +assertion_line: 172 +expression: "crate::terminal_hyperlinks::strip_osc8(&buffer_text(&buffer))" +--- + + 2 of 2 + Review this change (https:// + github.com/openai/codex/pull/12345? + diff=split)? + + › 1. Named + 2. Other + + enter submit ⌃] skip + ⇧→ prev question diff --git a/codex-rs/tui/src/bottom_pane/async_questions/state_tests.rs b/codex-rs/tui/src/bottom_pane/async_questions/state_tests.rs index dca4e202b2..1345dd543f 100644 --- a/codex-rs/tui/src/bottom_pane/async_questions/state_tests.rs +++ b/codex-rs/tui/src/bottom_pane/async_questions/state_tests.rs @@ -140,6 +140,46 @@ fn long_prompt_keeps_the_active_input_visible() { insta::assert_snapshot!("long_prompt_active_input", text); } +#[test] +fn wrapped_question_url_keeps_its_complete_destination() { + let url = "https://github.com/openai/codex/pull/12345?diff=split"; + let mut editor = editor(); + editor.navigate(/*forward*/ true); + editor.state.pending[1].question.title = format!("Review this change ({url})?"); + + let width = 40; + let full_height = editor.desired_height(width); + for height in [full_height, 4] { + let buffer = render_editor(&editor, width, height); + let linked_text = buffer + .content + .iter() + .filter_map(|cell| { + let symbol = cell.symbol(); + symbol.contains("\x1b]8;;").then(|| { + let text = crate::terminal_hyperlinks::strip_osc8(symbol); + assert_eq!( + symbol, + crate::terminal_hyperlinks::osc8_hyperlink(url, &text) + ); + text + }) + }) + .collect::(); + if height == full_height { + assert_eq!(linked_text, url); + insta::assert_snapshot!( + "question_wrapped_url", + crate::terminal_hyperlinks::strip_osc8(&buffer_text(&buffer)) + ); + } else { + assert!(!linked_text.is_empty()); + assert!(url.starts_with(&linked_text)); + assert_ne!(linked_text, url); + } + } +} + #[test] fn selected_other_renders_as_a_dim_placeholder() { let mut editor = editor(); diff --git a/codex-rs/tui/src/terminal_hyperlinks.rs b/codex-rs/tui/src/terminal_hyperlinks.rs index bea2472ad1..b887f46a43 100644 --- a/codex-rs/tui/src/terminal_hyperlinks.rs +++ b/codex-rs/tui/src/terminal_hyperlinks.rs @@ -586,6 +586,14 @@ fn trailing_url_end(candidate: &str) -> usize { unmatched } else { matches!(ch, ',' | '.' | ';' | '!' | '\'' | '"') + || ch == '?' + && match remaining.chars().rev().nth(1) { + Some(')') => balances[0] < 0, + Some(']') => balances[1] < 0, + Some('}') => balances[2] < 0, + Some('>') => balances[3] < 0, + _ => false, + } }; if !trim { break; @@ -890,13 +898,18 @@ mod tests { #[test] fn discovers_punctuated_web_url_columns() { - assert_eq!( - web_links_in_text("See (https://example.com/a)."), - vec![TerminalHyperlink::web( - /*columns*/ 5..26, - "https://example.com/a".to_string(), - )] - ); + for text in [ + "See (https://example.com/a).", + "See (https://example.com/a)?", + ] { + assert_eq!( + web_links_in_text(text), + vec![TerminalHyperlink::web( + /*columns*/ 5..26, + "https://example.com/a".to_string(), + )] + ); + } } #[test] @@ -920,14 +933,18 @@ mod tests { #[test] fn preserves_balanced_parentheses_in_bare_web_urls() { - let destination = "https://en.wikipedia.org/wiki/Function_(mathematics)"; - assert_eq!( - web_links_in_text(&format!("See ({destination}).")), - vec![TerminalHyperlink::web( - /*columns*/ 5..5 + usize::from(destination.cell_width()), - destination.to_string(), - )] - ); + for destination in [ + "https://en.wikipedia.org/wiki/Function_(mathematics)", + "https://en.wikipedia.org/wiki/Function_(mathematics)?q=(alpha)?", + ] { + assert_eq!( + web_links_in_text(&format!("See ({destination}).")), + vec![TerminalHyperlink::web( + /*columns*/ 5..5 + usize::from(destination.cell_width()), + destination.to_string(), + )] + ); + } } #[test]