clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-05-16 01:02:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(tui): keep narrow markdown tables boxed when readable

Browse Source
This commit is contained in:
Felipe Coury
2026-05-01 15:50:17 -03:00
parent 939a9ffd37
commit fc40b0394e
2 changed files with 214 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files

155
codex-rs/tui/src/markdown_render/table.rs
View File

@@ -23,6 +23,14 @@ struct TableMetrics {
hard_wrap_count: usize,
}
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
enum TableFallbackPolicy {
LastResort,
Balanced,
}
const DEFAULT_TABLE_FALLBACK_POLICY: TableFallbackPolicy = TableFallbackPolicy::LastResort;
pub(super) fn render_table_lines(
rows: &[Vec<TableCell>],
width: Option<usize>,
@@ -40,7 +48,13 @@ pub(super) fn render_table_lines(
let normalized_rows = normalize_table_rows(rows, column_count);
let widths = desired_column_widths(&normalized_rows, column_count);
match choose_table_layout(&normalized_rows, &widths, available_width, column_count) {
match choose_table_layout(
&normalized_rows,
&widths,
available_width,
column_count,
current_table_fallback_policy(),
) {
Some(candidate) => render_box_table(
&normalized_rows,
&candidate.column_widths,
@@ -51,6 +65,16 @@ pub(super) fn render_table_lines(
}
}
fn current_table_fallback_policy() -> TableFallbackPolicy {
match std::env::var("CODEX_TUI_TABLE_FALLBACK_POLICY")
.ok()
.as_deref()
{
Some("balanced") => TableFallbackPolicy::Balanced,
_ => DEFAULT_TABLE_FALLBACK_POLICY,
}
}
pub(super) fn normalize_table_boundaries(input: &str) -> Cow<'_, str> {
if !input.contains('|') {
return Cow::Borrowed(input);
@@ -188,12 +212,16 @@ fn choose_table_layout(
desired_widths: &[usize],
available_width: usize,
column_count: usize,
policy: TableFallbackPolicy,
) -> Option<TableLayoutCandidate> {
if available_width < 32 || width_shape_requires_vertical(column_count, available_width) {
if policy == TableFallbackPolicy::Balanced
&& (available_width < 32 || width_shape_requires_vertical(column_count, available_width))
{
return None;
}
let normal = allocate_table_widths(
rows,
desired_widths,
available_width,
column_count,
@@ -206,6 +234,7 @@ fn choose_table_layout(
widths,
available_width,
/*padding*/ 1,
policy,
)
});
if normal.is_some() {
@@ -213,6 +242,7 @@ fn choose_table_layout(
}
allocate_table_widths(
rows,
desired_widths,
available_width,
column_count,
@@ -225,6 +255,7 @@ fn choose_table_layout(
widths,
available_width,
/*padding*/ 0,
policy,
)
})
}
@@ -239,6 +270,7 @@ fn build_table_candidate(
column_widths: Vec<usize>,
available_width: usize,
padding: usize,
policy: TableFallbackPolicy,
) -> Option<TableLayoutCandidate> {
let metrics = table_metrics(rows, &column_widths, /*hard_wrap*/ false);
let needs_hard_wrap = metrics.hard_wrap_count > 0 || metrics.max_body_row_height > 12;
@@ -251,15 +283,25 @@ fn build_table_candidate(
(false, metrics)
};
if should_render_vertical(
rows,
desired_widths,
&column_widths,
available_width,
padding,
hard_wrap,
&metrics,
) {
let should_fallback = match policy {
TableFallbackPolicy::LastResort => should_render_vertical_last_resort(
rows,
&column_widths,
available_width,
padding,
&metrics,
),
TableFallbackPolicy::Balanced => should_render_vertical_balanced(
rows,
desired_widths,
&column_widths,
available_width,
padding,
hard_wrap,
&metrics,
),
};
if should_fallback {
return None;
}
@@ -271,6 +313,7 @@ fn build_table_candidate(
}
fn allocate_table_widths(
rows: &[Vec<TableCell>],
desired_widths: &[usize],
available_width: usize,
column_count: usize,
@@ -286,15 +329,54 @@ fn allocate_table_widths(
let mut widths = vec![3; column_count];
let mut remaining = available_content_width - min_total;
while remaining > 0 {
let basic_targets = desired_widths
.iter()
.enumerate()
.map(|(index, desired)| {
let header = rows
.first()
.and_then(|row| row.get(index))
.map(normalized_header)
.unwrap_or_default();
let is_icon_column = matches!(header.as_str(), "icon" | "emoji");
let compact_target = if is_index_column(rows, index) || is_icon_column {
4
} else {
6
};
(*desired).min(compact_target).max(3)
})
.collect::<Vec<_>>();
grow_columns_to_targets(&mut widths, &mut remaining, &basic_targets);
let content_targets = desired_widths
.iter()
.enumerate()
.map(|(index, desired)| {
if is_content_heavy_column(rows, index) {
(*desired).min(24).max(widths[index])
} else {
widths[index]
}
})
.collect::<Vec<_>>();
grow_columns_to_targets(&mut widths, &mut remaining, &content_targets);
grow_columns_to_targets(&mut widths, &mut remaining, desired_widths);
Some(widths)
}
fn grow_columns_to_targets(widths: &mut [usize], remaining: &mut usize, targets: &[usize]) {
while *remaining > 0 {
let mut changed = false;
for index in 0..column_count {
if remaining == 0 {
for index in 0..widths.len() {
if *remaining == 0 {
break;
}
if widths[index] < desired_widths[index] {
if widths[index] < targets[index] {
widths[index] += 1;
remaining -= 1;
*remaining -= 1;
changed = true;
}
}
@@ -302,8 +384,6 @@ fn allocate_table_widths(
break;
}
}
Some(widths)
}
fn render_box_table(
@@ -480,7 +560,44 @@ fn cell_needs_hard_wrap(cell: &TableCell, width: usize) -> bool {
.any(|token| token.width() > width)
}
fn should_render_vertical(
fn should_render_vertical_last_resort(
rows: &[Vec<TableCell>],
column_widths: &[usize],
available_width: usize,
padding: usize,
metrics: &TableMetrics,
) -> bool {
let column_count = column_widths.len();
let body_rows = rows.len().saturating_sub(1);
if metrics.max_body_row_height > 24
|| (body_rows >= 10 && metrics.average_body_row_height > 6.0)
|| (body_rows >= 24 && metrics.average_body_row_height > 4.0)
{
return true;
}
let non_index_columns = (0..column_count)
.filter(|index| !is_index_column(rows, *index))
.count();
let starved_non_index_columns = column_widths
.iter()
.enumerate()
.filter(|(index, width)| !is_index_column(rows, *index) && **width <= 3)
.count();
if column_count >= 4
&& body_rows >= 12
&& non_index_columns > 0
&& starved_non_index_columns == non_index_columns
{
return true;
}
has_width_risk_chars(rows)
&& available_width <= 24
&& table_total_width(column_widths, padding) >= available_width
}
fn should_render_vertical_balanced(
rows: &[Vec<TableCell>],
desired_widths: &[usize],
column_widths: &[usize],

109
codex-rs/tui/src/markdown_render_tests.rs
View File

@@ -172,55 +172,55 @@ fn table_readability_fallback_keeps_dense_large_table_boxed_when_wide() {
}
#[test]
fn table_readability_fallback_uses_vertical_for_dense_large_table_when_narrow() {
fn table_readability_fallback_keeps_dense_large_table_boxed_when_narrow() {
let rendered = render_markdown_text_with_width_and_cwd(
&dense_large_table_fixture(),
Some(64),
/*width*/ Some(64),
/*cwd*/ None,
);
let lines = plain_lines(&rendered);
assert!(
lines.iter().any(|line| line.contains("│ Token │ Row 31"))
lines.iter().any(|line| line.starts_with("┌"))
&& lines
.iter()
.any(|line| line.contains("│ Link │ row (https://example.com/31)")),
"vertical fallback should render aligned key/value records: {lines:?}"
.any(|line| line.contains("│ 31") && line.contains("Row 31")),
"last-resort fallback should keep narrow dense tables boxed with wrapping: {lines:?}"
);
assert!(
lines.iter().any(|line| line.contains('┼')),
"vertical fallback should separate source rows: {lines:?}"
);
assert!(
lines.iter().all(|line| !line.starts_with("Row ")),
"vertical fallback should not render row headers: {lines:?}"
lines
.iter()
.all(|line| !line.contains("│ Token │ Row 31")),
"narrow dense table should not use key/value fallback: {lines:?}"
);
assert_table_lines_leave_wrap_safety_column(&lines, /*width*/ 64);
}
#[test]
fn table_readability_fallback_wraps_vertical_values_under_value_column() {
fn table_readability_fallback_wraps_boxed_values_before_fallback() {
let markdown = "| # | Color Sample |\n| --- | --- |\n| 31 | The color sample is the same as the others, but with a touch of white too. |\n";
let rendered = render_markdown_text_with_width_and_cwd(markdown, Some(30), /*cwd*/ None);
let rendered =
render_markdown_text_with_width_and_cwd(markdown, /*width*/ Some(30), /*cwd*/ None);
let lines = plain_lines(&rendered);
assert_eq!(lines[0], "┌──────────────┬────────────┐");
assert!(
lines
.iter()
.any(|line| line.starts_with("│ Color Sample │ The color")),
"first vertical value line should include the boxed label: {lines:?}"
.any(|line| line.contains("│ 31") && line.contains("The color sample")),
"first boxed value line should include the row value: {lines:?}"
);
assert!(
lines
.iter()
.skip(1)
.any(|line| line.starts_with("│ │ sample")),
"wrapped vertical value should align under the value column: {lines:?}"
.any(|line| line.contains("│ │") && line.contains("the same as the")),
"wrapped boxed value should align under its column: {lines:?}"
);
assert!(
lines.iter().all(|line| !line.starts_with("Row ")),
"vertical fallback should not render row headers: {lines:?}"
lines
.iter()
.all(|line| !line.contains("│ Color Sample │ The color")),
"wrappable two-column table should not use key/value fallback: {lines:?}"
);
assert_table_lines_leave_wrap_safety_column(&lines, /*width*/ 30);
}
@@ -228,7 +228,8 @@ fn table_readability_fallback_wraps_vertical_values_under_value_column() {
#[test]
fn table_readability_fallback_keeps_small_status_table_boxed_when_narrow() {
let markdown = "| ID | Status | Owner | Summary |\n| --- | --- | --- | --- |\n| 1 | ✅ Done | Ana | Added markdown_table parsing |\n| 2 | 🟡 Review | Ben | Checks links like RFC (https://example.com/rfc) |\n| 3 | 🔴 Blocked | ci-bot | Fails on foo \\| bar escaping |\n| 4 | ⚪ Planned | Dana | Needs snapshot coverage |\n";
let rendered = render_markdown_text_with_width_and_cwd(markdown, Some(64), /*cwd*/ None);
let rendered =
render_markdown_text_with_width_and_cwd(markdown, /*width*/ Some(64), /*cwd*/ None);
let lines = plain_lines(&rendered);
assert!(
@@ -238,10 +239,33 @@ fn table_readability_fallback_keeps_small_status_table_boxed_when_narrow() {
assert_table_lines_leave_wrap_safety_column(&lines, /*width*/ 64);
}
#[test]
fn table_readability_fallback_keeps_video_small_table_boxed() {
let markdown = "| Emoji | Title | Content | State | Notes |\n| --- | --- | --- | --- | --- |\n| 🚀 | Alpha | **bold** text | done | launch |\n| 🎯 | Beta | *italic* text | pending | target |\n| 💡 | Gamma | `inline code` | review | snippet |\n| 🔗 | Delta | [delta](https://example.com/delta) | done | link cell |\n| ✨ | Epsilon | ~~strike~~ | done | cleanup |\n";
let rendered =
render_markdown_text_with_width_and_cwd(markdown, /*width*/ Some(64), /*cwd*/ None);
let lines = plain_lines(&rendered);
assert!(
lines
.iter()
.any(|line| line.starts_with("│ 🚀") && line.contains("Alpha")),
"video small table should render as a boxed table at 64 columns: {lines:?}"
);
assert!(
lines
.iter()
.all(|line| !line.contains("│ Emoji │ 🚀")),
"video small table should not use key/value fallback at 64 columns: {lines:?}"
);
assert_table_lines_leave_wrap_safety_column(&lines, /*width*/ 64);
}
#[test]
fn table_readability_fallback_keeps_tiny_table_boxed() {
let markdown = "| Tiny | Table |\n| --- | --- |\n| A | B |\n| C | D |\n";
let rendered = render_markdown_text_with_width_and_cwd(markdown, Some(40), /*cwd*/ None);
let rendered =
render_markdown_text_with_width_and_cwd(markdown, /*width*/ Some(40), /*cwd*/ None);
let lines = plain_lines(&rendered);
assert!(
@@ -252,28 +276,52 @@ fn table_readability_fallback_keeps_tiny_table_boxed() {
}
#[test]
fn table_readability_fallback_uses_vertical_for_emoji_near_fit() {
fn table_readability_fallback_keeps_emoji_near_fit_boxed() {
let markdown = "| Feature | Markdown Coverage | Sample Output |\n| --- | --- | --- |\n| Emoji | 😎 ✅ 🧩 | Visual glyphs |\n";
let rendered = render_markdown_text_with_width_and_cwd(markdown, Some(41), /*cwd*/ None);
let rendered =
render_markdown_text_with_width_and_cwd(markdown, /*width*/ Some(41), /*cwd*/ None);
let lines = plain_lines(&rendered);
assert!(
lines
.iter()
.any(|line| line.contains("│ Markdown Coverage │ 😎 ✅ 🧩")),
"emoji-heavy near-fit table should fall back to key/value layout: {lines:?}"
.any(|line| line.contains("│ Emoji") && line.contains("😎 ✅ 🧩")),
"emoji-heavy near-fit table should remain boxed: {lines:?}"
);
assert!(
lines.iter().all(|line| !line.starts_with("Row ")),
"emoji-heavy fallback should not render row headers: {lines:?}"
lines
.iter()
.all(|line| !line.contains("│ Markdown Coverage │ 😎 ✅ 🧩")),
"emoji-heavy near-fit table should not use key/value fallback: {lines:?}"
);
assert_table_lines_leave_wrap_safety_column(&lines, /*width*/ 41);
}
#[test]
fn table_readability_fallback_uses_vertical_when_boxed_is_impossible() {
let markdown = "| A | B | C | D | E |\n| --- | --- | --- | --- | --- |\n| one | two | three | four | five |\n| six | seven | eight | nine | ten |\n";
let rendered =
render_markdown_text_with_width_and_cwd(markdown, /*width*/ Some(20), /*cwd*/ None);
let lines = plain_lines(&rendered);
assert!(
lines
.iter()
.any(|line| line.contains("A │ one")),
"impossibly narrow table should use key/value fallback: {lines:?}"
);
assert!(
lines.iter().any(|line| line.contains('┼')),
"vertical fallback should separate source rows: {lines:?}"
);
assert_table_lines_leave_wrap_safety_column(&lines, /*width*/ 20);
}
#[test]
fn table_inline_links_and_html_breaks_stay_inside_table() {
let markdown = "| A | B |\n|---|---|\n| [link](https://example.com) | [CLI docs](https://example.com/cli) |\n| one<br>two | three<br>four |\n";
let rendered = render_markdown_text_with_width_and_cwd(markdown, Some(72), /*cwd*/ None);
let rendered =
render_markdown_text_with_width_and_cwd(markdown, /*width*/ Some(72), /*cwd*/ None);
let lines = plain_lines(&rendered);
assert!(
@@ -324,8 +372,7 @@ fn table_preserves_inline_styles_in_boxed_layout() {
#[test]
fn table_preserves_inline_styles_in_vertical_layout() {
let markdown =
"| Feature | Sample | Notes |\n| --- | --- | --- |\n| Code | `cargo test` | **done** |\n";
let markdown = "| # | Feature | Sample | Notes | Extra | A | B | C |\n| --- | --- | --- | --- | --- | --- | --- | --- |\n| 1 | Code | `cargo test` | **done** | x | y | z | q |\n";
let rendered =
render_markdown_text_with_width_and_cwd(markdown, /*width*/ Some(30), /*cwd*/ None);
let lines = plain_lines(&rendered);