From 8fe9baddb5cff4674a016204bdd8e20c0f9894ca Mon Sep 17 00:00:00 2001 From: kaffeochknivar Date: Sat, 10 Oct 2026 18:16:48 +0200 Subject: [PATCH] Menu commands open their dialogs; JSON calls never open file pickers (fixes #65, fixes #101) (#125) * Render tests: drop the always-true assert in degenerate_input `assert!(... || true)` trips clippy's overly_complex_bool_expr on current stable (1.96), which fails `cargo xtask ci` at the clippy step. The check only proves that NaN input does not panic, so call the function and discard the result instead. Co-Authored-By: Claude Opus 5.5 * Menu commands open their dialogs; JSON calls never open pickers (#65, #101) Tools > Drafting Settings... (DSETTINGS, DS, SE) ran the JSON form with no parameters, which changed nothing, so the menu item looked dead; only the status-bar gear opened the dialog. Typed or menu-invoked DSETTINGS now opens the Drafting Settings dialog, and PROPERTIES (PR, CH) shows the Properties palette. The engine's OPEN, SAVEAS and QSAVE opened a native file picker whenever their parameters had no path, so `engine.execute` and the bridged MCP `execute` tool could block on a desktop dialog. They now ask for a file only when typed or chosen from a menu; JSON calls get the command's own missing-path error. The ui.open / ui.saveas commands (Cmd+O, Cmd+Shift+S) still open the pickers. Typing the `cmd {json}` form at the command line (e.g. `layer {}`) no longer opens the command's dialog or picker; the engine runs it with those parameters. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- crates/ui-egui/src/lib.rs | 4 +- crates/ui-egui/src/menus.rs | 14 ++++- crates/ui-egui/tests/dialog_dispatch.rs | 82 +++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 2 deletions(-) create mode 100644 crates/ui-egui/tests/dialog_dispatch.rs diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index d6ee9d7..59cc389 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -188,7 +188,9 @@ impl CadApp { pub fn cmdline(&mut self, text: &str) { if self.session.running.is_none() { let first = text.split_whitespace().next().unwrap_or(""); - if !first.is_empty() && menus::run_ui_command(self, &first.to_ascii_lowercase(), &Value::Null).is_some() { + // `cmd {json}` is a programmatic call: the engine runs it with those parameters. + let json_form = text.trim_start().get(first.len()..).is_some_and(|rest| rest.trim_start().starts_with('{')); + if !first.is_empty() && !json_form && menus::run_ui_command(self, &first.to_ascii_lowercase(), &Value::Null).is_some() { return; } } diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index ba04a88..540d633 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -54,7 +54,11 @@ pub fn run_ui_command(app: &mut CadApp, id: &str, params: &Value) -> Option { let picked = app.services.pick_open.as_ref().and_then(|f| f()); @@ -201,6 +205,14 @@ pub fn run_ui_command(app: &mut CadApp, id: &str, params: &Value) -> Option { + app.ui.dialog = Some("dsettings".into()); + Ok(Value::Null) + } + "properties" | "pr" | "props" | "ch" if params.is_null() => { + app.ui.show_palettes = true; + Ok(Value::Null) + } _ => return None, }; Some(r) diff --git a/crates/ui-egui/tests/dialog_dispatch.rs b/crates/ui-egui/tests/dialog_dispatch.rs new file mode 100644 index 0000000..5706a0e --- /dev/null +++ b/crates/ui-egui/tests/dialog_dispatch.rs @@ -0,0 +1,82 @@ +//! Which calls open dialogs and file pickers: typed and menu-invoked commands do, JSON calls never. + +use std::cell::Cell; +use std::rc::Rc; + +use cadcraft_engine::Session; +use cadcraft_ui_egui::{CadApp, Services, menus}; +use serde_json::json; + +/// An app whose file pickers count their calls and cancel. +fn app() -> (CadApp, Rc>) { + let picks = Rc::new(Cell::new(0)); + let (a, b) = (picks.clone(), picks.clone()); + let services = Services { + pick_open: Some(Box::new(move || { + a.set(a.get() + 1); + None + })), + pick_save: Some(Box::new(move |_| { + b.set(b.get() + 1); + None + })), + }; + (CadApp::new(Session::new(), services), picks) +} + +#[test] +fn drafting_settings_menu_opens_the_dialog() { + let (mut app, _) = app(); + menus::activate(&mut app, "dsettings"); + assert_eq!(app.ui.dialog.as_deref(), Some("dsettings")); + app.ui.dialog = None; + app.cmdline("DS"); + assert_eq!(app.ui.dialog.as_deref(), Some("dsettings")); + app.ui.dialog = None; + app.run("dsettings", json!({ "gridmajor": 10 })).unwrap(); + assert!(app.ui.dialog.is_none()); + assert_eq!(app.session.settings.gridmajor, 10); +} + +#[test] +fn properties_menu_shows_the_palette() { + let (mut app, _) = app(); + app.ui.show_palettes = false; + app.run("properties", json!({})).unwrap(); + assert!(!app.ui.show_palettes, "JSON calls only read properties"); + menus::activate(&mut app, "properties"); + assert!(app.ui.show_palettes); +} + +#[test] +fn json_file_commands_never_open_pickers() { + let (mut app, picks) = app(); + for c in ["open", "saveas", "qsave"] { + assert!(app.run(c, json!({})).is_err(), "{c} {{}} reports the missing path"); + } + app.cmdline("open {}"); + assert_eq!(picks.get(), 0); + // Typed, chosen from the menu or the ui.* shortcut commands: the pickers. + for c in ["open", "saveas", "qsave"] { + app.start(c); + } + assert_eq!(picks.get(), 3); + app.run("ui.open", json!({})).unwrap(); + app.run("ui.saveas", json!({})).unwrap(); + assert_eq!(picks.get(), 5); +} + +#[test] +fn typed_json_form_runs_the_command() { + let (mut app, _) = app(); + app.cmdline("LAYER"); + assert_eq!(app.ui.dialog.as_deref(), Some("layers")); + app.ui.dialog = None; + let n = app.session.log.len(); + app.cmdline("layer {}"); + assert!(app.ui.dialog.is_none()); + assert!(app.session.log[n..].iter().any(|l| l.contains("\"layers\"")), "the layer list is echoed"); + app.cmdline(r#"dsettings {"gridmajor": 7}"#); + assert!(app.ui.dialog.is_none()); + assert_eq!(app.session.settings.gridmajor, 7); +}