forked from 1b-agent/opencadstudio-upstream
fix(input): let a rebound SPACE fire while the command line has focus
A focused text field reports Status::Captured, so the key dispatch dropped SPACE and a CUI rebind of SPACE to FINALIZE never ran while the command line held focus (#1710). Dispatch a captured SPACE as a shortcut only when SPACE is bound to something other than COMMANDSPACE, so the default binding still types a space. The closure stays a plain fn, so a static flag mirrors the binding and is refreshed wherever the bindings change.
This commit is contained in:
1 parent
4b6ced9b8c
commit
cbefecb120
5 files changed
+121
-3
No files matched your search
@@ -449,6 +449,7 @@ impl OpenCADStudio {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
crate::app::shortcuts::sync_space_dispatch_flag(&self.shortcut_bindings);
|
||||||
self.persist_settings_if_changed();
|
self.persist_settings_if_changed();
|
||||||
self.command_line.push_output(
|
self.command_line.push_output(
|
||||||
crate::tf!("CUIIMPORT: loaded {n} shortcut(s) from \"{path}\".")
|
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 <key> <command> e.g. SHORTCUTS SET CTRL+D DIST").as_ref());
|
self.command_line.push_error(crate::t!("Usage: SHORTCUTS SET <key> <command> e.g. SHORTCUTS SET CTRL+D DIST").as_ref());
|
||||||
} else {
|
} else {
|
||||||
self.shortcut_bindings.insert(key.clone(), cmd_str.clone());
|
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.persist_settings_if_changed();
|
||||||
self.command_line.push_output(
|
self.command_line.push_output(
|
||||||
crate::tf!("Shortcut set: {key} → {cmd_str}").as_ref(),
|
crate::tf!("Shortcut set: {key} → {cmd_str}").as_ref(),
|
||||||
@@ -500,6 +504,9 @@ impl OpenCADStudio {
|
|||||||
.remove(&crate::app::shortcuts::normalize_key(&key))
|
.remove(&crate::app::shortcuts::normalize_key(&key))
|
||||||
.is_some()
|
.is_some()
|
||||||
{
|
{
|
||||||
|
crate::app::shortcuts::sync_space_dispatch_flag(
|
||||||
|
&self.shortcut_bindings,
|
||||||
|
);
|
||||||
self.persist_settings_if_changed();
|
self.persist_settings_if_changed();
|
||||||
self.command_line
|
self.command_line
|
||||||
.push_output(crate::tf!("Shortcut '{key}' removed.").as_ref());
|
.push_output(crate::tf!("Shortcut '{key}' removed.").as_ref());
|
||||||
|
|||||||
@@ -4,6 +4,28 @@ use super::{Message, OpenCADStudio};
|
|||||||
use iced::Task;
|
use iced::Task;
|
||||||
use rustc_hash::FxHashMap;
|
use rustc_hash::FxHashMap;
|
||||||
use std::collections::BTreeMap;
|
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<String, String>) {
|
||||||
|
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<String, String>) -> bool {
|
||||||
|
matches!(bindings.get("SPACE"), Some(cmd) if cmd != "COMMANDSPACE")
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(target_os = "macos")]
|
#[cfg(target_os = "macos")]
|
||||||
const ACCEL: &str = "CMD";
|
const ACCEL: &str = "CMD";
|
||||||
@@ -263,6 +285,7 @@ impl OpenCADStudio {
|
|||||||
})
|
})
|
||||||
.collect();
|
.collect();
|
||||||
self.shortcut_bindings = bindings;
|
self.shortcut_bindings = bindings;
|
||||||
|
sync_space_dispatch_flag(&self.shortcut_bindings);
|
||||||
self.persist_settings_if_changed();
|
self.persist_settings_if_changed();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1390,6 +1390,7 @@ impl OpenCADStudio {
|
|||||||
self.shortcut_bindings
|
self.shortcut_bindings
|
||||||
.entry("SHIFT+TAB".to_string())
|
.entry("SHIFT+TAB".to_string())
|
||||||
.or_insert_with(|| "DYNTABPREV".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
|
/// Write the config only when it changed since the last write, so a toggle
|
||||||
|
|||||||
@@ -11499,6 +11499,29 @@ mod free_text_entry_tests {
|
|||||||
.as_ref()
|
.as_ref()
|
||||||
.is_some_and(|cmd| cmd.name() == "PASTECLIP"));
|
.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`.
|
/// Set or clear one bit of `SELECTIONPREVIEW`.
|
||||||
|
|||||||
+67
-3
@@ -171,6 +171,29 @@ fn shortcut_key_name(key: &keyboard::Key, modifiers: keyboard::Modifiers) -> Opt
|
|||||||
Some(parts.join("+"))
|
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,
|
/// `ViewportRenderMode` enum carries the raw DXF integers, not a label,
|
||||||
/// so wrap it locally with a friendly name renderer.
|
/// so wrap it locally with a friendly name renderer.
|
||||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||||
@@ -2813,9 +2836,14 @@ impl OpenCADStudio {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
let shortcut = shortcut_key_name(&key, modifiers)?;
|
let shortcut = shortcut_key_name(&key, modifiers)?;
|
||||||
(status == Status::Ignored
|
// When the command line has focus, a key that is bound
|
||||||
|| crate::app::shortcuts::is_global_key(&shortcut))
|
// to a shortcut still fires that shortcut unless it is
|
||||||
.then_some(Message::ShortcutPressed(shortcut))
|
// 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,
|
_ => None,
|
||||||
}
|
}
|
||||||
@@ -4403,4 +4431,40 @@ mod tests {
|
|||||||
super::super::StartSection::default(),
|
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<String, String> = 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));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
Reference in new issue
Block a user