mirror of
https://github.com/openai/codex.git
synced 2026-10-25 18:28:38 +00:00
fix(tui): detect single-column tables and narrow spillover
This commit is contained in:
committed by
Felipe Coury
parent
2d6cb4ed1a
commit
f6ef212f76
@@ -369,6 +369,16 @@ mod tests {
|
||||
assert!(!rendered.iter().any(|line| line.trim() == "A | B"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn append_markdown_agent_unwraps_markdown_fences_for_single_column_table() {
|
||||
let src = "```md\n| Only |\n|---|\n| value |\n```\n";
|
||||
let mut out = Vec::new();
|
||||
append_markdown_agent(src, None, &mut out);
|
||||
let rendered = lines_to_strings(&out);
|
||||
assert!(rendered.iter().any(|line| line.contains("┌")));
|
||||
assert!(!rendered.iter().any(|line| line.trim() == "| Only |"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn append_markdown_agent_keeps_non_markdown_fences_as_code() {
|
||||
let src = "```rust\n| A | B |\n|---|---|\n| 1 | 2 |\n```\n";
|
||||
|
||||
@@ -1308,7 +1308,7 @@ where
|
||||
///
|
||||
/// Heuristic: a row is spillover if its only non-empty cell is the first one
|
||||
/// AND (the row has only one cell, or the content looks like HTML, or it's a
|
||||
/// label line followed by HTML content, or a trailing multi-word label line).
|
||||
/// label line followed by HTML content, or a trailing HTML-intro label line).
|
||||
fn is_spillover_row(row: &[TableCell], next_row: Option<&Vec<TableCell>>) -> bool {
|
||||
let Some(first_text) = Self::first_non_empty_only_text(row) else {
|
||||
return false;
|
||||
@@ -1333,10 +1333,9 @@ where
|
||||
}
|
||||
|
||||
// pulldown can end the table before the corresponding HTML block line.
|
||||
// In that case, treat multi-word label lines (e.g., "HTML block:") as
|
||||
// spillover instead of keeping them inside the table grid.
|
||||
let has_multiple_words = first_text.split_whitespace().nth(1).is_some();
|
||||
if next_row.is_none() && has_multiple_words {
|
||||
// In that case, treat trailing HTML-intro labels (e.g., "HTML block:")
|
||||
// as spillover while keeping explicit sparse labels in real tables.
|
||||
if next_row.is_none() && Self::looks_like_html_label_line(&first_text) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
@@ -1359,6 +1358,17 @@ where
|
||||
text.contains('<') && text.contains('>')
|
||||
}
|
||||
|
||||
fn looks_like_html_label_line(text: &str) -> bool {
|
||||
let trimmed = text.trim();
|
||||
if !trimmed.ends_with(':') {
|
||||
return false;
|
||||
}
|
||||
let prefix = trimmed.trim_end_matches(':').trim();
|
||||
prefix
|
||||
.split_whitespace()
|
||||
.any(|word| word.eq_ignore_ascii_case("html"))
|
||||
}
|
||||
|
||||
fn spans_display_width(spans: &[Span<'_>]) -> usize {
|
||||
spans.iter().map(|span| span.content.width()).sum()
|
||||
}
|
||||
|
||||
@@ -1607,6 +1607,28 @@ fn table_keeps_single_word_label_row_at_end_inside_grid() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn table_keeps_multi_word_label_row_at_end_inside_grid() {
|
||||
let md = "| A | B | C |\n|---|---|---|\n| Build status: | | |\n";
|
||||
let text = render_markdown_text(md);
|
||||
let lines: Vec<String> = text
|
||||
.lines
|
||||
.iter()
|
||||
.map(|line| line.spans.iter().map(|span| span.content.clone()).collect())
|
||||
.collect();
|
||||
|
||||
assert!(
|
||||
lines
|
||||
.iter()
|
||||
.any(|line| line.contains("│ Build status:") && line.ends_with('│')),
|
||||
"expected multi-word trailing label row to remain inside table grid: {lines:?}"
|
||||
);
|
||||
assert!(
|
||||
!lines.iter().any(|line| line.trim() == "Build status:"),
|
||||
"did not expect multi-word trailing label row to spill outside table: {lines:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn table_preserves_structured_leading_columns_when_last_column_is_long() {
|
||||
let md = "| Milestone | Planned Date | Outcome | Retrospective Summary |\n|---|---|---|---|\n| Canary rollout | 2026-01-10 | Completed | Canary
|
||||
|
||||
@@ -1356,6 +1356,15 @@ mod tests {
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn table_holdback_state_detects_single_column_header_plus_delimiter() {
|
||||
let source = "| Only |\n| --- |\n";
|
||||
assert!(matches!(
|
||||
table_holdback_state(source),
|
||||
TableHoldbackState::Confirmed
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn table_holdback_state_ignores_table_like_lines_inside_unclosed_long_fence() {
|
||||
let source = "````sh\n```cmd\n| Key | Description |\n| --- | --- |\n````\n";
|
||||
|
||||
@@ -7,11 +7,11 @@
|
||||
|
||||
/// Split a pipe-delimited line into trimmed segments.
|
||||
///
|
||||
/// Returns `None` if the line is empty or has fewer than two segments.
|
||||
/// Returns `None` if the line is empty or has no `|` marker.
|
||||
/// Leading/trailing pipes are stripped before splitting.
|
||||
pub(crate) fn parse_table_segments(line: &str) -> Option<Vec<&str>> {
|
||||
let trimmed = line.trim();
|
||||
if trimmed.is_empty() {
|
||||
if trimmed.is_empty() || !trimmed.contains('|') {
|
||||
return None;
|
||||
}
|
||||
|
||||
@@ -24,7 +24,7 @@ pub(crate) fn parse_table_segments(line: &str) -> Option<Vec<&str>> {
|
||||
}
|
||||
|
||||
let segments: Vec<&str> = content.split('|').map(str::trim).collect();
|
||||
(segments.len() >= 2).then_some(segments)
|
||||
(!segments.is_empty()).then_some(segments)
|
||||
}
|
||||
|
||||
/// Whether `line` looks like a table header row (has pipe-separated
|
||||
@@ -86,8 +86,13 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_table_segments_single_segment_returns_none() {
|
||||
assert_eq!(parse_table_segments("| only |"), None);
|
||||
fn parse_table_segments_single_segment_is_allowed() {
|
||||
assert_eq!(parse_table_segments("| only |"), Some(vec!["only"]));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_table_segments_without_pipe_returns_none() {
|
||||
assert_eq!(parse_table_segments("just text"), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user