From c6c7c8d2706981bfc2368bc896e4d07876300c3b Mon Sep 17 00:00:00 2001 From: Benjamin Carlsson Date: Tue, 29 Sep 2026 03:41:36 +0000 Subject: [PATCH] Omit blockquote markers when copying quoted selections in the TUI (#49153) ## Why Copying text selected within a blockquote adds Markdown quote markers that are not part of the selected content. ## What changed Return plain text for selections containing only quoted content, preserving literal `>` characters and selected whitespace. For quoted tables and task lists, remove quote prefixes while retaining table reconstruction and checkbox states. Selections spanning quoted and unquoted content continue to preserve Markdown structure. Preserve the existing logical source when marking list separator lines as omitted from Markdown copying. ## Testing Add coverage for partial and nested blockquote selections, quoted tables and task lists at narrow and wide widths and after rewrapping, and copying a mouse-selected blockquote. GitOrigin-RevId: 5ac0c8c47a77937e5a35c376d8908065217ad2dc --- codex-rs/tui/src/markdown_copy.rs | 49 ++++++ codex-rs/tui/src/markdown_render.rs | 5 +- .../transcript_view/markdown_copy_tests.rs | 17 +- .../markdown_element_copy_tests.rs | 160 +++++++++++++++++- ...eading_whitespace_outside_code_blocks.snap | 5 +- 5 files changed, 227 insertions(+), 9 deletions(-) diff --git a/codex-rs/tui/src/markdown_copy.rs b/codex-rs/tui/src/markdown_copy.rs index ae135e1b75..d2513211a8 100644 --- a/codex-rs/tui/src/markdown_copy.rs +++ b/codex-rs/tui/src/markdown_copy.rs @@ -271,6 +271,55 @@ pub(crate) fn selection(lines: &[SelectedLine], plain: &str) -> (String, CopyFor { return (plain.to_owned(), CopyFormat::PlainText); } + // Quote-only selections omit quote markers. Tables and task lists still need their + // semantic metadata to reconstruct cell fragments and preserve checkbox state. + let mut content = lines + .iter() + .filter(|line| { + !line.range.is_empty() && !line.source.copy.as_ref().is_some_and(|copy| copy.omit) + }) + .peekable(); + let quote_only = content.peek().is_some() + && content.all(|line| { + line.source + .copy + .as_ref() + .is_some_and(|copy| copy.prefix.contains("> ")) + }); + let unquoted; + let lines = if quote_only { + if !lines.iter().any(|line| { + (!line.range.is_empty() || line.source.text.is_empty()) + && line.source.copy.as_ref().is_some_and(|copy| { + !copy.omit + && (copy.table.is_some() + || copy.item_prefix.contains("[x] ") + || copy.item_prefix.contains("[ ] ")) + }) + }) { + return (plain.to_owned(), CopyFormat::PlainText); + } + unquoted = lines + .iter() + .map(|line| { + let mut source = line.source.clone(); + if let Some(copy) = source.copy.as_mut() { + let copy = Arc::make_mut(copy); + copy.prefix = copy.prefix.replace("> ", ""); + copy.continuation = copy.continuation.replace("> ", ""); + copy.item_prefix = copy.item_prefix.replace("> ", ""); + } + SelectedLine { + source, + range: line.range.clone(), + separator: line.separator.clone(), + } + }) + .collect::>(); + &unquoted + } else { + lines + }; let rich = lines.iter().any(|line| { !line.range.is_empty() && line diff --git a/codex-rs/tui/src/markdown_render.rs b/codex-rs/tui/src/markdown_render.rs index 6c129f3b76..c5033f8045 100644 --- a/codex-rs/tui/src/markdown_render.rs +++ b/codex-rs/tui/src/markdown_render.rs @@ -997,8 +997,9 @@ impl<'a, 'policy> Writer<'a, 'policy> { let index = self.text.len(); self.push_blank_line(); if let Some(line) = self.text.get_mut(index) { - let mut source = - crate::terminal_hyperlinks::LogicalLineSource::from_line(&line.line); + let mut source = line.source.clone().unwrap_or_else(|| { + crate::terminal_hyperlinks::LogicalLineSource::from_line(&line.line) + }); let mut copy = crate::markdown_copy::CopyLine::default(); copy.omit = true; source.copy = Some(std::sync::Arc::new(copy)); 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 443afe61d0..6d7d5304a0 100644 --- a/codex-rs/tui/src/transcript_view/markdown_copy_tests.rs +++ b/codex-rs/tui/src/transcript_view/markdown_copy_tests.rs @@ -423,7 +423,7 @@ fn complete_rules_keep_structure_and_partial_rules_keep_selected_text() { let mut copies = Vec::new(); for (source, partial) in [ ("Before\n\n---\n\nAfter", "—"), - ("Before\n\n> ---\n\nAfter", "> —"), + ("Before\n\n> ---\n\nAfter", "—"), ] { let layout = markdown_layout(source, /*width*/ 80); let copied = payload(&layout, 0..layout.text().len()).0; @@ -461,7 +461,11 @@ fn partial_prose_keeps_leading_whitespace_outside_code_blocks() { ] { let layout = markdown_layout(source, /*width*/ 80); let start = layout.text().find("prefix").unwrap() + "prefix".len(); - let copied = payload(&layout, start..layout.text().len()).0; + let (copied, format) = payload(&layout, start..layout.text().len()); + if format == CopyFormat::PlainText { + copies.push(format!("{copied}\n{format:?}")); + continue; + } copies.push(format!( "{copied}\n{}", crate::clipboard_html::render_markdown(&copied) @@ -711,7 +715,14 @@ fn selected_list_and_code_containers_keep_their_structure() { let layout = markdown_layout(source, /*width*/ 80); let start = layout.text().find(start).expect(source); let end = layout.text().rfind(end).expect(source) + end.len(); - let copied = payload(&layout, start..end).0; + let (copied, format) = payload(&layout, start..end); + if source.starts_with('>') { + assert_eq!( + (copied.as_str(), format), + (&layout.text()[start..end], CopyFormat::PlainText) + ); + continue; + } assert_eq!( crate::clipboard_html::render_markdown(&copied), crate::clipboard_html::render_markdown(expected), diff --git a/codex-rs/tui/src/transcript_view/markdown_element_copy_tests.rs b/codex-rs/tui/src/transcript_view/markdown_element_copy_tests.rs index a22d3a8242..76028b0a8d 100644 --- a/codex-rs/tui/src/transcript_view/markdown_element_copy_tests.rs +++ b/codex-rs/tui/src/transcript_view/markdown_element_copy_tests.rs @@ -12,7 +12,7 @@ fn supported_elements_preserve_structure_at_narrow_and_wide_widths() { "Escaped \\*stars\\*, & entity, café 界, and 👩‍💻.", "- first\n- second\n - nested\n\n3. third\n4. fourth", "- [ ] pending\n- [x] complete", - "> quoted **text**\n>\n> - item\n> - nested", + "Before\n\n> quoted **text**\n>\n> - item\n> - nested\n\nAfter", "First paragraph.\n\nHard break: \nnext line.\n\n---\n\nLast paragraph.", "Prose\n\n```\nlet x = \"*literal* \";\n```\n\nAfter", "Image ![**alt** text](https://example.com/image.png).", @@ -73,3 +73,161 @@ fn math_and_mermaid_copy_the_visible_rendering() { } } } + +#[test] +fn blockquote_selection_copies_only_selected_content() { + let mut copies = Vec::new(); + for (source, selected) in [ + ("> quoted text", "quoted text"), + ("> quoted text", "quoted"), + ("> quoted text", "text"), + ("> first line\n> second line", "first line\nsecond line"), + ( + "> first paragraph\n>\n> second paragraph", + "first paragraph\n\nsecond paragraph", + ), + ( + "> outer\n>> nested café界\n>\n> outer again", + "outer\n\nnested café界\n\nouter again", + ), + ("> **bold** and `code`", "bold and code"), + ("> **bold**\n>\n> - [x] task", "bold\n\n"), + ("> **bold**\n>\n> | A |\n> |---|\n> | cell |", "bold\n\n"), + ( + "> literal \\> marker and a > b", + "literal > marker and a > b", + ), + ("Before\n\n> quoted text\n\nAfter", "quoted text"), + ("Before\n\n> quoted text\n\nAfter", "quoted text\n"), + ("Before\n\n> quoted text\n\nAfter", "\nquoted text"), + ( + "> - first\n> continuation\n> - second", + "first\ncontinuation\n\nsecond", + ), + ] { + for width in [18, 80] { + let layout = markdown_layout(source, width); + let start = layout.text().find(selected).expect(source); + let copied = payload(&layout, start..start + selected.len()); + assert_eq!(copied, (selected.into(), CopyFormat::PlainText), "{source}"); + assert_eq!( + payload(&layout.rewrap(/*width*/ 12), start..start + selected.len()), + copied + ); + if width == 80 && copies.len() < 3 { + copies.push(format!("{source}\nSelected {selected:?} -> {copied:?}")); + } + } + } + insta::assert_snapshot!(copies.join("\n"), @r#" + > quoted text + Selected "quoted text" -> ("quoted text", PlainText) + > quoted text + Selected "quoted" -> ("quoted", PlainText) + > quoted text + Selected "text" -> ("text", PlainText) + "#); +} + +#[test] +fn quoted_tables_reconstruct_wrapped_cell_values() { + let table = "| Path |\n|---|\n| `/long/path/to/a/file/with_a_long_name.rs` |"; + for prefix in ["> ", "> > "] { + let source = table + .lines() + .map(|line| format!("{prefix}{line}")) + .collect::>() + .join("\n"); + for width in [18, 80] { + let layout = markdown_layout(&source, width); + let copied = payload(&layout, 0..layout.text().len()); + assert_eq!(copied, (table.into(), CopyFormat::Markdown)); + let start = layout.text().find("/long").unwrap(); + let end = layout.text().rfind(".rs").unwrap() + 3; + let cell = payload(&layout, start..end); + assert_eq!( + cell, + ( + "/long/path/to/a/file/with_a_long_name.rs".into(), + CopyFormat::PlainText + ) + ); + let rewrapped = layout.rewrap(/*width*/ 12); + assert_eq!(payload(&rewrapped, start..end), cell); + assert_eq!(payload(&rewrapped, 0..rewrapped.text().len()), copied); + } + } +} + +#[test] +fn quoted_task_lists_preserve_checkbox_states() { + for (source, expected) in [ + ( + "> - [x] Tests passed\n> - [ ] Deploy", + "- [x] Tests passed\n- [ ] Deploy", + ), + ( + "> > 1. [ ] Tests passed\n> > 2. [x] Deploy", + "1. [ ] Tests passed\n2. [x] Deploy", + ), + ("> - [x] \n> - ordinary", "- [x]\n- ordinary"), + ("> - ordinary\n> - [ ] ", "- ordinary\n- [ ]"), + ] { + for width in [18, 80] { + let layout = markdown_layout(source, width); + let copied = payload(&layout, 0..layout.text().len()); + assert_eq!(copied, (expected.into(), CopyFormat::Markdown)); + let rewrapped = layout.rewrap(/*width*/ 12); + assert_eq!(payload(&rewrapped, 0..rewrapped.text().len()), copied); + if width == 80 && source.starts_with("> - [x] Tests") { + insta::assert_snapshot!(copied.0, @" + - [x] Tests passed + - [ ] Deploy + "); + } + } + } +} + +#[test] +fn mouse_selected_blockquote_copies_without_markers() { + use crossterm::event::KeyModifiers; + use crossterm::event::MouseButton; + use crossterm::event::MouseEvent; + use crossterm::event::MouseEventKind; + + let cells: Vec> = vec![Arc::new(AgentMarkdownCell::new( + "> quoted text".into(), + Path::new("/"), + ))]; + let mut view = TranscriptView::default(); + let area = Rect::new( + /*x*/ 0, /*y*/ 0, /*width*/ 32, /*height*/ 10, + ); + view.render(area, &mut Buffer::empty(area), &cells); + for (kind, column) in [ + (MouseEventKind::Down(MouseButton::Left), 0), + (MouseEventKind::Drag(MouseButton::Left), 31), + ] { + view.handle_mouse( + MouseEvent { + kind, + column, + row: 0, + modifiers: KeyModifiers::NONE, + }, + &cells, + ); + } + let plain = view.selected_text(&cells).unwrap(); + view.copy_selected_text_with( + &cells, + &plain, + /*clear_selection*/ true, + |text, format| { + assert_eq!((text, format), ("quoted text", CopyFormat::PlainText)); + Ok(crate::clipboard_copy::CopyStatus::Confirmed) + }, + ) + .unwrap(); +} diff --git a/codex-rs/tui/src/transcript_view/snapshots/codex_tui__transcript_view__markdown_copy_tests__partial_prose_keeps_leading_whitespace_outside_code_blocks.snap b/codex-rs/tui/src/transcript_view/snapshots/codex_tui__transcript_view__markdown_copy_tests__partial_prose_keeps_leading_whitespace_outside_code_blocks.snap index 720dc00cdd..01da63c642 100644 --- a/codex-rs/tui/src/transcript_view/snapshots/codex_tui__transcript_view__markdown_copy_tests__partial_prose_keeps_leading_whitespace_outside_code_blocks.snap +++ b/codex-rs/tui/src/transcript_view/snapshots/codex_tui__transcript_view__markdown_copy_tests__partial_prose_keeps_leading_whitespace_outside_code_blocks.snap @@ -1,6 +1,5 @@ --- source: tui/src/transcript_view/markdown_copy_tests.rs -assertion_line: 379 expression: "copies.join(\"\\n---\\n\")" --- suffix @@ -15,5 +14,5 @@ expression: "copies.join(\"\\n---\\n\")"

suffix

--- - suffix -

suffix

+ suffix +PlainText