M16.1: shortcuts — Lightroom-desktop secondary bindings, no double-firing keys
Adds secondary bindings for existing commands (Cmd+D select none, Shift+E export dialog, Space toggle zoom, Shift+M create version, Shift+X / Shift+U reject / unflag and advance), listed in Help > Keyboard Shortcuts. Shift+Y no longer also toggles the History panel; a UI command now shadows the engine command it wraps on the same key (W, Cmd+Shift+I). New tests reject conflicting bindings. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
b13274f711
commit
382cca190c
3 files changed
+93
-5
No files matched your search
@@ -39,7 +39,7 @@ pub const UI_COMMANDS: &[UiCommand] = &[
|
||||
("panel.info", "Info", Some("I"), "View"),
|
||||
("panel.keywords", "Keywords", Some("K"), "View"),
|
||||
("panel.versions", "Versions", Some("Shift+V"), "View"),
|
||||
("panel.activity", "History", Some("Shift+Y"), "View"),
|
||||
("panel.activity", "History", None, "View"),
|
||||
("panel.close", "Close Panel", None, "View"),
|
||||
("section.light", "Light", Some("Cmd+Alt+1"), "View"),
|
||||
("section.color", "Color", Some("Cmd+Alt+2"), "View"),
|
||||
|
||||
@@ -180,6 +180,18 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) {
|
||||
ui.end_row();
|
||||
}
|
||||
}
|
||||
for (sc, id, _) in crate::shortcuts::ALIASES {
|
||||
let label = crate::menus::UI_COMMANDS
|
||||
.iter()
|
||||
.find(|c| c.0 == *id)
|
||||
.map(|c| c.1)
|
||||
.or_else(|| lightcraft_engine::find_command(id).map(|c| c.label))
|
||||
.unwrap_or(id);
|
||||
ui.label(label);
|
||||
ui.label(*sc);
|
||||
ui.label(egui::RichText::new(*id).color(t.text_dim));
|
||||
ui.end_row();
|
||||
}
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
@@ -5,6 +5,18 @@ use serde_json::json;
|
||||
|
||||
use crate::LightcraftApp;
|
||||
|
||||
/// Secondary key bindings for commands that already exist: `(shortcut, command id, params JSON)`.
|
||||
/// They complement the primary shortcut declared on the command (Lightroom-desktop keys that our
|
||||
/// primary keymap assigns elsewhere, see docs/parity.md → Shortcuts). Shown in Help → Keyboard Shortcuts.
|
||||
pub const ALIASES: &[(&str, &str, &str)] = &[
|
||||
("Cmd+D", "library.selectNone", "{}"),
|
||||
("Shift+E", "dialog.export", "{}"),
|
||||
("Space", "view.zoomToggle", "{}"),
|
||||
("Shift+M", "version.create", "{}"),
|
||||
("Shift+X", "photo.flag", r#"{"flag": "reject", "advance": true}"#),
|
||||
("Shift+U", "photo.flag", r#"{"flag": "none", "advance": true}"#),
|
||||
];
|
||||
|
||||
pub fn parse(s: &str) -> Option<(Modifiers, Key)> {
|
||||
let mut m = Modifiers::NONE;
|
||||
let mut key = None;
|
||||
@@ -50,21 +62,34 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context) {
|
||||
return;
|
||||
}
|
||||
let mut fire: Vec<String> = Vec::new();
|
||||
let mut aliased: Vec<(&str, serde_json::Value)> = Vec::new();
|
||||
ctx.input(|i| {
|
||||
let mut ui_keys = Vec::new();
|
||||
for (id, _, sc, _) in crate::menus::UI_COMMANDS {
|
||||
if let Some((m, k)) = sc.and_then(parse)
|
||||
&& matches(i, m, k)
|
||||
{
|
||||
fire.push(id.to_string());
|
||||
if let Some((m, k)) = sc.and_then(parse) {
|
||||
ui_keys.push((m, k));
|
||||
if matches(i, m, k) {
|
||||
fire.push(id.to_string());
|
||||
}
|
||||
}
|
||||
}
|
||||
for c in lightcraft_engine::command_specs() {
|
||||
// A UI command bound to the same key wraps the engine command (e.g. `W` opens the
|
||||
// White Balance Selector tool rather than sampling without a point): the UI one wins.
|
||||
if let Some((m, k)) = c.shortcut.and_then(parse)
|
||||
&& !ui_keys.contains(&(m, k))
|
||||
&& matches(i, m, k)
|
||||
{
|
||||
fire.push(c.id.to_string());
|
||||
}
|
||||
}
|
||||
for (sc, id, params) in ALIASES {
|
||||
if let Some((m, k)) = parse(sc)
|
||||
&& matches(i, m, k)
|
||||
{
|
||||
aliased.push((id, serde_json::from_str(params).unwrap_or_default()));
|
||||
}
|
||||
}
|
||||
// rating 0-5, colour labels 6-9 (with Shift: and advance)
|
||||
for (n, key) in [Key::Num0, Key::Num1, Key::Num2, Key::Num3, Key::Num4, Key::Num5].iter().enumerate() {
|
||||
if matches(i, Modifiers::NONE, *key) {
|
||||
@@ -80,6 +105,9 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context) {
|
||||
}
|
||||
}
|
||||
});
|
||||
for (id, params) in aliased {
|
||||
let _ = app.run(id, params);
|
||||
}
|
||||
for f in fire {
|
||||
if let Some(rest) = f.strip_prefix("rate:") {
|
||||
let (n, adv) = rest.split_once(':').unwrap_or(("0", "0"));
|
||||
@@ -133,4 +161,52 @@ mod tests {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn aliases_parse_and_target_existing_commands() {
|
||||
let ui: Vec<&str> = crate::menus::UI_COMMANDS.iter().map(|c| c.0).collect();
|
||||
for (sc, id, params) in ALIASES {
|
||||
assert!(parse(sc).is_some(), "{id}: {sc}");
|
||||
assert!(ui.contains(id) || lightcraft_engine::find_command(id).is_some(), "alias {sc} → unknown command {id}");
|
||||
assert!(serde_json::from_str::<serde_json::Value>(params).is_ok(), "alias {sc}: bad params");
|
||||
}
|
||||
}
|
||||
|
||||
/// Engine commands that intentionally share a key and are disambiguated by context in [`handle`].
|
||||
const CONTEXTUAL: &[(&str, &str)] = &[("photo.reject", "crop.rotateAspect")];
|
||||
|
||||
/// No key fires two different actions (a UI command may shadow the engine command it wraps).
|
||||
#[test]
|
||||
fn no_conflicting_bindings() {
|
||||
let mut ui: Vec<((Modifiers, Key), String)> = Vec::new();
|
||||
for (id, _, sc, _) in crate::menus::UI_COMMANDS {
|
||||
if let Some(k) = sc.and_then(parse) {
|
||||
ui.push((k, id.to_string()));
|
||||
}
|
||||
}
|
||||
for (sc, id, _) in ALIASES {
|
||||
ui.push((parse(sc).unwrap(), format!("alias {id}")));
|
||||
}
|
||||
let mut engine: Vec<((Modifiers, Key), &str)> = Vec::new();
|
||||
for c in lightcraft_engine::command_specs() {
|
||||
if let Some(k) = c.shortcut.and_then(parse) {
|
||||
engine.push((k, c.id));
|
||||
}
|
||||
}
|
||||
for (sc, id, _) in ALIASES {
|
||||
let k = parse(sc).unwrap();
|
||||
assert!(!engine.iter().any(|(k2, _)| *k2 == k), "alias {sc} ({id}) shadows an engine shortcut");
|
||||
}
|
||||
for (i, (k, a)) in ui.iter().enumerate() {
|
||||
for (k2, b) in &ui[i + 1..] {
|
||||
assert!(k != k2, "{a} and {b} share a key");
|
||||
}
|
||||
}
|
||||
for (i, (k, a)) in engine.iter().enumerate() {
|
||||
for (k2, b) in &engine[i + 1..] {
|
||||
let contextual = CONTEXTUAL.iter().any(|(x, y)| (x == a && y == b) || (x == b && y == a));
|
||||
assert!(k != k2 || contextual, "{a} and {b} share a key");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user