Shortcuts: export, import, set, list and preset reject a param of the wrong type

shortcuts.export and shortcuts.import read `path` only when it was a
string, so a `path` such as 5 or true was dropped and the file dialog
opened. They now return "`path` must be a string" and open no dialog;
an omitted or null `path` still asks for the file.

shortcuts.set (`id`, `shortcut`), shortcuts.list (`query`) and
shortcuts.preset (`name`) give the same error for a value that is not a
string, and shortcuts.set gives "`force` must be true or false" for a
`force` that is not true, false or null. Nothing changes when a param
is rejected.
This commit is contained in:
dotCooCoo
2026-11-08 12:21:44 +07:00
committed by bflatastic
parent 313768efd5
commit 7a035cd88f
2 changed files with 81 additions and 9 deletions
+58 -9
View File
@@ -743,22 +743,35 @@ pub fn confirm(app: &mut VectorcraftApp) -> Result<Value, String> {
Ok(json!({"overrides": app.ui.shortcut_overrides.len()}))
}
/// The string `p` gives for `key`, or `None` when `p` leaves it out or gives null. Any other
/// value, such as `5`, is an error: "`{key}` must be a string".
fn text_param(p: &Value, key: &str) -> Result<Option<String>, String> {
match p.get(key) {
None | Some(Value::Null) => Ok(None),
Some(Value::String(v)) => Ok(Some(v.clone())),
Some(_) => Err(format!("`{key}` must be a string")),
}
}
/// `shortcuts.*` UI commands. `None` = not ours.
pub fn run_command(app: &mut VectorcraftApp, id: &str, p: &Value) -> Option<Result<Value, String>> {
let s = |k: &str| p.get(k).and_then(Value::as_str).map(str::to_string);
let s = |k: &str| text_param(p, k);
let r = match id {
"edit.keyboardShortcuts" => {
open(app);
Ok(Value::Null)
}
"shortcuts.set" => {
let key = s("id").unwrap_or_default();
let (key, shortcut, force) = match (s("id"), s("shortcut"), crate::menus::opt_bool(p, "force")) {
(Ok(key), Ok(shortcut), Ok(force)) => (key.unwrap_or_default(), shortcut, force.unwrap_or(false)),
(Err(e), _, _) | (_, Err(e), _) | (_, _, Err(e)) => return Some(Err(e)),
};
let mut ov = app.ui.shortcut_overrides.clone();
let r = if p.get("shortcut").is_some_and(Value::is_null) {
reset_one(&mut ov, &key);
Ok(vec![])
} else {
assign(&mut ov, &key, s("shortcut").as_deref(), p.get("force").and_then(Value::as_bool).unwrap_or(false))
assign(&mut ov, &key, shortcut.as_deref(), force)
};
r.map(|removed| {
app.ui.shortcut_overrides = ov;
@@ -768,7 +781,10 @@ pub fn run_command(app: &mut VectorcraftApp, id: &str, p: &Value) -> Option<Resu
})
}
"shortcuts.list" => {
let q = s("query").unwrap_or_default().to_lowercase();
let q = match s("query") {
Ok(q) => q.unwrap_or_default().to_lowercase(),
Err(e) => return Some(Err(e)),
};
let km = keymap_with(&app.ui.shortcut_overrides);
let ov = km.overrides();
Ok(Value::Array(
@@ -789,23 +805,28 @@ pub fn run_command(app: &mut VectorcraftApp, id: &str, p: &Value) -> Option<Resu
sync(&app.ui);
Ok(Value::Null)
}
"shortcuts.preset" => match s("name").as_deref().map(set_name).and_then(|n| preset(n).map(|p| (n.to_string(), p))) {
Some((n, p)) => {
"shortcuts.preset" => match s("name").map(|n| n.as_deref().map(set_name).and_then(|n| preset(n).map(|p| (n.to_string(), p)))) {
Ok(Some((n, p))) => {
app.ui.shortcut_overrides = p;
app.ui.shortcut_set = n;
sync(&app.ui);
Ok(Value::Null)
}
None => Err(format!("unknown preset (one of {})", PRESETS.join(", "))),
Ok(None) => Err(format!("unknown preset (one of {})", PRESETS.join(", "))),
Err(e) => Err(e),
},
"shortcuts.export" => {
let path = match s("path") {
Ok(path) => path,
Err(e) => return Some(Err(e)),
};
let v = export_json(&app.ui.shortcut_set, &app.ui.shortcut_overrides);
let bytes = serde_json::to_vec_pretty(&v).unwrap_or_default();
let pick = crate::FilePick {
filters: vec![crate::FilePick::filter("Shortcut sets", &["json"])],
..crate::FilePick::named("VectorCraft Shortcuts.json")
};
let path = s("path").or_else(|| crate::picks::save(app, &pick));
let path = path.or_else(|| crate::picks::save(app, &pick));
match path {
Some(path) => match app.services.write.as_mut() {
Some(w) => w(&path, &bytes).map(|_| json!({"path": path})),
@@ -848,11 +869,15 @@ pub fn run_command(app: &mut VectorcraftApp, id: &str, p: &Value) -> Option<Resu
let data = if let Some(v) = p.get("data") {
Ok(v.clone())
} else {
let path = match s("path") {
Ok(path) => path,
Err(e) => return Some(Err(e)),
};
let pick = crate::FilePick {
filters: vec![crate::FilePick::filter("Shortcut sets", crate::dialogs::shortcut_import::IMPORT_EXTS)],
..Default::default()
};
let path = s("path").or_else(|| crate::picks::open(app, &pick));
let path = path.or_else(|| crate::picks::open(app, &pick));
// A `.kys` file is read by Import Shortcut Set, up to its reader's cap.
if let Some(path) = path.as_deref().filter(|p| crate::dialogs::shortcut_import::is_kys(p)) {
return Some(crate::dialogs::shortcut_import::command(app, &json!({ "path": path })));
@@ -1827,4 +1852,28 @@ mod tests {
assign(&mut ov, "object.isolate", Some("Cmd+Alt+Shift+F11"), false).unwrap();
assert_eq!(effective_in(&ov, "object.isolate").as_deref(), Some("Cmd+Alt+Shift+F11"));
}
/// `shortcuts.set`, `shortcuts.list` and `shortcuts.preset` reject a supplied param of the wrong
/// type with an error naming it, and change nothing; an omitted or null param keeps its default.
#[test]
fn shortcut_commands_reject_a_param_of_the_wrong_type() {
let mut app = VectorcraftApp::new(vectorcraft_engine::Session::new(), Default::default());
let set = app.ui.shortcut_set.clone();
for (id, p, err) in [
("shortcuts.set", json!({"id": 5, "shortcut": "Cmd+Alt+Shift+F11"}), "`id` must be a string"),
("shortcuts.set", json!({"id": "object.isolate", "shortcut": 7}), "`shortcut` must be a string"),
("shortcuts.set", json!({"id": "object.isolate", "shortcut": "Cmd+Alt+Shift+F11", "force": "yes"}), "`force` must be true or false"),
("shortcuts.list", json!({"query": 5}), "`query` must be a string"),
("shortcuts.preset", json!({"name": true}), "`name` must be a string"),
] {
assert_eq!(app.run(id, p.clone()), Err(err.to_string()), "{id} {p}");
assert!(app.ui.shortcut_overrides.is_empty(), "{id} {p}");
assert_eq!(app.ui.shortcut_set, set, "{id} {p}");
}
let all = app.run("shortcuts.list", json!({})).unwrap();
assert_eq!(app.run("shortcuts.list", json!({"query": null})), Ok(all));
let r = app.run("shortcuts.set", json!({"id": "object.isolate", "shortcut": "Cmd+Alt+Shift+F11", "force": null})).unwrap();
assert_eq!(r["shortcut"], "Cmd+Alt+Shift+F11");
assert!(app.run("shortcuts.preset", json!({"name": null})).unwrap_err().starts_with("unknown preset"));
}
}
+23
View File
@@ -463,6 +463,29 @@ fn keyboard_shortcuts_import_opens_a_text_set_over_the_dialog() {
let _ = std::fs::remove_dir_all(dir);
}
/// `shortcuts.export` and `shortcuts.import` given a `path` that is not a string open no file
/// dialog, read and write nothing, and give an error naming `path`; a null `path` still asks.
#[test]
fn keyboard_shortcuts_commands_reject_a_path_that_is_not_a_string() {
let (mut app, shown, written) = app();
let read: Written = Default::default();
let r = read.clone();
app.services.read = Some(Box::new(move |path: &str| {
r.borrow_mut().push(path.to_string());
Ok(SVG.to_vec())
}));
for path in [json!(5), json!(true), json!({"file": "/keys/a.json"}), json!(["/keys/a.json"])] {
for id in ["shortcuts.export", "shortcuts.import"] {
assert_eq!(app.run(id, json!({ "path": path })), Err("`path` must be a string".to_string()), "{id} {path}");
assert!(shown.requests().is_empty(), "{id} {path}: {:?}", shown.requests());
}
}
assert!(written.borrow().is_empty() && read.borrow().is_empty());
assert!(app.ui.shortcut_overrides.is_empty());
assert_eq!(app.run("shortcuts.export", json!({"path": null})), Err("cancelled".to_string()));
assert!(matches!(shown.requests().as_slice(), [PickRequest::Save(_)]));
}
/// A dialog's folder button answered after the dialog closed fills nothing, and the status says
/// why.
#[test]