From 2afebab17524e4167996ad5d4cbff6cceec315f6 Mon Sep 17 00:00:00 2001 From: Adam Hitchcock Date: Sat, 10 Oct 2026 06:53:45 -0700 Subject: [PATCH] refactor(terminal): copy-unwrap review follow-ups (#1149) Copy Raw is a keymap action (default cmd-alt-c) instead of a chord hard-coded in the key handler, and `raw` threads through copy_contextual into the one copy path, which now owns the Claude-pane policy. copy_unwrap.rs depends only on alacritty_terminal: the boundary rule splits into a Block that tracks fences and tool output and a pure `sep` with early returns. The fixture tests run through a real Term at the fixture's width instead of a hand-built row model. Co-authored-by: Claude Opus 5.5 --- src/terminal/view.rs | 51 +++++--- src/terminal/view/copy_unwrap.rs | 218 +++++++++++++++---------------- src/ui/app.rs | 6 +- src/ui/keymap.rs | 7 +- src/ui/search/command.rs | 3 +- 5 files changed, 151 insertions(+), 134 deletions(-) diff --git a/src/terminal/view.rs b/src/terminal/view.rs index 8d951866..33239e1c 100644 --- a/src/terminal/view.rs +++ b/src/terminal/view.rs @@ -115,6 +115,7 @@ actions!( terminal, [ CopyText, + CopyRaw, CutText, PasteText, AlternatePaste, @@ -2649,10 +2650,6 @@ impl TerminalView { return; } - if m.platform && m.alt && !m.control && ks.key == "c" && self.copy_raw(cx) { - cx.stop_propagation(); - return; - } if m.platform && !m.control && !m.alt { match self.handle_cmd_shortcut(ks, window, cx) { CmdKey::Consumed => { @@ -2787,7 +2784,7 @@ impl TerminalView { let m = &ks.modifiers; match ks.key.as_str() { "c" => { - if self.copy_contextual(m.control, cx) { + if self.copy_contextual(m.control, false, cx) { CmdKey::Consumed } else { CmdKey::FallThrough @@ -3594,14 +3591,25 @@ impl TerminalView { } } - pub fn copy_selection(&mut self, cx: &mut Context) { - self.copy_selection_as(false, cx); - } - - fn copy_selection_as(&mut self, raw: bool, cx: &mut Context) { - let text = copy_unwrap::selection_text(self, raw, cx); + /// `raw` copies the rows as the grid holds them. Otherwise, in a Claude + /// pane with `copy_join_wrapped` on, the rows Claude wrapped are joined; + /// other programs' output is never joined, since `ls` columns, separators + /// and an editor's own wraps all look like Claude's. + pub fn copy_selection(&mut self, raw: bool, cx: &mut Context) { + let cfg = cx.global::(); + let join = !raw + && cfg.copy_join_wrapped + && self.terminal.foreground_agent() == Some(crate::core::cli_agent::CLIAgent::Claude); + let text = { + let term = self.terminal.term.lock(); + if join { + copy_unwrap::joined(&term) + } else { + term.selection_to_string() + } + }; if let Some(mut text) = text { - if cx.global::().clipboard_trim_trailing_spaces { + if cfg.clipboard_trim_trailing_spaces { text = trim_trailing_spaces(&text); } if !text.is_empty() { @@ -3610,7 +3618,12 @@ impl TerminalView { } } - pub fn copy_contextual(&mut self, clear_on_copy: bool, cx: &mut Context) -> bool { + pub fn copy_contextual( + &mut self, + clear_on_copy: bool, + raw: bool, + cx: &mut Context, + ) -> bool { if self.input_active() { if let Some(text) = self.cmd.selected_text() { cx.write_to_clipboard(ClipboardItem::new_string(text)); @@ -3622,7 +3635,7 @@ impl TerminalView { } } if self.has_selection() { - self.copy_selection(cx); + self.copy_selection(raw, cx); if clear_on_copy { self.terminal.term.lock().selection = None; cx.notify(); @@ -6055,7 +6068,7 @@ impl TerminalView { self.drag_scroll = None; match copy { SelectEndCopy::None => {} - SelectEndCopy::Grid => self.copy_selection(cx), + SelectEndCopy::Grid => self.copy_selection(false, cx), SelectEndCopy::Editor => { if let Some(text) = self.cmd.selected_text() { cx.write_to_clipboard(ClipboardItem::new_string(text)); @@ -7723,7 +7736,13 @@ impl Render for TerminalView { }, )) .on_action(cx.listener(|this, _: &CopyText, _w, cx| { - this.copy_contextual(false, cx); + this.copy_contextual(false, false, cx); + })) + .on_action(cx.listener(|this, _: &CopyRaw, _w, cx| { + // Nothing selected: let the chord reach the key handler. + if !this.copy_contextual(false, true, cx) { + cx.propagate(); + } })) .on_action(cx.listener(|this, _: &CutText, _w, cx| { this.cut_contextual(cx); diff --git a/src/terminal/view/copy_unwrap.rs b/src/terminal/view/copy_unwrap.rs index c7a33dee..5a018682 100644 --- a/src/terminal/view/copy_unwrap.rs +++ b/src/terminal/view/copy_unwrap.rs @@ -7,13 +7,8 @@ use alacritty_terminal::Term; use alacritty_terminal::grid::Dimensions; use alacritty_terminal::index::{Column, Line, Point}; use alacritty_terminal::term::cell::Flags; -use gpui::Context; use unicode_width::UnicodeWidthChar; -use super::TerminalView; -use crate::core::cli_agent::CLIAgent; -use crate::core::config::Config; - /// One line as the copy sees it: the grid's rows up to the next one without /// WRAPLINE, read from column 0 whatever the selection's start. struct Row { @@ -22,29 +17,9 @@ struct Row { end: usize, } -/// What `copy_selection` copies: the selection with Claude's wrapped rows -/// joined, in a Claude pane with `copy_join_wrapped` on and `raw` off. Every -/// other program's output is copied as the grid holds it: `ls` columns, -/// separators and an editor's own wraps all look like Claude's. -pub(super) fn selection_text( - view: &TerminalView, - raw: bool, - cx: &Context, -) -> Option { - let join = !raw - && cx.global::().copy_join_wrapped - && view.terminal.foreground_agent() == Some(CLIAgent::Claude); - let term = view.terminal.term.lock(); - if join { - joined(&term) - } else { - term.selection_to_string() - } -} - /// The selection's text with app-wrapped rows joined. Any doubt (block /// selection, a row count that doesn't line up) copies raw. -fn joined(term: &Term) -> Option { +pub(super) fn joined(term: &Term) -> Option { let raw = term.selection_to_string()?; let Some(range) = term.selection.as_ref().and_then(|s| s.to_range(term)) else { return Some(raw); @@ -58,12 +33,14 @@ fn joined(term: &Term) -> Option { /// `raw` split at its newlines, which must be one per row boundary. fn rejoin(raw: &str, rows: &[Row], cols: usize) -> String { + let mut parts: Vec<&str> = raw.split('\n').collect(); // A Lines selection ends in a newline its rows don't account for. - let (body, tail) = match raw.strip_suffix('\n') { - Some(body) if body.split('\n').count() == rows.len() => (body, "\n"), - _ => (raw, ""), + let tail = if parts.len() == rows.len() + 1 && parts.last() == Some(&"") { + parts.pop(); + "\n" + } else { + "" }; - let parts: Vec<&str> = body.split('\n').collect(); if parts.len() != rows.len() { return raw.to_string(); } @@ -112,62 +89,89 @@ enum Sep { Nothing, } +/// Where the rows before a boundary leave off. +#[derive(Default)] +struct Block { + /// (leading spaces, text column) of the row that opened the current run. + opened: Option<(usize, usize)>, + fenced: bool, + /// The indent of a `⎿` tool result whose rows we are inside. + tool: Option, +} + +impl Block { + fn advance(&mut self, row: &Row) { + let t = row.text.trim(); + if t.starts_with("```") { + self.fenced = !self.fenced; + } + if !t.is_empty() { + let indent = lead(&row.text); + if self.tool.is_some_and(|t| indent <= t) { + self.tool = None; + } + if t.starts_with('⎿') { + self.tool = Some(indent); + } + } + self.opened + .get_or_insert_with(|| (lead(&row.text), text_col(&row.text))); + } +} + /// For each boundary between `rows[i]` and `rows[i + 1]`, what goes there. fn joins(rows: &[Row], cols: usize) -> Vec { - let mut out = Vec::with_capacity(rows.len().saturating_sub(1)); - // (leading spaces, text column) of the row that opened the current run. - let mut opened: Option<(usize, usize)> = None; - let mut fenced = false; - // The indent of a `⎿` tool result whose rows we are inside. - let mut tool: Option = None; - for pair in rows.windows(2) { - let (a, b) = (&pair[0], &pair[1]); - let a_trim = a.text.trim(); - if a_trim.starts_with("```") { - fenced = !fenced; - } - if !a_trim.is_empty() { - if tool.is_some_and(|t| lead(&a.text) <= t) { - tool = None; + let mut block = Block::default(); + rows.windows(2) + .map(|pair| { + block.advance(&pair[0]); + let sep = sep(&pair[0], &pair[1], &block, cols); + if sep == Sep::Newline { + block.opened = None; } - if a_trim.starts_with('⎿') { - tool = Some(lead(&a.text)); - } - } - let (lo, hi) = *opened.get_or_insert_with(|| (lead(&a.text), text_col(&a.text))); - let b_trim = b.text.trim(); - let word = cells(b_trim.split(' ').next().unwrap_or("")); - // Ink breaks one cell early when the next word would land exactly on - // the edge, so a word that just fits still counts as not fitting. - // A word too long for any row tells nothing about where `a` ended. - let full = a.end + 1 + word >= cols && lead(&b.text) + word <= cols; - let join = full - && !fenced - && tool.is_none() - && !b_trim.is_empty() - && b_trim - .chars() - .next() - .is_some_and(|c| c.is_ascii() || c.is_alphabetic()) - && !a_trim.ends_with('…') - && (lo..=hi).contains(&lead(&b.text)) - && text_col(&b.text) == lead(&b.text) - && !structural(&a.text) - && !structural(&b.text); - let wide = |c: Option| c.is_some_and(|c| c.width() == Some(2)); - // A one-word row that fills the row is a word Ink cut in two: its hard - // breaks fill to the edge, its word wraps stop short of it. - let one_word = a.end >= cols && !a.text[text_byte(&a.text)..].trim_end().contains(' '); - out.push(if !join { - opened = None; - Sep::Newline - } else if one_word || (wide(a_trim.chars().last()) && wide(b_trim.chars().next())) { - Sep::Nothing - } else { - Sep::Space - }); + sep + }) + .collect() +} + +fn sep(a: &Row, b: &Row, block: &Block, cols: usize) -> Sep { + if block.fenced || block.tool.is_some() { + return Sep::Newline; + } + let (a_trim, b_trim) = (a.text.trim(), b.text.trim()); + let Some(first) = b_trim.chars().next() else { + return Sep::Newline; + }; + if !(first.is_ascii() || first.is_alphabetic()) + || a_trim.ends_with('…') + || structural(&a.text) + || structural(&b.text) + { + return Sep::Newline; + } + let indent = lead(&b.text); + let continues = block + .opened + .is_some_and(|(lo, hi)| (lo..=hi).contains(&indent)); + if !continues || text_col(&b.text) != indent { + return Sep::Newline; + } + let word = cells(b_trim.split(' ').next().unwrap_or("")); + // Ink breaks one cell early when the next word would land exactly on + // the edge, so a word that just fits still counts as not fitting. + // A word too long for any row tells nothing about where `a` ended. + if a.end + 1 + word < cols || indent + word > cols { + return Sep::Newline; + } + let wide = |c: Option| c.is_some_and(|c| c.width() == Some(2)); + // A one-word row that fills the row is a word Ink cut in two: its hard + // breaks fill to the edge, its word wraps stop short of it. + let one_word = a.end >= cols && !a.text[text_byte(&a.text)..].trim_end().contains(' '); + if one_word || (wide(a_trim.chars().last()) && wide(Some(first))) { + Sep::Nothing + } else { + Sep::Space } - out } fn apply(parts: &[&str], seps: &[Sep]) -> String { @@ -227,27 +231,16 @@ fn text_byte(text: &str) -> usize { } } -/// Box drawing, block elements, a raw table row, or a prompt: never joined. +/// Box drawing, block elements, a raw table row, a prompt or a code fence: +/// never joined. fn structural(text: &str) -> bool { let t = text.trim(); text.chars().any(|c| ('\u{2500}'..='\u{259f}').contains(&c)) - || ["|", "$ ", "> ", "❯ ", "```", "⎿"] + || ["|", "$ ", "> ", "❯ ", "```"] .iter() .any(|p| t.starts_with(p)) } -impl TerminalView { - /// ⌘⌥C and Copy Raw: Copy, with the grid's rows as it holds them. - pub(crate) fn copy_raw(&mut self, cx: &mut Context) -> bool { - let editor_selection = self.input_active() && self.cmd.selected_text().is_some(); - if editor_selection || !self.has_selection() { - return self.copy_contextual(false, cx); - } - self.copy_selection_as(true, cx); - true - } -} - #[cfg(test)] mod tests { use super::*; @@ -256,22 +249,6 @@ mod tests { use alacritty_terminal::index::Side; use alacritty_terminal::selection::{Selection, SelectionType}; - /// Rows the way `tty7 capture --plain` prints them; a capture line longer - /// than the pane was soft-wrapped, so its last row is the remainder. - fn unwrap(text: &str, cols: usize) -> String { - let rows: Vec = text - .split('\n') - .map(|p| { - let w = cells(p.trim_end()); - Row { - text: p.to_string(), - end: if w > cols { (w - 1) % cols + 1 } else { w }, - } - }) - .collect(); - rejoin(text, &rows, cols) - } - fn term(cols: usize, lines: usize, input: &str) -> Term { let mut term = Term::new( alacritty_terminal::term::Config::default(), @@ -294,6 +271,23 @@ mod tests { term.selection = Some(sel); } + /// `text` printed into a `cols`-wide pane and copied whole. A line longer + /// than the pane soft-wraps, as `tty7 capture --plain` saw it. + fn unwrap(text: &str, cols: usize) -> String { + let lines: usize = text + .split('\n') + .map(|l| cells(l).div_ceil(cols).max(1)) + .sum(); + let mut term = term(cols, lines, &text.replace('\n', "\r\n")); + select( + &mut term, + SelectionType::Simple, + (0, 0), + (lines as i32 - 1, cols - 1), + ); + joined(&term).unwrap() + } + const C111: &str = include_str!("fixtures/copy_unwrap_111.txt"); const C172: &str = include_str!("fixtures/copy_unwrap_172.txt"); const C_CODE: &str = include_str!("fixtures/copy_unwrap_code.txt"); diff --git a/src/ui/app.rs b/src/ui/app.rs index e69f1fa4..b9bcfb3c 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -6796,13 +6796,15 @@ impl Tty7App { CopyText => { if let Some(leaf) = self.focused_leaf(window, cx) { leaf.update(cx, |view, cx| { - view.copy_contextual(false, cx); + view.copy_contextual(false, false, cx); }); } } CopyRaw => { if let Some(leaf) = self.focused_leaf(window, cx) { - leaf.update(cx, |view, cx| view.copy_raw(cx)); + leaf.update(cx, |view, cx| { + view.copy_contextual(false, true, cx); + }); } } CutText => { diff --git a/src/ui/keymap.rs b/src/ui/keymap.rs index 08882fe8..070663d4 100644 --- a/src/ui/keymap.rs +++ b/src/ui/keymap.rs @@ -3,7 +3,7 @@ use gpui::{App, Global, KeyBinding, Keystroke, Modifiers}; use crate::core::actions::*; use crate::core::config::{Config, KeybindingOverride}; use crate::terminal::view::{ - AlternatePaste, ClearScrollback, CopyText, FindInTerminal, FindNext, FindPrevious, + AlternatePaste, ClearScrollback, CopyRaw, CopyText, FindInTerminal, FindNext, FindPrevious, InsertNewline, InsertNewlineFallback, PasteText, ToggleComposer, }; use crate::ui::i18n::{L10nKey, t, t_fmt}; @@ -602,6 +602,7 @@ fn shipped_bindings() -> Vec<(&'static str, &'static str)> { per_platform("secondary-i", "secondary-shift-i"), ), ("CopyText", per_platform("", "ctrl-shift-c")), + ("CopyRaw", "cmd-alt-c"), ("PasteText", paste_text_default()), ("AlternatePaste", alternate_paste_default()), ("OpenSettings", "secondary-,"), @@ -1013,6 +1014,7 @@ fn authored_entry(action: &str) -> Option<(CommandGroup, String)> { t(L10nKey::CmdClearScrollback).to_string(), ), "CopyText" => (CommandGroup::Terminal, t(L10nKey::CmdCopy).to_string()), + "CopyRaw" => (CommandGroup::Terminal, t(L10nKey::CmdCopyRaw).to_string()), "PasteText" => (CommandGroup::Terminal, t(L10nKey::CmdPaste).to_string()), "AlternatePaste" => ( CommandGroup::Terminal, @@ -1704,7 +1706,7 @@ fn keystroke_is_valid(s: &str) -> bool { fn action_context(action: &str) -> Option<&'static str> { match action { "FindInTerminal" | "FindNext" | "FindPrevious" | "ClearScrollback" | "InsertNewline" - | "CopyText" | "PasteText" | "ToggleComposer" => Some("Terminal"), + | "CopyText" | "CopyRaw" | "PasteText" | "ToggleComposer" => Some("Terminal"), // `alt_screen` is declared by the pane whenever a full-screen program // owns the grid, so this binding is simply absent there and Ctrl+V // carries on to the PTY as SYN (#677). @@ -1823,6 +1825,7 @@ fn make_binding(action: &str, keystroke: &str) -> Option { "ClearScrollback" => KeyBinding::new(keystroke, ClearScrollback, action_context(action)), "InsertNewline" => KeyBinding::new(keystroke, InsertNewline, action_context(action)), "CopyText" => KeyBinding::new(keystroke, CopyText, action_context(action)), + "CopyRaw" => KeyBinding::new(keystroke, CopyRaw, action_context(action)), "PasteText" => KeyBinding::new(keystroke, PasteText, action_context(action)), "AlternatePaste" => KeyBinding::new(keystroke, AlternatePaste, action_context(action)), "OpenSettings" => KeyBinding::new(keystroke, OpenSettings, None), diff --git a/src/ui/search/command.rs b/src/ui/search/command.rs index 8c565a3d..7139a76e 100644 --- a/src/ui/search/command.rs +++ b/src/ui/search/command.rs @@ -328,7 +328,6 @@ impl CommandKind { |spec: &str| -> Option { cfg!(target_os = "macos").then(|| spec.to_string()) }; match self { CopyText => return inline("secondary-c"), - CopyRaw => return inline("secondary-alt-c"), CutText => return inline("secondary-x"), PasteText => return inline("secondary-v"), SelectAllText => return inline("secondary-a"), @@ -442,8 +441,8 @@ impl CommandKind { OpenBranchPicker => "ScmCheckoutBranch", ToggleDiffViewMode => "ToggleDiffViewMode", QuickOpenFile => "QuickOpenFile", + CopyRaw => "CopyRaw", CopyText - | CopyRaw | CutText | PasteText | SelectAllText