diff --git a/.ast-grep/rules/core-app-overlay-owner.yml b/.ast-grep/rules/core-app-overlay-owner.yml new file mode 100644 index 000000000..4c59cb910 --- /dev/null +++ b/.ast-grep/rules/core-app-overlay-owner.yml @@ -0,0 +1,25 @@ +# Owner: openlogi_core::app::overlay_for — how a per-app map is matched +# against the foreground identifier: exact key first, then the Windows +# `exe:` fallback. Every per-app map (button overrides, Actions Ring +# layouts) resolves through it, so one selector means one thing everywhere. +# An editor's exact-key lookup (`per_app_overrides`) is a different question +# and is not matched here. +id: core-app-overlay-owner +language: rust +severity: error +message: Matching a per-app key against the foreground identifier is openlogi_core::app::overlay_for's; call it instead of building an `exe:` key or indexing the ring's per_app map by the raw identifier here. +note: | + action_ring.per_app looked the identifier up verbatim while + per_app_bindings fell back to `exe:` (#643), so a Windows Store + app kept its button overrides across an update and lost its ring layout. + See "Single source of truth" in AGENTS.md. +files: + - crates/**/*.rs +ignores: + - crates/openlogi-core/src/app.rs + - "**/tests.rs" + - "**/tests/**" +rule: + any: + - pattern: format!("exe:$$$") + - pattern: $MAP.per_app.get($$$) diff --git a/crates/openlogi-core/src/app.rs b/crates/openlogi-core/src/app.rs index 0eeb33de4..89071f894 100644 --- a/crates/openlogi-core/src/app.rs +++ b/crates/openlogi-core/src/app.rs @@ -6,6 +6,9 @@ //! the wire. That last one makes this a wire type — see //! `crates/openlogi-ipc/AGENTS.md`. +use std::collections::BTreeMap; +use std::path::Path; + use serde::{Deserialize, Serialize}; /// One application, named the way a per-app profile names it. @@ -41,3 +44,89 @@ impl ForegroundApp { } } } + +/// Resolve the most specific per-app entry for a foreground identifier — the +/// one matcher every per-app map uses, so a selector cannot mean one thing for +/// button overrides and another for Actions Ring layouts. +/// +/// The exact key wins. On Windows the identifier is a lower-cased executable +/// path, so `exe:` is the stable fallback for Store and self-updating +/// applications whose install directory changes between versions; both path +/// separators are recognized so hand-authored Windows config stays inspectable +/// on every platform. An identifier with no path separator is a macOS bundle +/// identifier or a Linux application class, never an executable, so a name +/// that merely ends in `.exe` is not reinterpreted as one. +#[must_use] +pub fn overlay_for<'a, T>(overlays: &'a BTreeMap, app: &str) -> Option<&'a T> { + overlays.get(app).or_else(|| { + let (_, executable_name) = app.rsplit_once(['\\', '/'])?; + if !Path::new(executable_name) + .extension() + .is_some_and(|ext| ext.eq_ignore_ascii_case("exe")) + { + return None; + } + overlays.get(&format!("exe:{}", executable_name.to_ascii_lowercase())) + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn overlays(keys: &[&str]) -> BTreeMap { + keys.iter() + .map(|key| ((*key).to_string(), "overlay")) + .collect() + } + + #[test] + fn the_exact_key_wins_over_the_executable_fallback() { + let mut map = BTreeMap::new(); + map.insert( + r"c:\program files\windowsapps\sharex_16.0_x64\sharex.exe".to_string(), + "exact", + ); + map.insert("exe:sharex.exe".to_string(), "fallback"); + assert_eq!( + overlay_for( + &map, + r"c:\program files\windowsapps\sharex_16.0_x64\sharex.exe" + ), + Some(&"exact") + ); + } + + #[test] + fn a_versioned_path_falls_back_to_the_executable_name() { + let map = overlays(&["exe:sharex.exe"]); + // The install directory carries the version, so only the basename is + // stable across updates. + for path in [ + r"c:\program files\windowsapps\sharex_16.0_x64\sharex.exe", + r"c:\program files\windowsapps\sharex_17.1_x64\sharex.exe", + r"C:\Program Files\ShareX\ShareX.EXE", + "/c/program files/sharex/sharex.exe", + ] { + assert_eq!(overlay_for(&map, path), Some(&"overlay"), "{path}"); + } + } + + #[test] + fn identifiers_that_name_no_executable_never_fall_back() { + let map = overlays(&["exe:code.exe", "exe:com.example.exe", "exe:.exe"]); + // macOS bundle ids and Linux classes are not paths, even when they end + // in `.exe`; a path with no executable name has nothing stable to match. + for app in [ + "com.microsoft.VSCode", + "com.example.exe", + "Firefox", + "code.exe", + r"c:\program files\microsoft vs code\code.exe.bak", + r"c:\program files\microsoft vs code\", + "", + ] { + assert_eq!(overlay_for(&map, app), None, "{app}"); + } + } +} diff --git a/crates/openlogi-core/src/binding/action_ring.rs b/crates/openlogi-core/src/binding/action_ring.rs index 1259dc258..a1c7323d5 100644 --- a/crates/openlogi-core/src/binding/action_ring.rs +++ b/crates/openlogi-core/src/binding/action_ring.rs @@ -12,6 +12,7 @@ use serde::{Deserialize, Serialize}; use thiserror::Error; use super::Action; +use crate::app::overlay_for; mod icon; @@ -324,10 +325,14 @@ impl ActionRingConfig { } /// Resolve the complete layout for the foreground application. + /// + /// `per_app` takes the same selectors as `per_app_bindings` and is matched + /// the same way ([`overlay_for`]): the exact key, then on Windows the + /// `exe:` fallback a versioned install path needs. #[must_use] pub fn effective_layout(&self, app_id: Option<&str>) -> ActionRingLayout { app_id - .and_then(|app| self.per_app.get(app)) + .and_then(|app| overlay_for(&self.per_app, app)) .cloned() .unwrap_or_else(|| self.default.clone()) } @@ -493,4 +498,34 @@ Bottom = { action = { CustomShortcut = "Cmd+Shift+P" } } assert_eq!(config.effective_layout(Some("com.apple.Safari")), safari); assert_eq!(config.effective_layout(Some("other")), config.default); } + + #[test] + fn a_ring_layout_keyed_by_executable_survives_a_versioned_install_path() { + let mut config = ActionRingConfig::default(); + let sharex = ActionRingLayout { + slots: BTreeMap::from([( + ActionRingSlot::Top, + ActionRingEntry::new( + RingAction::new(Action::Copy).expect("copy must be a valid ring action"), + ), + )]), + }; + config + .per_app + .insert("exe:sharex.exe".to_string(), sharex.clone()); + + // The same selector `per_app_bindings` honours: a Store app's path + // changes with every update, its executable name does not. + assert_eq!( + config.effective_layout(Some( + r"c:\program files\windowsapps\sharex_17.1_x64\sharex.exe" + )), + sharex + ); + assert_eq!( + config.effective_layout(Some("com.getsharex.exe")), + config.default, + "a bundle id is never reinterpreted as an executable" + ); + } } diff --git a/crates/openlogi-core/src/config/per_app.rs b/crates/openlogi-core/src/config/per_app.rs index 8ebafa114..8112d143a 100644 --- a/crates/openlogi-core/src/config/per_app.rs +++ b/crates/openlogi-core/src/config/per_app.rs @@ -2,12 +2,14 @@ //! application, and how the application in front is matched to them. //! //! An override replaces a whole button with a single action. Editing addresses -//! a profile by its exact key; matching also falls back from a Windows -//! executable path to that executable's `exe:` key. +//! a profile by its exact key; matching goes through [`overlay_for`], which +//! also falls back from a Windows executable path to that executable's +//! `exe:` key. -use std::{collections::BTreeMap, path::Path}; +use std::collections::BTreeMap; use super::Config; +use crate::app::overlay_for; use crate::binding::{Action, Binding, ButtonId}; impl Config { @@ -29,7 +31,7 @@ impl Config { }; let mut out = device.bindings.clone(); if let Some(bid) = bundle_id - && let Some(overlay) = app_overlay(&device.per_app_bindings, bid) + && let Some(overlay) = overlay_for(&device.per_app_bindings, bid) { for (k, v) in overlay { out.insert(*k, Binding::Single(v.clone())); @@ -112,30 +114,7 @@ impl Config { #[must_use] pub fn has_app_override(&self, device_key: &str, app: &str) -> bool { self.devices.get(device_key).is_some_and(|d| { - app_overlay(&d.per_app_bindings, app).is_some_and(|overlay| !overlay.is_empty()) + overlay_for(&d.per_app_bindings, app).is_some_and(|overlay| !overlay.is_empty()) }) } } - -/// Resolve the most specific application overlay for a foreground identifier. -/// -/// Exact keys retain precedence. On Windows the foreground identifier is a -/// lower-cased executable path, so `exe:` provides a stable fallback -/// for Store and self-updating applications whose install directory changes -/// between versions. Recognizing both path separators keeps hand-authored -/// Windows config inspectable on every platform without changing macOS bundle -/// identifiers or Linux application classes. -fn app_overlay<'a, T>(overlays: &'a BTreeMap, app: &str) -> Option<&'a T> { - overlays.get(app).or_else(|| { - let executable_name = app.rsplit(['\\', '/']).next()?; - if executable_name.is_empty() - || !Path::new(executable_name) - .extension() - .is_some_and(|ext| ext.eq_ignore_ascii_case("exe")) - { - return None; - } - - overlays.get(&format!("exe:{}", executable_name.to_ascii_lowercase())) - }) -} diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 0cb14db73..ba55400be 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -92,7 +92,9 @@ Common device fields are: differently and a profile authored under one namespace will not match under another. An overlay holds one action per button; gesture-direction maps live in `bindings` -- `action_ring`: default and complete per-application eight-slot layouts +- `action_ring`: default and complete per-application eight-slot layouts; + `action_ring.per_app` takes the same selectors as `per_app_bindings`, + including the Windows `exe:` fallback - `lighting`, `smartshift`, standalone `light`, and camera controls / profiles - `host_switch_targets` and `fn_lock` for compatible keyboards - `identity` and `disabled_gestures`, which are application-managed metadata