From 5f3edf45ef5c8aea703950fb7e4d3d13b462b80f Mon Sep 17 00:00:00 2001 From: Hakan Seven Date: Fri, 11 Sep 2026 10:06:51 +0300 Subject: [PATCH] Simplify Modify ribbon extensions --- build.rs | 22 ---- src/app/commands/blocks.rs | 13 ++ src/app/commands/styleprops.rs | 17 ++- src/app/commands/view.rs | 31 +++++ src/app/update/command.rs | 6 +- src/ui/ribbon/draw_panel.rs | 70 +++++++++-- src/ui/ribbon/mod.rs | 11 ++ src/ui/ribbon/modify_panel.rs | 117 +++++++++++++++++- src/ui/ribbon/modify_tools/01_setbylayer.rs | 6 - src/ui/ribbon/modify_tools/02_lengthen.rs | 6 - src/ui/ribbon/modify_tools/03_pedit.rs | 6 - src/ui/ribbon/modify_tools/04_splinedit.rs | 6 - src/ui/ribbon/modify_tools/05_hatchedit.rs | 6 - src/ui/ribbon/modify_tools/06_align.rs | 6 - src/ui/ribbon/modify_tools/07_break.rs | 6 - src/ui/ribbon/modify_tools/08_breakatpoint.rs | 6 - src/ui/ribbon/modify_tools/09_join.rs | 6 - src/ui/ribbon/modify_tools/10_reverse.rs | 6 - src/ui/ribbon/modify_tools/11_ncopy.rs | 6 - src/ui/ribbon/modify_tools/12_overkill.rs | 6 - src/ui/ribbon/modify_tools/13_draworder.rs | 6 - 21 files changed, 248 insertions(+), 117 deletions(-) delete mode 100644 src/ui/ribbon/modify_tools/01_setbylayer.rs delete mode 100644 src/ui/ribbon/modify_tools/02_lengthen.rs delete mode 100644 src/ui/ribbon/modify_tools/03_pedit.rs delete mode 100644 src/ui/ribbon/modify_tools/04_splinedit.rs delete mode 100644 src/ui/ribbon/modify_tools/05_hatchedit.rs delete mode 100644 src/ui/ribbon/modify_tools/06_align.rs delete mode 100644 src/ui/ribbon/modify_tools/07_break.rs delete mode 100644 src/ui/ribbon/modify_tools/08_breakatpoint.rs delete mode 100644 src/ui/ribbon/modify_tools/09_join.rs delete mode 100644 src/ui/ribbon/modify_tools/10_reverse.rs delete mode 100644 src/ui/ribbon/modify_tools/11_ncopy.rs delete mode 100644 src/ui/ribbon/modify_tools/12_overkill.rs delete mode 100644 src/ui/ribbon/modify_tools/13_draworder.rs diff --git a/build.rs b/build.rs index 85ce9861f..fd50c581c 100644 --- a/build.rs +++ b/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"); -} diff --git a/src/app/commands/blocks.rs b/src/app/commands/blocks.rs index 50f4aedbc..5e368f4cc 100644 --- a/src/app/commands/blocks.rs +++ b/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(); diff --git a/src/app/commands/styleprops.rs b/src/app/commands/styleprops.rs index 7ab8c22f1..fe427276b 100644 --- a/src/app/commands/styleprops.rs +++ b/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 { } } } - diff --git a/src/app/commands/view.rs b/src/app/commands/view.rs index 609681141..5a7249d6f 100644 --- a/src/app/commands/view.rs +++ b/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())); + } } diff --git a/src/app/update/command.rs b/src/app/update/command.rs index e82e3cc98..c455e14e3 100644 --- a/src/app/update/command.rs +++ b/src/app/update/command.rs @@ -890,9 +890,9 @@ pub(super) fn on_tab_close(&mut self, idx: usize) -> Task { } pub(super) fn on_command_escape(&mut self) -> Task { - 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; diff --git a/src/ui/ribbon/draw_panel.rs b/src/ui/ribbon/draw_panel.rs index a4d30522a..6ef8bf31d 100644 --- a/src/ui/ribbon/draw_panel.rs +++ b/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 { 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) -> 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) -> 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::>()) .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())); + } +} diff --git a/src/ui/ribbon/mod.rs b/src/ui/ribbon/mod.rs index feed284f9..5dc37ad6b 100644 --- a/src/ui/ribbon/mod.rs +++ b/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` /// diff --git a/src/ui/ribbon/modify_panel.rs b/src/ui/ribbon/modify_panel.rs index d04410b12..835e03c27 100644 --- a/src/ui/ribbon/modify_panel.rs +++ b/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())); + } +} diff --git a/src/ui/ribbon/modify_tools/01_setbylayer.rs b/src/ui/ribbon/modify_tools/01_setbylayer.rs deleted file mode 100644 index 953ee34ed..000000000 --- a/src/ui/ribbon/modify_tools/01_setbylayer.rs +++ /dev/null @@ -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: &[], -} diff --git a/src/ui/ribbon/modify_tools/02_lengthen.rs b/src/ui/ribbon/modify_tools/02_lengthen.rs deleted file mode 100644 index 384429fb5..000000000 --- a/src/ui/ribbon/modify_tools/02_lengthen.rs +++ /dev/null @@ -1,6 +0,0 @@ -Tool { - command: "LENGTHEN", - label: "Lengthen", - icon: include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/assets/icons/modify_lengthen.svg")), - options: &[], -} diff --git a/src/ui/ribbon/modify_tools/03_pedit.rs b/src/ui/ribbon/modify_tools/03_pedit.rs deleted file mode 100644 index 7847b06e9..000000000 --- a/src/ui/ribbon/modify_tools/03_pedit.rs +++ /dev/null @@ -1,6 +0,0 @@ -Tool { - command: "PEDIT", - label: "Edit Polyline", - icon: include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/assets/icons/modify_pedit.svg")), - options: &[], -} diff --git a/src/ui/ribbon/modify_tools/04_splinedit.rs b/src/ui/ribbon/modify_tools/04_splinedit.rs deleted file mode 100644 index 435434b33..000000000 --- a/src/ui/ribbon/modify_tools/04_splinedit.rs +++ /dev/null @@ -1,6 +0,0 @@ -Tool { - command: "SPLINEDIT", - label: "Edit Spline", - icon: include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/assets/icons/modify_splinedit.svg")), - options: &[], -} diff --git a/src/ui/ribbon/modify_tools/05_hatchedit.rs b/src/ui/ribbon/modify_tools/05_hatchedit.rs deleted file mode 100644 index 477696833..000000000 --- a/src/ui/ribbon/modify_tools/05_hatchedit.rs +++ /dev/null @@ -1,6 +0,0 @@ -Tool { - command: "HATCHEDIT", - label: "Edit Hatch", - icon: include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/assets/icons/modify_hatchedit.svg")), - options: &[], -} diff --git a/src/ui/ribbon/modify_tools/06_align.rs b/src/ui/ribbon/modify_tools/06_align.rs deleted file mode 100644 index 7f982c718..000000000 --- a/src/ui/ribbon/modify_tools/06_align.rs +++ /dev/null @@ -1,6 +0,0 @@ -Tool { - command: "ALIGN", - label: "Align", - icon: include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/assets/icons/modify_align.svg")), - options: &[], -} diff --git a/src/ui/ribbon/modify_tools/07_break.rs b/src/ui/ribbon/modify_tools/07_break.rs deleted file mode 100644 index 1f130b827..000000000 --- a/src/ui/ribbon/modify_tools/07_break.rs +++ /dev/null @@ -1,6 +0,0 @@ -Tool { - command: "BREAK", - label: "Break", - icon: include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/assets/icons/modify_break.svg")), - options: &[], -} diff --git a/src/ui/ribbon/modify_tools/08_breakatpoint.rs b/src/ui/ribbon/modify_tools/08_breakatpoint.rs deleted file mode 100644 index 298b28025..000000000 --- a/src/ui/ribbon/modify_tools/08_breakatpoint.rs +++ /dev/null @@ -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: &[], -} diff --git a/src/ui/ribbon/modify_tools/09_join.rs b/src/ui/ribbon/modify_tools/09_join.rs deleted file mode 100644 index 65d447e95..000000000 --- a/src/ui/ribbon/modify_tools/09_join.rs +++ /dev/null @@ -1,6 +0,0 @@ -Tool { - command: "JOIN", - label: "Join", - icon: include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/assets/icons/modify_join.svg")), - options: &[], -} diff --git a/src/ui/ribbon/modify_tools/10_reverse.rs b/src/ui/ribbon/modify_tools/10_reverse.rs deleted file mode 100644 index 7ce5c6d9e..000000000 --- a/src/ui/ribbon/modify_tools/10_reverse.rs +++ /dev/null @@ -1,6 +0,0 @@ -Tool { - command: "REVERSE", - label: "Reverse", - icon: include_bytes!(concat!(env!("CARGO_MANIFEST_DIR"), "/assets/icons/modify_reverse.svg")), - options: &[], -} diff --git a/src/ui/ribbon/modify_tools/11_ncopy.rs b/src/ui/ribbon/modify_tools/11_ncopy.rs deleted file mode 100644 index e926ce540..000000000 --- a/src/ui/ribbon/modify_tools/11_ncopy.rs +++ /dev/null @@ -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: &[], -} diff --git a/src/ui/ribbon/modify_tools/12_overkill.rs b/src/ui/ribbon/modify_tools/12_overkill.rs deleted file mode 100644 index 0c7f90e0e..000000000 --- a/src/ui/ribbon/modify_tools/12_overkill.rs +++ /dev/null @@ -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: &[], -} diff --git a/src/ui/ribbon/modify_tools/13_draworder.rs b/src/ui/ribbon/modify_tools/13_draworder.rs deleted file mode 100644 index ecd78fbdf..000000000 --- a/src/ui/ribbon/modify_tools/13_draworder.rs +++ /dev/null @@ -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")], -}