From 573e2da84a02618056590dcf39e5d21af5faa1ee Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Tue, 13 Oct 2026 03:59:05 -0700 Subject: [PATCH 01/12] Swatches: dropping an .ase file on the window loads its swatches open_dropped sent every dropped file that is not a document to file.place, so a swatch exchange (.ase) file dropped on the desktop window failed with a "Can't Place the File" alert: "invalid parameters for `file.place`: unsupported or corrupt image". open_dropped now sends .ase files to swatch.load with the dropped path, as Load Swatches in the Swatches panel menu does with the picked path. The web app already sent dropped .ase files to swatch.load. The swatches are added to the active document as Load Swatches adds them, and if the file can't be loaded, the status line shows the error. --- ROADMAP.md | 1 + crates/ui-egui/src/lib.rs | 28 +++++++++++++++++++++++++++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/ROADMAP.md b/ROADMAP.md index 8a0f8207..bf06425b 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -156,6 +156,7 @@ Newest first. - **2026-10-12** · Words in capitals hyphenate like capitalized words (FERRUM COM-MODI), as in InDesign; Hyphenate Capitalized Words off keeps them whole. No readiness percentage change. - **2026-10-11** · `transform.move {copy}`, Edit › Duplicate and Step and Repeat return the copies' ids, so a script can go on with them. No readiness percentage change. - **2026-10-11** · `object.fill` and `object.stroke` refuse a swatch name the document doesn't have (it painted nothing) and suggest the name meant (`Paper` → `[Paper]`). No readiness percentage change. +- **2026-10-13** · Dropping an `.ase` file on the window loads its swatches, as Load Swatches does (the drop failed with "Can't Place the File"). No readiness percentage change. - **2026-10-11 (printing):** `file.printers` and `file.print` use craft-libs/craft-print for CUPS and Windows. DesignCraft retains PDF export, page selection, spreads, marks and bleed; dry runs create no temporary PDF or print job. Windows selects matching or default driver paper and shrinks oversized sheets without enlarging smaller ones; queue duplex defaults are preserved. Headless tests exercise exported bytes, job settings, dry runs and refusals; shared native Windows CI passes, including paused-queue submission and cleanup. PostScript, production print controls and physical output validation remain open. No readiness percentage change. - **2026-10-10 (UI.4–UI.5):** Panel tabs can move into other groups, split the dock, float, resize and return to their saved locations through shared docking transactions. Layouts persist in custom workspaces; UI and control commands share the same validated action path. Shared pane geometry and widget-state theme application retain all five palettes and local fonts. Split panes send keyboard input only to the focused canvas and scope contextual toolbar IDs per pane. No readiness percentage change. diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index fc7a3e63..56858775 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -969,8 +969,12 @@ impl DesignApp { } } - /// A file dropped on the window: documents open, anything else is placed. + /// A file dropped on the window: documents open, the swatches in a swatch exchange (.ase) file + /// are loaded, and anything else is placed. pub fn open_dropped(&mut self, path: &str) -> Result { + if path.to_ascii_lowercase().ends_with(".ase") { + return self.run("swatch.load", json!({"path": path})); + } let cmd = if opens_as_document(path) { "file.open" } else { "file.place" }; self.open_file(cmd, json!({"path": path})) } @@ -1888,6 +1892,28 @@ mod tests { assert_eq!(alert(&app), ("Can't Place the File".to_string(), e)); } + #[test] + fn dropping_an_ase_file_loads_its_swatches() { + let mut source = Session::new(); + source.execute("file.new", &json!({})).unwrap(); + source.execute("swatch.create", &json!({"name": "Dropped Teal", "color": "#108080"})).unwrap(); + let data = source.execute("swatch.save", &json!({"names": ["Dropped Teal"]})).unwrap(); + let bytes = designcraft_engine::cmd::base64_decode(data["base64"].as_str().unwrap()); + for name in ["palette.ase", "PALETTE.ASE"] { + let path = std::env::temp_dir().join(format!("designcraft-ui-{}-{name}", std::process::id())); + std::fs::write(&path, &bytes).unwrap(); + let mut app = DesignApp::new(Session::new(), Services::default()); + app.run("file.new", json!({})).unwrap(); + let r = app.open_dropped(&path.to_string_lossy()); + let _ = std::fs::remove_file(&path); + assert!(app.ui.dialog.is_none(), "{name}: {:?}", app.ui.dialog.as_ref().map(|d| &d.fields)); + assert_eq!(r, Ok(json!({"added": 1, "replaced": 0})), "{name}"); + let doc = &app.session.doc().unwrap().doc; + assert!(doc.swatches.iter().any(|s| s.name == "Dropped Teal"), "{name}"); + assert!(doc.assets.is_empty(), "{name} isn't placed"); + } + } + #[test] fn file_open_failures_show_an_alert() { let path = unreadable_file("open.designcraft"); From d9b13ab702b3753ae626a72173a513d99860f12a Mon Sep 17 00:00:00 2001 From: PiMinusMax <121135695+PiMinusMax@users.noreply.github.com> Date: Tue, 13 Oct 2026 15:09:26 +0200 Subject: [PATCH 02/12] fix: Cmd/Ctrl + the + key zooms the canvas, not the whole window On layouts with a dedicated + key (German) or Shift+= (US), Zoom In's Cmd+= arrives as Key::Plus. No shortcut matched it, so egui's built-in keyboard zoom scaled the whole UI, and the UI-scale preference reset it on the next frame. Plus now counts as = for shortcuts, and egui's keyboard zoom is off since the UI scale is a preference. --- crates/ui-egui/src/lib.rs | 3 +++ crates/ui-egui/src/menus.rs | 49 +++++++++++++++++++++++++++++++++++-- 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index fc7a3e63..56866a5d 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -772,6 +772,9 @@ impl DesignApp { ctx.set_zoom_factor(scale); } if !self.styled { + // The UI scale is a preference (applied above); egui's own Cmd+Plus/Minus/0 would rescale the + // whole window instead of the canvas, only to be reset next frame. + ctx.options_mut(|o| o.zoom_with_keyboard = false); theme::install_fonts(ctx, &self.ui.language); self.fonts_lang = self.ui.language.clone(); self.styled = true; diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 872cb81d..680f0086 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -2172,10 +2172,12 @@ pub fn shortcuts(app: &mut DesignApp, ctx: &egui::Context) { let all = effective_shortcuts(app); for (id, sc) in &all { let id = id.as_str(); + // On layouts with a dedicated + key (German) or Shift+= (US), Cmd+= arrives as Plus. + let plus_for_equals = key == egui::Key::Plus; if let Some((m, k)) = parse_shortcut(sc) - && k == key + && (k == key || (plus_for_equals && k == egui::Key::Equals)) && m.command == modifiers.command - && m.shift == modifiers.shift + && (m.shift == modifiers.shift || (plus_for_equals && !m.shift)) && m.alt == modifiers.alt && (m.command || m.alt || !typing) && !(id == "edit.clear" && typing) @@ -3055,6 +3057,49 @@ mod tests { assert_eq!(app.session.tool_id(), "rectangleFrame"); } + #[test] + fn cmd_plus_zooms_the_canvas_not_the_window() { + // A German keyboard has its own + key, so Cmd+= arrives as Plus; it must zoom the canvas like + // Cmd+- does, not hand the key to egui's whole-window zoom. + let mut app = crate::DesignApp::new(designcraft_engine::Session::new(), crate::Services::default()); + app.session.execute("file.new", &json!({})).unwrap(); + let ctx = egui::Context::default(); + let frame = |app: &mut crate::DesignApp, events: Vec| { + let input = egui::RawInput { + screen_rect: Some(egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(1400.0, 900.0))), + events, + ..Default::default() + }; + let mut out = ctx.run_ui(input, |ui| { + app.logic(&ui.ctx().clone()); + app.ui(ui); + }); + out.textures_delta.clear(); + }; + let key = |k: egui::Key, modifiers: egui::Modifiers| { + vec![ + egui::Event::Key { key: k, physical_key: None, pressed: true, repeat: false, modifiers }, + egui::Event::Key { key: k, physical_key: None, pressed: false, repeat: false, modifiers }, + ] + }; + frame(&mut app, vec![]); + frame(&mut app, vec![]); + let scale = ctx.zoom_factor(); + for (k, m, zooms_in) in [ + (egui::Key::Plus, egui::Modifiers::COMMAND, true), + (egui::Key::Plus, egui::Modifiers::COMMAND | egui::Modifiers::SHIFT, true), + (egui::Key::Equals, egui::Modifiers::COMMAND, true), + (egui::Key::Minus, egui::Modifiers::COMMAND, false), + ] { + let z0 = app.view().unwrap().zoom; + frame(&mut app, key(k, m)); + frame(&mut app, vec![]); + let z = app.view().unwrap().zoom; + assert!(if zooms_in { z > z0 } else { z < z0 }, "{k:?} {m:?}: {z0} -> {z}"); + assert_eq!(ctx.zoom_factor(), scale, "{k:?} {m:?} leaves the UI scale alone"); + } + } + #[test] fn the_quick_apply_shortcut_opens_it_without_running_an_entry() { let mut app = crate::DesignApp::new(designcraft_engine::Session::new(), crate::Services::default()); From 1c82a5b892d59554b025f5616411a48e8174f79e Mon Sep 17 00:00:00 2001 From: PiMinusMax <121135695+PiMinusMax@users.noreply.github.com> Date: Tue, 13 Oct 2026 15:13:18 +0200 Subject: [PATCH 03/12] =?UTF-8?q?File=20=E2=80=BA=20Open=20Recent:=20recen?= =?UTF-8?q?t=20documents=20in=20the=20menu=20and=20on=20the=20start=20scre?= =?UTF-8?q?en=20(#706)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Opening or saving a document moves it to the front of File › Open Recent (no duplicates; Save a Copy and exports don't count). The submenu lists the files by name with the full path as tooltip, then Clear Recent Files; the start screen shows the last eight with their folders. A file that is gone is dimmed and, when chosen, says so in an alert and stays listed (a disk that isn't connected comes back). Preferences › File Handling sets how many are kept (default 20, 0–100). Engine: Prefs.recent_files / recent_files_max (saved in prefs.json), file.openRecent {index}, file.recentFiles, file.clearRecent. UI: app.openRecent; the macOS menu bar rebuilds the submenu when the list changes. --- ROADMAP.md | 3 +- apps/designcraft/src/native_menu.rs | 69 +++++++++++++- crates/engine/src/cmd/file.rs | 141 ++++++++++++++++++++++++++++ crates/engine/src/cmd/prefs.rs | 7 +- crates/engine/src/lib.rs | 27 ++++++ crates/ui-egui/src/chrome.rs | 33 +++++++ crates/ui-egui/src/dialogs.rs | 9 ++ crates/ui-egui/src/i18n.rs | 23 +++++ crates/ui-egui/src/i18n/ar.rs | 5 + crates/ui-egui/src/i18n/it.rs | 5 + crates/ui-egui/src/i18n/ja.rs | 5 + crates/ui-egui/src/i18n/pt_br.rs | 5 + crates/ui-egui/src/i18n/uk.rs | 5 + crates/ui-egui/src/menus.rs | 134 ++++++++++++++++++++++++++ docs/target-app-parity.md | 4 +- 15 files changed, 465 insertions(+), 10 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 8a0f8207..8e0b7a06 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,7 +2,7 @@ **Stage: pre-alpha** · next: alpha, ~0% and ~15–25 h away; beta ~28 points and ~420–750 h beyond that (the core-workflow gate, not the percentage: ready for real work is already ~47%) -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-12 · **Change:** minor (cancelable macOS Dock Quit) · **Target:** Adobe InDesign 2026 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-13 · **Change:** minor (File ▸ Open Recent) · **Target:** Adobe InDesign 2026 DesignCraft aims at full Adobe InDesign parity, and to be better: faster, open (documented JSON format + IDML), scriptable by agents (MCP), and available on the web. @@ -129,6 +129,7 @@ list) · [roadmap.md](docs/roadmap.md) (milestones, plan) · [parity-checklist.m Newest first. +- **2026-10-13** · File ▸ Open Recent lists the documents last opened or saved (newest first, no duplicates; Save a Copy and exports don't count), with Clear Recent Files; the start screen shows the last eight. A file that is gone is dimmed and says so when chosen, and stays listed. Preferences ▸ File Handling sets how many are kept (default 20, 0–100). `file.openRecent {index}`, `file.recentFiles`, `file.clearRecent` (#706). No readiness percentage change. - **2026-10-12** · macOS Dock Quit uses the same unsaved-document prompts as window and menu Quit. Repeated requests keep the current answers; Cancel retains every tab and unsaved recovery checkpoint, and completed Save/Don't Save answers exit through the ordinary event loop. No readiness percentage change. - **2026-10-12** · Orderly native exit writes the latest recovery state after finalizing current quit approvals; incomplete or stale plans keep their documents, and Undo to the saved state clears stale recovery. Unavailable or failed recovery is reported. Cancelable macOS Dock Quit remains open (#544). No readiness percentage change. - **2026-10-12** · Recovered documents keep a checkpoint until saved, closed or reverted, including a second crash before autosave. Restarted document IDs and explicit recovery folders preserve other snapshots; failed republishing retains the original. No readiness percentage change. diff --git a/apps/designcraft/src/native_menu.rs b/apps/designcraft/src/native_menu.rs index f51db197..40e06e40 100644 --- a/apps/designcraft/src/native_menu.rs +++ b/apps/designcraft/src/native_menu.rs @@ -20,6 +20,16 @@ pub struct NativeMenu { _menu: Menu, items: HashMap, last_refresh: f64, + /// File › Open Recent, rebuilt when the list of recent files changes. + recent: Option, +} + +struct RecentMenu { + submenu: Submenu, + /// The files shown, and the native items standing for them (with their trailing separator). + shown: Vec, + ids: Vec, + separator: Option, } /// Accelerator for "Cmd+Shift+]" (modifier-less shortcuts stay in the app so typing works). @@ -35,6 +45,7 @@ impl NativeMenu { let menu = Menu::new(); let mut items = HashMap::new(); let mut counter = 0usize; + let mut recent = None; // Application menu. let app_menu = Submenu::new("DesignCraft", true); // Our own About splash (with the community links) instead of the stock panel. @@ -60,7 +71,7 @@ impl NativeMenu { let _ = menu.append(&app_menu); for (title, entries) in menus::menu_tree() { let sub = Submenu::new(designcraft_ui_egui::i18n::tr(&app.ui.language, title), true); - build(app, &sub, &entries, &mut items, &mut counter); + build(app, &sub, &entries, &mut items, &mut counter, title, &mut recent); let _ = menu.append(&sub); } menu.init_for_nsapp(); @@ -72,7 +83,44 @@ impl NativeMenu { (p.is_null() && accel(sc).is_some()).then(|| id.clone()) }) .collect(); - Self { _menu: menu, items, last_refresh: 0.0 } + let recent = recent.map(|submenu| RecentMenu { submenu, shown: Vec::new(), ids: Vec::new(), separator: None }); + Self { _menu: menu, items, last_refresh: 0.0, recent } + } + + /// Put the current recent files at the top of File › Open Recent when they changed. + fn refresh_recent(&mut self, app: &DesignApp) { + let Some(r) = self.recent.as_mut() else { return }; + if r.shown == app.session.prefs.recent_files { + return; + } + for id in r.ids.drain(..) { + if let Some((_, _, Handle::Plain(item))) = self.items.remove(&id) { + let _ = r.submenu.remove(&item); + } + } + if let Some(sep) = r.separator.take() { + let _ = r.submenu.remove(&sep); + } + let mut position = 0; + for it in menus::recent_items(app) { + match it { + Item::Cmd { label, id, params, .. } => { + let mid = format!("dc-recent{position}"); + let item = MenuItem::with_id(mid.clone(), label, true, None); + let _ = r.submenu.insert(&item, position); + self.items.insert(mid.clone(), (id, params, Handle::Plain(item))); + r.ids.push(mid); + } + Item::Sep => { + let sep = PredefinedMenuItem::separator(); + let _ = r.submenu.insert(&sep, position); + r.separator = Some(sep); + } + Item::Sub(..) => continue, + } + position += 1; + } + r.shown = app.session.prefs.recent_files.clone(); } pub fn poll(&mut self, app: &mut DesignApp, ctx: &egui::Context) { @@ -86,6 +134,7 @@ impl NativeMenu { return; } self.last_refresh = now; + self.refresh_recent(app); for (cmd, params, h) in self.items.values() { let en = menus::native_menu_enabled(app, ctx, cmd); match h { @@ -99,7 +148,15 @@ impl NativeMenu { } } -fn build(app: &DesignApp, parent: &Submenu, entries: &[Item], items: &mut HashMap, counter: &mut usize) { +fn build( + app: &DesignApp, + parent: &Submenu, + entries: &[Item], + items: &mut HashMap, + counter: &mut usize, + path: &str, + recent: &mut Option, +) { for e in entries { match e { Item::Sep => { @@ -107,7 +164,11 @@ fn build(app: &DesignApp, parent: &Submenu, entries: &[Item], items: &mut HashMa } Item::Sub(label, children) => { let sub = Submenu::new(designcraft_ui_egui::i18n::tr(&app.ui.language, label), true); - build(app, &sub, children, items, counter); + let path = format!("{path}/{label}"); + build(app, &sub, children, items, counter, &path, recent); + if path == menus::OPEN_RECENT { + *recent = Some(sub.clone()); + } let _ = parent.append(&sub); } Item::Cmd { label, id, params, shortcut } => { diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index a1919b89..98e3aa9d 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -21,6 +21,16 @@ pub fn specs() -> Vec { "{path} — .designcraft or .idml; the fonts in a `Document Fonts` folder beside it load first → {index, documentFonts: faces loaded, warnings: font files skipped (and, for .idml, package parts missing)}", always, file_open), cmd!(noundo "file.openBytes", "Open Bytes", [], None, "{name, base64} — DesignCraft JSON or an IDML package", always, file_open_bytes), + cmd!(noundo "file.openRecent", "Open Recent File", [], None, + "{index} — reopen entry `index` of File › Open Recent (as file.open); a file that is gone is an error and stays listed", + has_recent, file_open_recent), + cmd!(query "file.recentFiles", "Recent Files", [], None, "{} → [{path, name, exists}] File › Open Recent, most recent first", always, |s, _| { + Ok(Value::Array(s.prefs.recent_files.iter().map(|p| recent_entry(p)).collect())) + }), + cmd!(noundo "file.clearRecent", "Clear Recent Files", ["File", "Open Recent"], None, "{} — empty File › Open Recent", has_recent, |s, _| { + s.prefs.recent_files.clear(); + ok() + }), cmd!(noundo "file.save", "Save", ["File"], Some("Cmd+S"), "{path?}", has_doc, file_save), cmd!(noundo "file.saveAs", "Save As…", ["File"], Some("Cmd+Shift+S"), "{path}", has_doc, file_save), cmd!(noundo "file.saveACopy", "Save a Copy…", ["File"], None, "{path} — writes the document without changing which file it is or its unsaved state", has_doc, |s, p| { @@ -157,7 +167,46 @@ fn file_sample(s: &mut Session, _p: &Value) -> Result { Ok(json!({"index": i})) } +fn has_recent(s: &Session) -> std::result::Result<(), String> { + if s.prefs.recent_files.is_empty() { Err("no recent files".into()) } else { Ok(()) } +} + +/// One File › Open Recent row: the path, its file name and whether it is still there. +fn recent_entry(path: &str) -> Value { + let name = std::path::Path::new(path).file_name().map_or_else(|| path.to_string(), |n| n.to_string_lossy().to_string()); + #[cfg(not(target_arch = "wasm32"))] + let exists = std::path::Path::new(path).is_file(); + #[cfg(target_arch = "wasm32")] + let exists = false; + json!({"path": path, "name": name, "exists": exists}) +} + +fn file_open_recent(s: &mut Session, p: &Value) -> Result { + let index = + p.get("index").and_then(Value::as_u64).and_then(|i| usize::try_from(i).ok()).ok_or_else(|| bad("file.openRecent", "missing `index`"))?; + let path = s + .prefs + .recent_files + .get(index) + .cloned() + .ok_or_else(|| bad("file.openRecent", format!("no recent file {index} (the list has {})", s.prefs.recent_files.len())))?; + #[cfg(not(target_arch = "wasm32"))] + if !std::path::Path::new(&path).is_file() { + return Err(EngineError::Other(format!("{path} can't be found: it was moved, renamed or deleted, or its disk isn't connected"))); + } + file_open(s, &json!({"path": path})) +} + +/// `file.open`: the document opens and moves to the front of File › Open Recent. fn file_open(s: &mut Session, p: &Value) -> Result { + let r = open_path(s, p)?; + if let Some(path) = str_param(p, "path") { + s.prefs.note_recent_file(path); + } + Ok(r) +} + +fn open_path(s: &mut Session, p: &Value) -> Result { let path = str_param(p, "path").ok_or_else(|| bad("file.open", "missing `path`"))?; #[cfg(not(target_arch = "wasm32"))] { @@ -259,6 +308,7 @@ fn file_save(s: &mut Session, p: &Value) -> Result { let recovery_dir = s.recovery_dir.clone(); let index = s.active_index().ok_or(EngineError::NoDocument)?; crate::recovery::discard_document(s, index, recovery_dir.as_deref()); + s.prefs.note_recent_file(&path); } Ok(json!({"path": path, "bytes": bytes.len()})) } @@ -990,3 +1040,94 @@ mod document_fonts_tests { let _ = std::fs::remove_dir_all(&dir); } } + +#[cfg(test)] +mod recent_tests { + use serde_json::json; + + use crate::Session; + + fn temp_dir(name: &str) -> std::path::PathBuf { + let dir = std::env::temp_dir().join(format!("dc-recent-{name}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + dir + } + + fn saved(s: &mut Session, path: &std::path::Path) -> String { + s.execute("file.new", &json!({})).unwrap(); + s.execute("file.saveAs", &json!({"path": path.to_string_lossy()})).unwrap(); + path.to_string_lossy().to_string() + } + + /// Opening and saving put a file at the front once; Save a Copy doesn't count. + #[test] + fn open_and_save_order_the_list_without_duplicates() { + let dir = temp_dir("order"); + let mut s = Session::new(); + let a = saved(&mut s, &dir.join("a.designcraft")); + let b = saved(&mut s, &dir.join("b.designcraft")); + assert_eq!(s.prefs.recent_files, [b.clone(), a.clone()]); + s.execute("file.open", &json!({"path": a})).unwrap(); + assert_eq!(s.prefs.recent_files, [a.clone(), b.clone()], "reopening moves it to the front, once"); + s.execute("file.saveACopy", &json!({"path": dir.join("copy.designcraft").to_string_lossy()})).unwrap(); + assert_eq!(s.prefs.recent_files, [a.clone(), b.clone()], "a copy isn't a recent file"); + let rows = s.execute("file.recentFiles", &json!({})).unwrap(); + assert_eq!(rows[0], json!({"path": a, "name": "a.designcraft", "exists": true})); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn open_recent_reopens_and_a_missing_file_is_an_error_that_keeps_it_listed() { + let dir = temp_dir("open"); + let mut s = Session::new(); + let a = saved(&mut s, &dir.join("a.designcraft")); + let b = saved(&mut s, &dir.join("b.designcraft")); + let r = s.execute("file.openRecent", &json!({"index": 1})).unwrap(); + assert_eq!(s.documents()[r["index"].as_u64().unwrap() as usize].path.as_deref(), Some(a.as_str())); + assert_eq!(s.prefs.recent_files, [a.clone(), b.clone()]); + std::fs::remove_file(&b).unwrap(); + let e = s.execute("file.openRecent", &json!({"index": 1})).unwrap_err(); + assert!(e.to_string().contains("can't be found"), "{e}"); + assert_eq!(s.prefs.recent_files, [a.clone(), b.clone()], "a disk that isn't connected comes back"); + assert_eq!(s.execute("file.recentFiles", &json!({})).unwrap()[1]["exists"], false); + // Hostile indices are errors, never panics. + for index in [json!(2), json!(u64::MAX), json!(-1), json!("0"), json!(null)] { + assert!(s.execute("file.openRecent", &json!({"index": index})).is_err(), "{index}"); + } + s.execute("file.clearRecent", &json!({})).unwrap(); + assert!(s.prefs.recent_files.is_empty()); + assert!(s.execute("file.openRecent", &json!({"index": 0})).is_err(), "disabled when empty"); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn the_list_keeps_at_most_the_preferred_count() { + let mut s = Session::new(); + for i in 0..30 { + s.prefs.note_recent_file(&format!("/work/{i}.designcraft")); + } + assert_eq!(s.prefs.recent_files.len(), 20, "default: 20"); + assert_eq!(s.prefs.recent_files[0], "/work/29.designcraft"); + s.execute("prefs.set", &json!({"recentFilesMax": 3})).unwrap(); + assert_eq!(s.prefs.recent_files, ["/work/29.designcraft", "/work/28.designcraft", "/work/27.designcraft"]); + assert_eq!(s.execute("prefs.set", &json!({"recentFilesMax": 4_000_000_000u64})).unwrap()["recentFilesMax"], crate::MAX_RECENT_FILES); + assert!(s.execute("prefs.set", &json!({"recentFilesMax": -1})).is_err()); + s.execute("prefs.set", &json!({"recentFilesMax": 0})).unwrap(); + s.prefs.note_recent_file("/work/new.designcraft"); + assert!(s.prefs.recent_files.is_empty(), "0 keeps no list"); + } + + /// The list is a preference: it survives the round trip through prefs.json. + #[test] + fn the_list_survives_a_restart() { + let mut s = Session::new(); + s.prefs.note_recent_file("/work/a.designcraft"); + let saved = serde_json::to_vec(&s.prefs).unwrap(); + let back: crate::Prefs = serde_json::from_slice(&saved).unwrap(); + assert_eq!(back.recent_files, ["/work/a.designcraft"]); + assert_eq!(back.recent_files_max, 20); + let old: crate::Prefs = serde_json::from_slice(br#"{"recoveryMinutes": 2.0}"#).unwrap(); + assert!(old.recent_files.is_empty(), "older prefs.json without the list still load"); + } +} diff --git a/crates/engine/src/cmd/prefs.rs b/crates/engine/src/cmd/prefs.rs index 68a5cf15..c74192c8 100644 --- a/crates/engine/src/cmd/prefs.rs +++ b/crates/engine/src/cmd/prefs.rs @@ -48,13 +48,14 @@ pub fn specs() -> Vec { "Preferences", [], None, - "{showHiddenCharacters?, typographersQuotes?, polygonSides?, starInset?, scaleStrokes?, dimensionsIncludeStroke?, transformationsAreTotals?, absolutePageNumbers?, highlightHj?, highlightKeeps?, highlightCustomTracking?, highlightSubstitutedFonts?, richBlackOutput?, favoriteFonts?: [family], showFontNamesInEnglish?} → all application preferences", + "{showHiddenCharacters?, typographersQuotes?, polygonSides?, starInset?, scaleStrokes?, dimensionsIncludeStroke?, transformationsAreTotals?, absolutePageNumbers?, highlightHj?, highlightKeeps?, highlightCustomTracking?, highlightSubstitutedFonts?, richBlackOutput?, favoriteFonts?: [family], showFontNamesInEnglish?, recoveryMinutes?, recentFilesMax?: 0–100} → all application preferences", super::always, |s, p| { let cur = serde_json::to_value(&s.prefs).map_err(|e| bad("prefs.set", e.to_string()))?; let new = merge("prefs.set", cur, p, None)?; - s.prefs = serde_json::from_value(new.clone()).map_err(|e| bad("prefs.set", e.to_string()))?; - Ok(new) + s.prefs = serde_json::from_value(new).map_err(|e| bad("prefs.set", e.to_string()))?; + s.prefs.trim_recent_files(); + serde_json::to_value(&s.prefs).map_err(|e| bad("prefs.set", e.to_string())) } ), cmd!( diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index caeeb80b..05e10ebd 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -201,6 +201,31 @@ pub struct Prefs { pub show_note_anchors: bool, /// Preferences › File Handling: minutes between document recovery saves. pub recovery_minutes: f64, + /// File › Open Recent: documents opened or saved, most recent first. + pub recent_files: Vec, + /// Preferences › File Handling › Number of Recent Items to Display (0 keeps no list). + pub recent_files_max: u32, +} + +/// The most recent files Preferences lets File › Open Recent keep. +pub const MAX_RECENT_FILES: u32 = 100; + +impl Prefs { + /// Move `path` to the front of File › Open Recent (no duplicates, at most `recent_files_max`). + pub fn note_recent_file(&mut self, path: &str) { + if path.is_empty() { + return; + } + self.recent_files.retain(|p| p != path); + self.recent_files.insert(0, path.to_string()); + self.trim_recent_files(); + } + + /// Drop the entries beyond `recent_files_max` (capped at `MAX_RECENT_FILES`). + pub fn trim_recent_files(&mut self) { + self.recent_files_max = self.recent_files_max.min(MAX_RECENT_FILES); + self.recent_files.truncate(self.recent_files_max as usize); + } } /// A starter autocorrect list (common English typing slips). @@ -249,6 +274,8 @@ impl Default for Prefs { show_added_text: true, show_note_anchors: true, recovery_minutes: 0.5, + recent_files: Vec::new(), + recent_files_max: 20, } } } diff --git a/crates/ui-egui/src/chrome.rs b/crates/ui-egui/src/chrome.rs index c2f1cb48..3720821d 100644 --- a/crates/ui-egui/src/chrome.rs +++ b/crates/ui-egui/src/chrome.rs @@ -710,6 +710,7 @@ pub fn start_screen(app: &mut DesignApp, ui: &mut egui::Ui) { let _ = app.run("file.newSample", json!({})); } }); + recent_files(app, ui); ui.add_space(20.0); community_card(app, ui); ui.add_space(24.0); @@ -744,6 +745,38 @@ pub fn start_screen(app: &mut DesignApp, ui: &mut egui::Ui) { let _ = Content::Unassigned; } +/// Recent files shown on the start screen (File › Open Recent has them all). +const START_SCREEN_RECENT: usize = 8; + +/// Start screen: the most recent files (name, then folder); a file that is gone is dimmed and says +/// so when clicked. Nothing when the list is empty. +fn recent_files(app: &mut DesignApp, ui: &mut egui::Ui) { + if app.session.prefs.recent_files.is_empty() { + return; + } + let t = Tokens::get(ui.ctx()); + ui.add_space(24.0); + ui.label(crate::rtl::widget( + ui, + egui::RichText::new(crate::i18n::tr(&app.ui.language, "Recent files")).font(semibold(15.0)).color(t.text_strong), + )); + ui.add_space(6.0); + let rows: Vec<(usize, String)> = app.session.prefs.recent_files.iter().take(START_SCREEN_RECENT).cloned().enumerate().collect(); + for (i, path) in rows { + let p = std::path::Path::new(&path); + let there = p.is_file(); + let folder = p.parent().map(|d| d.display().to_string()).unwrap_or_default(); + let mut job = egui::text::LayoutJob::default(); + let name_color = if there { t.text_strong } else { t.text_dim }; + job.append(&crate::menus::recent_label(&path), 0.0, egui::TextFormat { font_id: semibold(13.0), color: name_color, ..Default::default() }); + job.append(&folder, 10.0, egui::TextFormat { font_id: egui::FontId::proportional(12.0), color: t.text_dim, ..Default::default() }); + let r = ui.add(egui::Button::new(job).frame(false).min_size(vec2(0.0, 24.0))).on_hover_text(&path); + if r.clicked() { + let _ = app.run("app.openRecent", json!({"index": i})); + } + } +} + /// Start screen: the ArtCraft community (Discord first) and project links. fn community_card(app: &mut DesignApp, ui: &mut egui::Ui) { let t = Tokens::get(ui.ctx()); diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 695a66af..9df5f1a5 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -828,6 +828,14 @@ fn preferences(app: &crate::DesignApp, ui: &mut egui::Ui, d: &mut Dialog, max_he d.fields.insert("recoveryMinutes".into(), json!(v)); } }); + ui.add_space(6.0); + ui.horizontal(|ui| { + crate::rtl::label(ui, crate::i18n::tr(&app.ui.language, "Number of Recent Items to Display:")); + let mut v = d.n("recentFilesMax").unwrap_or(20.0); + if ui.add(egui::DragValue::new(&mut v).range(0.0..=f64::from(designcraft_engine::MAX_RECENT_FILES)).speed(0.2)).changed() { + d.fields.insert("recentFilesMax".into(), json!(v.round())); + } + }); } "type" => { crate::rtl::label(ui, egui::RichText::new(crate::i18n::tr(&app.ui.language, "Type Options")).font(semibold(12.0))); @@ -1998,6 +2006,7 @@ pub fn confirm(app: &mut DesignApp) -> Result { json!({"scaleStrokes": d.b("scaleStrokes"), "dimensionsIncludeStroke": d.b("dimensionsIncludeStroke"), "transformationsAreTotals": d.b("transformationsAreTotals"), "absolutePageNumbers": d.b("absolutePageNumbers"), "highlightHj": d.b("highlightHj"), "highlightKeeps": d.b("highlightKeeps"), "highlightCustomTracking": d.b("highlightCustomTracking"), "highlightSubstitutedFonts": d.b("highlightSubstitutedFonts"), "richBlackOutput": d.b("richBlackOutput"), "typographersQuotes": d.b("typographersQuotes"), "showFontNamesInEnglish": d.b("showFontNamesInEnglish"), "smartTextReflow": d.b("smartTextReflow"), "autocorrect": d.b("autocorrect"), "showAddedText": d.b("showAddedText"), "showNoteAnchors": d.b("showNoteAnchors"), "recoveryMinutes": d.n("recoveryMinutes").unwrap_or(0.5), + "recentFilesMax": d.n("recentFilesMax").unwrap_or(20.0).clamp(0.0, f64::from(designcraft_engine::MAX_RECENT_FILES)) as u32, "autocorrectList": d.s("autocorrectText").lines().filter_map(|l| { let (a, b) = l.split_once('→').or_else(|| l.split_once("->"))?; let (a, b) = (a.trim().to_lowercase(), b.trim().to_string()); diff --git a/crates/ui-egui/src/i18n.rs b/crates/ui-egui/src/i18n.rs index 5252c388..7ea22060 100644 --- a/crates/ui-egui/src/i18n.rs +++ b/crates/ui-egui/src/i18n.rs @@ -39,6 +39,29 @@ const TABLE: &[(&str, [&str; 5])] = &[ ("New", ["Neu", "Nouveau", "Nuevo", "新規", "新建"]), ("New Document…", ["Neues Dokument …", "Nouveau document…", "Nuevo documento…", "新規ドキュメント…", "新建文档…"]), ("Open…", ["Öffnen …", "Ouvrir…", "Abrir…", "開く…", "打开…"]), + ( + "Open Recent", + ["Zuletzt verwendete Datei öffnen", "Ouvrir les fichiers récents", "Abrir recientes", "最近使用したファイルを開く", "打开最近的文件"], + ), + ( + "Open Recent File", + ["Zuletzt verwendete Datei öffnen", "Ouvrir un fichier récent", "Abrir archivo reciente", "最近使用したファイルを開く", "打开最近的文件"], + ), + ( + "Clear Recent Files", + ["Liste leeren", "Effacer les fichiers récents", "Borrar archivos recientes", "最近使用したファイルをクリア", "清除最近的文件"], + ), + ("Recent files", ["Zuletzt geöffnet", "Fichiers récents", "Archivos recientes", "最近使用したファイル", "最近的文件"]), + ( + "Number of Recent Items to Display:", + [ + "Anzahl zuletzt verwendeter Dateien:", + "Nombre de fichiers récents à afficher :", + "Número de elementos recientes:", + "最近使用したファイルの表示数:", + "最近项目显示数量：", + ], + ), ("Close", ["Schließen", "Fermer", "Cerrar", "閉じる", "关闭"]), ("Save", ["Speichern", "Enregistrer", "Guardar", "保存", "保存"]), ("Save As…", ["Speichern unter …", "Enregistrer sous…", "Guardar como…", "別名で保存…", "另存为…"]), diff --git a/crates/ui-egui/src/i18n/ar.rs b/crates/ui-egui/src/i18n/ar.rs index a182d09e..f4251a0a 100644 --- a/crates/ui-egui/src/i18n/ar.rs +++ b/crates/ui-egui/src/i18n/ar.rs @@ -965,6 +965,11 @@ pub(super) const TABLE: &[(&str, &str)] = &[ ("Ruler Units", "وحدات المسطرة"), ("Save Swatches for Exchange…", "حفظ عينات الألوان للتبادل…"), ("Save recovery data every:", "حفظ بيانات الاسترداد كل:"), + ("Open Recent", "فتح الملفات الأخيرة"), + ("Open Recent File", "فتح ملف حديث"), + ("Clear Recent Files", "مسح الملفات الأخيرة"), + ("Recent files", "الملفات الأخيرة"), + ("Number of Recent Items to Display:", "عدد العناصر الأخيرة المعروضة:"), ("Saves which bars show and where the panels are.", "يحفظ الأشرطة الظاهرة ومواضع اللوحات."), ("Search:", "البحث:"), ( diff --git a/crates/ui-egui/src/i18n/it.rs b/crates/ui-egui/src/i18n/it.rs index 75ece03f..353a810a 100644 --- a/crates/ui-egui/src/i18n/it.rs +++ b/crates/ui-egui/src/i18n/it.rs @@ -300,6 +300,11 @@ pub(super) const TABLE: &[(&str, &str)] = &[ ("Reverse", "Inverti"), ("Save Swatches for Exchange…", "Salva campioni per lo scambio…"), ("Save recovery data every:", "Salva dati di ripristino ogni:"), + ("Open Recent", "Apri recenti"), + ("Open Recent File", "Apri file recente"), + ("Clear Recent Files", "Cancella file recenti"), + ("Recent files", "File recenti"), + ("Number of Recent Items to Display:", "Numero di elementi recenti da mostrare:"), ("Search:", "Cerca:"), ("Select an object.", "Seleziona un oggetto."), ("Select text or a text frame.", "Seleziona del testo o una cornice di testo."), diff --git a/crates/ui-egui/src/i18n/ja.rs b/crates/ui-egui/src/i18n/ja.rs index 642b4a4c..11f81022 100644 --- a/crates/ui-egui/src/i18n/ja.rs +++ b/crates/ui-egui/src/i18n/ja.rs @@ -302,6 +302,11 @@ pub(super) const TABLE: &[(&str, &str)] = &[ ("Reverse", "反転"), ("Save Swatches for Exchange…", "交換用スウォッチを保存…"), ("Save recovery data every:", "復旧データの保存間隔:"), + ("Open Recent", "最近使用したファイルを開く"), + ("Open Recent File", "最近使用したファイルを開く"), + ("Clear Recent Files", "最近使用したファイルをクリア"), + ("Recent files", "最近使用したファイル"), + ("Number of Recent Items to Display:", "最近使用したファイルの表示数:"), ("Search:", "検索範囲:"), ("Select an object.", "オブジェクトを選択してください。"), ("Select text or a text frame.", "テキストまたはテキストフレームを選択してください。"), diff --git a/crates/ui-egui/src/i18n/pt_br.rs b/crates/ui-egui/src/i18n/pt_br.rs index 982dfb20..3c101fcb 100644 --- a/crates/ui-egui/src/i18n/pt_br.rs +++ b/crates/ui-egui/src/i18n/pt_br.rs @@ -973,6 +973,11 @@ pub(super) const TABLE: &[(&str, &str)] = &[ ("Ruler Units", "Unidades da régua"), ("Save Swatches for Exchange…", "Salvar amostras para intercâmbio…"), ("Save recovery data every:", "Salvar dados de recuperação a cada:"), + ("Open Recent", "Abrir recentes"), + ("Open Recent File", "Abrir arquivo recente"), + ("Clear Recent Files", "Limpar arquivos recentes"), + ("Recent files", "Arquivos recentes"), + ("Number of Recent Items to Display:", "Número de itens recentes a exibir:"), ("Saves which bars show and where the panels are.", "Salva quais barras são exibidas e onde ficam os painéis."), ("Search:", "Pesquisar:"), ("Select an object to color its fill or stroke.", "Selecione um objeto para colorir seu preenchimento ou traçado."), diff --git a/crates/ui-egui/src/i18n/uk.rs b/crates/ui-egui/src/i18n/uk.rs index f3bc8fb3..be930d79 100644 --- a/crates/ui-egui/src/i18n/uk.rs +++ b/crates/ui-egui/src/i18n/uk.rs @@ -303,6 +303,11 @@ pub(super) const TABLE: &[(&str, &str)] = &[ ("Reverse", "Змінити напрямок"), ("Save Swatches for Exchange…", "Зберегти зразки для обміну…"), ("Save recovery data every:", "Зберігати дані відновлення через:"), + ("Open Recent", "Відкрити нещодавні"), + ("Open Recent File", "Відкрити нещодавній файл"), + ("Clear Recent Files", "Очистити нещодавні файли"), + ("Recent files", "Нещодавні файли"), + ("Number of Recent Items to Display:", "Кількість нещодавніх елементів:"), ("Search:", "Пошук:"), ("Select an object.", "Виділіть об’єкт."), ("Select text or a text frame.", "Виділіть текст або текстовий фрейм."), diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 872cb81d..e617d59f 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -8,6 +8,7 @@ use crate::{DesignApp, ScreenMode}; pub const UI_COMMANDS: &[(&str, &str, Option<&str>, &str)] = &[ ("app.newDocumentDialog", "New Document…", Some("Cmd+N"), "{}"), ("app.openDialog", "Open…", Some("Cmd+O"), "{}"), + ("app.openRecent", "Open Recent File", None, "{index} — file.openRecent, with an alert when the file can't be opened"), ("app.placeDialog", "Place…", Some("Cmd+D"), "{}"), ("app.saveDialog", "Save As…", Some("Cmd+Shift+S"), "{}"), ("app.save", "Save", Some("Cmd+S"), "{}"), @@ -205,6 +206,9 @@ pub const MENUS: &[(&str, &[&str])] = &[ &[ "ui:app.newDocumentDialog", "ui:app.openDialog", + ">Open Recent", + "cmd:file.clearRecent", + "<", "-", "cmd:file.close", "ui:app.save", @@ -766,6 +770,16 @@ pub fn run_ui(app: &mut DesignApp, id: &str, p: &Value) -> Option app.pick_and_open("open"), + "app.openRecent" => { + let index = p.get("index").cloned().unwrap_or(Value::Null); + let name = index + .as_u64() + .and_then(|i| usize::try_from(i).ok()) + .and_then(|i| app.session.prefs.recent_files.get(i)) + .map(|path| recent_label(path)) + .unwrap_or_default(); + app.open_file("file.openRecent", json!({"index": index, "name": name})) + } "app.placeDialog" => app.pick_and_open("place"), "app.placeWithOptions" => { // Word/RTF Import Options: style mapping and conflicts before the text lands. @@ -2084,6 +2098,9 @@ fn menu_items(app: &mut DesignApp, ui: &mut egui::Ui, items: &[Item], path: &str let sub = format!("{path}/{name}"); let shown = crate::i18n::tr(&app.ui.language, name).to_owned(); menu_button(ui, crate::rtl::widget(ui, shown), |ui| { + if sub == OPEN_RECENT { + recent_menu_items(app, ui); + } hidden += menu_items(app, ui, children, &sub); }); } @@ -2109,6 +2126,55 @@ fn menu_items(app: &mut DesignApp, ui: &mut egui::Ui, items: &[Item], path: &str hidden } +/// The File › Open Recent submenu, whose files come before its fixed items. +pub const OPEN_RECENT: &str = "File/Open Recent"; + +/// A recent file's menu label: its file name. +pub fn recent_label(path: &str) -> String { + std::path::Path::new(path).file_name().map_or_else(|| path.to_string(), |n| n.to_string_lossy().to_string()) +} + +/// File › Open Recent's files (`app.openRecent {index}`), then a separator; nothing when empty. +pub fn recent_items(app: &DesignApp) -> Vec { + let mut out: Vec = app + .session + .prefs + .recent_files + .iter() + .enumerate() + .map(|(i, path)| Item::Cmd { label: recent_label(path), id: "app.openRecent".into(), params: json!({"index": i}), shortcut: None }) + .collect(); + if !out.is_empty() { + out.push(Item::Sep); + } + out +} + +/// The recent files at the top of File › Open Recent: file names (not translated), the full path +/// as tooltip, dimmed when the file is gone (choosing it then says so). +fn recent_menu_items(app: &mut DesignApp, ui: &mut egui::Ui) { + let dim = crate::theme::Tokens::get(ui.ctx()).text_dim; + for it in recent_items(app) { + match it { + Item::Cmd { label, id, params, .. } => { + let path = params["index"].as_u64().and_then(|i| app.session.prefs.recent_files.get(i as usize)).cloned().unwrap_or_default(); + let mut text = egui::RichText::new(label); + if !std::path::Path::new(&path).is_file() { + text = text.color(dim); + } + if ui.button(crate::rtl::widget(ui, text)).on_hover_text(&path).clicked() { + activate(app, &id, ¶ms); + ui.close(); + } + } + Item::Sep => { + ui.separator(); + } + Item::Sub(..) => {} + } + } +} + /// Every menu item as (menu path, label), for Edit › Menus. pub fn all_menu_items() -> Vec<(String, String)> { fn walk(items: &[Item], path: &str, out: &mut Vec<(String, String)>) { @@ -3604,4 +3670,72 @@ mod tests { let (_, Some((_, _, open))) = frame(500.0, vec![]) else { panic!("test menu remains open") }; assert!(!open, "an open submenu closes when its row becomes fully clipped"); } + + fn start_screen_texts(app: &mut crate::DesignApp) -> Vec { + let ctx = egui::Context::default(); + crate::theme::install_fonts(&ctx, ""); + let mut shapes = Vec::new(); + for _ in 0..3 { + let input = + egui::RawInput { screen_rect: Some(egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(1400.0, 900.0))), ..Default::default() }; + let mut out = ctx.run_ui(input, |ui| { + app.logic(&ui.ctx().clone()); + app.ui(ui); + }); + out.textures_delta.clear(); + shapes = out.shapes; + } + shapes + .iter() + .filter_map(|s| match &s.shape { + egui::Shape::Text(t) => Some(t.galley.text().to_string()), + _ => None, + }) + .collect() + } + + /// File › Open Recent: the files first (`app.openRecent {index}`), then Clear Recent Files. + #[test] + fn open_recent_lists_the_files_then_clear() { + let tree = menu_tree(); + let file = &tree.iter().find(|(m, _)| *m == "File").unwrap().1; + let Some(Item::Sub(name, children)) = file.get(2) else { panic!("Open Recent after Open…: {:?}", file.get(2)) }; + assert_eq!(format!("File/{name}"), OPEN_RECENT); + assert!(matches!(children.as_slice(), [Item::Cmd { id, .. }] if id == "file.clearRecent")); + let mut app = crate::DesignApp::new(designcraft_engine::Session::new(), crate::Services::default()); + assert!(recent_items(&app).is_empty()); + assert!(!menu_enabled(&app, "file.clearRecent")); + app.session.prefs.note_recent_file("/work/a.designcraft"); + app.session.prefs.note_recent_file("/work/b.idml"); + let items = recent_items(&app); + let labels: Vec<_> = items + .iter() + .map(|i| match i { + Item::Cmd { label, id, params, .. } => format!("{label} {id} {params}"), + Item::Sep => "-".into(), + Item::Sub(..) => "sub".into(), + }) + .collect(); + assert_eq!(labels, ["b.idml app.openRecent {\"index\":0}", "a.designcraft app.openRecent {\"index\":1}", "-"]); + assert!(menu_enabled(&app, "file.clearRecent")); + assert!(menu_enabled(&app, "app.openRecent")); + } + + /// A recent file that is gone says so in an alert (and the start screen still lists it). + #[test] + fn open_recent_of_a_missing_file_alerts_and_the_start_screen_lists_recent_files() { + let mut app = crate::DesignApp::new(designcraft_engine::Session::new(), crate::Services::default()); + assert!(!start_screen_texts(&mut app).iter().any(|t| t == "Recent files"), "no section without recent files"); + app.session.prefs.note_recent_file("/nonexistent/dc-recent/gone.designcraft"); + assert!(app.run("app.openRecent", json!({"index": 0})).is_err()); + let d = app.ui.dialog.take().expect("an alert"); + assert_eq!(d.fields["title"], "Can't Open the File"); + assert_eq!(d.fields["file"], "gone.designcraft"); + assert!(d.fields["message"].as_str().unwrap().contains("can't be found")); + assert!(app.run("app.openRecent", json!({"index": 9})).is_err(), "out of range: an error, not a panic"); + app.ui.dialog = None; + let texts = start_screen_texts(&mut app); + assert!(texts.iter().any(|t| t == "Recent files"), "{texts:?}"); + assert!(texts.iter().any(|t| t.starts_with("gone.designcraft")), "{texts:?}"); + } } diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index eb7d7e5c..019b2510 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -1,6 +1,6 @@ # DesignCraft parity with Adobe InDesign -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-12 · **Change:** minor (cancelable macOS Dock Quit) · **Target:** Adobe InDesign 2026 (21.6.0.57, macOS) +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-13 · **Change:** minor (File ▸ Open Recent) · **Target:** Adobe InDesign 2026 (21.6.0.57, macOS) The authoritative assessment of how close DesignCraft is to Adobe InDesign. [ROADMAP.md](../ROADMAP.md) summarizes it; [gaps.md](gaps.md) lists every known shortfall one at a time; the feature-by-feature @@ -220,7 +220,7 @@ table is the one summed. | Output & production | 92% | 55% | 40–70 | PDF/X-4 only (no X-1a/X-3), no PDF/print presets or `.joboptions`, print via craft-print (CUPS/Windows), no trapping; Output Preview panel | | XML & automation | 100% | 35% | (ecosystem dimension) | no ExtendScript/UXP; XML structure depth | | View & navigation | 100% | 65% | 10–20 | Navigator, display badges, Show/Hide extras (content grabber, live corners) | -| Undo, history, saving | 100% | 70% | 5–10 | Undo/Redo name the action; File ▸ Open Recent | +| Undo, history, saving | 100% | 70% | 5–10 | Undo/Redo name the action | | Accessibility | 100% | 50% | 15–25 | tagged PDF exists; PDF/UA validation, reading order depth, screen-reader support in the UI | ## Remaining effort and calibration From 049ff99590d7abf78553e8666c533f8024a59e04 Mon Sep 17 00:00:00 2001 From: PiMinusMax <121135695+PiMinusMax@users.noreply.github.com> Date: Tue, 13 Oct 2026 15:23:33 +0200 Subject: [PATCH 04/12] contributors: add PiMinusMax's display name --- contributors/people.toml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/contributors/people.toml b/contributors/people.toml index 67a8fdf3..827d12ad 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -28,5 +28,8 @@ display_name = "insanityxoff" [people.moppeeer] sync_github_name = true +[people.PiMinusMax] +display_name = "PiMinusMax" + [people.thaynej] real_name = "Jeffrey Thayne" From 0fb3d677cc9aa831841745fb815aa733848dd247 Mon Sep 17 00:00:00 2001 From: PiMinusMax <121135695+PiMinusMax@users.noreply.github.com> Date: Tue, 13 Oct 2026 17:01:57 +0200 Subject: [PATCH 05/12] Start screen: recent files show a picture of the first page (#706) Save and Save a Copy put a picture of the first page in the .designcraft zip as preview.png (320 px on the longer side). The start screen reads only that entry and shows the last eight recent files as cards with the picture and the name, below the community card. Files saved before have a blank page until they are saved again. The package fallback-font test writes its fixture without Save's preview: drawing it before the document's fonts load pulled in a system font that covers the test's characters (Unifont CSUR) ahead of the document's own. --- ROADMAP.md | 2 +- crates/engine/src/cmd/file.rs | 69 +++++++++++++++++++++++++++- crates/engine/src/cmd/mod.rs | 2 +- crates/engine/src/cmd/package.rs | 5 ++- crates/format/src/lib.rs | 42 +++++++++++++++++ crates/ui-egui/src/chrome.rs | 77 +++++++++++++++++++++++++------- crates/ui-egui/src/menus.rs | 2 +- docs/architecture.md | 2 +- 8 files changed, 179 insertions(+), 22 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 8e0b7a06..9cf408a4 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -129,7 +129,7 @@ list) · [roadmap.md](docs/roadmap.md) (milestones, plan) · [parity-checklist.m Newest first. -- **2026-10-13** · File ▸ Open Recent lists the documents last opened or saved (newest first, no duplicates; Save a Copy and exports don't count), with Clear Recent Files; the start screen shows the last eight. A file that is gone is dimmed and says so when chosen, and stays listed. Preferences ▸ File Handling sets how many are kept (default 20, 0–100). `file.openRecent {index}`, `file.recentFiles`, `file.clearRecent` (#706). No readiness percentage change. +- **2026-10-13** · File ▸ Open Recent lists the documents last opened or saved (newest first, no duplicates; Save a Copy and exports don't count), with Clear Recent Files; the start screen shows the last eight, below the community card, as cards with a picture of the first page: saving puts one in the file (`preview.png`), so the start screen reads it without opening the document. A file that is gone is dimmed and says so when chosen, and stays listed. Preferences ▸ File Handling sets how many are kept (default 20, 0–100). `file.openRecent {index}`, `file.recentFiles`, `file.clearRecent` (#706). No readiness percentage change. - **2026-10-12** · macOS Dock Quit uses the same unsaved-document prompts as window and menu Quit. Repeated requests keep the current answers; Cancel retains every tab and unsaved recovery checkpoint, and completed Save/Don't Save answers exit through the ordinary event loop. No readiness percentage change. - **2026-10-12** · Orderly native exit writes the latest recovery state after finalizing current quit approvals; incomplete or stale plans keep their documents, and Undo to the saved state clears stale recovery. Unavailable or failed recovery is reported. Cancelable macOS Dock Quit remains open (#544). No readiness percentage change. - **2026-10-12** · Recovered documents keep a checkpoint until saved, closed or reverted, including a second crash before autosave. Restarted document IDs and explicit recovery folders preserve other snapshots; failed republishing retains the original. No readiness percentage change. diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 98e3aa9d..4c9e7e36 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -37,7 +37,7 @@ pub fn specs() -> Vec { let path = str_param(p, "path").ok_or_else(|| bad("file.saveACopy", "missing `path`"))?.to_string(); let mut copy = (*s.doc()?.doc).clone(); super::datamerge::refresh_relative_paths(&mut copy, std::path::Path::new(&path)); - let bytes = to_bytes(©); + let bytes = to_bytes_with_preview(©, &s.cache); #[cfg(not(target_arch = "wasm32"))] crate::atomic::write_file(&path, &bytes)?; Ok(json!({"path": path, "bytes": bytes.len()})) @@ -123,6 +123,22 @@ pub fn to_bytes(d: &Document) -> Vec { designcraft_format::save(d).unwrap_or_default() } +/// The longer side of the first-page picture saved in the file, in pixels. +const PREVIEW_PX: f64 = 320.0; + +/// [`to_bytes`] with a picture of the first page, which the start screen shows next to the file +/// in its recent files. A page that can't be drawn just leaves the picture out. +fn to_bytes_with_preview(d: &Document, cache: &crate::Cache) -> Vec { + let preview = d.page_loc(0).and_then(|(si, pi)| { + let b = d.spreads.get(si)?.pages.get(pi)?.bounds(); + let side = b.width().max(b.height()); + (side.is_finite() && side > 0.0).then_some(PREVIEW_PX / side) + }); + let png = + preview.and_then(|scale| designcraft_render::Renderer::new().render_page(d, cache, 0, scale, false, &Default::default())).map(|r| r.to_png()); + designcraft_format::save_with_preview(d, png.as_deref()).unwrap_or_default() +} + pub fn from_bytes(b: &[u8]) -> Result { designcraft_format::load(b).map_err(|e| EngineError::Other(e.to_string())) } @@ -172,6 +188,31 @@ fn has_recent(s: &Session) -> std::result::Result<(), String> { } /// One File › Open Recent row: the path, its file name and whether it is still there. +/// A recent file's first-page picture as straight RGBA, for the start screen. +pub struct RecentPreview { + pub width: u32, + pub height: u32, + pub rgba: Vec, +} + +/// The picture a `.designcraft` file saved of its first page; None when the file is gone, isn't +/// ours, or was saved before files had one (it gets one when saved again). +pub fn recent_preview(path: &str) -> Option { + #[cfg(not(target_arch = "wasm32"))] + { + let file = std::fs::File::open(path).ok()?; + let png = designcraft_format::read_preview(std::io::BufReader::new(file))?; + let img = designcraft_images::decode_rgba(&png)?; + let (width, height) = img.dimensions(); + Some(RecentPreview { width, height, rgba: img.into_raw() }) + } + #[cfg(target_arch = "wasm32")] + { + let _ = path; + None + } +} + fn recent_entry(path: &str) -> Value { let name = std::path::Path::new(path).file_name().map_or_else(|| path.to_string(), |n| n.to_string_lossy().to_string()); #[cfg(not(target_arch = "wasm32"))] @@ -293,9 +334,10 @@ fn file_save(s: &mut Session, p: &Value) -> Result { super::datamerge::refresh_relative_paths(&mut saved, std::path::Path::new(&path)); let paths_changed = saved.data_merge != st.doc.data_merge; let saved = if paths_changed { Arc::new(saved) } else { st.doc.clone() }; - let bytes = to_bytes(&saved); + let bytes = to_bytes_with_preview(&saved, &s.cache); #[cfg(not(target_arch = "wasm32"))] crate::atomic::write_file(&path, &bytes)?; + let st = s.doc_mut()?; if paths_changed { st.doc = saved; st.revision = st.revision.saturating_add(1); @@ -1101,6 +1143,29 @@ mod recent_tests { let _ = std::fs::remove_dir_all(&dir); } + /// Saving puts a small picture of the first page in the file, which the start screen reads + /// without opening the document. + #[test] + fn saved_files_carry_a_first_page_preview() { + let dir = temp_dir("preview"); + let mut s = Session::new(); + let a = saved(&mut s, &dir.join("a.designcraft")); + let p = super::recent_preview(&a).expect("a preview"); + assert_eq!(p.width.max(p.height), super::PREVIEW_PX as u32); + assert_eq!(p.rgba.len(), (p.width * p.height * 4) as usize); + assert!(p.rgba.as_chunks::<4>().0.iter().any(|px| px[3] == 255), "the page is drawn, not left transparent"); + let copy = dir.join("copy.designcraft"); + s.execute("file.saveACopy", &json!({"path": copy.to_string_lossy()})).unwrap(); + assert!(super::recent_preview(©.to_string_lossy()).is_some(), "Save a Copy has one too"); + // Files without one (saved before previews), missing files and other files have none. + let old = dir.join("old.designcraft"); + std::fs::write(&old, super::to_bytes(&s.doc().unwrap().doc)).unwrap(); + for path in [old, dir.join("gone.designcraft"), dir.clone()] { + assert!(super::recent_preview(&path.to_string_lossy()).is_none(), "{path:?}"); + } + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn the_list_keeps_at_most_the_preferred_count() { let mut s = Session::new(); diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index 31e4544f..80fc298f 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -310,5 +310,5 @@ pub fn file_from(b: &[u8]) -> Result { } pub(crate) use datamerge::sync_placeholders; pub(crate) use file::load_document_fonts; -pub use file::{base64_decode, base64_encode, from_bytes, to_bytes}; +pub use file::{RecentPreview, base64_decode, base64_encode, from_bytes, recent_preview, to_bytes}; pub(crate) use place_text::autoflow as place_text_autoflow; diff --git a/crates/engine/src/cmd/package.rs b/crates/engine/src/cmd/package.rs index 25f09537..8587db47 100644 --- a/crates/engine/src/cmd/package.rs +++ b/crates/engine/src/cmd/package.rs @@ -590,7 +590,10 @@ mod tests { s.execute("text.select", &json!({"story": r["story"], "anchor": 0, "focus": text.len()})).unwrap(); s.execute("type.char", &json!({"fontFamily": NAMED, "fontStyle": "Regular"})).unwrap(); let src = dir.join("src").join("Fallback.designcraft"); - s.execute("file.save", &json!({"path": src.to_string_lossy()})).unwrap(); + // Written without File › Save's preview: drawing it before the document's fonts load would + // pull in a system font that covers these characters (Unifont CSUR does) and draws them + // ahead of the document's own fonts in this process. + std::fs::write(&src, crate::cmd::to_bytes(&s.doc().unwrap().doc)).unwrap(); let mut s = Session::new(); assert_eq!(s.execute("file.open", &json!({"path": src.to_string_lossy()})).unwrap()["documentFonts"], 3); diff --git a/crates/format/src/lib.rs b/crates/format/src/lib.rs index 408bb3cc..149945ef 100644 --- a/crates/format/src/lib.rs +++ b/crates/format/src/lib.rs @@ -5,6 +5,7 @@ //! - `assets/.`: embedded placed files, byte-for-byte: one per distinct file, and only //! the ones the document references (see [`Document::compact_assets`]) //! - `meta.json`: format version and generator +//! - `preview.png` (optional): a small picture of the first page, for the start screen's recent files //! //! Older single-file JSON documents (assets as base64 in `assetData`) still open. #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] @@ -46,6 +47,11 @@ fn ext_for(mime: &str) -> &'static str { /// Serialize a document to `.designcraft` bytes. The file holds each distinct placed file once /// and leaves out assets nothing references; `doc` itself keeps them (undo can bring them back). pub fn save(doc: &Document) -> Result, FormatError> { + save_with_preview(doc, None) +} + +/// [`save`], plus `preview` (PNG bytes) as `preview.png`. +pub fn save_with_preview(doc: &Document, preview: Option<&[u8]>) -> Result, FormatError> { let mut compact = doc.clone(); compact.compact_assets(); let doc = &compact; @@ -60,6 +66,10 @@ pub fn save(doc: &Document) -> Result, FormatError> { z.start_file("meta.json", deflate).map_err(io)?; let meta = json!({"format": "designcraft", "version": VERSION, "generator": concat!("DesignCraft ", env!("CARGO_PKG_VERSION"))}); z.write_all(meta.to_string().as_bytes()).map_err(|e| FormatError::Io(e.to_string()))?; + if let Some(png) = preview { + z.start_file(PREVIEW, stored).map_err(io)?; + z.write_all(png).map_err(|e| FormatError::Io(e.to_string()))?; + } z.start_file("document.json", deflate).map_err(io)?; let json = serde_json::to_vec_pretty(doc).map_err(|e| FormatError::Io(e.to_string()))?; z.write_all(&json).map_err(|e| FormatError::Io(e.to_string()))?; @@ -77,6 +87,25 @@ pub fn save(doc: &Document) -> Result, FormatError> { Ok(buf.into_inner()) } +/// The zip entry holding the first page's picture. +pub const PREVIEW: &str = "preview.png"; + +/// The PNG bytes of a `.designcraft` file's preview, reading only the zip's directory and that +/// entry. None for files without one (saved before previews, or legacy JSON) or that aren't ours. +pub fn read_preview(file: R) -> Option> { + let mut z = zip::ZipArchive::new(file).ok()?; + let mut f = z.by_name(PREVIEW).ok()?; + // A preview is small; a huge entry is not one we wrote. + if f.size() > MAX_PREVIEW_BYTES { + return None; + } + let mut v = Vec::new(); + f.read_to_end(&mut v).ok()?; + Some(v) +} + +const MAX_PREVIEW_BYTES: u64 = 4 << 20; + /// Read `.designcraft` bytes (zip, or the legacy single JSON file). pub fn load(bytes: &[u8]) -> Result { if bytes.starts_with(b"PK") { @@ -406,6 +435,19 @@ mod tests { assert!(load(&serde_json::to_vec(&v).unwrap()).unwrap().settings.glyph_fallback); } + #[test] + fn the_preview_is_read_back_without_the_document() { + let d = Document::new(&NewDocument::default()); + let png = b"\x89PNG fake".to_vec(); + let bytes = save_with_preview(&d, Some(&png)).unwrap(); + assert_eq!(read_preview(Cursor::new(&bytes)), Some(png)); + assert!(load(&bytes).is_ok(), "the preview doesn't get in the way of opening"); + // Files without one, legacy JSON and garbage have none. + assert_eq!(read_preview(Cursor::new(save(&d).unwrap())), None); + assert_eq!(read_preview(Cursor::new(b"{}".to_vec())), None); + assert_eq!(read_preview(Cursor::new(Vec::new())), None); + } + #[test] fn rejects_garbage_and_newer_versions() { assert!(load(b"nope").is_err()); diff --git a/crates/ui-egui/src/chrome.rs b/crates/ui-egui/src/chrome.rs index 3720821d..3ce1dbef 100644 --- a/crates/ui-egui/src/chrome.rs +++ b/crates/ui-egui/src/chrome.rs @@ -710,9 +710,9 @@ pub fn start_screen(app: &mut DesignApp, ui: &mut egui::Ui) { let _ = app.run("file.newSample", json!({})); } }); - recent_files(app, ui); ui.add_space(20.0); community_card(app, ui); + recent_files(app, ui); ui.add_space(24.0); ui.label(crate::rtl::widget( ui, @@ -748,8 +748,9 @@ pub fn start_screen(app: &mut DesignApp, ui: &mut egui::Ui) { /// Recent files shown on the start screen (File › Open Recent has them all). const START_SCREEN_RECENT: usize = 8; -/// Start screen: the most recent files (name, then folder); a file that is gone is dimmed and says -/// so when clicked. Nothing when the list is empty. +/// Start screen: the most recent files as cards with the first page's picture (saved in the file) +/// and the name; the full path is the tooltip. A file that is gone is dimmed and says so when +/// clicked. Nothing when the list is empty. fn recent_files(app: &mut DesignApp, ui: &mut egui::Ui) { if app.session.prefs.recent_files.is_empty() { return; @@ -760,23 +761,69 @@ fn recent_files(app: &mut DesignApp, ui: &mut egui::Ui) { ui, egui::RichText::new(crate::i18n::tr(&app.ui.language, "Recent files")).font(semibold(15.0)).color(t.text_strong), )); - ui.add_space(6.0); + ui.add_space(8.0); let rows: Vec<(usize, String)> = app.session.prefs.recent_files.iter().take(START_SCREEN_RECENT).cloned().enumerate().collect(); - for (i, path) in rows { - let p = std::path::Path::new(&path); - let there = p.is_file(); - let folder = p.parent().map(|d| d.display().to_string()).unwrap_or_default(); - let mut job = egui::text::LayoutJob::default(); - let name_color = if there { t.text_strong } else { t.text_dim }; - job.append(&crate::menus::recent_label(&path), 0.0, egui::TextFormat { font_id: semibold(13.0), color: name_color, ..Default::default() }); - job.append(&folder, 10.0, egui::TextFormat { font_id: egui::FontId::proportional(12.0), color: t.text_dim, ..Default::default() }); - let r = ui.add(egui::Button::new(job).frame(false).min_size(vec2(0.0, 24.0))).on_hover_text(&path); - if r.clicked() { - let _ = app.run("app.openRecent", json!({"index": i})); + let mut open = None; + ui.horizontal_wrapped(|ui| { + for (i, path) in rows { + let modified = std::fs::metadata(&path).ok().filter(|m| m.is_file()).map(|m| m.modified().ok()); + let there = modified.is_some(); + let (cr, resp) = ui.allocate_exact_size(vec2(140.0, 170.0), Sense::click()); + let resp = resp.on_hover_text(&path); + ui.painter().rect_filled(cr, 6.0, if resp.hovered() { t.hover } else { t.panel }); + let area = egui::Rect::from_min_size(cr.min + vec2(20.0, 12.0), vec2(100.0, 108.0)); + match modified.and_then(|m| recent_preview_texture(ui.ctx(), &path, m)) { + Some(tex) => { + let size = tex.size_vec2(); + let k = (area.width() / size.x).min(area.height() / size.y); + let pr = egui::Rect::from_center_size(area.center(), size * k); + ui.painter().image(tex.id(), pr, egui::Rect::from_min_max(egui::pos2(0.0, 0.0), egui::pos2(1.0, 1.0)), Color32::WHITE); + ui.painter().rect_stroke(pr, 0.0, Stroke::new(1.0, t.border), egui::StrokeKind::Outside); + } + // No picture (saved before files had one) or the file is gone: a blank page. + None => { + let pr = egui::Rect::from_center_size(area.center(), vec2(62.0, 80.0)); + ui.painter().rect_filled(pr, 0.0, if there { Color32::from_gray(245) } else { Color32::from_gray(110) }); + } + } + let color = if there { t.text_strong } else { t.text_dim }; + // The name without `.designcraft`, on up to two lines (broken after a space or dash). + let label = crate::menus::recent_label(&path); + let name = label.strip_suffix(".designcraft").unwrap_or(&label).to_string(); + let mut job = egui::text::LayoutJob::simple(name, semibold(12.0), color, cr.width() - 12.0); + job.wrap.max_rows = 2; + job.halign = egui::Align::Center; + let galley = ui.painter().layout_job(job); + ui.painter().galley(egui::pos2(cr.center().x, area.max.y + 8.0), galley, color); + if resp.clicked() { + open = Some(i); + } } + }); + if let Some(i) = open { + let _ = app.run("app.openRecent", json!({"index": i})); } } +/// The texture of a recent file's saved first-page picture, read again when the file changes. +fn recent_preview_texture(ctx: &egui::Context, path: &str, modified: Option) -> Option { + type Cached = (Option, Option); + let key = egui::Id::new(("recent_preview", path)); + if let Some((m, tex)) = ctx.data(|d| d.get_temp::(key)) + && m == modified + { + return tex; + } + let tex = designcraft_engine::cmd::recent_preview(path).and_then(|p| { + let size = [usize::try_from(p.width).ok()?, usize::try_from(p.height).ok()?]; + (size[0].checked_mul(size[1])?.checked_mul(4)? == p.rgba.len()).then(|| { + ctx.load_texture(format!("recent_preview:{path}"), egui::ColorImage::from_rgba_unmultiplied(size, &p.rgba), egui::TextureOptions::LINEAR) + }) + }); + ctx.data_mut(|d| d.insert_temp::(key, (modified, tex.clone()))); + tex +} + /// Start screen: the ArtCraft community (Discord first) and project links. fn community_card(app: &mut DesignApp, ui: &mut egui::Ui) { let t = Tokens::get(ui.ctx()); diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index e617d59f..b9744f4b 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -3736,6 +3736,6 @@ mod tests { app.ui.dialog = None; let texts = start_screen_texts(&mut app); assert!(texts.iter().any(|t| t == "Recent files"), "{texts:?}"); - assert!(texts.iter().any(|t| t.starts_with("gone.designcraft")), "{texts:?}"); + assert!(texts.iter().any(|t| t == "gone"), "the card names it without `.designcraft`: {texts:?}"); } } diff --git a/docs/architecture.md b/docs/architecture.md index 39a9039d..38e46ca7 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -25,7 +25,7 @@ lines of Rust, 1,122 tests (2026-10-10). Pure Rust, no C dependencies by default | L3 | `idml` | IDML import and export (designmap, spreads, parents, stories, styles, graphics, preferences; CJK and Arabic attributes) | 8.0k | | L3 | `epub` | EPUB 3 reflowable and fixed-layout export | 0.9k | | L3 | `textimport` | Word (.docx), RTF, Excel (.xlsx), Tagged Text import; text export | 1.8k | -| L3 | `format` | the native `.designcraft` zip (`document.json`, `assets/`, `meta.json`), versioned | 0.3k | +| L3 | `format` | the native `.designcraft` zip (`document.json`, `assets/`, `meta.json`, optional `preview.png`), versioned | 0.3k | | L4 | `tools` | tool state machines: pointer events and modifiers to commands and overlays; snapping and smart guides | 5.9k | | L5 | `engine` | `Session`: documents, history, selection, the command registry (446 commands in `cmd/`), compose cache, preflight, find/change, scripts, recovery, links | 29.3k | | L6 | `ui-egui` | the InDesign-style egui frontend: menus, Control panel, Tools, dock and panels, dialogs, canvas, theme tokens, i18n, the control channel | 33.4k | From 68596ea69613a619351594c4d07ee2efed10c118 Mon Sep 17 00:00:00 2001 From: Gert Pellin Date: Tue, 13 Oct 2026 17:25:27 +0200 Subject: [PATCH 06/12] Refuse command params the description doesn't name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Callers from outside the app now get a parameter error for a key a command's params description doesn't name, instead of the command running with its defaults: `text.select {story, start, end}` and `style.list {bogus: 1}` are refused with "unknown parameter `…`" and the nearest or the accepted keys. - engine: `cmd::params` (ported from VectorCraft #1442) reads the keys from each params description; `check_params` / `check_keys`. `Session::execute` itself stays lenient so old journals replay; in the engine's own tests it checks too, as the audit of the descriptions. - MCP: tools/call answers -32602 for an argument a tool doesn't take and for params a command doesn't take (`execute`, every `batch` step before the first runs); the headless backend checks as well. - Control channel (`engine.execute`) and `designcraft-cli run --cmd`. - Descriptions now name every key their commands read (table target cells, anchored objects, gap.move, file.place, data merge, prefs.set, …); type.char / type.para stay unchecked (their attributes may be the params object itself; the setters refuse unknown attributes). - Tests that passed keys the commands never read are fixed, and the pen tool no longer sends the unread `notifyPen`. Fixes #776 --- ROADMAP.md | 1 + apps/designcraft-cli/src/main.rs | 2 + apps/designcraft-cli/tests/cli.rs | 16 ++ crates/engine/src/cmd/anchored.rs | 4 +- crates/engine/src/cmd/datamerge/mod.rs | 4 +- crates/engine/src/cmd/export.rs | 2 +- crates/engine/src/cmd/file.rs | 4 +- crates/engine/src/cmd/find.rs | 4 +- crates/engine/src/cmd/mathexpr.rs | 2 +- crates/engine/src/cmd/mod.rs | 1 + crates/engine/src/cmd/object.rs | 8 +- crates/engine/src/cmd/params.rs | 230 +++++++++++++++++++++++++ crates/engine/src/cmd/prefs.rs | 2 +- crates/engine/src/cmd/style.rs | 23 ++- crates/engine/src/cmd/table.rs | 202 ++++++++++++++++++---- crates/engine/src/cmd/text.rs | 4 +- crates/engine/src/cmd/xref.rs | 2 +- crates/engine/src/lib.rs | 7 + crates/engine/src/tests_params.rs | 106 ++++++++++++ crates/engine/tests/param_keys.rs | 14 ++ crates/mcp/src/headless.rs | 2 + crates/mcp/src/server.rs | 39 +++++ crates/mcp/src/tests.rs | 38 ++++ crates/tools/src/pen.rs | 2 +- crates/ui-egui/src/control.rs | 4 + crates/ui-egui/src/dialogs.rs | 3 +- docs/control-protocol.md | 2 +- docs/mcp.md | 2 +- 28 files changed, 665 insertions(+), 65 deletions(-) create mode 100644 crates/engine/src/cmd/params.rs create mode 100644 crates/engine/src/tests_params.rs create mode 100644 crates/engine/tests/param_keys.rs diff --git a/ROADMAP.md b/ROADMAP.md index 8a0f8207..2b2fe338 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -129,6 +129,7 @@ list) · [roadmap.md](docs/roadmap.md) (milestones, plan) · [parity-checklist.m Newest first. +- **2026-10-13** · Commands refuse parameter keys their params description doesn't name (#776): the control channel, MCP (`-32602`, every `batch` step checked before the first runs) and `designcraft-cli --cmd` answer with an “unknown parameter” error naming the nearest or the accepted keys instead of running with defaults; journals still replay. Descriptions of the table, anchored-object, data-merge and placing commands now name every key they read. No readiness percentage change. - **2026-10-12** · macOS Dock Quit uses the same unsaved-document prompts as window and menu Quit. Repeated requests keep the current answers; Cancel retains every tab and unsaved recovery checkpoint, and completed Save/Don't Save answers exit through the ordinary event loop. No readiness percentage change. - **2026-10-12** · Orderly native exit writes the latest recovery state after finalizing current quit approvals; incomplete or stale plans keep their documents, and Undo to the saved state clears stale recovery. Unavailable or failed recovery is reported. Cancelable macOS Dock Quit remains open (#544). No readiness percentage change. - **2026-10-12** · Recovered documents keep a checkpoint until saved, closed or reverted, including a second crash before autosave. Restarted document IDs and explicit recovery folders preserve other snapshots; failed republishing retains the original. No readiness percentage change. diff --git a/apps/designcraft-cli/src/main.rs b/apps/designcraft-cli/src/main.rs index dd62cd91..17ba8e85 100644 --- a/apps/designcraft-cli/src/main.rs +++ b/apps/designcraft-cli/src/main.rs @@ -252,6 +252,8 @@ fn run(args: &[String]) -> Result<(), String> { let (id, p) = c.split_once('=').unwrap_or((&c, "{}")); let p: Value = serde_json::from_str(p).map_err(|e| format!("--cmd {id}: {e}"))?; let p = designcraft_engine::script::resolve(&p, &results).map_err(|e| format!("--cmd {id}: {e}"))?; + // A key the command's description doesn't name is an error, not a default (#776). + designcraft_engine::check_params(id, &p).map_err(|e| e.to_string())?; let r = s.execute(id, &p).map_err(|e| e.to_string())?; results.push(r.clone()); if !r.is_null() { diff --git a/apps/designcraft-cli/tests/cli.rs b/apps/designcraft-cli/tests/cli.rs index 92f68ab5..dd187108 100644 --- a/apps/designcraft-cli/tests/cli.rs +++ b/apps/designcraft-cli/tests/cli.rs @@ -67,6 +67,22 @@ fn run_cmd_references_and_describe() { assert!(v.as_array().unwrap().iter().all(|c| c.to_string().contains("footnote"))); } +/// #776: a key the command doesn't take fails the run (`--cmd`) or the step (`script`) instead of +/// being ignored. +#[test] +fn unknown_params_fail() { + let out = cli().args(["run", "--cmd", r#"style.list={"bogus":1}"#]).output().unwrap(); + assert!(!out.status.success()); + assert!(String::from_utf8_lossy(&out.stderr).contains("unknown parameter `bogus`; the command takes no parameters")); + let mut child = cli().args(["script", "-"]).stdin(Stdio::piped()).stdout(Stdio::piped()).stderr(Stdio::piped()).spawn().unwrap(); + child.stdin.take().unwrap().write_all(b"document.inspect\nfile.new {\"widht\": 300}\n").unwrap(); + let out = child.wait_with_output().unwrap(); + assert!(!out.status.success()); + let v: serde_json::Value = serde_json::from_slice(&out.stdout).unwrap(); + assert_eq!(v["failedIndex"], 1); + assert!(v.to_string().contains("did you mean `width`?"), "{v}"); +} + /// `designcraft-cli --help` printed usage to stderr and exited 1; asking for help is not an /// error (#243). It now goes to stdout and succeeds, while an unknown command keeps the same /// usage on stderr and fails. diff --git a/crates/engine/src/cmd/anchored.rs b/crates/engine/src/cmd/anchored.rs index 8c85c706..6ad49f8c 100644 --- a/crates/engine/src/cmd/anchored.rs +++ b/crates/engine/src/cmd/anchored.rs @@ -18,7 +18,7 @@ pub fn specs() -> Vec { "Insert Anchored Object…", ["Object", "Anchored Object"], None, - "{ids, story?, pos? (default: the text insertion point), position?: inline|aboveLine|custom, yOffset?, align?: left|center|right, spaceBefore?, spaceAfter?, xRelative?/yRelative?: anchor|textFrame|columnEdge|pageMargin|pageEdge, xOffset?, objectPoint?/refPoint?: 0..8, keepWithinColumn?} — moves the items into the text", + "{ids, story?, pos? (default: the text insertion point), position?: inline|aboveLine|custom, yOffset?, align?: left|center|right, spaceBefore?, spaceAfter?, xRelative? | yRelative?: anchor|textFrame|columnEdge|pageMargin|pageEdge, xOffset?, objectPoint? | refPoint?: 0..8, keepWithinColumn?} — moves the items into the text", has_doc, insert ), @@ -27,7 +27,7 @@ pub fn specs() -> Vec { "Anchored Object Options…", ["Object", "Anchored Object"], None, - "{story?, index? (default: the object at the insertion point), position?: inline|aboveLine|custom, yOffset?, align?: left|center|right, spaceBefore?, spaceAfter?, xRelative?/yRelative?: anchor|textFrame|columnEdge|pageMargin|pageEdge, xOffset?, objectPoint?/refPoint?: 0..8, keepWithinColumn?}", + "{story?, index? (default: the object at the insertion point), position?: inline|aboveLine|custom, yOffset?, align?: left|center|right, spaceBefore?, spaceAfter?, xRelative? | yRelative?: anchor|textFrame|columnEdge|pageMargin|pageEdge, xOffset?, objectPoint? | refPoint?: 0..8, keepWithinColumn?}", has_doc, options ), diff --git a/crates/engine/src/cmd/datamerge/mod.rs b/crates/engine/src/cmd/datamerge/mod.rs index 3b3e1dd3..265f2d62 100644 --- a/crates/engine/src/cmd/datamerge/mod.rs +++ b/crates/engine/src/cmd/datamerge/mod.rs @@ -44,7 +44,7 @@ pub fn specs() -> Vec { "Data Fields", [], None, - "{csv? | rows? | path? | bytes?, name?} → [{name, kind, uses}]; no params reads the linked source and does not attach one", + "{csv? | rows? | path? | bytes? | base64?, name?, delimiter?, sheet?} → [{name, kind, uses}]; no params reads the linked source and does not attach one", has_doc, fields_cmd ), @@ -74,7 +74,7 @@ pub fn specs() -> Vec { "Create Merged Document…", [], None, - "{csv? | rows? | path? | bytes?, name?, records?, one?, range?, perPage?, arrange?, insets?, columnSpacing?, rowSpacing?, fitting?, center?, linkImages?, limit?} → {records, pages, missingImages, oversetStories, warnings}", + "{csv? | rows? | path? | bytes? | base64?, name?, delimiter?, sheet?, records?, one?, range?, perPage?, arrange?, insets?, columnSpacing?, rowSpacing?, fitting?, center?, linkImages?, limit?, spread?: removed (passing it is an error)} → {records, pages, missingImages, oversetStories, warnings}", has_doc, merge_cmd ), diff --git a/crates/engine/src/cmd/export.rs b/crates/engine/src/cmd/export.rs index 8e0f04d6..4bf2a759 100644 --- a/crates/engine/src/cmd/export.rs +++ b/crates/engine/src/cmd/export.rs @@ -882,7 +882,7 @@ mod print_group_tests { fn print_transparency_groups_blend_in_cmyk() { let mut s = page_with(&logo()); let text = |s: &mut Session| { - let r = s.execute("file.exportPdf", &json!({"compress": false})).unwrap(); + let r = s.execute("file.exportPdf", &json!({"compressImages": false})).unwrap(); String::from_utf8_lossy(&base64_decode(r["base64"].as_str().unwrap())).into_owned() }; let t = text(&mut s); diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index a1919b89..d793b8a8 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -13,7 +13,7 @@ use crate::{DocState, EngineError, Result, Session}; pub fn specs() -> Vec { vec![ cmd!(noundo "file.new", "Document…", ["File", "New"], Some("Cmd+N"), - "{preset?: \"Letter\"|\"A4\"|…, width?, height?, pages?, facingPages?, columns?, gutter?, margins?: number|{top,bottom,inside,outside}, bleed?, title?}", + "{preset?: \"Letter\"|\"A4\"|…, width?, height?, pages?, facingPages?, columns?, gutter?, margins?: number|{top,bottom,inside,outside}, bleed?, title?, primaryTextFrame?: bool}", always, file_new), cmd!(noundo "file.newSample", "Sample Document", ["Help"], None, "{} — a multi-page magazine sample", always, file_sample), cmd!(query "file.presets", "Document Presets", [], None, "{}", always, |_, _| Ok(serde_json::to_value(PRESETS).unwrap_or_default())), @@ -102,7 +102,7 @@ pub fn specs() -> Vec { "Place…", ["File"], Some("Cmd+D"), - "{path?|base64?, name?, frame?: id (place into), spread?, x?, y?, width?, pdfPage?: n (1-based, Image Import Options), pdfCrop?: crop|trim|bleed|art|media, layoutPage?: n (IDML / .designcraft: that page's objects as a group)} — places an image (into the selected empty frame if any); text files (.txt, .docx, .rtf, .md) and Excel workbooks (.xlsx, as a table) go into the insertion point, the selected frame or a new frame on `page`/`rect` — {autoflow?: adds pages with threaded frames until the text fits, removeStyles?, styleMap?: {imported name: document style}, styleConflicts?: useExisting|redefine|autoRename}", + "{path?|base64?, name?, frame?: id (place into), spread?, x?, y?, width?, height? (video/audio), pdfPage?: n (1-based, Image Import Options), pdfCrop?: crop|trim|bleed|art|media, layoutPage?: n (IDML / .designcraft: that page's objects as a group), page?, rect?: [x0,y0,x1,y1] (text: a new frame there), autoflow?: adds pages with threaded frames until the text fits, removeStyles?, styleMap?: {imported name: document style}, styleConflicts?: useExisting|redefine|autoRename} — places an image (into the selected empty frame if any); text files (.txt, .docx, .rtf, .md) and Excel workbooks (.xlsx, as a table) go into the insertion point, the selected frame or a new frame on `page`/`rect`", has_doc, file_place ), diff --git a/crates/engine/src/cmd/find.rs b/crates/engine/src/cmd/find.rs index 5dc0bc8a..ef9157c5 100644 --- a/crates/engine/src/cmd/find.rs +++ b/crates/engine/src/cmd/find.rs @@ -25,7 +25,7 @@ pub fn specs() -> Vec { "Change All (Object)", [], None, - "{…find.objects criteria, change: {fill?, stroke?, strokeWeight?, opacity?}} → {changed}", + "{…the criteria of find.objects, change: {fill?, stroke?, strokeWeight?, opacity?}} → {changed}", has_doc, |s, p| { let ids = find_objects(&s.doc()?.doc, p); @@ -65,7 +65,7 @@ pub fn specs() -> Vec { has_doc, change ), - cmd!(noundo "find.next", "Find Next", ["Edit"], Some("Cmd+Alt+F"), "{find, grep?, …} — selects the next match after the caret", has_doc, find_next), + cmd!(noundo "find.next", "Find Next", ["Edit"], Some("Cmd+Alt+F"), "{find, grep?, caseSensitive?, wholeWord?} — selects the next match after the caret", has_doc, find_next), ] } diff --git a/crates/engine/src/cmd/mathexpr.rs b/crates/engine/src/cmd/mathexpr.rs index d9ce5114..ecee8272 100644 --- a/crates/engine/src/cmd/mathexpr.rs +++ b/crates/engine/src/cmd/mathexpr.rs @@ -40,7 +40,7 @@ pub fn specs() -> Vec { "Insert Math Expression…", ["Type"], None, - "{latex, size?: points (12), x?, y?, spread?} — at the text insertion point (inline), else as a frame; or {id, latex} to edit one", + "{latex, size?: points (12), x?, y?, spread?} or {id, latex, size?} — at the text insertion point (inline), else as a frame; with `id`: edits that equation", has_doc, insert ), diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index 31e4544f..80d8f295 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -34,6 +34,7 @@ mod notes; mod object; mod overrides; mod package; +pub mod params; mod path_type; mod paths; mod pdflayers; diff --git a/crates/engine/src/cmd/object.rs b/crates/engine/src/cmd/object.rs index aa5a2cc0..2ab04377 100644 --- a/crates/engine/src/cmd/object.rs +++ b/crates/engine/src/cmd/object.rs @@ -32,7 +32,7 @@ pub fn specs() -> Vec { "Create Frame", [], None, - "{spread?: index | {kind: doc|parent, index} (a parent spread: index into `parents` of document.inspect), rect: [x0,y0,x1,y1] (spread coords), shape?: rectangle|ellipse|polygon, content?: graphic|text|unassigned, sides?: 6, text?: string, caret?: bool, vertical?: bool (text: a new vertical story)}", + "{spread?: index | {kind: doc|parent, index} (a parent spread: index into `parents` of document.inspect), rect: [x0,y0,x1,y1] (spread coords), shape?: rectangle|ellipse|polygon, content?: graphic|text|unassigned, sides?: 6, starInset?: 0–100 (%), text?: string, caret?: bool, vertical?: bool (text: a new vertical story)}", has_doc, frame_create ), @@ -532,7 +532,7 @@ pub fn specs() -> Vec { "Text Frame Options…", ["Object"], Some("Cmd+B"), - "{columns?, gutter?, inset?: number|[t,l,b,r] (null keeps that side), verticalJustification?: top|center|bottom|justify, firstBaseline?, autoSize?, ignoreWrap?, balanceColumns?, columnRule?: bool, columnRuleWeight? (pt), columnRuleColor? (swatch name), columnRuleTint? (0..1), columnRuleOffset? (pt, horizontal), columnRuleTopInset? (pt), columnRuleBottomInset? (pt), vertical?: bool (sets the story direction of the frames' stories, as Type ▸ Story Direction), ids?}", + "{columns?, gutter?, inset?: number|[t,l,b,r] (null keeps that side), verticalJustification?: top|center|bottom|justify, firstBaseline?, autoSize?, columnsKind?: fixedNumber|fixedWidth|flexibleWidth, ignoreWrap?, balanceColumns?, columnRule?: bool, columnRuleWeight? (pt), columnRuleColor? (swatch name), columnRuleTint? (0..1), columnRuleOffset? (pt, horizontal), columnRuleTopInset? (pt), columnRuleBottomInset? (pt), vertical?: bool (sets the story direction of the frames' stories, as Type ▸ Story Direction), ids?}", has_selection, text_frame_options ), @@ -599,7 +599,7 @@ pub fn specs() -> Vec { has_selection, fitting_options ), - cmd!("object.rename", "Rename", [], None, "{id, name}", has_doc, |s, p| { + cmd!("object.rename", "Rename", [], None, "{id | ids?, name}", has_doc, |s, p| { let name = str_param(p, "name").unwrap_or("").to_string(); set_flag(s, p, move |i| i.name = name.clone(), false) }), @@ -682,7 +682,7 @@ pub fn specs() -> Vec { "Move Gap", [], None, - "{at: [x, y] (spread), delta | dx/dy, spread?} — Gap tool: the gap at `at` between objects (or an object and the page edge) moves by `delta`; the objects on both sides resize → {axis, changed}", + "{at: [x, y] (spread), delta?: pt | dx? | dy?: a drag (the part along the gap's axis counts), spread?} — Gap tool: the gap at `at` between objects (or an object and the page edge) moves by `delta`; the objects on both sides resize → {axis, changed}", has_doc, gap_move ), diff --git a/crates/engine/src/cmd/params.rs b/crates/engine/src/cmd/params.rs new file mode 100644 index 00000000..8a33752c --- /dev/null +++ b/crates/engine/src/cmd/params.rs @@ -0,0 +1,230 @@ +//! The parameter keys a command accepts, read from its params description (#776). +//! +//! A description starts with the params object: `{x, width?: pt, name?|title?: …, sides=6, …}`, or +//! with the objects a command takes instead of one another: `{path} or {name, dataBase64}`, +//! `{name} | {names: […]}`. Each entry of an object starts with a key, or with alternatives split +//! by `|`; a `?` marks an optional one, and a `:`, `=` or `(` starts what the entry says about it. +//! An entry that starts with `…` stands for keys listed elsewhere: those of each command it names +//! after `as` or `of` (`…settings as document.new`), or, naming none, any key. Objects inside a +//! value aren't described key by key, so only the top level is checked. +//! +//! Callers outside the app (the control channel, MCP, the CLI) are answered with a parameter error +//! for a key the description doesn't name ([`check_params`]): a misspelt `widht` no longer runs the +//! command with its default. [`Session::execute`](crate::Session::execute) itself takes what it is +//! given, so the journal of an older version still replays. +//! +//! Ported from VectorCraft (storytold/vectorcraft#1442). + +use serde_json::Value; + +use super::{bad, find_command}; +use crate::Result; + +/// What a params description says about the keys of the params object. +#[derive(Debug, Default, PartialEq)] +pub struct ParamKeys<'a> { + /// The keys it names. + pub keys: Vec<&'a str>, + /// Its `…` entries, without the `…`. + pub more: Vec<&'a str>, +} + +/// The text between the braces of the object `doc` starts with and what follows it, or why it +/// has none. +fn object_body(doc: &str) -> std::result::Result<(&str, &str), String> { + // (Named, not written in the `Err`: these are for the tests of the descriptions, not messages + // the app shows, which the message catalogs would have to translate.) + let (no_object, not_closed) = ("the description doesn't start with the params object `{…}`", "the params object `{…}` isn't closed"); + if !doc.starts_with('{') { + return Err(no_object.into()); + } + let (mut depth, mut quote) = (0usize, None); + for (i, c) in doc.char_indices() { + match (quote, c) { + (Some(q), _) if c == q => quote = None, + (Some(_), _) => {} + (None, '"' | '`') => quote = Some(c), + (None, '{' | '[' | '(') => depth += 1, + (None, '}' | ']' | ')') => { + depth = depth.saturating_sub(1); + if depth == 0 { + return Ok((doc.get(1..i).unwrap_or_default(), doc.get(i + 1..).unwrap_or_default())); + } + } + _ => {} + } + } + Err(not_closed.into()) +} + +/// `s` split at each `sep` that is outside brackets, quotes and backticks. +fn split_top(s: &str, sep: char) -> Vec<&str> { + let (mut out, mut depth, mut quote, mut start) = (Vec::new(), 0usize, None, 0); + for (i, c) in s.char_indices() { + match (quote, c) { + (Some(q), _) if c == q => quote = None, + (Some(_), _) => {} + (None, '"' | '`') => quote = Some(c), + (None, '{' | '[' | '(') => depth += 1, + (None, '}' | ']' | ')') => depth = depth.saturating_sub(1), + (None, _) if c == sep && depth == 0 => { + out.push(s.get(start..i).unwrap_or_default()); + start = i + c.len_utf8(); + } + _ => {} + } + } + out.push(s.get(start..).unwrap_or_default()); + out +} + +fn is_key(s: &str) -> bool { + let mut chars = s.chars(); + chars.next().is_some_and(|c| c.is_ascii_alphabetic() || c == '_') && chars.all(|c| c.is_ascii_alphanumeric() || c == '_') +} + +/// The keys a params description names, or what in it doesn't read as one. +pub fn doc_keys(doc: &str) -> std::result::Result, String> { + let mut out = ParamKeys::default(); + let (mut body, mut rest) = object_body(doc)?; + let mut entries = split_top(body, ','); + // `{…} or {…}`, `{…} | {…}`: the objects the command takes instead. + while let Some(next) = rest.trim_start().strip_prefix("or ").or_else(|| rest.trim_start().strip_prefix("| ")).filter(|r| r.starts_with('{')) { + (body, rest) = object_body(next)?; + entries.extend(split_top(body, ',')); + } + for entry in entries.into_iter().map(str::trim).filter(|e| !e.is_empty()) { + if let Some(more) = entry.strip_prefix('…') { + out.more.push(more.trim()); + continue; + } + // `a?|b?: note` names two keys, and so does `a?: note | b?: note`; in `a?: x|y` the `y` + // is a value. + let mut noted = false; + for alternative in split_top(entry, '|') { + let head = alternative.split([':', '=', '(']).next().unwrap_or_default(); + let rest = alternative.get(head.len()..).unwrap_or_default(); + let name = head.trim().trim_end_matches('?').trim_end(); + if !noted { + if !is_key(name) { + let start: String = entry.chars().take(60).collect(); + let why = format!("`{start}` doesn't start with a parameter name"); + return Err(why); + } + out.keys.push(name); + noted = !rest.is_empty(); + } else if is_key(name) && (head.trim_end().ends_with('?') || rest.starts_with(':')) { + out.keys.push(name); + } + } + } + Ok(out) +} + +/// The commands whose keys a `…` entry stands for: those it names after `as` or `of` (`settings as +/// file.new`, `the PDF options of document.exportPdf`), not one it only points to (`see …`). +fn named_commands(more: &str) -> impl Iterator + '_ { + let words: Vec<&str> = more.split_whitespace().collect(); + let id = |w: &str| find_command(w.trim_matches(|c: char| !c.is_ascii_alphanumeric())); + (1..words.len()).filter_map(move |i| id(words.get(i)?).filter(|_| matches!(words.get(i - 1), Some(&"as" | &"of")))) +} + +/// Every key command `doc` accepts, the keys of the commands its `…` entries name included. +/// `None`: any key (a `…` entry naming no command, or a description that doesn't parse, which +/// the tests rule out). +fn accepted(doc: &'static str, depth: usize) -> Option> { + let parsed = doc_keys(doc).ok()?; + let mut keys = parsed.keys; + // A command that acts on `ids` takes one object as `id` too (see `cmd::targets`). + if keys.contains(&"ids") && !keys.contains(&"id") { + keys.push("id"); + } + for more in parsed.more { + let mut named = named_commands(more).peekable(); + // A reference that leads back here would never end: descriptions refer one or two deep. + if named.peek().is_none() || depth > 4 { + return None; + } + for spec in named { + // A command that isn't checked yet doesn't vouch for the ones that take its keys. + if UNCHECKED.contains(&spec.id) { + return None; + } + keys.extend(accepted(spec.params, depth + 1)?); + } + } + Some(keys) +} + +/// The edits (insert, delete, replace, swap of neighbours) that turn `a` into `b`, ignoring case. +fn distance(a: &str, b: &str) -> usize { + let (a, b): (Vec, Vec) = (a.chars().map(|c| c.to_ascii_lowercase()).collect(), b.chars().map(|c| c.to_ascii_lowercase()).collect()); + let (mut before, mut prev): (Vec, Vec) = (Vec::new(), (0..=b.len()).collect()); + for (i, ca) in a.iter().enumerate() { + let mut row = vec![i + 1]; + for (j, cb) in b.iter().enumerate() { + let at = |r: &[usize], k: usize| r.get(k).copied().unwrap_or(usize::MAX / 2); + let mut d = (at(&prev, j) + usize::from(ca != cb)).min(at(&prev, j + 1) + 1).min(at(&row, j) + 1); + if i > 0 && j > 0 && a.get(i - 1) == Some(cb) && b.get(j - 1) == Some(ca) { + d = d.min(at(&before, j - 1) + 1); + } + row.push(d); + } + before = std::mem::replace(&mut prev, row); + } + prev.last().copied().unwrap_or(0) +} + +/// The key of `keys` a misspelt `key` most likely meant: the nearest one, when it is near. +fn nearest<'a>(key: &str, keys: &[&'a str]) -> Option<&'a str> { + let near = keys.iter().map(|k| (distance(key, k), *k)).min()?; + (near.0 <= (key.chars().count() / 3).max(1)).then_some(near.1) +} + +/// `params` has only keys the description `doc` of command `id` names, or the parameter error +/// naming the first one it doesn't, with the nearest accepted key or the accepted ones. +pub fn check_keys(id: &str, doc: &'static str, params: &Value) -> Result<()> { + let Value::Object(given) = params else { return Ok(()) }; + if given.is_empty() { + return Ok(()); + } + let Some(keys) = accepted(doc, 0) else { return Ok(()) }; + match given.keys().find(|k| !keys.contains(&k.as_str())) { + Some(key) => Err(unknown(id, key, &keys)), + None => Ok(()), + } +} + +/// The parameter error of command `id` for `key`, which isn't one of `keys`: with the nearest of +/// them, or all of them. +fn unknown(id: &str, key: &str, keys: &[&str]) -> crate::EngineError { + bad( + id, + match nearest(key, keys) { + Some(near) => format!("unknown parameter `{key}`; did you mean `{near}`?"), + None if keys.is_empty() => format!("unknown parameter `{key}`; the command takes no parameters"), + None => format!("unknown parameter `{key}`; the command takes: {}", keys.join(", ")), + }, + ) +} + +/// Commands whose params aren't checked yet (#776): they read keys in ways the audit of the +/// descriptions couldn't follow (a list of keys in a loop, a whole object read as settings), so +/// their descriptions aren't proven to name every key they take. Sorted; take a command out once +/// its description is checked against its code. +pub const UNCHECKED: &[&str] = &[ + // Take the attributes under `attrs` or as the params object itself; the attribute setters + // refuse a key they don't know. + "type.char", + "type.para", +]; + +/// [`check_keys`] for an engine command. A command the engine doesn't have passes: running it +/// reports that. +pub fn check_params(id: &str, params: &Value) -> Result<()> { + let Some(spec) = find_command(id) else { return Ok(()) }; + if UNCHECKED.contains(&id) { + return Ok(()); + } + check_keys(id, spec.params, params) +} diff --git a/crates/engine/src/cmd/prefs.rs b/crates/engine/src/cmd/prefs.rs index 68a5cf15..e8bdd47c 100644 --- a/crates/engine/src/cmd/prefs.rs +++ b/crates/engine/src/cmd/prefs.rs @@ -48,7 +48,7 @@ pub fn specs() -> Vec { "Preferences", [], None, - "{showHiddenCharacters?, typographersQuotes?, polygonSides?, starInset?, scaleStrokes?, dimensionsIncludeStroke?, transformationsAreTotals?, absolutePageNumbers?, highlightHj?, highlightKeeps?, highlightCustomTracking?, highlightSubstitutedFonts?, richBlackOutput?, favoriteFonts?: [family], showFontNamesInEnglish?} → all application preferences", + "{showHiddenCharacters?, typographersQuotes?, polygonSides?, starInset?, scaleStrokes?, dimensionsIncludeStroke?, transformationsAreTotals?, absolutePageNumbers?, highlightHj?, highlightKeeps?, highlightCustomTracking?, highlightSubstitutedFonts?, richBlackOutput?, favoriteFonts?: [family], recentFonts?: [family], showFontNamesInEnglish?, smartTextReflow?, autocorrect?, autocorrectList?: [[word, correction]], showAddedText?, showNoteAnchors?, recoveryMinutes?} → all application preferences", super::always, |s, p| { let cur = serde_json::to_value(&s.prefs).map_err(|e| bad("prefs.set", e.to_string()))?; diff --git a/crates/engine/src/cmd/style.rs b/crates/engine/src/cmd/style.rs index c4d20f09..7af19431 100644 --- a/crates/engine/src/cmd/style.rs +++ b/crates/engine/src/cmd/style.rs @@ -1260,7 +1260,12 @@ mod color_tests { let mut session = fill_session(); session.execute("style.paragraph.apply", &json!({"name": "Existing"})).unwrap(); session.execute("style.character.apply", &json!({"name": "Existing"})).unwrap(); - assert_fill_rejected(&mut session, command, json!({"name": "Existing", "rename": "Renamed", "chars": {"fill": fill, "size": 24}})); + let mut params = json!({"name": "Existing", "chars": {"fill": fill, "size": 24}}); + // Only the edit commands take `rename`. + if command.ends_with(".edit") { + params["rename"] = json!("Renamed"); + } + assert_fill_rejected(&mut session, command, params); } } } @@ -1611,7 +1616,7 @@ mod export_tag_tests { assert!(html.contains("

Origins

"), "{html}"); assert!(html.contains("word"), "{html}"); // Tagged PDF: a heading element and a paragraph element. - let r = s.execute("file.exportPdf", &json!({"tagged": true, "compress": false})).unwrap(); + let r = s.execute("file.exportPdf", &json!({"tagged": true, "compressImages": false})).unwrap(); let pdf = super::super::file::base64_decode(r["base64"].as_str().unwrap()); let text = String::from_utf8_lossy(&pdf); assert!(text.contains("/S/H1") && text.contains("/S/P"), "structure elements"); @@ -1728,12 +1733,20 @@ mod sample_color_tests { #[test] fn sampling_and_explicit_colour_use_the_same_text_application_path() { for command in ["color.sample", "object.color"] { + // Each command gets its own key: the eyedropper samples at `at`, object.color takes `color`. + let params = |at: [f64; 2], color: &str, extra: serde_json::Value| { + let mut p = if command == "color.sample" { json!({"at": at}) } else { json!({"color": color}) }; + if let (Some(p), Some(extra)) = (p.as_object_mut(), extra.as_object()) { + p.extend(extra.clone()); + } + p + }; let (mut s, _) = session_with_image(); let f = s.execute("frame.create", &json!({"rect": [300, 300, 500, 360], "content": "text"})).unwrap(); let story = designcraft_doc::StoryId(f["story"].as_u64().unwrap()); s.execute("text.select", &json!({"story": story.0, "anchor": 0})).unwrap(); let undo = s.doc().unwrap().history.undo.len(); - let r = s.execute(command, &json!({"at": [100, 100], "color": "#ff0000"})).unwrap(); + let r = s.execute(command, ¶ms([100.0, 100.0], "#ff0000", json!({}))).unwrap(); assert_eq!(s.doc().unwrap().history.undo.len(), undo + 1, "{command}"); s.execute("text.insert", &json!({"text": "Hi"})).unwrap(); let text = s.doc().unwrap().doc.story(story).unwrap(); @@ -1741,11 +1754,11 @@ mod sample_color_tests { // Explicit ids override text targeting, including with a text selection active. let fid = ItemId(f["id"].as_u64().unwrap()); s.execute("text.select", &json!({"story": story.0, "anchor": 0, "focus": 2})).unwrap(); - let r = s.execute(command, &json!({"at": [180, 100], "color": "#0000ff", "id": fid.0})).unwrap(); + let r = s.execute(command, ¶ms([180.0, 100.0], "#0000ff", json!({"id": fid.0}))).unwrap(); assert_eq!(s.doc().unwrap().doc.item(fid).unwrap().fill.swatch, r["swatch"].as_str().unwrap()); let text = s.doc().unwrap().doc.story(story).unwrap(); assert_ne!(text.format_after(0).over.fill.as_deref(), r["swatch"].as_str(), "text unchanged: {command}"); - let r = s.execute(command, &json!({"at": [180, 100], "color": "#0000ff", "target": "stroke"})).unwrap(); + let r = s.execute(command, ¶ms([180.0, 100.0], "#0000ff", json!({"target": "stroke"}))).unwrap(); let text = s.doc().unwrap().doc.story(story).unwrap(); assert_eq!(text.format_after(0).over.stroke.as_deref(), r["swatch"].as_str(), "selected text stroke: {command}"); } diff --git a/crates/engine/src/cmd/table.rs b/crates/engine/src/cmd/table.rs index ad306be8..9a62ce24 100644 --- a/crates/engine/src/cmd/table.rs +++ b/crates/engine/src/cmd/table.rs @@ -29,7 +29,7 @@ fn style_specs() -> Vec { "New Cell Style…", [], None, - "{name, fromSelection?: bool (the target cell's look), fill?, tint?, insets?: n|[t,l,b,r], vj?: top|center|bottom|justify, stroke?: {weight, color, tint?}, paragraphStyle?} → {name}", + "{name, fromSelection?: bool (the target cell's look), fill?, tint?, insets?: n|[t,l,b,r], vj?: top|center|bottom|justify, stroke?: {weight, color, tint?}, paragraphStyle?, basedOn?: cell style name, story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)} → {name}", has_doc, |s, p| cell_style_create(s, p) ), @@ -38,7 +38,7 @@ fn style_specs() -> Vec { "Apply Cell Style", [], None, - "{name, story?, table?} — the target cells (the whole table when the cursor is in it)", + "{name, story?, table?, rows?: [a,b] | row?, cols?: [a,b] | col?} — the target cells (the whole table when the cursor is in it)", in_table, cell_style_apply ), @@ -56,11 +56,19 @@ fn style_specs() -> Vec { "New Table Style…", [], None, - "{name, header?, body?, footer?, leftColumn?, rightColumn?: cell style names, border?: {weight, color}, altRows?: {first, firstColor, next, nextColor}, spaceBefore?, spaceAfter?} → {name}", + "{name, basedOn?: table style name, header?, body?, footer?, leftColumn?, rightColumn?: cell style names, border?: {weight, color}, altRows?: {first, firstColor, next, nextColor}, spaceBefore?, spaceAfter?} → {name}", has_doc, table_style_create ), - cmd!("style.table.apply", "Apply Table Style", [], None, "{name, story?, table?}", in_table, table_style_apply), + cmd!( + "style.table.apply", + "Apply Table Style", + [], + None, + "{name, story?, table?, rows?: [a,b] | row?, cols?: [a,b] | col?}", + in_table, + table_style_apply + ), cmd!( "style.table.edit", "Table Style Options…", @@ -80,7 +88,7 @@ fn table_specs() -> Vec { "Create Table", [], Some("Cmd+Alt+Shift+T"), - "{rows?: body rows (4), cols? (4), headerRows? (0), footerRows? (0), width?} — at the text insertion point", + "{rows?: body rows (4), cols? | columns? (4), headerRows? (0), footerRows? (0), width?} — at the text insertion point", has_text, insert ), @@ -89,7 +97,7 @@ fn table_specs() -> Vec { "Convert Text to Table", ["Table"], None, - "{columnSeparator?: tab|comma} — selected paragraphs become rows", + "{columnSeparator?: tab|comma, width?: table width (default: the column)} — selected paragraphs become rows", has_text, convert_from_text ), @@ -98,20 +106,100 @@ fn table_specs() -> Vec { "Convert Table to Text", ["Table"], None, - "{} — cells separated by tabs, rows by paragraphs", + "{story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)} — cells separated by tabs, rows by paragraphs", in_table, convert_to_text ), - cmd!("table.insertRow", "Insert Row", [], None, "{where?: above|below, count? (1)}", in_table, |s, p| insert_row(s, p, None)), - cmd!("table.insertRowAbove", "Insert Row Above", ["Table", "Insert"], None, "{count?}", in_table, |s, p| insert_row(s, p, Some(true))), - cmd!("table.insertRowBelow", "Insert Row Below", ["Table", "Insert"], None, "{count?}", in_table, |s, p| insert_row(s, p, Some(false))), - cmd!("table.insertColumn", "Insert Column", [], None, "{where?: left|right, count? (1), width?}", in_table, |s, p| insert_col(s, p, None)), - cmd!("table.insertColumnLeft", "Insert Column Left", ["Table", "Insert"], None, "{count?}", in_table, |s, p| insert_col(s, p, Some(true))), - cmd!("table.insertColumnRight", "Insert Column Right", ["Table", "Insert"], None, "{count?}", in_table, |s, p| insert_col(s, p, Some(false))), - cmd!("table.deleteRow", "Delete Row", ["Table", "Delete"], None, "{} — the rows of the target cells", in_table, delete_rows), - cmd!("table.deleteColumn", "Delete Column", ["Table", "Delete"], None, "{} — the columns of the target cells", in_table, delete_cols), - cmd!("table.delete", "Delete Table", ["Table", "Delete"], None, "{}", in_table, delete_table), - cmd!("table.merge", "Merge Cells", ["Table"], None, "{} — merge the target cell range", in_table, merge), + cmd!( + "table.insertRow", + "Insert Row", + [], + None, + "{where?: above|below, count? (1), story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}", + in_table, + |s, p| insert_row(s, p, None) + ), + cmd!( + "table.insertRowAbove", + "Insert Row Above", + ["Table", "Insert"], + None, + "{count?, story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}", + in_table, + |s, p| insert_row(s, p, Some(true)) + ), + cmd!( + "table.insertRowBelow", + "Insert Row Below", + ["Table", "Insert"], + None, + "{count?, story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}", + in_table, + |s, p| insert_row(s, p, Some(false)) + ), + cmd!( + "table.insertColumn", + "Insert Column", + [], + None, + "{where?: left|right, count? (1), width?, story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}", + in_table, + |s, p| insert_col(s, p, None) + ), + cmd!( + "table.insertColumnLeft", + "Insert Column Left", + ["Table", "Insert"], + None, + "{count?, width?, story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}", + in_table, + |s, p| insert_col(s, p, Some(true)) + ), + cmd!( + "table.insertColumnRight", + "Insert Column Right", + ["Table", "Insert"], + None, + "{count?, width?, story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}", + in_table, + |s, p| insert_col(s, p, Some(false)) + ), + cmd!( + "table.deleteRow", + "Delete Row", + ["Table", "Delete"], + None, + "{story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)} — the rows of the target cells", + in_table, + delete_rows + ), + cmd!( + "table.deleteColumn", + "Delete Column", + ["Table", "Delete"], + None, + "{story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)} — the columns of the target cells", + in_table, + delete_cols + ), + cmd!( + "table.delete", + "Delete Table", + ["Table", "Delete"], + None, + "{story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}", + in_table, + delete_table + ), + cmd!( + "table.merge", + "Merge Cells", + ["Table"], + None, + "{story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)} — merge the target cell range", + in_table, + merge + ), cmd!( "table.dropCells", "Drag Rows or Columns", @@ -126,7 +214,7 @@ fn table_specs() -> Vec { "Convert Cell to Graphic Cell", ["Table", "Convert Cell Type"], None, - "{path | base64+name, fit?: proportional|fill (default proportional)} — an image in the target cell (its text is kept but hidden)", + "{path? | base64?, name?: with base64, fit?: proportional|fill (default proportional), story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)} — an image in the target cell (its text is kept but hidden)", in_table, place_graphic ), @@ -135,7 +223,7 @@ fn table_specs() -> Vec { "Convert Cell to Text Cell", ["Table", "Convert Cell Type"], None, - "{} — drop the target cells' graphics", + "{story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)} — drop the target cells' graphics", in_table, |s, p| { let g = target(s, p, "table.textCell")?; @@ -156,29 +244,43 @@ fn table_specs() -> Vec { "Move Row", [], None, - "{from?, to} (0-based; from defaults to the target row) — like dragging a row", + "{from?, to, story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)} (0-based; from defaults to the target row) — like dragging a row", in_table, |s, p| move_rc(s, p, true) ), - cmd!("table.moveColumn", "Move Column", [], None, "{from?, to} (0-based; from defaults to the target column)", in_table, |s, p| move_rc( - s, p, false - )), + cmd!( + "table.moveColumn", + "Move Column", + [], + None, + "{from?, to, story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)} (0-based; from defaults to the target column)", + in_table, + |s, p| move_rc(s, p, false) + ), cmd!( "table.sortRows", "Sort", ["Table"], None, - "{column?: 0-based (default: the target cell's), descending?: bool} — body rows by that column's text (numbers numerically)", + "{column?: 0-based (default: the target cell's), descending?: bool, story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)} — body rows by that column's text (numbers numerically)", in_table, sort_rows ), - cmd!("table.unmerge", "Unmerge Cells", ["Table"], None, "{}", in_table, unmerge), + cmd!( + "table.unmerge", + "Unmerge Cells", + ["Table"], + None, + "{story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}", + in_table, + unmerge + ), cmd!( "table.splitHorizontally", "Split Cell Horizontally", ["Table"], None, - "{} — each target cell becomes two, one above the other", + "{story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)} — each target cell becomes two, one above the other", in_table, |s, p| split(s, p, true) ), @@ -187,7 +289,7 @@ fn table_specs() -> Vec { "Split Cell Vertically", ["Table"], None, - "{} — each target cell becomes two, side by side", + "{story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)} — each target cell becomes two, side by side", in_table, |s, p| split(s, p, false) ), @@ -196,19 +298,43 @@ fn table_specs() -> Vec { "Cell Options", [], None, - "{fill?: swatch, tint?, insets?: n | [t,l,b,r], vj?: top|center|bottom|justify, text?, stroke?: {weight?, color?, tint?, type?: solid|dashed|dotted, edges?: all|outer|inner|top|left|bottom|right}}", + "{fill?: swatch, tint?, insets?: n | [t,l,b,r], vj? | verticalJustification?: top|center|bottom|justify, text?, stroke?: {weight?, color?, tint?, type?: solid|dashed|dotted, edges?: all|outer|inner|top|left|bottom|right}, story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}", in_table, set_cell ), - cmd!("table.setRowHeight", "Row Height", [], None, "{height, mode?: atLeast|exactly}", in_table, set_row_height), - cmd!("table.setColumnWidth", "Column Width", [], None, "{width}", in_table, set_col_width), - cmd!("table.distributeColumns", "Distribute Columns Evenly", ["Table"], None, "{}", in_table, distribute_cols), + cmd!( + "table.setRowHeight", + "Row Height", + [], + None, + "{height, mode?: atLeast|exactly, story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}", + in_table, + set_row_height + ), + cmd!( + "table.setColumnWidth", + "Column Width", + [], + None, + "{width, story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}", + in_table, + set_col_width + ), + cmd!( + "table.distributeColumns", + "Distribute Columns Evenly", + ["Table"], + None, + "{story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}", + in_table, + distribute_cols + ), cmd!( "table.distributeRows", "Distribute Rows Evenly", ["Table"], None, - "{story?, table?, rows?: [a,b]} — equal fixed heights preserving the selected rows' composed total height; may overset cell text; unmerge row-spanning cells first", + "{story?, table?, rows?: [a,b] | row?, cols?: [a,b] | col?} — equal fixed heights preserving the selected rows' composed total height; may overset cell text; unmerge row-spanning cells first", in_table, distribute_rows ), @@ -217,23 +343,23 @@ fn table_specs() -> Vec { "Table Options", [], None, - "{direction?: leftToRight|rightToLeft, border?: {weight?, color?, tint?, type?}, spaceBefore?, spaceAfter?, headerRows?, footerRows?, repeatHeader?, repeatFooter?, altRows?: {first, firstColor, firstTint, next, nextColor, nextTint, skipFirst, skipLast} | null, altCols?: … | null}", + "{direction?: leftToRight|rightToLeft, border?: {weight?, color?, tint?, type?}, spaceBefore?, spaceAfter?, headerRows?, footerRows?, repeatHeader?, repeatFooter?, altRows?: {first, firstColor, firstTint, next, nextColor, nextTint, skipFirst, skipLast} | null, altCols?: … | null, story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}", in_table, options ), - cmd!(noundo "table.select", "Select Cells", [], None, "{story?, table?, rows?: [a,b], cols?: [a,b], what?: cell|row|column|table}", in_table_or_ids, select), - cmd!(noundo "table.selectTable", "Select Table", ["Table", "Select"], None, "{}", in_table, |s, p| { + cmd!(noundo "table.select", "Select Cells", [], None, "{story?, table?, rows?: [a,b] | row?, cols?: [a,b] | col?, what?: cell|row|column|table}", in_table_or_ids, select), + cmd!(noundo "table.selectTable", "Select Table", ["Table", "Select"], None, "{story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}", in_table, |s, p| { select(s, &super::with_param(p, "what", json!("table"))) }), - cmd!(noundo "table.selectRow", "Select Row", ["Table", "Select"], None, "{}", in_table, |s, p| { + cmd!(noundo "table.selectRow", "Select Row", ["Table", "Select"], None, "{story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}", in_table, |s, p| { select(s, &super::with_param(p, "what", json!("row"))) }), - cmd!(noundo "table.selectColumn", "Select Column", ["Table", "Select"], None, "{}", in_table, |s, p| { + cmd!(noundo "table.selectColumn", "Select Column", ["Table", "Select"], None, "{story?, table?, rows?: [a, b] | row?, cols?: [a, b] | col? (the target cells; default: the selection)}", in_table, |s, p| { select(s, &super::with_param(p, "what", json!("column"))) }), cmd!(noundo "table.nextCell", "Next Cell", [], None, "{}", in_cell, |s, _| step_cell(s, true)), cmd!(noundo "table.prevCell", "Previous Cell", [], None, "{}", in_cell, |s, _| step_cell(s, false)), - cmd!(query "table.get", "Get Table", [], None, "{story?, table?} → rows, columns, cells (text, spans, fill), options", in_table_or_ids, get), + cmd!(query "table.get", "Get Table", [], None, "{story?, table?, rows?: [a,b] | row?, cols?: [a,b] | col?} → rows, columns, cells (text, spans, fill), options", in_table_or_ids, get), ] } diff --git a/crates/engine/src/cmd/text.rs b/crates/engine/src/cmd/text.rs index b57e9e88..0911a365 100644 --- a/crates/engine/src/cmd/text.rs +++ b/crates/engine/src/cmd/text.rs @@ -68,7 +68,7 @@ pub fn specs() -> Vec { "Story Column Direction", ["Type", "Story Column Direction"], None, - "{story?, direction: leftToRight|rightToLeft} — column progression, independent of paragraph direction", + "{story?, frame?: a text frame of the story, direction: leftToRight|rightToLeft} — column progression, independent of paragraph direction", has_doc, |s, p| { let sid = story_of(s, p).ok_or_else(|| bad("story.setDirection", "no story"))?; @@ -129,7 +129,7 @@ pub fn specs() -> Vec { v["lines"] = json!(cs.line_count()); Ok(v) }), - cmd!("type.fillWithPlaceholder", "Fill with Placeholder Text", ["Type"], None, "{frame?}", has_text_or_frames, fill_placeholder), + cmd!("type.fillWithPlaceholder", "Fill with Placeholder Text", ["Type"], None, "{frame?, story?}", has_text_or_frames, fill_placeholder), cmd!( "type.char", "Character Formatting", diff --git a/crates/engine/src/cmd/xref.rs b/crates/engine/src/cmd/xref.rs index b0b97ebd..284c820b 100644 --- a/crates/engine/src/cmd/xref.rs +++ b/crates/engine/src/cmd/xref.rs @@ -13,7 +13,7 @@ pub fn specs() -> Vec { "Insert Cross-Reference…", ["Type", "Hyperlinks & Cross-References"], None, - "{anchor? | paragraph?: text to find (first paragraph containing it) | story? + para? (0-based), format?: name (default Full Paragraph & Page Number)} — at the insertion point; the text updates itself as the destination moves", + "{anchor? | paragraph?: text to find (first paragraph containing it) | story?: with para? | para?: 0-based paragraph of `story`, format?: name (default Full Paragraph & Page Number)} — at the insertion point; the text updates itself as the destination moves", in_story_text, insert ), diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index caeeb80b..f7902ab5 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -25,6 +25,7 @@ use designcraft_compose::Cache; use designcraft_doc::{Document, LayerId, Selection}; use serde_json::Value; +pub use cmd::params::{check_keys, check_params}; pub use cmd::{CommandInfo, CommandSpec, command_specs, find_command}; pub use designcraft_compose as compose; pub use designcraft_doc as doc; @@ -402,6 +403,10 @@ impl Session { fn execute_unguarded(&mut self, id: &str, params: &Value) -> Result { let spec = find_command(id).ok_or_else(|| EngineError::UnknownCommand(id.into()))?; + // The crate's own tests are the audit of the params descriptions (#776): a key a test + // passes that its command's description doesn't name fails the test. + #[cfg(test)] + check_params(id, params)?; if let Err(e) = (spec.enabled)(self) { // A command that takes `ids` acts on the objects the call names, so it needs no // selection: with a text caret or nothing selected, `{"ids": [5]}` still runs. @@ -561,4 +566,6 @@ mod tests; #[cfg(test)] mod tests_idml; #[cfg(test)] +mod tests_params; +#[cfg(test)] mod tests_table; diff --git a/crates/engine/src/tests_params.rs b/crates/engine/src/tests_params.rs new file mode 100644 index 00000000..19625d90 --- /dev/null +++ b/crates/engine/src/tests_params.rs @@ -0,0 +1,106 @@ +//! The params descriptions name the keys their commands accept (#776). + +use serde_json::json; + +use crate::cmd::params::{UNCHECKED, check_keys, check_params, doc_keys}; +use crate::{EngineError, Session, command_specs, find_command}; + +fn keys(doc: &str) -> Vec<&str> { + doc_keys(doc).unwrap().keys +} + +#[test] +fn a_description_names_its_keys() { + assert_eq!(keys("{} → {id}"), Vec::<&str>::new()); + assert_eq!( + keys("{x, y, width?: pt=612|\"210 mm\", sides=6, name?|title?: a name (default: Untitled-N)} → {index}"), + ["x", "y", "width", "sides", "name", "title"] + ); + // Brackets, quotes and backticks keep their commas and bars; a nested object isn't listed. + assert_eq!( + keys("{at: [x, y] (page, not view), stops: [{position: pt, leader?: \", \"}], csv?: a `|, ` in a label is a break, on?: bool}"), + ["at", "stops", "csv", "on"] + ); + // Alternatives after a note are keys when they have their own `?` or `:`; else values. + assert_eq!( + keys("{steps?: n | distance?: pt | smooth?: bool, spacing?: smooth|steps|distance, path? | data?: text | dataBase64?}"), + ["steps", "distance", "smooth", "spacing", "path", "data", "dataBase64"] + ); + assert_eq!(keys("{latex, size?} or {id, latex}"), ["latex", "size", "id", "latex"]); + let more = doc_keys("{name, …same as style.cell.create, …the format's options}").unwrap(); + assert_eq!((more.keys, more.more), (vec!["name"], vec!["same as style.cell.create", "the format's options"])); + for bad in ["same as paint.setFill", "{a, b", "{opacity? 0..1}", "{path | name+dataBase64}", "{xRelative?/yRelative?: anchor}"] { + assert!(doc_keys(bad).is_err(), "{bad}"); + } +} + +/// Every command's description reads as a params object, so none is left unchecked by accident. +#[test] +fn every_description_parses() { + let bad: Vec = command_specs().iter().filter_map(|c| doc_keys(c.params).err().map(|e| format!("{}: {e}", c.id))).collect(); + assert!(bad.is_empty(), "{} descriptions don't parse:\n{}", bad.len(), bad.join("\n")); + let stale: Vec<_> = UNCHECKED.iter().filter(|id| find_command(id).is_none()).collect(); + assert!(stale.is_empty() && UNCHECKED.is_sorted(), "UNCHECKED: sorted ids of commands; not commands: {stale:?}"); +} + +fn refusal(id: &str, params: serde_json::Value) -> String { + match check_params(id, ¶ms) { + Err(EngineError::BadParams { cmd, msg }) if cmd == id => msg, + other => panic!("{id} {params}: {other:?}"), + } +} + +/// The examples of #776: both ran and did nothing (or the default) without a word. +#[test] +fn the_issue_examples_are_refused() { + // (The keys are checked in order: `end` comes before `start`.) + assert_eq!(refusal("text.select", json!({"story": 1, "start": 0, "end": 5})), "unknown parameter `end`; the command takes: story, anchor, focus"); + assert_eq!(refusal("text.select", json!({"story": 1, "start": 0})), "unknown parameter `start`; the command takes: story, anchor, focus"); + assert_eq!(refusal("style.list", json!({"bogus": 1})), "unknown parameter `bogus`; the command takes no parameters"); +} + +#[test] +fn an_unknown_key_is_named_with_the_nearest_accepted_one() { + assert_eq!(refusal("file.new", json!({"widht": 300})), "unknown parameter `widht`; did you mean `width`?"); + assert_eq!(refusal("file.new", json!({"Width": 300})), "unknown parameter `Width`; did you mean `width`?"); + assert_eq!( + refusal("line.create", json!({"a": [0, 0], "b": [5, 5], "colour": "red"})), + "unknown parameter `colour`; the command takes: spread, a, b" + ); + for (id, ok) in [ + ("file.new", json!({"width": 300, "title": "a", "margins": {"anything": 1}})), + ("file.new", json!({})), + ("file.new", json!(null)), + // A command that acts on `ids` takes one object as `id` too. + ("transform.move", json!({"dx": 1, "dy": 1, "id": 3})), + // Keys listed by reference. + ("style.cell.edit", json!({"name": "a", "fill": "Paper"})), + // On UNCHECKED: the attributes may be the params object itself. + ("type.char", json!({"size": 12})), + // Not a command: running it says so. + ("no.such", json!({"x": 1})), + ] { + assert!(check_params(id, &ok).is_ok(), "{id} {ok}"); + } + assert!(check_keys("x", "{a, …same as file.new}", &json!({"a": 1, "width": 2})).is_ok()); + assert!(check_keys("x", "{a, …same as file.new}", &json!({"b": 1})).is_err()); + // A `…` naming no command stands for any key. + assert!(check_keys("x", "{a, …the format's options}", &json!({"b": 1})).is_ok()); +} + +/// [`Session::execute`] itself takes what it is given (old journals replay); only callers from +/// outside (control channel, MCP, CLI) are checked. In this crate's tests it checks too, as the +/// audit of the descriptions, so this goes through a journal entry instead. +#[test] +fn journal_entries_pass_the_check() { + let mut s = Session::new(); + s.execute("file.new", &json!({})).unwrap(); + let f = s.execute("frame.create", &json!({"rect": [36, 36, 300, 200], "content": "text", "text": "Hello world"})).unwrap(); + s.execute("text.select", &json!({"story": f["story"], "anchor": 0, "focus": 5})).unwrap(); + s.execute("type.char", &json!({"attrs": {"size": 24}})).unwrap(); + s.execute("transform.move", &json!({"dx": 10, "dy": 0, "ids": [f["id"]]})).unwrap(); + assert!(s.journal.len() >= 4); + for (id, params) in &s.journal { + assert!(check_params(id, params).is_ok(), "{id} {params}"); + } +} diff --git a/crates/engine/tests/param_keys.rs b/crates/engine/tests/param_keys.rs new file mode 100644 index 00000000..96f4c836 --- /dev/null +++ b/crates/engine/tests/param_keys.rs @@ -0,0 +1,14 @@ +//! #776: callers from outside are checked with `check_params`; `Session::execute` itself takes +//! what it is given, so a journal written by an older version still replays. + +use designcraft_engine::{Session, check_params}; +use serde_json::json; + +#[test] +fn execute_replays_what_the_check_refuses() { + let mut s = Session::new(); + s.execute("file.new", &json!({})).unwrap(); + let params = json!({"bogus": 1}); + assert!(check_params("style.list", ¶ms).is_err()); + assert!(s.execute("style.list", ¶ms).is_ok()); +} diff --git a/crates/mcp/src/headless.rs b/crates/mcp/src/headless.rs index de035357..ccc2a5d2 100644 --- a/crates/mcp/src/headless.rs +++ b/crates/mcp/src/headless.rs @@ -95,6 +95,8 @@ impl Headless { } fn exec(&mut self, id: &str, params: &Value) -> Result { + // A key the command's description doesn't name is an error, not a default (#776). + designcraft_engine::check_params(id, params).map_err(|e| e.to_string())?; let r = self.session.execute(id, params).map_err(|e| { let ui_only = ["app.", "view.", "window."].iter().any(|p| id.starts_with(p)); if ui_only && designcraft_engine::find_command(id).is_none() { format!("`{id}` is a UI command; {NEEDS_APP}") } else { e.to_string() } diff --git a/crates/mcp/src/server.rs b/crates/mcp/src/server.rs index cd7bffda..328df76d 100644 --- a/crates/mcp/src/server.rs +++ b/crates/mcp/src/server.rs @@ -149,6 +149,7 @@ impl Server { "tools/call" => { let name = params.get("name").and_then(Value::as_str).ok_or((INVALID_PARAMS, "missing tool `name`".to_string()))?; let args = params.get("arguments").cloned().unwrap_or(Value::Null); + check_arguments(name, &args).map_err(|m| (INVALID_PARAMS, m))?; Ok(call_tool(self.backend.as_mut(), name, &args).to_value()) } "resources/list" => Ok(json!({"resources": [ @@ -170,3 +171,41 @@ impl Server { } } } + +/// Arguments a tool doesn't take, and command params a command doesn't take, are invalid params +/// (#776): a misspelt key is refused instead of being ignored. `execute` and `batch` are checked +/// against the params descriptions of the commands they run, every step before the first runs. +fn check_arguments(name: &str, args: &Value) -> Result<(), String> { + let Some(arguments) = args.as_object() else { return Ok(()) }; + if let Some(def) = tool_definitions().into_iter().find(|t| t.get("name").and_then(Value::as_str) == Some(name)) + && let Some(properties) = def.get("inputSchema").and_then(|s| s.get("properties")).and_then(Value::as_object) + && let Some(key) = arguments.keys().find(|k| !properties.contains_key(*k)) + { + return Err(format!("unknown argument `{key}` for `{name}`")); + } + let command = |step: &Value| -> Result<(), String> { + match (step.get("command").and_then(Value::as_str), step.get("params")) { + (Some(id), Some(params)) => designcraft_engine::check_params(id, params).map_err(|e| e.to_string()), + _ => Ok(()), + } + }; + match name { + "execute" => command(args), + "batch" => { + let steps = match (arguments.get("commands").and_then(Value::as_array), arguments.get("script").and_then(Value::as_str)) { + (Some(steps), _) => steps.clone(), + // A script that doesn't parse is the tool's to report. + (None, Some(text)) => match designcraft_engine::script::parse(text) { + Ok(steps) => steps.into_iter().map(|s| json!({"command": s.command, "params": s.params})).collect(), + Err(_) => return Ok(()), + }, + (None, None) => return Ok(()), + }; + for (i, step) in steps.iter().enumerate() { + command(step).map_err(|e| format!("step {i}: {e}"))?; + } + Ok(()) + } + _ => Ok(()), + } +} diff --git a/crates/mcp/src/tests.rs b/crates/mcp/src/tests.rs index 3051a5ad..2ce8088f 100644 --- a/crates/mcp/src/tests.rs +++ b/crates/mcp/src/tests.rs @@ -500,3 +500,41 @@ fn explicit_typing_rejects_stale_selection_and_closed_document() { b.session.execute("file.close", &json!({})).unwrap(); assert!(b.call("ui.text", json!({"text":"X"})).is_err()); } + +/// #776: a key a command (or a tool) doesn't take is an invalid-params error, not ignored. +#[test] +fn unknown_params_are_invalid_params() { + let mut s = server(); + let code_and_message = |v: &Value| (v["error"]["code"].as_i64(), v["error"]["message"].as_str().unwrap_or("").to_string()); + let f = ok(&mut s, "execute", json!({"command": "frame.create", "params": {"rect": [36, 36, 300, 200], "content": "text", "text": "Hello"}})); + let v = rpc( + &mut s, + 1, + "tools/call", + json!({"name": "execute", "arguments": {"command": "text.select", "params": {"story": f["story"], "start": 0, "end": 5}}}), + ); + let (code, msg) = code_and_message(&v); + assert_eq!(code, Some(-32602), "{v}"); + assert!(msg.contains("unknown parameter `end`; the command takes: story, anchor, focus"), "{msg}"); + let v = rpc(&mut s, 2, "tools/call", json!({"name": "execute", "arguments": {"command": "style.list", "params": {"bogus": 1}}})); + let (code, msg) = code_and_message(&v); + assert_eq!(code, Some(-32602), "{v}"); + assert!(msg.contains("unknown parameter `bogus`; the command takes no parameters"), "{msg}"); + // A batch is checked step by step before any step runs: the frame isn't created. + let before = ok(&mut s, "inspect_document", json!({})); + for args in [ + json!({"commands": [{"command": "frame.create", "params": {"rect": [0, 0, 50, 50]}}, {"command": "file.new", "params": {"widht": 300}}]}), + json!({"script": "frame.create {\"rect\": [0, 0, 50, 50]}\nfile.new {\"widht\": 300}"}), + ] { + let v = rpc(&mut s, 3, "tools/call", json!({"name": "batch", "arguments": args})); + let (code, msg) = code_and_message(&v); + assert_eq!(code, Some(-32602), "{v}"); + assert!(msg.contains("step 1") && msg.contains("did you mean `width`?"), "{msg}"); + } + assert_eq!(ok(&mut s, "inspect_document", json!({})), before); + // An argument the tool doesn't take. + let v = rpc(&mut s, 4, "tools/call", json!({"name": "execute", "arguments": {"command": "style.list", "parms": {}}})); + assert_eq!(code_and_message(&v), (Some(-32602), "unknown argument `parms` for `execute`".to_string())); + // The right keys still run. + ok(&mut s, "execute", json!({"command": "text.select", "params": {"story": f["story"], "anchor": 0, "focus": 5}})); +} diff --git a/crates/tools/src/pen.rs b/crates/tools/src/pen.rs index 21e90f73..35406435 100644 --- a/crates/tools/src/pen.rs +++ b/crates/tools/src/pen.rs @@ -117,7 +117,7 @@ impl PenTool { match self.path { None if self.anchors.len() >= 2 => { let anchors: Vec = self.anchors.iter().map(anchor_json).collect(); - vec![Action::Exec("path.create".into(), json!({"spread": spread_json(sr), "anchors": anchors, "closed": false, "notifyPen": true}))] + vec![Action::Exec("path.create".into(), json!({"spread": spread_json(sr), "anchors": anchors, "closed": false}))] } Some(id) => { let mut p = json!({"id": id, "anchor": anchor_json(last)}); diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 5a4de005..76f01e4a 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -114,6 +114,10 @@ pub fn handle(app: &mut DesignApp, ctx: &egui::Context, req: &ControlRequest) -> let Some(id) = s("command").or(s("id")) else { return err("missing `command`") }; let params = p.get("params").cloned().unwrap_or(json!({})); let params = if params.is_null() { json!({}) } else { params }; + // A key the command's description doesn't name is an error, not a default (#776). + if let Err(e) = designcraft_engine::check_params(id, ¶ms) { + return err(e.to_string()); + } wrap(app.run(id, params)) } "engine.commands" => ok(all_commands(app)), diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 695a66af..147b071b 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -4312,7 +4312,8 @@ mod tests { assert_eq!(page.margins.top, 10.0); assert_eq!(page.margins.outside, 40.0); } - assert!(doc.spreads.iter().all(|s| s.items.is_empty()), "undocumented fields stay excluded"); + // `primaryTextFrame` is documented since #776, so the dialog passes it on (`status` isn't). + assert!(doc.spreads.iter().any(|s| !s.items.is_empty()), "documented fields reach the command"); assert!(app.ui.dialog.is_none()); } } diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 9e514561..4b889e35 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -31,7 +31,7 @@ so only enable it while you use it. Transport: `apps/designcraft/src/control_ser | Method | Params | What it does | |---|---|---| -| `engine.execute` / `ui.menu.invoke` | `{command, params}` | Run any engine or UI command (see `engine.commands`). `ui.menu.invoke` without `params` acts like choosing the menu item: a command labelled "…" that takes parameters opens its dialog | +| `engine.execute` / `ui.menu.invoke` | `{command, params}` | Run any engine or UI command (see `engine.commands`). `ui.menu.invoke` without `params` acts like choosing the menu item: a command labelled "…" that takes parameters opens its dialog. A key the command's params description doesn't name is an error (`unknown parameter …`), not ignored | | `engine.commands` | — | Every command: id, label, menu path, shortcut, params doc, enabled | | `document.inspect` | — | Pages, spreads, items, stories (overset), styles, swatches, selection | | `ui.inspect` | — | Tool, UI state, view (zoom/origin), canvas rect, perf | diff --git a/docs/mcp.md b/docs/mcp.md index f3558574..af05aea5 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -76,7 +76,7 @@ and its second character starts at 3. An offset inside a character snaps back to | Tool | What it does | |---|---| | `list_commands` | Every command (id, label, menu path, shortcut, params, enabled), with `filter` / `enabledOnly` | -| `execute` | Run any command: `{command, params}` | +| `execute` | Run any command: `{command, params}`. A params key the command doesn't take is a JSON-RPC `-32602` (invalid params) error naming it, as is an argument a tool doesn't take; `batch` checks every step before running the first | | `batch` | Run `commands: [{command, params}]` (or `script` text, one `command.id {json}` per line) in order, stopping at the first error (reports `failedIndex`); `"$N.path"` parameters use earlier results ([agents.md](agents.md)) | | `inspect_document` | Pages, spreads, items (ids, bounds, fill, story), stories (overset), layers, styles, swatches, selection | | `get_story` | Text, frames, paragraphs, lines, overset of a story (`story` or text `frame`) | From 06cbcc0058005f7522b324c085a5f16f7e757706 Mon Sep 17 00:00:00 2001 From: aiongg Date: Tue, 13 Oct 2026 20:03:39 +0000 Subject: [PATCH 07/12] Integration: #757 the Plus key standing in for = zooms the canvas under a native menu too (#781) --- crates/ui-egui/src/menus.rs | 43 ++++++++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 8b07be9f..90b26f84 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -2612,6 +2612,18 @@ fn parse_shortcut(sc: &str) -> Option<(egui::Modifiers, egui::Key)> { key.map(|k| (m, k)) } +/// Whether a key press matches a parsed shortcut `m`+`k`: `None` when it doesn't, `Some(true)` for +/// the shortcut's own key and modifiers, `Some(false)` for the Plus key standing in for `=`. On +/// layouts with a dedicated + key (German) or Shift+= (US), Cmd+= arrives as Plus. +fn key_matches(m: egui::Modifiers, k: egui::Key, key: egui::Key, modifiers: egui::Modifiers) -> Option { + let plus_for_equals = key == egui::Key::Plus; + let matches = (k == key || (plus_for_equals && k == egui::Key::Equals)) + && m.command == modifiers.command + && (m.shift == modifiers.shift || (plus_for_equals && !m.shift)) + && m.alt == modifiers.alt; + matches.then_some(k == key && m.shift == modifiers.shift) +} + /// Global keyboard shortcuts: menu commands and single-key tool shortcuts. pub fn shortcuts(app: &mut DesignApp, ctx: &egui::Context) { // Only a focused text field takes the keys: the canvas (or a button) having focus after a click @@ -2639,17 +2651,12 @@ pub fn shortcuts(app: &mut DesignApp, ctx: &egui::Context) { continue; } // Command shortcuts. - let mut fired: Option = None; + let mut fired: Option<(String, bool)> = None; let all = effective_shortcuts(app); for (id, sc) in &all { let id = id.as_str(); - // On layouts with a dedicated + key (German) or Shift+= (US), Cmd+= arrives as Plus. - let plus_for_equals = key == egui::Key::Plus; if let Some((m, k)) = parse_shortcut(sc) - && (k == key || (plus_for_equals && k == egui::Key::Equals)) - && m.command == modifiers.command - && (m.shift == modifiers.shift || (plus_for_equals && !m.shift)) - && m.alt == modifiers.alt + && let Some(exact) = key_matches(m, k, key, modifiers) && (m.command || m.alt || !typing) && !(id == "edit.clear" && typing) { @@ -2660,12 +2667,14 @@ pub fn shortcuts(app: &mut DesignApp, ctx: &egui::Context) { if id == "edit.clear" || (typing && matches!(id, "edit.copy" | "edit.cut" | "edit.paste" | "edit.pasteInPlace")) { continue; // Handled by the active tool / text clipboard events. } - fired = Some(id.to_string()); + fired = Some((id.to_string(), exact)); break; } } - if let Some(id) = fired { - if app.native_shortcuts.contains(&id) && !app.ui.shortcuts.contains_key(&id) { + if let Some((id, exact)) = fired { + // The native menu's accelerator fires only for the shortcut's own key and modifiers; the + // Plus key standing in for = never reaches it. + if exact && app.native_shortcuts.contains(&id) && !app.ui.shortcuts.contains_key(&id) { continue; // The native menu handles it. } // The key press belongs to the shortcut: what it opens (Quick Apply's list, a dialog's @@ -3747,6 +3756,20 @@ mod tests { assert!(if zooms_in { z > z0 } else { z < z0 }, "{k:?} {m:?}: {z0} -> {z}"); assert_eq!(ctx.zoom_factor(), scale, "{k:?} {m:?} leaves the UI scale alone"); } + // With a native menu (macOS) its accelerator takes Cmd+= itself, but never sees the Plus key + // standing in for = (#781). + app.native_shortcuts.insert("view.zoomIn".to_string()); + for (k, m, zooms_in) in [ + (egui::Key::Plus, egui::Modifiers::COMMAND, true), + (egui::Key::Plus, egui::Modifiers::COMMAND | egui::Modifiers::SHIFT, true), + (egui::Key::Equals, egui::Modifiers::COMMAND, false), + ] { + let z0 = app.view().unwrap().zoom; + frame(&mut app, key(k, m)); + frame(&mut app, vec![]); + let z = app.view().unwrap().zoom; + assert_eq!(z > z0, zooms_in, "native menu, {k:?} {m:?}: {z0} -> {z}"); + } } #[test] From 548d700b9f09a8f3e691a4639f615103b6bfefc0 Mon Sep 17 00:00:00 2001 From: aiongg Date: Tue, 13 Oct 2026 20:08:28 +0000 Subject: [PATCH 08/12] Integration: #802 a document that can't be serialized fails Save and Save a Copy instead of writing an empty file; Open Recent's alert names the file without an undocumented param --- crates/engine/src/cmd/file.rs | 20 ++++++++++++++------ crates/ui-egui/src/lib.rs | 6 ++++++ crates/ui-egui/src/menus.rs | 2 +- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index f9c620c8..39c21592 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -59,7 +59,7 @@ pub fn specs() -> Vec { let path = with_document_extension(path); let mut copy = (*s.doc()?.doc).clone(); super::datamerge::refresh_relative_paths(&mut copy, std::path::Path::new(&path)); - let bytes = to_bytes_with_preview(©, &s.cache); + let bytes = to_bytes_with_preview(©, &s.cache)?; #[cfg(not(target_arch = "wasm32"))] crate::atomic::write_file(&path, &bytes)?; Ok(json!({"path": path, "bytes": bytes.len()})) @@ -149,11 +149,16 @@ thread_local! { /// The document as `.designcraft` bytes, or an error when it can't be written; never an empty file. pub fn to_bytes(d: &Document) -> Result> { + save_bytes(d, None) +} + +/// [`to_bytes`] with `preview` (PNG bytes) stored as the file's first-page picture. +fn save_bytes(d: &Document, preview: Option<&[u8]>) -> Result> { #[cfg(test)] if FAIL_TO_BYTES.with(std::cell::Cell::get) { return Err(EngineError::Other("could not write the document: injected failure".into())); } - designcraft_format::save(d).map_err(|e| EngineError::Other(format!("could not write the document: {e}"))) + designcraft_format::save_with_preview(d, preview).map_err(|e| EngineError::Other(format!("could not write the document: {e}"))) } /// `path` ending in `.designcraft`: the extension is added when it is missing or another one, so @@ -177,7 +182,7 @@ const PREVIEW_PX: f64 = 320.0; /// [`to_bytes`] with a picture of the first page, which the start screen shows next to the file /// in its recent files. A page that can't be drawn just leaves the picture out. -fn to_bytes_with_preview(d: &Document, cache: &crate::Cache) -> Vec { +fn to_bytes_with_preview(d: &Document, cache: &crate::Cache) -> Result> { let preview = d.page_loc(0).and_then(|(si, pi)| { let b = d.spreads.get(si)?.pages.get(pi)?.bounds(); let side = b.width().max(b.height()); @@ -185,7 +190,7 @@ fn to_bytes_with_preview(d: &Document, cache: &crate::Cache) -> Vec { }); let png = preview.and_then(|scale| designcraft_render::Renderer::new().render_page(d, cache, 0, scale, false, &Default::default())).map(|r| r.to_png()); - designcraft_format::save_with_preview(d, png.as_deref()).unwrap_or_default() + save_bytes(d, png.as_deref()) } pub fn from_bytes(b: &[u8]) -> Result { @@ -391,7 +396,6 @@ fn has_recent(s: &Session) -> std::result::Result<(), String> { if s.prefs.recent_files.is_empty() { Err("no recent files".into()) } else { Ok(()) } } -/// One File › Open Recent row: the path, its file name and whether it is still there. /// A recent file's first-page picture as straight RGBA, for the start screen. pub struct RecentPreview { pub width: u32, @@ -417,6 +421,7 @@ pub fn recent_preview(path: &str) -> Option { } } +/// One File › Open Recent row: the path, its file name and whether it is still there. fn recent_entry(path: &str) -> Value { let name = std::path::Path::new(path).file_name().map_or_else(|| path.to_string(), |n| n.to_string_lossy().to_string()); #[cfg(not(target_arch = "wasm32"))] @@ -549,7 +554,7 @@ fn file_save(s: &mut Session, p: &Value) -> Result { super::datamerge::refresh_relative_paths(&mut saved, std::path::Path::new(&path)); let paths_changed = saved.data_merge != st.doc.data_merge; let saved = if paths_changed { Arc::new(saved) } else { st.doc.clone() }; - let bytes = to_bytes_with_preview(&saved, &s.cache); + let bytes = to_bytes_with_preview(&saved, &s.cache)?; #[cfg(not(target_arch = "wasm32"))] crate::atomic::write_file(&path, &bytes)?; let st = s.doc_mut()?; @@ -1574,6 +1579,7 @@ mod save_tests { super::FAIL_TO_BYTES.with(|f| f.set(true)); let save = s.execute("file.save", &json!({})); let copy = s.execute("file.saveACopy", &json!({"path": dir.join("copy.designcraft").to_string_lossy()})); + let save_as = s.execute("file.saveAs", &json!({"path": dir.join("other.designcraft").to_string_lossy()})); let serialized = s.execute("file.serialize", &json!({})); super::FAIL_TO_BYTES.with(|f| f.set(false)); @@ -1582,6 +1588,8 @@ mod save_tests { assert!(copy.is_err() && serialized.is_err(), "no empty copy and no empty download"); assert_eq!(std::fs::read(&path).unwrap(), before, "the saved file is untouched"); assert!(!dir.join("copy.designcraft").exists()); + assert!(save_as.is_err() && !dir.join("other.designcraft").exists()); + assert!(!s.prefs.recent_files.iter().any(|p| p.ends_with("other.designcraft")), "a failed Save As isn't a recent file"); assert!(s.doc().unwrap().is_dirty(), "the document still has unsaved changes"); assert!(checkpoint.exists(), "the recovery checkpoint stays"); let _ = std::fs::remove_dir_all(&dir); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 22b9ba46..c877bfc6 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -1285,6 +1285,12 @@ impl DesignApp { Some(path) => std::path::Path::new(path).file_name().map_or_else(|| path.to_string(), |n| n.to_string_lossy().to_string()), None => params.get("name").and_then(Value::as_str).unwrap_or_default().to_string(), }; + self.open_named_file(cmd, params, file) + } + + /// [`Self::open_file`] for a command whose params don't name the file: `file` is shown in the + /// alert or warnings. + pub fn open_named_file(&mut self, cmd: &str, params: Value, file: String) -> Result { let r = self.run(cmd, params); if let Err(e) = &r { let title = if cmd == "file.place" { "Can't Place the File" } else { "Can't Open the File" }; diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index e0af5bed..388ff4ad 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -860,7 +860,7 @@ pub fn run_ui(app: &mut DesignApp, id: &str, p: &Value) -> Option app.pick_and_open("place"), "app.placeWithOptions" => { From acb63fe198e378d51b7f2ae5c0f33ef1ef655abe Mon Sep 17 00:00:00 2001 From: aiongg Date: Tue, 13 Oct 2026 20:12:43 +0000 Subject: [PATCH 09/12] Integration: #802 the start screen and Open Recent menu look recent files up on disk at most every 3 s, not every frame --- crates/ui-egui/src/chrome.rs | 54 ++++++++++++++++++++++++++++++++++-- crates/ui-egui/src/menus.rs | 2 +- 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/crates/ui-egui/src/chrome.rs b/crates/ui-egui/src/chrome.rs index c490895e..f4423f25 100644 --- a/crates/ui-egui/src/chrome.rs +++ b/crates/ui-egui/src/chrome.rs @@ -739,8 +739,9 @@ fn recent_files(app: &mut DesignApp, ui: &mut egui::Ui) { let mut open = None; ui.horizontal_wrapped(|ui| { for (i, path) in rows { - let modified = std::fs::metadata(&path).ok().filter(|m| m.is_file()).map(|m| m.modified().ok()); - let there = modified.is_some(); + let stat = recent_file_stat(ui.ctx(), &path); + let there = stat.exists; + let modified = there.then_some(stat.modified); let (cr, resp) = ui.allocate_exact_size(vec2(140.0, 170.0), Sense::click()); let resp = resp.on_hover_text(&path); ui.painter().rect_filled(cr, 6.0, if resp.hovered() { t.hover } else { t.panel }); @@ -778,7 +779,38 @@ fn recent_files(app: &mut DesignApp, ui: &mut egui::Ui) { } } -/// The texture of a recent file's saved first-page picture, read again when the file changes. +/// How often a recent file is looked up on disk again, in ms: the start screen and the Open Recent +/// menu draw every frame and must not touch the file system each time. +const RECENT_STAT_MS: f64 = 3000.0; + +/// A recent file as last seen on disk. +#[derive(Clone, Debug, PartialEq)] +pub(crate) struct RecentStat { + pub exists: bool, + pub modified: Option, + checked: f64, +} + +/// Whether recent file `path` is there and when it last changed, looked up again at most every +/// [`RECENT_STAT_MS`]. +pub(crate) fn recent_file_stat(ctx: &egui::Context, path: &str) -> RecentStat { + recent_file_stat_at(ctx, path, crate::now_ms()) +} + +fn recent_file_stat_at(ctx: &egui::Context, path: &str, now: f64) -> RecentStat { + let key = egui::Id::new(("recent_stat", path)); + if let Some(st) = ctx.data(|d| d.get_temp::(key)) + && (0.0..RECENT_STAT_MS).contains(&(now - st.checked)) + { + return st; + } + let meta = std::fs::metadata(path).ok().filter(std::fs::Metadata::is_file); + let st = RecentStat { exists: meta.is_some(), modified: meta.and_then(|m| m.modified().ok()), checked: now }; + ctx.data_mut(|d| d.insert_temp(key, st.clone())); + st +} + +/// The texture of a recent file's saved first-page picture, decoded once per file version. fn recent_preview_texture(ctx: &egui::Context, path: &str, modified: Option) -> Option { type Cached = (Option, Option); let key = egui::Id::new(("recent_preview", path)); @@ -858,6 +890,22 @@ mod tests { const LAST_CONTROL: &str = "Align bottom edges"; const FIRST_CONTROL: &str = "Rotate 90° Counterclockwise"; + /// The start screen and Open Recent menu look a recent file up on disk at most every few + /// seconds, not every frame. + #[test] + fn recent_files_are_looked_up_on_disk_every_few_seconds() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("a.designcraft"); + std::fs::write(&path, b"x").unwrap(); + let path = path.to_string_lossy().to_string(); + let ctx = egui::Context::default(); + let first = super::recent_file_stat_at(&ctx, &path, 10_000.0); + assert!(first.exists && first.modified.is_some()); + std::fs::remove_file(&path).unwrap(); + assert_eq!(super::recent_file_stat_at(&ctx, &path, 10_000.0 + super::RECENT_STAT_MS - 1.0), first, "cached"); + assert!(!super::recent_file_stat_at(&ctx, &path, 10_000.0 + super::RECENT_STAT_MS).exists, "looked up again"); + } + fn app() -> crate::DesignApp { let mut app = crate::DesignApp::new(designcraft_engine::Session::new(), crate::Services::default()); app.ui.control_bar = true; diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 388ff4ad..8438ab5e 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -2615,7 +2615,7 @@ fn recent_menu_items(app: &mut DesignApp, ui: &mut egui::Ui) { Item::Cmd { label, id, params, .. } => { let path = params["index"].as_u64().and_then(|i| app.session.prefs.recent_files.get(i as usize)).cloned().unwrap_or_default(); let mut text = egui::RichText::new(label); - if !std::path::Path::new(&path).is_file() { + if !crate::chrome::recent_file_stat(ui.ctx(), &path).exists { text = text.color(dim); } if ui.button(crate::rtl::widget(ui, text)).on_hover_text(&path).clicked() { From d03b17ef56269a81854ae84cd92cbbb32e777ace Mon Sep 17 00:00:00 2001 From: aiongg Date: Tue, 13 Oct 2026 20:15:59 +0000 Subject: [PATCH 10/12] Integration: #803 guide.add, layout.marginsAndColumns, layout.numberingOptions and file.savePreset describe every param they take --- crates/engine/src/cmd/file.rs | 2 +- crates/engine/src/cmd/guides.rs | 2 +- crates/engine/src/cmd/layout.rs | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 00d85383..82fc9209 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -27,7 +27,7 @@ pub fn specs() -> Vec { Ok(Value::Array(presets)) }), cmd!(noundo "file.savePreset", "Save Document Preset", [], None, - "{name, …the settings file.new takes} — kept with the preferences; `file.new {preset: name}` uses it. A saved preset of that name is replaced; built-in names are refused", + "{name, …the settings as file.new} — kept with the preferences; `file.new {preset: name}` uses it. A saved preset of that name is replaced; built-in names are refused", always, save_preset), cmd!(noundo "file.deletePreset", "Delete Document Preset", [], None, "{name} — a saved document preset", always, |s, p| { let name = str_param(p, "name").unwrap_or_default().trim().to_string(); diff --git a/crates/engine/src/cmd/guides.rs b/crates/engine/src/cmd/guides.rs index b2bc378a..6acf6a15 100644 --- a/crates/engine/src/cmd/guides.rs +++ b/crates/engine/src/cmd/guides.rs @@ -21,7 +21,7 @@ pub fn specs() -> Vec { "New Guide", [], None, - "{orientation: horizontal|vertical, position (spread coordinate), spread?, page? (index in the spread; default: the page under `at` or the position), at? (the other coordinate), spreadGuide?: bool} → {page, index}; {orientation: both, point: [x, y], …} adds a horizontal guide through y and a vertical guide through x as one undo step → {guides: [{page, index}, {page, index}]}", + "{orientation: horizontal|vertical, position (spread coordinate), spread?, page? (index in the spread; default: the page under `at` or the position), at? (the other coordinate), spreadGuide?: bool} or {orientation: both, point: [x, y], spread?, page?, spreadGuide?: bool} → {page, index}; `both` adds a horizontal guide through y and a vertical guide through x as one undo step → {guides: [{page, index}, {page, index}]}", has_doc, add ), diff --git a/crates/engine/src/cmd/layout.rs b/crates/engine/src/cmd/layout.rs index 2761b17e..2468faef 100644 --- a/crates/engine/src/cmd/layout.rs +++ b/crates/engine/src/cmd/layout.rs @@ -246,7 +246,7 @@ pub fn specs() -> Vec { "Margins and Columns…", ["Layout"], None, - "{pages?: [index] (default all), margins?: number|{top,bottom,inside,outside}, columns?, gutter?, adjustLayout?: bool (objects follow the margins)} — document pages take these as their own margins; with `parent` (a parent's prefix, label or index) the parent's pages change instead (`pages` then indexes them), and so do the document pages without their own", + "{pages?: [index] (default all), margins?: number|{top,bottom,inside,outside}, columns?, gutter?, adjustLayout?: bool (objects follow the margins), parent?: a parent's prefix, label or index} — document pages take these as their own margins; with `parent` the parent's pages change instead (`pages` then indexes them), and so do the document pages without their own", has_doc, |s, p| { let pages: Option> = @@ -508,7 +508,7 @@ pub fn specs() -> Vec { "Apply Numbering & Section Options", [], None, - "{page, …layout.section's settings, chapter?: {…layout.chapterNumbering's settings}} — the section and the document's chapter numbering in one undo step; if either is refused nothing changes", + "{page, …the settings as layout.section, chapter?: {…the settings as layout.chapterNumbering}} — the section and the document's chapter numbering in one undo step; if either is refused nothing changes", has_doc, |s, p| { let section = SectionEdit::parse("layout.numberingOptions", s, p)?; From 0278f6bbea0a2d2f98a1f91af7c9a4eba0f04a4c Mon Sep 17 00:00:00 2001 From: aiongg Date: Tue, 13 Oct 2026 20:16:12 +0000 Subject: [PATCH 11/12] Integration: cargo fmt --- apps/designcraft/src/native_menu.rs | 9 +-------- crates/engine/src/cmd/mod.rs | 2 +- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/apps/designcraft/src/native_menu.rs b/apps/designcraft/src/native_menu.rs index 3ab86d3f..27e0ade9 100644 --- a/apps/designcraft/src/native_menu.rs +++ b/apps/designcraft/src/native_menu.rs @@ -99,14 +99,7 @@ impl NativeMenu { }) .collect(); let recent = recent.map(|submenu| RecentMenu { submenu, shown: Vec::new(), ids: Vec::new(), separator: None }); - Self { - _menu: menu, - items, - events, - last_refresh: 0.0, - shown_for: (app.ui.language.clone(), designcraft_ui_egui::cjk_features(app)), - recent, - } + Self { _menu: menu, items, events, last_refresh: 0.0, shown_for: (app.ui.language.clone(), designcraft_ui_egui::cjk_features(app)), recent } } /// Put the current recent files at the top of File › Open Recent when they changed. diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index 6c35016f..faa13d58 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -35,8 +35,8 @@ mod notes; mod object; mod overrides; mod package; -mod paste_svg; pub mod params; +mod paste_svg; mod path_type; mod paths; mod pdflayers; From da8a3daa8c5ef45ad7368f2977a71ead158d0527 Mon Sep 17 00:00:00 2001 From: aiongg Date: Tue, 13 Oct 2026 20:26:51 +0000 Subject: [PATCH 12/12] Integration: #803 layout.marginsAndColumns' parent note in parentheses, so its description parses --- crates/engine/src/cmd/layout.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/layout.rs b/crates/engine/src/cmd/layout.rs index 2468faef..fd9691f3 100644 --- a/crates/engine/src/cmd/layout.rs +++ b/crates/engine/src/cmd/layout.rs @@ -246,7 +246,7 @@ pub fn specs() -> Vec { "Margins and Columns…", ["Layout"], None, - "{pages?: [index] (default all), margins?: number|{top,bottom,inside,outside}, columns?, gutter?, adjustLayout?: bool (objects follow the margins), parent?: a parent's prefix, label or index} — document pages take these as their own margins; with `parent` the parent's pages change instead (`pages` then indexes them), and so do the document pages without their own", + "{pages?: [index] (default all), margins?: number|{top,bottom,inside,outside}, columns?, gutter?, adjustLayout?: bool (objects follow the margins), parent? (a parent's prefix, label or index)} — document pages take these as their own margins; with `parent` the parent's pages change instead (`pages` then indexes them), and so do the document pages without their own", has_doc, |s, p| { let pages: Option> =