Simplify Modify ribbon extensions

build.rs

@@ -4,7 +4,6 @@
 use std::path::Path;
 
 fn main() {
-    generate_modify_tools();
     let version = std::env::var("CARGO_PKG_VERSION").expect("Cargo package version");
     let parts: Vec<&str> = version.split('.').collect();
     let app_version = if parts.len() == 3 && parts[0].len() == 4
@@ -82,24 +81,3 @@ fn main() {
         }
     }
 }
-
-fn generate_modify_tools() {
-    let root = std::path::PathBuf::from(std::env::var_os("CARGO_MANIFEST_DIR").unwrap());
-    let directory = root.join("src/ui/ribbon/modify_tools");
-    println!("cargo:rerun-if-changed=src/ui/ribbon/modify_tools");
-    let mut paths: Vec<_> = match std::fs::read_dir(&directory) {
-        Ok(entries) => entries.map(|entry| entry.expect("Modify tool entry").path())
-            .filter(|path| path.extension().is_some_and(|ext| ext == "rs")).collect(),
-        Err(error) if error.kind() == std::io::ErrorKind::NotFound => Vec::new(),
-        Err(error) => panic!("Cannot read Modify tools: {error}"),
-    };
-    paths.sort();
-    let mut source = String::from("pub(super) const TOOLS: &[Tool] = &[\n");
-    for path in paths {
-        println!("cargo:rerun-if-changed={}", path.display());
-        source.push_str(&format!("include!({:?}),\n", path.to_string_lossy()));
-    }
-    source.push_str("];\n");
-    let output = std::path::PathBuf::from(std::env::var_os("OUT_DIR").unwrap());
-    std::fs::write(output.join("modify_panel_tools.rs"), source).expect("Write Modify tool registry");
-}

src/app/commands/blocks.rs

@@ -713,6 +713,19 @@ mod tests {
         app
     }
 
+    #[test]
+    fn ncopy_without_preselection_gathers_objects() {
+        let mut app = fresh_app();
+        let _ = app.run_command_line("NCOPY");
+        assert_eq!(
+            app.tabs[app.active_tab]
+                .active_cmd
+                .as_ref()
+                .map(|active| active.name()),
+            Some("SELECT")
+        );
+    }
+
     /// Run one command line and return only the command-line text it appended.
     fn run_capture(app: &mut OpenCADStudio, cmd: &str) -> String {
         let start = app.command_line.history.len();

src/app/commands/styleprops.rs

@@ -2997,6 +2997,22 @@ mod tests {
         app
     }
 
+    #[test]
+    fn selection_commands_gather_instead_of_using_the_whole_drawing() {
+        for command in ["SETBYLAYER", "OVERKILL"] {
+            let mut app = fresh_app();
+            let _ = app.run_command_line(command);
+            assert_eq!(
+                app.tabs[app.active_tab]
+                    .active_cmd
+                    .as_ref()
+                    .map(|active| active.name()),
+                Some("SELECT"),
+                "{command} should gather an explicit selection"
+            );
+        }
+    }
+
     #[test]
     fn test_cad_selection_and_model_space_sysvars() {
         let mut app = fresh_app();
@@ -3209,4 +3225,3 @@ mod scale_validation_tests {
         }
     }
 }
-

src/app/commands/view.rs

@@ -2341,4 +2341,35 @@ mod tests {
         let line_sort = entries.get(&h_line.value()).copied().unwrap_or(h_line.value());
         assert!(hatch_sort > line_sort);
     }
+
+    #[test]
+    fn draworder_mode_aliases_route_directly() {
+        let mut app = fresh_app();
+        let i = app.active_tab;
+        let line = app.tabs[i]
+            .scene
+            .add_entity_clone(EntityType::Line(Default::default()));
+        let hatch = app.tabs[i]
+            .scene
+            .add_entity_clone(EntityType::Hatch(Default::default()));
+
+        app.tabs[i]
+            .scene
+            .replace_selection(std::iter::once(hatch).collect());
+        let _ = app.run_command_line("DRAWORDER_FRONT");
+        let entries = effective_sort_map(&app);
+        assert!(
+            entries.get(&hatch.value()).copied().unwrap_or(hatch.value())
+                > entries.get(&line.value()).copied().unwrap_or(line.value())
+        );
+
+        app.tabs[i]
+            .scene
+            .replace_selection(std::iter::once(hatch).collect());
+        let _ = app.run_command_line("DRAWORDER_ABOVE");
+        assert!(app.tabs[i]
+            .active_cmd
+            .as_ref()
+            .is_some_and(|command| command.name() == "DRAWORDER" && command.needs_entity_pick()));
+    }
 }

src/app/update/command.rs

@@ -890,9 +890,9 @@ pub(super) fn on_tab_close(&mut self, idx: usize) -> Task<Message> {
     }
 
     pub(super) fn on_command_escape(&mut self) -> Task<Message> {
-                if self.ribbon.escape_extension() {
-                    return Task::none();
-                }
+        if self.ribbon.escape_extension() {
+            return Task::none();
+        }
                 // Esc drops an unconsumed one-shot snap override and closes
                 // its menu (#337). Falls through — Esc keeps its usual effect.
                 self.snap_override_popup = None;

src/ui/ribbon/draw_panel.rs

@@ -127,13 +127,27 @@ struct Panel {
 }
 
 const PANELS: &[Panel] = &[
-    Panel { id: PANEL_ID, title_id: TITLE_ID, title: "Draw", tools: TOOLS },
-    Panel { id: "modify_extension", title_id: "modify_extension_title", title: "Modify", tools: super::modify_panel::TOOLS },
+    Panel {
+        id: PANEL_ID,
+        title_id: TITLE_ID,
+        title: "Draw",
+        tools: TOOLS,
+    },
+    Panel {
+        id: "modify_extension",
+        title_id: "modify_extension_title",
+        title: "Modify",
+        tools: super::modify_panel::TOOLS,
+    },
 ];
 
 fn panel_for_dropdown(id: &str) -> Option<Panel> {
     PANELS.iter().copied().find(|panel| {
-        panel.id == id || panel.tools.iter().any(|tool| tool.command == id && !tool.options.is_empty())
+        panel.id == id
+            || panel
+                .tools
+                .iter()
+                .any(|tool| tool.command == id && !tool.options.is_empty())
     })
 }
 
@@ -146,7 +160,10 @@ pub(super) fn parent_panel(id: &str) -> Option<&'static str> {
 }
 
 pub(super) fn group_title<'a>(title: &'static str, open: &Option<String>) -> Element<'a, Message> {
-    let Some(panel) = PANELS.iter().find(|panel| panel.title == title && !panel.tools.is_empty()) else {
+    let Some(panel) = PANELS
+        .iter()
+        .find(|panel| panel.title == title && !panel.tools.is_empty())
+    else {
         return container(text(t!(title)).size(9).style(muted_text_style))
             .padding([1, 4])
             .into();
@@ -160,9 +177,12 @@ pub(super) fn group_title<'a>(title: &'static str, open: &Option<String>) -> Ele
     PosReport::new(
         panel.id,
         button(
-            row![PosReport::new(panel.title_id, text(t!(title)).size(9)), arrow]
-                .spacing(4)
-                .align_y(iced::Center),
+            row![
+                PosReport::new(panel.title_id, text(t!(title)).size(9)),
+                arrow
+            ]
+            .spacing(4)
+            .align_y(iced::Center),
         )
         .on_press(Message::ToggleRibbonDropdown(panel.id.to_string()))
         .style(move |theme: &Theme, status| tool_btn_style(theme, expanded, status))
@@ -225,8 +245,6 @@ pub(super) fn overlay<'a>(ribbon: &Ribbon, id: &str, win: (f32, f32)) -> Element
         || tools.len().div_ceil(cols) as f32 * (CELL + GAP) - GAP + 12.0,
         |tool| tool.options.len() as f32 * OPTION_HEIGHT,
     );
-    let ordered: Vec<&Tool> = tools.iter().filter(|tool| tool.options.is_empty())
-        .chain(tools.iter().filter(|tool| !tool.options.is_empty())).collect();
     let contents: Element<'static, Message> = if let Some(tool) = option_tool {
         column(
             tool.options
@@ -248,13 +266,17 @@ pub(super) fn overlay<'a>(ribbon: &Ribbon, id: &str, win: (f32, f32)) -> Element
         .into()
     } else {
         column(
-            ordered
+            tools
                 .chunks(cols)
                 .map(|tools| {
                     row(tools
                         .iter()
                         .map(|tool| {
-                            tool_button(tool, ribbon.active_tool.as_deref() == Some(tool.command), panel.id)
+                            tool_button(
+                                tool,
+                                ribbon.active_tool.as_deref() == Some(tool.command),
+                                panel.id,
+                            )
                         })
                         .collect::<Vec<_>>())
                     .spacing(GAP)
@@ -275,3 +297,29 @@ pub(super) fn overlay<'a>(ribbon: &Ribbon, id: &str, win: (f32, f32)) -> Element
         });
     dropdown_backdrop(position_ribbon_dropdown(panel.into(), false, left, top))
 }
+
+#[cfg(test)]
+mod tests {
+    use super::{parent_panel, PANEL_ID, TOOLS};
+
+    #[test]
+    fn extension_submenus_resolve_to_their_parent_panel() {
+        assert_eq!(parent_panel("REVCLOUD"), Some(PANEL_ID));
+        assert_eq!(parent_panel("DRAWORDER_FRONT"), Some("modify_extension"));
+        assert_eq!(parent_panel("unknown"), None);
+    }
+
+    #[test]
+    fn split_tools_follow_plain_tools() {
+        let first_split = TOOLS
+            .iter()
+            .position(|tool| !tool.options.is_empty())
+            .unwrap_or(TOOLS.len());
+        assert!(TOOLS[..first_split]
+            .iter()
+            .all(|tool| tool.options.is_empty()));
+        assert!(TOOLS[first_split..]
+            .iter()
+            .all(|tool| !tool.options.is_empty()));
+    }
+}

src/ui/ribbon/mod.rs

@@ -1600,6 +1600,17 @@ mod tests {
         assert!(!is_active_tool("BLOCKPALETTE", &None, &off));
     }
 
+    #[test]
+    fn escape_returns_from_an_extension_submenu_then_closes_the_panel() {
+        let mut ribbon = Ribbon::default();
+        ribbon.open_dropdown = Some("DRAWORDER_FRONT".to_string());
+
+        assert!(ribbon.escape_extension());
+        assert_eq!(ribbon.open_dropdown.as_deref(), Some("modify_extension"));
+        assert!(ribbon.escape_extension());
+        assert_eq!(ribbon.open_dropdown, None);
+    }
+
     /// Reproducible element-construction benchmark for the ribbon view. Run with:
     /// `cargo test --lib --release -- --ignored --nocapture bench_ribbon_view_construction`
     ///

src/ui/ribbon/modify_panel.rs

@@ -1,4 +1,117 @@
-//! Independently registered commands in the Modify extension.
 use super::draw_panel::Tool;
 
-include!(concat!(env!("OUT_DIR"), "/modify_panel_tools.rs"));
+pub(super) const TOOLS: &[Tool] = &[
+    Tool {
+        command: "SETBYLAYER",
+        label: "Set to ByLayer",
+        icon: include_bytes!("../../../assets/icons/modify_setbylayer.svg"),
+        options: &[],
+    },
+    Tool {
+        command: "LENGTHEN",
+        label: "Lengthen",
+        icon: include_bytes!("../../../assets/icons/modify_lengthen.svg"),
+        options: &[],
+    },
+    Tool {
+        command: "PEDIT",
+        label: "Edit Polyline",
+        icon: include_bytes!("../../../assets/icons/modify_pedit.svg"),
+        options: &[],
+    },
+    Tool {
+        command: "SPLINEDIT",
+        label: "Edit Spline",
+        icon: include_bytes!("../../../assets/icons/modify_splinedit.svg"),
+        options: &[],
+    },
+    Tool {
+        command: "HATCHEDIT",
+        label: "Edit Hatch",
+        icon: include_bytes!("../../../assets/icons/modify_hatchedit.svg"),
+        options: &[],
+    },
+    Tool {
+        command: "ALIGN",
+        label: "Align",
+        icon: include_bytes!("../../../assets/icons/modify_align.svg"),
+        options: &[],
+    },
+    Tool {
+        command: "BREAK",
+        label: "Break",
+        icon: include_bytes!("../../../assets/icons/modify_break.svg"),
+        options: &[],
+    },
+    Tool {
+        command: "BREAKATPOINT",
+        label: "Break at Point",
+        icon: include_bytes!("../../../assets/icons/modify_breakatpoint.svg"),
+        options: &[],
+    },
+    Tool {
+        command: "JOIN",
+        label: "Join",
+        icon: include_bytes!("../../../assets/icons/modify_join.svg"),
+        options: &[],
+    },
+    Tool {
+        command: "REVERSE",
+        label: "Reverse",
+        icon: include_bytes!("../../../assets/icons/modify_reverse.svg"),
+        options: &[],
+    },
+    Tool {
+        command: "NCOPY",
+        label: "Copy Nested Objects",
+        icon: include_bytes!("../../../assets/icons/modify_ncopy.svg"),
+        options: &[],
+    },
+    Tool {
+        command: "OVERKILL",
+        label: "Delete Duplicate Objects",
+        icon: include_bytes!("../../../assets/icons/modify_overkill.svg"),
+        options: &[],
+    },
+    Tool {
+        command: "DRAWORDER_FRONT",
+        label: "Draw Order",
+        icon: include_bytes!("../../../assets/icons/modify_draworder.svg"),
+        options: &[
+            ("DRAWORDER_FRONT", "Bring to Front"),
+            ("DRAWORDER_BACK", "Send to Back"),
+            ("DRAWORDER_ABOVE", "Bring Above Objects"),
+            ("DRAWORDER_UNDER", "Send Under Objects"),
+        ],
+    },
+];
+
+#[cfg(test)]
+mod tests {
+    use super::TOOLS;
+
+    #[test]
+    fn every_tool_command_is_registered() {
+        let registered = crate::command::all_registered_command_names();
+        for command in TOOLS
+            .iter()
+            .flat_map(|tool| std::iter::once(tool.command).chain(tool.options.iter().map(|o| o.0)))
+        {
+            assert!(registered.contains(&command), "{command} is not registered");
+        }
+    }
+
+    #[test]
+    fn split_tools_follow_plain_tools() {
+        let first_split = TOOLS
+            .iter()
+            .position(|tool| !tool.options.is_empty())
+            .unwrap_or(TOOLS.len());
+        assert!(TOOLS[..first_split]
+            .iter()
+            .all(|tool| tool.options.is_empty()));
+        assert!(TOOLS[first_split..]
+            .iter()
+            .all(|tool| !tool.options.is_empty()));
+    }
+}

src/ui/ribbon/modify_tools/01_setbylayer.rs

@@ -1,6 +0,0 @@
-Tool {
-    command: "SETBYLAYER",
-    label: "Set to ByLayer",
-    icon: include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/assets/icons/modify_setbylayer.svg")),
-    options: &[],
-}

src/ui/ribbon/modify_tools/02_lengthen.rs

@@ -1,6 +0,0 @@
-Tool {
-    command: "LENGTHEN",
-    label: "Lengthen",
-    icon: include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/assets/icons/modify_lengthen.svg")),
-    options: &[],
-}

src/ui/ribbon/modify_tools/03_pedit.rs

@@ -1,6 +0,0 @@
-Tool {
-    command: "PEDIT",
-    label: "Edit Polyline",
-    icon: include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/assets/icons/modify_pedit.svg")),
-    options: &[],
-}

src/ui/ribbon/modify_tools/04_splinedit.rs

@@ -1,6 +0,0 @@
-Tool {
-    command: "SPLINEDIT",
-    label: "Edit Spline",
-    icon: include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/assets/icons/modify_splinedit.svg")),
-    options: &[],
-}

src/ui/ribbon/modify_tools/05_hatchedit.rs

@@ -1,6 +0,0 @@
-Tool {
-    command: "HATCHEDIT",
-    label: "Edit Hatch",
-    icon: include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/assets/icons/modify_hatchedit.svg")),
-    options: &[],
-}

src/ui/ribbon/modify_tools/06_align.rs

@@ -1,6 +0,0 @@
-Tool {
-    command: "ALIGN",
-    label: "Align",
-    icon: include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/assets/icons/modify_align.svg")),
-    options: &[],
-}

src/ui/ribbon/modify_tools/07_break.rs

@@ -1,6 +0,0 @@
-Tool {
-    command: "BREAK",
-    label: "Break",
-    icon: include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/assets/icons/modify_break.svg")),
-    options: &[],
-}

src/ui/ribbon/modify_tools/08_breakatpoint.rs

@@ -1,6 +0,0 @@
-Tool {
-    command: "BREAKATPOINT",
-    label: "Break at Point",
-    icon: include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/assets/icons/modify_breakatpoint.svg")),
-    options: &[],
-}

src/ui/ribbon/modify_tools/09_join.rs

@@ -1,6 +0,0 @@
-Tool {
-    command: "JOIN",
-    label: "Join",
-    icon: include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/assets/icons/modify_join.svg")),
-    options: &[],
-}

src/ui/ribbon/modify_tools/10_reverse.rs

@@ -1,6 +0,0 @@
-Tool {
-    command: "REVERSE",
-    label: "Reverse",
-    icon: include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/assets/icons/modify_reverse.svg")),
-    options: &[],
-}

src/ui/ribbon/modify_tools/11_ncopy.rs

@@ -1,6 +0,0 @@
-Tool {
-    command: "NCOPY",
-    label: "Copy Nested Objects",
-    icon: include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/assets/icons/modify_ncopy.svg")),
-    options: &[],
-}

src/ui/ribbon/modify_tools/12_overkill.rs

@@ -1,6 +0,0 @@
-Tool {
-    command: "OVERKILL",
-    label: "Delete Duplicate Objects",
-    icon: include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/assets/icons/modify_overkill.svg")),
-    options: &[],
-}

src/ui/ribbon/modify_tools/13_draworder.rs

@@ -1,6 +0,0 @@
-Tool {
-    command: "DRAWORDER_FRONT",
-    label: "Draw Order",
-    icon: include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/assets/icons/modify_draworder.svg")),
-    options: &[("DRAWORDER_FRONT", "Bring to Front"), ("DRAWORDER_BACK", "Send to Back"), ("DRAWORDER_ABOVE", "Bring Above Objects"), ("DRAWORDER_UNDER", "Send Under Objects")],
-}