mirror of
https://github.com/openai/codex.git
synced 2026-10-21 04:48:12 +00:00
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
This commit is contained in:
@@ -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<String>,
|
||||
pub(super) question_lines: Vec<HyperlinkLine>,
|
||||
pub(super) options_area: Rect,
|
||||
pub(super) notes_area: Rect,
|
||||
pub(super) footer_lines: u16,
|
||||
|
||||
@@ -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<String> {
|
||||
pub(super) fn wrapped_question_lines(&self, width: u16) -> Vec<HyperlinkLine> {
|
||||
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::<Vec<_>>()
|
||||
.map(|range| WrappedLine {
|
||||
line: (&q.title[range.clone()]).into(),
|
||||
range,
|
||||
prefix_bytes: 0,
|
||||
})
|
||||
.collect();
|
||||
remap_source_wrapped_line(&source, wrapped)
|
||||
})
|
||||
.unwrap_or_default()
|
||||
}
|
||||
|
||||
@@ -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();
|
||||
|
||||
|
||||
+16
@@ -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
|
||||
@@ -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::<String>();
|
||||
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();
|
||||
|
||||
@@ -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]
|
||||
|
||||
Reference in New Issue
Block a user