diff --git a/src/app/commands/view.rs b/src/app/commands/view.rs index bf72046d8..4a3e99ba3 100644 --- a/src/app/commands/view.rs +++ b/src/app/commands/view.rs @@ -449,6 +449,7 @@ impl OpenCADStudio { } } } + crate::app::shortcuts::sync_space_dispatch_flag(&self.shortcut_bindings); self.persist_settings_if_changed(); self.command_line.push_output( crate::tf!("CUIIMPORT: loaded {n} shortcut(s) from \"{path}\".") @@ -483,6 +484,9 @@ impl OpenCADStudio { self.command_line.push_error(crate::t!("Usage: SHORTCUTS SET e.g. SHORTCUTS SET CTRL+D DIST").as_ref()); } else { self.shortcut_bindings.insert(key.clone(), cmd_str.clone()); + crate::app::shortcuts::sync_space_dispatch_flag( + &self.shortcut_bindings, + ); self.persist_settings_if_changed(); self.command_line.push_output( crate::tf!("Shortcut set: {key} → {cmd_str}").as_ref(), @@ -500,6 +504,9 @@ impl OpenCADStudio { .remove(&crate::app::shortcuts::normalize_key(&key)) .is_some() { + crate::app::shortcuts::sync_space_dispatch_flag( + &self.shortcut_bindings, + ); self.persist_settings_if_changed(); self.command_line .push_output(crate::tf!("Shortcut '{key}' removed.").as_ref()); diff --git a/src/app/shortcuts.rs b/src/app/shortcuts.rs index 814cff8ad..a43b0286d 100644 --- a/src/app/shortcuts.rs +++ b/src/app/shortcuts.rs @@ -4,6 +4,28 @@ use super::{Message, OpenCADStudio}; use iced::Task; use rustc_hash::FxHashMap; use std::collections::BTreeMap; +use std::sync::atomic::{AtomicBool, Ordering}; + +/// True when the live `SPACE` binding resolves to something other than +/// `COMMANDSPACE`. The view's key-dispatch closure must stay a non-capturing fn +/// pointer, so instead of reading the binding map it reads this flag when a +/// text field reports a Captured space. When SPACE is default-bound to +/// COMMANDSPACE the flag is false and the text field keeps the space for +/// typing; when SPACE is rebound (e.g. to FINALIZE) the flag is true so the +/// Captured space still fires its shortcut. (#1710) +pub(crate) static SPACE_REBOUND_OFF_COMMANDSPACE: AtomicBool = AtomicBool::new(false); + +/// Recompute the SPACE-dispatch flag from the current binding map. Call after +/// every change to `shortcut_bindings`. (#1710) +pub(super) fn sync_space_dispatch_flag(bindings: &FxHashMap) { + SPACE_REBOUND_OFF_COMMANDSPACE + .store(space_rebound_off_commandspace(bindings), Ordering::Relaxed); +} + +/// Whether `SPACE` is bound to something other than `COMMANDSPACE`. (#1710) +pub(crate) fn space_rebound_off_commandspace(bindings: &FxHashMap) -> bool { + matches!(bindings.get("SPACE"), Some(cmd) if cmd != "COMMANDSPACE") +} #[cfg(target_os = "macos")] const ACCEL: &str = "CMD"; @@ -263,6 +285,7 @@ impl OpenCADStudio { }) .collect(); self.shortcut_bindings = bindings; + sync_space_dispatch_flag(&self.shortcut_bindings); self.persist_settings_if_changed(); } diff --git a/src/app/update/file.rs b/src/app/update/file.rs index a32cd3fa9..d37dc2a01 100644 --- a/src/app/update/file.rs +++ b/src/app/update/file.rs @@ -1390,6 +1390,7 @@ impl OpenCADStudio { self.shortcut_bindings .entry("SHIFT+TAB".to_string()) .or_insert_with(|| "DYNTABPREV".to_string()); + crate::app::shortcuts::sync_space_dispatch_flag(&self.shortcut_bindings); } /// Write the config only when it changed since the last write, so a toggle diff --git a/src/app/update/mod.rs b/src/app/update/mod.rs index d6709d0ee..5d020da01 100644 --- a/src/app/update/mod.rs +++ b/src/app/update/mod.rs @@ -11503,6 +11503,29 @@ mod free_text_entry_tests { .as_ref() .is_some_and(|cmd| cmd.name() == "PASTECLIP")); } + + #[test] + fn space_bound_to_finalize_finalizes_in_free_text_mode() { + // #1710: when the user rebinds SPACE to FINALIZE in the CUI, pressing + // SPACE must run FINALIZE even while the command line holds focus, + // instead of falling through to the command line's CommandSpace + // (space-insert) behavior. FreeText mode distinguishes the two: a + // COMMANDSPACE would insert a space, while FINALIZE finishes the edit. + let (mut app, _) = app_with_cell_command(""); + assert_eq!(app.text_entry_mode(), TextEntryMode::FreeText); + + // Bind SPACE to FINALIZE (the CUI assignment from the issue). + app.shortcut_bindings + .insert("SPACE".to_string(), "FINALIZE".to_string()); + + // Pressing SPACE runs FINALIZE, which finishes the edit instead of + // inserting a space. + let _ = app.update(Message::ShortcutPressed("SPACE".to_string())); + assert!( + app.tabs[0].active_cmd.is_none(), + "SPACE bound to FINALIZE must finalize, not insert a space" + ); + } } /// Set or clear one bit of `SELECTIONPREVIEW`. diff --git a/src/app/view/mod.rs b/src/app/view/mod.rs index add05e0f9..16a339f04 100644 --- a/src/app/view/mod.rs +++ b/src/app/view/mod.rs @@ -171,6 +171,29 @@ fn shortcut_key_name(key: &keyboard::Key, modifiers: keyboard::Modifiers) -> Opt Some(parts.join("+")) } +/// Whether a key that resolved to the shortcut name `shortcut` should fire as +/// a shortcut for the given keyboard dispatch `status`. A focused command line +/// (Status::Captured) normally swallows a key so the text field can type it, +/// but SPACE must still dispatch so its binding runs (COMMANDSPACE by default, +/// FINALIZE if rebound) even while a free-text edit is in progress. To avoid +/// firing the default COMMANDSPACE every time a text field captures a space, a +/// Captured-space only dispatches when SPACE is rebound away from COMMANDSPACE +/// (tracked in a static, since this closure is a non-capturing fn pointer). (#1710) +fn should_dispatch_shortcut(status: iced::event::Status, shortcut: &str) -> bool { + dispatch_shortcut( + status, + shortcut, + crate::app::shortcuts::SPACE_REBOUND_OFF_COMMANDSPACE + .load(std::sync::atomic::Ordering::Relaxed), + ) +} + +fn dispatch_shortcut(status: iced::event::Status, shortcut: &str, space_rebound: bool) -> bool { + status == iced::event::Status::Ignored + || crate::app::shortcuts::is_global_key(shortcut) + || (status == iced::event::Status::Captured && shortcut == "SPACE" && space_rebound) +} + /// `ViewportRenderMode` enum carries the raw DXF integers, not a label, /// so wrap it locally with a friendly name renderer. #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -2825,9 +2848,14 @@ impl OpenCADStudio { } } let shortcut = shortcut_key_name(&key, modifiers)?; - (status == Status::Ignored - || crate::app::shortcuts::is_global_key(&shortcut)) - .then_some(Message::ShortcutPressed(shortcut)) + // When the command line has focus, a key that is bound + // to a shortcut still fires that shortcut unless it is + // the space key bound to COMMANDSPACE (the default), + // which the command line needs for typing. The binding + // check happens in the update handler so this closure + // stays a non-capturing fn pointer. (#1710) + should_dispatch_shortcut(status, &shortcut) + .then_some(Message::ShortcutPressed(shortcut)) } _ => None, } @@ -4416,4 +4444,40 @@ mod tests { super::super::StartSection::default(), ); } + + #[test] + fn captured_space_dispatches_only_when_rebound_off_commandspace() { + // #1710: when the command line holds focus it reports Status::Captured + // and swallows a key so the text field can type it. A Captured-space + // must only fire as a shortcut when SPACE is rebound away from the + // default COMMANDSPACE (e.g. to FINALIZE); otherwise the default + // COMMANDSPACE would fire on every space typed into a text field. + // The pure helpers are tested here so no test touches the shared flag. + use crate::app::shortcuts::space_rebound_off_commandspace; + use iced::event::Status; + use rustc_hash::FxHashMap; + + let mut bindings: FxHashMap = FxHashMap::default(); + bindings.insert("SPACE".to_string(), "COMMANDSPACE".to_string()); + let rebound = space_rebound_off_commandspace(&bindings); + assert!(!rebound); + assert!( + !dispatch_shortcut(Status::Captured, "SPACE", rebound), + "a Captured-space bound to COMMANDSPACE must be typed, not dispatched" + ); + + bindings.insert("SPACE".to_string(), "FINALIZE".to_string()); + let rebound = space_rebound_off_commandspace(&bindings); + assert!(rebound); + assert!( + dispatch_shortcut(Status::Captured, "SPACE", rebound), + "a Captured-space rebound away from COMMANDSPACE must dispatch" + ); + + // A non-space captured key is typed, not dispatched as a shortcut. + assert!(!dispatch_shortcut(Status::Captured, "A", true)); + // Ignored keys always dispatch, regardless of the SPACE binding. + assert!(dispatch_shortcut(Status::Ignored, "SPACE", false)); + assert!(dispatch_shortcut(Status::Ignored, "F5", false)); + } }