From 1bbced51a499d5b07bcf8ca594845fc945fc8d11 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sun, 1 Nov 2026 13:53:50 +0100 Subject: [PATCH] Recent list: saved is null when no preferences were written The desktop hook returns Ok(false) when preferences are switched off or there is no settings folder; the commands report that like a host without a hook. Also move add_saved out of works_without_document's doc comment. Co-Authored-By: Claude Sonnet 5.5 --- apps/wordcraft/src/main.rs | 13 +++++++------ crates/ui-egui/src/app/commands.rs | 14 +++++++------- crates/ui-egui/src/app/frame.rs | 14 +++++++++----- crates/ui-egui/src/services.rs | 8 +++++--- crates/ui-egui/src/tests.rs | 18 +++++++++++------- docs/control-protocol.md | 2 +- 6 files changed, 40 insertions(+), 29 deletions(-) diff --git a/apps/wordcraft/src/main.rs b/apps/wordcraft/src/main.rs index 755ba292ef..5ac5c7ee54 100644 --- a/apps/wordcraft/src/main.rs +++ b/apps/wordcraft/src/main.rs @@ -231,18 +231,19 @@ fn save_prefs(app: &WordApp) { } } -/// Saves the preferences and says why when it could not. Nothing to write (prefs disabled or no -/// settings folder) counts as success. -fn try_save_prefs(app: &WordApp) -> Result<(), String> { +/// Saves the preferences and says why when it could not. `Ok(false)`: nothing was written +/// because preferences are switched off or there is no settings folder. +fn try_save_prefs(app: &WordApp) -> Result { if !prefs_enabled() { - return Ok(()); + return Ok(false); } - let Some(p) = prefs_path() else { return Ok(()) }; + let Some(p) = prefs_path() else { return Ok(false) }; if let Some(dir) = p.parent() { std::fs::create_dir_all(dir).map_err(|e| format!("cannot create {}: {e}", dir.display()))?; } let bytes = serde_json::to_vec_pretty(&app.prefs()).map_err(|e| format!("cannot serialize: {e}"))?; - write_prefs(&p, &bytes).map_err(|e| format!("cannot save {}: {e}", p.display())) + write_prefs(&p, &bytes).map_err(|e| format!("cannot save {}: {e}", p.display()))?; + Ok(true) } /// Writes `bytes` to a sibling temporary file and renames it over `path`, so a failed or diff --git a/crates/ui-egui/src/app/commands.rs b/crates/ui-egui/src/app/commands.rs index 40336b2401..e41d4aed4a 100644 --- a/crates/ui-egui/src/app/commands.rs +++ b/crates/ui-egui/src/app/commands.rs @@ -1027,15 +1027,9 @@ fn input_dialog_other(id: &str, params: &Value) -> Option<&'static str> { } } -/// Commands that work while no document is open (#1129): opening or creating one, closing the -/// window, Help, adding fonts (a font file dropped on the empty window), and the settings (File › -/// Options, the view and interface preferences it sets, AutoCorrect Options, Customize Ribbon and -/// Keyboard). -/// Everything else needs a document, `ui.*` included: only the interface commands listed here -/// (tabs, Backstage, dialogs, appearance, opening files) work without one. /// The result fields of a command that saves the preferences at once: `saved` is true when the /// write worked, false with `saveError` when it failed (the change still holds for this run) and -/// null when the host has no preferences hook. +/// null when the host has no preferences hook or wrote nothing. fn add_saved(r: &mut Value, saved: Option>) { r["saved"] = match &saved { None => Value::Null, @@ -1046,6 +1040,12 @@ fn add_saved(r: &mut Value, saved: Option>) { } } +/// Commands that work while no document is open (#1129): opening or creating one, closing the +/// window, Help, adding fonts (a font file dropped on the empty window), and the settings (File › +/// Options, the view and interface preferences it sets, AutoCorrect Options, Customize Ribbon and +/// Keyboard). +/// Everything else needs a document, `ui.*` included: only the interface commands listed here +/// (tabs, Backstage, dialogs, appearance, opening files) work without one. pub(crate) fn works_without_document(id: &str) -> bool { matches!( id, diff --git a/crates/ui-egui/src/app/frame.rs b/crates/ui-egui/src/app/frame.rs index 97fe85e91d..b7dfc4df04 100644 --- a/crates/ui-egui/src/app/frame.rs +++ b/crates/ui-egui/src/app/frame.rs @@ -24,14 +24,18 @@ impl WordApp { /// Write the current preferences out through the host's hook, if it has one (#2716). The /// desktop app saves them at exit; a change that should survive a crash, like taking a /// document off the recent list, needs a save at the moment of the change. `None`: the host - /// has no hook; `Some(Err)`: the write failed, and the status line says so. + /// has no hook or wrote nothing; `Some(Err)`: the write failed, and the status line says so. pub(super) fn save_prefs_now(&mut self) -> Option> { let save = self.services.save_prefs.as_ref()?; - let res = save(self); - if let Err(e) = &res { - self.status(i18n::fmt(tl!("Could not save the preferences: {error}"), &[("error", e)])); + match save(self) { + Ok(true) => Some(Ok(())), + // The host wrote nothing (preferences switched off): same as having no hook. + Ok(false) => None, + Err(e) => { + self.status(i18n::fmt(tl!("Could not save the preferences: {error}"), &[("error", &e)])); + Some(Err(e)) + } } - Some(res) } /// Document title for the title bar (and the suggested file name). An untitled document is diff --git a/crates/ui-egui/src/services.rs b/crates/ui-egui/src/services.rs index 40eb1efa21..53a0df09b0 100644 --- a/crates/ui-egui/src/services.rs +++ b/crates/ui-egui/src/services.rs @@ -90,9 +90,11 @@ pub struct Services { pub make_default_app: Option Result>>, /// Desktop: write the app's current preferences out now, not only at exit (#2716): a /// document taken off the recent list should survive the app being killed before a normal - /// close. An error is the reason the write failed; the recent commands report it. `None` - /// (web, tests) keeps the change to this run, and the commands then report `saved: null`. - pub save_prefs: Option Result<(), String>>>, + /// close. `Ok(true)`: written; `Ok(false)`: nothing written because the host keeps no + /// preferences (switched off, no settings folder); an error is the reason the write failed. + /// The recent commands report these as `saved` true / null / false. `None` (web, tests) keeps + /// the change to this run and reports `saved: null`. + pub save_prefs: Option Result>>, /// Desktop: the app's own user templates folder (`Templates` in the settings folder, #2852), /// used while File › Options names none. File › New lists personal templates from it and Save /// as Word Template starts in it. `None` (web) hides personal templates. diff --git a/crates/ui-egui/src/tests.rs b/crates/ui-egui/src/tests.rs index eff225a92b..f8631b2d34 100644 --- a/crates/ui-egui/src/tests.rs +++ b/crates/ui-egui/src/tests.rs @@ -7199,7 +7199,7 @@ fn recent_list_edits_are_final_and_persist() { a.services.save_prefs = Some(Box::new(move |app: &WordApp| { saved_hook.set(saved_hook.get() + 1); let _ = serde_json::to_string(&app.prefs()).unwrap(); - Ok(()) + Ok(true) })); a.ui.recent = vec!["/tmp/alpha.docx".into(), "/tmp/beta.docx".into()]; // The open document is on the list, at the back. @@ -7251,11 +7251,11 @@ fn recent_list_edits_are_final_and_persist() { /// Issue #2716: when the immediate preferences write fails, both recent commands say so (result /// and status line) instead of reporting plain success; the removal still holds for this run. -/// A host with no hook reports `saved: null`. +/// A host with no hook, or one that wrote nothing, reports `saved: null`. #[test] fn recent_list_edits_report_a_failed_preferences_save() { let mut a = app(); - a.services.save_prefs = Some(Box::new(|_: &WordApp| Err("disk full".to_string()))); + a.services.save_prefs = Some(Box::new(|_: &WordApp| Err::("disk full".to_string()))); a.ui.recent = vec!["/tmp/alpha.docx".into(), "/tmp/beta.docx".into()]; let r = a.run("ui.recentRemove", json!({"path": "/tmp/alpha.docx"})).unwrap(); @@ -7270,10 +7270,14 @@ fn recent_list_edits_report_a_failed_preferences_save() { assert_eq!(r["removed"], 1); assert!(a.ui.recent.is_empty()); - a.services.save_prefs = None; - a.ui.recent = vec!["/tmp/alpha.docx".into()]; - let r = a.run("ui.recentClear", json!({})).unwrap(); - assert!(r["saved"].is_null() && r.get("saveError").is_none(), "{r}"); + // No hook, or a hook that wrote nothing (preferences switched off): `saved` is null. + for hook in [None, Some(Box::new(|_: &WordApp| Ok(false)) as Box Result>)] { + a.services.save_prefs = hook; + a.ui.recent = vec!["/tmp/alpha.docx".into()]; + let r = a.run("ui.recentClear", json!({})).unwrap(); + assert!(r["saved"].is_null() && r.get("saveError").is_none(), "{r}"); + assert!(a.status_msg.as_ref().is_some_and(|(m, _)| m.starts_with("Removed")), "{:?}", a.status_msg); + } } /// Issue #2716: taking documents off the list needs no open document — the Start screen shows diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 57045cc103..d2f6ebe059 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -54,7 +54,7 @@ and set `WORDCRAFT_CONTROL_KEY` to that app's key. | Method | Params | Result | |---|---|---| -| `engine.execute` | `command`, `params`, `doc?` | the command's result (any id from `engine.commands`, including UI commands `ui.tab`, `ui.dialog` (`{"name": dialog, "part"?: table style part}`; `part` opens New / Modify Table Style (`newTableStyle` / `modifyTableStyle`) on that part: `wholeTable`, `headerRow`, `firstColumn`, `bandedRows`, `nwCell`… as `table.modifyStyle` names them), `ui.backstage`, `ui.zotero.*` (see `docs/zotero.md`), `ui.language` (`{"value": "auto"\|"en"\|"zh-hans"\|"zh-hant"\|"ja"\|"pt-br"\|"es"\|"uk"\|"sr"\|"sr-latn"\|"et"\|"tr"\|"ru"\|"pl"\|"de"\|"it"\|"cs"\|"hu"\|"fr"}`), `ui.theme` (`{"value": "system"\|"light"\|"dark"}`; `system` follows the OS appearance, light when it reports none), `ui.interfaceSize` (`{"value": "auto"}` or `{"percent": 50–300}`, clamped; no value reads it; `auto` keeps the system display scale, a percentage is absolute, so 100 is one screen pixel per point (#822); the result's `percent` is the size drawn at and `auto` whether it follows the system; document zoom stays `view.zoom`), `ui.animations` (`{"value": "system"\|"on"\|"off"}`; no value reads it; `system` follows the system's animation setting, which the desktop app reads on Windows, macOS, Linux and the BSDs and the web app from `prefers-reduced-motion`, and means on where the system doesn't say; the result's `enabled` is what the setting comes to and `system` what the system reports, `null` when it doesn't say), `ui.fileBrowser` (desktop: `{"purpose": "document"\|"picture"\|"recipients"\|"save"}` opens WordCraft's own file browser, for when the system file dialog doesn't show up, #576), `ui.saveAsTemplate` (File › Save As › Save as Word Template: the Save dialog for a `.dotx`, starting in the user templates folder; the result's `dir` is that folder, `null` on the web, #2852), `ui.caretAnimation` (`{"value": true\|false}`; no value reads it; off, the caret jumps and blinks on and off instead of gliding and fading), `ui.startScreen` (`{"value": true\|false}`; no value reads it; on, the default, the desktop and web apps start on File › Home when launched with nothing to open, #766), `ui.alignmentGuides` (`{"value": true\|false}`; no value reads it; on, the default, moving a picture or shape snaps it to the page, the margins and other objects, with guide lines while dragging; Alt moves freely), `ui.quickAccessToolbar` (`{"value": true\|false}`; no value toggles it; false takes the Quick Access Toolbar out of the title bar, as its menu's Hide Quick Access Toolbar does, and true brings it back, #1945), `ui.recentRemove` (`{"path": entry}` takes that document off the recent list — `path` must be an entry on the list; on the desktop the new list is saved right away: the result's `saved` is true then, false with `saveError` when the write failed (the removal still holds for this run) and null where the host keeps no preferences, #2716), `ui.recentClear` (`{}` clears the whole list; the result reports `removed`, the new `recent` and `saved` as above), `ui.focusRibbon` (`{"value": true\|false}`; no value toggles it; in Focus, true drops the ribbon down over the page, as the bar's ⋯ does, and false puts it away, as a click on the page does; outside Focus it stays put away, #1678), `ui.contextMenu` (opens the page's context menu below the caret, as Shift+F10 does), `ui.paneDock` (`{"pane": "navigation"\|"clipboard"\|"styles"\|"styleInspector"\|"accessibility"\|"altText"\|"formatShape"\|"restrictEditing"\|"comments"\|"smartArtText", "dock"?: "left"\|"right"\|"float", "before"?: pane\|null}`; no `dock` reads it; docks or floats a task pane as its menu does, kept between runs; Styles is `view.stylesPaneDock`, #1756; `before` puts the pane directly before that pane in the order, outside it on their edge, as a drop over a docked pane's outer third does, and `null` after every pane; the order is kept between runs)…) | +| `engine.execute` | `command`, `params`, `doc?` | the command's result (any id from `engine.commands`, including UI commands `ui.tab`, `ui.dialog` (`{"name": dialog, "part"?: table style part}`; `part` opens New / Modify Table Style (`newTableStyle` / `modifyTableStyle`) on that part: `wholeTable`, `headerRow`, `firstColumn`, `bandedRows`, `nwCell`… as `table.modifyStyle` names them), `ui.backstage`, `ui.zotero.*` (see `docs/zotero.md`), `ui.language` (`{"value": "auto"\|"en"\|"zh-hans"\|"zh-hant"\|"ja"\|"pt-br"\|"es"\|"uk"\|"sr"\|"sr-latn"\|"et"\|"tr"\|"ru"\|"pl"\|"de"\|"it"\|"cs"\|"hu"\|"fr"}`), `ui.theme` (`{"value": "system"\|"light"\|"dark"}`; `system` follows the OS appearance, light when it reports none), `ui.interfaceSize` (`{"value": "auto"}` or `{"percent": 50–300}`, clamped; no value reads it; `auto` keeps the system display scale, a percentage is absolute, so 100 is one screen pixel per point (#822); the result's `percent` is the size drawn at and `auto` whether it follows the system; document zoom stays `view.zoom`), `ui.animations` (`{"value": "system"\|"on"\|"off"}`; no value reads it; `system` follows the system's animation setting, which the desktop app reads on Windows, macOS, Linux and the BSDs and the web app from `prefers-reduced-motion`, and means on where the system doesn't say; the result's `enabled` is what the setting comes to and `system` what the system reports, `null` when it doesn't say), `ui.fileBrowser` (desktop: `{"purpose": "document"\|"picture"\|"recipients"\|"save"}` opens WordCraft's own file browser, for when the system file dialog doesn't show up, #576), `ui.saveAsTemplate` (File › Save As › Save as Word Template: the Save dialog for a `.dotx`, starting in the user templates folder; the result's `dir` is that folder, `null` on the web, #2852), `ui.caretAnimation` (`{"value": true\|false}`; no value reads it; off, the caret jumps and blinks on and off instead of gliding and fading), `ui.startScreen` (`{"value": true\|false}`; no value reads it; on, the default, the desktop and web apps start on File › Home when launched with nothing to open, #766), `ui.alignmentGuides` (`{"value": true\|false}`; no value reads it; on, the default, moving a picture or shape snaps it to the page, the margins and other objects, with guide lines while dragging; Alt moves freely), `ui.quickAccessToolbar` (`{"value": true\|false}`; no value toggles it; false takes the Quick Access Toolbar out of the title bar, as its menu's Hide Quick Access Toolbar does, and true brings it back, #1945), `ui.recentRemove` (`{"path": entry}` takes that document off the recent list — `path` must be an entry on the list; on the desktop the new list is saved right away: the result's `saved` is true then, false with `saveError` when the write failed (the removal still holds for this run) and null where the host keeps no preferences (the web app, or preferences switched off), #2716), `ui.recentClear` (`{}` clears the whole list; the result reports `removed`, the new `recent` and `saved` as above), `ui.focusRibbon` (`{"value": true\|false}`; no value toggles it; in Focus, true drops the ribbon down over the page, as the bar's ⋯ does, and false puts it away, as a click on the page does; outside Focus it stays put away, #1678), `ui.contextMenu` (opens the page's context menu below the caret, as Shift+F10 does), `ui.paneDock` (`{"pane": "navigation"\|"clipboard"\|"styles"\|"styleInspector"\|"accessibility"\|"altText"\|"formatShape"\|"restrictEditing"\|"comments"\|"smartArtText", "dock"?: "left"\|"right"\|"float", "before"?: pane\|null}`; no `dock` reads it; docks or floats a task pane as its menu does, kept between runs; Styles is `view.stylesPaneDock`, #1756; `before` puts the pane directly before that pane in the order, outside it on their edge, as a drop over a docked pane's outer third does, and `null` after every pane; the order is kept between runs)…) | | `engine.commands` | — | every command: id, label, location, shortcut, params, enabled | | `document.inspect` | `text?`, `doc?` | blocks (text, style, runs, lists, tables), parts, sections, selection, pages | | `ui.inspect` | — | UI state, view, dialog, window size, page rects on screen, caret, comment balloons (`balloons`: id + screen rect; `balloon`: the selected one), perf; `document`: the active document's id; `documents`: the open documents (`id`, `title`, `path`, `dirty`, `active`) |