From 75a714843b7fc3c4e76235f31e2fd32aaf596ebf Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Sat, 26 Sep 2026 21:47:01 +0000 Subject: [PATCH] Preserve Markdown tables and whitespace when copying TUI responses (#48549) ## Why Copied table selections became code blocks containing the rendered grid, losing table structure. Stripping trailing whitespace from completed responses also removed Markdown hard breaks and spaces in code. ## What changed - Reconstruct Markdown tables from selected cells across grid and record layouts, preserving alignment, inline formatting, and list or blockquote nesting. - Copy a single selected cell as inline content and leave unselected cells empty without adding their text. Escape literal pipes in table code spans and link destinations. - Preserve trailing whitespace on lines unchanged by assistant directive removal. - Keep blank rows in copied text without painting newline selection highlights on empty rows. ## Testing Add regression coverage for wrapped and rewrapped tables, partial selections, empty cells, literal pipes, and separate table and code blocks. Update snapshots for nested tables and selection highlights, and verify completed-response copying preserves hard breaks and code whitespace. GitOrigin-RevId: 661e4c8331f2737bedff286548f9343bac44e0cb --- .../tui/src/app/tests/transcript_selection.rs | 4 +- .../tests/copy_export_picker_tests.rs | 22 +++ .../src/chatwidget/tests/slash_commands.rs | 2 +- codex-rs/tui/src/git_action_directives.rs | 10 +- codex-rs/tui/src/markdown_copy.rs | 51 ++++- codex-rs/tui/src/markdown_copy/table.rs | 117 ++++++++++- codex-rs/tui/src/markdown_render.rs | 2 - .../transcript_view/markdown_copy_tests.rs | 5 +- ...ted_tables_keep_their_copy_containers.snap | 32 ++- ...keeps_prose_outside_the_literal_table.snap | 18 +- ...py_spacing_without_newline_highlights.snap | 27 +++ .../src/transcript_view/table_copy_tests.rs | 183 ++++++++++++++++++ codex-rs/tui/src/transcript_view/text.rs | 7 +- .../tui/src/transcript_view/text_tests.rs | 2 +- 14 files changed, 437 insertions(+), 45 deletions(-) create mode 100644 codex-rs/tui/src/transcript_view/snapshots/codex_tui__transcript_view__markdown_copy_tests__tables__empty_selected_rows_keep_copy_spacing_without_newline_highlights.snap create mode 100644 codex-rs/tui/src/transcript_view/table_copy_tests.rs diff --git a/codex-rs/tui/src/app/tests/transcript_selection.rs b/codex-rs/tui/src/app/tests/transcript_selection.rs index 735a7db672..29f2782ef3 100644 --- a/codex-rs/tui/src/app/tests/transcript_selection.rs +++ b/codex-rs/tui/src/app/tests/transcript_selection.rs @@ -72,8 +72,8 @@ async fn text_selection_temporarily_replaces_the_rendered_backtrack_highlight() .await?; } let (_, highlighted) = render(&mut app); - // The selected leading newline paints one blank cell before the first prompt character. - assert_eq!(highlighted, " S"); + // The selected leading newline does not paint an empty cell. + assert_eq!(highlighted, "S"); app.handle_tui_event( &mut tui, diff --git a/codex-rs/tui/src/chatwidget/tests/copy_export_picker_tests.rs b/codex-rs/tui/src/chatwidget/tests/copy_export_picker_tests.rs index c26fbe6849..58e4fb6257 100644 --- a/codex-rs/tui/src/chatwidget/tests/copy_export_picker_tests.rs +++ b/codex-rs/tui/src/chatwidget/tests/copy_export_picker_tests.rs @@ -5,6 +5,28 @@ use crate::app_event::TranscriptExportDestination; use crate::clipboard_copy::CopyFormat; use pretty_assertions::assert_eq; +#[tokio::test] +async fn completed_response_copy_preserves_markdown_line_endings() { + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + let markdown = "Hard break: \nstarts a new line.\n\n```text\ncode with trailing spaces \n```"; + replay_agent_message( + &mut chat, + "hard-break", + format!("{markdown}\n\n::git-stage{{cwd=\"/repo\"}}"), + ReplayKind::ThreadSnapshot, + ); + chat.show_copy_picker(); + chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); + let copied = std::iter::from_fn(|| rx.try_recv().ok()).find_map(|event| match event { + AppEvent::CopySelection { text, format, .. } => Some((text.to_string(), format)), + _ => None, + }); + assert_eq!(copied, Some((markdown.to_string(), CopyFormat::Markdown))); + insta::assert_debug_snapshot!(crate::clipboard_html::render_markdown( + chat.transcript.last_agent_markdown.as_deref().unwrap() + ), @r#""

Hard break:
\nstarts a new line.

\n
code with trailing spaces  \n
\n""#); +} + #[tokio::test] async fn copy_export_picker_custom_keys_preserve_payloads_and_composer_draft() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; diff --git a/codex-rs/tui/src/chatwidget/tests/slash_commands.rs b/codex-rs/tui/src/chatwidget/tests/slash_commands.rs index 1392e14ec4..e8e5b4c459 100644 --- a/codex-rs/tui/src/chatwidget/tests/slash_commands.rs +++ b/codex-rs/tui/src/chatwidget/tests/slash_commands.rs @@ -1665,7 +1665,7 @@ async fn slash_copy_picker_preserves_completed_source_whitespace_and_hides_direc for (key, expected, label) in [ ( '1', - "Intro\n\n```powershell\nWrite-Output value\nWrite-Output done\n```\n\n> Keep **formatting**\n> > Nested quote\n> hidden\n\n>", + "Intro\n\n```powershell\nWrite-Output value \nWrite-Output done\t\n```\n\n> Keep **formatting** \n> > Nested quote\n> hidden\n\n>", "Whole response", ), ( diff --git a/codex-rs/tui/src/git_action_directives.rs b/codex-rs/tui/src/git_action_directives.rs index 7477d7053b..2d86284cbc 100644 --- a/codex-rs/tui/src/git_action_directives.rs +++ b/codex-rs/tui/src/git_action_directives.rs @@ -1,4 +1,6 @@ //! Codex App directives embedded in assistant markdown. +//! +//! Preserve whitespace on unchanged lines: Markdown hard breaks and code can depend on it. use crate::assistant_directives::AssistantDirective; use crate::assistant_directives::QuoteEscaping; @@ -67,12 +69,16 @@ pub(crate) fn parse_assistant_markdown(markdown: &str, cwd: &Path) -> ParsedAssi git_actions.push(action); } } - visible_lines.push(visible_line.trim_end().to_string()); + visible_lines.push(if visible_line == line { + visible_line + } else { + visible_line.trim_end().to_string() + }); } while visible_lines .last() - .is_some_and(std::string::String::is_empty) + .is_some_and(|line| line.trim().is_empty()) { visible_lines.pop(); } diff --git a/codex-rs/tui/src/markdown_copy.rs b/codex-rs/tui/src/markdown_copy.rs index 7579a003a9..f70d7639d9 100644 --- a/codex-rs/tui/src/markdown_copy.rs +++ b/codex-rs/tui/src/markdown_copy.rs @@ -59,6 +59,7 @@ pub(crate) struct CopyLine { pub(crate) item_prefix: String, pub(crate) code: bool, pub(crate) table: Option, + table_cell: bool, pub(crate) rule: bool, pub(crate) heading: usize, pub(crate) hard_break: bool, @@ -120,7 +121,12 @@ impl CopyLine { match mark { Some(Inline::Code) => { let content = &text[start..end]; - let fence = fence(content, /*minimum*/ 1); + let content = if self.table_cell { + content.replace('|', "\\|") + } else { + content.to_owned() + }; + let fence = fence(&content, /*minimum*/ 1); let padding = if content.starts_with(['`', ' ']) || content.ends_with(['`', ' ']) { if content.chars().all(|ch| ch == ' ') { @@ -154,6 +160,7 @@ impl CopyLine { .replace('&', "&") .replace('<', "%3C") .replace('>', "%3E") + .replace('|', "%7C") .replace('\n', "%0A") .replace('\r', "%0D"); append_inline(&mut out, &format!("[{trimmed}](<{destination}>)")); @@ -291,7 +298,47 @@ pub(crate) fn selection(lines: &[SelectedLine], plain: &str) -> (String, CopyFor first = false; continue; } - if line.source.copy_as_prose { + if let Some(table) = line + .source + .copy + .as_ref() + .and_then(|copy| copy.table.as_ref()) + { + let mut selected = vec![line]; + while let Some(next) = lines.peek() + && next + .source + .copy + .as_ref() + .and_then(|copy| copy.table.as_ref()) + .is_some_and(|next| Arc::ptr_eq(&next.table, &table.table)) + { + selected.extend(lines.next()); + } + let body = table::render(&selected, table); + let continuation = line + .source + .copy + .as_ref() + .map_or("", |copy| copy.continuation.as_str()); + let continuation = dedent(continuation, indentation.unwrap_or(/*default*/ 0)); + for (index, row) in body.lines().enumerate() { + if index > 0 { + out.push('\n'); + out.push_str(&continuation); + } else { + out.push_str(&prefix); + } + out.push_str(row); + } + if lines + .peek() + .is_some_and(|next| next.separator == "\n" && !next.range.is_empty()) + { + out.push('\n'); + out.push_str(continuation.trim_end()); + } + } else if line.source.copy_as_prose { let text = &line.source.text[line.range.clone()]; push_prose_fragment(&mut out, &escape(text)); } else if let Some(copy) = &line.source.copy diff --git a/codex-rs/tui/src/markdown_copy/table.rs b/codex-rs/tui/src/markdown_copy/table.rs index 0add17b119..63dfc8a329 100644 --- a/codex-rs/tui/src/markdown_copy/table.rs +++ b/codex-rs/tui/src/markdown_copy/table.rs @@ -1,11 +1,14 @@ -//! Retain original table-cell ranges through grid padding, wrapping, and record labels. +//! Copy selected table cells independently of grid padding, wrapping, and record labels. //! -//! Each displayed fragment keeps its source line and cell coordinates so selection -//! serialization can recover only visible, selected content. +//! Each visual fragment refers to an original cell line. Only selected fragments are +//! serialized; repeated record labels and whitespace removed by wrapping are reconciled +//! by their shared source identity. Unselected cells never supply hidden text. use super::CopyLine; +use super::SelectedLine; use crate::terminal_hyperlinks::HyperlinkLine; use crate::terminal_hyperlinks::LogicalLineSource; +use std::collections::BTreeMap; use std::ops::Range; use std::sync::Arc; @@ -96,3 +99,111 @@ pub(crate) fn append(output: &mut Option, source: &LogicalLineSource, }); } } + +pub(super) fn render(lines: &[&SelectedLine], table: &TableLine) -> String { + let mut cells: BTreeMap<(usize, usize), Vec> = BTreeMap::new(); + for line in lines { + let Some(copy) = line + .source + .copy + .as_ref() + .and_then(|copy| copy.table.as_ref()) + else { + continue; + }; + for fragment in ©.fragments { + // A selected blank row may have lost all display padding during wrapping. + let blank = line.source.range.is_empty() + && line.source.text.trim().is_empty() + && fragment.source.text.trim().is_empty(); + let start = line.range.start.max(fragment.output.start); + let end = line.range.end.min(fragment.output.end); + if !blank && (start > end || start == end && !fragment.source.text.is_empty()) { + continue; + } + let mut source = fragment.source.clone(); + if !blank { + source.range = source.range.start + start - fragment.output.start + ..source.range.start + end - fragment.output.start; + } + let parts = cells.entry((fragment.row, fragment.column)).or_default(); + let mut insert_at = parts.len(); + while let Some(index) = parts.iter().position(|previous| { + Arc::ptr_eq(&previous.text, &source.text) + && (previous.range.start <= source.range.end + && source.range.start <= previous.range.end + || previous.text[previous.range.end.min(source.range.end) + ..previous.range.start.max(source.range.start)] + .trim() + .is_empty()) + }) { + let previous = parts.remove(index); + insert_at = insert_at.min(index); + source.range.start = previous.range.start.min(source.range.start); + source.range.end = previous.range.end.max(source.range.end); + } + parts.insert(insert_at.min(parts.len()), source); + } + } + let includes_separator = lines.iter().any(|line| { + !line.range.is_empty() + && line + .source + .copy + .as_ref() + .and_then(|copy| copy.table.as_ref()) + .is_some_and(|table| table.fragments.is_empty()) + }); + let standalone = cells.len() == 1 && !includes_separator; + let cells: BTreeMap<_, _> = cells + .into_iter() + .map(|(key, parts)| { + let text = parts + .into_iter() + .map(|source| { + source.copy.as_ref().map_or_else( + || super::escape(&source.text[source.range.clone()]), + |copy| { + let mut copy = (**copy).clone(); + copy.table_cell = !standalone; + copy.render(&source.text, source.range.clone(), /*depth*/ 0) + }, + ) + }) + .collect::>() + .join(" "); + (key, text) + }) + .collect(); + if standalone { + return cells.into_values().next().unwrap_or_default(); + } + if cells.is_empty() { + return lines + .iter() + .map(|line| super::escape(&line.source.text[line.range.clone()])) + .collect::>() + .join("\n"); + } + let mut rows: BTreeMap> = BTreeMap::new(); + rows.insert(/*key*/ 0, vec![""; table.table.len()]); + for ((row, column), text) in &cells { + rows.entry(*row) + .or_insert_with(|| vec![""; table.table.len()])[*column] = text; + } + let mut out = String::new(); + for (row, values) in rows { + if !out.is_empty() { + out.push('\n'); + } + out.push_str(&format!("| {} |", values.join(" | "))); + if row == 0 { + out.push_str("\n|"); + for alignment in table.table.iter() { + out.push_str(alignment); + out.push('|'); + } + } + } + out +} diff --git a/codex-rs/tui/src/markdown_render.rs b/codex-rs/tui/src/markdown_render.rs index 7f6823512d..a7109210a2 100644 --- a/codex-rs/tui/src/markdown_render.rs +++ b/codex-rs/tui/src/markdown_render.rs @@ -1167,7 +1167,6 @@ impl<'a, 'policy> Writer<'a, 'policy> { .and_then(|copy| copy.table.clone()); self.push_hyperlink_line(line); self.copy_line.table = table; - self.copy_line.code = true; self.flush_current_line(); } pending_marker_line = false; @@ -2332,7 +2331,6 @@ impl<'a, 'policy> Writer<'a, 'policy> { copy.prefix = self.copy_prefix(pending_marker_line); copy.continuation = self.copy_prefix(/*pending_marker_line*/ false); copy.item_prefix = self.copy_prefix(/*pending_marker_line*/ true); - copy.code = true; source.copy = Some(std::sync::Arc::new(copy)); source.prefix_bytes = spans.iter().map(|span| span.content.len()).sum(); source.continuation_indent = self.prefix_spans(/*pending_marker_line*/ false).into(); diff --git a/codex-rs/tui/src/transcript_view/markdown_copy_tests.rs b/codex-rs/tui/src/transcript_view/markdown_copy_tests.rs index a3ae0aa448..8642e6e366 100644 --- a/codex-rs/tui/src/transcript_view/markdown_copy_tests.rs +++ b/codex-rs/tui/src/transcript_view/markdown_copy_tests.rs @@ -7,6 +7,9 @@ use ratatui::buffer::Buffer; use ratatui::layout::Rect; use std::path::Path; +#[path = "table_copy_tests.rs"] +mod tables; + #[path = "markdown_element_copy_tests.rs"] mod elements; @@ -361,7 +364,7 @@ fn task_lists_and_transformed_tables_keep_their_meaning() { ); let copied = payload(&layout, 0..layout.text().len()).0; let html = crate::clipboard_html::render_markdown(&copied); - assert!(html.contains("
"), "{copied}\n{html}");
+    assert!(html.contains(""), "{copied}\n{html}");
 }
 
 #[test]
diff --git a/codex-rs/tui/src/transcript_view/snapshots/codex_tui__transcript_view__markdown_copy_tests__nested_tables_keep_their_copy_containers.snap b/codex-rs/tui/src/transcript_view/snapshots/codex_tui__transcript_view__markdown_copy_tests__nested_tables_keep_their_copy_containers.snap
index 63f7c7523e..38fb1d81b2 100644
--- a/codex-rs/tui/src/transcript_view/snapshots/codex_tui__transcript_view__markdown_copy_tests__nested_tables_keep_their_copy_containers.snap
+++ b/codex-rs/tui/src/transcript_view/snapshots/codex_tui__transcript_view__markdown_copy_tests__nested_tables_keep_their_copy_containers.snap
@@ -1,21 +1,18 @@
 ---
 source: tui/src/transcript_view/markdown_copy_tests.rs
-assertion_line: 393
+assertion_line: 460
 expression: "copies.join(\"\\n---\\n\")"
 ---
 Before
 
-> ```
->  A      B
-> ━━━━━  ━━━━━
->  one    two
-> ```
+> | A | B |
+> |---|---|
+> | one | two |
 

Before

-
 A      B
-━━━━━  ━━━━━
- one    two
-
+
+ +
AB
onetwo
--- @@ -23,18 +20,15 @@ Before - intro - ``` - A B - ━━━━━ ━━━━━ - one two - ``` + | A | B | + |---|---| + | one | two |

Before

  • intro

    -
     A      B
    -━━━━━  ━━━━━
    - one    two
    -
    + + +
    AB
    onetwo
diff --git a/codex-rs/tui/src/transcript_view/snapshots/codex_tui__transcript_view__markdown_copy_tests__table_spillover_keeps_prose_outside_the_literal_table.snap b/codex-rs/tui/src/transcript_view/snapshots/codex_tui__transcript_view__markdown_copy_tests__table_spillover_keeps_prose_outside_the_literal_table.snap index 66653c0aa7..543eb1234e 100644 --- a/codex-rs/tui/src/transcript_view/snapshots/codex_tui__transcript_view__markdown_copy_tests__table_spillover_keeps_prose_outside_the_literal_table.snap +++ b/codex-rs/tui/src/transcript_view/snapshots/codex_tui__transcript_view__markdown_copy_tests__table_spillover_keeps_prose_outside_the_literal_table.snap @@ -1,24 +1,22 @@ --- source: tui/src/transcript_view/markdown_copy_tests.rs -assertion_line: 358 +assertion_line: 420 expression: "format!(\"{copied}\\n{}\", crate::clipboard_html::render_markdown(&copied))" --- **Before** -``` - A B -━━━━━ ━━━━━ - one two -``` +| A | B | +|---|---| +| one | two | + HTML block: \Prose after the table.\ After

Before

-
 A      B
-━━━━━  ━━━━━
- one    two
-
+ + +
AB
onetwo

HTML block: <div>Prose after the table.</div>

After

diff --git a/codex-rs/tui/src/transcript_view/snapshots/codex_tui__transcript_view__markdown_copy_tests__tables__empty_selected_rows_keep_copy_spacing_without_newline_highlights.snap b/codex-rs/tui/src/transcript_view/snapshots/codex_tui__transcript_view__markdown_copy_tests__tables__empty_selected_rows_keep_copy_spacing_without_newline_highlights.snap new file mode 100644 index 0000000000..8c83e00fc1 --- /dev/null +++ b/codex-rs/tui/src/transcript_view/snapshots/codex_tui__transcript_view__markdown_copy_tests__tables__empty_selected_rows_keep_copy_spacing_without_newline_highlights.snap @@ -0,0 +1,27 @@ +--- +source: tui/src/transcript_view/table_copy_tests.rs +assertion_line: 108 +expression: "highlighted.join(\"\\n\")" +--- +• Before + ^^^^^^^ + + + Task Status Next step + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + ━━━━━━━━ ━━━━━━━━━━━━━ ━━━━━━━━━━━━━━━━━━━━━━━ + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + Task 1 Complete Review results + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + ──────── ───────────── ─────────────────────── + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + Task 2 In progress Finish implementation + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + ──────── ───────────── ─────────────────────── + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + Task 3 Planned Confirm requirements + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + + + After + ^^^^^ diff --git a/codex-rs/tui/src/transcript_view/table_copy_tests.rs b/codex-rs/tui/src/transcript_view/table_copy_tests.rs new file mode 100644 index 0000000000..b792eaa31a --- /dev/null +++ b/codex-rs/tui/src/transcript_view/table_copy_tests.rs @@ -0,0 +1,183 @@ +//! Selection regressions through the real Markdown renderer, layout, and clipboard serializer. + +use super::*; +use pretty_assertions::assert_eq; + +const TABLE: &str = "| Task | Status | Next step |\n|---|---|---|\n| Task 1 | Complete | Review results |\n| Task 2 | In progress | Finish implementation |\n| Task 3 | Planned | Confirm requirements |"; + +#[test] +fn table_copy_preserves_structure_across_grid_and_record_layouts() { + let source = format!( + "The project is moving forward, with one task complete and two still to finish.\n\n{TABLE}\n\nThe next priority is to finish Task 2 while gathering the requirements needed to begin Task 3." + ); + for width in [12, 24, 40, 80, 120] { + let layout = markdown_layout(&source, width); + assert_eq!( + payload(&layout, 0..layout.text().len()), + (source.clone(), CopyFormat::Markdown), + "width {width}" + ); + let rewrapped = layout.rewrap(/*width*/ 17); + assert_eq!( + payload(&rewrapped, 0..rewrapped.text().len()).0, + source, + "rewrapped {width}" + ); + } +} + +#[test] +fn partial_table_selection_never_adds_unselected_cell_text() { + let code = markdown_layout( + "| A | B |\n|---|---|\n| `a\\|b` | value |", + /*width*/ 80, + ); + let start = code.text().find("a|b").unwrap(); + assert_eq!(payload(&code, start..start + 3).0, "`a|b`"); + let repeated = markdown_layout( + "| alpha SECRET omega |\n|---|\n| long_value_one |\n| long_value_two |", + /*width*/ 12, + ); + let start = repeated.text().find("omega").unwrap(); + let end = repeated.text().rfind("alpha").unwrap() + "alpha".len(); + assert!(!payload(&repeated, start..end).0.contains("SECRET")); + assert!( + payload(&repeated, start..repeated.text().len()) + .0 + .starts_with("| alpha SECRET omega |") + ); + let layout = markdown_layout( + "| Label | Value |\n|---|---|\n| **alpha** | beta |\n| gamma | delta |", + /*width*/ 80, + ); + let start = layout.text().find("alpha").unwrap(); + assert_eq!( + payload(&layout, start + 1..start + 4), + ("**lph**".into(), CopyFormat::Markdown) + ); + let end = layout.text().find("delta").unwrap() + "delta".len(); + let copied = payload(&layout, start..end).0; + assert_eq!( + copied, + "| | |\n|---|---|\n| **alpha** | beta |\n| gamma | delta |" + ); +} + +#[test] +fn table_cells_keep_inline_formatting_alignment_and_literal_pipes() { + let source = "| Left | Center | Right |\n|:---|:---:|---:|\n| **bold** _italic_ ~~strike~~ | `a\\|b` | [link]() |\n| café 界 | escaped \\*star\\* | last |"; + let expected_html = crate::clipboard_html::render_markdown( + &source.replace("[link]", "[link (https://example.com/?a=1&b=2)]"), + ); + for width in [16, 48, 120] { + let layout = markdown_layout(source, width); + let copied = payload(&layout, 0..layout.text().len()).0; + assert_eq!( + crate::clipboard_html::render_markdown(&copied), + expected_html, + "width {width}: {copied}" + ); + } +} + +#[test] +fn header_only_and_empty_cells_keep_their_table_positions() { + for source in [ + "| A | B |\n|---|---|", + "| A |\n|---|", + "| A | B |\n|---|---|\n| | right |\n| | |\n| left | |", + "| A | B |\n|---|---|\n| ` ` | value |", + ] { + for width in [4, 80] { + let layout = markdown_layout(source, width); + let copied = payload(&layout, 0..layout.text().len()).0; + assert_eq!( + crate::clipboard_html::render_markdown(&copied), + crate::clipboard_html::render_markdown(source), + "width {width}: {copied}" + ); + } + } +} + +#[test] +fn separate_tables_and_code_remain_separate_blocks() { + let source = "Before\n\n| A | B |\n|---|---|\n| one | two |\n\n```text\n| literal | code |\n```\n\n| C | D |\n|---|---|\n| three | four |\n\nAfter"; + let layout = markdown_layout(source, /*width*/ 80); + let copied = payload(&layout, 0..layout.text().len()).0; + assert_eq!(copied, source.replace("```text", "```")); +} + +#[test] +fn table_separator_only_selection_keeps_visible_text() { + for width in [12, 80] { + let layout = markdown_layout(TABLE, width); + let mut offset = 0; + let mut checked = false; + for line in layout.text().split('\n') { + if line.contains('─') || line.contains('━') { + assert_eq!(payload(&layout, offset..offset + line.len()).0, line); + checked = true; + } + offset += line.len() + 1; + } + assert!(checked, "no separator at width {width}"); + } +} + +#[test] +fn empty_selected_rows_keep_copy_spacing_without_newline_highlights() { + let cells: Vec> = vec![Arc::new(AgentMarkdownCell::new( + format!("Before\n\n{TABLE}\n\nAfter"), + Path::new("/"), + ))]; + let mut view = TranscriptView::default(); + let area = Rect::new( + /*x*/ 0, /*y*/ 0, /*width*/ 80, /*height*/ 20, + ); + let mut buffer = Buffer::empty(area); + view.render(area, &mut buffer, &cells); + view.begin_selection(&cells, /*column*/ 2, /*row*/ 0, /*clicks*/ 1); + view.extend_selection(/*column*/ 79, /*row*/ 19); + view.render(area, &mut buffer, &cells); + let mut highlighted = Vec::new(); + for row in 0..area.height { + let text = (0..area.width) + .map(|column| buffer[(column, row)].symbol()) + .collect::(); + let selected = (0..area.width) + .map(|column| { + if buffer[(column, row)] + .modifier + .contains(ratatui::style::Modifier::REVERSED) + { + '^' + } else { + ' ' + } + }) + .collect::(); + if text.trim().is_empty() { + assert!(selected.trim().is_empty(), "blank row {row}"); + } + highlighted.push(format!("{}\n{}", text.trim_end(), selected.trim_end())); + } + let plain = view.selected_text(&cells).unwrap(); + view.copy_selected_text_with( + &cells, + &plain, + /*clear_selection*/ false, + |text, format| { + assert_eq!( + (text, format), + ( + format!("Before\n\n{TABLE}\n\nAfter").as_str(), + CopyFormat::Markdown + ) + ); + Ok(crate::clipboard_copy::CopyStatus::Confirmed) + }, + ) + .unwrap(); + insta::assert_snapshot!(highlighted.join("\n")); +} diff --git a/codex-rs/tui/src/transcript_view/text.rs b/codex-rs/tui/src/transcript_view/text.rs index 5779498cc6..44877552ca 100644 --- a/codex-rs/tui/src/transcript_view/text.rs +++ b/codex-rs/tui/src/transcript_view/text.rs @@ -4,7 +4,7 @@ //! lines introduce hard breaks. Display wrapping and synthetic controls never enter `text`. //! Layout offsets use `usize`; terminal coordinates are narrowed only for visible rows. //! Disclosure controls follow their activity's source text without changing its indentation. -//! Selected hard breaks highlight one trailing cell when space permits. +//! Selected hard breaks on nonempty rows highlight one trailing cell when space permits. use std::borrow::Cow; use std::ops::Range; @@ -318,7 +318,7 @@ impl TextLayout { } } - /// Add a trailing cell for selected hard breaks, including a copied separator to `next`. + /// Mark selected hard breaks on nonempty rows, including a copied separator to `next`. pub(super) fn highlight_selection( &self, range: Range, @@ -329,6 +329,9 @@ impl TextLayout { ) { self.highlight(range.clone(), area, buf, start_row); for (screen_row, row) in self.visible_rows(area, start_row) { + if row.source.is_empty() { + continue; + } let Some(end) = row.line_end else { continue }; let selected = range.contains(&end) || (end == self.text.len() diff --git a/codex-rs/tui/src/transcript_view/text_tests.rs b/codex-rs/tui/src/transcript_view/text_tests.rs index af220f891e..6b050c00ba 100644 --- a/codex-rs/tui/src/transcript_view/text_tests.rs +++ b/codex-rs/tui/src/transcript_view/text_tests.rs @@ -113,7 +113,7 @@ fn selection_highlights_wide_graphemes_without_trailing_padding() { #[test] fn selected_newlines_respect_wrapping_and_synthetic_rows() { - for (width, expected) in [(8, vec![(2, 2), (6, 5), (2, 6)]), (4, vec![(2, 2), (2, 7)])] { + for (width, expected) in [(8, vec![(6, 5)]), (4, vec![])] { let layout = TextLayout::new( vec!["".into(), "alpha beta ".into(), "".into(), "界".into()], width,