diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 56f839ad83..35c2ba2984 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -345,12 +345,10 @@ jobs: with: path: docs/wiki/ - publish-docs: + build-docs: if: github.event_name == 'push' && github.ref == 'refs/heads/main' && (github.repository == 'niri-wm/niri' || github.repository == 'barrulus/biri') needs: - test - permissions: - contents: write runs-on: ubuntu-24.04 steps: - uses: actions/checkout@v6 @@ -376,14 +374,23 @@ jobs: - name: Generate rustdoc documentation run: cargo doc --no-deps -p niri-ipc - - run: mkdir -p publish/niri_ipc - - run: cp -r ./target/doc/* ./publish/ - - run: cp -r ./docs/site/* ./publish/ + - run: | + mkdir -p publish/niri_ipc + cp -r ./target/doc/* ./publish/ + cp -r ./docs/site/* ./publish/ - - name: Deploy documentation - if: github.ref == 'refs/heads/main' - uses: peaceiris/actions-gh-pages@v4 + - name: Upload pages artifact + uses: actions/upload-pages-artifact@v5 with: - github_token: ${{ secrets.GITHUB_TOKEN }} - publish_dir: ./publish - force_orphan: true + path: ./publish + + publish-docs: + runs-on: ubuntu-24.04 + needs: + - build-docs + permissions: + id-token: write + pages: write + steps: + - name: Deploy artifact to GitHub Pages + uses: actions/deploy-pages@v5 diff --git a/docs/wiki/Configuration:-Debug-Options.md b/docs/wiki/Configuration:-Debug-Options.md index ae546b536d..da01c89ce6 100644 --- a/docs/wiki/Configuration:-Debug-Options.md +++ b/docs/wiki/Configuration:-Debug-Options.md @@ -35,6 +35,7 @@ debug { skip-cursor-only-updates-during-vrr deactivate-unfocused-windows disable-10bit-output + nested-show-host-cursor } binds { @@ -399,3 +400,15 @@ binds { Mod+Shift+Ctrl+D { debug-toggle-damage; } } ``` + +### `nested-show-host-cursor` + +Since: next release + +Shows the host cursor when running niri as a nested window, on top of the nested niri cursor. + +```kdl +debug { + nested-show-host-cursor +} +``` diff --git a/docs/wiki/Configuration:-Key-Bindings.md b/docs/wiki/Configuration:-Key-Bindings.md index d981a929e7..52bd3aa331 100644 --- a/docs/wiki/Configuration:-Key-Bindings.md +++ b/docs/wiki/Configuration:-Key-Bindings.md @@ -15,7 +15,8 @@ binds { } ``` -The hotkey consists of modifiers separated by `+` signs, followed by an XKB key name in the end. +The hotkey consists of modifiers separated by `+` signs, followed by a trigger key in the end. +The trigger key can be either an XKB key name or Since: next release a modifier. Valid modifiers are: @@ -31,6 +32,38 @@ Valid modifiers are: This way, you can test niri in a window without causing too many conflicts with the host compositor's key bindings. For this reason, most of the default keys use the `Mod` modifier. +Specific left or right modifier keys like `Alt_L` or `Alt_R` can be used as triggers, but not as modifiers: + +```kdl +binds { + // Valid: + Ctrl+Alt_L { spawn "alacritty"; } + + // Invalid: + // Alt_L+Ctrl { spawn "alacritty"; } + + // Also, this is equivalent to just Alt_L + // because Alt_L implies the Alt modifier: + Alt+Alt_L { spawn "the Alt here is unnecessary but allowed"; } + // This is the same: + // Alt_L { spawn "alacritty"; } +} +``` + +Since: next release Generic modifiers like `Shift` or `Alt` can be used as trigger keys. +Binds consisting only of generic modifiers do not depend on the order that you press them: + +```kdl +binds { + // Will trigger both when pressing Alt then Shift, and Shift then Alt. + Alt+Shift { spawn "alacritty"; } + + // The trigger is not a generic modifier, so + // you need to press it last. Only Ctrl then Left Shift will work. + Ctrl+Shift_L { spawn "alacritty"; } +} +``` + Since: 25.05 You can customize the `Mod` key [in the `input` section of the config](./Configuration:-Input.md#mod-key-mod-key-nested). > [!TIP] @@ -53,14 +86,25 @@ For this reason, most of the default keys use the `Mod` modifier. > Here, look at `sym: Left` and `sym: Right`: these are the key names. > I was pressing the left and the right arrow in this example. > -> Keep in mind that binding shifted keys requires spelling out Shift and the unshifted version of the key, according to your XKB layout. -> For example, on the US QWERTY layout, < is on Shift + ,, so to bind it, you spell out something like `Mod+Shift+Comma`. +> Keep in mind that binding a key requires spelling out the modifiers that you hold when typing it. +> So, symbols that you type while holding Shift need the Shift modifier in the corresponding binding. +> For example, on the US QWERTY layout, < is on Shift + ,, so to bind it, you spell out something like `Mod+Shift+Less` or `Mod+Shift+Comma` (just `Mod+Less` will not work). > > As another example, if you've configured the French [BÉPO](https://en.wikipedia.org/wiki/B%C3%89PO) XKB layout, your < is on AltGr + «. -> AltGr is `ISO_Level3_Shift`, or equivalently `Mod5`, so to bind it, you spell out something like `Mod+Mod5+guillemotleft`. +> AltGr is `ISO_Level3_Shift`, or equivalently `Mod5`, so to bind it, you spell out something like `Mod+Mod5+Less` or `Mod+Mod5+guillemotleft`. > -> When resolving latin keys, niri will search for the *first* configured XKB layout that has the latin key. -> So for example with US QWERTY and RU layouts configured, US QWERTY will be used for latin binds. +> When resolving ASCII (latin) keys, niri will search for the *first* configured XKB layout that has the ASCII key. +> So for example with US Colemak and RU layouts configured, US Colemak will be used for ASCII binds. +> +> Keys from the current layout have priority though. +> For example, if you have US QWERTY and Russian layouts, and both `Mod+Z` and `Mod+Cyrillic_ya` binds, then on Russian the `Mod+Cyrillic_ya` bind will trigger, while on US, the `Mod+Z` bind will trigger. +> +> Also, binds that spell out the typed character have priority over binds that spell out an unshifted character. +> For example, on US QWERTY, `Mod+Shift+At` will have priority over `Mod+Shift+2` if both are configured. +> +> Since: next release You used to need to spell out the *unshifted* version of the trigger key. +> So to bind < on US QWERTY you could use only `Mod+Shift+Comma` (since , is the unshifted version of that key). +> This is no longer the case: you can spell out either version of the key. Since: 0.1.8 Binds will repeat by default (i.e. holding down a bind will make it trigger repeatedly). You can disable that for specific binds with `repeat=false`: @@ -81,6 +125,93 @@ binds { This is mostly useful for the scroll bindings. +### Press and Release Bindings + +Since: next release + +Binds can be set to trigger on key press, on key release, or both. +By default, binds trigger on key press. +You can change it using `press {}` and `release {}` blocks: + +```kdl +binds { + // Trigger on press (default behavior). + Mod+T { spawn "alacritty"; } + + // Trigger on release. + // Press and release Mod by itself to open the overview. + Mod { + release { toggle-overview; } + } + + // Trigger on both press and release with different actions. + // + // If the press action runs, the release action is guaranteed to run, + // no matter how you release the keys. + // You can safely use these binds for things like push-to-talk. + Control_R { + press { spawn "notify-send" "Enable push-to-talk"; } + release { spawn "notify-send" "Disable push-to-talk"; } + } + + // Release binds trigger only if you don't press anything else in between. + // So, an Alt+Shift bind like this still lets you hit + // Alt+Shift+arrows and other shortcuts in applications. + Alt+Shift { + release { switch-layout "next"; } + } +} +``` + +Release bindings are especially useful when you want to bind a modifier key. +A normal press bind would effectively prevent you from having any other bind containing that modifier. + +Release-only binds only trigger when you don't press anything else while holding it. +If you have a release-only `Mod` bind from the example above, and press any other key or mouse button while holding `Mod`, the bind will not trigger (the overview won't open). + +In bindings with both press and release, the release action is guaranteed to trigger if the press action triggers. +So, your push-to-talk press-release binding will never get stuck pressed, no matter how you release the keys. + +By default, any bind with both press and release action does not repeat. +You can explicitly set `repeat=true` if needed: + +```kdl +binds { + // Doesn't repeat automatically + // because it has a release {} section. + Mod+Shift+Q repeat=true { + press { spawn "notify-send" "Pressed (repeating)"; } + release { spawn "notify-send" "Released"; } + } +} +``` + +By default, bound key presses/releases are not passed to applications. +You can set `pass-through=true` if you want applications to see the bound key: + +```kdl +binds { + // Let applications see the Alt key presses/releases. + Alt pass-through=true { + press { spawn "notify-send" "Pressed"; } + release { spawn "notify-send" "Released"; } + } +} +``` + +Keyboard release-only bindings are passed through by default though. +You can set `pass-through=false` on those if you don't like that: + +```kdl +binds { + // Do not pass Mod press/release event to applications. + // (They will still see the modifiers event.) + Mod pass-through=false { + release { toggle-overview; } + } +} +``` + ### Scroll Bindings You can bind mouse wheel scroll ticks using the following syntax. @@ -199,12 +330,23 @@ binds { } ``` +`press {}` and `release {}` blocks can also hold several actions each. + +```kdl +binds { + Mod+P { + press { spawn "notify-send" "Recording"; spawn "pw-record" "/tmp/out.wav"; } + release { spawn "pkill" "pw-record"; } + } +} +``` + Two limitations apply to multi-action binds: -- They never appear in the hotkey overlay, and `hotkey-overlay-title` has no effect on - them. -- `allow-when-locked=true` is accepted only when *every* action in the bind is `spawn` - or `spawn-sh`. Otherwise the flag would let a non-spawn action run from the lock +- A press or release with several actions never appears in the hotkey overlay, and + `hotkey-overlay-title` has no effect on it. +- `allow-when-locked=true` is accepted only when *every* action in the bind, on press + and on release, is `spawn` or `spawn-sh`. Otherwise the flag would let a non-spawn action run from the lock screen. If an action cannot run — for example a non-spawn action on a locked screen — it is diff --git a/docs/wiki/Configuration:-Recent-Windows.md b/docs/wiki/Configuration:-Recent-Windows.md index 162abf2b22..d9d29a5a7a 100644 --- a/docs/wiki/Configuration:-Recent-Windows.md +++ b/docs/wiki/Configuration:-Recent-Windows.md @@ -167,7 +167,23 @@ recent-windows { The recent windows binds have lower precedence than the [normal binds](./Configuration:-Key-Bindings.md), meaning that if you have AltTab bound to something else in the normal binds, the `recent-windows` bind won't work. In this case, you can remove the conflicting normal bind. -All binds in this section must have a modifier key like Alt or Mod because the recent windows switcher remains open only while you hold any modifier key. +All binds in this section must have a modifier key like Alt or Mod (that isn't the same as the trigger key) because the recent windows switcher remains open only while you hold any modifier key. + +```kdl +recent-windows { + binds { + // Allowed: has a modifier, two keys. + Ctrl+Alt { next-window; } + + // Not allowed: only a single key. + // Alt { next-window; } + + // Not allowed: modifier matches the key + // (effectively only a single key). + // Alt+Alt_L { next-window; } + } +} +``` #### Bindings inside the switcher diff --git a/niri-config/src/animations.rs b/niri-config/src/animations.rs index 957a43b871..5306ecad88 100644 --- a/niri-config/src/animations.rs +++ b/niri-config/src/animations.rs @@ -170,8 +170,12 @@ fn parse_custom_shader_path( for (name, val) in &spanned.properties { if &***name == "path" { let path_val = knuffel::traits::DecodeScalar::decode(val, ctx)?; - let Some(path) = expand_home_path(path_val, spanned, ctx) else { - continue; + let path = match expand_home_path(path_val, spanned, ctx) { + Ok(p) => p, + Err(e) => { + ctx.emit_error(e); + continue; + } }; let includes = ctx.get::>>().unwrap(); diff --git a/niri-config/src/binds.rs b/niri-config/src/binds.rs index 73636649ad..0057547125 100644 --- a/niri-config/src/binds.rs +++ b/niri-config/src/binds.rs @@ -1,5 +1,3 @@ -use std::collections::hash_map::Entry; -use std::collections::HashMap; use std::str::FromStr; use std::time::Duration; @@ -12,7 +10,9 @@ use niri_ipc::{ use smithay::input::keyboard::keysyms::KEY_NoSymbol; use smithay::input::keyboard::xkb::{keysym_from_name, KEYSYM_CASE_INSENSITIVE, KEYSYM_NO_FLAGS}; use smithay::input::keyboard::Keysym; +use tracing::warn; +use crate::input::ModKey; use crate::recent_windows::{MruDirection, MruFilter, MruScope}; use crate::utils::{expect_only_children, MergeWith}; @@ -22,14 +22,84 @@ pub struct Binds(pub Vec); #[derive(Debug, Clone, PartialEq)] pub struct Bind { pub key: Key, - pub actions: Vec, + pub action: BoundAction, pub repeat: bool, + pub pass_through: Option, pub cooldown: Option, pub allow_when_locked: bool, pub allow_inhibiting: bool, pub hotkey_overlay_title: Option>, } +/// The actions a bind runs. Each list is non-empty and runs in order. +#[derive(Debug, Clone, PartialEq)] +pub enum BoundAction { + Press(Vec), + Release(Vec), + Both { + press: Vec, + release: Vec, + }, +} + +impl BoundAction { + /// Iterates over all press actions, then all release actions. + pub fn iter(&self) -> impl Iterator { + let (press, release): (&[Action], &[Action]) = match self { + BoundAction::Press(press) => (press, &[]), + BoundAction::Release(release) => (&[], release), + BoundAction::Both { press, release } => (press, release), + }; + press.iter().chain(release) + } +} + +impl Bind { + pub fn has_press(&self) -> bool { + matches!( + self.action, + BoundAction::Press(_) | BoundAction::Both { .. } + ) + } + + pub fn has_release(&self) -> bool { + matches!( + self.action, + BoundAction::Release(_) | BoundAction::Both { .. } + ) + } + + /// Returns the press actions, or an empty slice for release-only binds. + pub fn press_actions(&self) -> &[Action] { + match &self.action { + BoundAction::Press(actions) => actions, + BoundAction::Both { press, .. } => press, + BoundAction::Release(_) => &[], + } + } + + /// Returns the release actions, or an empty slice for press-only binds. + pub fn release_actions(&self) -> &[Action] { + match &self.action { + BoundAction::Release(actions) => actions, + BoundAction::Both { release, .. } => release, + BoundAction::Press(_) => &[], + } + } + + /// Returns the press actions if `pressed`, otherwise the release actions. + pub fn into_actions(self, pressed: bool) -> Option> { + match (self.action, pressed) { + (BoundAction::Press(actions), true) | (BoundAction::Release(actions), false) => { + Some(actions) + } + (BoundAction::Both { press, .. }, true) => Some(press), + (BoundAction::Both { release, .. }, false) => Some(release), + _ => None, + } + } +} + #[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)] pub struct Key { pub trigger: Trigger, @@ -39,6 +109,8 @@ pub struct Key { #[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)] pub enum Trigger { Keysym(Keysym), + CompositorMod, + Modifier(ModKey), MouseLeft, MouseRight, MouseMiddle, @@ -57,6 +129,87 @@ pub enum Trigger { TabletStylusButton3, } +/// Returns whether `keysym` is a modifier key like `Shift_L`. +/// +/// This function matches what the niri config considers modifiers. +/// So, it does not correspond exactly to `keysym.is_modifier_key()`. +/// For example, unlike XKB, `Caps_Lock` and `ISO_Next_Group` aren't considered modifiers. +pub fn is_keysym_modifier(keysym: Keysym) -> bool { + ModKey::from_keysym(keysym).is_some() +} + +impl Trigger { + pub fn is_modifier(self) -> bool { + match self { + Self::CompositorMod | Self::Modifier(_) => true, + Self::Keysym(keysym) => is_keysym_modifier(keysym), + _ => false, + } + } + + pub fn is_generic_modifier(self) -> bool { + matches!(self, Self::CompositorMod | Self::Modifier(_)) + } + + /// The modifier flags that this trigger sets, if it is a modifier key. + pub fn implied_modifier(self) -> Modifiers { + match self { + Self::CompositorMod => Modifiers::COMPOSITOR, + Self::Modifier(modifier) => modifier.to_modifiers(), + Self::Keysym(keysym) => ModKey::from_keysym(keysym) + .map_or(Modifiers::empty(), |modifier| modifier.to_modifiers()), + _ => Modifiers::empty(), + } + } + + pub fn is_keyboard(self) -> bool { + matches!( + self, + Trigger::Keysym(_) | Trigger::CompositorMod | Trigger::Modifier(_) + ) + } + + pub fn is_scroll(self) -> bool { + matches!( + self, + Trigger::WheelScrollDown + | Trigger::WheelScrollUp + | Trigger::WheelScrollLeft + | Trigger::WheelScrollRight + | Trigger::TouchpadScrollDown + | Trigger::TouchpadScrollUp + | Trigger::TouchpadScrollLeft + | Trigger::TouchpadScrollRight + ) + } +} + +impl Key { + /// Whether the two keys are triggered by the same key press. + /// + /// Two different keybinds can be triggered by the same key press. For example, a modifier key + /// can be spelled as the modifier itself (`Alt`) or as its keysym (`Alt_L`). + pub fn conflicts_with(&self, other: &Self) -> bool { + // The trigger and the modifiers match exactly. + // + // All non-modifier triggers fall into this category. + if *self == *other { + return true; + } + + // Modifiers include the trigger's implied modifier, so differing modifiers can never be + // triggered by the same key press. + if self.modifiers != other.modifiers { + return false; + } + + // Generic-modifier-only binds are press-order-independent, so only their modifier set + // (which already includes the trigger implied modifier) has to match. + (self.trigger.is_generic_modifier() && other.trigger.is_modifier()) + || (self.trigger.is_modifier() && other.trigger.is_generic_modifier()) + } +} + bitflags! { #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct Modifiers : u8 { @@ -826,9 +979,8 @@ where ) -> Result> { expect_only_children(node, ctx); - let mut seen_keys: HashMap> = HashMap::new(); - - let mut binds = Vec::new(); + let mut nodes: Vec<&knuffel::ast::SpannedNode> = Vec::new(); + let mut binds: Vec = Vec::new(); for child in node.children() { match Bind::decode_node(child, ctx) { @@ -836,26 +988,30 @@ where ctx.emit_error(e); } Ok(bind) => { - match seen_keys.entry(bind.key) { - Entry::Occupied(entry) => { - // Even though it's technically incorrect, we use - // `DecodeError::Missing` here because it labels the bind with - // "node starts here", which is the least bad option - ctx.emit_error(DecodeError::missing( - entry.get(), - "keybind first defined here", - )); - - ctx.emit_error(DecodeError::unexpected( - &child.node_name, - "keybind", - "duplicate keybind later defined here", - )); - } - Entry::Vacant(entry) => { - entry.insert(child); - binds.push(bind); - } + let conflict = binds + .iter() + .position(|prev| prev.key.conflicts_with(&bind.key)); + if let Some(idx) = conflict { + // Even though it's technically incorrect, we use + // `DecodeError::Missing` here because it labels the bind with + // "node starts here", which is the least bad option + ctx.emit_error(DecodeError::missing( + nodes[idx], + "keybind first defined here", + )); + + ctx.emit_error(DecodeError::unexpected( + &child.node_name, + "keybind", + if binds[idx].key == bind.key { + "duplicate keybind later defined here" + } else { + "conflicting keybind later defined here" + }, + )); + } else { + nodes.push(child); + binds.push(bind); } } } @@ -894,16 +1050,38 @@ where .parse::() .map_err(|e| DecodeError::conversion(&node.node_name, e.wrap_err("invalid keybind")))?; - let mut repeat = true; + let mut repeat = None; + let mut repeat_node = None; + let mut pass_through = None; let mut cooldown = None; let mut allow_when_locked = false; let mut allow_when_locked_node = None; let mut allow_inhibiting = true; let mut hotkey_overlay_title = None; + for (name, val) in &node.properties { match &***name { "repeat" => { - repeat = knuffel::traits::DecodeScalar::decode(val, ctx)?; + repeat = Some(knuffel::traits::DecodeScalar::decode(val, ctx)?); + repeat_node = Some(name); + } + "pass-through" => { + pass_through = Some(knuffel::traits::DecodeScalar::decode(val, ctx)?); + + // Supporting this for mouse binds would be a bit weird. Mouse binds trigger + // compositor gestures, so should pass-through also trigger the corresponding + // gesture, on top of the bind? If yes, then there are some conflicts, e.g. with + // toggle-overview and the Mod+LMB move grab or LMB press-to-close-overview. + // + // All in all, this needs design, and I'm not sure why you'd want pass-through + // mouse binds anyway, so, let's do keyboard only for now. + if !key.trigger.is_keyboard() { + ctx.emit_error(DecodeError::unexpected( + name, + "property", + "pass-through is only supported for keyboard binds", + )); + } } "cooldown-ms" => { cooldown = Some(Duration::from_millis( @@ -935,46 +1113,134 @@ where // even if their contents are not valid. let dummy = Self { key, - actions: Vec::new(), + action: BoundAction::Press(vec![Action::Spawn(vec![])]), repeat: true, + pass_through: None, cooldown: None, allow_when_locked: false, allow_inhibiting: true, hotkey_overlay_title: None, }; - let mut actions = Vec::new(); + // Direct actions are action nodes that are not wrapped in a press or release section. + let mut direct_actions = Vec::new(); + let mut press_actions = Vec::new(); + let mut release_actions = Vec::new(); + let mut has_direct_actions = false; + let mut has_press_section = false; + let mut has_release_section = false; let mut had_error = false; - for child in node.children() { + + let decode_into = |ctx: &mut knuffel::decode::Context, + child: &knuffel::ast::SpannedNode, + actions: &mut Vec, + had_error: &mut bool| { match Action::decode_node(child, ctx) { // Emit the error and keep going, so that a typo in one action still surfaces // the errors in the actions after it. Err(e) => { ctx.emit_error(e); - had_error = true; + *had_error = true; } Ok(action) => actions.push(action), } + }; + + for child in node.children() { + let child_name = &*child.node_name; + let child_name: &str = child_name.as_ref(); + + if child_name == "press" || child_name == "release" { + let (seen, actions) = if child_name == "press" { + (&mut has_press_section, &mut press_actions) + } else { + (&mut has_release_section, &mut release_actions) + }; + + if *seen { + ctx.emit_error(DecodeError::unexpected( + &child.node_name, + "section", + format!("duplicate `{child_name}` section"), + )); + had_error = true; + continue; + } + + if has_direct_actions { + ctx.emit_error(DecodeError::unexpected( + &child.node_name, + "section", + "cannot mix direct actions with press/release sections", + )); + had_error = true; + continue; + } + + *seen = true; + + let mut has_children = false; + for action_child in child.children() { + has_children = true; + decode_into(ctx, action_child, actions, &mut had_error); + } + + if !has_children { + ctx.emit_error(DecodeError::missing( + child, + format!("expected an action for this {child_name} section"), + )); + had_error = true; + } + } else { + if has_press_section || has_release_section { + ctx.emit_error(DecodeError::unexpected( + &child.node_name, + "node", + "cannot mix direct actions with press/release sections", + )); + had_error = true; + continue; + } + + has_direct_actions = true; + decode_into(ctx, child, &mut direct_actions, &mut had_error); + } } - if actions.is_empty() { - if !had_error { - ctx.emit_error(DecodeError::missing( + // Direct actions and press/release sections are mutually exclusive, so at most one of + // these is non-empty. + press_actions.append(&mut direct_actions); + + if has_release_section && key.trigger.is_scroll() { + ctx.emit_error(DecodeError::unexpected( + node, + "bind", + "release sections are not supported for scroll binds", + )); + had_error = true; + release_actions.clear(); + } + + if let Some(node) = repeat_node { + if repeat == Some(true) && press_actions.is_empty() && !release_actions.is_empty() { + ctx.emit_error(DecodeError::unexpected( node, - "expected an action for this keybind", + "property", + "release-only binds cannot repeat", )); } - return Ok(dummy); } // allow-when-locked must hold for *every* action: handle_bind passes one flag into // every do_action call, so a mixed bind would let a non-spawn action run from the // lock screen. - if !actions - .iter() - .all(|a| matches!(a, Action::Spawn(_) | Action::SpawnSh(_))) - { - if let Some(node) = allow_when_locked_node { + if let Some(node) = allow_when_locked_node { + if !press_actions + .iter() + .chain(&release_actions) + .all(|a| matches!(a, Action::Spawn(_) | Action::SpawnSh(_))) + { ctx.emit_error(DecodeError::unexpected( node, "property", @@ -985,17 +1251,40 @@ where // The toggle-inhibit action must always be uninhibitable. Otherwise, it would be // impossible to trigger it. - if actions + if press_actions .iter() + .chain(&release_actions) .any(|a| matches!(a, Action::ToggleKeyboardShortcutsInhibit)) { allow_inhibiting = false; } + let repeat = repeat.unwrap_or(release_actions.is_empty()); + + let action = match (press_actions.is_empty(), release_actions.is_empty()) { + (false, false) => BoundAction::Both { + press: press_actions, + release: release_actions, + }, + (false, true) => BoundAction::Press(press_actions), + (true, false) => BoundAction::Release(release_actions), + (true, true) => { + // If anything failed to decode, an error was already emitted above. + if !had_error { + ctx.emit_error(DecodeError::missing( + node, + "expected an action for this keybind", + )); + } + return Ok(dummy); + } + }; + Ok(Self { key, - actions, + action, repeat, + pass_through, cooldown, allow_when_locked, allow_inhibiting, @@ -1008,34 +1297,40 @@ impl FromStr for Key { type Err = miette::Error; fn from_str(s: &str) -> Result { - let mut modifiers = Modifiers::empty(); + if s.contains('\0') { + return Err(miette!("invalid key with a null byte: {s:?}")); + } + + let mut written_modifiers: Vec<(Modifiers, &str)> = Vec::new(); let mut split = s.split('+'); let key = split.next_back().unwrap(); for part in split { let part = part.trim(); - if part.eq_ignore_ascii_case("mod") { - modifiers |= Modifiers::COMPOSITOR + let modifier = if part.eq_ignore_ascii_case("mod") { + Modifiers::COMPOSITOR } else if part.eq_ignore_ascii_case("ctrl") || part.eq_ignore_ascii_case("control") { - modifiers |= Modifiers::CTRL; + Modifiers::CTRL } else if part.eq_ignore_ascii_case("shift") { - modifiers |= Modifiers::SHIFT; + Modifiers::SHIFT } else if part.eq_ignore_ascii_case("alt") { - modifiers |= Modifiers::ALT; + Modifiers::ALT } else if part.eq_ignore_ascii_case("super") || part.eq_ignore_ascii_case("win") { - modifiers |= Modifiers::SUPER; + Modifiers::SUPER } else if part.eq_ignore_ascii_case("iso_level3_shift") || part.eq_ignore_ascii_case("mod5") { - modifiers |= Modifiers::ISO_LEVEL3_SHIFT; + Modifiers::ISO_LEVEL3_SHIFT } else if part.eq_ignore_ascii_case("iso_level5_shift") || part.eq_ignore_ascii_case("mod3") { - modifiers |= Modifiers::ISO_LEVEL5_SHIFT; + Modifiers::ISO_LEVEL5_SHIFT } else { return Err(miette!("invalid modifier: {part}")); - } + }; + + written_modifiers.push((modifier, part)); } let trigger = if key.eq_ignore_ascii_case("MouseLeft") { @@ -1070,6 +1365,20 @@ impl FromStr for Key { Trigger::TabletStylusButton2 } else if key.eq_ignore_ascii_case("TabletStylusButton3") { Trigger::TabletStylusButton3 + } else if key.eq_ignore_ascii_case("Mod") { + Trigger::CompositorMod + } else if key.eq_ignore_ascii_case("Ctrl") || key.eq_ignore_ascii_case("Control") { + Trigger::Modifier(ModKey::Ctrl) + } else if key.eq_ignore_ascii_case("Shift") { + Trigger::Modifier(ModKey::Shift) + } else if key.eq_ignore_ascii_case("Alt") { + Trigger::Modifier(ModKey::Alt) + } else if key.eq_ignore_ascii_case("Super") || key.eq_ignore_ascii_case("Win") { + Trigger::Modifier(ModKey::Super) + } else if key.eq_ignore_ascii_case("ISO_Level3_Shift") || key.eq_ignore_ascii_case("Mod5") { + Trigger::Modifier(ModKey::IsoLevel3Shift) + } else if key.eq_ignore_ascii_case("ISO_Level5_Shift") || key.eq_ignore_ascii_case("Mod3") { + Trigger::Modifier(ModKey::IsoLevel5Shift) } else { let mut keysym = keysym_from_name(key, KEYSYM_CASE_INSENSITIVE); // The keyboard event handling code can receive either @@ -1105,6 +1414,15 @@ impl FromStr for Key { Trigger::Keysym(keysym) }; + let mut modifiers = trigger.implied_modifier(); + for (modifier, spelling) in written_modifiers { + if modifiers.contains(modifier) { + warn!("redundant modifier `{spelling}` in keybind `{s}`"); + } + + modifiers |= modifier; + } + Ok(Key { trigger, modifiers }) } } @@ -1126,11 +1444,57 @@ mod tests { assert_eq!(config.binds.0.len(), 1); assert_eq!( - config.binds.0[0].actions, + config.binds.0[0].press_actions(), [Action::FocusColumnRight, Action::ConsumeOrExpelWindowLeft], ); } + #[test] + fn parse_multiple_actions_in_press_and_release_sections() { + let config = crate::Config::parse_mem( + r#" + binds { + Mod+G { + press { focus-column-right; consume-or-expel-window-left; } + release { toggle-overview; close-window; } + } + Mod+H { + release { focus-column-left; focus-window-down; } + } + } + "#, + ) + .unwrap(); + + assert_eq!( + config.binds.0[0].action, + BoundAction::Both { + press: vec![Action::FocusColumnRight, Action::ConsumeOrExpelWindowLeft], + release: vec![Action::ToggleOverview, Action::CloseWindow], + } + ); + assert!(!config.binds.0[0].repeat); + assert_eq!( + config.binds.0[1].action, + BoundAction::Release(vec![Action::FocusColumnLeft, Action::FocusWindowDown]) + ); + } + + #[test] + fn allow_when_locked_checks_release_actions() { + assert!(crate::Config::parse_mem( + r#" + binds { + Mod+X allow-when-locked=true { + press { spawn "foo"; } + release { spawn "bar"; quit; } + } + } + "#, + ) + .is_err()); + } + #[test] fn parse_bind_with_no_actions_still_errors() { assert!(crate::Config::parse_mem( @@ -1204,7 +1568,7 @@ mod tests { .unwrap(); assert!(config.binds.0[0].allow_when_locked); - assert_eq!(config.binds.0[0].actions.len(), 2); + assert_eq!(config.binds.0[0].press_actions().len(), 2); } #[test] @@ -1221,7 +1585,7 @@ mod tests { .unwrap(); assert_eq!( - config.binds.0[0].actions, + config.binds.0[0].press_actions(), [ Action::ToggleOutputShader(None), Action::CycleOutputShader(Some(String::from("DP-2"))), @@ -1258,7 +1622,7 @@ mod tests { .binds .0 .iter() - .map(|b| b.actions[0].clone()) + .map(|b| b.press_actions()[0].clone()) .collect(); assert_eq!( actions, @@ -1290,7 +1654,11 @@ mod tests { .unwrap() .binds; - let actions: Vec<_> = binds.0.iter().map(|b| b.actions[0].clone()).collect(); + let actions: Vec<_> = binds + .0 + .iter() + .map(|b| b.press_actions()[0].clone()) + .collect(); assert_eq!( actions, [ @@ -1380,4 +1748,177 @@ mod tests { }, ); } + + #[test] + fn parse_bare_modifier_spellings() { + // Each modifier can be spelled with its canonical name or an alias. A bare modifier key + // parses to `Trigger::Modifier`. + for (text, modifier) in [ + ("Ctrl", ModKey::Ctrl), + ("Control", ModKey::Ctrl), + ("Shift", ModKey::Shift), + ("Alt", ModKey::Alt), + ("Super", ModKey::Super), + ("Win", ModKey::Super), + ("Mod5", ModKey::IsoLevel3Shift), + ("ISO_Level3_Shift", ModKey::IsoLevel3Shift), + ("Mod3", ModKey::IsoLevel5Shift), + ("ISO_Level5_Shift", ModKey::IsoLevel5Shift), + ] { + assert_eq!( + text.parse::().unwrap(), + Key { + trigger: Trigger::Modifier(modifier), + modifiers: modifier.to_modifiers(), + }, + "parsing `{text}`", + ); + } + } + + #[test] + fn parse_mod_as_trigger() { + // The mod key keeps its own trigger, which is resolved to a real modifier at runtime. + assert_eq!( + "Mod".parse::().unwrap(), + Key { + trigger: Trigger::CompositorMod, + modifiers: Modifiers::COMPOSITOR, + }, + ); + + // It can also be used together with other held modifiers. + assert_eq!( + "Ctrl+Mod".parse::().unwrap(), + Key { + trigger: Trigger::CompositorMod, + modifiers: Modifiers::CTRL | Modifiers::COMPOSITOR, + }, + ); + + // `Mod` also works as a held modifier of another trigger. + assert_eq!( + "Mod+Control_L".parse::().unwrap(), + Key { + trigger: Trigger::Keysym(Keysym::Control_L), + modifiers: Modifiers::COMPOSITOR | Modifiers::CTRL, + }, + ); + } + + #[test] + fn parse_multi_modifier_triggers() { + // A modifier key as the trigger with another held modifier, either spelled as the modifier + // itself or as its keysym. + assert_eq!( + "Ctrl+Alt".parse::().unwrap(), + Key { + trigger: Trigger::Modifier(ModKey::Alt), + modifiers: Modifiers::CTRL | Modifiers::ALT, + }, + ); + assert_eq!( + "Ctrl+Alt_L".parse::().unwrap(), + Key { + trigger: Trigger::Keysym(Keysym::Alt_L), + modifiers: Modifiers::CTRL | Modifiers::ALT, + }, + ); + } + + #[test] + fn parse_normalizes_redundant_modifiers() { + let key = |text: &str| text.parse::().unwrap(); + + // Modifier triggers can be written with or without the redundant modifier. + assert_eq!(key("Alt+Alt_L"), key("Alt_L")); + assert_eq!(key("Alt+Alt_R"), key("Alt_R")); + assert_eq!(key("Alt+Alt"), key("Alt")); + assert_eq!(key("Alt+Alt+Alt_L"), key("Alt_L")); + assert_eq!(key("Ctrl+Shift+Control_L"), key("Shift+Control_L")); + assert_eq!(key("Mod+Mod"), key("Mod")); + + // Duplicate modifiers are fine with non-modifier triggers. + assert_eq!(key("Alt+Alt+Q"), key("Alt+Q")); + assert_eq!(key("Mod+Mod+Q"), key("Mod+Q")); + + // Still works if spelled differently. + assert_eq!(key("Ctrl+Control+Q"), key("Ctrl+Q")); + assert_eq!(key("Super+Win+Q"), key("Super+Q")); + } + + #[test] + fn parse_modifier_trigger_includes_implied_modifier() { + // The modifier that the trigger implies is part of the key. + assert_eq!( + "Alt_L".parse::().unwrap(), + Key { + trigger: Trigger::Keysym(Keysym::Alt_L), + modifiers: Modifiers::ALT, + }, + ); + assert_eq!( + "Alt+Alt".parse::().unwrap(), + Key { + trigger: Trigger::Modifier(ModKey::Alt), + modifiers: Modifiers::ALT, + }, + ); + // `Mod` is only resolved at runtime, so no trigger implies it. + assert_eq!( + "Alt+Mod".parse::().unwrap(), + Key { + trigger: Trigger::CompositorMod, + modifiers: Modifiers::ALT | Modifiers::COMPOSITOR, + }, + ); + } + + #[test] + fn key_conflicts() { + let key = |text: &str| text.parse::().unwrap(); + + // Alt_L and Alt_R conflict with Alt, and vice versa. + let alt = key("Alt"); + let alt_l = key("Alt_L"); + let alt_r = key("Alt_R"); + assert!(alt.conflicts_with(&alt_l)); + assert!(alt.conflicts_with(&alt_r)); + assert!(alt_l.conflicts_with(&alt)); + assert!(alt_r.conflicts_with(&alt)); + + // The left and right keysyms are different keys. + assert!(!alt_l.conflicts_with(&alt_r)); + + // Different modifiers mean different binds, even for the same trigger. + assert!(!alt.conflicts_with(&key("Ctrl+Alt_L"))); + + // Non-overlapping triggers don't conflict, even with the same modifiers. + assert!(!key("Mod+Q").conflicts_with(&key("Mod+L"))); + assert!(!key("Q").conflicts_with(&key("Ctrl"))); + + // Identical keys are duplicates. + assert!(key("Q").conflicts_with(&key("Q"))); + assert!(alt.conflicts_with(&alt)); + + // The compositor mod key is only resolved at runtime, so `Mod` isn't compared against the + // mod key's own spellings. + let compositor_mod = key("Mod"); + assert!(!compositor_mod.conflicts_with(&key("Super_L"))); + assert!(!compositor_mod.conflicts_with(&key("Super"))); + + // Generic-modifier-only keys are order-independent. + assert!(key("Ctrl+Alt").conflicts_with(&key("Alt+Ctrl"))); + assert!(key("Mod+Shift+Alt").conflicts_with(&key("Shift+Alt+Mod"))); + assert!(key("Ctrl+Alt").conflicts_with(&key("Alt+Control_L"))); + assert!(key("Ctrl+Alt_L").conflicts_with(&key("Alt+Ctrl"))); + + assert!(!key("Ctrl+Alt").conflicts_with(&key("Ctrl+Alt+A"))); + } + + #[test] + fn null_byte() { + // Shouldn't panic. + let _ = "W\0".parse::(); + } } diff --git a/niri-config/src/debug.rs b/niri-config/src/debug.rs index 32de7f5310..d63e5ccd57 100644 --- a/niri-config/src/debug.rs +++ b/niri-config/src/debug.rs @@ -26,6 +26,7 @@ pub struct Debug { pub deactivate_unfocused_windows: bool, pub skip_cursor_only_updates_during_vrr: bool, pub disable_10bit_output: bool, + pub nested_show_host_cursor: bool, } #[derive(knuffel::Decode, Debug, Default, PartialEq)] @@ -74,6 +75,8 @@ pub struct DebugPart { pub skip_cursor_only_updates_during_vrr: Option, #[knuffel(child)] pub disable_10bit_output: Option, + #[knuffel(child)] + pub nested_show_host_cursor: Option, } impl MergeWith for Debug { @@ -99,6 +102,7 @@ impl MergeWith for Debug { deactivate_unfocused_windows, skip_cursor_only_updates_during_vrr, disable_10bit_output, + nested_show_host_cursor, ); merge_clone_opt!((self, part), preview_render, render_drm_device); diff --git a/niri-config/src/input.rs b/niri-config/src/input.rs index 73495e97bd..4332aa0e37 100644 --- a/niri-config/src/input.rs +++ b/niri-config/src/input.rs @@ -1,12 +1,18 @@ +use std::cell::RefCell; +use std::fs; +use std::path::PathBuf; +use std::rc::Rc; use std::str::FromStr; +use knuffel::errors::DecodeError; +use knuffel::DecodeScalar; use miette::miette; -use smithay::input::keyboard::XkbConfig; +use smithay::input::keyboard::{keysyms, Keysym, XkbConfig}; use smithay::reexports::input; use crate::binds::Modifiers; -use crate::utils::{Flag, MergeWith, Percent}; -use crate::FloatOrInt; +use crate::utils::{expand_home_path, Flag, MergeWith, Percent}; +use crate::{FloatOrInt, Includes}; #[derive(Debug, Default, PartialEq)] pub struct Input { @@ -198,7 +204,7 @@ pub struct Xkb { #[knuffel(child, unwrap(argument))] pub options: Option, #[knuffel(child, unwrap(argument))] - pub file: Option, + pub file: Option, } impl Xkb { @@ -213,6 +219,9 @@ impl Xkb { } } +#[derive(PartialEq, Eq, Clone, Debug)] +pub struct XkbFile(pub Option); + #[derive(knuffel::DecodeScalar, Debug, Default, Clone, Copy, PartialEq, Eq)] pub enum TrackLayout { /// The layout change is global. @@ -473,7 +482,7 @@ impl FromStr for WarpMouseToFocusMode { } } -#[derive(Debug, PartialEq, Eq, Clone, Copy)] +#[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)] pub enum ModKey { Ctrl, Shift, @@ -494,6 +503,23 @@ impl ModKey { ModKey::IsoLevel5Shift => Modifiers::ISO_LEVEL5_SHIFT, } } + + /// Returns the modifier key for a keysym, if the keysym is a modifier key. + pub fn from_keysym(keysym: Keysym) -> Option { + match keysym.raw() { + keysyms::KEY_Control_L | keysyms::KEY_Control_R => Some(Self::Ctrl), + keysyms::KEY_Shift_L | keysyms::KEY_Shift_R => Some(Self::Shift), + keysyms::KEY_Alt_L | keysyms::KEY_Alt_R => Some(Self::Alt), + keysyms::KEY_Super_L | keysyms::KEY_Super_R => Some(Self::Super), + keysyms::KEY_ISO_Level3_Shift => Some(Self::IsoLevel3Shift), + keysyms::KEY_ISO_Level5_Shift => Some(Self::IsoLevel5Shift), + _ => None, + } + } + + pub fn matches_keysym(self, keysym: Keysym) -> bool { + Self::from_keysym(keysym) == Some(self) + } } impl FromStr for ModKey { @@ -570,6 +596,49 @@ impl FromStr for TapButtonMap { } } +impl DecodeScalar for XkbFile +where + S: knuffel::traits::ErrorSpan, +{ + fn type_check( + type_name: &Option>, + ctx: &mut knuffel::decode::Context, + ) { + if let Some(type_name) = &type_name { + ctx.emit_error(DecodeError::unexpected( + type_name, + "type name", + "no type name expected for this node", + )); + } + } + + fn raw_decode( + value: &knuffel::span::Spanned, + ctx: &mut knuffel::decode::Context, + ) -> Result> { + let path = PathBuf::raw_decode(value, ctx)?; + match expand_home_path(path, value, ctx) { + Ok(p) => { + let includes = ctx.get::>>().unwrap(); + includes.borrow_mut().0.push(p.to_path_buf()); + + match fs::read_to_string(&p) { + Ok(c) => Ok(Self(Some(c))), + Err(e) => { + warn!("failed to read xkb file from {p:?}: {e}"); + Ok(Self(None)) + } + } + } + Err(e) => { + warn!("failed to expand xkb file path: {:?}", e); + Ok(Self(None)) + } + } + } +} + #[cfg(test)] mod tests { use insta::assert_debug_snapshot; @@ -584,6 +653,55 @@ mod tests { Input::from_part(&part) } + #[test] + fn mod_key_names_are_consistent() { + // The X11 `ModN` modifier indices are a separate numbering from the ISO level numbers, + // and the two happen to be crossed over: ISO Level 3 Shift is Mod5, and ISO Level 5 Shift + // is Mod3. This test guards against "fixing" this apparent mismatch. + for (name, modifier, keysym, modifiers) in [ + ( + "Mod5", + ModKey::IsoLevel3Shift, + keysyms::KEY_ISO_Level3_Shift, + Modifiers::ISO_LEVEL3_SHIFT, + ), + ( + "Mod3", + ModKey::IsoLevel5Shift, + keysyms::KEY_ISO_Level5_Shift, + Modifiers::ISO_LEVEL5_SHIFT, + ), + ] { + // Name -> variant. + assert_eq!( + name.parse::().unwrap(), + modifier, + "parsing `{name}`" + ); + + // Keysym -> variant. + assert_eq!( + ModKey::from_keysym(Keysym::from(keysym)), + Some(modifier), + "keysym for `{name}`", + ); + assert!(modifier.matches_keysym(Keysym::from(keysym))); + + // Variant -> held modifiers. + assert_eq!(modifier.to_modifiers(), modifiers, "modifiers for `{name}`"); + } + + // ISO_Level3_Shift and Mod5 are the same modifier, and so are ISO_Level5_Shift and Mod3. + assert_eq!( + "ISO_Level3_Shift".parse::().unwrap(), + "Mod5".parse::().unwrap() + ); + assert_eq!( + "ISO_Level5_Shift".parse::().unwrap(), + "Mod3".parse::().unwrap() + ); + } + #[test] fn parse_scroll_factor_combined() { // Test combined scroll-factor syntax diff --git a/niri-config/src/lib.rs b/niri-config/src/lib.rs index 8eea7e0b9a..d6f237f4fc 100644 --- a/niri-config/src/lib.rs +++ b/niri-config/src/lib.rs @@ -274,7 +274,8 @@ where let mut config = config.borrow_mut(); let binds = &mut config.binds.0; // Remove existing binds matching any new bind. - binds.retain(|bind| !part.0.iter().any(|new| new.key == bind.key)); + binds + .retain(|bind| !part.0.iter().any(|new| new.key.conflicts_with(&bind.key))); // Add all new binds. binds.extend(part.0); } @@ -402,8 +403,12 @@ where let decoded_path = knuffel::traits::DecodeScalar::decode(path_val, ctx)?; // Expand ~ into the home dir - let Some(path) = expand_home_path(decoded_path, node, ctx) else { - continue; + let path = match expand_home_path(decoded_path, node, ctx) { + Ok(p) => p, + Err(e) => { + ctx.emit_error(e); + continue; + } }; let recursion = ctx.get::().unwrap().0 + 1; @@ -679,6 +684,7 @@ impl ConfigPath { mod tests { use insta::{assert_debug_snapshot, assert_snapshot}; use pretty_assertions::assert_eq; + use smithay::input::keyboard::Keysym; use super::*; @@ -719,7 +725,7 @@ mod tests { .binds .0 .iter() - .map(|bind| bind.actions[0].clone()) + .map(|bind| bind.press_actions()[0].clone()) .collect(); let name = || "scratch".to_string(); assert_eq!( @@ -813,6 +819,461 @@ mod tests { } } + #[test] + fn parse_release_binds_for_supported_triggers() { + // Keys and mouse buttons have release events, so release sections are supported. + do_parse( + r#" + binds { + Mod+MouseLeft { + release { close-window; } + } + Mod+Q { + press { close-window; } + release { close-window; } + } + } + "#, + ); + } + + #[test] + fn parse_release_binds_for_unsupported_triggers() { + // Scroll ticks are dispatched as press events only, so scroll binds can't have a release + // action. + assert_parse_error( + r#" + binds { + Mod+WheelScrollDown { + release { toggle-overview; } + } + } + "#, + "release sections are not supported for scroll binds", + ); + assert_parse_error( + r#" + binds { + Mod+TouchpadScrollDown { + release { toggle-overview; } + } + } + "#, + "release sections are not supported for scroll binds", + ); + } + + #[test] + fn parse_bind_repeat_on_release_only() { + // A release-only bind has no press action to repeat, so repeat=true is rejected. + assert_parse_error( + r#" + binds { + Mod+Q repeat=true { + release { close-window; } + } + } + "#, + "release-only binds cannot repeat", + ); + + do_parse( + r#" + binds { + Mod+T repeat=true { spawn "alacritty"; } + Mod+Q repeat=true { + press { close-window; } + release { close-window; } + } + Mod+W { + release { close-window; } + } + Mod+E repeat=false { + release { close-window; } + } + } + "#, + ); + } + + #[test] + fn parse_bind_pass_through() { + let config = do_parse( + r#" + binds { + Mod+A pass-through=true { close-window; } + Mod+B pass-through=false { close-window; } + Mod+C { close-window; } // Should be off by default + } + "#, + ); + + assert_eq!(config.binds.0[0].pass_through, Some(true)); + assert_eq!(config.binds.0[1].pass_through, Some(false)); + assert_eq!(config.binds.0[2].pass_through, None); + } + + #[test] + fn parse_bind_pass_through_on_pointer_binds() { + assert_parse_error( + r#" + binds { + Mod+MouseLeft pass-through=true { close-window; } + } + "#, + "pass-through is only supported for keyboard binds", + ); + } + + /// Asserts that parsing fails with a diagnostic containing `message`. + #[track_caller] + fn assert_parse_error(text: &str, message: &str) { + let err = match Config::parse_mem(text) { + Ok(_) => panic!("expected a parse error for:\n{text}"), + Err(err) => err, + }; + + // knuffel keeps the individual diagnostics in a private field, so check the Debug + // representation, which includes their messages. + let err = format!("{err:?}"); + assert!( + err.contains(message), + "expected {message:?} in parse error:\n{err}" + ); + } + + #[test] + fn parse_bind_section_errors() { + // A direct action node and a `press` section both specify the press action, so mixing them + // is reported as mixing the two syntaxes rather than as a duplicate `press` section. + assert_parse_error( + r#" + binds { + Mod+Q { + close-window; + press { center-column; } + } + } + "#, + "cannot mix direct actions with press/release sections", + ); + + // A direct action following a `press` section is mixing the two syntaxes. + assert_parse_error( + r#" + binds { + Mod+Q { + press { close-window; } + center-column; + } + } + "#, + "cannot mix direct actions with press/release sections", + ); + + // Two `press` sections are still duplicates. + assert_parse_error( + r#" + binds { + Mod+Q { + press { close-window; } + press { center-column; } + } + } + "#, + "duplicate `press` section", + ); + + assert_parse_error( + r#" + binds { + Mod+Q { + center-column; + release { close-window; } + } + } + "#, + "cannot mix direct actions with press/release sections", + ); + + assert_parse_error( + r#" + binds { + Mod+Q { + release { close-window; } + center-column; + } + } + "#, + "cannot mix direct actions with press/release sections", + ); + } + + #[test] + fn parse_bind_bound_action() { + let config = do_parse( + r#" + binds { + Mod+A { close-window; } + Mod+B { + press { close-window; } + } + Mod+C { + release { toggle-overview; } + } + Mod+D { + press { close-window; } + release { toggle-overview; } + } + } + "#, + ); + + let action = |index: usize| &config.binds.0[index].action; + + // A direct action and a press section both bind only the press phase. + assert_eq!(action(0), &BoundAction::Press(vec![Action::CloseWindow])); + assert_eq!(action(1), &BoundAction::Press(vec![Action::CloseWindow])); + + // A release section binds only the release phase. + assert_eq!( + action(2), + &BoundAction::Release(vec![Action::ToggleOverview]) + ); + + // Both sections bind both phases. + assert_eq!( + action(3), + &BoundAction::Both { + press: vec![Action::CloseWindow], + release: vec![Action::ToggleOverview], + } + ); + } + + #[test] + fn parse_bare_modifier_keys() { + let config = do_parse( + r#" + binds { + Mod { + release { toggle-overview; } + } + Ctrl { + release { close-window; } + } + Shift { + release { close-window; } + } + Alt { + release { close-window; } + } + Super { + release { close-window; } + } + Mod5 { + release { close-window; } + } + Mod3 { + release { close-window; } + } + + Ctrl+Alt_L { + release { close-window; } + } + } + "#, + ); + + let triggers = config + .binds + .0 + .iter() + .map(|bind| bind.key.trigger) + .collect::>(); + + assert_eq!( + triggers, + vec![ + Trigger::CompositorMod, + Trigger::Modifier(ModKey::Ctrl), + Trigger::Modifier(ModKey::Shift), + Trigger::Modifier(ModKey::Alt), + Trigger::Modifier(ModKey::Super), + Trigger::Modifier(ModKey::IsoLevel3Shift), + Trigger::Modifier(ModKey::IsoLevel5Shift), + Trigger::Keysym(Keysym::Alt_L), + ] + ); + + // Ctrl+Alt_L has modifiers of Ctrl (manually specified) and Alt (implied by the keysym). + assert_eq!( + config.binds.0[7].key.modifiers, + Modifiers::CTRL | Modifiers::ALT + ); + } + + #[test] + fn parse_bind_conflicting_modifier_keys() { + // A modifier key can be spelled as the modifier itself (`Alt`) or as its keysym (`Alt_L`), + // and a modifier key is triggered by either of its keysyms, so one of these two binds + // would always shadow the other. + assert_parse_error( + r#" + binds { + Alt { + release { toggle-overview; } + } + Alt_L { + release { close-window; } + } + } + "#, + "conflicting keybind later defined here", + ); + assert_parse_error( + r#" + recent-windows { + binds { + Alt+Control_L { next-window; } + Alt+Ctrl { next-window; } + } + } + "#, + "conflicting keybind later defined here", + ); + + // The left and right keysyms are different keys, so those can be bound separately. + do_parse( + r#" + binds { + Alt_L { + release { close-window; } + } + Alt_R { + release { center-column; } + } + } + "#, + ); + + // A modifier keysym on its own is an ordinary modifier-only bind. + let config = do_parse( + r#" + binds { + Alt_L { + release { close-window; } + } + } + "#, + ); + assert!(config.binds.0[0].key.trigger.is_modifier()); + } + + #[test] + fn parse_recent_windows_bind_modifier() { + // Modifier-only binds are allowed. + do_parse( + r#" + recent-windows { + binds { + Ctrl+Alt_L { next-window; } + Ctrl+Shift { next-window; } + } + } + "#, + ); + + // Binds must have an extra modifier. + assert_parse_error( + r#" + recent-windows { + binds { + Alt_L { next-window; } + } + } + "#, + "keybind must have a modifier key", + ); + assert_parse_error( + r#" + recent-windows { + binds { + Alt+Alt_L { next-window; } + } + } + "#, + "keybind must have a modifier key", + ); + + // A trigger with no modifier at all is rejected. + assert_parse_error( + r#" + recent-windows { + binds { + Tab { next-window; } + } + } + "#, + "keybind must have a modifier key", + ); + + // Only keyboard keys are supported here for now. + assert_parse_error( + r#" + recent-windows { + binds { + Alt+MouseLeft { next-window; } + } + } + "#, + "key must be a keyboard key", + ); + } + + #[test] + fn parse_bind_redundant_modifiers_are_the_same_bind() { + // Redundant modifiers are normalized, so these should error as duplicates. + assert_parse_error( + r#" + binds { + Alt_L { close-window; } + Alt+Alt_L { center-column; } + } + "#, + "duplicate keybind later defined here", + ); + assert_parse_error( + r#" + binds { + Alt { close-window; } + Alt+Alt { center-column; } + } + "#, + "duplicate keybind later defined here", + ); + assert_parse_error( + r#" + binds { + Mod { close-window; } + Mod+Mod { center-column; } + } + "#, + "duplicate keybind later defined here", + ); + assert_parse_error( + r#" + recent-windows { + binds { + Alt+Control_L { next-window; } + Alt+Ctrl+Control_L { next-window; } + } + } + "#, + "duplicate keybind later defined here", + ); + } + #[test] fn parse_on_xdg_activate() { let parsed = do_parse( @@ -1118,6 +1579,12 @@ mod tests { Mod+Shift+E allow-inhibiting=false { quit skip-confirmation=true; } Mod+WheelScrollDown cooldown-ms=150 { focus-workspace-down; } Super+Alt+S allow-when-locked=true { spawn-sh "pkill orca || exec orca"; } + Mod { + release { toggle-overview; } + } + Shift+Mod { + release { toggle-window-floating; } + } } switch-events { @@ -2234,10 +2701,13 @@ mod tests { COMPOSITOR, ), }, - actions: [ - ToggleKeyboardShortcutsInhibit, - ], + action: Press( + [ + ToggleKeyboardShortcutsInhibit, + ], + ), repeat: true, + pass_through: None, cooldown: None, allow_when_locked: false, allow_inhibiting: false, @@ -2256,10 +2726,13 @@ mod tests { SHIFT | COMPOSITOR, ), }, - actions: [ - ToggleKeyboardShortcutsInhibit, - ], + action: Press( + [ + ToggleKeyboardShortcutsInhibit, + ], + ), repeat: true, + pass_through: None, cooldown: None, allow_when_locked: false, allow_inhibiting: false, @@ -2274,14 +2747,17 @@ mod tests { COMPOSITOR, ), }, - actions: [ - Spawn( - [ - "alacritty", - ], - ), - ], + action: Press( + [ + Spawn( + [ + "alacritty", + ], + ), + ], + ), repeat: true, + pass_through: None, cooldown: None, allow_when_locked: true, allow_inhibiting: true, @@ -2296,10 +2772,13 @@ mod tests { COMPOSITOR, ), }, - actions: [ - CloseWindow, - ], + action: Press( + [ + CloseWindow, + ], + ), repeat: true, + pass_through: None, cooldown: None, allow_when_locked: false, allow_inhibiting: true, @@ -2316,10 +2795,13 @@ mod tests { SHIFT | COMPOSITOR, ), }, - actions: [ - FocusMonitorLeft, - ], + action: Press( + [ + FocusMonitorLeft, + ], + ), repeat: true, + pass_through: None, cooldown: None, allow_when_locked: false, allow_inhibiting: true, @@ -2334,12 +2816,15 @@ mod tests { SHIFT | COMPOSITOR, ), }, - actions: [ - FocusMonitor( - "eDP-1", - ), - ], + action: Press( + [ + FocusMonitor( + "eDP-1", + ), + ], + ), repeat: true, + pass_through: None, cooldown: None, allow_when_locked: false, allow_inhibiting: true, @@ -2354,10 +2839,13 @@ mod tests { CTRL | SHIFT | COMPOSITOR, ), }, - actions: [ - MoveWindowToMonitorRight, - ], + action: Press( + [ + MoveWindowToMonitorRight, + ], + ), repeat: true, + pass_through: None, cooldown: None, allow_when_locked: false, allow_inhibiting: true, @@ -2372,12 +2860,15 @@ mod tests { CTRL | ALT | COMPOSITOR, ), }, - actions: [ - MoveWindowToMonitor( - "eDP-1", - ), - ], + action: Press( + [ + MoveWindowToMonitor( + "eDP-1", + ), + ], + ), repeat: true, + pass_through: None, cooldown: None, allow_when_locked: false, allow_inhibiting: true, @@ -2392,12 +2883,15 @@ mod tests { CTRL | ALT | COMPOSITOR, ), }, - actions: [ - MoveColumnToMonitor( - "DP-1", - ), - ], + action: Press( + [ + MoveColumnToMonitor( + "DP-1", + ), + ], + ), repeat: true, + pass_through: None, cooldown: None, allow_when_locked: false, allow_inhibiting: true, @@ -2412,10 +2906,13 @@ mod tests { COMPOSITOR, ), }, - actions: [ - ConsumeWindowIntoColumn, - ], + action: Press( + [ + ConsumeWindowIntoColumn, + ], + ), repeat: true, + pass_through: None, cooldown: None, allow_when_locked: false, allow_inhibiting: true, @@ -2430,14 +2927,17 @@ mod tests { COMPOSITOR, ), }, - actions: [ - FocusWorkspace( - Index( - 1, + action: Press( + [ + FocusWorkspace( + Index( + 1, + ), ), - ), - ], + ], + ), repeat: true, + pass_through: None, cooldown: None, allow_when_locked: false, allow_inhibiting: true, @@ -2452,14 +2952,17 @@ mod tests { SHIFT | COMPOSITOR, ), }, - actions: [ - FocusWorkspace( - Name( - "workspace-1", + action: Press( + [ + FocusWorkspace( + Name( + "workspace-1", + ), ), - ), - ], + ], + ), repeat: true, + pass_through: None, cooldown: None, allow_when_locked: false, allow_inhibiting: true, @@ -2474,12 +2977,15 @@ mod tests { SHIFT | COMPOSITOR, ), }, - actions: [ - Quit( - true, - ), - ], + action: Press( + [ + Quit( + true, + ), + ], + ), repeat: true, + pass_through: None, cooldown: None, allow_when_locked: false, allow_inhibiting: false, @@ -2492,10 +2998,13 @@ mod tests { COMPOSITOR, ), }, - actions: [ - FocusWorkspaceDown, - ], + action: Press( + [ + FocusWorkspaceDown, + ], + ), repeat: true, + pass_through: None, cooldown: Some( 150ms, ), @@ -2512,17 +3021,58 @@ mod tests { ALT | SUPER, ), }, - actions: [ - SpawnSh( - "pkill orca || exec orca", - ), - ], + action: Press( + [ + SpawnSh( + "pkill orca || exec orca", + ), + ], + ), repeat: true, + pass_through: None, cooldown: None, allow_when_locked: true, allow_inhibiting: true, hotkey_overlay_title: None, }, + Bind { + key: Key { + trigger: CompositorMod, + modifiers: Modifiers( + COMPOSITOR, + ), + }, + action: Release( + [ + ToggleOverview, + ], + ), + repeat: false, + pass_through: None, + cooldown: None, + allow_when_locked: false, + allow_inhibiting: true, + hotkey_overlay_title: None, + }, + Bind { + key: Key { + trigger: CompositorMod, + modifiers: Modifiers( + SHIFT | COMPOSITOR, + ), + }, + action: Release( + [ + ToggleWindowFloating, + ], + ), + repeat: false, + pass_through: None, + cooldown: None, + allow_when_locked: false, + allow_inhibiting: true, + hotkey_overlay_title: None, + }, ], ), switch_events: SwitchBinds { @@ -2575,6 +3125,7 @@ mod tests { deactivate_unfocused_windows: false, skip_cursor_only_updates_during_vrr: false, disable_10bit_output: false, + nested_show_host_cursor: false, }, workspaces: [ Workspace { @@ -2639,16 +3190,19 @@ mod tests { ALT, ), }, - actions: [ - MruAdvance { - direction: Forward, - scope: None, - filter: Some( - All, - ), - }, - ], + action: Press( + [ + MruAdvance { + direction: Forward, + scope: None, + filter: Some( + All, + ), + }, + ], + ), repeat: true, + pass_through: None, cooldown: None, allow_when_locked: false, allow_inhibiting: true, @@ -2663,16 +3217,19 @@ mod tests { ALT, ), }, - actions: [ - MruAdvance { - direction: Forward, - scope: None, - filter: Some( - AppId, - ), - }, - ], + action: Press( + [ + MruAdvance { + direction: Forward, + scope: None, + filter: Some( + AppId, + ), + }, + ], + ), repeat: true, + pass_through: None, cooldown: None, allow_when_locked: false, allow_inhibiting: true, @@ -2687,18 +3244,21 @@ mod tests { SUPER, ), }, - actions: [ - MruAdvance { - direction: Forward, - scope: Some( - Output, - ), - filter: Some( - All, - ), - }, - ], + action: Press( + [ + MruAdvance { + direction: Forward, + scope: Some( + Output, + ), + filter: Some( + All, + ), + }, + ], + ), repeat: true, + pass_through: None, cooldown: None, allow_when_locked: false, allow_inhibiting: true, diff --git a/niri-config/src/recent_windows.rs b/niri-config/src/recent_windows.rs index e7594975db..fa39d8fe6c 100644 --- a/niri-config/src/recent_windows.rs +++ b/niri-config/src/recent_windows.rs @@ -1,10 +1,8 @@ -use std::collections::HashSet; - use knuffel::errors::DecodeError; use smithay::input::keyboard::Keysym; use crate::utils::{expect_only_children, MergeWith}; -use crate::{Action, Bind, Color, FloatOrInt, Key, Modifiers, Trigger}; +use crate::{Action, Bind, BoundAction, Color, FloatOrInt, Key, Modifiers, Trigger}; #[derive(Debug, PartialEq)] pub struct RecentWindows { @@ -60,7 +58,7 @@ impl MergeWith for RecentWindows { if let Some(part) = &part.binds { // Remove existing binds matching any new bind. self.binds - .retain(|bind| !part.0.iter().any(|new| new.key == bind.key)); + .retain(|bind| !part.0.iter().any(|new| new.key.conflicts_with(&bind.key))); // Add all new binds. self.binds.extend(part.0.iter().cloned().map(Bind::from)); } @@ -148,8 +146,9 @@ impl From for Bind { fn from(x: MruBind) -> Self { Self { key: x.key, - actions: vec![Action::from(x.action)], + action: BoundAction::Press(vec![Action::from(x.action)]), repeat: true, + pass_through: None, cooldown: None, allow_when_locked: false, allow_inhibiting: x.allow_inhibiting, @@ -262,26 +261,40 @@ where ) -> Result> { expect_only_children(node, ctx); - let mut seen_keys = HashSet::new(); - - let mut binds = Vec::new(); + let mut nodes: Vec<&knuffel::ast::SpannedNode> = Vec::new(); + let mut binds: Vec = Vec::new(); for child in node.children() { match MruBind::decode_node(child, ctx) { + Err(e) => { + ctx.emit_error(e); + } Ok(bind) => { - if !seen_keys.insert(bind.key) { + let conflict = binds + .iter() + .position(|prev| prev.key.conflicts_with(&bind.key)); + if let Some(idx) = conflict { + // Even though it's technically incorrect, we use + // `DecodeError::Missing` here because it labels the bind with + // "node starts here", which is the least bad option + ctx.emit_error(DecodeError::missing( + nodes[idx], + "keybind first defined here", + )); + ctx.emit_error(DecodeError::unexpected( &child.node_name, "keybind", - "duplicate keybind", + if binds[idx].key == bind.key { + "duplicate keybind later defined here" + } else { + "conflicting keybind later defined here" + }, )); - continue; + } else { + nodes.push(child); + binds.push(bind); } - - binds.push(bind); - } - Err(e) => { - ctx.emit_error(e); } } } @@ -319,8 +332,11 @@ where .parse::() .map_err(|e| DecodeError::conversion(&node.node_name, e.wrap_err("invalid keybind")))?; + let mut modifiers = key.modifiers; + modifiers &= !key.trigger.implied_modifier(); + // A modifier is required because MRU remains on screen as long as any modifier is held. - if key.modifiers.is_empty() { + if modifiers.is_empty() { ctx.emit_error(DecodeError::unexpected( &node.node_name, "keybind", @@ -330,7 +346,7 @@ where // FIXME: To support this, all the mods_with_mouse_binds()/mods_with_*wheel_binds()/etc. // will need to learn about recent-windows bindings. - if !matches!(key.trigger, Trigger::Keysym(_)) { + if !key.trigger.is_keyboard() { ctx.emit_error(DecodeError::unexpected( &node.node_name, "key", diff --git a/niri-config/src/utils.rs b/niri-config/src/utils.rs index 9787435cb1..acf397b275 100644 --- a/niri-config/src/utils.rs +++ b/niri-config/src/utils.rs @@ -2,7 +2,6 @@ use std::env::home_dir; use std::path::PathBuf; use std::str::FromStr; -use knuffel::ast::SpannedNode; use knuffel::errors::DecodeError; use miette::miette; use regex::Regex; @@ -298,22 +297,21 @@ pub fn parse_arg_node( +pub fn expand_home_path( path: PathBuf, - spanned: &SpannedNode, + spanned: &knuffel::span::Spanned, ctx: &mut knuffel::decode::Context, -) -> Option { +) -> Result> { if let Ok(rest) = path.strip_prefix("~") { let Some(home) = home_dir() else { - ctx.emit_error(DecodeError::missing( + return Err(DecodeError::conversion( spanned, format!("error retrieving home directory to expand {:?}", path), )); - return None; }; - home.join(rest).into() + Ok(home.join(rest)) } else { let base = ctx.get::().unwrap(); - base.0.join(path).into() + Ok(base.0.join(path)) } } diff --git a/resources/default-config.kdl b/resources/default-config.kdl index e059ee4ca0..0ac6e401e9 100644 --- a/resources/default-config.kdl +++ b/resources/default-config.kdl @@ -487,7 +487,21 @@ binds { // Open/close the Overview: a zoomed-out view of workspaces and windows. // You can also move the mouse into the top-left hot corner, // or do a four-finger swipe up on a touchpad. - Mod+O repeat=false { toggle-overview; } + // + // This is a release bind. + // Press and release Mod by itself to trigger it. + Mod { + release { toggle-overview; } + } + + // You can also bind both press and release actions like so. + // If the press action fires, the release action + // is also guaranteed to fire. + // This is useful for binds like push-to-talk. + // Alt+Shift { + // press { spawn "notify-send" "Enable push-to-talk!"; } + // release { spawn "notify-send" "Disable push-to-talk!"; } + // } Mod+Q repeat=false { close-window; } Alt+F4 repeat=false { close-window; } diff --git a/src/backend/mod.rs b/src/backend/mod.rs index 388f64f6ee..054c81e19e 100644 --- a/src/backend/mod.rs +++ b/src/backend/mod.rs @@ -230,6 +230,13 @@ impl Backend { } } + pub fn set_host_cursor_visible(&mut self, visible: bool) { + match self { + Backend::Winit(winit) => winit.set_cursor_visible(visible), + Backend::Tty(_) | Backend::Headless(_) => (), + } + } + pub fn tty_checked(&mut self) -> Option<&mut Tty> { if let Self::Tty(v) = self { Some(v) diff --git a/src/backend/tty.rs b/src/backend/tty.rs index f90aaae240..16ce70770a 100644 --- a/src/backend/tty.rs +++ b/src/backend/tty.rs @@ -631,6 +631,14 @@ impl Tty { SessionEvent::PauseSession => { debug!("pausing session"); + // We won't receive key or button releases for whatever is held down while the + // session is paused, so forget about all of it. + // + // Slightly cursed to do from an idle... But we don't have State here conveniently, + // and it should be fine to delay this until the loop callback. + niri.event_loop + .insert_idle(|state| state.release_held_inputs()); + self.libinput.suspend(); for device in self.devices.values_mut() { diff --git a/src/backend/winit.rs b/src/backend/winit.rs index fbcb9f299f..2c7e470e4c 100644 --- a/src/backend/winit.rs +++ b/src/backend/winit.rs @@ -13,6 +13,7 @@ use smithay::backend::renderer::damage::OutputDamageTracker; use smithay::backend::renderer::gles::GlesRenderer; use smithay::backend::renderer::{DebugFlags, ImportDma, ImportEgl, Renderer}; use smithay::backend::winit::{self, WinitEvent, WinitGraphicsBackend}; +use smithay::input::keyboard::KeyboardSource; use smithay::output::{Mode, Output, PhysicalProperties, Subpixel}; use smithay::reexports::calloop::LoopHandle; use smithay::reexports::wayland_protocols::wp::presentation_time::server::wp_presentation_feedback; @@ -56,6 +57,11 @@ impl Winit { )); let (backend, winit) = winit::init_from_attributes(builder)?; + // unless debug option is set, we dont show cursor in nested mode + if !config.borrow().debug.nested_show_host_cursor { + backend.window().set_cursor_visible(false); + } + let output = Output::new( "winit".to_string(), PhysicalProperties { @@ -147,7 +153,15 @@ impl Winit { state.niri.output_resized(&output); } WinitEvent::Input(event) => state.process_input_event(event), - WinitEvent::Focus(_) => (), + WinitEvent::Focus(false) => { + // Lost focus. Reset key pressed (like alt or windows key) + state.release_held_inputs(); + + if let Some(keyboard) = state.niri.seat.get_keyboard() { + keyboard.release_source(state, KeyboardSource::MAIN); + } + } + WinitEvent::Focus(true) => (), WinitEvent::Redraw => state.niri.queue_redraw(&state.backend.winit().output), WinitEvent::CloseRequested => state.niri.stop_signal.stop(), }) @@ -404,4 +418,8 @@ impl Winit { pub fn ipc_outputs(&self) -> Arc> { self.ipc_outputs.clone() } + + pub fn set_cursor_visible(&self, visible: bool) { + self.backend.window().set_cursor_visible(visible); + } } diff --git a/src/input/mod.rs b/src/input/mod.rs index 17c6e23cdd..c4543de2cb 100644 --- a/src/input/mod.rs +++ b/src/input/mod.rs @@ -1,13 +1,15 @@ use std::any::Any; use std::collections::hash_map::Entry; -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; +use std::sync::LazyLock; use std::time::Duration; use calloop::timer::{TimeoutAction, Timer}; +use calloop::RegistrationToken; use input::event::gesture::GestureEventCoordinates as _; use niri_config::{ - Action, Bind, Binds, Config, Key, ModKey, Modifiers, MruDirection, SwitchBinds, Trigger, - WorkspaceReference, + Action, Bind, Binds, BoundAction, Config, Key, ModKey, Modifiers, MruDirection, SwitchBinds, + Trigger, WorkspaceReference, }; use niri_ipc::LayoutSwitchTarget; use smithay::backend::input::{ @@ -21,9 +23,7 @@ use smithay::backend::input::{ use smithay::backend::libinput::LibinputInputBackend; use smithay::desktop::Window; use smithay::input::dnd::DnDGrab; -use smithay::input::keyboard::{ - keysyms, FilterResult, KeyboardSource, Keysym, Layout, ModifiersState, -}; +use smithay::input::keyboard::{keysyms, KeyboardSource, Keysym, Layout, ModifiersState}; use smithay::input::pointer::{ AxisFrame, ButtonEvent, CursorIcon, CursorImageStatus, Focus, GestureHoldBeginEvent, GestureHoldEndEvent, GesturePinchBeginEvent, GesturePinchEndEvent, GesturePinchUpdateEvent, @@ -55,7 +55,7 @@ use self::spatial_movement_grab::SpatialMovementGrab; use crate::dbus::freedesktop_a11y::KbMonBlock; use crate::layout::scrolling::ScrollDirection; use crate::layout::{ActivateWindow, LayoutElement as _}; -use crate::niri::{CastTarget, PointerVisibility, State}; +use crate::niri::{CastTarget, Niri, PointerVisibility, State}; use crate::ui::mru::{WindowMru, WindowMruUi}; use crate::ui::screenshot_ui::ScreenshotUi; use crate::utils::spawning::{spawn, spawn_sh}; @@ -138,6 +138,19 @@ impl AnyStartData { } } +/// What to do with a key event that the keyboard filter looked at. +#[derive(Debug)] +enum ShouldInterceptResult { + /// Forward the event to the focused client. + Forward, + /// Forward the event to the focused client, and handle the given bind. + ForwardAndHandle(Bind), + /// Do not forward the event, and handle the given bind. + InterceptAndHandle(Bind), + /// Do not forward the event, and do nothing else. + InterceptOnly, +} + impl State { pub fn process_input_event(&mut self, event: InputEvent) where @@ -541,6 +554,13 @@ impl State { // The accessibility modifier first press must not change XKB state, so we return // early here. if block == KbMonBlock::ModifierFirstPress { + // A pending release action recorded for the key must still run. + if !pressed { + if let Some(bind) = self.niri.pending_release_binds.remove(&event.key_code()) { + self.niri.suppressed_keys.remove(&event.key_code()); + self.handle_bind(bind, false); + } + } return; } block @@ -548,19 +568,88 @@ impl State { #[cfg(not(feature = "dbus"))] let _ = consumed_by_a11y; - let Some(Some(bind)) = self.niri.seat.get_keyboard().unwrap().input_from_source( + // We can't use smithay's `input` here because we want to forward _and_ handle + // events for modifier keys. This lets us do things like bind Control_L to an + // action without confusing applications about its state. + let keyboard = self.niri.seat.get_keyboard().unwrap(); + let Some((result, mods_changed)) = keyboard.input_intercept( source, self, event.key_code(), event.state(), - serial, - time, |this, mods, keysym| { - let key_code = event.key_code(); - let modified = keysym.modified_sym(); - let raw = keysym.raw_latin_sym_or_raw_current_sym(); + let key_code = keysym.raw_code(); let modifiers = modifiers_from_state(*mods); + // When resolving a bind, we can try three keysyms: + // - modified: the actual final keysym we got by pressing this key + // - raw: keysym from the 0th layer of the current layout (usually lowercase letter) + // - raw_ascii: keysym from the same key on an ASCII layout + // (if raw isn't ASCII itself) + let (modified, raw, raw_ascii) = { + let xkb = keysym.xkb(); + let xkb = xkb.lock().unwrap(); + + // SAFETY: xkb_state does not outlive xkb. + let (modified, layout) = unsafe { + let xkb_state = xkb.state(); + ( + xkb_state.key_get_one_sym(key_code), + Layout(xkb_state.key_get_layout(key_code)), + ) + }; + let modified = (modified != Keysym::NoSymbol).then_some(modified); + + let raw_sym = |layout| { + let raw_syms = xkb.raw_syms_for_key_in_layout(key_code, layout); + + // Match the xkb_key_get_one_sym() logic. + if raw_syms.len() == 1 { + Some(raw_syms[0]) + } else { + None + } + }; + + let raw = raw_sym(layout); + + // We include numbers and symbols too to have more binds working on us,ru and + // avoid some surprising behavior. + let is_ascii = |keysym: Keysym| { + keysym.key_char().is_some_and(|key| key.is_ascii_graphic()) + }; + + let mut raw_ascii = None; + + // Only check if raw is ASCII and not modified, so that e.g. pressing Shift + // does not change the fallback behavior on layouts where raw isn't ASCII but + // the shifted key is. + if !raw.is_some_and(is_ascii) { + // Haven't seen a latin keysym yet; try to find. + for layout in xkb.layouts().filter(|l| *l != layout) { + if let Some(raw) = raw_sym(layout) { + if is_ascii(raw) { + raw_ascii = Some(raw); + break; + } + } + } + } + + (modified, raw, raw_ascii) + }; + + // Avoid unnecessary lookups if raw is the same as modified. + let raw = raw.filter(|k| Some(*k) != modified); + + // We don't differentiate between raw and raw_ascii when it comes to bind handling. + let raw = match (raw, raw_ascii) { + (None, None) => &[][..], + (None, Some(b)) => &[b][..], + (Some(a), None) => &[a][..], + (Some(a), Some(b)) => &[a, b][..], + }; + // After updating XKB state from accessibility-grabbed keys, return right away and // don't handle them. #[cfg(feature = "dbus")] @@ -580,34 +669,40 @@ impl State { // modifiers without accidentally leaking them to wl_keyboard.enter. So for now // let's forward modifier releases to the clients here to deal with the most // common case. - if !pressed - && matches!( + if !pressed { + // The accessibility handler consumes the release. + // So, make sure to run any pending release actions. + let bind = this.niri.pending_release_binds.remove(&key_code); + this.niri.suppressed_keys.remove(&key_code); + + let forward = matches!( modified, - Keysym::Shift_L - | Keysym::Shift_R - | Keysym::Control_L - | Keysym::Control_R - | Keysym::Super_L - | Keysym::Super_R - | Keysym::Alt_L - | Keysym::Alt_R - ) - { - return FilterResult::Forward; - } else { - return FilterResult::Intercept(None); + Some( + Keysym::Shift_L + | Keysym::Shift_R + | Keysym::Control_L + | Keysym::Control_R + | Keysym::Super_L + | Keysym::Super_R + | Keysym::Alt_L + | Keysym::Alt_R + ) + ); + return bind_intercept_result(bind, forward); } + + return ShouldInterceptResult::InterceptOnly; } if this.niri.exit_confirm_dialog.is_open() && pressed { - if raw == Some(Keysym::Return) { + if modified == Some(Keysym::Return) { info!("quitting after confirming exit dialog"); this.niri.stop_signal.stop(); } // Don't send this press to any clients. this.niri.suppressed_keys.insert(key_code); - return FilterResult::Intercept(None); + return ShouldInterceptResult::InterceptOnly; } // Check if all modifiers were released while the MRU UI was open. If so, close the @@ -615,14 +710,16 @@ impl State { if this.niri.window_mru_ui.is_open() && !pressed && modifiers.is_empty() { this.do_action(Action::MruConfirm, false); - if this.niri.suppressed_keys.remove(&key_code) { - return FilterResult::Intercept(None); - } else { - return FilterResult::Forward; - } + // A pending release action recorded for the key must still run if it + // closed the MRU instead of going through the regular bind lookup. + return released_key_intercept_result( + &mut this.niri.suppressed_keys, + &mut this.niri.pending_release_binds, + key_code, + ); } - if pressed && raw == Some(Keysym::Escape) { + if pressed && modified == Some(Keysym::Escape) { // Cancel certain grabs on Escape. let pointer = this.niri.seat.get_pointer().unwrap(); if pointer @@ -631,60 +728,69 @@ impl State { { pointer.unset_grab(this, serial, time); this.niri.suppressed_keys.insert(key_code); - return FilterResult::Intercept(None); + return ShouldInterceptResult::InterceptOnly; } } - if let Some(Keysym::space) = raw { + if modified == Some(Keysym::space) { this.niri.screenshot_ui.set_space_down(pressed); } + if pressed { + this.niri.cancel_interrupted_release_binds(); + } + + let is_locked = this.niri.is_locked(); + let is_in_screenshot = this.niri.screenshot_ui.is_open(); let res = { let config = this.niri.config.borrow(); - let bindings = - make_binds_iter(&config, &mut this.niri.window_mru_ui, modifiers); + let bindings = make_binds_iter( + &config, + &this.niri.screenshot_ui, + &mut this.niri.window_mru_ui, + this.niri.keyboard_focus.is_overview(), + modifiers, + ); should_intercept_key( &mut this.niri.suppressed_keys, + &mut this.niri.pending_release_binds, + &this.niri.bind_cooldown_timers, bindings, mod_key, key_code, modified, raw, pressed, - *mods, - &this.niri.screenshot_ui, + modifiers, this.niri.config.borrow().input.disable_power_key_handling, is_inhibiting_shortcuts, + is_locked, + is_in_screenshot, ) }; - if matches!(res, FilterResult::Forward) { - // If we didn't find any bind, try other hardcoded keys. - if this.niri.keyboard_focus.is_overview() && pressed { - // Enter on the settled-remote carousel lens confirms the jump to that - // output's currently-focused window, same as clicking the lens — takes - // priority over the ordinary Escape/Return -> ToggleOverview hardcoded - // bind below, which would otherwise just close the overview back onto - // the (still-focused) host output. - if raw == Some(Keysym::Return) - && modifiers_from_state(*mods).is_empty() - && this.niri.layout.in_carousel_lens() - { - this.niri.layout.carousel_focus_jump_to_focused(); - // FIXME: granular. - this.niri.queue_redraw_all(); - this.niri.suppressed_keys.insert(key_code); - return FilterResult::Intercept(None); - } - - if let Some(bind) = raw.and_then(|raw| hardcoded_overview_bind(raw, *mods)) - { - this.niri.suppressed_keys.insert(key_code); - return FilterResult::Intercept(Some(bind)); - } + // Enter on the settled-remote carousel lens confirms the jump to that output's + // currently-focused window, same as clicking the lens. This takes priority over + // the hardcoded Return -> ToggleOverview overview bind, which would otherwise just + // close the overview back onto the (still-focused) host output. The key is + // already in suppressed_keys, and a press-only bind records no release. + if let ShouldInterceptResult::InterceptAndHandle(bind) = &res { + if bind.key.trigger == Trigger::Keysym(Keysym::Return) + && hardcoded_overview_binds().any(|b| b == bind) + && this.niri.layout.in_carousel_lens() + { + this.niri.layout.carousel_focus_jump_to_focused(); + // FIXME: granular. + this.niri.queue_redraw_all(); + return ShouldInterceptResult::InterceptOnly; } + } + if matches!( + res, + ShouldInterceptResult::Forward | ShouldInterceptResult::ForwardAndHandle(_) + ) { // Interaction with the active window, immediately update the active window's // focus timestamp without waiting for a possible pending MRU lock-in delay. this.niri.mru_apply_keyboard_commit(); @@ -696,13 +802,36 @@ impl State { return; }; - if !pressed { - return; + // Forward the event to the client, unless the key was intercepted. + match &result { + ShouldInterceptResult::Forward | ShouldInterceptResult::ForwardAndHandle(_) => { + keyboard.input_forward( + self, + event.key_code(), + event.state(), + serial, + time, + mods_changed, + ); + } + ShouldInterceptResult::InterceptAndHandle(_) | ShouldInterceptResult::InterceptOnly => { + if mods_changed { + keyboard.advertise_modifier_state(self); + } + } } - self.handle_bind(bind.clone()); - - self.start_key_repeat(bind); + // Handle the bind, if the result came with one. + match result { + ShouldInterceptResult::ForwardAndHandle(bind) + | ShouldInterceptResult::InterceptAndHandle(bind) => { + if pressed && bind.repeat { + self.start_key_repeat(bind.clone()); + } + self.handle_bind(bind, pressed); + } + ShouldInterceptResult::Forward | ShouldInterceptResult::InterceptOnly => {} + } } fn start_key_repeat(&mut self, bind: Bind) { @@ -731,7 +860,7 @@ impl State { .niri .event_loop .insert_source(repeat_timer, move |_, _, state| { - state.handle_bind(bind.clone()); + state.handle_bind(bind.clone(), true); TimeoutAction::ToDuration(repeat_duration) }) .unwrap(); @@ -812,8 +941,8 @@ impl State { let mut xkb = keyboard_config.xkb.clone(); let mut set_xkb_config = true; - if let Some(xkb_file) = xkb.file.take() { - if let Err(err) = self.set_xkb_file(xkb_file) { + if let Some(niri_config::input::XkbFile(Some(keymap))) = xkb.file.take() { + if let Err(err) = self.set_xkb_file_content(keymap) { warn!("error loading xkb_file for keyboard device: {err:?}"); } else { set_xkb_config = false; @@ -844,42 +973,89 @@ impl State { self.niri.current_keyboard = keyboard_config; } - pub fn handle_bind(&mut self, bind: Bind) { - let Some(cooldown) = bind.cooldown else { - self.do_actions(bind.actions, bind.allow_when_locked); - return; - }; + pub fn handle_bind(&mut self, bind: Bind, pressed: bool) { + // When true, every action of this phase may run while locked. + let bind_allowed = bind_allowed_when_locked(&bind, pressed); + let allowed_during_screenshot = bind_allowed_during_screenshot(&bind, pressed); // Check this first so that it doesn't trigger the cooldown. // // Run the bind if *any* of its actions would be allowed; do_action skips the rest // individually. - if self.niri.is_locked() - && !(bind.allow_when_locked || bind.actions.iter().any(allowed_when_locked)) + let phase_actions = if pressed { + bind.press_actions() + } else { + bind.release_actions() + }; + if self.niri.is_locked() && !(bind_allowed || phase_actions.iter().any(allowed_when_locked)) { return; } + if self.niri.screenshot_ui.is_open() && !allowed_during_screenshot { + return; + } - match self.niri.bind_cooldown_timers.entry(bind.key) { - // The bind is on cooldown. - Entry::Occupied(_) => (), - Entry::Vacant(entry) => { - let timer = Timer::from_duration(cooldown); - let token = self - .niri - .event_loop - .insert_source(timer, move |_, _, state| { - if state.niri.bind_cooldown_timers.remove(&bind.key).is_none() { - error!("bind cooldown timer entry disappeared"); - } - TimeoutAction::Drop - }) - .unwrap(); - entry.insert(token); - - self.do_actions(bind.actions, bind.allow_when_locked); + // Check to see if we are under a cooldown. + // We apply the cooldown to the press action for press-only and press/release. + // We apply the cooldown to the release action for release-only. + if pressed == bind.has_press() { + if let Some(cooldown) = bind.cooldown { + match self.niri.bind_cooldown_timers.entry(bind.key) { + // The bind is on cooldown, so don't run its action. + Entry::Occupied(_) => return, + Entry::Vacant(entry) => { + let timer = Timer::from_duration(cooldown); + let token = self + .niri + .event_loop + .insert_source(timer, move |_, _, state| { + if state.niri.bind_cooldown_timers.remove(&bind.key).is_none() { + error!("bind cooldown timer entry disappeared"); + } + TimeoutAction::Drop + }) + .unwrap(); + entry.insert(token); + } + } } } + + let Some(actions) = bind.into_actions(pressed) else { + return; + }; + + self.do_actions(actions, bind_allowed); + } + + /// Forgets all held inputs, running the release actions of anything that was still held. + pub fn release_held_inputs(&mut self) { + let mut binds = Vec::new(); + binds.extend( + self.niri + .pending_release_binds + .drain() + .map(|(_, bind)| bind), + ); + binds.extend( + self.niri + .pending_mouse_release_binds + .drain() + .map(|(_, bind)| bind), + ); + binds.extend( + self.niri + .pending_tablet_release_binds + .drain() + .map(|(_, bind)| bind), + ); + + // Forget the rest of the input state: the releases will never arrive. + self.niri.clear_input_state(); + + for bind in binds { + self.handle_bind(bind, false); + } } pub fn do_actions(&mut self, actions: Vec, allow_when_locked: bool) { @@ -908,26 +1084,18 @@ impl State { self.niri.stop_signal.stop() } Action::ChangeVt(vt) => { - self.backend.change_vt(vt); - - // Changing VT may not deliver the key releases, so clear the state. - self.niri.suppressed_keys.clear(); + // Changing VT may not deliver the key releases, so treat the held binds as + // released while the session is still usable. + self.release_held_inputs(); - // Stop ongoing key repeat if any. - if let Some(token) = self.niri.bind_repeat_timer.take() { - self.niri.event_loop.remove(token); - } + self.backend.change_vt(vt); } Action::Suspend => { - self.backend.suspend(); - - // Suspend may not deliver the key releases, so clear the state. - self.niri.suppressed_keys.clear(); + // Suspend may not deliver the key releases, so treat the held binds as + // released before suspending. + self.release_held_inputs(); - // Stop ongoing key repeat if any. - if let Some(token) = self.niri.bind_repeat_timer.take() { - self.niri.event_loop.remove(token); - } + self.backend.suspend(); } Action::PowerOffMonitors(skip_isolated) => { self.niri @@ -3334,6 +3502,26 @@ impl State { self.niri.queue_redraw_all(); } + fn find_pointer_bind( + &mut self, + mod_key: ModKey, + trigger: Trigger, + mods: ModifiersState, + ) -> Option { + let modifiers = modifiers_from_state(mods); + + let config = self.niri.config.borrow(); + let bindings = make_binds_iter( + &config, + &self.niri.screenshot_ui, + &mut self.niri.window_mru_ui, + self.niri.keyboard_focus.is_overview(), + modifiers, + ); + + find_configured_bind(bindings, mod_key, trigger, mods) + } + fn on_pointer_button(&mut self, event: I::PointerButtonEvent) { let pointer = self.niri.seat.get_pointer().unwrap(); @@ -3347,16 +3535,25 @@ impl State { let mod_key = self.backend.mod_key(&self.niri.config.borrow()); - // Ignore release events for mouse clicks that triggered a bind. - if self.niri.suppressed_buttons.remove(&button_code) { - return; - } - let mods = self.niri.seat.get_keyboard().unwrap().modifier_state(); let modifiers = modifiers_from_state(mods); let mod_down = modifiers.contains(mod_key.to_modifiers()); + if ButtonState::Released == button_state { + if let Some(bind) = self.niri.pending_mouse_release_binds.remove(&button_code) { + self.niri.suppressed_buttons.remove(&button_code); + self.handle_bind(bind, false); + return; + } + } + + if self.niri.suppressed_buttons.remove(&button_code) { + return; + } + if ButtonState::Pressed == button_state { + self.niri.cancel_interrupted_release_binds(); + let mut is_mru_open = false; if let Some(mru_output) = self.niri.window_mru_ui.output() { is_mru_open = true; @@ -3381,6 +3578,8 @@ impl State { if is_mru_open || self.niri.mods_with_mouse_binds.contains(&modifiers) { if let Some(bind) = match button { + // Screenshot UI takes precedence over binds. + Some(MouseButton::Left) if self.niri.screenshot_ui.is_open() => None, Some(MouseButton::Left) => Some(Trigger::MouseLeft), Some(MouseButton::Right) => Some(Trigger::MouseRight), Some(MouseButton::Middle) => Some(Trigger::MouseMiddle), @@ -3388,19 +3587,24 @@ impl State { Some(MouseButton::Forward) => Some(Trigger::MouseForward), _ => None, } - .and_then(|trigger| { - let config = self.niri.config.borrow(); - let bindings = - make_binds_iter(&config, &mut self.niri.window_mru_ui, modifiers); - find_configured_bind(bindings, mod_key, trigger, mods) - }) - .filter(|bind| { - !self.niri.screenshot_ui.is_open() || bind_allowed_during_screenshot(bind) - }) { + .and_then(|trigger| self.find_pointer_bind(mod_key, trigger, mods)) + { self.niri.suppressed_buttons.insert(button_code); - self.handle_bind(bind.clone()); + if should_record_release_bind( + &bind, + self.niri.is_locked(), + self.niri.screenshot_ui.is_open(), + &self.niri.bind_cooldown_timers, + ) { + self.niri + .pending_mouse_release_binds + .insert(button_code, bind.clone()); + } + if bind.has_press() { + self.handle_bind(bind.clone(), true); + } return; - }; + } } // We received an event for the regular pointer, so show it now. @@ -3726,6 +3930,8 @@ impl State { } fn on_pointer_axis(&mut self, event: I::PointerAxisEvent) { + self.niri.cancel_interrupted_release_binds(); + let pointer = &self.niri.seat.get_pointer().unwrap(); let source = event.source(); @@ -3793,13 +3999,14 @@ impl State { trigger: Trigger::WheelScrollLeft, modifiers: Modifiers::empty(), }, - actions: vec![match target { + action: BoundAction::Press(vec![match target { OverviewWheelTarget::Column => Action::FocusColumnLeftUnderMouse, OverviewWheelTarget::Workspace => { Action::FocusWorkspaceUpUnderMouse } - }], + }]), repeat: true, + pass_through: None, cooldown: None, allow_when_locked: false, allow_inhibiting: false, @@ -3810,13 +4017,14 @@ impl State { trigger: Trigger::WheelScrollRight, modifiers: Modifiers::empty(), }, - actions: vec![match target { + action: BoundAction::Press(vec![match target { OverviewWheelTarget::Column => Action::FocusColumnRightUnderMouse, OverviewWheelTarget::Workspace => { Action::FocusWorkspaceDownUnderMouse } - }], + }]), repeat: true, + pass_through: None, cooldown: None, allow_when_locked: false, allow_inhibiting: false, @@ -3824,40 +4032,20 @@ impl State { }); (bind_left, bind_right) } else { - let config = self.niri.config.borrow(); - let bindings = - make_binds_iter(&config, &mut self.niri.window_mru_ui, modifiers); - let bind_left = find_configured_bind( - bindings.clone(), - mod_key, - Trigger::WheelScrollLeft, - mods, - ) - .filter(|bind| { - !self.niri.screenshot_ui.is_open() - || bind_allowed_during_screenshot(bind) - }); - let bind_right = find_configured_bind( - bindings, - mod_key, - Trigger::WheelScrollRight, - mods, + ( + self.find_pointer_bind(mod_key, Trigger::WheelScrollLeft, mods), + self.find_pointer_bind(mod_key, Trigger::WheelScrollRight, mods), ) - .filter(|bind| { - !self.niri.screenshot_ui.is_open() - || bind_allowed_during_screenshot(bind) - }); - (bind_left, bind_right) }; if let Some(right) = bind_right { for _ in 0..ticks { - self.handle_bind(right.clone()); + self.handle_bind(right.clone(), true); } } if let Some(left) = bind_left { for _ in ticks..0 { - self.handle_bind(left.clone()); + self.handle_bind(left.clone(), true); } } } @@ -3900,13 +4088,14 @@ impl State { trigger: Trigger::WheelScrollUp, modifiers: Modifiers::empty(), }, - actions: vec![match target { + action: BoundAction::Press(vec![match target { OverviewWheelTarget::Column => Action::FocusColumnLeftUnderMouse, OverviewWheelTarget::Workspace => { Action::FocusWorkspaceUpUnderMouse } - }], + }]), repeat: true, + pass_through: None, cooldown: Some(Duration::from_millis(50)), allow_when_locked: false, allow_inhibiting: false, @@ -3917,13 +4106,14 @@ impl State { trigger: Trigger::WheelScrollDown, modifiers: Modifiers::empty(), }, - actions: vec![match target { + action: BoundAction::Press(vec![match target { OverviewWheelTarget::Column => Action::FocusColumnRightUnderMouse, OverviewWheelTarget::Workspace => { Action::FocusWorkspaceDownUnderMouse } - }], + }]), repeat: true, + pass_through: None, cooldown: Some(Duration::from_millis(50)), allow_when_locked: false, allow_inhibiting: false, @@ -3931,36 +4121,20 @@ impl State { }); (bind_up, bind_down) } else { - let config = self.niri.config.borrow(); - let bindings = - make_binds_iter(&config, &mut self.niri.window_mru_ui, modifiers); - let bind_up = find_configured_bind( - bindings.clone(), - mod_key, - Trigger::WheelScrollUp, - mods, + ( + self.find_pointer_bind(mod_key, Trigger::WheelScrollUp, mods), + self.find_pointer_bind(mod_key, Trigger::WheelScrollDown, mods), ) - .filter(|bind| { - !self.niri.screenshot_ui.is_open() - || bind_allowed_during_screenshot(bind) - }); - let bind_down = - find_configured_bind(bindings, mod_key, Trigger::WheelScrollDown, mods) - .filter(|bind| { - !self.niri.screenshot_ui.is_open() - || bind_allowed_during_screenshot(bind) - }); - (bind_up, bind_down) }; if let Some(down) = bind_down { for _ in 0..ticks { - self.handle_bind(down.clone()); + self.handle_bind(down.clone(), true); } } if let Some(up) = bind_up { for _ in ticks..0 { - self.handle_bind(up.clone()); + self.handle_bind(up.clone(), true); } } } @@ -4090,34 +4264,19 @@ impl State { .horizontal_finger_scroll_tracker .accumulate(horizontal); if ticks != 0 { - let config = self.niri.config.borrow(); - let bindings = - make_binds_iter(&config, &mut self.niri.window_mru_ui, modifiers); - let bind_left = find_configured_bind( - bindings.clone(), - mod_key, - Trigger::TouchpadScrollLeft, - mods, - ) - .filter(|bind| { - !self.niri.screenshot_ui.is_open() || bind_allowed_during_screenshot(bind) - }); - let bind_right = - find_configured_bind(bindings, mod_key, Trigger::TouchpadScrollRight, mods) - .filter(|bind| { - !self.niri.screenshot_ui.is_open() - || bind_allowed_during_screenshot(bind) - }); - drop(config); + let (bind_left, bind_right) = ( + self.find_pointer_bind(mod_key, Trigger::TouchpadScrollLeft, mods), + self.find_pointer_bind(mod_key, Trigger::TouchpadScrollRight, mods), + ); if let Some(right) = bind_right { for _ in 0..ticks { - self.handle_bind(right.clone()); + self.handle_bind(right.clone(), true); } } if let Some(left) = bind_left { for _ in ticks..0 { - self.handle_bind(left.clone()); + self.handle_bind(left.clone(), true); } } } @@ -4127,34 +4286,19 @@ impl State { .vertical_finger_scroll_tracker .accumulate(vertical); if ticks != 0 { - let config = self.niri.config.borrow(); - let bindings = - make_binds_iter(&config, &mut self.niri.window_mru_ui, modifiers); - let bind_up = find_configured_bind( - bindings.clone(), - mod_key, - Trigger::TouchpadScrollUp, - mods, - ) - .filter(|bind| { - !self.niri.screenshot_ui.is_open() || bind_allowed_during_screenshot(bind) - }); - let bind_down = - find_configured_bind(bindings, mod_key, Trigger::TouchpadScrollDown, mods) - .filter(|bind| { - !self.niri.screenshot_ui.is_open() - || bind_allowed_during_screenshot(bind) - }); - drop(config); + let (bind_up, bind_down) = ( + self.find_pointer_bind(mod_key, Trigger::TouchpadScrollUp, mods), + self.find_pointer_bind(mod_key, Trigger::TouchpadScrollDown, mods), + ); if let Some(down) = bind_down { for _ in 0..ticks { - self.handle_bind(down.clone()); + self.handle_bind(down.clone(), true); } } if let Some(up) = bind_up { for _ in ticks..0 { - self.handle_bind(up.clone()); + self.handle_bind(up.clone(), true); } } } @@ -4331,6 +4475,8 @@ impl State { match tip_state { TabletToolTipState::Down => { + self.niri.cancel_interrupted_release_binds(); + if let Some(pos) = self.niri.tablet_cursor_location { let under = self.niri.contents_under(pos); @@ -4546,10 +4692,9 @@ impl State { if let Some(tool) = tool { let button = event.button(); + let button_state = event.button_state(); - if self.niri.suppressed_buttons.remove(&button) { - return; - } + let mod_key = self.backend.mod_key(&self.niri.config.borrow()); let trigger = match button { BTN_STYLUS => Some(Trigger::TabletStylusButton1), @@ -4558,25 +4703,45 @@ impl State { _ => None, }; + // Handle release binds. + if button_state == ButtonState::Released { + if let Some(bind) = self.niri.pending_tablet_release_binds.remove(&button) { + self.niri.suppressed_buttons.remove(&button); + self.handle_bind(bind, false); + return; + } + } + + if self.niri.suppressed_buttons.remove(&button) { + return; + } + + if button_state == ButtonState::Pressed { + self.niri.cancel_interrupted_release_binds(); + } + + // Handle press binds. if let Some(trigger) = trigger { - if event.button_state() == ButtonState::Pressed { - let mod_key = self.backend.mod_key(&self.niri.config.borrow()); + if button_state == ButtonState::Pressed { let mods = self.niri.seat.get_keyboard().unwrap().modifier_state(); let modifiers = modifiers_from_state(mods); if self.niri.mods_with_tablet_stylus_binds.contains(&modifiers) { - let bind = { - let config = self.niri.config.borrow(); - let bindings = config.binds.0.iter(); - find_configured_bind(bindings, mod_key, trigger, mods) - } - .filter(|bind| { - !self.niri.screenshot_ui.is_open() - || bind_allowed_during_screenshot(bind) - }); - if let Some(bind) = bind { + if let Some(bind) = self.find_pointer_bind(mod_key, trigger, mods) { self.niri.suppressed_buttons.insert(button); - self.handle_bind(bind.clone()); + if should_record_release_bind( + &bind, + self.niri.is_locked(), + self.niri.screenshot_ui.is_open(), + &self.niri.bind_cooldown_timers, + ) { + self.niri + .pending_tablet_release_binds + .insert(button, bind.clone()); + } + if bind.has_press() { + self.handle_bind(bind.clone(), true); + } return; } } @@ -4590,7 +4755,7 @@ impl State { &tablet::tool::ButtonEvent { serial: SERIAL_COUNTER.next_serial(), button, - state: event.button_state(), + state: button_state, time, }, ); @@ -4600,6 +4765,8 @@ impl State { } fn on_gesture_swipe_begin(&mut self, event: I::GestureSwipeBeginEvent) { + self.niri.cancel_interrupted_release_binds(); + if self.niri.window_mru_ui.is_open() || self.niri.screenshot_ui.is_open() { // Don't start swipe gestures while in the compositor UI. return; @@ -4808,6 +4975,8 @@ impl State { } fn on_gesture_pinch_begin(&mut self, event: I::GesturePinchBeginEvent) { + self.niri.cancel_interrupted_release_binds(); + let serial = SERIAL_COUNTER.next_serial(); let pointer = self.niri.seat.get_pointer().unwrap(); @@ -4879,6 +5048,8 @@ impl State { } fn on_gesture_hold_begin(&mut self, event: I::GestureHoldBeginEvent) { + self.niri.cancel_interrupted_release_binds(); + let serial = SERIAL_COUNTER.next_serial(); let pointer = self.niri.seat.get_pointer().unwrap(); @@ -4950,6 +5121,8 @@ impl State { }; let slot = evt.slot(); + self.niri.cancel_interrupted_release_binds(); + let serial = SERIAL_COUNTER.next_serial(); let under = self.niri.contents_under(pos); @@ -5182,31 +5355,83 @@ impl State { } } -/// Check whether the key should be intercepted and mark intercepted -/// pressed keys as `suppressed`, thus preventing `releases` corresponding -/// to them from being delivered. +impl Niri { + fn cancel_interrupted_release_binds(&mut self) { + retain_press_binds(&mut self.pending_release_binds); + retain_press_binds(&mut self.pending_mouse_release_binds); + retain_press_binds(&mut self.pending_tablet_release_binds); + } +} + +fn retain_press_binds(binds: &mut HashMap) { + binds.retain(|_, bind| bind.has_press()); +} + +fn released_key_intercept_result( + suppressed_keys: &mut HashSet, + pending_release_binds: &mut HashMap, + key_code: Keycode, +) -> ShouldInterceptResult { + let bind = pending_release_binds.remove(&key_code); + let suppressed = suppressed_keys.remove(&key_code); + + bind_intercept_result(bind, !suppressed) +} + +fn bind_intercept_result(bind: Option, forward: bool) -> ShouldInterceptResult { + match (bind, forward) { + (Some(bind), true) => ShouldInterceptResult::ForwardAndHandle(bind), + (Some(bind), false) => ShouldInterceptResult::InterceptAndHandle(bind), + (None, true) => ShouldInterceptResult::Forward, + (None, false) => ShouldInterceptResult::InterceptOnly, + } +} + +fn is_bind_on_cooldown(bind: &Bind, cooldown_timers: &HashMap) -> bool { + bind.cooldown.is_some() && cooldown_timers.contains_key(&bind.key) +} + +/// Checks whether the key should be intercepted, and tracks the presses of intercepted keys. +/// +/// An intercepted key press is recorded in `suppressed_keys`, which prevents its release from +/// being forwarded to clients. If the matching bind has a release action, the bind is also +/// recorded in `pending_release_binds`, so that its release action triggers when the key is +/// released, regardless of the modifiers held at that point. +/// +/// A release action only triggers if the bind's modifiers matched when the key was pressed. +/// +/// A release-only bind is cancelled by other input before this runs; see +/// `cancel_interrupted_release_binds()`. A bind with a press action always runs its release action. +/// +/// A bind with `pass-through` still runs its actions, but also forwards the presses and releases to +/// clients. +/// +/// A bind whose press action is rate-limited by its cooldown is skipped entirely: +/// No action runs, and no release action is recorded. #[allow(clippy::too_many_arguments)] fn should_intercept_key<'a>( suppressed_keys: &mut HashSet, - bindings: impl IntoIterator, + pending_release_binds: &mut HashMap, + cooldown_timers: &HashMap, + bindings: impl Iterator + Clone, mod_key: ModKey, key_code: Keycode, - modified: Keysym, - raw: Option, + modified: Option, + raw: &[Keysym], pressed: bool, - mods: ModifiersState, - screenshot_ui: &ScreenshotUi, + mods: Modifiers, disable_power_key_handling: bool, is_inhibiting_shortcuts: bool, -) -> FilterResult> { - // Actions are only triggered on presses, release of the key - // shouldn't try to intercept anything unless we have marked - // the key to suppress. - if !pressed && !suppressed_keys.contains(&key_code) { - return FilterResult::Forward; + is_locked: bool, + is_in_screenshot: bool, +) -> ShouldInterceptResult { + // A release never runs a press action, so its result depends only on the bind recorded when + // the key was pressed, and on whether that press was suppressed. + if !pressed { + return released_key_intercept_result(suppressed_keys, pending_release_binds, key_code); } - let mut final_bind = find_bind( + let bind = find_bind( bindings, mod_key, modified, @@ -5214,75 +5439,56 @@ fn should_intercept_key<'a>( mods, disable_power_key_handling, ); + // If we don't have a bind, forward the key. + let Some(bind) = bind else { + return ShouldInterceptResult::Forward; + }; - // Allow only a subset of compositor actions while the screenshot UI is open, since the user - // cannot see the screen. - if screenshot_ui.is_open() { - let mut use_screenshot_ui_action = true; - - if let Some(bind) = &final_bind { - if bind_allowed_during_screenshot(bind) { - use_screenshot_ui_action = false; - } - } + // When inhibiting shortcuts, forward the key. + if is_inhibiting_shortcuts && bind.allow_inhibiting { + return ShouldInterceptResult::Forward; + } - if use_screenshot_ui_action { - if let Some(raw) = raw { - final_bind = screenshot_ui.action(raw, mods).map(|action| Bind { - key: Key { - trigger: Trigger::Keysym(raw), - // Not entirely correct but it doesn't matter in how we currently use it. - modifiers: Modifiers::empty(), - }, - actions: vec![action], - repeat: true, - cooldown: None, - allow_when_locked: false, - // The screenshot UI owns the focus anyway, so this doesn't really matter. - // But logically, nothing can inhibit its actions. Only opening it can be - // inhibited. - allow_inhibiting: false, - hotkey_overlay_title: None, - }); - } - } + // By default we intercept the key which hides both the press and release from clients. + // Clients still learn about the modifier state via `advertise_modifier_state()`. + // + // Release-only binds are not intercepted by default so that bindings like `Ctrl+Alt` + // do not break application shortcuts. + let forward = bind.pass_through.unwrap_or(!bind.has_press()); + if !forward { + suppressed_keys.insert(key_code); } - match (final_bind, pressed) { - (Some(bind), true) => { - if is_inhibiting_shortcuts && bind.allow_inhibiting { - FilterResult::Forward - } else { - suppressed_keys.insert(key_code); - FilterResult::Intercept(Some(bind)) - } - } - (_, false) => { - // By this point, we know that the key was suppressed on press. Even if we're inhibiting - // shortcuts, we should still suppress the release. - // But we don't need to check for shortcuts inhibition here, because - // if it was inhibited on press (forwarded to the client), it wouldn't be suppressed, - // so the release would already have been forwarded at the start of this function. - suppressed_keys.remove(&key_code); - FilterResult::Intercept(None) - } - (None, true) => FilterResult::Forward, + if should_record_release_bind(&bind, is_locked, is_in_screenshot, cooldown_timers) { + pending_release_binds.insert(key_code, bind.clone()); } + + // We need to check cooldown here matching the check in should_record_release_bind() above. + // Otherwise, a bind on cooldown would start repeat, which would then trigger its press action + // without recording the corresponding release action. + // + // Another (arguably more correct) way would be to ensure a pending release bind when running + // key repeat. If that's implemented, this condition can be simplified. + let run_press_action = bind.has_press() && !is_bind_on_cooldown(&bind, cooldown_timers); + + bind_intercept_result(run_press_action.then_some(bind), forward) } -fn find_bind<'a>( - bindings: impl IntoIterator, +fn find_bind<'a, I: Iterator + Clone>( + bindings: I, mod_key: ModKey, - modified: Keysym, - raw: Option, - mods: ModifiersState, + modified: Option, + raw: &[Keysym], + mods: Modifiers, disable_power_key_handling: bool, ) -> Option { + let _span = tracy_client::span!("find_bind"); + use keysyms::*; // Handle hardcoded binds. #[allow(non_upper_case_globals)] // wat - let hardcoded_action = match modified.raw() { + let hardcoded_action = match modified.map_or(KEY_NoSymbol, Keysym::raw) { modified @ KEY_XF86Switch_VT_1..=KEY_XF86Switch_VT_12 => { let vt = (modified - KEY_XF86Switch_VT_1 + 1) as i32; Some(Action::ChangeVt(vt)) @@ -5295,13 +5501,14 @@ fn find_bind<'a>( return Some(Bind { key: Key { // Not entirely correct but it doesn't matter in how we currently use it. - trigger: Trigger::Keysym(modified), + trigger: Trigger::Keysym(modified.unwrap()), modifiers: Modifiers::empty(), }, - actions: vec![action], + action: BoundAction::Press(vec![action]), // Repeating VT switches and power off doesn't make sense (and it actually happens on // FreeBSD where it doesn't get released and ends up broken). repeat: false, + pass_through: None, cooldown: None, allow_when_locked: false, // In a worst-case scenario, the user has no way to unlock the compositor and a @@ -5314,42 +5521,122 @@ fn find_bind<'a>( }); } - let trigger = Trigger::Keysym(raw?); - find_configured_bind(bindings, mod_key, trigger, mods) + // A modifier key can trigger binds bound as the compositor mod key (`Mod`), by its own keysym + // (like `Alt_L`), or as the modifier itself (like `Alt`). Return the first bind that matches. + let find = |mut bindings: I, key: Keysym| { + bindings + .find(|bind| { + let bind_trigger = bind.key.trigger; + // Check that the bind modifiers match the held modifiers. + // + // Pass the bind's trigger itself since we'll check the trigger match below. + // (Note that this means mod_key is not properly checked when it matches the + // trigger.) + if !bind_matches_trigger(bind, bind_trigger, mod_key, mods) { + return false; + } + + // Binds that consist only of generic modifiers can be hit in any order (trigger key + // does not have to be the last). This is consistent with how these types of binds + // work elsewhere. + // + // For example: + // - Alt + Shift to switch keyboard layout on Windows + // - Ctrl + Alt to show inlay hints in VSCode + if bind_trigger.is_generic_modifier() { + // Bind trigger matching compositor mod isn't properly checked above. + // Check it here. + if (bind_trigger == Trigger::CompositorMod + || bind_trigger == Trigger::Modifier(mod_key)) + && !mods.contains(mod_key.to_modifiers()) + { + return false; + } + + // Check that the key we pressed is one of the bind modifiers. + let mut raw_mods = Trigger::Keysym(key).implied_modifier(); + if mod_key.matches_keysym(key) { + raw_mods |= Modifiers::COMPOSITOR; + } + return !raw_mods.is_empty() && bind.key.modifiers.intersects(raw_mods); + } + + // Modifier-only binds can be written in several ways: + // - `Mod { quit; }` - Trigger::CompositorMod + // - `Super { quit; }` - Trigger::Modifier(_) + // - `Super_L { quit; }` - Trigger::Keysym(_) + // + // These only matter for keyboard keys and not for mouse or others. So, check them + // here. + trigger_matches_keysym(bind_trigger, key, mod_key) + }) + .cloned() + }; + + let mut keys = modified.into_iter().chain(raw.iter().copied()); + keys.find_map(|key| find(bindings.clone(), key)) } +fn trigger_matches_keysym(trigger: Trigger, key: Keysym, mod_key: ModKey) -> bool { + match trigger { + Trigger::CompositorMod => mod_key.matches_keysym(key), + Trigger::Keysym(keysym) => keysym == key, + Trigger::Modifier(modifier) => modifier.matches_keysym(key), + _ => false, + } +} + +/// Finds the configured bind for a trigger and held modifiers. +/// +/// Returns the first bind with this exact trigger whose modifiers match, so overlapping binds +/// like `Mod+Q` and `Super+Q` are resolved by definition order. fn find_configured_bind<'a>( bindings: impl IntoIterator, mod_key: ModKey, trigger: Trigger, mods: ModifiersState, ) -> Option { - // Handle configured binds. - let mut modifiers = modifiers_from_state(mods); + let modifiers = modifiers_from_state(mods); + + bindings + .into_iter() + .find(|bind| bind_matches_trigger(bind, trigger, mod_key, modifiers)) + .cloned() +} - let mod_down = modifiers_from_state(mods).contains(mod_key.to_modifiers()); - if mod_down { - modifiers |= Modifiers::COMPOSITOR; +fn bind_matches_trigger(bind: &Bind, trigger: Trigger, mod_key: ModKey, mods: Modifiers) -> bool { + if bind.key.trigger != trigger { + return false; } - for bind in bindings { - if bind.key.trigger != trigger { - continue; - } + let mut held_modifiers = mods; + let mut required_modifiers = bind.key.modifiers; - let mut bind_modifiers = bind.key.modifiers; - if bind_modifiers.contains(Modifiers::COMPOSITOR) { - bind_modifiers |= mod_key.to_modifiers(); - } else if bind_modifiers.contains(mod_key.to_modifiers()) { - bind_modifiers |= Modifiers::COMPOSITOR; - } + // Check if the trigger is the mod key itself, either as `Mod`, as its own keysym (like + // `Super_L`), or as the modifier itself (like `Super`). If so, normalize the modifiers not + // to need it. + let trigger_is_mod_key = match trigger { + Trigger::CompositorMod => true, + Trigger::Modifier(modifier) => modifier == mod_key, + Trigger::Keysym(keysym) => mod_key.matches_keysym(keysym), + _ => false, + }; - if bind_modifiers == modifiers { - return Some(bind.clone()); + if trigger_is_mod_key { + held_modifiers.remove(mod_key.to_modifiers()); + required_modifiers.remove(Modifiers::COMPOSITOR | mod_key.to_modifiers()); + } else { + if held_modifiers.contains(mod_key.to_modifiers()) { + held_modifiers |= Modifiers::COMPOSITOR; + } + if required_modifiers.contains(Modifiers::COMPOSITOR) { + required_modifiers |= mod_key.to_modifiers(); + } else if required_modifiers.contains(mod_key.to_modifiers()) { + required_modifiers |= Modifiers::COMPOSITOR; } } - None + required_modifiers == held_modifiers } fn find_configured_switch_action( @@ -5477,6 +5764,41 @@ fn allowed_when_locked(action: &Action) -> bool { ) } +fn bind_allowed_when_locked(bind: &Bind, pressed: bool) -> bool { + if bind.allow_when_locked { + return true; + } + + match &bind.action { + BoundAction::Press(actions) | BoundAction::Release(actions) => { + actions.iter().all(allowed_when_locked) + } + BoundAction::Both { press, release } => { + if pressed { + press.iter().chain(release).all(allowed_when_locked) + } else { + // The press ran, so the release must run too. + true + } + } + } +} + +fn should_record_release_bind( + bind: &Bind, + is_locked: bool, + is_in_screenshot: bool, + cooldown_timers: &HashMap, +) -> bool { + if bind.has_press() && is_bind_on_cooldown(bind, cooldown_timers) { + return false; + } + + bind.has_release() + && (!is_locked || bind_allowed_when_locked(bind, true)) + && (!is_in_screenshot || bind_allowed_during_screenshot(bind, true)) +} + fn allowed_during_screenshot(action: &Action) -> bool { matches!( action, @@ -5515,48 +5837,29 @@ fn allowed_during_screenshot(action: &Action) -> bool { | Action::SetWindowWidth(_) | Action::SetWindowHeight(_) | Action::SetColumnWidth(_) + // Screenshot UI's own actions. + | Action::CancelScreenshot + | Action::ConfirmScreenshot { .. } + | Action::ScreenshotTogglePointer ) } /// A bind is usable while the screenshot UI is open only if *every* one of its actions is. /// The screenshot UI must never be handed a bind it can only half-run. -fn bind_allowed_during_screenshot(bind: &Bind) -> bool { - bind.actions.iter().all(allowed_during_screenshot) -} - -fn hardcoded_overview_bind(raw: Keysym, mods: ModifiersState) -> Option { - let mods = modifiers_from_state(mods); - if !mods.is_empty() { - return None; - } - - let mut repeat = true; - let action = match raw { - Keysym::Escape | Keysym::Return => { - repeat = false; - Action::ToggleOverview +fn bind_allowed_during_screenshot(bind: &Bind, pressed: bool) -> bool { + match &bind.action { + BoundAction::Press(actions) | BoundAction::Release(actions) => { + actions.iter().all(allowed_during_screenshot) } - Keysym::Left => Action::FocusColumnLeft, - Keysym::Right => Action::FocusColumnRight, - Keysym::Up => Action::FocusWindowOrWorkspaceUp, - Keysym::Down => Action::FocusWindowOrWorkspaceDown, - _ => { - return None; + BoundAction::Both { press, release } => { + if pressed { + press.iter().chain(release).all(allowed_during_screenshot) + } else { + // The press ran, so the release must run too. + true + } } - }; - - Some(Bind { - key: Key { - trigger: Trigger::Keysym(raw), - modifiers: Modifiers::empty(), - }, - actions: vec![action], - repeat, - cooldown: None, - allow_when_locked: false, - allow_inhibiting: false, - hotkey_overlay_title: None, - }) + } } pub fn apply_libinput_settings( @@ -5955,16 +6258,51 @@ fn grab_allows_hot_corner(grab: &(dyn PointerGrab + 'static)) -> bool { true } +fn hardcoded_overview_binds<'a>() -> impl Iterator + Clone { + fn bind(key: Keysym, action: Action, repeat: bool) -> Bind { + Bind { + key: Key { + trigger: Trigger::Keysym(key), + modifiers: Modifiers::empty(), + }, + action: BoundAction::Press(vec![action]), + repeat, + pass_through: None, + cooldown: None, + allow_when_locked: false, + allow_inhibiting: false, + hotkey_overlay_title: None, + } + } + + static BINDS: LazyLock<[Bind; 6]> = LazyLock::new(|| { + [ + bind(Keysym::Escape, Action::ToggleOverview, false), + bind(Keysym::Return, Action::ToggleOverview, false), + bind(Keysym::Left, Action::FocusColumnLeft, true), + bind(Keysym::Right, Action::FocusColumnRight, true), + bind(Keysym::Up, Action::FocusWindowOrWorkspaceUp, true), + bind(Keysym::Down, Action::FocusWindowOrWorkspaceDown, true), + ] + }); + + BINDS.iter() +} + /// Returns an iterator over bindings. /// /// Includes dynamically populated bindings like the MRU UI. fn make_binds_iter<'a>( config: &'a Config, + screenshot_ui: &'a ScreenshotUi, mru: &'a mut WindowMruUi, + overview_focused: bool, mods: Modifiers, ) -> impl Iterator + Clone { + let general_binds = config.binds.0.iter(); + // Figure out the binds to use depending on whether the MRU is enabled and/or open. - let general_binds = (!mru.is_open()).then_some(config.binds.0.iter()); + let general_binds = (!mru.is_open()).then_some(general_binds); let general_binds = general_binds.into_iter().flatten(); let mru_binds = @@ -5974,77 +6312,162 @@ fn make_binds_iter<'a>( let mru_open_binds = mru.is_open().then(|| mru.opened_bindings(mods)); let mru_open_binds = mru_open_binds.into_iter().flatten(); - // General binds take precedence over the MRU binds. - general_binds.chain(mru_binds).chain(mru_open_binds) + // Screenshot UI binds already disable themselves when it's closed. + let screenshot_binds = screenshot_ui.binds(); + + let overview_binds = overview_focused + .then(hardcoded_overview_binds) + .into_iter() + .flatten(); + + // The precedence is screenshot UI, then general, then MRU binds. + screenshot_binds + .chain(general_binds) + .chain(mru_binds) + .chain(mru_open_binds) + .chain(overview_binds) } #[cfg(test)] mod tests { use super::*; + const CLOSE_KEYSYM: Keysym = Keysym::q; + const CLOSE_KEY_CODE: Keycode = Keycode::new(CLOSE_KEYSYM.raw()); + struct TestState { + disable_power_key_handling: bool, + is_inhibiting: bool, + is_locked: bool, + is_in_screenshot: bool, + suppressed_keys: HashSet, + pending_release_binds: HashMap, + cooldown_timers: HashMap, + } + + fn create_test_state() -> TestState { + TestState { + disable_power_key_handling: false, + is_inhibiting: false, + is_locked: false, + is_in_screenshot: false, + suppressed_keys: HashSet::new(), + pending_release_binds: HashMap::new(), + cooldown_timers: HashMap::new(), + } + } + + /// Returns a bind with the given trigger, modifiers, and action, and default flags. + fn bind(trigger: Trigger, modifiers: Modifiers, action: BoundAction) -> Bind { + Bind { + key: Key { trigger, modifiers }, + action, + repeat: false, + pass_through: None, + cooldown: None, + allow_when_locked: false, + allow_inhibiting: true, + hotkey_overlay_title: None, + } + } + + /// Returns a `Mod+Q` bind with the given action and default flags. + fn close_bind(action: BoundAction) -> Bind { + bind(Trigger::Keysym(CLOSE_KEYSYM), Modifiers::COMPOSITOR, action) + } + + /// Processes a press of the `Mod+Q` bind that the tests are written with. + fn press_close_key( + state: &mut TestState, + bindings: &Binds, + mods: Modifiers, + ) -> ShouldInterceptResult { + process_close_key(state, bindings, mods, KeyState::Pressed) + } + + /// Processes a release of the `Mod+Q` bind that the tests are written with. + fn release_close_key( + state: &mut TestState, + bindings: &Binds, + mods: Modifiers, + ) -> ShouldInterceptResult { + process_close_key(state, bindings, mods, KeyState::Released) + } + + fn process_close_key( + state: &mut TestState, + bindings: &Binds, + mods: Modifiers, + key_state: KeyState, + ) -> ShouldInterceptResult { + let pressed = key_state == KeyState::Pressed; + + if pressed { + retain_press_binds(&mut state.pending_release_binds); + } + + should_intercept_key( + &mut state.suppressed_keys, + &mut state.pending_release_binds, + &state.cooldown_timers, + bindings.0.iter(), + ModKey::Super, + CLOSE_KEY_CODE, + Some(CLOSE_KEYSYM), + &[], + pressed, + mods, + state.disable_power_key_handling, + state.is_inhibiting, + state.is_locked, + state.is_in_screenshot, + ) + } + + macro_rules! assert_matches { + ($expression:expr, $pattern:pat) => { + let value = $expression; + assert!( + matches!(value, $pattern), + "Expected {:?} to match {}", + value, + stringify!($pattern) + ); + }; + } + #[test] fn comp_mod_handling() { let bindings = Binds(vec![ - Bind { - key: Key { - trigger: Trigger::Keysym(Keysym::q), - modifiers: Modifiers::COMPOSITOR, - }, - actions: vec![Action::CloseWindow], - repeat: true, - cooldown: None, - allow_when_locked: false, - allow_inhibiting: true, - hotkey_overlay_title: None, - }, - Bind { - key: Key { - trigger: Trigger::Keysym(Keysym::h), - modifiers: Modifiers::SUPER, - }, - actions: vec![Action::FocusColumnLeft], - repeat: true, - cooldown: None, - allow_when_locked: false, - allow_inhibiting: true, - hotkey_overlay_title: None, - }, - Bind { - key: Key { - trigger: Trigger::Keysym(Keysym::j), - modifiers: Modifiers::empty(), - }, - actions: vec![Action::FocusWindowDown], - repeat: true, - cooldown: None, - allow_when_locked: false, - allow_inhibiting: true, - hotkey_overlay_title: None, - }, - Bind { - key: Key { - trigger: Trigger::Keysym(Keysym::k), - modifiers: Modifiers::COMPOSITOR | Modifiers::SUPER, - }, - actions: vec![Action::FocusWindowUp], - repeat: true, - cooldown: None, - allow_when_locked: false, - allow_inhibiting: true, - hotkey_overlay_title: None, - }, - Bind { - key: Key { - trigger: Trigger::Keysym(Keysym::l), - modifiers: Modifiers::SUPER | Modifiers::ALT, - }, - actions: vec![Action::FocusColumnRight], - repeat: true, - cooldown: None, - allow_when_locked: false, - allow_inhibiting: true, - hotkey_overlay_title: None, - }, + bind( + Trigger::Keysym(Keysym::q), + Modifiers::COMPOSITOR, + BoundAction::Press(vec![Action::CloseWindow]), + ), + bind( + Trigger::Keysym(Keysym::h), + Modifiers::SUPER, + BoundAction::Press(vec![Action::FocusColumnLeft]), + ), + bind( + Trigger::Keysym(Keysym::j), + Modifiers::empty(), + BoundAction::Press(vec![Action::FocusWindowDown]), + ), + bind( + Trigger::Keysym(Keysym::k), + Modifiers::COMPOSITOR | Modifiers::SUPER, + BoundAction::Press(vec![Action::FocusWindowUp]), + ), + bind( + Trigger::Keysym(Keysym::l), + Modifiers::SUPER | Modifiers::ALT, + BoundAction::Press(vec![Action::FocusColumnRight]), + ), + bind( + Trigger::Keysym(Keysym::Super_L), + Modifiers::empty(), + BoundAction::Release(vec![Action::ToggleOverview]), + ), ]); assert_eq!( @@ -6055,7 +6478,7 @@ mod tests { ModifiersState { logo: true, ..Default::default() - } + }, ) .as_ref(), Some(&bindings.0[0]) @@ -6078,7 +6501,7 @@ mod tests { ModifiersState { logo: true, ..Default::default() - } + }, ) .as_ref(), Some(&bindings.0[1]) @@ -6101,7 +6524,7 @@ mod tests { ModifiersState { logo: true, ..Default::default() - } + }, ), None, ); @@ -6124,7 +6547,7 @@ mod tests { ModifiersState { logo: true, ..Default::default() - } + }, ) .as_ref(), Some(&bindings.0[3]) @@ -6148,7 +6571,7 @@ mod tests { logo: true, alt: true, ..Default::default() - } + }, ) .as_ref(), Some(&bindings.0[4]) @@ -6165,5 +6588,270 @@ mod tests { ), None, ); + + // A release-only bind is found the same as any other: what to do with it depends on the + // event, not on the lookup. + assert_eq!( + find_configured_bind( + &bindings.0, + ModKey::Super, + Trigger::Keysym(Keysym::Super_L), + ModifiersState::default(), + ) + .as_ref(), + Some(&bindings.0[5]) + ); + } + + #[test] + fn bind_allowed_when_locked_requires_both_actions_of_a_both_bind() { + // A press could start something that its release would never undo, so both actions must be + // allowed while locked. + let both = |press, release| { + close_bind(BoundAction::Both { + press: vec![press], + release: vec![release], + }) + }; + assert!(!bind_allowed_when_locked( + &both(Action::Suspend, Action::CloseWindow), + true + )); + assert!(!bind_allowed_when_locked( + &both(Action::CloseWindow, Action::Suspend), + true + )); + assert!(bind_allowed_when_locked( + &both(Action::Suspend, Action::PowerOffMonitors(false)), + true + )); + } + + #[test] + fn bind_allowed_when_locked_requires_only_the_single_action() { + assert!(bind_allowed_when_locked( + &close_bind(BoundAction::Press(vec![Action::Suspend])), + true + )); + assert!(!bind_allowed_when_locked( + &close_bind(BoundAction::Press(vec![Action::CloseWindow])), + true + )); + assert!(bind_allowed_when_locked( + &close_bind(BoundAction::Release(vec![Action::Suspend])), + false + )); + assert!(!bind_allowed_when_locked( + &close_bind(BoundAction::Release(vec![Action::CloseWindow])), + false + )); + } + + #[test] + fn bind_allowed_when_locked_always_allows_a_both_bind_release() { + // The release only happens if its press ran, and the press required both actions to be + // allowed, so the release is never blocked separately. + let both = close_bind(BoundAction::Both { + press: vec![Action::Suspend], + release: vec![Action::CloseWindow], + }); + assert!(bind_allowed_when_locked(&both, false)); + } + + #[test] + fn allow_when_locked_allows_the_whole_bind() { + let bind = |action| Bind { + allow_when_locked: true, + ..close_bind(action) + }; + assert!(bind_allowed_when_locked( + &bind(BoundAction::Press(vec![Action::CloseWindow])), + true + )); + assert!(bind_allowed_when_locked( + &bind(BoundAction::Release(vec![Action::CloseWindow])), + false + )); + + let both = bind(BoundAction::Both { + press: vec![Action::CloseWindow], + release: vec![Action::CenterColumn], + }); + assert!(bind_allowed_when_locked(&both, true)); + assert!(bind_allowed_when_locked(&both, false)); + } + + #[test] + fn locked_both_bind_does_not_run_its_press_when_its_release_is_not_allowed() { + // `Mod+Q { press { suspend; } release { close-window; } }`: the press is allowed while + // locked but the release isn't, so the press must not run, since nothing could undo it. + let bindings = Binds(vec![close_bind(BoundAction::Both { + press: vec![Action::Suspend], + release: vec![Action::CloseWindow], + })]); + + let mut state = TestState { + is_locked: true, + ..create_test_state() + }; + let result = press_close_key(&mut state, &bindings, Modifiers::SUPER); + + // `handle_bind()` drops the press while locked, and nothing is recorded to undo. + assert_matches!(result, ShouldInterceptResult::InterceptAndHandle(_)); + assert!(state.pending_release_binds.is_empty()); + + let result = release_close_key(&mut state, &bindings, Modifiers::SUPER); + assert_matches!(result, ShouldInterceptResult::InterceptOnly); + } + + #[test] + fn both_bind_release_recorded_while_unlocked_runs_while_locked() { + // Its press action ran while unlocked, so the release must run later even if the screen got + // locked in between. + let bindings = Binds(vec![close_bind(BoundAction::Both { + press: vec![Action::Suspend], + release: vec![Action::CloseWindow], + })]); + + let mut state = create_test_state(); + let result = press_close_key(&mut state, &bindings, Modifiers::SUPER); + assert_matches!(result, ShouldInterceptResult::InterceptAndHandle(_)); + assert!(state.pending_release_binds.contains_key(&CLOSE_KEY_CODE)); + + state.is_locked = true; + let result = release_close_key(&mut state, &bindings, Modifiers::SUPER); + let ShouldInterceptResult::InterceptAndHandle(bind) = &result else { + panic!("unexpected result: {result:?}"); + }; + assert!(bind.has_press()); + assert_eq!(bind.release_actions(), [Action::CloseWindow]); + assert!(state.pending_release_binds.is_empty()); + } + + #[test] + fn release_only_bind_is_recorded_on_press_and_runs_on_release() { + let bindings = Binds(vec![close_bind(BoundAction::Release(vec![ + Action::CloseWindow, + ]))]); + + let mut state = create_test_state(); + let result = press_close_key(&mut state, &bindings, Modifiers::SUPER); + assert_matches!(result, ShouldInterceptResult::Forward); + assert!(state.pending_release_binds.contains_key(&CLOSE_KEY_CODE)); + + let result = release_close_key(&mut state, &bindings, Modifiers::SUPER); + let ShouldInterceptResult::ForwardAndHandle(bind) = &result else { + panic!("unexpected result: {result:?}"); + }; + assert_eq!(bind.action, BoundAction::Release(vec![Action::CloseWindow])); + assert!(state.pending_release_binds.is_empty()); + } + + #[test] + fn release_only_bind_allowed_while_locked_is_recorded_and_runs_while_locked() { + let bindings = Binds(vec![Bind { + allow_when_locked: true, + ..close_bind(BoundAction::Release(vec![Action::CloseWindow])) + }]); + + let mut state = TestState { + is_locked: true, + ..create_test_state() + }; + let result = press_close_key(&mut state, &bindings, Modifiers::SUPER); + assert_matches!(result, ShouldInterceptResult::Forward); + assert!(state.pending_release_binds.contains_key(&CLOSE_KEY_CODE)); + + let result = release_close_key(&mut state, &bindings, Modifiers::SUPER); + let ShouldInterceptResult::ForwardAndHandle(bind) = &result else { + panic!("unexpected result: {result:?}"); + }; + assert_eq!(bind.action, BoundAction::Release(vec![Action::CloseWindow])); + assert!(state.pending_release_binds.is_empty()); + } + + #[test] + fn release_only_bind_not_allowed_while_locked_is_not_recorded_while_locked() { + let bindings = Binds(vec![close_bind(BoundAction::Release(vec![ + Action::CloseWindow, + ]))]); + + let mut state = TestState { + is_locked: true, + ..create_test_state() + }; + let result = press_close_key(&mut state, &bindings, Modifiers::SUPER); + assert_matches!(result, ShouldInterceptResult::Forward); + assert!(state.pending_release_binds.is_empty()); + + let result = release_close_key(&mut state, &bindings, Modifiers::SUPER); + assert_matches!(result, ShouldInterceptResult::Forward); + assert!(state.pending_release_binds.is_empty()); + } + + #[test] + fn bind_allowed_during_screenshot_requires_both_actions_of_a_both_bind() { + // A press could start something that its release would never undo, so both actions must be + // allowed while the screenshot UI is open. + let both = |press, release| { + close_bind(BoundAction::Both { + press: vec![press], + release: vec![release], + }) + }; + assert!(!bind_allowed_during_screenshot( + &both(Action::MoveColumnLeft, Action::CloseWindow), + true + )); + assert!(!bind_allowed_during_screenshot( + &both(Action::CloseWindow, Action::MoveColumnLeft), + true + )); + assert!(bind_allowed_during_screenshot( + &both(Action::MoveColumnLeft, Action::MoveWindowUp), + true + )); + } + + #[test] + fn bind_allowed_during_screenshot_requires_only_the_single_action() { + assert!(bind_allowed_during_screenshot( + &close_bind(BoundAction::Press(vec![Action::MoveColumnLeft])), + true + )); + assert!(!bind_allowed_during_screenshot( + &close_bind(BoundAction::Press(vec![Action::CloseWindow])), + true + )); + } + + #[test] + fn bind_allowed_during_screenshot_always_allows_a_both_bind_release() { + // The release only happens if its press ran, so the screenshot UI opening in between must + // not block it. + let both = |press, release| { + close_bind(BoundAction::Both { + press: vec![press], + release: vec![release], + }) + }; + assert!(bind_allowed_during_screenshot( + &both(Action::MoveColumnLeft, Action::CloseWindow), + false + )); + assert!(bind_allowed_during_screenshot( + &both(Action::CloseWindow, Action::MoveColumnLeft), + false + )); + + // A release-only bind has no press to undo, so its own action must be allowed. + assert!(bind_allowed_during_screenshot( + &close_bind(BoundAction::Release(vec![Action::MoveColumnLeft])), + false + )); + assert!(!bind_allowed_during_screenshot( + &close_bind(BoundAction::Release(vec![Action::CloseWindow])), + false + )); } } diff --git a/src/niri.rs b/src/niri.rs index 94d6730bbc..47867b561f 100644 --- a/src/niri.rs +++ b/src/niri.rs @@ -14,10 +14,11 @@ use _server_decoration::server::org_kde_kwin_server_decoration_manager::Mode as use anyhow::{bail, ensure, Context}; use calloop::futures::Scheduler; use niri_config::debug::PreviewRender; +use niri_config::input::XkbFile; use niri_config::output::MaxBpc; use niri_config::window_rule::VrrRule; use niri_config::{ - Config, FloatOrInt, Key, Modifiers, OutputName, TrackLayout, WarpMouseToFocusMode, + Bind, Config, FloatOrInt, Key, Modifiers, OutputName, TrackLayout, WarpMouseToFocusMode, WorkspaceReference, Xkb, }; use smithay::backend::allocator::Fourcc; @@ -392,6 +393,12 @@ pub struct Niri { pub suppressed_keys: HashSet, /// Button codes of the mouse buttons to suppress. pub suppressed_buttons: HashSet, + /// Binds whose release action is pending, keyed by the key code. + pub pending_release_binds: HashMap, + /// Same as `pending_release_binds`, but for mouse buttons, keyed by the button code. + pub pending_mouse_release_binds: HashMap, + /// Same as `pending_release_binds`, but for tablet tool buttons, keyed by the button code. + pub pending_tablet_release_binds: HashMap, pub bind_cooldown_timers: HashMap, pub bind_repeat_timer: Option, pub keyboard_focus: KeyboardFocus, @@ -1896,14 +1903,7 @@ impl State { } /// Loads the xkb keymap from a file config setting. - pub(crate) fn set_xkb_file(&mut self, xkb_file: String) -> anyhow::Result<()> { - let xkb_file = PathBuf::from(xkb_file); - let xkb_file = expand_home(&xkb_file) - .context("failed to expand ~")? - .unwrap_or(xkb_file); - - let keymap = std::fs::read_to_string(xkb_file).context("failed to read xkb_file")?; - + pub(crate) fn set_xkb_file_content(&mut self, keymap: String) -> anyhow::Result<()> { let keyboard = self.niri.seat.get_keyboard().unwrap(); let restore = self.keyboard_state_to_restore(); @@ -1927,8 +1927,8 @@ impl State { .xkb .file .clone(); - if let Some(xkb_file) = xkb_file { - if let Err(err) = self.set_xkb_file(xkb_file) { + if let Some(XkbFile(Some(keymap))) = xkb_file { + if let Err(err) = self.set_xkb_file_content(keymap) { warn!("error loading xkb_file: {err:?}"); } } @@ -2035,8 +2035,8 @@ impl State { let mut xkb = self.niri.current_keyboard.xkb.clone(); let mut set_xkb_config = true; - if let Some(xkb_file) = xkb.file.take() { - if let Err(err) = self.set_xkb_file(xkb_file) { + if let Some(XkbFile(Some(keymap))) = xkb.file.take() { + if let Err(err) = self.set_xkb_file_content(keymap) { warn!("error loading xkb_file: {err:?}"); } else { set_xkb_config = false; @@ -2123,6 +2123,11 @@ impl State { self.niri.cursor_texture_cache.clear(); } + if config.debug.nested_show_host_cursor != old_config.debug.nested_show_host_cursor { + self.backend + .set_host_cursor_visible(config.debug.nested_show_host_cursor); + } + // Reset to the fallback configuration when any keyboard block changes. The next // physical key event resolves its device configuration again. Reload the actual keymap // as well as the tracked configuration, including when only a named block changed. @@ -2164,10 +2169,10 @@ impl State { let binds_changed = config.binds != old_config.binds; let new_mod_key = self.backend.mod_key(&config); + let mut hotkey_overlay_changed = + config.hotkey_overlay.hide_not_bound != old_config.hotkey_overlay.hide_not_bound; if new_mod_key != self.backend.mod_key(&old_config) || binds_changed { - self.niri - .hotkey_overlay - .on_hotkey_config_updated(new_mod_key); + hotkey_overlay_changed = true; self.niri.mods_with_mouse_binds = mods_with_mouse_binds(new_mod_key, &config.binds); self.niri.mods_with_horizontal_wheel_binds = mods_with_horizontal_wheel_binds(new_mod_key, &config.binds); @@ -2178,6 +2183,11 @@ impl State { self.niri.mods_with_finger_scroll_binds = mods_with_finger_scroll_binds(new_mod_key, &config.binds); } + if hotkey_overlay_changed { + self.niri + .hotkey_overlay + .on_hotkey_config_updated(new_mod_key); + } if config.window_rules != old_config.window_rules { window_rules_changed = true; @@ -3318,6 +3328,9 @@ impl Niri { popup_grab: None, suppressed_keys: HashSet::new(), suppressed_buttons: HashSet::new(), + pending_release_binds: HashMap::new(), + pending_mouse_release_binds: HashMap::new(), + pending_tablet_release_binds: HashMap::new(), bind_cooldown_timers: HashMap::new(), bind_repeat_timer: Option::default(), presentation_state, @@ -8231,6 +8244,19 @@ impl Niri { Ok(()) } + pub fn clear_input_state(&mut self) { + self.suppressed_keys.clear(); + self.suppressed_buttons.clear(); + self.pending_release_binds.clear(); + self.pending_mouse_release_binds.clear(); + self.pending_tablet_release_binds.clear(); + + // Stop ongoing key repeat if any. + if let Some(token) = self.bind_repeat_timer.take() { + self.event_loop.remove(token); + } + } + pub fn is_locked(&self) -> bool { match self.lock_state { LockState::Unlocked | LockState::WaitingForSurfaces { .. } => false, diff --git a/src/render_helpers/framebuffer_effect.rs b/src/render_helpers/framebuffer_effect.rs index 0cf3c31ddb..591fe8c174 100644 --- a/src/render_helpers/framebuffer_effect.rs +++ b/src/render_helpers/framebuffer_effect.rs @@ -1,11 +1,12 @@ use std::cell::RefCell; +use std::sync::Once; use glam::{Mat3, Vec2}; use niri_config::CornerRadius; use smithay::backend::allocator::Fourcc; use smithay::backend::renderer::element::{Element, Id, RenderElement}; use smithay::backend::renderer::gles::{ - ffi, GlesError, GlesFrame, GlesRenderer, GlesTexture, Uniform, + ffi, Capability, GlesError, GlesFrame, GlesRenderer, GlesTexture, Uniform, }; use smithay::backend::renderer::utils::CommitCounter; use smithay::backend::renderer::{Frame as _, FrameContext, Offscreen, Texture as _}; @@ -189,6 +190,7 @@ impl RenderElement for FramebufferEffectElement { let output_rect = Rectangle::from_size(frame.output_size()); let transform = frame.transformation(); + let supports_blit = frame.capabilities().contains(&Capability::Blit); let mut guard = frame.renderer(); let inner = cache @@ -198,6 +200,14 @@ impl RenderElement for FramebufferEffectElement { inner.intermediate = None; + if !supports_blit { + static WARN: Once = Once::new(); + WARN.call_once(|| { + warn!("non-xray background effects disabled: renderer does not support framebuffer blitting"); + }); + return Ok(()); + } + // We want clamp-to-edge behavior for out-of-bounds pixels. However, glBlitFramebuffer // seems to skip out-of-bounds pixels, even though my reading of the docs suggests // otherwise (we use GL_LINEAR filter). So, clamp dst to the framebuffer bounds diff --git a/src/tests/binds.rs b/src/tests/binds.rs index 929daf0638..4e703ababd 100644 --- a/src/tests/binds.rs +++ b/src/tests/binds.rs @@ -1,33 +1,90 @@ use std::fmt::Write as _; use insta::assert_snapshot; -use niri_config::{Action, Config}; +use niri_config::{Action, BoundAction, Config}; use smithay::backend::input::{InputEvent, InputTime, KeyState, Keycode}; use smithay::input::keyboard::xkb::Keymap; +use wayland_client::protocol::wl_pointer; use wayland_client::protocol::wl_surface::WlSurface; use crate::tests::client::ClientId; use crate::tests::fixture::Fixture; use crate::tests::test_input_backend::{TestInputBackend, TestKeyboardKeyEvent}; +#[derive(Clone, Copy)] enum Op { Press(Keycode), Release(Keycode), + MouseButton { + name: &'static str, + code: u32, + state: wl_pointer::ButtonState, + }, + /// A scroll tick. Scrolls are momentary events, so they have no press or release. + Scroll { + name: &'static str, + source: wl_pointer::AxisSource, + /// Positive scrolls down. + amount: f64, + }, } +/// Mouse buttons that can be pressed and released, named after the binds that trigger them. +const MOUSE_BUTTONS: &[(&str, u32)] = &[ + ("MouseLeft", 0x110), + ("MouseRight", 0x111), + ("MouseMiddle", 0x112), + ("MouseBack", 0x113), + ("MouseForward", 0x114), +]; + +/// Scrolls, named after the binds that trigger them. +const SCROLLS: &[(&str, wl_pointer::AxisSource, f64)] = &[ + ("WheelScrollDown", wl_pointer::AxisSource::Wheel, 10.), + ("WheelScrollUp", wl_pointer::AxisSource::Wheel, -10.), +]; + fn parse(keymap: &Keymap, input: &str) -> Vec { let mut ops = Vec::new(); for part in input.split_ascii_whitespace() { - let name = &part[1..]; + // Scrolls are momentary, so they are written without a prefix. + let (pressed, name) = match part.as_bytes().first() { + Some(b'+') => (Some(true), &part[1..]), + Some(b'-') => (Some(false), &part[1..]), + _ => (None, part), + }; + + if let Some((name, source, amount)) = SCROLLS.iter().copied().find(|(n, _, _)| *n == name) { + assert!( + pressed.is_none(), + "scrolls have no press or release: {part}" + ); + ops.push(Op::Scroll { + name, + source, + amount, + }); + continue; + } + + if let Some((name, code)) = MOUSE_BUTTONS.iter().copied().find(|(n, _)| *n == name) { + let state = match pressed { + Some(true) => wl_pointer::ButtonState::Pressed, + Some(false) => wl_pointer::ButtonState::Released, + None => panic!("a mouse button needs a + or - prefix: {part}"), + }; + ops.push(Op::MouseButton { name, code, state }); + continue; + } + let Some(key) = keymap.key_by_name(name) else { panic!("unknown key {name}"); }; - let c = part.bytes().next().unwrap(); - let op = match c { - b'+' => Op::Press(key), - b'-' => Op::Release(key), - _ => panic!("keys must begin with + or -, got {c}"), + let op = match pressed { + Some(true) => Op::Press(key), + Some(false) => Op::Release(key), + None => panic!("a key needs a + or - prefix: {part}"), }; ops.push(op); @@ -37,9 +94,19 @@ fn parse(keymap: &Keymap, input: &str) -> Vec { fn set_up(config: &str) -> (Fixture, ClientId, WlSurface) { let mut config = Config::parse_mem(config).unwrap(); - // knuffel doesn't understand #[cfg(test)]... + for bind in &mut config.binds.0 { - bind.actions = vec![Action::TestAction]; + let actions = match &mut bind.action { + BoundAction::Press(press) => [Some(press), None], + BoundAction::Release(release) => [None, Some(release)], + BoundAction::Both { press, release } => [Some(press), Some(release)], + }; + for action in actions.into_iter().flatten().flatten() { + // knuffel doesn't understand #[cfg(test)]... + if !matches!(action, Action::Screenshot(_, _)) { + *action = Action::TestAction; + } + } } let mut f = Fixture::with_config(config); @@ -71,48 +138,86 @@ fn run_f(f: &mut Fixture, id: ClientId, surface: &WlSurface, input: &str) -> Str }); let mut rv = String::new(); + for event in f.client(id).state.recent_keyboard_events(surface) { + let _ = writeln!(&mut rv, " surface {event}"); + } for op in ops { - let (code, key_state) = match op { - Op::Press(code) => (code, KeyState::Pressed), - Op::Release(code) => (code, KeyState::Released), - }; + let prev = f.niri().test_action_count; - let state = f.niri_state(); - let keyboard = state.niri.seat.get_keyboard().unwrap(); - keyboard.with_xkb_state(state, |xkb| { - let xkb = xkb.xkb().lock().unwrap(); - let xkb_state = unsafe { xkb.state() }; - let keymap = xkb_state.get_keymap(); + match op { + Op::Press(code) | Op::Release(code) => { + let key_state = match op { + Op::Press(_) => KeyState::Pressed, + Op::Release(_) => KeyState::Released, + _ => unreachable!(), + }; - let c = match key_state { - KeyState::Pressed => "+", - KeyState::Released => "-", - }; + let state = f.niri_state(); + let keyboard = state.niri.seat.get_keyboard().unwrap(); + keyboard.with_xkb_state(state, |xkb| { + let xkb = xkb.xkb().lock().unwrap(); + let xkb_state = unsafe { xkb.state() }; + let keymap = xkb_state.get_keymap(); - let name = keymap.key_get_name(code).unwrap_or("None"); - let keysym = xkb_state.key_get_one_sym(code); + let c = match key_state { + KeyState::Pressed => "+", + KeyState::Released => "-", + }; - let _ = writeln!(&mut rv, "{c}{name} {:>3} {keysym:?}", code.raw()); - }); + let name = keymap.key_get_name(code).unwrap_or("None"); + let keysym = xkb_state.key_get_one_sym(code); - let prev = state.niri.test_action_count; + let _ = writeln!(&mut rv, "{c}{name} {:>3} {keysym:?}", code.raw()); + }); - state.process_input_event(InputEvent::::Keyboard { - event: TestKeyboardKeyEvent { - time: InputTime::from_micros(0), + let state = f.niri_state(); + state.process_input_event(InputEvent::::Keyboard { + event: TestKeyboardKeyEvent { + time: InputTime::from_micros(0), + code, + state: key_state, + count: 1, // niri doesn't use this + }, + }); + } + Op::MouseButton { + name, code, - state: key_state, - count: 1, // niri doesn't use this - }, - }); + state: button_state, + } => { + let c = match button_state { + wl_pointer::ButtonState::Pressed => "+", + wl_pointer::ButtonState::Released => "-", + _ => unreachable!(), + }; + let _ = writeln!(&mut rv, "{c}{name}"); + + let pointer = f.client(id).state.virtual_pointer(); + pointer.button(0, code, button_state); + pointer.frame(); + } + Op::Scroll { + name, + source, + amount, + } => { + let _ = writeln!(&mut rv, "{name}"); + + let pointer = f.client(id).state.virtual_pointer(); + pointer.axis_source(source); + pointer.axis(0, wl_pointer::Axis::VerticalScroll, amount); + pointer.frame(); + } + } + + f.roundtrip(id); let diff = f.niri().test_action_count - prev; for _ in 0..diff { let _ = writeln!(&mut rv, " niri test-action"); } - f.roundtrip(id); for event in f.client(id).state.recent_keyboard_events(surface) { let _ = writeln!(&mut rv, " surface {event}"); } @@ -422,6 +527,9 @@ fn layouts() { binds { Q { close-window; } Shift+Slash { close-window; } + Cyrillic_ya { close-window; } + Shift+Cyrillic_che { close-window; } + Shift+Cyrillic_ES { close-window; } } "#; @@ -478,9 +586,8 @@ fn layouts() { " ); - // In ru, / is on the \ / key (so, Shift + \). So, arguably, it would make sense for Shift + / - // to trigger it, but it currently doesn't (niri requires an unshifted trigger key, despite - // working fine with capital case alphabetic keys). + // In ru, / is on the \ / key (so, Shift + \). So, Shift + \ inputs / and triggers + // the Shift + / bind. assert_snapshot!( run(c, "+LALT -LALT +LFSH +BKSL -BKSL -LFSH"), @" @@ -493,12 +600,1861 @@ fn layouts() { surface key pressed: 42 surface modifiers: depressed=1, latched=0, locked=0, group=1 +BKSL 51 XK_slash - surface key pressed: 43 + niri test-action -BKSL 51 XK_slash - surface key released: 43 -LFSH 50 XK_Shift_L surface key released: 42 surface modifiers: depressed=0, latched=0, locked=0, group=1 " ); + + // Я should trigger the Cyrillic_ya bind. (Nobody binds keys like this in Russia, + // but people do in other countries with their non-ASCII characters.) + assert_snapshot!( + run(c, "+LALT -LALT +LatZ -LatZ"), + @" + +LALT 64 XK_ISO_Next_Group + surface key pressed: 56 + surface modifiers: depressed=0, latched=0, locked=0, group=1 + -LALT 64 XK_ISO_Next_Group + surface key released: 56 + +AB01 52 XK_Cyrillic_ya + niri test-action + -AB01 52 XK_Cyrillic_ya + " + ); + + // Non-ASCII binds should also be case-insensitive where reasonable, just like ASCII ones. Or, + // more accurately, the trigger key for a Modifier+key bind should be Modifier-insensitive. + assert_snapshot!( + run(c, "+LALT -LALT +LFSH +LatX -LatX -LFSH"), + @" + +LALT 64 XK_ISO_Next_Group + surface key pressed: 56 + surface modifiers: depressed=0, latched=0, locked=0, group=1 + -LALT 64 XK_ISO_Next_Group + surface key released: 56 + +LFSH 50 XK_Shift_L + surface key pressed: 42 + surface modifiers: depressed=1, latched=0, locked=0, group=1 + +AB02 53 XK_Cyrillic_CHE + niri test-action + -AB02 53 XK_Cyrillic_CHE + -LFSH 50 XK_Shift_L + surface key released: 42 + surface modifiers: depressed=0, latched=0, locked=0, group=1 + " + ); + assert_snapshot!( + run(c, "+LALT -LALT +LFSH +LatC -LatC -LFSH"), + @" + +LALT 64 XK_ISO_Next_Group + surface key pressed: 56 + surface modifiers: depressed=0, latched=0, locked=0, group=1 + -LALT 64 XK_ISO_Next_Group + surface key released: 56 + +LFSH 50 XK_Shift_L + surface key pressed: 42 + surface modifiers: depressed=1, latched=0, locked=0, group=1 + +AB03 54 XK_Cyrillic_ES + niri test-action + -AB03 54 XK_Cyrillic_ES + -LFSH 50 XK_Shift_L + surface key released: 42 + surface modifiers: depressed=0, latched=0, locked=0, group=1 + " + ); + + // ASCII binds on Russian should search the configured ASCII layout, not QWERTY. + // LatR corresponds to P on colemak_dh_ortho. + let c = r#" + input { + keyboard { + xkb { + layout "us,ru" + variant "colemak_dh_ortho," + options "grp:lalt_toggle" + } + } + } + + binds { + P { close-window; } + } + "#; + assert_snapshot!(run(c, "+LALT -LALT +LatR -LatR"), + @" + +LALT 64 XK_ISO_Next_Group + surface key pressed: 56 + surface modifiers: depressed=0, latched=0, locked=0, group=1 + -LALT 64 XK_ISO_Next_Group + surface key released: 56 + +AD04 27 XK_Cyrillic_ka + niri test-action + -AD04 27 XK_Cyrillic_ka + "); + + // ASCII fallback should find the first configured ASCII layout. + let c = r#" + input { + keyboard { + xkb { + layout "ru,us,us" + variant ",colemak_dh_ortho," + options "grp:lalt_toggle" + } + } + } + + binds { + P { close-window; } + } + "#; + // Input in ru (should fall back to us-colemak), + // then input in us-colemak (no fallback, works), + // then input in us-qwerty (no fallback, no bind). + assert_snapshot!(run(c, "+LatR -LatR +LALT -LALT +LatR -LatR +LALT -LALT +LatR -LatR"), + @" + +AD04 27 XK_Cyrillic_ka + niri test-action + -AD04 27 XK_Cyrillic_ka + +LALT 64 XK_ISO_Next_Group + surface key pressed: 56 + surface modifiers: depressed=0, latched=0, locked=0, group=1 + -LALT 64 XK_ISO_Next_Group + surface key released: 56 + +AD04 27 XK_p + niri test-action + -AD04 27 XK_p + +LALT 64 XK_ISO_Next_Group + surface key pressed: 56 + surface modifiers: depressed=0, latched=0, locked=0, group=2 + -LALT 64 XK_ISO_Next_Group + surface key released: 56 + +AD04 27 XK_r + surface key pressed: 19 + -AD04 27 XK_r + surface key released: 19 + "); + + // ASCII fallback should include non-alphabetic characters so more binds keep working on us,ru. + let c = r#" + input { + keyboard { + xkb { + layout "ru,us" + } + } + } + + binds { + Comma { close-window; } + BracketLeft { close-window; } + } + "#; + assert_snapshot!(run(c, "+AD11 -AD11 +AB08 -AB08"), + @" + +AD11 34 XK_Cyrillic_ha + niri test-action + -AD11 34 XK_Cyrillic_ha + +AB08 59 XK_Cyrillic_be + niri test-action + -AB08 59 XK_Cyrillic_be + "); + + // Non-alphabetic ASCII characters shouldn't fallback to other layouts to avoid surprises. + // Q is ' on US Dvorak. + let c = r#" + input { + keyboard { + xkb { + layout "us,us" + variant "dvorak," + options "grp:lalt_toggle" + } + } + } + + binds { + Q { close-window; } + } + "#; + assert_snapshot!(run(c, "+LatQ -LatQ +LALT +LALT +LatQ -LatQ"), + @" + +AD01 24 XK_apostrophe + surface key pressed: 16 + -AD01 24 XK_apostrophe + surface key released: 16 + +LALT 64 XK_ISO_Next_Group + surface key pressed: 56 + surface modifiers: depressed=0, latched=0, locked=0, group=1 + +LALT 64 XK_ISO_Next_Group + +AD01 24 XK_q + niri test-action + -AD01 24 XK_q + "); + + // ASCII fallback shouldn't depend on holding modifiers like Shift so that binds with these + // modifiers can reliably be fallen back to. + // + // On il, the ; : key is hebrew_finalpe :, meaning that ; is not ASCII but Shift+; is. + // We don't want the fallback to differ in this case. + let c = r#" + input { + keyboard { + xkb { + layout "il,us" + } + } + } + + binds { + Semicolon { close-window; } + Shift+Semicolon { close-window; } + } + "#; + assert_snapshot!(run(c, "+AC10 -AC10 +LFSH +AC10 -AC10"), + @" + +AC10 47 XK_hebrew_finalpe + niri test-action + -AC10 47 XK_hebrew_finalpe + +LFSH 50 XK_Shift_L + surface key pressed: 42 + surface modifiers: depressed=1, latched=0, locked=0, group=0 + +AC10 47 XK_colon + niri test-action + -AC10 47 XK_colon + "); + + // ASCII fallback should work on dead keys too. + // + // Greek has a dead key on ;. + let c = r#" + input { + keyboard { + xkb { + layout "gr,us" + } + } + } + + binds { + Semicolon { close-window; } + } + "#; + assert_snapshot!(run(c, "+AC10 -AC10"), + @" + +AC10 47 XK_dead_acute + niri test-action + -AC10 47 XK_dead_acute + "); + + // We require the modifiers to match the bind for it to trigger. + // So Ctrl-? cannot be triggered on us-qwerty since entering ? necessarily requires a Shift. + // If you spell out Shift like Super-Shift-?, it should be triggerable. + let c = r#" + input { + keyboard { + xkb { + layout "us" + } + } + } + + binds { + Ctrl+question { close-window; } + Super+Shift+question { close-window; } + } + "#; + assert_snapshot!(run(c, "+LCTL +AB10 -AB10 +LFSH +AB10 -AB10"), + @" + +LCTL 37 XK_Control_L + surface key pressed: 29 + surface modifiers: depressed=4, latched=0, locked=0, group=0 + +AB10 61 XK_slash + surface key pressed: 53 + -AB10 61 XK_slash + surface key released: 53 + +LFSH 50 XK_Shift_L + surface key pressed: 42 + surface modifiers: depressed=5, latched=0, locked=0, group=0 + +AB10 61 XK_question + surface key pressed: 53 + -AB10 61 XK_question + surface key released: 53 + "); + assert_snapshot!(run(c, "+LWIN +LFSH +AB10 -AB10"), + @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +LFSH 50 XK_Shift_L + surface key pressed: 42 + surface modifiers: depressed=65, latched=0, locked=0, group=0 + +AB10 61 XK_question + niri test-action + -AB10 61 XK_question + "); + + // The BÉPO example from the docs. The key should be bindable by both « and < trigger. + // But you have to spell out Mod5 since niri requires all modifiers be spelled. + let c = r#" + input { + keyboard { + xkb { + layout "fr" + variant "bepo" + } + } + } + + binds { + Super+Mod5+guillemotleft { close-window; } + Ctrl+Mod5+less { close-window; } + } + "#; + assert_snapshot!(run(c, "+LWIN +RALT +AE02"), + @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +RALT 108 XK_ISO_Level3_Shift + surface key pressed: 100 + surface modifiers: depressed=192, latched=0, locked=0, group=0 + +AE02 11 XK_less + niri test-action + "); + assert_snapshot!(run(c, "+LCTL +RALT +AE02"), + @" + +LCTL 37 XK_Control_L + surface key pressed: 29 + surface modifiers: depressed=4, latched=0, locked=0, group=0 + +RALT 108 XK_ISO_Level3_Shift + surface key pressed: 100 + surface modifiers: depressed=132, latched=0, locked=0, group=0 + +AE02 11 XK_less + niri test-action + "); +} + +#[test] +fn release_binds() { + let c = " + binds { + Mod { + release { toggle-overview; } + } + Mod+Q { + release { close-window; } + } + Mod+U { center-column; } + Ctrl+Alt_L { + release { switch-layout \"next\"; } + } + Ctrl+Alt+U { close-window; } + } + "; + + // Releasing Mod by itself runs its release action. + assert_snapshot!(run(c, "+LWIN -LWIN"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + -LWIN 133 XK_Super_L + niri test-action + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + // Pressing another key in between cancels a modifier-only release bind. + assert_snapshot!(run(c, "+LWIN +LatA -LatA -LWIN"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +AC01 38 XK_a + surface key pressed: 30 + -AC01 38 XK_a + surface key released: 30 + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + // Pressing a key that triggers its own bind cancels the pending release bind. + assert_snapshot!(run(c, "+LWIN +LatU -LatU -LWIN"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +AD07 30 XK_u + niri test-action + -AD07 30 XK_u + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + // A key that was already held when the bind was pressed doesn't cancel it: only input in + // between does. + assert_snapshot!(run(c, "+LatA +LWIN -LatA -LWIN"), @" + +AC01 38 XK_a + surface key pressed: 30 + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + -AC01 38 XK_a + surface key released: 30 + -LWIN 133 XK_Super_L + niri test-action + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + // A release-only bind on a regular key. + assert_snapshot!(run(c, "+LWIN +LatQ -LatQ -LWIN"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +AD01 24 XK_q + surface key pressed: 16 + -AD01 24 XK_q + niri test-action + surface key released: 16 + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + // The release action runs even if the modifiers are released before the trigger key. + assert_snapshot!(run(c, "+LWIN +LatQ -LWIN -LatQ"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +AD01 24 XK_q + surface key pressed: 16 + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + -AD01 24 XK_q + niri test-action + surface key released: 16 + "); + + // Another key in between cancels it, no matter the trigger. + assert_snapshot!(run(c, "+LWIN +LatQ +LatA -LatA -LWIN -LatQ"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +AD01 24 XK_q + surface key pressed: 16 + +AC01 38 XK_a + surface key pressed: 30 + -AC01 38 XK_a + surface key released: 30 + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + -AD01 24 XK_q + surface key released: 16 + "); + + // Ctrl+Alt_L is a modifier-only bind: its whole key consists of modifiers, so releasing the + // combo on its own runs its release action. + assert_snapshot!(run(c, "+LCTL +LALT -LALT -LCTL"), @" + +LCTL 37 XK_Control_L + surface key pressed: 29 + surface modifiers: depressed=4, latched=0, locked=0, group=0 + +LALT 64 XK_Alt_L + surface key pressed: 56 + surface modifiers: depressed=12, latched=0, locked=0, group=0 + -LALT 64 XK_Alt_L + niri test-action + surface key released: 56 + surface modifiers: depressed=4, latched=0, locked=0, group=0 + -LCTL 37 XK_Control_L + surface key released: 29 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + // Pressing another key in between cancels it, so using Ctrl+Alt+A as an app shortcut does not + // additionally switch the layout. + assert_snapshot!(run(c, "+LCTL +LALT +LatA -LatA -LALT -LCTL"), @" + +LCTL 37 XK_Control_L + surface key pressed: 29 + surface modifiers: depressed=4, latched=0, locked=0, group=0 + +LALT 64 XK_Alt_L + surface key pressed: 56 + surface modifiers: depressed=12, latched=0, locked=0, group=0 + +AC01 38 XK_a + surface key pressed: 30 + -AC01 38 XK_a + surface key released: 30 + -LALT 64 XK_Alt_L + surface key released: 56 + surface modifiers: depressed=4, latched=0, locked=0, group=0 + -LCTL 37 XK_Control_L + surface key released: 29 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + // Pressing a key that triggers its own bind cancels the Ctrl+Alt_L release bind. + assert_snapshot!(run(c, "+LCTL +LALT +LatU -LatU -LALT -LCTL"), @" + +LCTL 37 XK_Control_L + surface key pressed: 29 + surface modifiers: depressed=4, latched=0, locked=0, group=0 + +LALT 64 XK_Alt_L + surface key pressed: 56 + surface modifiers: depressed=12, latched=0, locked=0, group=0 + +AD07 30 XK_u + niri test-action + -AD07 30 XK_u + -LALT 64 XK_Alt_L + surface key released: 56 + surface modifiers: depressed=4, latched=0, locked=0, group=0 + -LCTL 37 XK_Control_L + surface key released: 29 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); +} + +#[test] +fn press_and_release_binds() { + let c = " + binds { + Mod+Q { + press { close-window; } + release { center-column; } + } + } + "; + + // Both actions run, and the key never reaches the surface. + assert_snapshot!(run(c, "+LWIN +LatQ -LatQ -LWIN"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +AD01 24 XK_q + niri test-action + -AD01 24 XK_q + niri test-action + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + // The release action runs even if the modifiers are released first. + assert_snapshot!(run(c, "+LWIN +LatQ -LWIN -LatQ"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +AD01 24 XK_q + niri test-action + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + -AD01 24 XK_q + niri test-action + "); + + // Extra modifiers held at release don't stop the release action either. + assert_snapshot!(run(c, "+LWIN +LatQ +LCTL +LFSH -LatQ -LFSH -LCTL -LWIN"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +AD01 24 XK_q + niri test-action + +LCTL 37 XK_Control_L + surface key pressed: 29 + surface modifiers: depressed=68, latched=0, locked=0, group=0 + +LFSH 50 XK_Shift_L + surface key pressed: 42 + surface modifiers: depressed=69, latched=0, locked=0, group=0 + -AD01 24 XK_Q + niri test-action + -LFSH 50 XK_Shift_L + surface key released: 42 + surface modifiers: depressed=68, latched=0, locked=0, group=0 + -LCTL 37 XK_Control_L + surface key released: 29 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + // Push-to-talk: other input in between doesn't cancel the release action. + assert_snapshot!(run(c, "+LWIN +LatQ +LatA -LatA -LatQ -LWIN"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +AD01 24 XK_q + niri test-action + +AC01 38 XK_a + surface key pressed: 30 + -AC01 38 XK_a + surface key released: 30 + -AD01 24 XK_q + niri test-action + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + // Pressing the key without the mod key held doesn't run either action. + assert_snapshot!(run(c, "+LatQ +LWIN -LatQ -LWIN"), @" + +AD01 24 XK_q + surface key pressed: 16 + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + -AD01 24 XK_q + surface key released: 16 + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); +} + +#[test] +fn generic_modifier_press_release() { + let c = " + binds { + Super+Shift { + press { close-window; } + release { center-column; } + } + + Ctrl+Shift { + release { close-window; } + } + } + "; + + // Press Super and then two Shifts. + assert_snapshot!(run(c, "+LWIN +LFSH +RTSH -RTSH -LFSH -LWIN"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +LFSH 50 XK_Shift_L + niri test-action + surface modifiers: depressed=65, latched=0, locked=0, group=0 + +RTSH 62 XK_Shift_R + niri test-action + -RTSH 62 XK_Shift_R + niri test-action + -LFSH 50 XK_Shift_L + niri test-action + surface modifiers: depressed=64, latched=0, locked=0, group=0 + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + // Other release order. + assert_snapshot!(run(c, "+LWIN +LFSH +RTSH -LFSH -RTSH -LWIN"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +LFSH 50 XK_Shift_L + niri test-action + surface modifiers: depressed=65, latched=0, locked=0, group=0 + +RTSH 62 XK_Shift_R + niri test-action + -LFSH 50 XK_Shift_L + niri test-action + -RTSH 62 XK_Shift_R + niri test-action + surface modifiers: depressed=64, latched=0, locked=0, group=0 + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + // Same but release-only. + assert_snapshot!(run(c, "+LCTL +LFSH +RTSH -RTSH -LFSH -LCTL"), @" + +LCTL 37 XK_Control_L + surface key pressed: 29 + surface modifiers: depressed=4, latched=0, locked=0, group=0 + +LFSH 50 XK_Shift_L + surface key pressed: 42 + surface modifiers: depressed=5, latched=0, locked=0, group=0 + +RTSH 62 XK_Shift_R + surface key pressed: 54 + -RTSH 62 XK_Shift_R + niri test-action + surface key released: 54 + -LFSH 50 XK_Shift_L + surface key released: 42 + surface modifiers: depressed=4, latched=0, locked=0, group=0 + -LCTL 37 XK_Control_L + surface key released: 29 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + // Other release order (release-only). + assert_snapshot!(run(c, "+LCTL +LFSH +RTSH -LFSH -RTSH -LCTL"), @" + +LCTL 37 XK_Control_L + surface key pressed: 29 + surface modifiers: depressed=4, latched=0, locked=0, group=0 + +LFSH 50 XK_Shift_L + surface key pressed: 42 + surface modifiers: depressed=5, latched=0, locked=0, group=0 + +RTSH 62 XK_Shift_R + surface key pressed: 54 + -LFSH 50 XK_Shift_L + surface key released: 42 + -RTSH 62 XK_Shift_R + niri test-action + surface key released: 54 + surface modifiers: depressed=4, latched=0, locked=0, group=0 + -LCTL 37 XK_Control_L + surface key released: 29 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + // Now, Shift first. + assert_snapshot!(run(c, "+LFSH +LWIN -LWIN -LFSH"), @" + +LFSH 50 XK_Shift_L + surface key pressed: 42 + surface modifiers: depressed=1, latched=0, locked=0, group=0 + +LWIN 133 XK_Super_L + niri test-action + surface modifiers: depressed=65, latched=0, locked=0, group=0 + -LWIN 133 XK_Super_L + niri test-action + surface modifiers: depressed=1, latched=0, locked=0, group=0 + -LFSH 50 XK_Shift_L + surface key released: 42 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + // Two Supers. + assert_snapshot!(run(c, "+LFSH +LWIN +RWIN -RWIN -LWIN -LFSH"), @" + +LFSH 50 XK_Shift_L + surface key pressed: 42 + surface modifiers: depressed=1, latched=0, locked=0, group=0 + +LWIN 133 XK_Super_L + niri test-action + surface modifiers: depressed=65, latched=0, locked=0, group=0 + +RWIN 134 XK_Super_R + niri test-action + -RWIN 134 XK_Super_R + niri test-action + -LWIN 133 XK_Super_L + niri test-action + surface modifiers: depressed=1, latched=0, locked=0, group=0 + -LFSH 50 XK_Shift_L + surface key released: 42 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + // Unrelated modifier in the middle. + assert_snapshot!(run(c, "+LWIN +LCTL +LFSH -LCTL -LFSH -LWIN"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +LCTL 37 XK_Control_L + surface key pressed: 29 + surface modifiers: depressed=68, latched=0, locked=0, group=0 + +LFSH 50 XK_Shift_L + surface key pressed: 42 + surface modifiers: depressed=69, latched=0, locked=0, group=0 + -LCTL 37 XK_Control_L + surface key released: 29 + surface modifiers: depressed=65, latched=0, locked=0, group=0 + -LFSH 50 XK_Shift_L + surface key released: 42 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + // Regressions. + let c = " + binds { + Shift+Mod { close-window; } + Ctrl+Super { close-window; } + } + "; + + assert_snapshot!(run(c, "+LFSH -LFSH"), @" + +LFSH 50 XK_Shift_L + surface key pressed: 42 + surface modifiers: depressed=1, latched=0, locked=0, group=0 + -LFSH 50 XK_Shift_L + surface key released: 42 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + assert_snapshot!(run(c, "+LFSH +LWIN -LWIN -LFSH"), @" + +LFSH 50 XK_Shift_L + surface key pressed: 42 + surface modifiers: depressed=1, latched=0, locked=0, group=0 + +LWIN 133 XK_Super_L + niri test-action + surface modifiers: depressed=65, latched=0, locked=0, group=0 + -LWIN 133 XK_Super_L + surface modifiers: depressed=1, latched=0, locked=0, group=0 + -LFSH 50 XK_Shift_L + surface key released: 42 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + assert_snapshot!(run(c, "+LCTL -LCTL"), @" + +LCTL 37 XK_Control_L + surface key pressed: 29 + surface modifiers: depressed=4, latched=0, locked=0, group=0 + -LCTL 37 XK_Control_L + surface key released: 29 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + assert_snapshot!(run(c, "+LCTL +LWIN -LWIN -LCTL"), @" + +LCTL 37 XK_Control_L + surface key pressed: 29 + surface modifiers: depressed=4, latched=0, locked=0, group=0 + +LWIN 133 XK_Super_L + niri test-action + surface modifiers: depressed=68, latched=0, locked=0, group=0 + -LWIN 133 XK_Super_L + surface modifiers: depressed=4, latched=0, locked=0, group=0 + -LCTL 37 XK_Control_L + surface key released: 29 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); +} + +#[test] +fn pass_through() { + // A normal press-only bind runs its action and forwards the key. + let c = " + binds { + Mod+Q pass-through=true { close-window; } + } + "; + + assert_snapshot!(run(c, "+LWIN +LatQ -LatQ -LWIN"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +AD01 24 XK_q + niri test-action + surface key pressed: 16 + -AD01 24 XK_q + surface key released: 16 + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + // A modifier-only press-and-release bind runs both actions and forwards the key. + let c = " + binds { + Mod pass-through=true { + press { close-window; } + release { center-column; } + } + } + "; + + assert_snapshot!(run(c, "+LWIN -LWIN"), @" + +LWIN 133 XK_Super_L + niri test-action + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + -LWIN 133 XK_Super_L + niri test-action + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + // A bind on cooldown does not run its action, but still forwards the key. + let c = " + binds { + Mod+Q pass-through=true cooldown-ms=1000 { close-window; } + } + "; + + assert_snapshot!(run(c, "+LWIN +LatQ -LatQ +LatQ -LatQ -LWIN"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +AD01 24 XK_q + niri test-action + surface key pressed: 16 + -AD01 24 XK_q + surface key released: 16 + +AD01 24 XK_q + surface key pressed: 16 + -AD01 24 XK_q + surface key released: 16 + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); +} + +#[test] +fn mod_key_press_and_release_bind() { + let c = " + binds { + Mod { + press { close-window; } + release { center-column; } + } + } + "; + + // Both actions run, and the window doesn't see the key press or release themselves, only the + // modifier state. + assert_snapshot!(run(c, "+LWIN -LWIN"), @" + +LWIN 133 XK_Super_L + niri test-action + surface modifiers: depressed=64, latched=0, locked=0, group=0 + -LWIN 133 XK_Super_L + niri test-action + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + // Other input in between doesn't cancel the release action. + assert_snapshot!(run(c, "+LWIN +LatA -LatA -LWIN"), @" + +LWIN 133 XK_Super_L + niri test-action + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +AC01 38 XK_a + surface key pressed: 30 + -AC01 38 XK_a + surface key released: 30 + -LWIN 133 XK_Super_L + niri test-action + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); +} + +#[test] +fn mod_key_press_and_release_bind_focus_change() { + let c = " + binds { + Mod { + press { close-window; } + release { center-column; } + } + } + "; + + let (mut f, id, surface) = set_up(c); + + // Hold mod: the press action runs, and the window sees the modifier state. + run_f(&mut f, id, &surface, "+LWIN"); + + // A new window takes the keyboard focus while mod is held. + let window = f.client(id).create_window(); + let surface2 = window.surface.clone(); + window.commit(); + f.roundtrip(id); + let window = f.client(id).window(&surface2); + window.attach_new_buffer(); + window.ack_last_and_commit(); + f.roundtrip(id); + + // The new window doesn't see the key press or release, but the focus change and the + // intercepted release still update its modifier state, so it doesn't get stuck with mod held. + assert_snapshot!(run_f(&mut f, id, &surface2, "-LWIN +LatA -LatA"), @" + surface enter: [] + surface modifiers: depressed=64, latched=0, locked=0, group=0 + -LWIN 133 XK_Super_L + niri test-action + surface modifiers: depressed=0, latched=0, locked=0, group=0 + +AC01 38 XK_a + surface key pressed: 30 + -AC01 38 XK_a + surface key released: 30 + "); +} + +#[test] +fn modifier_key_trigger_binds() { + let c = " + binds { + Mod+Control_L { + press { close-window; } + release { center-column; } + } + Alt+Control_L { close-window; } + } + "; + + // A modifier key used as the trigger is intercepted, so the window doesn't see the key press + // and release, only the modifier state; both actions run. + assert_snapshot!(run(c, "+LWIN +LCTL -LCTL -LWIN"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +LCTL 37 XK_Control_L + niri test-action + surface modifiers: depressed=68, latched=0, locked=0, group=0 + -LCTL 37 XK_Control_L + niri test-action + surface modifiers: depressed=64, latched=0, locked=0, group=0 + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + // Without the mod key held the bind doesn't match. + assert_snapshot!(run(c, "+LCTL -LCTL"), @" + +LCTL 37 XK_Control_L + surface key pressed: 29 + surface modifiers: depressed=4, latched=0, locked=0, group=0 + -LCTL 37 XK_Control_L + surface key released: 29 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + // With Alt held instead, the Control_L press is intercepted, and only the press action runs. + assert_snapshot!(run(c, "+LALT +LCTL -LCTL -LALT"), @" + +LALT 64 XK_Alt_L + surface key pressed: 56 + surface modifiers: depressed=8, latched=0, locked=0, group=0 + +LCTL 37 XK_Control_L + niri test-action + surface modifiers: depressed=12, latched=0, locked=0, group=0 + -LCTL 37 XK_Control_L + surface modifiers: depressed=8, latched=0, locked=0, group=0 + -LALT 64 XK_Alt_L + surface key released: 56 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); +} + +#[test] +fn release_bind_on_modifier_key() { + let c = " + binds { + Mod+Control_L { + release { close-window; } + } + } + "; + + // The press is forwarded so that apps see the modifier, and the release action runs when the + // modifier is released on its own. + assert_snapshot!(run(c, "+LWIN +LCTL -LCTL -LWIN"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +LCTL 37 XK_Control_L + surface key pressed: 29 + surface modifiers: depressed=68, latched=0, locked=0, group=0 + -LCTL 37 XK_Control_L + niri test-action + surface key released: 29 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + // Pressing another key in between cancels the release action. + assert_snapshot!(run(c, "+LWIN +LCTL +LFSH -LCTL -LFSH -LWIN"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +LCTL 37 XK_Control_L + surface key pressed: 29 + surface modifiers: depressed=68, latched=0, locked=0, group=0 + +LFSH 50 XK_Shift_L + surface key pressed: 42 + surface modifiers: depressed=69, latched=0, locked=0, group=0 + -LCTL 37 XK_Control_L + surface key released: 29 + surface modifiers: depressed=65, latched=0, locked=0, group=0 + -LFSH 50 XK_Shift_L + surface key released: 42 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); +} + +#[test] +fn modifier_key_and_keysym_triggers() { + // `Ctrl` is not the mod key, so it is matched by keysym directly: both Control_L and + // Control_R trigger it. + let by_modifier = " + binds { + Ctrl { + release { close-window; } + } + } + "; + + assert_snapshot!(run(by_modifier, "+LCTL -LCTL"), @" + +LCTL 37 XK_Control_L + surface key pressed: 29 + surface modifiers: depressed=4, latched=0, locked=0, group=0 + -LCTL 37 XK_Control_L + niri test-action + surface key released: 29 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + assert_snapshot!(run(by_modifier, "+RCTL -RCTL"), @" + +RCTL 105 XK_Control_R + surface key pressed: 97 + surface modifiers: depressed=4, latched=0, locked=0, group=0 + -RCTL 105 XK_Control_R + niri test-action + surface key released: 97 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + // `Control_L` is a specific key instead, so it doesn't trigger on Control_R, which reaches + // the window instead. + let by_keysym = " + binds { + Control_L { + release { close-window; } + } + } + "; + + assert_snapshot!(run(by_keysym, "+LCTL -LCTL"), @" + +LCTL 37 XK_Control_L + surface key pressed: 29 + surface modifiers: depressed=4, latched=0, locked=0, group=0 + -LCTL 37 XK_Control_L + niri test-action + surface key released: 29 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + assert_snapshot!(run(by_keysym, "+RCTL -RCTL"), @" + +RCTL 105 XK_Control_R + surface key pressed: 97 + surface modifiers: depressed=4, latched=0, locked=0, group=0 + -RCTL 105 XK_Control_R + surface key released: 97 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); +} + +#[test] +fn mod_key_trigger_with_own_modifier() { + // The trigger is the configured mod key, and has Mod at a modifier. + let bind_by_mod = " + binds { + Mod+Super_L { close-window; } + } + "; + + assert_snapshot!(run(bind_by_mod, "+LWIN -LWIN"), @" + +LWIN 133 XK_Super_L + niri test-action + surface modifiers: depressed=64, latched=0, locked=0, group=0 + -LWIN 133 XK_Super_L + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + let bind_by_keysym = " + binds { + Super_L { close-window; } + } + "; + + assert_snapshot!(run(bind_by_keysym, "+LWIN -LWIN"), @" + +LWIN 133 XK_Super_L + niri test-action + surface modifiers: depressed=64, latched=0, locked=0, group=0 + -LWIN 133 XK_Super_L + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + let bind_by_generic = " + binds { + Mod+Super { close-window; } + } + "; + + assert_snapshot!(run(bind_by_generic, "+LWIN -LWIN"), @" + +LWIN 133 XK_Super_L + niri test-action + surface modifiers: depressed=64, latched=0, locked=0, group=0 + -LWIN 133 XK_Super_L + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); +} + +#[test] +fn release_bind_requires_matching_modifiers_on_press() { + let c = " + binds { + Mod+Ctrl+Q { + release { close-window; } + } + } + "; + + // Pressing Q without the mod key held forwards it and doesn't arm the release action, even if + // the mod key is held by the time Q is released. + assert_snapshot!(run(c, "+LCTL +LatQ +LWIN -LatQ -LWIN -LCTL"), @" + +LCTL 37 XK_Control_L + surface key pressed: 29 + surface modifiers: depressed=4, latched=0, locked=0, group=0 + +AD01 24 XK_q + surface key pressed: 16 + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=68, latched=0, locked=0, group=0 + -AD01 24 XK_q + surface key released: 16 + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=4, latched=0, locked=0, group=0 + -LCTL 37 XK_Control_L + surface key released: 29 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); +} + +/// `Mod` and `Mod+Q` release-only binds, used by the inhibitor tests below. +const INHIBITING_BINDS: &str = " + binds { + Mod { + release { toggle-overview; } + } + Mod+Q { + release { close-window; } + } + } +"; + +#[test] +fn inhibiting_release_binds() { + let (mut f, id, surface) = set_up(INHIBITING_BINDS); + let _inhibitor = f.client(id).state.inhibit_shortcuts(&surface); + f.roundtrip(id); + + // While inhibiting, release binds don't run, and the whole key is forwarded to the window. + assert_snapshot!(run_f(&mut f, id, &surface, "+LWIN +LatQ -LatQ -LWIN"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +AD01 24 XK_q + surface key pressed: 16 + -AD01 24 XK_q + surface key released: 16 + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); +} + +#[test] +fn release_binds_run_after_the_inhibitor_is_gone() { + let (mut f, id, surface) = set_up(INHIBITING_BINDS); + let inhibitor = f.client(id).state.inhibit_shortcuts(&surface); + f.roundtrip(id); + inhibitor.destroy(); + f.roundtrip(id); + + assert_snapshot!(run_f(&mut f, id, &surface, "+LWIN +LatQ -LatQ -LWIN"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +AD01 24 XK_q + surface key pressed: 16 + -AD01 24 XK_q + niri test-action + surface key released: 16 + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); +} + +#[test] +fn deactivating_the_inhibitor_does_not_arm_a_release_bind_retroactively() { + let (mut f, id, surface) = set_up(INHIBITING_BINDS); + let inhibitor = f.client(id).state.inhibit_shortcuts(&surface); + f.roundtrip(id); + + // The press while inhibiting is forwarded and arms nothing. + run_f(&mut f, id, &surface, "+LWIN +LatQ"); + + inhibitor.destroy(); + f.roundtrip(id); + + // The release is still forwarded, and still runs no action. + assert_snapshot!(run_f(&mut f, id, &surface, "-LatQ -LWIN"), @" + -AD01 24 XK_q + surface key released: 16 + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); +} + +#[test] +fn activating_the_inhibitor_keeps_an_armed_release_bind() { + let (mut f, id, surface) = set_up(INHIBITING_BINDS); + + // The press while not inhibiting is intercepted and arms the release action. + run_f(&mut f, id, &surface, "+LWIN +LatQ"); + + let _inhibitor = f.client(id).state.inhibit_shortcuts(&surface); + f.roundtrip(id); + + // The release stays intercepted, and still runs the armed release action. + assert_snapshot!(run_f(&mut f, id, &surface, "-LatQ -LWIN"), @" + -AD01 24 XK_q + niri test-action + surface key released: 16 + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); +} + +#[test] +fn inhibiting_a_modifier_press_still_forwards_it() { + let (mut f, id, surface) = set_up(INHIBITING_BINDS); + let _inhibitor = f.client(id).state.inhibit_shortcuts(&surface); + f.roundtrip(id); + + // A modifier press is forwarded either way; the inhibitor only decides whether it arms the + // release action, and here it doesn't. + assert_snapshot!(run_f(&mut f, id, &surface, "+LWIN"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + "); +} + +#[test] +fn armed_modifier_release_runs_after_the_inhibitor_activates() { + let (mut f, id, surface) = set_up(INHIBITING_BINDS); + + // The modifier press while not inhibiting arms its release action. + run_f(&mut f, id, &surface, "+LWIN"); + + let _inhibitor = f.client(id).state.inhibit_shortcuts(&surface); + f.roundtrip(id); + + assert_snapshot!(run_f(&mut f, id, &surface, "-LWIN"), @" + -LWIN 133 XK_Super_L + niri test-action + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); +} + +#[test] +fn egl_release_action_runs_when_press_opens_screenshot_ui() { + // Release actions must always run if their press action ran, + // including if the screenshot UI is opened before the key is released. + let c = " + binds { + Mod+Q { + press { screenshot; } + release { close-window; } + } + } + "; + + let (mut f, id, surface) = set_up(c); + f.niri_state().backend.headless().add_renderer().unwrap(); + + assert_snapshot!(run_f(&mut f, id, &surface, "+LWIN +LatQ -LatQ -LWIN"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +AD01 24 XK_q + surface leave + -AD01 24 XK_q + niri test-action + -LWIN 133 XK_Super_L + "); + assert!(f.niri().screenshot_ui.is_open()); +} + +#[test] +fn cooldown_skips_whole_bind() { + // A bind on cooldown is skipped entirely: its press runs no action, and no release action is + // recorded, so its release runs no action either. + let c = " + binds { + Mod+MouseLeft cooldown-ms=1000 { + press { close-window; } + release { center-column; } + } + } + "; + + assert_snapshot!(run(c, "+LWIN +MouseLeft -MouseLeft +MouseLeft -MouseLeft -LWIN"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +MouseLeft + niri test-action + -MouseLeft + niri test-action + +MouseLeft + -MouseLeft + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); +} + +#[test] +fn cooldown_only_skips_the_action() { + let c = " + binds { + Mod+Control_L cooldown-ms=1000 { close-window; } + } + "; + + // The cooldown only skips the action: the second press runs no action, but it is still + // intercepted and hidden from the window, only advertising the modifier state. + assert_snapshot!(run(c, "+LWIN +LCTL -LCTL +LCTL"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +LCTL 37 XK_Control_L + niri test-action + surface modifiers: depressed=68, latched=0, locked=0, group=0 + -LCTL 37 XK_Control_L + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +LCTL 37 XK_Control_L + surface modifiers: depressed=68, latched=0, locked=0, group=0 + "); +} + +#[test] +fn cooldown_applies_to_release_action_of_release_only_bind() { + // The release action is what starts this bind, so the cooldown rate-limits it: it runs for the + // first release, and not for the second while the bind is on cooldown. + let c = " + binds { + Mod cooldown-ms=1000 { + release { toggle-overview; } + } + } + "; + + assert_snapshot!(run(c, "+LWIN -LWIN +LWIN -LWIN"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + -LWIN 133 XK_Super_L + niri test-action + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); +} + +#[test] +fn cooldown_applies_to_press_action_only() { + // The cooldown applies to the press action only, and the release action runs only if its press + // action ran, so the second press runs no action at all. + let c = " + binds { + Mod+Q cooldown-ms=1000 { + press { close-window; } + release { center-column; } + } + } + "; + + assert_snapshot!(run(c, "+LWIN +LatQ -LatQ +LatQ -LatQ -LWIN"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +AD01 24 XK_q + niri test-action + -AD01 24 XK_q + niri test-action + +AD01 24 XK_q + -AD01 24 XK_q + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); +} + +#[test] +fn release_bind_runs_when_mru_closes() { + // `Alt` is a push-to-talk bind: the default Alt+Tab opens the MRU, and releasing Alt closes it + // instead of going through the regular bind lookup, so its release action must still run. + let c = " + binds { + Alt { + press { spawn \"ptt-toggle\"; } + release { spawn \"ptt-toggle\"; } + } + } + "; + + assert_snapshot!(run(c, "+LALT +TAB -LALT"), @" + +LALT 64 XK_Alt_L + niri test-action + surface modifiers: depressed=8, latched=0, locked=0, group=0 + +TAB 23 XK_Tab + surface leave + -LALT 64 XK_Alt_L + niri test-action + surface enter: [] + surface modifiers: depressed=0, latched=0, locked=0, group=0 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); +} + +#[test] +fn mru_close_forwards_key_release_without_bind() { + // No bind is recorded for Alt, so releasing it while the MRU is open closes the MRU and the + // release is forwarded to the window like any other unsuppressed key. + let c = " + binds { + Mod+Q { close-window; } + } + "; + + assert_snapshot!(run(c, "+LALT +TAB -LALT"), @" + +LALT 64 XK_Alt_L + surface key pressed: 56 + surface modifiers: depressed=8, latched=0, locked=0, group=0 + +TAB 23 XK_Tab + surface leave + -LALT 64 XK_Alt_L + surface enter: [56] + surface modifiers: depressed=0, latched=0, locked=0, group=0 + surface key released: 56 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); +} + +#[test] +fn mru_ignores_generic_modifier_binds() { + let c = " + binds { + Alt+Ctrl { screenshot; } + } + "; + + // If a press runs the screenshot action, + // we will get surface enter after it. + assert_snapshot!(run(c, "+LALT +TAB -TAB +LFSH -LFSH -LALT"), @" + +LALT 64 XK_Alt_L + surface key pressed: 56 + surface modifiers: depressed=8, latched=0, locked=0, group=0 + +TAB 23 XK_Tab + surface leave + -TAB 23 XK_Tab + +LFSH 50 XK_Shift_L + -LFSH 50 XK_Shift_L + -LALT 64 XK_Alt_L + surface enter: [56] + surface modifiers: depressed=0, latched=0, locked=0, group=0 + surface key released: 56 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + assert_snapshot!(run(c, "+LALT +TAB -TAB +LCTL -LCTL -LALT"), @" + +LALT 64 XK_Alt_L + surface key pressed: 56 + surface modifiers: depressed=8, latched=0, locked=0, group=0 + +TAB 23 XK_Tab + surface leave + -TAB 23 XK_Tab + +LCTL 37 XK_Control_L + -LCTL 37 XK_Control_L + -LALT 64 XK_Alt_L + surface enter: [56] + surface modifiers: depressed=0, latched=0, locked=0, group=0 + surface key released: 56 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + let c = " + binds { + Ctrl+Shift { screenshot; } + } + "; + + assert_snapshot!(run(c, "+LALT +TAB -TAB +LFSH -LFSH -LALT"), @" + +LALT 64 XK_Alt_L + surface key pressed: 56 + surface modifiers: depressed=8, latched=0, locked=0, group=0 + +TAB 23 XK_Tab + surface leave + -TAB 23 XK_Tab + +LFSH 50 XK_Shift_L + -LFSH 50 XK_Shift_L + -LALT 64 XK_Alt_L + surface enter: [56] + surface modifiers: depressed=0, latched=0, locked=0, group=0 + surface key released: 56 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + assert_snapshot!(run(c, "+LALT +TAB -TAB +LCTL +LFSH -LFSH -LCTL -LALT"), @" + +LALT 64 XK_Alt_L + surface key pressed: 56 + surface modifiers: depressed=8, latched=0, locked=0, group=0 + +TAB 23 XK_Tab + surface leave + -TAB 23 XK_Tab + +LCTL 37 XK_Control_L + +LFSH 50 XK_Shift_L + -LFSH 50 XK_Shift_L + -LCTL 37 XK_Control_L + -LALT 64 XK_Alt_L + surface enter: [56] + surface modifiers: depressed=0, latched=0, locked=0, group=0 + surface key released: 56 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + // Now with a normal trigger. + let c = " + binds { + Alt+Control_L { screenshot; } + } + "; + + assert_snapshot!(run(c, "+LALT +TAB -TAB +LFSH -LFSH -LALT"), @" + +LALT 64 XK_Alt_L + surface key pressed: 56 + surface modifiers: depressed=8, latched=0, locked=0, group=0 + +TAB 23 XK_Tab + surface leave + -TAB 23 XK_Tab + +LFSH 50 XK_Shift_L + -LFSH 50 XK_Shift_L + -LALT 64 XK_Alt_L + surface enter: [56] + surface modifiers: depressed=0, latched=0, locked=0, group=0 + surface key released: 56 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + // The headless test backend has a renderer, so the screenshot UI opens and keeps the + // keyboard focus. The surface getting no enter on the final Alt release shows that the + // MRU was closed by the bind rather than confirmed. + assert_snapshot!(run(c, "+LALT +TAB -TAB +LCTL -LCTL -LALT"), @" + +LALT 64 XK_Alt_L + surface key pressed: 56 + surface modifiers: depressed=8, latched=0, locked=0, group=0 + +TAB 23 XK_Tab + surface leave + -TAB 23 XK_Tab + +LCTL 37 XK_Control_L + -LCTL 37 XK_Control_L + -LALT 64 XK_Alt_L + "); + + let c = " + binds { + Ctrl+Shift_L { screenshot; } + } + "; + + assert_snapshot!(run(c, "+LALT +TAB -TAB +LFSH -LFSH -LALT"), @" + +LALT 64 XK_Alt_L + surface key pressed: 56 + surface modifiers: depressed=8, latched=0, locked=0, group=0 + +TAB 23 XK_Tab + surface leave + -TAB 23 XK_Tab + +LFSH 50 XK_Shift_L + -LFSH 50 XK_Shift_L + -LALT 64 XK_Alt_L + "); + + assert_snapshot!(run(c, "+LALT +TAB -TAB +LCTL +LFSH -LFSH -LCTL -LALT"), @" + +LALT 64 XK_Alt_L + surface key pressed: 56 + surface modifiers: depressed=8, latched=0, locked=0, group=0 + +TAB 23 XK_Tab + surface leave + -TAB 23 XK_Tab + +LCTL 37 XK_Control_L + +LFSH 50 XK_Shift_L + -LFSH 50 XK_Shift_L + -LCTL 37 XK_Control_L + -LALT 64 XK_Alt_L + "); +} + +#[test] +fn vt_switch_runs_release_actions_of_held_binds() { + // A VT switch can take the input away without ever delivering the releases of everything held + // down, so the held binds are treated as released rather than forgotten. + let c = " + binds { + Mod+Q { + press { close-window; } + release { center-column; } + } + Mod+MouseLeft { + press { close-window; } + release { center-column; } + } + } + "; + + let (mut f, id, surface) = set_up(c); + + // Hold a key and a mouse button, both with press and release actions. + run_f(&mut f, id, &surface, "+LWIN +LatQ +MouseLeft"); + assert_eq!(f.niri().test_action_count, 2); + + // The VT switch runs both release actions. + f.niri_state().do_action(Action::ChangeVt(1), false); + assert_eq!(f.niri().test_action_count, 4); + + // If the releases arrive anyway, they run nothing. + run_f(&mut f, id, &surface, "-LatQ -MouseLeft"); + assert_eq!(f.niri().test_action_count, 4); +} + +#[test] +fn other_input_cancels_release_only_mouse_bind() { + // A release-only mouse bind is cancelled by other input in between, just like a release-only + // keyboard bind. + let c = " + binds { + Mod+MouseLeft { + release { close-window; } + } + } + "; + + // Pressing a key in between cancels it. + assert_snapshot!(run(c, "+LWIN +MouseLeft +LatA -LatA -MouseLeft -LWIN"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +MouseLeft + +AC01 38 XK_a + surface key pressed: 30 + -AC01 38 XK_a + surface key released: 30 + -MouseLeft + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + // Pressing another mouse button in between cancels it. + assert_snapshot!(run(c, "+LWIN +MouseLeft +MouseRight -MouseRight -MouseLeft -LWIN"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +MouseLeft + +MouseRight + -MouseRight + -MouseLeft + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + // With nothing else in between, the release action runs. + assert_snapshot!(run(c, "+LWIN +MouseLeft -MouseLeft -LWIN"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +MouseLeft + -MouseLeft + niri test-action + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); +} + +#[test] +fn mouse_button_press_cancels_release_only_key_bind() { + // A mouse button press is other input, so it interrupts a pending release-only keyboard bind, + // including one on a bare modifier. Only the mouse bind's press action runs. + let c = " + binds { + Mod { + release { center-column; } + } + Mod+MouseLeft { close-window; } + } + "; + + assert_snapshot!(run(c, "+LWIN +MouseLeft -MouseLeft -LWIN"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + +MouseLeft + niri test-action + -MouseLeft + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); +} + +#[test] +fn scroll_cancels_release_only_key_bind() { + // Scrolling is input, so it cancels a pending release-only bind. Here there is no scroll bind + // for the held modifiers. + let c = " + binds { + Mod { + release { center-column; } + } + } + "; + + // The scroll in between cancels the pending release bind, so the release runs no action. + assert_snapshot!(run(c, "+LWIN WheelScrollDown -LWIN"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + WheelScrollDown + -LWIN 133 XK_Super_L + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); + + // Without a scroll in between, the release action runs. + assert_snapshot!(run(c, "+LWIN -LWIN"), @" + +LWIN 133 XK_Super_L + surface key pressed: 125 + surface modifiers: depressed=64, latched=0, locked=0, group=0 + -LWIN 133 XK_Super_L + niri test-action + surface key released: 125 + surface modifiers: depressed=0, latched=0, locked=0, group=0 + "); +} + +#[test] +fn redundant_modifier_binds() { + // Binds should behave identically with or without redundant modifiers. + let input = "+LALT -LALT"; + assert_eq!( + run("binds { Alt_L { release { close-window; }; }; }", input), + run("binds { Alt+Alt_L { release { close-window; }; }; }", input) + ); + + let input = "+LALT +LatQ -LatQ -LALT"; + assert_eq!( + run("binds { Alt+Q { release { close-window; }; }; }", input), + run("binds { Alt+Alt+Q { release { close-window; }; }; }", input) + ); } diff --git a/src/tests/client.rs b/src/tests/client.rs index e86d07b7fb..dd87ed1f10 100644 --- a/src/tests/client.rs +++ b/src/tests/client.rs @@ -65,6 +65,7 @@ pub struct State { pub layer_shell: Option, pub virtual_keyboard_manager: Option, pub virtual_pointer_manager: Option, + pub virtual_pointer: Option, pub spbm: Option, pub viewporter: Option, pub ksim: Option, @@ -121,7 +122,7 @@ pub struct Keyboard { #[derive(Debug)] pub enum KeyboardEvent { Enter { - keys: Vec, + keys: Vec, }, Leave, Key { @@ -266,6 +267,7 @@ impl Client { layer_shell: None, virtual_keyboard_manager: None, virtual_pointer_manager: None, + virtual_pointer: None, spbm: None, viewporter: None, ksim: None, @@ -434,6 +436,17 @@ impl State { let seat = self.seats.keys().next().unwrap(); ksim.inhibit_shortcuts(surface, seat, &self.qh, ()) } + + pub fn virtual_pointer(&mut self) -> ZwlrVirtualPointerV1 { + if let Some(pointer) = &self.virtual_pointer { + return pointer.clone(); + } + + let manager = self.virtual_pointer_manager.as_ref().unwrap(); + let pointer = manager.create_virtual_pointer(None, &self.qh, ()); + self.virtual_pointer = Some(pointer.clone()); + pointer + } } impl Window { @@ -751,6 +764,10 @@ impl Dispatch for State { wl_keyboard::Event::Enter { surface, keys, .. } => { keyboard.surface = Some(surface.clone()); + // The order is arbitrary, but we need a fixed order for our tests. + let mut keys: Vec = bytemuck::pod_collect_to_vec(&keys); + keys.sort_unstable(); + let event = KeyboardEvent::Enter { keys }; let surface = state.surfaces.get_mut(&surface).unwrap(); surface.keyboard_events_received.push(event); diff --git a/src/tests/virtual_pointer.rs b/src/tests/virtual_pointer.rs index b9aa02ae69..85dcbea451 100644 --- a/src/tests/virtual_pointer.rs +++ b/src/tests/virtual_pointer.rs @@ -8,8 +8,7 @@ fn axis_discrete_overflow() { let id = f.add_client(); let client = f.client(id); - let manager = client.state.virtual_pointer_manager.as_ref().unwrap(); - let pointer = manager.create_virtual_pointer(None, &client.qh, ()); + let pointer = client.state.virtual_pointer(); pointer.axis_discrete(0, wl_pointer::Axis::VerticalScroll, 0., i32::MAX); f.roundtrip(id); } diff --git a/src/ui/hotkey_overlay.rs b/src/ui/hotkey_overlay.rs index e3105c80d4..998e56632e 100644 --- a/src/ui/hotkey_overlay.rs +++ b/src/ui/hotkey_overlay.rs @@ -152,14 +152,34 @@ impl HotkeyOverlay { } } +/// Returns the actions that a bind triggers. +/// +/// A press or release with several actions has no single action to show, so it is skipped. +fn bind_actions(bind: &Bind) -> impl Iterator { + fn single(actions: &[Action]) -> Option<&Action> { + match actions { + [action] => Some(action), + _ => None, + } + } + + single(bind.press_actions()) + .into_iter() + .chain(single(bind.release_actions())) +} + +/// Whether the bind triggers the action, on press or on release. +fn bind_triggers(bind: &Bind, action: &Action) -> bool { + bind_actions(bind).any(|a| a == action) +} + fn format_bind(binds: &[Bind], action: &Action) -> Option<(Option, String)> { let mut bind_with_non_null = None; let mut bind_with_custom_title = None; let mut found_null_title = false; for bind in binds { - // Multi-action binds are not shown in the hotkey overlay. - if bind.actions.as_slice() != std::slice::from_ref(action) { + if !bind_triggers(bind, action) { continue; } @@ -205,12 +225,12 @@ fn collect_actions(config: &Config) -> Vec<&Action> { // Quit(false). if binds .iter() - .any(|bind| bind.actions.as_slice() == [Action::Quit(false)]) + .any(|bind| bind_triggers(bind, &Action::Quit(false))) { actions.push(&Action::Quit(false)); } else if binds .iter() - .any(|bind| bind.actions.as_slice() == [Action::Quit(true)]) + .any(|bind| bind_triggers(bind, &Action::Quit(true))) { actions.push(&Action::Quit(true)); } else { @@ -228,38 +248,34 @@ fn collect_actions(config: &Config) -> Vec<&Action> { ]); // Prefer move-column-to-workspace-down, but fall back to move-window-to-workspace-down. - if let Some(bind) = binds.iter().find(|bind| { - matches!( - bind.actions.as_slice(), - [Action::MoveColumnToWorkspaceDown(_)] - ) - }) { - actions.push(&bind.actions[0]); - } else if binds.iter().any(|bind| { - matches!( - bind.actions.as_slice(), - [Action::MoveWindowToWorkspaceDown(_)] - ) - }) { + if let Some(action) = binds + .iter() + .flat_map(bind_actions) + .find(|action| matches!(action, Action::MoveColumnToWorkspaceDown(_))) + { + actions.push(action); + } else if binds + .iter() + .flat_map(bind_actions) + .any(|action| matches!(action, Action::MoveWindowToWorkspaceDown(_))) + { actions.push(&Action::MoveWindowToWorkspaceDown(true)); } else { actions.push(&Action::MoveColumnToWorkspaceDown(true)); } // Same for -up. - if let Some(bind) = binds.iter().find(|bind| { - matches!( - bind.actions.as_slice(), - [Action::MoveColumnToWorkspaceUp(_)] - ) - }) { - actions.push(&bind.actions[0]); - } else if binds.iter().any(|bind| { - matches!( - bind.actions.as_slice(), - [Action::MoveWindowToWorkspaceUp(_)] - ) - }) { + if let Some(action) = binds + .iter() + .flat_map(bind_actions) + .find(|action| matches!(action, Action::MoveColumnToWorkspaceUp(_))) + { + actions.push(action); + } else if binds + .iter() + .flat_map(bind_actions) + .any(|action| matches!(action, Action::MoveWindowToWorkspaceUp(_))) + { actions.push(&Action::MoveWindowToWorkspaceUp(true)); } else { actions.push(&Action::MoveColumnToWorkspaceUp(true)); @@ -277,42 +293,42 @@ fn collect_actions(config: &Config) -> Vec<&Action> { ]); // Screenshot is not as important, can omit if not bound. - if let Some(bind) = binds + if let Some(action) = binds .iter() - .find(|bind| matches!(bind.actions.as_slice(), [Action::Screenshot(_, _)])) + .flat_map(bind_actions) + .find(|action| matches!(action, Action::Screenshot(_, _))) { - actions.push(&bind.actions[0]); + actions.push(action); } // Add actions with a custom hotkey-overlay-title. for bind in binds { if matches!(bind.hotkey_overlay_title, Some(Some(_))) { - // Multi-action binds don't have a single action to show. - let [bind_action] = bind.actions.as_slice() else { - continue; - }; - // Avoid duplicate actions. - if !actions.contains(&bind_action) { - actions.push(bind_action); + if let Some(action) = bind_actions(bind).next() { + if !actions.contains(&action) { + actions.push(action); + } } } } // Add the spawn actions. - for bind in binds.iter().filter(|bind| { - matches!( - bind.actions.as_slice(), - [Action::Spawn(_)] | [Action::SpawnSh(_)] - ) - // Only show binds with Mod or Super to filter out stuff like volume up/down. - && (bind.key.modifiers.contains(Modifiers::COMPOSITOR) - || bind.key.modifiers.contains(Modifiers::SUPER)) - // Also filter out wheel and touchpad scroll binds. - && matches!(bind.key.trigger, Trigger::Keysym(_)) + for action in binds.iter().filter_map(|bind| { + // Includes the trigger's implied modifier. + let modifiers = bind.key.modifiers; + + // Only show binds with Mod or Super to filter out stuff like volume up/down. + // Also filter out wheel and touchpad scroll binds. + if bind.key.trigger.is_keyboard() + && modifiers.intersects(Modifiers::COMPOSITOR | Modifiers::SUPER) + { + bind_actions(bind) + .find(|action| matches!(action, Action::Spawn(_) | Action::SpawnSh(_))) + } else { + None + } }) { - let action = &bind.actions[0]; - // We only show one bind for each action, so we need to deduplicate the Spawn actions. if !actions.contains(&action) { actions.push(action); @@ -320,13 +336,9 @@ fn collect_actions(config: &Config) -> Vec<&Action> { } if config.hotkey_overlay.hide_not_bound { - // Only keep actions that have been bound. Multi-action binds are not shown in the - // hotkey overlay, so an action must be the sole action of some bind to count. - actions.retain(|&action| { - binds - .iter() - .any(|bind| bind.actions.as_slice() == std::slice::from_ref(action)) - }) + // Only keep actions that have been bound. A press or release with several actions is + // not shown in the hotkey overlay, so it doesn't count. + actions.retain(|&action| binds.iter().any(|bind| bind_triggers(bind, action))) } actions @@ -530,50 +542,43 @@ fn action_name(action: &Action) -> String { fn key_name(screen_reader: bool, mod_key: ModKey, key: &Key) -> String { let mut name = String::new(); - let has_comp_mod = key.modifiers.contains(Modifiers::COMPOSITOR); + // Do not show modifiers implied by the trigger. + let mut implied = key.trigger.implied_modifier(); + // Spellings of the mod key also imply the compositor modifier (e.g. Mod+Super_L). + let mod_modifiers = Modifiers::COMPOSITOR | mod_key.to_modifiers(); + if implied.intersects(mod_modifiers) { + implied |= mod_modifiers; + } + let modifiers = key.modifiers & !implied; + + let has_comp_mod = modifiers.contains(Modifiers::COMPOSITOR); + + let mod_key_pretty = modifier_name(mod_key); // Compositor mod goes first. if has_comp_mod { - match mod_key { - ModKey::Super => { - name.push_str("Super + "); - } - ModKey::Alt => { - name.push_str("Alt + "); - } - ModKey::Shift => { - name.push_str("Shift + "); - } - ModKey::Ctrl => { - name.push_str("Ctrl + "); - } - ModKey::IsoLevel3Shift => { - name.push_str("Mod5 + "); - } - ModKey::IsoLevel5Shift => { - name.push_str("Mod3 + "); - } - } + name.push_str(mod_key_pretty); + name.push_str(" + "); } - if key.modifiers.contains(Modifiers::SUPER) && !(has_comp_mod && mod_key == ModKey::Super) { + if modifiers.contains(Modifiers::SUPER) && !(has_comp_mod && mod_key == ModKey::Super) { name.push_str("Super + "); } - if key.modifiers.contains(Modifiers::CTRL) && !(has_comp_mod && mod_key == ModKey::Ctrl) { + if modifiers.contains(Modifiers::CTRL) && !(has_comp_mod && mod_key == ModKey::Ctrl) { name.push_str("Ctrl + "); } - if key.modifiers.contains(Modifiers::SHIFT) && !(has_comp_mod && mod_key == ModKey::Shift) { + if modifiers.contains(Modifiers::SHIFT) && !(has_comp_mod && mod_key == ModKey::Shift) { name.push_str("Shift + "); } - if key.modifiers.contains(Modifiers::ALT) && !(has_comp_mod && mod_key == ModKey::Alt) { + if modifiers.contains(Modifiers::ALT) && !(has_comp_mod && mod_key == ModKey::Alt) { name.push_str("Alt + "); } - if key.modifiers.contains(Modifiers::ISO_LEVEL3_SHIFT) + if modifiers.contains(Modifiers::ISO_LEVEL3_SHIFT) && !(has_comp_mod && mod_key == ModKey::IsoLevel3Shift) { name.push_str("Mod5 + "); } - if key.modifiers.contains(Modifiers::ISO_LEVEL5_SHIFT) + if modifiers.contains(Modifiers::ISO_LEVEL5_SHIFT) && !(has_comp_mod && mod_key == ModKey::IsoLevel5Shift) { name.push_str("Mod3 + "); @@ -581,6 +586,8 @@ fn key_name(screen_reader: bool, mod_key: ModKey, key: &Key) -> String { let pretty = match key.trigger { Trigger::Keysym(keysym) => prettify_keysym_name(screen_reader, &keysym_get_name(keysym)), + Trigger::CompositorMod => mod_key_pretty.into(), + Trigger::Modifier(modifier) => modifier_name(modifier).into(), Trigger::MouseLeft => String::from("Mouse Left"), Trigger::MouseRight => String::from("Mouse Right"), Trigger::MouseMiddle => String::from("Mouse Middle"), @@ -603,6 +610,17 @@ fn key_name(screen_reader: bool, mod_key: ModKey, key: &Key) -> String { name } +fn modifier_name(modifier: ModKey) -> &'static str { + match modifier { + ModKey::Super => "Super", + ModKey::Alt => "Alt", + ModKey::Shift => "Shift", + ModKey::Ctrl => "Ctrl", + ModKey::IsoLevel3Shift => "Mod5", + ModKey::IsoLevel5Shift => "Mod3", + } +} + fn prettify_keysym_name(screen_reader: bool, name: &str) -> String { let name = if screen_reader { name @@ -666,6 +684,114 @@ mod tests { // Not bound. assert_snapshot!(check("", Action::Screenshot(true, None)), @" (not bound) : Take a Screenshot"); + // Bare modifier binds. + assert_snapshot!( + check( + r#"binds { + Alt { close-window; } + }"#, + Action::CloseWindow, + ), + @" Alt : Close Focused Window" + ); + assert_snapshot!( + check( + r#"binds { + Ctrl { close-window; } + Shift { close-window; } + }"#, + Action::CloseWindow, + ), + @" Ctrl : Close Focused Window" + ); + assert_snapshot!( + check( + r#"binds { + Mod5 { close-window; } + Mod3 { close-window; } + }"#, + Action::CloseWindow, + ), + @" Mod5 : Close Focused Window" + ); + assert_snapshot!( + check( + r#"binds { + Super { close-window; } + }"#, + Action::CloseWindow, + ), + @" Super : Close Focused Window" + ); + + // Modifier + modifier binds: a held modifier key with another modifier key as the trigger. + assert_snapshot!( + check( + r#"binds { + Ctrl+Alt_L { close-window; } + }"#, + Action::CloseWindow, + ), + @" Ctrl + Alt_L : Close Focused Window" + ); + assert_snapshot!( + check( + r#"binds { + Alt+Ctrl { close-window; } + }"#, + Action::CloseWindow, + ), + @" Alt + Ctrl : Close Focused Window" + ); + + // A Ctrl keysym names only one side of the keyboard, so it keeps its exact name instead of + // being shortened to `Ctrl`. + assert_snapshot!( + check( + r#"binds { + Control_L { close-window; } + }"#, + Action::CloseWindow, + ), + @" Control_L : Close Focused Window" + ); + assert_snapshot!( + check( + r#"binds { + Mod+Control_R { close-window; } + }"#, + Action::CloseWindow, + ), + @" Super + Control_R : Close Focused Window" + ); + + // A release bind is shown too. + assert_snapshot!( + check( + r#"binds { + Mod { + release { toggle-overview; } + } + }"#, + Action::ToggleOverview, + ), + @" Super : Open the Overview" + ); + + // Same for the release action of a bind with both actions. + assert_snapshot!( + check( + r#"binds { + Mod+Shift+Q { + press { close-window; } + release { toggle-overview; } + } + }"#, + Action::ToggleOverview, + ), + @" Super + Shift + Q : Open the Overview" + ); + // Bound with a default title. assert_snapshot!( check( @@ -747,5 +873,110 @@ mod tests { ), @" Super + P : Hello" ); + + // Mod overlapping with trigger. + assert_snapshot!( + check( + r#"binds { + Mod+Super_L { screenshot; } + }"#, + Action::Screenshot(true, None), + ), + @" Super_L : Take a Screenshot" + ); + assert_snapshot!( + check( + r#"binds { + Mod+Super { screenshot; } + }"#, + Action::Screenshot(true, None), + ), + @" Super : Take a Screenshot" + ); + assert_snapshot!( + check( + r#"binds { + Super+Mod { screenshot; } + }"#, + Action::Screenshot(true, None), + ), + @" Super : Take a Screenshot" + ); + } + + #[test] + fn test_collect_actions() { + // Release binds count as bound, both for the actions that are always listed and for the + // ones that are only listed when bound. + let config = Config::parse_mem( + r#"binds { + Alt { + release { screenshot; } + } + Mod { + release { spawn "foot"; } + } + }"#, + ) + .unwrap(); + + let actions = collect_actions(&config); + assert!(actions.contains(&&Action::Screenshot(true, None))); + assert!(actions.contains(&&Action::Spawn(vec![String::from("foot")]))); + + // Release binds keep actions in the list with hide-not-bound. + let config = Config::parse_mem( + r#"binds { + Mod { + release { toggle-overview; } + } + } + hotkey-overlay { hide-not-bound; }"#, + ) + .unwrap(); + + assert!(collect_actions(&config).contains(&&Action::ToggleOverview)); + + // A release bind with a custom title adds its action to the list. + let config = Config::parse_mem( + r#"binds { + Mod+Q hotkey-overlay-title="Custom" { + release { center-column; } + } + }"#, + ) + .unwrap(); + + assert!(collect_actions(&config).contains(&&Action::CenterColumn)); + + // Various filtering behavior. + let config = Config::parse_mem( + r#"binds { + XF86AudioMute { spawn "Mute"; } + Super_L { spawn "Super_L"; } + Mod { spawn "Mod"; } + Shift_L { spawn "Shift_L"; } + Alt { spawn "Alt"; } + Mod+Shift_L { spawn "Mod+Shift_L"; } + Super+Ctrl { spawn "Super+Ctrl"; } + Mod+Alt { spawn "Mod+Alt"; } + }"#, + ) + .unwrap(); + + let mut snapshot = String::new(); + for action in collect_actions(&config) { + if let Action::Spawn(args) = action { + snapshot.push_str(&args[0]); + snapshot.push('\n'); + } + } + assert_snapshot!(snapshot, @" + Super_L + Mod + Mod+Shift_L + Super+Ctrl + Mod+Alt + "); } } diff --git a/src/ui/mru.rs b/src/ui/mru.rs index 346a97b64f..6f265f7d22 100644 --- a/src/ui/mru.rs +++ b/src/ui/mru.rs @@ -7,8 +7,8 @@ use std::time::Duration; use anyhow::ensure; use niri_config::{ - Action, Bind, Color, Config, CornerRadius, GradientInterpolation, Key, Modifiers, MruDirection, - MruFilter, MruScope, Trigger, + Action, Bind, BoundAction, Color, Config, CornerRadius, GradientInterpolation, Key, Modifiers, + MruDirection, MruFilter, MruScope, Trigger, }; use pango::FontDescription; use pangocairo::cairo::{self, ImageSurface}; @@ -1855,8 +1855,9 @@ fn make_preset_opened_binds() -> Vec { // The modifier is filled dynamically. modifiers: Modifiers::empty(), }, - actions: vec![action], + action: BoundAction::Press(vec![action]), repeat: true, + pass_through: None, cooldown: None, allow_when_locked: false, allow_inhibiting: false, @@ -1903,12 +1904,20 @@ fn make_dynamic_opened_binds(config: &Config) -> Vec { let mut binds: HashMap> = HashMap::new(); for bind in &config.binds.0 { - // Multi-action binds don't map onto a single MRU action. - let [bind_action] = bind.actions.as_slice() else { + // Release-only and multi-action binds don't map onto a single MRU action. + let [press_action] = bind.press_actions() else { continue; }; - let action = match bind_action { + // The idea with dynamic binds is that their modifiers are ignored + // (set to whatever the user is currently holding). This idea does not + // make sense with order-independent generic-modifier-only binds. + // Effectively, they would trigger on every key press. + if bind.key.trigger.is_generic_modifier() { + continue; + } + + let action = match press_action { Action::FocusColumnRight | Action::FocusColumnRightOrFirst | Action::FocusColumnOrMonitorRight @@ -1933,7 +1942,7 @@ fn make_dynamic_opened_binds(config: &Config) -> Vec { }; binds.entry(bind.key.trigger).or_default().push(Bind { - actions: vec![action], + action: BoundAction::Press(vec![action]), ..bind.clone() }); } diff --git a/src/ui/screenshot_ui.rs b/src/ui/screenshot_ui.rs index a1586361f0..aebce2087a 100644 --- a/src/ui/screenshot_ui.rs +++ b/src/ui/screenshot_ui.rs @@ -4,10 +4,11 @@ use std::collections::HashMap; use std::f64::consts::TAU; use std::iter::zip; use std::rc::Rc; +use std::sync::LazyLock; use anyhow::Context; use arrayvec::ArrayVec; -use niri_config::{Action, Config}; +use niri_config::{Action, Bind, BoundAction, Config, Key, Modifiers, Trigger}; use niri_ipc::SizeChange; use pango::{Alignment, FontDescription}; use pangocairo::cairo::{self, ImageSurface}; @@ -17,7 +18,7 @@ use smithay::backend::renderer::element::utils::{Relocate, RelocateRenderElement use smithay::backend::renderer::element::Kind; use smithay::backend::renderer::gles::{GlesRenderer, GlesTexture}; use smithay::backend::renderer::{ExportMem, Texture as _}; -use smithay::input::keyboard::{Keysym, ModifiersState}; +use smithay::input::keyboard::Keysym; use smithay::output::{Output, WeakOutput}; use smithay::utils::{Buffer, Physical, Point, Rectangle, Scale, Size, Transform}; @@ -764,17 +765,82 @@ impl ScreenshotUi { Ok((rect.size, copy.to_vec())) } - pub fn action(&self, raw: Keysym, mods: ModifiersState) -> Option { + pub fn binds(&self) -> impl Iterator + Clone { + fn bind(key: Key, action: Action) -> Bind { + Bind { + key, + action: BoundAction::Press(vec![action]), + repeat: false, // Not useful for these actions. + pass_through: None, + cooldown: None, + allow_when_locked: true, + allow_inhibiting: false, + hotkey_overlay_title: None, + } + } + + static BINDS: LazyLock<[Bind; 4]> = LazyLock::new(|| { + [ + bind( + Key { + trigger: Trigger::Keysym(Keysym::Escape), + modifiers: Modifiers::empty(), + }, + Action::CancelScreenshot, + ), + bind( + Key { + trigger: Trigger::Keysym(Keysym::Return), + modifiers: Modifiers::empty(), + }, + Action::ConfirmScreenshot { + write_to_disk: true, + }, + ), + bind( + Key { + trigger: Trigger::Keysym(Keysym::c), + modifiers: Modifiers::CTRL, + }, + Action::ConfirmScreenshot { + write_to_disk: false, + }, + ), + bind( + Key { + trigger: Trigger::Keysym(Keysym::p), + modifiers: Modifiers::empty(), + }, + Action::ScreenshotTogglePointer, + ), + ] + }); + + static SPACE: LazyLock<[Bind; 1]> = LazyLock::new(|| { + [bind( + Key { + trigger: Trigger::Keysym(Keysym::space), + modifiers: Modifiers::empty(), + }, + Action::ConfirmScreenshot { + write_to_disk: true, + }, + )] + }); + + // A bit of gymnastics to return an iterator over static &Bind values. let Self::Open { button, .. } = self else { - return None; + return [].iter().chain([].iter()); }; // Pressing Space while the button is down goes into origin moving rather than capture. - if matches!(button, Button::Down { .. }) && raw == Keysym::space { - return None; - } + let space: &[Bind] = if matches!(button, Button::Down { .. }) { + &[] + } else { + &*SPACE + }; - action(raw, mods) + space.iter().chain(BINDS.iter()) } pub fn selection_output(&self) -> Option<&Output> { @@ -1068,33 +1134,6 @@ impl OutputScreenshot { } } -fn action(raw: Keysym, mods: ModifiersState) -> Option { - if raw == Keysym::Escape { - return Some(Action::CancelScreenshot); - } - - if mods.alt || mods.shift { - return None; - } - - if !mods.ctrl && (raw == Keysym::space || raw == Keysym::Return) { - return Some(Action::ConfirmScreenshot { - write_to_disk: true, - }); - } - if mods.ctrl && raw == Keysym::c { - return Some(Action::ConfirmScreenshot { - write_to_disk: false, - }); - } - - if !mods.ctrl && raw == Keysym::p { - return Some(Action::ScreenshotTogglePointer); - } - - None -} - pub fn rect_from_corner_points( a: Point, b: Point,