mirror of
https://github.com/openai/codex.git
synced 2026-11-11 06:31:37 +00:00
feat(tui): render cramped markdown tables as key-value records [2 of 2] (#24636)
## Stack - **Base: #24489 [1 of 2]** - render markdown tables in app style. - **Current: #24636 [2 of 2]** - render cramped markdown tables as key/value records. Review this PR against `fcoury/app-style-markdown-tables`; it contains only the fallback behavior for cramped tables. ## Why The row-separated markdown table rendering in #24489 remains readable while columns have usable room. Once long links or multiple prose-heavy columns are compressed into narrow allocations, however, the grid can turn words and paths into tall vertical strips that are difficult to scan. In those cases the content matters more than preserving the grid shape. ## What Changed <table> <tr><td> <p align="center"><b> Normal </b></p> <img width="1722" height="619" alt="CleanShot 2026-05-27 at 14 32 57" src="https://github.com/user-attachments/assets/d04f5fbd-6064-4acd-91bd-072d19b983df" /> </td></tr> <tr><td> <p align="center"><b> Narrow </b></p> <img width="863" height="1013" alt="CleanShot 2026-05-27 at 14 33 12" src="https://github.com/user-attachments/assets/6a7d2968-0a68-48fd-ab5d-209b3dbaf03e" /> </td></tr> <tr><td> <p align="center"><b> Very narrow </b></p> <img width="435" height="746" alt="CleanShot 2026-05-27 at 14 33 47" src="https://github.com/user-attachments/assets/f6a59e30-b1d2-4063-9c05-43933abc77d6" /> </td></tr> </table> - Detect tables whose grid allocation causes systemic token fragmentation or starves multiple prose-heavy columns. - Render those tables as repeated key/value records instead of retaining an unreadable grid. - Use aligned label/value records when there is useful horizontal room, and switch to a stacked narrow-record layout where each label is followed by a full-width value when width is especially constrained. - Preserve the themed label color, rich inline formatting, links, and the existing grid presentation for tables that remain readable. - Add snapshot coverage for path-heavy narrow tables, prose-heavy issue tables, systemic compact fragmentation, and a control case that should continue to render as a grid. ## How to Test 1. Start Codex from this branch and render a normal multi-column markdown table at a comfortable terminal width. Confirm it still appears as the styled row-separated grid from #24489. 2. Render a table containing a long linked record identifier or file-like value, then narrow the terminal until the grid would split the value into vertical fragments. Confirm it switches to key/value records, with labels above values at very narrow widths. 3. Render a table with multiple prose-heavy columns, such as an issue summary table with `Issue`, `Activity`, `Complexity`, and `Why start`. Confirm a cramped width switches to records rather than wrapping several columns into hard-to-read strips. 4. Render a compact table where only one value wraps mildly. Confirm it stays in grid form rather than switching prematurely. ## Validation - Ran `just test -p codex-tui` while developing the fallback and reviewed/accepted the intended new markdown-render snapshots. The command still reports two unrelated existing guardian feature-flag test failures outside this diff. - Ran `just fix -p codex-tui` and `just fmt` after the Rust changes were complete. - `just argument-comment-lint` cannot reach source linting locally because Bazel fails while resolving LLVM sanitizer headers; touched positional literal callsites were inspected manually and annotated where needed.
This commit is contained in:
Felipe Coury authored and GitHub committed
1 parent
7a26497836
commit
26c9502121
7 files changed
+528
-32
No files matched your search
@@ -0,0 +1,254 @@
|
||||
//! Vertical key/value rendering for markdown tables that no longer scan well as grids.
|
||||
|
||||
use super::TABLE_BODY_SEPARATOR_CHAR;
|
||||
use super::TableCell;
|
||||
use super::TableColumnKind;
|
||||
use super::TableColumnMetrics;
|
||||
use crate::render::line_utils::push_owned_lines;
|
||||
use crate::wrapping::RtOptions;
|
||||
use crate::wrapping::word_wrap_line;
|
||||
use ratatui::style::Style;
|
||||
use ratatui::text::Line;
|
||||
use ratatui::text::Span;
|
||||
use unicode_width::UnicodeWidthStr;
|
||||
|
||||
const FIELD_LEADING_PADDING: usize = 1;
|
||||
const FIELD_GAP: usize = 2;
|
||||
const MIN_VALUE_WIDTH: usize = 3;
|
||||
const MIN_ALIGNED_COMPACT_VALUE_WIDTH: usize = 12;
|
||||
const MIN_ALIGNED_EXPANSIVE_VALUE_WIDTH: usize = 24;
|
||||
const MIN_SCANNABLE_NARRATIVE_WIDTH: usize = 12;
|
||||
const MIN_SCANNABLE_TOKEN_HEAVY_WIDTH: usize = 12;
|
||||
const CRAMPED_EXPANSIVE_CELL_LINES: usize = 4;
|
||||
const CATASTROPHIC_NARRATIVE_CELL_LINES: usize = 7;
|
||||
const STACKED_VALUE_INDENT: usize = 2;
|
||||
|
||||
/// Switch modes after enough records contain values the grid can no longer
|
||||
/// present in useful chunks or expansive content collapses into tall strips.
|
||||
pub(super) fn should_render_records(
|
||||
rows: &[Vec<TableCell>],
|
||||
column_widths: &[usize],
|
||||
metrics: &[TableColumnMetrics],
|
||||
) -> bool {
|
||||
if rows.is_empty() {
|
||||
return false;
|
||||
}
|
||||
|
||||
let affected_rows = rows
|
||||
.iter()
|
||||
.filter(|row| {
|
||||
let contains_fragmented_value =
|
||||
row.iter()
|
||||
.zip(column_widths)
|
||||
.zip(metrics)
|
||||
.any(|((cell, width), metrics)| {
|
||||
let has_fragmented_token = cell
|
||||
.plain_text()
|
||||
.split_whitespace()
|
||||
.any(|token| token.width() > *width);
|
||||
match metrics.kind {
|
||||
TableColumnKind::Compact => has_fragmented_token,
|
||||
TableColumnKind::TokenHeavy => {
|
||||
*width < MIN_SCANNABLE_TOKEN_HEAVY_WIDTH && has_fragmented_token
|
||||
}
|
||||
TableColumnKind::Narrative => false,
|
||||
}
|
||||
});
|
||||
|
||||
contains_fragmented_value || expansive_cells_are_starved(row, column_widths, metrics)
|
||||
})
|
||||
.count();
|
||||
let threshold = if rows.len() == 1 {
|
||||
1
|
||||
} else {
|
||||
2.max(rows.len().div_ceil(3))
|
||||
};
|
||||
|
||||
affected_rows >= threshold
|
||||
}
|
||||
|
||||
fn expansive_cells_are_starved(
|
||||
row: &[TableCell],
|
||||
column_widths: &[usize],
|
||||
metrics: &[TableColumnMetrics],
|
||||
) -> bool {
|
||||
let expansive_cells: Vec<(TableColumnKind, usize, usize)> = row
|
||||
.iter()
|
||||
.zip(column_widths)
|
||||
.zip(metrics)
|
||||
.filter(|&((_cell, _width), metrics)| metrics.kind != TableColumnKind::Compact)
|
||||
.map(|((cell, width), metrics)| (metrics.kind, *width, wrap_cell(cell, *width).len()))
|
||||
.collect();
|
||||
|
||||
expansive_cells
|
||||
.iter()
|
||||
.filter(|(_, _, height)| *height >= CRAMPED_EXPANSIVE_CELL_LINES)
|
||||
.count()
|
||||
>= 2
|
||||
|| expansive_cells.iter().any(|(kind, width, height)| {
|
||||
*kind == TableColumnKind::Narrative
|
||||
&& *width < MIN_SCANNABLE_NARRATIVE_WIDTH
|
||||
&& *height >= CATASTROPHIC_NARRATIVE_CELL_LINES
|
||||
})
|
||||
}
|
||||
|
||||
pub(super) fn render_records(
|
||||
headers: &[TableCell],
|
||||
rows: &[Vec<TableCell>],
|
||||
metrics: &[TableColumnMetrics],
|
||||
available_width: Option<usize>,
|
||||
label_style: Style,
|
||||
separator_style: Style,
|
||||
) -> Vec<Line<'static>> {
|
||||
let label_width = headers
|
||||
.iter()
|
||||
.map(|header| header.plain_text().width())
|
||||
.max()
|
||||
.unwrap_or(0);
|
||||
let minimum_value_width = if metrics
|
||||
.iter()
|
||||
.any(|metrics| metrics.kind != TableColumnKind::Compact)
|
||||
{
|
||||
MIN_ALIGNED_EXPANSIVE_VALUE_WIDTH
|
||||
} else {
|
||||
MIN_ALIGNED_COMPACT_VALUE_WIDTH
|
||||
};
|
||||
let aligned_fields = available_width.is_none_or(|width| {
|
||||
FIELD_LEADING_PADDING + label_width + FIELD_GAP + minimum_value_width <= width
|
||||
});
|
||||
let mut out = Vec::new();
|
||||
|
||||
for (row_index, row) in rows.iter().enumerate() {
|
||||
for (header, value) in headers.iter().zip(row) {
|
||||
if aligned_fields {
|
||||
render_aligned_field(
|
||||
&mut out,
|
||||
header,
|
||||
value,
|
||||
label_width,
|
||||
available_width,
|
||||
label_style,
|
||||
);
|
||||
} else {
|
||||
render_stacked_field(&mut out, header, value, available_width, label_style);
|
||||
}
|
||||
}
|
||||
if row_index + 1 < rows.len() {
|
||||
let width = available_width.unwrap_or_else(|| widest_line_width(&out));
|
||||
out.push(Line::from(Span::styled(
|
||||
TABLE_BODY_SEPARATOR_CHAR.to_string().repeat(width),
|
||||
separator_style,
|
||||
)));
|
||||
}
|
||||
}
|
||||
|
||||
out
|
||||
}
|
||||
|
||||
fn render_aligned_field(
|
||||
out: &mut Vec<Line<'static>>,
|
||||
header: &TableCell,
|
||||
value: &TableCell,
|
||||
label_width: usize,
|
||||
available_width: Option<usize>,
|
||||
label_style: Style,
|
||||
) {
|
||||
let value_indent = FIELD_LEADING_PADDING + label_width + FIELD_GAP;
|
||||
let value_width = available_width
|
||||
.map(|width| width.saturating_sub(value_indent).max(MIN_VALUE_WIDTH))
|
||||
.unwrap_or_else(|| cell_width(value).max(MIN_VALUE_WIDTH));
|
||||
let wrapped_value = wrap_cell(value, value_width);
|
||||
for (line_index, value_line) in wrapped_value.into_iter().enumerate() {
|
||||
let mut spans = Vec::new();
|
||||
if line_index == 0 {
|
||||
let label = header.plain_text();
|
||||
spans.push(Span::raw(" ".repeat(FIELD_LEADING_PADDING)));
|
||||
spans.push(Span::styled(label.clone(), label_style));
|
||||
spans.push(Span::raw(
|
||||
" ".repeat(label_width.saturating_sub(label.width()) + FIELD_GAP),
|
||||
));
|
||||
} else {
|
||||
spans.push(Span::raw(" ".repeat(value_indent)));
|
||||
}
|
||||
spans.extend(value_line.spans);
|
||||
out.push(Line::from(spans));
|
||||
}
|
||||
}
|
||||
|
||||
fn render_stacked_field(
|
||||
out: &mut Vec<Line<'static>>,
|
||||
header: &TableCell,
|
||||
value: &TableCell,
|
||||
available_width: Option<usize>,
|
||||
label_style: Style,
|
||||
) {
|
||||
let label_width = available_width
|
||||
.map(|width| width.saturating_sub(FIELD_LEADING_PADDING).max(1))
|
||||
.unwrap_or_else(|| header.plain_text().width().max(1));
|
||||
let label = Line::from(Span::styled(header.plain_text(), label_style));
|
||||
let mut wrapped_labels = Vec::new();
|
||||
push_owned_lines(
|
||||
&word_wrap_line(&label, RtOptions::new(label_width)),
|
||||
&mut wrapped_labels,
|
||||
);
|
||||
for label_line in wrapped_labels {
|
||||
let mut spans = vec![Span::raw(" ".repeat(FIELD_LEADING_PADDING))];
|
||||
spans.extend(label_line.spans);
|
||||
out.push(Line::from(spans));
|
||||
}
|
||||
|
||||
let value_width = available_width
|
||||
.map(|width| width.saturating_sub(STACKED_VALUE_INDENT).max(1))
|
||||
.unwrap_or_else(|| cell_width(value).max(1));
|
||||
for value_line in wrap_cell(value, value_width) {
|
||||
let mut spans = vec![Span::raw(" ".repeat(STACKED_VALUE_INDENT))];
|
||||
spans.extend(value_line.spans);
|
||||
out.push(Line::from(spans));
|
||||
}
|
||||
}
|
||||
|
||||
fn wrap_cell(cell: &TableCell, width: usize) -> Vec<Line<'static>> {
|
||||
if cell.lines.is_empty() {
|
||||
return vec![Line::default()];
|
||||
}
|
||||
|
||||
let mut wrapped = Vec::new();
|
||||
for source_line in &cell.lines {
|
||||
let rendered = word_wrap_line(source_line, RtOptions::new(width.max(1)));
|
||||
if rendered.is_empty() {
|
||||
wrapped.push(Line::default());
|
||||
} else {
|
||||
push_owned_lines(&rendered, &mut wrapped);
|
||||
}
|
||||
}
|
||||
if wrapped.is_empty() {
|
||||
wrapped.push(Line::default());
|
||||
}
|
||||
wrapped
|
||||
}
|
||||
|
||||
fn cell_width(cell: &TableCell) -> usize {
|
||||
cell.lines
|
||||
.iter()
|
||||
.map(|line| {
|
||||
line.spans
|
||||
.iter()
|
||||
.map(|span| span.content.width())
|
||||
.sum::<usize>()
|
||||
})
|
||||
.max()
|
||||
.unwrap_or(0)
|
||||
}
|
||||
|
||||
fn widest_line_width(lines: &[Line<'_>]) -> usize {
|
||||
lines
|
||||
.iter()
|
||||
.map(|line| {
|
||||
line.spans
|
||||
.iter()
|
||||
.map(|span| span.content.width())
|
||||
.sum::<usize>()
|
||||
})
|
||||
.max()
|
||||
.unwrap_or(0)
|
||||
}
|
||||
Reference in new issue
Block a user