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 <noreply@anthropic.com>

* 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 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

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;
             }
         }

crates/ui-egui/src/menus.rs

@@ -54,7 +54,11 @@ pub fn run_ui_command(app: &mut CadApp, id: &str, params: &Value) -> Option<Resu
     let toggle = |b: &mut bool, p: &Value| {
         *b = p.get("on").and_then(Value::as_bool).unwrap_or(!*b);
     };
-    let no_path = params.is_null() || (params.get("path").is_none() && params.get("data").is_none());
+    // When a file command should ask for a file. The `ui.*` commands are the pickers; the engine's
+    // OPEN/SAVEAS/QSAVE ask only when typed or chosen from a menu (no parameters), so JSON calls
+    // never open a picker.
+    let no_path =
+        if id.starts_with("ui.") { params.is_null() || (params.get("path").is_none() && params.get("data").is_none()) } else { params.is_null() };
     let r = match id {
         "ui.open" | "open" if no_path => {
             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<Resu
             }
             Ok(Value::Null)
         }
+        "dsettings" | "ds" | "se" if params.is_null() => {
+            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)

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<Cell<u32>>) {
+    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);
+}