shifathosn
/
gridcraft
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Review fix-up: return copy/cut HTML only when requested 

edit.copy and edit.cut build and return the HTML table only with
{"html": true}; the UI asks for it when the host can publish HTML, so
MCP and control-channel responses are not bloated by up to 4 MB. Tests
cover the default, html:false and html:true.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Brandon Thomas 2026-10-10 12:27:41 -04:00
parent 7712eb1b0e
commit d2cc499c1d
4 changed files with 31 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
crates/engine/src/cmd/edit.rs
View File

@ -40,8 +40,8 @@ pub fn specs() -> Vec<CommandSpec> {
cmd!(noundo "edit.goToSpecial", "Go To Special…", ["Home", "Editing", "Find & Select"], None, "{kind: blanks|constants|formulas|comments|lastCell|currentRegion|visible|conditionalFormats|dataValidation|errors|numbers|text}", has_doc, go_to_special),
cmd!(noundo "edit.undo", "Undo", ["Edit"], Some("Cmd+Z"), "{steps?: 1}", can_undo, undo),
cmd!(noundo "edit.redo", "Redo", ["Edit"], Some("Cmd+Y"), "{steps?: 1}", can_redo, redo),
cmd!(noundo "edit.copy", "Copy", ["Home", "Clipboard"], Some("Cmd+C"), "{range?}", has_doc, copy),
cmd!(noundo "edit.cut", "Cut", ["Home", "Clipboard"], Some("Cmd+X"), "{range?}", has_doc, cut),
cmd!(noundo "edit.copy", "Copy", ["Home", "Clipboard"], Some("Cmd+C"), "{range?, html?: bool} → {range, text, html?} (html: a styled HTML table, only when requested and small enough)", has_doc, copy),
cmd!(noundo "edit.cut", "Cut", ["Home", "Clipboard"], Some("Cmd+X"), "{range?, html?: bool} → {range, text, html?} (html: a styled HTML table, only when requested and small enough)", has_doc, cut),
cmd!(
"edit.paste",
"Paste",
@ -611,7 +611,9 @@ fn copy_impl(s: &mut Session, p: &Json, cut: bool) -> Result<Json> {
range
};
let text = crate::display::range_text(&d.wb, sheet, range);
let html = crate::io::range_html(&d.wb, sheet, range, &text);
// The HTML rendering is for the host clipboard (UI); it can reach 4 MB, so agents and
// scripts get it only when they ask for it.
let html = if p.get("html").and_then(Json::as_bool) == Some(true) { crate::io::range_html(&d.wb, sheet, range, &text) } else { None };
let clip = Clipboard { wb: d.wb.clone(), sheet, range, cut, doc_uid: d.uid, text: text.clone() };
s.clipboard = Some(clip);
let mut result = json!({"range": range.a1(), "text": text});

29
crates/engine/src/tests.rs
View File

@ -212,7 +212,7 @@ fn rich_clipboard_copies_selected_displayed_cells_and_styles() {
s.execute("home.italic", json!({"range": "B2", "on": true})).unwrap();
s.execute("home.fontColor", json!({"range": "B2", "color": "#123456"})).unwrap();
s.execute("home.fillColor", json!({"range": "B2", "color": "#FEDCBA"})).unwrap();
let copied = s.execute("edit.copy", json!({"range": "B2:C3"})).unwrap();
let copied = s.execute("edit.copy", json!({"range": "B2:C3", "html": true})).unwrap();
let html = copied["html"].as_str().expect("copy publishes HTML as well as plain text");
assert_eq!(copied["text"], "12.5%\t\"<tag>&\"\"\nnext\"\n25.0%\tlast\n");
assert!(html.starts_with("<table "));
@ -226,7 +226,7 @@ fn rich_clipboard_copies_selected_displayed_cells_and_styles() {
assert!(!html.contains("outside selection"));
assert!(!html.contains("<html"));
let cut = s.execute("edit.cut", json!({"range": "B2:C3"})).unwrap();
let cut = s.execute("edit.cut", json!({"range": "B2:C3", "html": true})).unwrap();
assert_eq!(cut["html"], copied["html"]);
assert_eq!(cut["text"], copied["text"]);
s.execute("edit.paste", json!({"at": "E5", "text": cut["text"]})).unwrap();
@ -234,6 +234,21 @@ fn rich_clipboard_copies_selected_displayed_cells_and_styles() {
assert_eq!(v(&s, "B2"), Value::Empty);
}
#[test]
fn copy_and_cut_return_html_only_when_requested() {
let mut s = s();
s.execute("cell.set", json!({"cell": "A1", "input": "x"})).unwrap();
for cmd in ["edit.copy", "edit.cut"] {
for params in [json!({"range": "A1"}), json!({"range": "A1", "html": false})] {
let r = s.execute(cmd, params).unwrap();
assert_eq!(r["text"], "x\n");
assert!(r.get("html").is_none(), "{cmd} must not bloat programmatic responses with HTML");
}
let r = s.execute(cmd, json!({"range": "A1", "html": true})).unwrap();
assert!(r["html"].as_str().is_some_and(|h| h.contains(">x</td>")));
}
}
#[test]
fn rich_clipboard_clips_merges_and_skips_hidden_rows() {
let mut s = s();
@ -241,7 +256,7 @@ fn rich_clipboard_clips_merges_and_skips_hidden_rows() {
s.execute("home.mergeCenter", json!({"range": "B2:D5"})).unwrap();
s.execute("home.hideRows", json!({"rows": "3:3"})).unwrap();
s.execute("cell.set", json!({"cell": "E4", "input": "side"})).unwrap();
let copied = s.execute("edit.copy", json!({"range": "B2:C4"})).unwrap();
let copied = s.execute("edit.copy", json!({"range": "B2:C4", "html": true})).unwrap();
let html = copied["html"].as_str().expect("merged copy publishes HTML");
assert_eq!(copied["text"], "merged\t\n\t\n");
assert_eq!(html.matches("<tr>").count(), 2);
@ -251,7 +266,7 @@ fn rich_clipboard_clips_merges_and_skips_hidden_rows() {
// A selection beginning inside a merge must keep its shape without copying
// the original anchor's value from outside the selected rectangle.
let copied = s.execute("edit.copy", json!({"range": "C3:E5"})).unwrap();
let copied = s.execute("edit.copy", json!({"range": "C3:E5", "html": true})).unwrap();
let html = copied["html"].as_str().expect("partially selected merge publishes HTML");
assert_eq!(html.matches("<tr>").count(), 2);
assert_eq!(html.matches("<td ").count(), 3);
@ -264,12 +279,12 @@ fn rich_clipboard_clips_merges_and_skips_hidden_rows() {
fn rich_clipboard_omits_html_for_large_ranges_without_truncating_text() {
let mut s = s();
s.execute("cell.set", json!({"cell": "A10001", "input": "last cell"})).unwrap();
let copied = s.execute("edit.copy", json!({"range": "A1:A10001"})).unwrap();
let copied = s.execute("edit.copy", json!({"range": "A1:A10001", "html": true})).unwrap();
assert!(copied.get("html").is_none(), "oversized HTML must be omitted, not partially copied");
let text = copied["text"].as_str().unwrap();
assert_eq!(text.lines().count(), 10_001);
assert!(text.ends_with("last cell\n"));
let copied = s.execute("edit.copy", json!({"range": "A10001"})).unwrap();
let copied = s.execute("edit.copy", json!({"range": "A10001", "html": true})).unwrap();
assert!(copied["html"].as_str().is_some_and(|html| html.contains("last cell")));
}
@ -278,7 +293,7 @@ fn rich_clipboard_omits_html_when_escaping_exceeds_byte_budget() {
let mut s = s();
let text = "&".repeat(1_000_000);
s.execute("range.setValues", json!({"range": "A1", "values": [[text]]})).unwrap();
let copied = s.execute("edit.copy", json!({"range": "A1"})).unwrap();
let copied = s.execute("edit.copy", json!({"range": "A1", "html": true})).unwrap();
assert!(copied.get("html").is_none(), "an oversized escaped value must not produce partial HTML");
assert_eq!(copied["text"].as_str().unwrap().len(), 1_000_001);
assert!(copied["text"].as_str().unwrap().ends_with("&\n"));

3
crates/ui-egui/src/lib.rs
View File

@ -190,7 +190,8 @@ impl SheetApp {
/// Copy or cut the selected cells to the system clipboard. Engine-only commands keep
/// their existing internal clipboard behavior and do not write to the host clipboard.
pub fn copy_to_clipboard(&mut self, ctx: &egui::Context, command: &str) {
let Ok(result) = self.run(command, json!({})) else { return };
// Only ask the engine for HTML when the host can publish it.
let Ok(result) = self.run(command, json!({"html": self.services.copy_html.is_some()})) else { return };
let Some(text) = result.get("text").and_then(Json::as_str) else { return };
if let Some(html) = result.get("html").and_then(Json::as_str)
&& let Some(copy_html) = &mut self.services.copy_html

2
crates/ui-egui/tests/clipboard.rs
View File

@ -77,6 +77,8 @@ fn programmatic_copy_does_not_write_to_the_system_clipboard() {
..Default::default()
});
let copied = h.state_mut().run("edit.copy", json!({})).unwrap();
assert!(copied.get("html").is_none(), "HTML is only built when requested");
let copied = h.state_mut().run("edit.copy", json!({"html": true})).unwrap();
assert!(copied["html"].is_string());
assert_eq!(*calls.borrow(), 0);
assert!(!h.output().platform_output.commands.iter().any(|c| matches!(c, OutputCommand::CopyText(_))));