Skip to content

Fix keyboard shortcuts broken by NumLock/CapsLock#1918

Merged
christianparpart merged 2 commits intomasterfrom
fix/1901-numlock-keybindings
Mar 24, 2026
Merged

Fix keyboard shortcuts broken by NumLock/CapsLock#1918
christianparpart merged 2 commits intomasterfrom
fix/1901-numlock-keybindings

Conversation

@christianparpart
Copy link
Copy Markdown
Member

@christianparpart christianparpart commented Mar 21, 2026

Summary

Fixes #1901.

  • Strip lock modifiers (NumLock, CapsLock) from incoming key events before matching against configured key bindings in config::apply()
  • Promote LockModifiers constant from InputGenerator.cpp (file-local) to InputGenerator.h (shared) so it can be reused across binding matching and legacy input generation
  • Add 4 new test cases verifying lock-modifier stripping behavior

Root cause

The Modifier enum includes NumLock=128 and CapsLock=64 as first-class bit flags. Key binding matching in config::apply() used exact equality (mapping.modifiers == modifiers), so Shift|Control|NumLockShift|Control and the binding would silently fail. The VT input generator already stripped lock modifiers — this fix applies the same treatment to the shortcut matching path.

Risk assessment

Low risk. The change is a single bitwise AND-NOT on a uint8_t before comparison. When no lock keys are active, modifiers.without(LockModifiers) is a no-op. No behavior change for users without lock keys active.

Test plan

  • All existing tests pass (ctest --preset=clang-debug — 5/5)
  • New tests: InputBinding.match_with_NumLock_stripped, InputBinding.match_with_CapsLock_stripped, InputBinding.match_with_NumLock_and_CapsLock_stripped, InputBinding.match_still_requires_real_modifiers
  • Manual: With NumLock on, verify Ctrl+Shift+N triggers NewTerminal

@github-actions github-actions bot added VT: Backend Virtual Terminal Backend (libterminal API) frontend Contour Terminal Emulator (GUI frontend) test Unit tests labels Mar 21, 2026
Strip lock modifiers (NumLock, CapsLock) before matching key bindings
in config::apply(), so user-configured shortcuts work regardless of
lock-key state. Promote LockModifiers constant to InputGenerator.h
for shared use across binding matching and legacy input generation.

Signed-off-by: Christian Parpart <christian@parpart.family>
@christianparpart christianparpart added the no changelog Tells the CI to not require a changelog entry label Mar 21, 2026
@christianparpart christianparpart self-assigned this Mar 21, 2026
@christianparpart christianparpart force-pushed the fix/1901-numlock-keybindings branch from 4da2aee to b8f8536 Compare March 21, 2026 17:07
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes issue #1901 where keyboard shortcuts could fail to match when NumLock/CapsLock were active by stripping lock modifiers from incoming modifier masks before comparing against configured input bindings.

Changes:

  • Promote LockModifiers to InputGenerator.h for shared reuse.
  • Strip LockModifiers in contour::config::apply() before matching bindings.
  • Add new unit tests intended to cover lock-modifier stripping behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/vtbackend/InputGenerator.h Exposes LockModifiers as a shared constexpr for reuse across components.
src/vtbackend/InputGenerator.cpp Removes file-local LockModifiers and relies on the shared header constant.
src/contour/Config.h Strips lock modifiers before comparing mapping.modifiers against event modifiers in apply().
src/vtbackend/InputGenerator_test.cpp Adds new tests around lock-modifier behavior (currently not directly exercising config::apply()).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- InputGenerator_test.cpp: Add CHECK_FALSE assertions with raw (unstripped)
  lock modifiers to demonstrate the bug alongside the fix (per @Copilot)

Signed-off-by: Christian Parpart <christian@parpart.family>
@christianparpart christianparpart merged commit 603c589 into master Mar 24, 2026
32 checks passed
@christianparpart christianparpart deleted the fix/1901-numlock-keybindings branch March 24, 2026 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend Contour Terminal Emulator (GUI frontend) no changelog Tells the CI to not require a changelog entry test Unit tests VT: Backend Virtual Terminal Backend (libterminal API)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

keyboard shortcuts confused by numlock

2 participants