From fcbab5762707eccbc676b98dce4d355f8853d305 Mon Sep 17 00:00:00 2001 From: Hakan Seven Date: Tue, 22 Sep 2026 13:35:25 +0300 Subject: [PATCH] fix: review fixes for the merged command-line, plugin and font changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Direct distance entry (#1429) turned every bare number at a point prompt into a distance, so ROTATE's angle and the values automation types were read as coordinates. The step reads the token first now — `feed_command` reports whether it claimed it — and only an unclaimed one becomes a distance. That also puts the token back on the unified step driver, which owns the P / L selection keywords and the dimension-snap bookkeeping, and stops `on_text_input` running twice for the same token. A tracking ray carries its own base, so it no longer demands an anchor that the first point of a command does not have yet. Plugin command execution (#1425) re-implemented the command line inside the plugin host: it dispatched families directly, skipping the teardown that starts a command (grip edits, selection gestures, `last_point`, the accepted snaps), and made every `CmdResult::Dispatch` in the application skip plugin dispatch to avoid its own re-entrancy. A scripted line now runs through `run_command_line`, with the trailing Enter left off while the plugin streams the command, and the re-entrancy is handled by a flag set for the length of the plugin's call. A pause parks the line's Enter with the rest of its tokens instead of dropping it. A font reference (#1432) tried the family lookup before the face-file index, and that lookup ends in a prefix match a longer file name loses to (`ISOCPEUI.TTF` would take `ISOCP`). The exact file answers first. Co-Authored-By: Claude Opus 5 --- src/app/command_driver.rs | 69 ++++++++++++++++++++++++------------ src/app/commands/mod.rs | 17 ++++----- src/app/mod.rs | 6 ++++ src/app/plugin_host.rs | 74 +++++++++++---------------------------- src/app/update/command.rs | 23 ++++++------ src/scene/text/sysfont.rs | 5 ++- 6 files changed, 94 insertions(+), 100 deletions(-) diff --git a/src/app/command_driver.rs b/src/app/command_driver.rs index 349c295e6..24c4a00c4 100644 --- a/src/app/command_driver.rs +++ b/src/app/command_driver.rs @@ -429,12 +429,20 @@ impl OpenCADStudio { /// through, so the routing from input → `on_*` method → `apply_cmd_result` /// lives in exactly one place. No-op when no command is active. pub(super) fn feed_command(&mut self, input: StepInput) -> Task { + self.feed_command_consumed(input).0 + } + + /// `feed_command`, also reporting whether the step actually took the input. + /// Only a `Text` token can come back unclaimed (`on_text_input` returning + /// `None`), which is what lets the caller read it as something else — a + /// typed distance, say — instead of guessing beforehand. + pub(super) fn feed_command_consumed(&mut self, input: StepInput) -> (Task, bool) { // Selection keywords (P / PREVIOUS, L / LAST) consume the token // before it reaches the command (#426). if let StepInput::Text(s) = &input { let kw_input = s.clone(); if let Some(task) = self.try_selection_keyword(&kw_input) { - return task; + return (task, true); } } let i = self.active_tab; @@ -450,14 +458,14 @@ impl OpenCADStudio { }; if let StepInput::Point(point) = &input { if !self.command_point_allowed(i, *point) { - return Task::none(); + return (Task::none(), true); } // Typed / dynamic-input / headless points carry no snap, but the // accepted-snap list must stay index-parallel with the points the // command collects. Interactive picks record themselves in // the click handler and never reach here. if !self.record_accepted_snap(i, None, None, *point) { - return Task::none(); + return (Task::none(), true); } } if default_start { @@ -473,7 +481,7 @@ impl OpenCADStudio { } if let StepInput::EntityPick(handle, point) = &input { if !self.dimension_acquisition_allowed(i, None) { - return Task::none(); + return (Task::none(), true); } let solid_pick = matches!( self.tabs[i].scene.document.get_entity(*handle), @@ -555,7 +563,7 @@ impl OpenCADStudio { let is_escape = matches!(&input, StepInput::Escape); let result: Option = { let Some(cmd) = self.tabs[i].active_cmd.as_mut() else { - return Task::none(); + return (Task::none(), false); }; cmd.set_ctrl(ctrl); cmd.set_shift(shift); @@ -579,8 +587,8 @@ impl OpenCADStudio { self.tabs[i].pending_pause_tokens = None; } match result { - Some(r) => self.apply_cmd_result(r), - None => Task::none(), + Some(r) => (self.apply_cmd_result(r), true), + None => (Task::none(), false), } } @@ -629,6 +637,14 @@ impl OpenCADStudio { /// terminated as if Enter were pressed. Shared by the GUI command line and /// the headless automation feeder so both behave identically. pub(super) fn run_command_line(&mut self, cmd: &str) -> Task { + self.run_command_line_streaming(cmd, true) + } + + /// `run_command_line`, with the trailing Enter left off when `finish` is + /// false: the line is one instalment and the tool stays at its next prompt + /// for the following one. Only a plugin streaming a command feeds it that + /// way; the command line and the automation feeder always finish. + pub(super) fn run_command_line_streaming(&mut self, cmd: &str, finish: bool) -> Task { let i = self.active_tab; self.command_line.unconsumed.clear(); let tokens: Vec<&str> = cmd.split_whitespace().collect(); @@ -640,9 +656,9 @@ impl OpenCADStudio { // dispatch first. A built-in interactive tool matches only its bare name // (`LINE`), so the full line is not a plugin command and falls through to // the first-word + fed-tokens path below. (#162) - if crate::plugin::try_dispatch(self, i, cmd) { + if !self.suppress_plugin_dispatch && crate::plugin::try_dispatch(self, i, cmd) { let toks: Vec = tokens.iter().map(|s| s.to_string()).collect(); - return self.finish_active_command(&toks); + return self.finish_active_command(&toks, finish); } if tokens[0].eq_ignore_ascii_case("BACKGROUND") || tokens[0].eq_ignore_ascii_case("COLORSCHEME") @@ -655,7 +671,7 @@ impl OpenCADStudio { return self.dispatch_command(cmd); } let toks: Vec = tokens.iter().map(|s| s.to_string()).collect(); - let finish_task = self.finish_active_command(&toks); + let finish_task = self.finish_active_command(&toks, finish); Task::batch([start_task, finish_task]) } @@ -674,7 +690,7 @@ impl OpenCADStudio { /// silently dropped and `TEXT 0,0 5 0 hi` created nothing at all. /// * Anything still left over was never claimed by any prompt; it is /// recorded in [`CommandLine::unconsumed`] instead of vanishing. - pub(super) fn finish_active_command(&mut self, tokens: &[String]) -> Task { + pub(super) fn finish_active_command(&mut self, tokens: &[String], finish: bool) -> Task { let i = self.active_tab; if self.tabs[i].active_cmd.is_none() { return Task::none(); @@ -694,7 +710,12 @@ impl OpenCADStudio { } let trimmed = tok.trim(); if trimmed.eq_ignore_ascii_case("PAUSE") || trimmed == "\\" { - let remainder: Vec = tokens[1 + idx + 1..].to_vec(); + let mut remainder: Vec = tokens[1 + idx + 1..].to_vec(); + // The line still ends with an Enter; it waits behind the pause + // instead of being dropped with it. + if finish { + remainder.push("ENTER".to_string()); + } self.tabs[i].pending_pause_tokens = Some(remainder); consumed = tokens.len(); paused = true; @@ -730,7 +751,7 @@ impl OpenCADStudio { if consumed < tokens.len() { self.command_line.unconsumed = tokens[consumed..].to_vec(); } - if !paused { + if !paused && finish { tasks.push(self.feed_command(StepInput::Enter)); } Task::batch(tasks) @@ -981,17 +1002,19 @@ impl OpenCADStudio { self.push_ucs_to_cmd(i); return self.feed_command(StepInput::Point(wcs)); } else { - let consumed = self.tabs[i] - .active_cmd - .as_mut() - .and_then(|c| c.on_text_input(token)); - if let Some(r) = consumed { - return self.apply_cmd_result(r); - } - if let Some(task) = self.try_direct_distance_entry(token) { + // The step reads the token first: at a great many point prompts a + // bare number is the step's own value, not a distance — ROTATE's + // angle, SCALE's factor, CIRCLE's radius. Only a token no step + // claims becomes direct distance entry, measured along the cursor + // direction from the command's anchor. + let (task, consumed) = self.feed_command_consumed(StepInput::Text(token.to_string())); + if consumed { return task; } - return self.feed_command(StepInput::Text(token.to_string())); + if let Some(distance) = self.try_direct_distance_entry(token) { + return distance; + } + return task; } } @@ -5958,7 +5981,7 @@ impl OpenCADStudio { self.tabs[i].snap_result = None; self.tabs[i].scene.clear_preview_wire(); self.restore_pre_cmd_tangent(); - dispatched = self.dispatch_command_without_plugins(&cmd); + dispatched = self.dispatch_command(&cmd); } CmdResult::EditTableCell { handle, point } => { // TABLEDIT's pick: end the pick phase and hand (table, point) diff --git a/src/app/commands/mod.rs b/src/app/commands/mod.rs index 97cd4ed3d..26329416d 100644 --- a/src/app/commands/mod.rs +++ b/src/app/commands/mod.rs @@ -43,12 +43,7 @@ impl OpenCADStudio { } pub(super) fn dispatch_command(&mut self, cmd: &str) -> Task { - self.dispatch_command_inner(cmd, false, false) - } - - /// Dispatch a command skipping external plugin dispatch to prevent reentrant deadlocks. - pub(super) fn dispatch_command_without_plugins(&mut self, cmd: &str) -> Task { - self.dispatch_command_inner(cmd, false, true) + self.dispatch_command_inner(cmd, false) } /// Dispatch a verb typed at the interactive command line, falling back to @@ -60,10 +55,10 @@ impl OpenCADStudio { /// callers (ribbon, plugins, headless automation) use `dispatch_command` /// and never get silent substitution. pub(super) fn dispatch_command_or_suggest(&mut self, cmd: &str) -> Task { - self.dispatch_command_inner(cmd, true, false) + self.dispatch_command_inner(cmd, true) } - fn dispatch_command_inner(&mut self, cmd: &str, allow_suggest: bool, skip_plugins: bool) -> Task { + fn dispatch_command_inner(&mut self, cmd: &str, allow_suggest: bool) -> Task { let i = self.active_tab; // Expand a command alias ("L" → "LINE") on the leading token before any // routing, so every path below (Start-tab gate, plugins, all dispatch @@ -191,7 +186,7 @@ impl OpenCADStudio { return Task::none(); } - if !skip_plugins && crate::plugin::try_dispatch(self, i, cmd) { + if !self.suppress_plugin_dispatch && crate::plugin::try_dispatch(self, i, cmd) { // try_dispatch returns true for both finished commands and interactive // commands that it just installed. If no command is now active, the // tool was a one-shot and we must turn the ribbon highlight off here — @@ -230,7 +225,7 @@ impl OpenCADStudio { .next() { if !top.eq_ignore_ascii_case(cmd) { - return self.dispatch_command_inner(&top, false, skip_plugins); + return self.dispatch_command_inner(&top, false); } } } @@ -244,7 +239,7 @@ impl OpenCADStudio { /// `Some(task)` for an arm it owns (early-returning or falling through to /// `finish_dispatch`), or `None` to defer to the next — equivalent to one /// sequential `match` over all arms. - pub(crate) fn dispatch_families(&mut self, cmd: &str, i: usize) -> Option> { + fn dispatch_families(&mut self, cmd: &str, i: usize) -> Option> { if let Some(t) = self.dispatch_fileops(cmd, i) { return Some(t); } diff --git a/src/app/mod.rs b/src/app/mod.rs index 9490a9977..c33dbcec5 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -623,6 +623,11 @@ pub(super) struct OpenCADStudio { pub constraint_form_annotational: bool, /// The dimensional constraint DIMCONSTRAINT offers by default: the last one used. pub dim_constraint_last: &'static str, + /// Set while a plugin drives the command line (`HostApi::execute_command`). + /// Dispatching a plugin command from there would call back into the plugin + /// that is still blocked waiting for this request, so plugin dispatch is + /// skipped for the length of the call. + pub(crate) suppress_plugin_dispatch: bool, pub constraint_bar_mode: i16, /// Minutes between autosaves to a `.sv$` recovery file (SAVETIME command); /// 0 disables autosave. @@ -3964,6 +3969,7 @@ impl OpenCADStudio { constraint_bar_display: 3, constraint_form_annotational: false, dim_constraint_last: "Aligned", + suppress_plugin_dispatch: false, constraint_bar_mode: 4095, savetime_min: 10, default_bg_color: None, diff --git a/src/app/plugin_host.rs b/src/app/plugin_host.rs index 1d0705f7a..54230af20 100644 --- a/src/app/plugin_host.rs +++ b/src/app/plugin_host.rs @@ -420,62 +420,28 @@ impl<'a> HostSession<'a> { return false; } - // 4. Command execution - let parts: Vec<&str> = trimmed.split_whitespace().collect(); - if parts.is_empty() { - return false; - } - + // 4. Command execution. The command line's own driver owns this: it + // resolves the alias, tears down whatever was running, dispatches an + // inline-argument line as a whole (`CLAYER TEST`, `UCS ORIGIN 0,0`), + // and otherwise starts the tool and feeds it the rest of the tokens, + // PAUSE included. Leaving that here rather than repeating it keeps a + // scripted line and a typed one on the same path. let tab = self.tab; - let is_new_cmd = self.app.tabs[tab].active_cmd.is_none(); - if is_new_cmd { - // Check if the complete command string is handled directly by command families - // (e.g. "CLAYER TEST", "ZOOM EXTENTS", "COLOR RED", "VSCURRENT FLATSHADED", etc.) without needing token-by-token feeding - let full_resolved = self.app.resolve_alias(trimmed); - let full_effective = full_resolved.as_deref().unwrap_or(trimmed); - if let Some(task) = self.app.dispatch_families(full_effective, tab) { - let _ = self.app.drive_headless_task(task); - self.app.command_line.record_recent(full_effective); - self.app.refresh_layer_panel(); - self.publish_document_view(); - return true; + self.app.suppress_plugin_dispatch = true; + let task = if self.app.tabs[tab].active_cmd.is_some() { + // A running command owns these tokens: they answer its prompts. + let mut queue: Vec = + trimmed.split_whitespace().map(str::to_string).collect(); + if has_newline { + queue.push("ENTER".to_string()); } - - let cmd_name = parts[0]; - let resolved = self.app.resolve_alias(cmd_name); - let effective = resolved.as_deref().unwrap_or(cmd_name); - let task = self.app.dispatch_command_without_plugins(effective); - let _ = self.app.drive_headless_task(task); - } - - let mut paused = false; - let start_idx = if is_new_cmd { 1 } else { 0 }; - for (idx, &part) in parts.iter().skip(start_idx).enumerate() { - if self.app.tabs[tab].active_cmd.is_none() { - break; - } - if part.eq_ignore_ascii_case("PAUSE") || part == "\\" { - let mut remainder: Vec = - parts[start_idx + idx + 1..].iter().map(|s| s.to_string()).collect(); - if has_newline { - remainder.push("ENTER".to_string()); - } - self.app.tabs[tab].pending_pause_tokens = Some(remainder); - paused = true; - break; - } - let task = if part.eq_ignore_ascii_case("ENTER") || part.eq_ignore_ascii_case("RETURN") { - self.app.feed_command(crate::command::StepInput::Enter) - } else { - self.app.feed_active_cmd(part) - }; - let _ = self.app.drive_headless_task(task); - } - - if !paused && has_newline && self.app.tabs[tab].active_cmd.is_some() { - let task = self.app.feed_command(crate::command::StepInput::Enter); - let _ = self.app.drive_headless_task(task); - } + self.app.tabs[tab].pending_pause_tokens = Some(queue); + self.app.drain_pending_pause_tokens(tab) + } else { + self.app.run_command_line_streaming(trimmed, has_newline) + }; + let _ = self.app.drive_headless_task(task); + self.app.suppress_plugin_dispatch = false; self.app.refresh_layer_panel(); self.publish_document_view(); diff --git a/src/app/update/command.rs b/src/app/update/command.rs index a61ebb3a7..52603b0dc 100644 --- a/src/app/update/command.rs +++ b/src/app/update/command.rs @@ -876,28 +876,29 @@ pub(super) fn on_tab_close(&mut self, idx: usize) -> Task { /// direction in the active UCS plane. pub(in crate::app) fn try_direct_distance_entry(&mut self, text: &str) -> Option> { let i = self.active_tab; - let not_entity_pick = !self.tabs[i] + if self.tabs[i] .active_cmd .as_ref() - .map(|c| c.needs_entity_pick()) - .unwrap_or(false); - if !not_entity_pick { + .is_some_and(|c| c.needs_entity_pick()) + { return None; } - let anchor = self.tabs[i] - .active_cmd - .as_ref() - .and_then(|c| c.resolved_anchor()) - .or(self.tabs[i].dyn_anchor) - .or(self.last_point)?; - let dist = crate::entities::common::parse_length(text.trim()) .or_else(|| crate::app::expr_eval::eval_number(text.trim()))?; + // A tracking ray carries its own base, so it needs no anchor: that is + // how OTRACK and Extension accepted a bare distance for the *first* + // point of a command, before any anchor exists. let pt = if let Some((base, dir)) = self.active_distance_ray(i) { base + dir * dist } else { + let anchor = self.tabs[i] + .active_cmd + .as_ref() + .and_then(|c| c.resolved_anchor()) + .or(self.tabs[i].dyn_anchor) + .or(self.last_point)?; let w = self.tabs[i].last_cursor_world; let xf = self.tabs[i].ucs_xform(); let d_ucs = xf.vec_to_ucs(w - anchor); diff --git a/src/scene/text/sysfont.rs b/src/scene/text/sysfont.rs index 09e2427fb..a5d8458d2 100644 --- a/src/scene/text/sysfont.rs +++ b/src/scene/text/sysfont.rs @@ -179,7 +179,10 @@ pub fn family_for_reference(reference: &str) -> Option { if reference.is_empty() { return None; } - canonical_family_name(reference).or_else(|| family_for_file(reference)) + // The file name is exact; the family lookup ends in a prefix/subset match + // that a longer file name loses to (`ISOCPEUI.TTF` would take `ISOCP`), so + // an installed face file answers first. + family_for_file(reference).or_else(|| canonical_family_name(reference)) } /// Whether `family` matches an installed system font (case-insensitive via