mirror of
https://github.com/openai/codex.git
synced 2026-08-23 13:09:46 +00:00
Preserve user message styling when wrapping long URLs (#38380)
## Why Terminal autowrap for oversized URL tokens can drop the user-message gutter and background on continuation rows. ## What changed - Explicitly wrap long URLs within the available message width. - Preserve the complete OSC 8 hyperlink destination on every wrapped fragment. - Keep the user-message gutter and background styling across continuation rows. ## Testing Add history-cell and VT100 coverage for URL content, hyperlink targets, gutters, and backgrounds across wrapped rows. GitOrigin-RevId: 59514eee4ab967bd937b55574b9f8d0d1c82df7c
This commit is contained in:
@@ -2,6 +2,10 @@
|
||||
|
||||
use super::markdown_render_cache::MarkdownRenderCache;
|
||||
use super::*;
|
||||
use crate::terminal_hyperlinks::annotate_web_urls_in_line;
|
||||
use crate::terminal_hyperlinks::remap_wrapped_line;
|
||||
use crate::wrapping::url_preserving_wrap_options;
|
||||
use crate::wrapping::word_wrap_line;
|
||||
use std::borrow::Cow;
|
||||
|
||||
#[derive(Debug)]
|
||||
@@ -147,18 +151,12 @@ fn remote_image_display_line(style: Style, index: usize) -> Line<'static> {
|
||||
Line::from(local_image_label_text(index)).style(style)
|
||||
}
|
||||
|
||||
fn trim_trailing_blank_lines(mut lines: Vec<Line<'static>>) -> Vec<Line<'static>> {
|
||||
while lines
|
||||
.last()
|
||||
.is_some_and(|line| line.spans.iter().all(|span| span.content.trim().is_empty()))
|
||||
{
|
||||
lines.pop();
|
||||
}
|
||||
lines
|
||||
}
|
||||
|
||||
impl HistoryCell for UserHistoryCell {
|
||||
fn display_lines(&self, width: u16) -> Vec<Line<'static>> {
|
||||
visible_lines(self.display_hyperlink_lines(width))
|
||||
}
|
||||
|
||||
fn display_hyperlink_lines(&self, width: u16) -> Vec<HyperlinkLine> {
|
||||
let message = sanitize_user_text((&self.message).into());
|
||||
let text_elements = if message.as_ref() == self.message {
|
||||
self.text_elements.as_slice()
|
||||
@@ -177,7 +175,7 @@ impl HistoryCell for UserHistoryCell {
|
||||
let wrapped_remote_images = if self.remote_image_urls.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(adaptive_wrap_lines(
|
||||
Some(plain_hyperlink_lines(adaptive_wrap_lines(
|
||||
self.remote_image_urls
|
||||
.iter()
|
||||
.enumerate()
|
||||
@@ -186,36 +184,61 @@ impl HistoryCell for UserHistoryCell {
|
||||
}),
|
||||
RtOptions::new(usize::from(wrap_width))
|
||||
.wrap_algorithm(textwrap::WrapAlgorithm::FirstFit),
|
||||
))
|
||||
)))
|
||||
};
|
||||
|
||||
let wrapped_message = if message.is_empty() && text_elements.is_empty() {
|
||||
None
|
||||
} else if text_elements.is_empty() {
|
||||
let message_without_trailing_newlines = message.trim_end_matches(['\r', '\n']);
|
||||
let wrapped = adaptive_wrap_lines(
|
||||
message_without_trailing_newlines
|
||||
.split('\n')
|
||||
.map(|line| Line::from(line).style(style)),
|
||||
// Wrap algorithm matches textarea.rs.
|
||||
RtOptions::new(usize::from(wrap_width))
|
||||
.wrap_algorithm(textwrap::WrapAlgorithm::FirstFit),
|
||||
);
|
||||
let wrapped = trim_trailing_blank_lines(wrapped);
|
||||
(!wrapped.is_empty()).then_some(wrapped)
|
||||
} else {
|
||||
let raw_lines = build_user_message_lines_with_elements(
|
||||
message.as_ref(),
|
||||
text_elements,
|
||||
style,
|
||||
element_style,
|
||||
);
|
||||
let wrapped = adaptive_wrap_lines(
|
||||
raw_lines,
|
||||
RtOptions::new(usize::from(wrap_width))
|
||||
.wrap_algorithm(textwrap::WrapAlgorithm::FirstFit),
|
||||
);
|
||||
let wrapped = trim_trailing_blank_lines(wrapped);
|
||||
let wrap_options = RtOptions::new(usize::from(wrap_width))
|
||||
.wrap_algorithm(textwrap::WrapAlgorithm::FirstFit);
|
||||
let mut wrapped = if text_elements.is_empty() {
|
||||
let message_without_trailing_newlines = message.trim_end_matches(['\r', '\n']);
|
||||
adaptive_wrap_lines(
|
||||
message_without_trailing_newlines
|
||||
.split('\n')
|
||||
.map(|line| Line::from(line).style(style)),
|
||||
wrap_options,
|
||||
)
|
||||
} else {
|
||||
adaptive_wrap_lines(
|
||||
build_user_message_lines_with_elements(
|
||||
message.as_ref(),
|
||||
text_elements,
|
||||
style,
|
||||
element_style,
|
||||
),
|
||||
wrap_options,
|
||||
)
|
||||
}
|
||||
.into_iter()
|
||||
.flat_map(|line| {
|
||||
if line.width() <= usize::from(wrap_width) {
|
||||
return vec![HyperlinkLine::new(line)];
|
||||
}
|
||||
|
||||
// Terminal autowrap loses the message gutter and background. Explicitly split
|
||||
// oversized URL tokens while retaining their complete OSC-8 destination.
|
||||
let line = annotate_web_urls_in_line(line);
|
||||
let forced_lines = word_wrap_line(
|
||||
&line.line,
|
||||
url_preserving_wrap_options(RtOptions::new(usize::from(wrap_width)))
|
||||
.break_words(/*break_words*/ true),
|
||||
)
|
||||
.iter()
|
||||
.map(line_to_static)
|
||||
.collect();
|
||||
remap_wrapped_line(&line, forced_lines)
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
while wrapped.last().is_some_and(|line| {
|
||||
line.line
|
||||
.spans
|
||||
.iter()
|
||||
.all(|span| span.content.trim().is_empty())
|
||||
}) {
|
||||
wrapped.pop();
|
||||
}
|
||||
(!wrapped.is_empty()).then_some(wrapped)
|
||||
};
|
||||
|
||||
@@ -223,31 +246,35 @@ impl HistoryCell for UserHistoryCell {
|
||||
return Vec::new();
|
||||
}
|
||||
|
||||
let mut lines: Vec<Line<'static>> = vec![Line::from("").style(style)];
|
||||
let mut lines = vec![HyperlinkLine::new(Line::from("").style(style))];
|
||||
|
||||
if let Some(wrapped_remote_images) = wrapped_remote_images {
|
||||
lines.extend(prefix_lines(
|
||||
lines.extend(prefix_hyperlink_lines(
|
||||
wrapped_remote_images,
|
||||
" ".into(),
|
||||
" ".into(),
|
||||
));
|
||||
if wrapped_message.is_some() {
|
||||
lines.push(Line::from("").style(style));
|
||||
lines.push(HyperlinkLine::new(Line::from("").style(style)));
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(wrapped_message) = wrapped_message {
|
||||
lines.extend(prefix_lines(
|
||||
lines.extend(prefix_hyperlink_lines(
|
||||
wrapped_message,
|
||||
"› ".bold().dim(),
|
||||
" ".into(),
|
||||
));
|
||||
}
|
||||
|
||||
lines.push(Line::from("").style(style));
|
||||
lines.push(HyperlinkLine::new(Line::from("").style(style)));
|
||||
lines
|
||||
}
|
||||
|
||||
fn transcript_hyperlink_lines(&self, width: u16) -> Vec<HyperlinkLine> {
|
||||
self.display_hyperlink_lines(width)
|
||||
}
|
||||
|
||||
fn raw_lines(&self) -> Vec<Line<'static>> {
|
||||
let message = sanitize_user_text((&self.message).into());
|
||||
let mut lines = raw_lines_from_source(message.as_ref().trim_end_matches(['\r', '\n']));
|
||||
|
||||
@@ -0,0 +1,14 @@
|
||||
---
|
||||
source: tui/src/history_cell/tests.rs
|
||||
expression: "render_lines(&cell.display_lines(width)).join(\"\\n\")"
|
||||
---
|
||||
|
||||
› Skip tests.
|
||||
|
||||
I just reprocessed
|
||||
https://example.test/forwarded/threads/10930?page=1&search=&f
|
||||
ilter=all&queue=customer_support_unprocessed&sort=latest_desc
|
||||
&forwardedScope=all
|
||||
can you check where we are with it?
|
||||
|
||||
[Image #1]
|
||||
@@ -2225,6 +2225,61 @@ fn user_history_cell_wraps_and_prefixes_each_line_snapshot() {
|
||||
insta::assert_snapshot!(rendered);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn user_history_cell_wraps_long_urls_inside_the_message_gutter() {
|
||||
let url = "https://example.test/forwarded/threads/10930?page=1&search=&filter=all&queue=customer_support_unprocessed&sort=latest_desc&forwardedScope=all";
|
||||
let message = format!(
|
||||
"Skip tests.\n\nI just reprocessed\n{url}\ncan you check where we are with it?\n\n[Image #1]"
|
||||
);
|
||||
let image_start = message.find("[Image #1]").unwrap();
|
||||
let cell = UserHistoryCell {
|
||||
message,
|
||||
text_elements: vec![TextElement::new(
|
||||
(image_start..image_start + "[Image #1]".len()).into(),
|
||||
Some("[Image #1]".to_string()),
|
||||
)],
|
||||
local_image_paths: Vec::new(),
|
||||
remote_image_urls: Vec::new(),
|
||||
};
|
||||
let width = 64;
|
||||
let hyperlink_lines = cell.display_hyperlink_lines(width);
|
||||
|
||||
assert!(
|
||||
hyperlink_lines
|
||||
.iter()
|
||||
.all(|line| line.width() <= usize::from(width)),
|
||||
"every user-message row must fit its viewport: {hyperlink_lines:?}"
|
||||
);
|
||||
|
||||
let linked_rows = hyperlink_lines
|
||||
.iter()
|
||||
.filter(|line| !line.hyperlinks.is_empty())
|
||||
.collect::<Vec<_>>();
|
||||
assert!(linked_rows.len() > 1, "expected the long URL to wrap");
|
||||
assert!(
|
||||
linked_rows.iter().all(|line| {
|
||||
line.line
|
||||
.spans
|
||||
.first()
|
||||
.is_some_and(|span| span.content == " ")
|
||||
}),
|
||||
"wrapped URL rows must retain the user-message gutter: {linked_rows:?}"
|
||||
);
|
||||
assert!(
|
||||
linked_rows.iter().all(|line| {
|
||||
line.hyperlinks
|
||||
.iter()
|
||||
.all(|hyperlink| hyperlink.destination == url)
|
||||
}),
|
||||
"each wrapped URL fragment must preserve the complete clickable destination"
|
||||
);
|
||||
|
||||
insta::assert_snapshot!(
|
||||
"user_history_cell_wraps_long_urls_inside_the_message_gutter",
|
||||
render_lines(&cell.display_lines(width)).join("\n")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn user_history_cell_renders_remote_image_urls() {
|
||||
let cell = UserHistoryCell {
|
||||
@@ -2364,7 +2419,7 @@ fn render_uses_wrapping_for_long_url_like_line() {
|
||||
.map(|y| {
|
||||
(0..area.width)
|
||||
.map(|x| {
|
||||
let symbol = buf[(x, y)].symbol();
|
||||
let symbol = crate::terminal_hyperlinks::strip_osc8(buf[(x, y)].symbol());
|
||||
if symbol.is_empty() {
|
||||
' '
|
||||
} else {
|
||||
@@ -2375,10 +2430,22 @@ fn render_uses_wrapping_for_long_url_like_line() {
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
let rendered_blob = rendered.join("\n");
|
||||
let rendered_url = rendered
|
||||
.iter()
|
||||
.filter(|row| !row.trim().is_empty())
|
||||
.enumerate()
|
||||
.map(|(index, row)| {
|
||||
if index == 0 {
|
||||
row.strip_prefix("› ").unwrap().trim()
|
||||
} else {
|
||||
row.trim()
|
||||
}
|
||||
})
|
||||
.collect::<String>();
|
||||
|
||||
assert!(
|
||||
rendered_blob.contains("session_id=abc123"),
|
||||
"expected URL tail to be visible after wrapping, got:\n{rendered_blob}"
|
||||
assert_eq!(
|
||||
rendered_url, url,
|
||||
"wrapped URL must preserve every character"
|
||||
);
|
||||
|
||||
let non_empty_rows = rendered.iter().filter(|row| !row.trim().is_empty()).count() as u16;
|
||||
|
||||
@@ -803,6 +803,94 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn vt100_user_message_url_wrap_preserves_gutter_and_background() {
|
||||
use crate::history_cell::HistoryCell;
|
||||
use crate::history_cell::UserHistoryCell;
|
||||
|
||||
let width = 36;
|
||||
let height = 12;
|
||||
let backend = VT100Backend::new(width, height);
|
||||
let mut term = crate::custom_terminal::Terminal::with_options(backend).expect("terminal");
|
||||
term.set_viewport_area(Rect::new(
|
||||
/*x*/ 0,
|
||||
/*y*/ height - 1,
|
||||
/*width*/ width,
|
||||
/*height*/ 1,
|
||||
));
|
||||
|
||||
let url = "https://example.test/forwarded/threads/10930?page=1&queue=customer_support_unprocessed&forwardedScope=all";
|
||||
let cell = UserHistoryCell {
|
||||
message: url.to_string(),
|
||||
text_elements: Vec::new(),
|
||||
local_image_paths: Vec::new(),
|
||||
remote_image_urls: Vec::new(),
|
||||
};
|
||||
let lines = cell
|
||||
.display_hyperlink_lines(width)
|
||||
.into_iter()
|
||||
.map(|line| line.style(ratatui::style::Style::default().bg(Color::Blue)))
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
insert_history_hyperlink_lines_with_mode_and_wrap_policy(
|
||||
&mut term,
|
||||
&lines,
|
||||
InsertHistoryMode::Standard,
|
||||
HistoryLineWrapPolicy::PreWrap,
|
||||
)
|
||||
.expect("insert wrapped user message");
|
||||
|
||||
let screen = term.backend().vt100().screen();
|
||||
let rows = screen.rows(/*start*/ 0, width).collect::<Vec<_>>();
|
||||
let message_rows = rows
|
||||
.iter()
|
||||
.enumerate()
|
||||
.filter(|(_, row)| !row.trim().is_empty())
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
assert!(message_rows.len() > 1, "expected wrapped URL: {rows:?}");
|
||||
assert!(
|
||||
message_rows[0].1.starts_with("› "),
|
||||
"the first user-message row must retain its prompt: {rows:?}"
|
||||
);
|
||||
assert!(
|
||||
message_rows
|
||||
.iter()
|
||||
.skip(/*n*/ 1)
|
||||
.all(|(_, row)| row.starts_with(" ")),
|
||||
"all wrapped URL rows must preserve the message gutter: {rows:?}"
|
||||
);
|
||||
assert_eq!(
|
||||
message_rows
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(index, (_, row))| {
|
||||
if index == 0 {
|
||||
row.strip_prefix("› ").unwrap().trim()
|
||||
} else {
|
||||
row.trim()
|
||||
}
|
||||
})
|
||||
.collect::<String>(),
|
||||
url
|
||||
);
|
||||
for (row, _) in message_rows {
|
||||
assert_ne!(
|
||||
screen.cell(row as u16, /*col*/ 0).unwrap().bgcolor(),
|
||||
vt100::Color::Default,
|
||||
"wrapped user-message gutter lost its background on row {row}"
|
||||
);
|
||||
assert_ne!(
|
||||
screen
|
||||
.cell(row as u16, /*col*/ width - 1)
|
||||
.unwrap()
|
||||
.bgcolor(),
|
||||
vt100::Color::Default,
|
||||
"wrapped user-message row lost its background after the URL on row {row}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn vt100_prefixed_mixed_url_line_wraps_suffix_words_together() {
|
||||
let width: u16 = 24;
|
||||
|
||||
Reference in New Issue
Block a user