openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-18 17:18:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

Preserve clickable URLs in TUI selection rows (#51472)

Browse Source 
## What changed

Render web URLs in selection labels and descriptions as terminal hyperlinks, preserving their complete destinations when text wraps or is truncated. Carry hyperlink ranges through two-column layouts and keep truncation ellipses outside links.

## Testing

Add regression assertions and a snapshot covering wrapped and single-line rows, prefixed labels, descriptions, and wide Unicode characters. Verify that visible URL fragments retain complete destinations and ellipses are not linked.

GitOrigin-RevId: 9d50b3ced79cbd29200e48a676445ec169d60758
This commit is contained in:
Charlie Marsh
2026-10-06 19:52:03 +00:00
committed by copyberry
parent 4854cfa643
commit b0a5191bd6
5 changed files with 368 additions and 127 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/tui/src/bottom_pane/selection_popup_common.rs
+26 -110
View File
@@ -17,12 +17,16 @@ use crate::render::Insets;
use crate::render::RectExt as _;
use crate::style::accent_style;
use crate::style::user_message_style;
use crate::width::display_width;
use super::scroll_state::ScrollState;
use super::selection_row_layout::SelectionDescriptionLayout;
use super::selection_row_layout::build_full_line;
use super::selection_row_layout::line_to_owned;
#[path = "selection_row_links.rs"]
mod hyperlinks;
use hyperlinks::full_hyperlink_row;
use hyperlinks::wrap_row_lines;
#[cfg(test)]
use hyperlinks::wrap_two_column_row;
/// Render-ready representation of one row in a selection popup.
///
@@ -207,104 +211,6 @@ fn should_wrap_name_in_column(row: &GenericDisplayRow) -> bool {
&& row.name_prefix_spans.is_empty()
}
fn wrap_two_column_row(row: &GenericDisplayRow, desc_col: usize, width: u16) -> Vec<Line<'static>> {
use crate::wrapping::RtOptions;
use crate::wrapping::word_wrap_lines;
let Some(description) = row.description.as_deref() else {
return Vec::new();
};
let width = width.max(1);
let max_desc_col = width.saturating_sub(1) as usize;
if max_desc_col == 0 {
// No valid description column exists at this width; let callers fall
// back to single-line wrapping path.
return Vec::new();
}
let desc_col = desc_col.clamp(1, max_desc_col);
let left_width = desc_col.saturating_sub(2).max(1);
let right_width = width.saturating_sub(desc_col as u16).max(1) as usize;
let name_wrap_indent = row
.wrap_indent
.unwrap_or(0)
.min(left_width.saturating_sub(1));
let name_options = RtOptions::new(left_width)
.initial_indent(Line::from(""))
.subsequent_indent(Line::from(" ".repeat(name_wrap_indent)));
let name_lines = word_wrap_lines(row.name.lines(), name_options);
let desc_options = RtOptions::new(right_width).initial_indent(Line::from(""));
let desc_lines = word_wrap_lines(description.lines(), desc_options);
let rows = name_lines.len().max(desc_lines.len()).max(1);
let mut out = Vec::with_capacity(rows);
for idx in 0..rows {
let mut spans: Vec<Span<'static>> = Vec::new();
if let Some(name) = name_lines.get(idx) {
spans.push(name.to_string().into());
}
if let Some(desc) = desc_lines.get(idx) {
let left_used = spans
.iter()
.map(|span| display_width(span.content.as_ref()))
.sum::<usize>();
let gap = if left_used == 0 {
desc_col
} else {
desc_col.saturating_sub(left_used).max(2)
};
if gap > 0 {
spans.push(" ".repeat(gap).into());
}
spans.push(desc.to_string().dim());
}
out.push(Line::from(spans));
}
out
}
fn wrap_standard_row(
row: &GenericDisplayRow,
desc_col: usize,
width: u16,
description_layout: SelectionDescriptionLayout,
) -> Vec<Line<'static>> {
use crate::wrapping::RtOptions;
use crate::wrapping::word_wrap_line;
let full_line = build_full_line(row, desc_col, width, description_layout);
let continuation_indent = wrap_indent(row, desc_col, width);
let options = RtOptions::new(width.max(1) as usize)
.initial_indent(Line::from(""))
.subsequent_indent(Line::from(" ".repeat(continuation_indent)));
word_wrap_line(&full_line, options)
.into_iter()
.map(line_to_owned)
.collect()
}
fn wrap_row_lines(
row: &GenericDisplayRow,
desc_col: usize,
width: u16,
description_layout: SelectionDescriptionLayout,
) -> Vec<Line<'static>> {
if desc_col > 0 && should_wrap_name_in_column(row) {
let wrapped = wrap_two_column_row(row, desc_col, width);
if !wrapped.is_empty() {
return wrapped;
}
}
wrap_standard_row(row, desc_col, width, description_layout)
}
fn apply_row_state_style(
lines: &mut [Line<'static>],
selected: bool,
@@ -524,12 +430,14 @@ fn render_rows_inner(
let mut wrapped =
wrap_row_lines(row, desc_col, area.width, column_width.description_layout);
clipped |= wrapped.len() > usize::from(area.bottom().saturating_sub(cur_y));
apply_row_state_style(
&mut wrapped,
Some(i) == state.selected_idx && !row.is_disabled,
row.is_disabled,
row.selection_style,
);
for line in &mut wrapped {
apply_row_state_style(
std::slice::from_mut(&mut line.line),
Some(i) == state.selected_idx && !row.is_disabled,
row.is_disabled,
row.selection_style,
);
}
// Render the wrapped lines.
let mut rendered_item = false;
@@ -537,7 +445,7 @@ fn render_rows_inner(
if cur_y >= area.y + area.height {
break;
}
line.render(
(&line).render(
Rect {
x: area.x,
y: cur_y,
@@ -680,8 +588,16 @@ pub(crate) fn render_rows_single_line_with_col_width_mode(
row.selection_style,
);
let full_line = truncate_line_with_ellipsis_if_overflow(full_line, area.width as usize);
full_line.render(
let mut full_line = full_hyperlink_row(row, full_line);
let clipped = line_width(&full_line.line) > usize::from(area.width);
full_line.line =
truncate_line_with_ellipsis_if_overflow(full_line.line, area.width as usize);
let visible_width = line_width(&full_line.line).saturating_sub(usize::from(clipped));
full_line.hyperlinks.retain_mut(|link| {
link.columns.end = link.columns.end.min(visible_width);
!link.columns.is_empty()
});
(&full_line).render(
Rect {
x: area.x,
y: cur_y,
@@ -1036,7 +952,7 @@ mod tests {
let rendered = wrap_two_column_row(&row, /*desc_col*/ 8, /*width*/ 24)
.into_iter()
.map(|line| line.to_string())
.map(|line| line.line.to_string())
.collect::<Vec<_>>()
.join("\n");
codex-rs/tui/src/bottom_pane/selection_row_layout.rs
-17
View File
@@ -1,5 +1,3 @@
use std::borrow::Cow;
use ratatui::style::Stylize;
use ratatui::text::Line;
use ratatui::text::Span;
@@ -32,21 +30,6 @@ impl SelectionDescriptionLayout {
}
}
pub(super) fn line_to_owned(line: Line<'_>) -> Line<'static> {
Line {
style: line.style,
alignment: line.alignment,
spans: line
.spans
.into_iter()
.map(|span| Span {
style: span.style,
content: Cow::Owned(span.content.into_owned()),
})
.collect(),
}
}
fn combined_description(
row: &GenericDisplayRow,
description_layout: SelectionDescriptionLayout,
codex-rs/tui/src/bottom_pane/selection_row_links.rs
+180
View File
@@ -0,0 +1,180 @@
//! Preserve complete hyperlink destinations through selection-row wrapping and clipping.
use super::GenericDisplayRow;
use super::SelectionDescriptionLayout;
use super::build_full_line;
use super::should_wrap_name_in_column;
use super::wrap_indent;
use crate::terminal_hyperlinks::HyperlinkLine;
use crate::width::display_width;
use ratatui::style::Stylize;
use ratatui::text::Line;
use ratatui::text::Span;
pub(super) fn full_hyperlink_row(row: &GenericDisplayRow, line: Line<'static>) -> HyperlinkLine {
let prefix = row
.name_prefix_spans
.iter()
.map(|span| span.content.as_ref())
.collect::<String>();
let text = line.to_string();
let visible_name = text.strip_prefix(&prefix).unwrap_or("");
let common = row
.name
.chars()
.zip(visible_name.chars())
.take_while(|(a, b)| a == b)
.map(|(ch, _)| ch)
.collect::<String>();
let start = display_width(&prefix);
let end = start + display_width(&common);
let mut source = crate::terminal_hyperlinks::annotate_web_urls_in_line(line);
source
.hyperlinks
.retain(|link| link.columns.end <= start || link.columns.start >= end);
let name = crate::terminal_hyperlinks::annotate_web_urls_in_line(row.name.clone().into());
source
.hyperlinks
.extend(name.hyperlinks.into_iter().filter_map(|link| {
let last = (start + link.columns.end).min(end);
let first = start + link.columns.start;
(first < last).then(|| link.with_columns(first..last))
}));
source.hyperlinks.sort_by_key(|link| link.columns.start);
source
}
fn wrap_column(text: &str, options: crate::wrapping::RtOptions<'static>) -> Vec<HyperlinkLine> {
text.lines()
.enumerate()
.flat_map(|(index, text)| {
let options = if index == 0 {
options.clone()
} else {
options
.clone()
.initial_indent(options.subsequent_indent.clone())
};
let source =
crate::terminal_hyperlinks::annotate_web_urls_in_line(text.to_owned().into());
let wrapped =
crate::wrapping::word_wrap_line_with_source(&source.line, options.clone());
crate::terminal_hyperlinks::remap_source_wrapped_line(&source, wrapped)
})
.collect()
}
pub(super) fn wrap_two_column_row(
row: &GenericDisplayRow,
desc_col: usize,
width: u16,
) -> Vec<HyperlinkLine> {
use crate::wrapping::RtOptions;
let Some(description) = row.description.as_deref() else {
return Vec::new();
};
let width = width.max(1);
let max_desc_col = width.saturating_sub(1) as usize;
if max_desc_col == 0 {
// No valid description column exists at this width; let callers fall
// back to single-line wrapping path.
return Vec::new();
}
let desc_col = desc_col.clamp(1, max_desc_col);
let left_width = desc_col.saturating_sub(2).max(1);
let right_width = width.saturating_sub(desc_col as u16).max(1) as usize;
let name_wrap_indent = row
.wrap_indent
.unwrap_or(0)
.min(left_width.saturating_sub(1));
let name_options = RtOptions::new(left_width)
.initial_indent(Line::from(""))
.subsequent_indent(Line::from(" ".repeat(name_wrap_indent)));
let name_lines = wrap_column(&row.name, name_options);
let desc_options = RtOptions::new(right_width).initial_indent(Line::from(""));
let desc_lines = wrap_column(description, desc_options);
let rows = name_lines.len().max(desc_lines.len()).max(1);
let mut out = Vec::with_capacity(rows);
for idx in 0..rows {
let mut hyperlinks = name_lines
.get(idx)
.map(|name| name.hyperlinks.clone())
.unwrap_or_default();
let mut spans: Vec<Span<'static>> = Vec::new();
if let Some(name) = name_lines.get(idx) {
spans.push(name.line.to_string().into());
}
if let Some(desc) = desc_lines.get(idx) {
let left_used = spans
.iter()
.map(|span| display_width(span.content.as_ref()))
.sum::<usize>();
let gap = if left_used == 0 {
desc_col
} else {
desc_col.saturating_sub(left_used).max(2)
};
if gap > 0 {
spans.push(" ".repeat(gap).into());
}
let column = left_used + gap;
hyperlinks.extend(desc.hyperlinks.iter().map(|link| {
link.with_columns(column + link.columns.start..column + link.columns.end)
}));
spans.push(desc.line.to_string().dim());
}
let mut line = HyperlinkLine::new(Line::from(spans));
line.hyperlinks = hyperlinks;
out.push(line);
}
out
}
fn wrap_standard_row(
row: &GenericDisplayRow,
desc_col: usize,
width: u16,
description_layout: SelectionDescriptionLayout,
) -> Vec<HyperlinkLine> {
use crate::wrapping::RtOptions;
let full_line = full_hyperlink_row(
row,
build_full_line(row, desc_col, width, description_layout),
);
let continuation_indent = wrap_indent(row, desc_col, width);
let options = RtOptions::new(width.max(1) as usize)
.initial_indent(Line::from(""))
.subsequent_indent(Line::from(" ".repeat(continuation_indent)));
let wrapped = crate::wrapping::word_wrap_line_with_source(&full_line.line, options);
crate::terminal_hyperlinks::remap_source_wrapped_line(&full_line, wrapped)
}
pub(super) fn wrap_row_lines(
row: &GenericDisplayRow,
desc_col: usize,
width: u16,
description_layout: SelectionDescriptionLayout,
) -> Vec<HyperlinkLine> {
if desc_col > 0 && should_wrap_name_in_column(row) {
let wrapped = wrap_two_column_row(row, desc_col, width);
if !wrapped.is_empty() {
return wrapped;
}
}
wrap_standard_row(row, desc_col, width, description_layout)
}
#[cfg(test)]
#[path = "selection_row_links_tests.rs"]
mod tests;
codex-rs/tui/src/bottom_pane/selection_row_links_tests.rs
+91
View File
@@ -0,0 +1,91 @@
use super::super::*;
#[test]
fn choice_labels_and_descriptions_keep_complete_url_destinations() {
let label = "https://github.com/openai/codex/pull/12345?diff=split";
let description = "https://example.com/releases/review/complete-destination";
let mut snapshots = Vec::new();
for prefix_label in [false, true] {
for wrap_indent in [Some(0), None] {
let mut row = GenericDisplayRow {
name: format!("日本語 {label}"),
description: Some(description.into()),
wrap_indent,
..Default::default()
};
if prefix_label {
row.name_prefix_spans = vec!["1. ".into(), std::mem::take(&mut row.name).bold()];
row.description = None;
}
let rows = [row];
for single_line in [false, true] {
let area = Rect::new(0, 0, 48, if single_line { 1 } else { 12 });
let mut buf = Buffer::empty(area);
if single_line {
render_rows_single_line_with_col_width_mode(
area,
&mut buf,
&rows,
&ScrollState::new(),
/*max_results*/ 1,
"",
ColumnWidthConfig::default(),
);
} else {
render_rows(
area,
&mut buf,
&rows,
&ScrollState::new(),
/*max_results*/ 1,
"",
);
}
let mut linked = 0;
for cell in &buf.content {
if cell.symbol().contains("\x1b]8;;") {
linked += 1;
assert!(
[label, description].iter().any(|url| cell
.symbol()
.starts_with(&format!("\x1b]8;;{url}\x07")))
);
assert_ne!(crate::terminal_hyperlinks::strip_osc8(cell.symbol()), "…");
}
}
assert!(linked > 0);
let urls = if prefix_label {
vec![label]
} else {
vec![label, description]
};
for url in urls {
let visible = buf
.content
.iter()
.filter(|cell| cell.symbol().starts_with(&format!("\x1b]8;;{url}\x07")))
.map(|cell| crate::terminal_hyperlinks::strip_osc8(cell.symbol()))
.collect::<String>();
assert!(!visible.is_empty() && url.starts_with(&visible));
}
let visible = buf
.content
.chunks(48)
.map(|row| {
row.iter()
.map(|cell| crate::terminal_hyperlinks::strip_osc8(cell.symbol()))
.collect::<String>()
.trim_end()
.to_owned()
})
.collect::<Vec<_>>()
.join("\n");
snapshots.push(format!(
"prefix label: {prefix_label}, wrapped label: {}, single line: {single_line}\n{visible}",
wrap_indent.is_some()
));
}
}
}
insta::assert_snapshot!("choice_wrapped_urls", snapshots.join("\n\n"));
}
codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__selection_popup_common__hyperlinks__tests__choice_wrapped_urls.snap
+71
View File
@@ -0,0 +1,71 @@
---
source: tui/src/bottom_pane/selection_row_links_tests.rs
expression: "snapshots.join(\"\\n\\n\")"
---
prefix label: false, wrapped label: true, single line: false
日 本 語 https://github.com/ https://
openai/codex/pull/12345? example.com/
diff=split releases/
review/
complete-
destination
prefix label: false, wrapped label: true, single line: true
日 本 語 https://github.com/opena… https://exampl…
prefix label: false, wrapped label: false, single line: false
日 本 語 https://github.com/opena… https://
example.com/
releases/
review/
complete-
destination
prefix label: false, wrapped label: false, single line: true
日 本 語 https://github.com/opena… https://exampl…
prefix label: true, wrapped label: true, single line: false
1. 日 本 語 https://github.com/openai/codex/
pull/12345?diff=split
prefix label: true, wrapped label: true, single line: true
1. 日 本 語 https://github.com/openai/codex/pull/…
prefix label: true, wrapped label: false, single line: false
1. 日 本 語 https://github.com/openai/codex/
pull/12345?diff=split
prefix label: true, wrapped label: false, single line: true
1. 日 本 語 https://github.com/openai/codex/pull/…