mirror of
https://github.com/openai/codex.git
synced 2026-10-20 00:18:49 +00:00
Preserve clickable URLs in pending input previews (#51471)
## What changed Render web URLs in pending steers, rejected steers, and queued follow-up messages as terminal hyperlinks. Preserve the complete destination when the visible URL is clipped to the preview width. Add `HyperlinkRows` to render already-wrapped rows without changing their spacing or row count, clipping text and hyperlink ranges to the viewport. ## Testing Add regression coverage for complete URL destinations across all three preview sections, plus snapshots for oversized URLs and clipping overflow text and the edit hint in a narrow preview. GitOrigin-RevId: 959df27bba381b1b3895f5a86cc2ecb1419806b1
This commit is contained in:
@@ -1,9 +1,10 @@
|
||||
use crate::terminal_hyperlinks::HyperlinkLine;
|
||||
use crate::terminal_hyperlinks::HyperlinkRows;
|
||||
use crossterm::event::KeyCode;
|
||||
use ratatui::buffer::Buffer;
|
||||
use ratatui::layout::Rect;
|
||||
use ratatui::style::Stylize;
|
||||
use ratatui::text::Line;
|
||||
use ratatui::widgets::Paragraph;
|
||||
|
||||
use crate::key_hint;
|
||||
use crate::render::renderable::Renderable;
|
||||
@@ -59,23 +60,25 @@ impl PendingInputPreview {
|
||||
}
|
||||
|
||||
fn push_truncated_preview_lines(
|
||||
lines: &mut Vec<Line<'static>>,
|
||||
wrapped: Vec<Line<'static>>,
|
||||
lines: &mut Vec<HyperlinkLine>,
|
||||
wrapped: Vec<HyperlinkLine>,
|
||||
overflow_line: Line<'static>,
|
||||
) {
|
||||
let wrapped_len = wrapped.len();
|
||||
lines.extend(wrapped.into_iter().take(PREVIEW_LINE_LIMIT));
|
||||
if wrapped_len > PREVIEW_LINE_LIMIT {
|
||||
lines.push(overflow_line);
|
||||
lines.push(HyperlinkLine::new(overflow_line));
|
||||
}
|
||||
}
|
||||
|
||||
fn push_section_header(lines: &mut Vec<Line<'static>>, width: u16, header: Line<'static>) {
|
||||
fn push_section_header(lines: &mut Vec<HyperlinkLine>, width: u16, header: Line<'static>) {
|
||||
let mut spans = vec!["• ".dim()];
|
||||
spans.extend(header.spans);
|
||||
lines.extend(adaptive_wrap_lines(
|
||||
std::iter::once(Line::from(spans)),
|
||||
RtOptions::new(width as usize).subsequent_indent(Line::from(" ".dim())),
|
||||
lines.extend(crate::terminal_hyperlinks::plain_hyperlink_lines(
|
||||
adaptive_wrap_lines(
|
||||
std::iter::once(Line::from(spans)),
|
||||
RtOptions::new(width as usize).subsequent_indent(Line::from(" ".dim())),
|
||||
),
|
||||
));
|
||||
}
|
||||
|
||||
@@ -102,11 +105,11 @@ impl PendingInputPreview {
|
||||
Self::push_section_header(&mut lines, width, Line::from(header));
|
||||
|
||||
for steer in &self.pending_steers {
|
||||
let wrapped = adaptive_wrap_lines(
|
||||
let wrapped = wrap_preview_lines(
|
||||
steer
|
||||
.lines()
|
||||
.take(PREVIEW_LINE_LIMIT + 1)
|
||||
.map(|line| Line::from(line.dim())),
|
||||
.map(|line| Line::from(line.to_owned().dim())),
|
||||
RtOptions::new(width as usize)
|
||||
.initial_indent(Line::from(" ↳ ".dim()))
|
||||
.subsequent_indent(Line::from(" ")),
|
||||
@@ -117,7 +120,7 @@ impl PendingInputPreview {
|
||||
|
||||
if !self.rejected_steers.is_empty() {
|
||||
if !lines.is_empty() {
|
||||
lines.push(Line::from(""));
|
||||
lines.push(HyperlinkLine::new(Line::from("")));
|
||||
}
|
||||
Self::push_section_header(
|
||||
&mut lines,
|
||||
@@ -126,11 +129,11 @@ impl PendingInputPreview {
|
||||
);
|
||||
|
||||
for steer in &self.rejected_steers {
|
||||
let wrapped = adaptive_wrap_lines(
|
||||
let wrapped = wrap_preview_lines(
|
||||
steer
|
||||
.lines()
|
||||
.take(PREVIEW_LINE_LIMIT + 1)
|
||||
.map(|line| Line::from(line.dim())),
|
||||
.map(|line| Line::from(line.to_owned().dim())),
|
||||
RtOptions::new(width as usize)
|
||||
.initial_indent(Line::from(" ↳ ".dim()))
|
||||
.subsequent_indent(Line::from(" ")),
|
||||
@@ -141,16 +144,16 @@ impl PendingInputPreview {
|
||||
|
||||
if !self.queued_messages.is_empty() || has_questions {
|
||||
if !lines.is_empty() {
|
||||
lines.push(Line::from(""));
|
||||
lines.push(HyperlinkLine::new(Line::from("")));
|
||||
}
|
||||
Self::push_section_header(&mut lines, width, "Queued follow-up inputs".into());
|
||||
|
||||
for message in &self.queued_messages {
|
||||
let wrapped = adaptive_wrap_lines(
|
||||
let wrapped = wrap_preview_lines(
|
||||
message
|
||||
.lines()
|
||||
.take(PREVIEW_LINE_LIMIT + 1)
|
||||
.map(|line| Line::from(line.dim().italic())),
|
||||
.map(|line| Line::from(line.to_owned().dim().italic())),
|
||||
RtOptions::new(width as usize)
|
||||
.initial_indent(Line::from(" ↳ ".dim()))
|
||||
.subsequent_indent(Line::from(" ")),
|
||||
@@ -170,13 +173,47 @@ impl PendingInputPreview {
|
||||
let mut hint = Line::from(" ");
|
||||
hint.spans.extend(edit_binding.spans());
|
||||
hint.spans.push(" edit last queued message".dim());
|
||||
lines.push(hint);
|
||||
lines.push(HyperlinkLine::new(hint));
|
||||
}
|
||||
|
||||
Paragraph::new(lines).into()
|
||||
Box::new(HyperlinkRows::from(lines))
|
||||
}
|
||||
}
|
||||
|
||||
/// Preserve complete destinations while clipping oversized tokens to the preview width.
|
||||
fn wrap_preview_lines(
|
||||
lines: impl IntoIterator<Item = Line<'static>>,
|
||||
options: RtOptions<'static>,
|
||||
) -> Vec<HyperlinkLine> {
|
||||
lines
|
||||
.into_iter()
|
||||
.enumerate()
|
||||
.flat_map(|(index, line)| {
|
||||
let source = crate::terminal_hyperlinks::annotate_web_urls_in_line(line);
|
||||
let options = if index == 0 {
|
||||
options.clone()
|
||||
} else {
|
||||
options
|
||||
.clone()
|
||||
.initial_indent(options.subsequent_indent.clone())
|
||||
};
|
||||
let width = options.width;
|
||||
let wrapped = crate::wrapping::adaptive_wrap_line_with_source(&source.line, options);
|
||||
let mut lines = crate::terminal_hyperlinks::remap_source_wrapped_line(&source, wrapped);
|
||||
for line in &mut lines {
|
||||
line.line =
|
||||
crate::line_truncation::truncate_line_to_width(line.line.clone(), width);
|
||||
let visible_width = crate::line_truncation::line_width(&line.line);
|
||||
line.hyperlinks.retain_mut(|link| {
|
||||
link.columns.end = link.columns.end.min(visible_width);
|
||||
!link.columns.is_empty()
|
||||
});
|
||||
}
|
||||
lines
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
impl Renderable for PendingInputPreview {
|
||||
fn render(&self, area: Rect, buf: &mut Buffer) {
|
||||
if area.is_empty() {
|
||||
@@ -222,6 +259,60 @@ mod tests {
|
||||
assert_eq!(queue.desired_height(/*width*/ 40), 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn oversized_preview_urls_keep_complete_destinations() {
|
||||
let url = "https://github.com/openai/codex/pull/12345?diff=split";
|
||||
for section in 0..3 {
|
||||
let mut queue = PendingInputPreview::new();
|
||||
let messages = match section {
|
||||
0 => &mut queue.pending_steers,
|
||||
1 => &mut queue.rejected_steers,
|
||||
2 => &mut queue.queued_messages,
|
||||
_ => unreachable!(),
|
||||
};
|
||||
messages.push(format!("Review {url}"));
|
||||
let area = Rect::new(0, 0, 32, queue.desired_height(/*width*/ 32));
|
||||
let mut buf = Buffer::empty(area);
|
||||
queue.render(area, &mut buf);
|
||||
let linked = buf
|
||||
.content
|
||||
.iter()
|
||||
.filter(|cell| cell.symbol().contains("\x1b]8;;"))
|
||||
.map(|cell| {
|
||||
assert!(cell.symbol().starts_with(&format!("\x1b]8;;{url}\x07")));
|
||||
crate::terminal_hyperlinks::strip_osc8(cell.symbol())
|
||||
})
|
||||
.collect::<String>();
|
||||
assert!(!linked.is_empty() && url.starts_with(&linked));
|
||||
if section == 2 {
|
||||
let visible = buf
|
||||
.content
|
||||
.chunks(32)
|
||||
.map(|row| {
|
||||
row.iter()
|
||||
.map(|cell| crate::terminal_hyperlinks::strip_osc8(cell.symbol()))
|
||||
.collect::<String>()
|
||||
})
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
assert_snapshot!("render_oversized_preview_url", visible);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn narrow_preview_clips_the_overflow_and_edit_hint() {
|
||||
let mut queue = PendingInputPreview::new();
|
||||
queue
|
||||
.queued_messages
|
||||
.push("First\nSecond\nThird\nFourth".to_string());
|
||||
let width = 4;
|
||||
let area = Rect::new(0, 0, width, queue.desired_height(width));
|
||||
let mut buf = Buffer::empty(area);
|
||||
queue.render(area, &mut buf);
|
||||
assert_snapshot!("render_narrow_preview", format!("{buf:?}"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn render_one_message() {
|
||||
let mut queue = PendingInputPreview::new();
|
||||
|
||||
+56
@@ -0,0 +1,56 @@
|
||||
---
|
||||
source: tui/src/bottom_pane/pending_input_preview.rs
|
||||
expression: "format!(\"{buf:?}\")"
|
||||
---
|
||||
Buffer {
|
||||
area: Rect { x: 0, y: 0, width: 4, height: 17 },
|
||||
content: [
|
||||
"• ",
|
||||
" Qu",
|
||||
" eu",
|
||||
" ed",
|
||||
" fo",
|
||||
" ll",
|
||||
" ow",
|
||||
" - ",
|
||||
" up",
|
||||
" in",
|
||||
" pu",
|
||||
" ts",
|
||||
" ↳ ",
|
||||
" ",
|
||||
" ",
|
||||
" ",
|
||||
" ",
|
||||
],
|
||||
styles: [
|
||||
x: 0, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
x: 1, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
|
||||
x: 0, y: 1, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
x: 2, y: 1, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
|
||||
x: 0, y: 2, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
x: 2, y: 2, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
|
||||
x: 0, y: 3, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
x: 2, y: 3, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
|
||||
x: 0, y: 4, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
x: 2, y: 4, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
|
||||
x: 0, y: 5, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
x: 2, y: 5, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
|
||||
x: 0, y: 6, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
x: 2, y: 6, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
|
||||
x: 0, y: 7, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
x: 2, y: 7, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
|
||||
x: 0, y: 8, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
x: 2, y: 8, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
|
||||
x: 0, y: 9, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
x: 2, y: 9, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
|
||||
x: 0, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
x: 2, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
|
||||
x: 0, y: 11, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
x: 2, y: 11, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
|
||||
x: 0, y: 12, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
x: 0, y: 13, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
|
||||
x: 0, y: 15, fg: Reset, bg: Reset, underline: Reset, modifier: DIM | ITALIC,
|
||||
x: 0, y: 16, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
|
||||
]
|
||||
}
|
||||
+8
@@ -0,0 +1,8 @@
|
||||
---
|
||||
source: tui/src/bottom_pane/pending_input_preview.rs
|
||||
expression: visible
|
||||
---
|
||||
• Queued follow-up inputs
|
||||
↳ Review
|
||||
https://github.com/openai/co
|
||||
⇧← edit last queued message
|
||||
@@ -7,6 +7,7 @@ mod paragraph;
|
||||
mod source;
|
||||
|
||||
pub(crate) use paragraph::HyperlinkParagraph;
|
||||
pub(crate) use paragraph::HyperlinkRows;
|
||||
pub(crate) use paragraph::HyperlinkText;
|
||||
pub(crate) use source::LineWrapPolicy;
|
||||
pub(crate) use source::LogicalLineSource;
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
//! Paragraph rendering that keeps visible text and hyperlink annotations aligned.
|
||||
//! Paragraph and prelaid-out row rendering that keeps visible text and links aligned.
|
||||
|
||||
use super::HyperlinkLine;
|
||||
use super::annotate_web_urls;
|
||||
@@ -6,6 +6,7 @@ use super::mark_buffer_hyperlinks;
|
||||
use super::visible_lines_ref;
|
||||
use crate::render::renderable::Renderable;
|
||||
use ratatui::buffer::Buffer;
|
||||
use ratatui::buffer::CellWidth;
|
||||
use ratatui::layout::Rect;
|
||||
use ratatui::style::Style;
|
||||
use ratatui::text::Line;
|
||||
@@ -69,3 +70,63 @@ impl Widget for HyperlinkParagraph<'_> {
|
||||
mark_buffer_hyperlinks(buf, area, self.lines, usize::from(self.scroll_rows));
|
||||
}
|
||||
}
|
||||
|
||||
/// Owns rows that have already been wrapped or clipped by their layout code.
|
||||
/// Rendering clips to the viewport without changing row count or spacing.
|
||||
pub(crate) struct HyperlinkRows(Vec<HyperlinkLine>);
|
||||
|
||||
impl From<Vec<HyperlinkLine>> for HyperlinkRows {
|
||||
fn from(lines: Vec<HyperlinkLine>) -> Self {
|
||||
Self(lines)
|
||||
}
|
||||
}
|
||||
|
||||
impl Renderable for HyperlinkRows {
|
||||
fn render(&self, area: Rect, buf: &mut Buffer) {
|
||||
for (row, line) in self.0.iter().take(usize::from(area.height)).enumerate() {
|
||||
line.render(Rect::new(area.x, area.y + row as u16, area.width, 1), buf);
|
||||
}
|
||||
}
|
||||
|
||||
fn desired_height(&self, width: u16) -> u16 {
|
||||
if width == 0 {
|
||||
0
|
||||
} else {
|
||||
self.0.len().min(usize::from(u16::MAX)) as u16
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Render one already-laid-out row, clipping text and links without wrapping it again.
|
||||
impl Widget for &HyperlinkLine {
|
||||
fn render(self, area: Rect, buf: &mut Buffer) {
|
||||
self.line.clone().render(area, buf);
|
||||
if area.is_empty() {
|
||||
return;
|
||||
}
|
||||
for link in &self.hyperlinks {
|
||||
let Some(destination) = link.terminal_destination() else {
|
||||
continue;
|
||||
};
|
||||
let mut trailing_columns = 0usize;
|
||||
for column in link.columns.start.min(usize::from(area.width))
|
||||
..link.columns.end.min(usize::from(area.width))
|
||||
{
|
||||
if trailing_columns > 0 {
|
||||
trailing_columns -= 1;
|
||||
continue;
|
||||
}
|
||||
let cell = &mut buf[(area.x + column as u16, area.y)];
|
||||
if cell.diff_option == ratatui::buffer::CellDiffOption::Skip {
|
||||
continue;
|
||||
}
|
||||
trailing_columns = usize::from(cell.cell_width()).saturating_sub(1);
|
||||
let symbol = format!("\x1b]8;;{destination}\x07{}\x1b]8;;\x07", cell.symbol());
|
||||
let width = std::num::NonZeroU16::new(cell.cell_width())
|
||||
.unwrap_or(std::num::NonZeroU16::MIN);
|
||||
cell.set_symbol(&symbol)
|
||||
.set_diff_option(ratatui::buffer::CellDiffOption::ForcedWidth(width));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user