Selection commands prompt "Select objects" without a pickfirst selection (#126) (#176)

Edit > Cut / Copy / Copy with Base Point, View > Zoom > Object, Dimension >
Update, Tools > Draw Order > Bring to Front / Send to Back and the Format >
Layer Tools LAYCUR, LAYFRZ, LAYOFF, LAYLCK and LAYULK did nothing when started
from a menu, toolbar or the command line with nothing selected.

- machines::SelectRun: "Select objects:", then run the command's JSON body on
  the picked handles. With a pickfirst selection it runs at once, as before.
  It replaces the DIMREASSOCIATE/DIMDISASSOCIATE SelectThen in annotate.rs and
  calls the command body directly, so the command records one undo step.
- LAYFRZ/LAYOFF/LAYLCK/LAYULK pick one object ("Select an object on the layer
  to be frozen" etc.).
- COPYBASE asks for the base point first, then for objects (skipped with a
  pickfirst selection).
- Objects picked at the prompt are deselected afterwards.
- JSON/programmatic calls are unchanged and never prompt.

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

crates/engine/src/cmd/annotate.rs

@@ -7,6 +7,7 @@ use cadcraft_geom::{Segment, Vec2, Vec3};
 use serde_json::{Value, json};
 
 use super::helpers::v3;
+use super::machines::SelectRun;
 use super::*;
 use crate::{Accept, EngineError, Input, Interactive, Prompt, Result, Session, Step};
 
@@ -70,7 +71,10 @@ pub fn specs() -> Vec<CommandSpec> {
             .params("{points: [[arrow], ..., [landing]], text}")
             .interactive(|_| Ok(Box::new(MLeaderM::default()))),
         CommandSpec::new("leader", "Leader", run_leader).alias(&["lead"]).params("{points: [[x,y]...], text?}"),
-        CommandSpec::new("dimstyle.update", "Update", run_update).menu(&["Dimension", "Update"]).params("{handles?}"),
+        CommandSpec::new("dimstyle.update", "Update", run_update)
+            .menu(&["Dimension", "Update"])
+            .params("{handles?}")
+            .interactive(|_| Ok(Box::new(SelectRun::new("dimstyle.update", "DIMSTYLE")))),
         CommandSpec::new("dimoverride", "Override", run_override)
             .menu(&["Dimension", "Override"])
             .alias(&["dov", "dimover"])
@@ -80,11 +84,11 @@ pub fn specs() -> Vec<CommandSpec> {
             .menu(&["Dimension", "Reassociate Dimensions"])
             .alias(&["dre"])
             .params("{handles?} (dimensions; attaches their definition points to the objects under them)")
-            .interactive(|s| Ok(Box::new(SelectThen::new(s, "dimreassociate")))),
+            .interactive(|_| Ok(Box::new(SelectRun::new("dimreassociate", "DIMREASSOCIATE")))),
         CommandSpec::new("dimdisassociate", "Disassociate Dimensions", run_disassociate)
             .alias(&["dda"])
             .params("{handles?}")
-            .interactive(|s| Ok(Box::new(SelectThen::new(s, "dimdisassociate")))),
+            .interactive(|_| Ok(Box::new(SelectRun::new("dimdisassociate", "DIMDISASSOCIATE")))),
         CommandSpec::new("dimtedit.home", "Home", |s, p| run_tedit(s, p, "home")).menu(&["Dimension", "Align Text", "Home"]).params("{handles?}"),
         CommandSpec::new("dimtedit.angle", "Angle", |s, p| run_tedit(s, p, "angle"))
             .menu(&["Dimension", "Align Text", "Angle"])
@@ -668,53 +672,6 @@ fn run_dimspace(s: &mut Session, p: &Value) -> Result<Value> {
     Ok(json!({ "spaced": others.len() }))
 }
 
-/// Select objects, then run a JSON command on them.
-struct SelectThen {
-    sel: super::machines::SelectPhase,
-    id: &'static str,
-}
-
-impl SelectThen {
-    fn new(_s: &Session, id: &'static str) -> Self {
-        SelectThen { sel: super::machines::SelectPhase::default(), id }
-    }
-    fn run(&self, s: &mut Session, hs: Vec<Handle>) -> Result<Step> {
-        let hex: Vec<String> = hs.iter().map(|h| h.hex()).collect();
-        let r = s.execute(self.id, &json!({ "handles": hex }))?;
-        if let Some(m) = r.get("message").and_then(Value::as_str) {
-            s.echo(m.to_string());
-        }
-        Ok(Step::Done)
-    }
-}
-
-impl Interactive for SelectThen {
-    fn name(&self) -> &'static str {
-        if self.id == "dimreassociate" { "DIMREASSOCIATE" } else { "DIMDISASSOCIATE" }
-    }
-    fn begin(&mut self, s: &mut Session) -> Result<Step> {
-        self.sel = super::machines::SelectPhase::begin(s);
-        if self.sel.done {
-            let hs = self.sel.picked.clone();
-            return self.run(s, hs);
-        }
-        Ok(Step::Continue)
-    }
-    fn prompt(&self, _s: &Session) -> Prompt {
-        self.sel.prompt()
-    }
-    fn input(&mut self, s: &mut Session, i: Input) -> Result<Step> {
-        match self.sel.feed(s, &i)? {
-            super::machines::SelOutcome::More => Ok(Step::Continue),
-            super::machines::SelOutcome::Empty => Ok(Step::Done),
-            super::machines::SelOutcome::Done(hs) => self.run(s, hs),
-        }
-    }
-    fn preview(&self, _s: &Session, _c: Vec2) -> Vec<EntityKind> {
-        Vec::new()
-    }
-}
-
 /// DIMOVERRIDE at the command line: variable, value (repeat), Enter, then select dimensions.
 struct OverrideM {
     vars: serde_json::Map<String, Value>,

crates/engine/src/cmd/edit.rs

@@ -4,6 +4,7 @@ use cadcraft_doc::{Entity, Handle, entity_bounds};
 use cadcraft_geom::{Bounds2, Mat3, Vec2};
 use serde_json::{Value, json};
 
+use super::machines::{SelOutcome, SelectPhase, SelectRun};
 use super::*;
 use crate::{Accept, Input, Interactive, Prompt, Result, Session, Step};
 
@@ -11,13 +12,23 @@ pub fn specs() -> Vec<CommandSpec> {
     vec![
         CommandSpec::new("undo", "Undo", run_undo).menu(&["Edit", "Undo"]).key("Cmd+Z").alias(&["u"]).params("{count?: n}").noundo(),
         CommandSpec::new("redo", "Redo", run_redo).menu(&["Edit", "Redo"]).key("Cmd+Shift+Z").alias(&["mredo"]).params("{count?: n}").noundo(),
-        CommandSpec::new("cutclip", "Cut", run_cut).menu(&["Edit", "Cut"]).key("Cmd+X").params("{handles?}"),
-        CommandSpec::new("copyclip", "Copy", run_copyclip).menu(&["Edit", "Copy"]).key("Cmd+C").params("{handles?}").noundo(),
+        CommandSpec::new("cutclip", "Cut", run_cut)
+            .menu(&["Edit", "Cut"])
+            .key("Cmd+X")
+            .params("{handles?}")
+            .interactive(|_| Ok(Box::new(SelectRun::new("cutclip", "CUTCLIP")))),
+        CommandSpec::new("copyclip", "Copy", run_copyclip)
+            .menu(&["Edit", "Copy"])
+            .key("Cmd+C")
+            .params("{handles?}")
+            .noundo()
+            .interactive(|_| Ok(Box::new(SelectRun::new("copyclip", "COPYCLIP")))),
         CommandSpec::new("copybase", "Copy with Base Point", run_copybase)
             .menu(&["Edit", "Copy with Base Point"])
             .key("Cmd+Shift+C")
             .params("{handles?, base: [x,y]}")
-            .noundo(),
+            .noundo()
+            .interactive(|_| Ok(Box::new(CopyBaseM::default()))),
         CommandSpec::new("pasteclip", "Paste", run_paste)
             .menu(&["Edit", "Paste"])
             .key("Cmd+V")
@@ -240,7 +251,7 @@ impl Interactive for PasteM {
 
 #[derive(Default)]
 struct SelectM {
-    sel: super::machines::SelectPhase,
+    sel: SelectPhase,
 }
 
 impl Interactive for SelectM {
@@ -252,8 +263,56 @@ impl Interactive for SelectM {
     }
     fn input(&mut self, s: &mut Session, i: Input) -> Result<Step> {
         match self.sel.feed(s, &i)? {
-            super::machines::SelOutcome::More => Ok(Step::Continue),
+            SelOutcome::More => Ok(Step::Continue),
             _ => Ok(Step::Done),
         }
     }
 }
+
+/// COPYBASE: base point first, then "Select objects:" (skipped with a pickfirst selection).
+#[derive(Default)]
+struct CopyBaseM {
+    base: Option<Vec2>,
+    sel: SelectPhase,
+}
+
+impl CopyBaseM {
+    fn copy(&self, s: &mut Session, hs: &[Handle]) -> Result<Step> {
+        copy_to_clip(s, hs, self.base)?;
+        Ok(Step::Done)
+    }
+}
+
+impl Interactive for CopyBaseM {
+    fn name(&self) -> &'static str {
+        "COPYBASE"
+    }
+    fn begin(&mut self, s: &mut Session) -> Result<Step> {
+        self.sel = SelectPhase::begin(s);
+        Ok(Step::Continue)
+    }
+    fn prompt(&self, _s: &Session) -> Prompt {
+        if self.base.is_none() { Prompt::new("Specify base point", Accept::POINT) } else { self.sel.prompt() }
+    }
+    fn input(&mut self, s: &mut Session, i: Input) -> Result<Step> {
+        if self.base.is_none() {
+            if let Input::Point(p) = i {
+                self.base = Some(p);
+                if self.sel.done {
+                    let hs = self.sel.picked.clone();
+                    return self.copy(s, &hs);
+                }
+            }
+            return Ok(Step::Continue);
+        }
+        match self.sel.feed(s, &i)? {
+            SelOutcome::More => Ok(Step::Continue),
+            SelOutcome::Empty => Ok(Step::Done),
+            SelOutcome::Done(hs) => {
+                let step = self.copy(s, &hs)?;
+                s.set_selection(Vec::new());
+                Ok(step)
+            }
+        }
+    }
+}

crates/engine/src/cmd/layer.rs

@@ -4,6 +4,7 @@ use cadcraft_color::Color;
 use cadcraft_doc::{Layer, Lineweight};
 use serde_json::{Value, json};
 
+use super::machines::SelectRun;
 use super::*;
 use crate::{Result, Session};
 
@@ -22,21 +23,26 @@ pub fn specs() -> Vec<CommandSpec> {
         CommandSpec::new("laymch", "Layer Match", run_laymch).menu(&["Format", "Layer Tools", "Layer Match"]).params("{handles?, layer}"),
         CommandSpec::new("laycur", "Change to Current Layer", run_laycur)
             .menu(&["Format", "Layer Tools", "Change to Current Layer"])
-            .params("{handles?}"),
+            .params("{handles?}")
+            .interactive(|_| Ok(Box::new(SelectRun::new("laycur", "LAYCUR")))),
         CommandSpec::new("layiso", "Isolate Layer", run_layiso).menu(&["Format", "Layer Tools", "Isolate Layer"]).params("{handles?}"),
         CommandSpec::new("layuniso", "Unisolate Layer", run_layuniso).menu(&["Format", "Layer Tools", "Unisolate Layer"]),
         CommandSpec::new("layfrz", "Freeze Layer", |s, p| set_obj_layers(s, p, |l| l.frozen = true))
             .menu(&["Format", "Layer Tools", "Freeze Layer"])
-            .params("{handles?}"),
+            .params("{handles?}")
+            .interactive(|_| Ok(Box::new(SelectRun::single("layfrz", "LAYFRZ", "Select an object on the layer to be frozen")))),
         CommandSpec::new("layoff", "Layer Off", |s, p| set_obj_layers(s, p, |l| l.on = false))
             .menu(&["Format", "Layer Tools", "Layer Off"])
-            .params("{handles?}"),
+            .params("{handles?}")
+            .interactive(|_| Ok(Box::new(SelectRun::single("layoff", "LAYOFF", "Select an object on the layer to be turned off")))),
         CommandSpec::new("laylck", "Lock Layer", |s, p| set_obj_layers(s, p, |l| l.locked = true))
             .menu(&["Format", "Layer Tools", "Lock Layer"])
-            .params("{handles?}"),
+            .params("{handles?}")
+            .interactive(|_| Ok(Box::new(SelectRun::single("laylck", "LAYLCK", "Select an object on the layer to be locked")))),
         CommandSpec::new("layulk", "Unlock Layer", |s, p| set_obj_layers(s, p, |l| l.locked = false))
             .menu(&["Format", "Layer Tools", "Unlock Layer"])
-            .params("{handles?}"),
+            .params("{handles?}")
+            .interactive(|_| Ok(Box::new(SelectRun::single("layulk", "LAYULK", "Select an object on the layer to be unlocked")))),
         CommandSpec::new("layon", "Turn All Layers On", |s, _| all_layers(s, |l| l.on = true)),
         CommandSpec::new("laythw", "Thaw All Layers", |s, _| all_layers(s, |l| l.frozen = false)),
         CommandSpec::new("layerp", "Previous Layer", run_layerp).menu(&["Format", "Layer Tools", "Previous Layer"]),

crates/engine/src/cmd/machines.rs

@@ -1,8 +1,9 @@
 //! Building blocks for interactive commands: object selection and simple point sequences.
 
 use cadcraft_doc::Handle;
+use serde_json::{Value, json};
 
-use crate::{Accept, Input, Prompt, Result, Session};
+use crate::{Accept, EngineError, Input, Interactive, Prompt, Result, Session, Step};
 
 /// A "Select objects:" phase. Uses the pickfirst selection when there is one.
 #[derive(Clone, Debug, Default)]
@@ -95,7 +96,137 @@ impl SelectPhase {
     }
 }
 
+/// "Select objects:", then run a JSON command on them as `{handles}`. With a pickfirst selection it
+/// runs at once, exactly like the JSON form does on the current selection.
+pub struct SelectRun {
+    id: &'static str,
+    name: &'static str,
+    /// Single-object prompt (e.g. LAYFRZ's "Select an object on the layer to be frozen").
+    single: Option<&'static str>,
+    sel: SelectPhase,
+}
+
+impl SelectRun {
+    pub fn new(id: &'static str, name: &'static str) -> Self {
+        SelectRun { id, name, single: None, sel: SelectPhase::default() }
+    }
+    /// Pick one object, with a command-specific prompt.
+    pub fn single(id: &'static str, name: &'static str, msg: &'static str) -> Self {
+        SelectRun { single: Some(msg), ..SelectRun::new(id, name) }
+    }
+    fn run(&self, s: &mut Session, hs: &[Handle]) -> Result<Step> {
+        let spec = super::find_command(self.id).ok_or_else(|| EngineError::UnknownCommand(self.id.into()))?;
+        // Call the command body directly: the running command records the single undo step.
+        let r = (spec.run)(s, &json!({ "handles": hs.iter().map(|h| h.hex()).collect::<Vec<_>>() }))?;
+        if let Some(m) = r.get("message").and_then(Value::as_str) {
+            s.echo(m.to_string());
+        }
+        Ok(Step::Done)
+    }
+}
+
+impl Interactive for SelectRun {
+    fn name(&self) -> &'static str {
+        self.name
+    }
+    fn begin(&mut self, s: &mut Session) -> Result<Step> {
+        self.sel = SelectPhase::begin(s);
+        if self.sel.done {
+            let hs = self.sel.picked.clone();
+            return self.run(s, &hs);
+        }
+        self.sel.single = self.single.is_some();
+        Ok(Step::Continue)
+    }
+    fn prompt(&self, _s: &Session) -> Prompt {
+        match self.single {
+            Some(msg) if !self.sel.removing => Prompt::new(msg, Accept::SELECT),
+            _ => self.sel.prompt(),
+        }
+    }
+    fn input(&mut self, s: &mut Session, i: Input) -> Result<Step> {
+        match self.sel.feed(s, &i)? {
+            SelOutcome::More => Ok(Step::Continue),
+            SelOutcome::Empty => Ok(Step::Done),
+            SelOutcome::Done(hs) => {
+                // Objects picked at the prompt don't stay selected afterwards.
+                let step = self.run(s, &hs)?;
+                s.set_selection(Vec::new());
+                Ok(step)
+            }
+        }
+    }
+}
+
 /// Parse a number from text input.
 pub fn number(t: &str) -> Option<f64> {
     crate::units::parse_distance(t)
 }
+
+#[cfg(test)]
+mod tests {
+    use cadcraft_doc::Handle;
+    use cadcraft_geom::Vec2;
+    use serde_json::json;
+
+    use crate::{Input, Session};
+
+    /// Menu/typed invocation of selection commands prompts "Select objects" when nothing is selected
+    /// (#126), and acts on the picked objects in one undo step.
+    #[test]
+    fn selection_commands_prompt_without_pickfirst() {
+        let mut s = Session::new();
+        s.execute("layer.new", &json!({ "name": "A", "current": true })).unwrap();
+        s.execute("line", &json!({ "points": [[0, 0], [10, 0]] })).unwrap();
+        s.execute("layer.current", &json!({ "name": "0" })).unwrap();
+        s.execute("line", &json!({ "points": [[0, 5], [10, 5]] })).unwrap();
+        let hs: Vec<Handle> = s.doc().unwrap().model.iter().map(|e| e.handle).collect();
+        let undo_before = s.state().unwrap().undo.len();
+
+        s.start("cutclip").unwrap();
+        assert!(s.prompt_text().contains("Select objects"), "{}", s.prompt_text());
+        s.input(Input::Pick(vec![hs[1]])).unwrap();
+        s.input(Input::Enter).unwrap();
+        assert!(s.running.is_none());
+        assert_eq!(s.doc().unwrap().model.len(), 1);
+        assert_eq!(s.clipboard.len(), 1);
+        assert_eq!(s.state().unwrap().undo.len(), undo_before + 1, "one undo step");
+
+        // COPYBASE asks for the base point first, then for objects.
+        s.start("copybase").unwrap();
+        assert!(s.prompt_text().contains("Specify base point"), "{}", s.prompt_text());
+        s.input(Input::Point(Vec2::new(3.0, 4.0))).unwrap();
+        assert!(s.prompt_text().contains("Select objects"), "{}", s.prompt_text());
+        s.input(Input::Pick(vec![hs[0]])).unwrap();
+        s.input(Input::Enter).unwrap();
+        assert!(s.running.is_none());
+        assert_eq!(s.clipboard_base, Vec2::new(3.0, 4.0));
+
+        // LAYFRZ picks one object and freezes its layer.
+        s.start("layfrz").unwrap();
+        assert!(s.prompt_text().contains("layer to be frozen"), "{}", s.prompt_text());
+        s.input(Input::Pick(vec![hs[0]])).unwrap();
+        assert!(s.running.is_none());
+        assert!(s.doc().unwrap().layer("A").is_some_and(|l| l.frozen));
+    }
+
+    /// With a pickfirst selection the commands run at once; JSON calls never prompt.
+    #[test]
+    fn selection_commands_pickfirst_and_json_unchanged() {
+        let mut s = Session::new();
+        s.execute("line", &json!({ "points": [[0, 0], [10, 0]] })).unwrap();
+        let h = s.doc().unwrap().model.iter().next().unwrap().handle;
+        s.set_selection(vec![h]);
+        s.start("copyclip").unwrap();
+        assert!(s.running.is_none(), "pickfirst: no prompt");
+        assert_eq!(s.clipboard.len(), 1);
+
+        s.set_selection(Vec::new());
+        for id in ["cutclip", "copyclip", "zoom.object", "draworder.front", "laycur", "layoff"] {
+            let _ = s.execute(id, &json!({}));
+            assert!(s.running.is_none(), "{id}: JSON form must not prompt");
+        }
+        assert_eq!(s.doc().unwrap().model.len(), 1);
+        assert!(s.doc().unwrap().layer("0").is_some_and(|l| l.on));
+    }
+}

crates/engine/src/cmd/modify.rs

@@ -8,7 +8,7 @@ use cadcraft_geom::{
 use serde_json::{Value, json};
 
 use super::helpers::*;
-use super::machines::{SelOutcome, SelectPhase, number};
+use super::machines::{SelOutcome, SelectPhase, SelectRun, number};
 use super::*;
 use crate::{Accept, EngineError, Input, Interactive, Prompt, Result, Session, Step};
 
@@ -87,8 +87,14 @@ pub fn specs() -> Vec<CommandSpec> {
             .menu(&["Modify", "Array", "Polar Array"])
             .params("{handles?, center, count, angle? (degrees, default 360), rotate?: bool}")
             .interactive(|_| Ok(Box::new(SelectThen::new(Op::ArrayPolar)))),
-        CommandSpec::new("draworder.front", "Bring to Front", run_front).menu(&["Tools", "Draw Order", "Bring to Front"]).params("{handles?}"),
-        CommandSpec::new("draworder.back", "Send to Back", run_back).menu(&["Tools", "Draw Order", "Send to Back"]).params("{handles?}"),
+        CommandSpec::new("draworder.front", "Bring to Front", run_front)
+            .menu(&["Tools", "Draw Order", "Bring to Front"])
+            .params("{handles?}")
+            .interactive(|_| Ok(Box::new(SelectRun::new("draworder.front", "DRAWORDER")))),
+        CommandSpec::new("draworder.back", "Send to Back", run_back)
+            .menu(&["Tools", "Draw Order", "Send to Back"])
+            .params("{handles?}")
+            .interactive(|_| Ok(Box::new(SelectRun::new("draworder.back", "DRAWORDER")))),
         CommandSpec::new("break", "Break", run_break)
             .menu(&["Modify", "Break"])
             .alias(&["br"])

crates/engine/src/cmd/view.rs

@@ -4,7 +4,7 @@ use cadcraft_doc::{Space, entity_bounds};
 use cadcraft_geom::{Bounds2, Vec2};
 use serde_json::{Value, json};
 
-use super::machines::number;
+use super::machines::{SelectRun, number};
 use super::*;
 use crate::{Accept, Input, Interactive, Prompt, Result, Session, Step, View};
 
@@ -40,7 +40,8 @@ pub fn specs() -> Vec<CommandSpec> {
         .menu(&["View", "Zoom", "Object"])
         .params("{handles?}")
         .noundo()
-        .transparent(),
+        .transparent()
+        .interactive(|_| Ok(Box::new(SelectRun::new("zoom.object", "ZOOM")))),
         CommandSpec::new("pan", "Pan", run_pan)
             .menu(&["View", "Pan", "Realtime"])
             .alias(&["p"])