mirror of
https://github.com/openai/codex.git
synced 2026-11-10 08:43:33 +00:00
[codex] Fix hyperlink-aware key-value table rendering (#24825)
## Why The key/value markdown table renderer added in #24636 still operates on `Line` values, while table cells and rendered table output now carry `HyperlinkLine`. That mismatch breaks `codex-tui` compilation on `main` and would risk losing semantic web-link annotations if corrected by flattening the values. ## What changed - Make key/value record rendering wrap and emit `HyperlinkLine` values consistently with the existing grid renderer. - Remap wrapped hyperlink ranges and shift them when value content is prefixed by record-mode indentation or labels. - Add focused coverage verifying key/value fallback output preserves web-link destinations. ## Verification - `just test -p codex-tui -E 'test(key_value_table_keeps_web_annotations) | test(/table_renders_(key_value_records_when_compact_fragmentation_is_systemic_snapshot|stacked_key_value_records_when_path_column_becomes_too_narrow_snapshot|records_when_multiple_prose_columns_are_starved_snapshot)/)'`
This commit is contained in:
1 parent
910578792f
commit
090144e0ec
2 files changed
+69
-20
No files matched your search
@@ -2712,6 +2712,26 @@ mod tests {
|
|||||||
}));
|
}));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn key_value_table_keeps_web_annotations() {
|
||||||
|
let destination = "https://example.com/a/very/long/path";
|
||||||
|
let markdown = format!(
|
||||||
|
"| c1 | c2 | c3 | c4 | c5 | c6 |\n| --- | --- | --- | --- | --- | --- |\n| {destination} | 2 | 3 | 4 | 5 | 6 |\n"
|
||||||
|
);
|
||||||
|
let lines = render_markdown_lines_with_width_and_cwd(
|
||||||
|
&markdown,
|
||||||
|
/*width*/ Some(20),
|
||||||
|
/*cwd*/ None,
|
||||||
|
);
|
||||||
|
let destinations = lines
|
||||||
|
.iter()
|
||||||
|
.flat_map(|line| line.hyperlinks.iter().map(|link| link.destination.as_str()))
|
||||||
|
.collect::<Vec<_>>();
|
||||||
|
|
||||||
|
assert!(!destinations.is_empty());
|
||||||
|
assert!(destinations.iter().all(|link| *link == destination));
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn does_not_annotate_code_or_non_web_markdown_links() {
|
fn does_not_annotate_code_or_non_web_markdown_links() {
|
||||||
let markdown = "`https://example.com/inline`\n\n```text\nhttps://example.com/block\n```\n\n[mail](mailto:test@example.com)\n\n[https://example.com/label](mailto:test@example.com)\n\n| Target |\n| --- |\n| [https://example.com/table-label](mailto:test@example.com) |";
|
let markdown = "`https://example.com/inline`\n\n```text\nhttps://example.com/block\n```\n\n[mail](mailto:test@example.com)\n\n[https://example.com/label](mailto:test@example.com)\n\n| Target |\n| --- |\n| [https://example.com/table-label](mailto:test@example.com) |";
|
||||||
|
|||||||
@@ -4,7 +4,10 @@ use super::TABLE_BODY_SEPARATOR_CHAR;
|
|||||||
use super::TableCell;
|
use super::TableCell;
|
||||||
use super::TableColumnKind;
|
use super::TableColumnKind;
|
||||||
use super::TableColumnMetrics;
|
use super::TableColumnMetrics;
|
||||||
|
use crate::render::line_utils::line_to_static;
|
||||||
use crate::render::line_utils::push_owned_lines;
|
use crate::render::line_utils::push_owned_lines;
|
||||||
|
use crate::terminal_hyperlinks::HyperlinkLine;
|
||||||
|
use crate::terminal_hyperlinks::remap_wrapped_line;
|
||||||
use crate::wrapping::RtOptions;
|
use crate::wrapping::RtOptions;
|
||||||
use crate::wrapping::word_wrap_line;
|
use crate::wrapping::word_wrap_line;
|
||||||
use ratatui::style::Style;
|
use ratatui::style::Style;
|
||||||
@@ -99,7 +102,7 @@ pub(super) fn render_records(
|
|||||||
available_width: Option<usize>,
|
available_width: Option<usize>,
|
||||||
label_style: Style,
|
label_style: Style,
|
||||||
separator_style: Style,
|
separator_style: Style,
|
||||||
) -> Vec<Line<'static>> {
|
) -> Vec<HyperlinkLine> {
|
||||||
let label_width = headers
|
let label_width = headers
|
||||||
.iter()
|
.iter()
|
||||||
.map(|header| header.plain_text().width())
|
.map(|header| header.plain_text().width())
|
||||||
@@ -135,10 +138,10 @@ pub(super) fn render_records(
|
|||||||
}
|
}
|
||||||
if row_index + 1 < rows.len() {
|
if row_index + 1 < rows.len() {
|
||||||
let width = available_width.unwrap_or_else(|| widest_line_width(&out));
|
let width = available_width.unwrap_or_else(|| widest_line_width(&out));
|
||||||
out.push(Line::from(Span::styled(
|
out.push(HyperlinkLine::new(Line::from(Span::styled(
|
||||||
TABLE_BODY_SEPARATOR_CHAR.to_string().repeat(width),
|
TABLE_BODY_SEPARATOR_CHAR.to_string().repeat(width),
|
||||||
separator_style,
|
separator_style,
|
||||||
)));
|
))));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -146,7 +149,7 @@ pub(super) fn render_records(
|
|||||||
}
|
}
|
||||||
|
|
||||||
fn render_aligned_field(
|
fn render_aligned_field(
|
||||||
out: &mut Vec<Line<'static>>,
|
out: &mut Vec<HyperlinkLine>,
|
||||||
header: &TableCell,
|
header: &TableCell,
|
||||||
value: &TableCell,
|
value: &TableCell,
|
||||||
label_width: usize,
|
label_width: usize,
|
||||||
@@ -170,13 +173,12 @@ fn render_aligned_field(
|
|||||||
} else {
|
} else {
|
||||||
spans.push(Span::raw(" ".repeat(value_indent)));
|
spans.push(Span::raw(" ".repeat(value_indent)));
|
||||||
}
|
}
|
||||||
spans.extend(value_line.spans);
|
push_prefixed_value_line(out, spans, value_line);
|
||||||
out.push(Line::from(spans));
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fn render_stacked_field(
|
fn render_stacked_field(
|
||||||
out: &mut Vec<Line<'static>>,
|
out: &mut Vec<HyperlinkLine>,
|
||||||
header: &TableCell,
|
header: &TableCell,
|
||||||
value: &TableCell,
|
value: &TableCell,
|
||||||
available_width: Option<usize>,
|
available_width: Option<usize>,
|
||||||
@@ -194,35 +196,60 @@ fn render_stacked_field(
|
|||||||
for label_line in wrapped_labels {
|
for label_line in wrapped_labels {
|
||||||
let mut spans = vec![Span::raw(" ".repeat(FIELD_LEADING_PADDING))];
|
let mut spans = vec![Span::raw(" ".repeat(FIELD_LEADING_PADDING))];
|
||||||
spans.extend(label_line.spans);
|
spans.extend(label_line.spans);
|
||||||
out.push(Line::from(spans));
|
out.push(HyperlinkLine::new(Line::from(spans)));
|
||||||
}
|
}
|
||||||
|
|
||||||
let value_width = available_width
|
let value_width = available_width
|
||||||
.map(|width| width.saturating_sub(STACKED_VALUE_INDENT).max(1))
|
.map(|width| width.saturating_sub(STACKED_VALUE_INDENT).max(1))
|
||||||
.unwrap_or_else(|| cell_width(value).max(1));
|
.unwrap_or_else(|| cell_width(value).max(1));
|
||||||
for value_line in wrap_cell(value, value_width) {
|
for value_line in wrap_cell(value, value_width) {
|
||||||
let mut spans = vec![Span::raw(" ".repeat(STACKED_VALUE_INDENT))];
|
push_prefixed_value_line(
|
||||||
spans.extend(value_line.spans);
|
out,
|
||||||
out.push(Line::from(spans));
|
vec![Span::raw(" ".repeat(STACKED_VALUE_INDENT))],
|
||||||
|
value_line,
|
||||||
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fn wrap_cell(cell: &TableCell, width: usize) -> Vec<Line<'static>> {
|
fn push_prefixed_value_line(
|
||||||
|
out: &mut Vec<HyperlinkLine>,
|
||||||
|
mut prefix: Vec<Span<'static>>,
|
||||||
|
mut value_line: HyperlinkLine,
|
||||||
|
) {
|
||||||
|
let shift = prefix
|
||||||
|
.iter()
|
||||||
|
.map(|span| span.content.width())
|
||||||
|
.sum::<usize>();
|
||||||
|
prefix.append(&mut value_line.line.spans);
|
||||||
|
let mut output_line = HyperlinkLine::new(Line::from(prefix));
|
||||||
|
output_line
|
||||||
|
.hyperlinks
|
||||||
|
.extend(value_line.hyperlinks.into_iter().map(|mut link| {
|
||||||
|
link.columns = link.columns.start + shift..link.columns.end + shift;
|
||||||
|
link
|
||||||
|
}));
|
||||||
|
out.push(output_line);
|
||||||
|
}
|
||||||
|
|
||||||
|
fn wrap_cell(cell: &TableCell, width: usize) -> Vec<HyperlinkLine> {
|
||||||
if cell.lines.is_empty() {
|
if cell.lines.is_empty() {
|
||||||
return vec![Line::default()];
|
return vec![HyperlinkLine::new(Line::default())];
|
||||||
}
|
}
|
||||||
|
|
||||||
let mut wrapped = Vec::new();
|
let mut wrapped = Vec::new();
|
||||||
for source_line in &cell.lines {
|
for source_line in &cell.lines {
|
||||||
let rendered = word_wrap_line(source_line, RtOptions::new(width.max(1)));
|
let rendered = word_wrap_line(&source_line.line, RtOptions::new(width.max(1)))
|
||||||
|
.into_iter()
|
||||||
|
.map(|line| line_to_static(&line))
|
||||||
|
.collect::<Vec<_>>();
|
||||||
if rendered.is_empty() {
|
if rendered.is_empty() {
|
||||||
wrapped.push(Line::default());
|
wrapped.push(HyperlinkLine::new(Line::default()));
|
||||||
} else {
|
} else {
|
||||||
push_owned_lines(&rendered, &mut wrapped);
|
wrapped.extend(remap_wrapped_line(source_line, rendered));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if wrapped.is_empty() {
|
if wrapped.is_empty() {
|
||||||
wrapped.push(Line::default());
|
wrapped.push(HyperlinkLine::new(Line::default()));
|
||||||
}
|
}
|
||||||
wrapped
|
wrapped
|
||||||
}
|
}
|
||||||
@@ -231,7 +258,8 @@ fn cell_width(cell: &TableCell) -> usize {
|
|||||||
cell.lines
|
cell.lines
|
||||||
.iter()
|
.iter()
|
||||||
.map(|line| {
|
.map(|line| {
|
||||||
line.spans
|
line.line
|
||||||
|
.spans
|
||||||
.iter()
|
.iter()
|
||||||
.map(|span| span.content.width())
|
.map(|span| span.content.width())
|
||||||
.sum::<usize>()
|
.sum::<usize>()
|
||||||
@@ -240,11 +268,12 @@ fn cell_width(cell: &TableCell) -> usize {
|
|||||||
.unwrap_or(0)
|
.unwrap_or(0)
|
||||||
}
|
}
|
||||||
|
|
||||||
fn widest_line_width(lines: &[Line<'_>]) -> usize {
|
fn widest_line_width(lines: &[HyperlinkLine]) -> usize {
|
||||||
lines
|
lines
|
||||||
.iter()
|
.iter()
|
||||||
.map(|line| {
|
.map(|line| {
|
||||||
line.spans
|
line.line
|
||||||
|
.spans
|
||||||
.iter()
|
.iter()
|
||||||
.map(|span| span.content.width())
|
.map(|span| span.content.width())
|
||||||
.sum::<usize>()
|
.sum::<usize>()
|
||||||
|
|||||||
Reference in new issue
Block a user