From 80108c345699534786f393ccf453096e16dfb3d1 Mon Sep 17 00:00:00 2001 From: Jatin K Malik Date: Sat, 14 Mar 2026 12:37:51 -0700 Subject: [PATCH] feat(shortcuts): add left/right modifier key distinction for keyboard shortcuts (#308) Allow users to bind voice commands to specific modifier key sides (e.g., Right Ctrl only), preventing conflicts with common left-side shortcuts like Alt+Tab or Ctrl+C. - Expand SUPPORTED_SHORTCUTS from 3 to 9 entries (either/left/right) - Add SHORTCUT_GROUPS for grouped display in settings UI - Implement side-aware key matching in pynput and evdev backends - Update settings dialog with grouped shortcut selector - Add comprehensive tests for side-specific shortcuts Closes #306 --- .../ui/keyboard_backends/__init__.py | 2 + src/vocalinux/ui/keyboard_backends/base.py | 54 ++++++++++++++- .../ui/keyboard_backends/evdev_backend.py | 6 ++ .../ui/keyboard_backends/pynput_backend.py | 53 +++++++++----- src/vocalinux/ui/settings_dialog.py | 19 ++++- tests/test_keyboard_shortcuts.py | 69 ++++++++++++++++++- tests/test_settings_shortcuts.py | 32 ++++++++- 7 files changed, 208 insertions(+), 27 deletions(-) diff --git a/src/vocalinux/ui/keyboard_backends/__init__.py b/src/vocalinux/ui/keyboard_backends/__init__.py index 6224c18..9b11f2e 100644 --- a/src/vocalinux/ui/keyboard_backends/__init__.py +++ b/src/vocalinux/ui/keyboard_backends/__init__.py @@ -13,6 +13,7 @@ from .base import ( DEFAULT_SHORTCUT, DEFAULT_SHORTCUT_MODE, SHORTCUT_DISPLAY_NAMES, + SHORTCUT_GROUPS, SHORTCUT_MODE_DISPLAY_NAMES, SHORTCUT_MODES, SUPPORTED_SHORTCUTS, @@ -165,6 +166,7 @@ __all__ = [ "PYNPUT_AVAILABLE", "SUPPORTED_SHORTCUTS", "SHORTCUT_DISPLAY_NAMES", + "SHORTCUT_GROUPS", "SHORTCUT_MODES", "DEFAULT_SHORTCUT", "DEFAULT_SHORTCUT_MODE", diff --git a/src/vocalinux/ui/keyboard_backends/base.py b/src/vocalinux/ui/keyboard_backends/base.py index 2026d06..8761196 100644 --- a/src/vocalinux/ui/keyboard_backends/base.py +++ b/src/vocalinux/ui/keyboard_backends/base.py @@ -11,17 +11,41 @@ from typing import Callable, Optional Callback = Callable[[], None] # Supported double-tap shortcut keys +# Maps shortcut string -> modifier key name used by backends +# "ctrl" means either side; "left_ctrl"/"right_ctrl" means specific side SUPPORTED_SHORTCUTS = { + # Either side (backward-compatible) "ctrl+ctrl": "ctrl", "alt+alt": "alt", "shift+shift": "shift", + # Left side only + "left_ctrl+left_ctrl": "left_ctrl", + "left_alt+left_alt": "left_alt", + "left_shift+left_shift": "left_shift", + # Right side only + "right_ctrl+right_ctrl": "right_ctrl", + "right_alt+right_alt": "right_alt", + "right_shift+right_shift": "right_shift", } # Human-readable names for shortcuts (mode-agnostic base names) SHORTCUT_DISPLAY_NAMES = { - "ctrl+ctrl": "Ctrl", - "alt+alt": "Alt", - "shift+shift": "Shift", + "ctrl+ctrl": "Ctrl (either side)", + "alt+alt": "Alt (either side)", + "shift+shift": "Shift (either side)", + "left_ctrl+left_ctrl": "Left Ctrl", + "left_alt+left_alt": "Left Alt", + "left_shift+left_shift": "Left Shift", + "right_ctrl+right_ctrl": "Right Ctrl", + "right_alt+right_alt": "Right Alt", + "right_shift+right_shift": "Right Shift", +} + +# Grouping for UI display: maps group label -> list of shortcut IDs +SHORTCUT_GROUPS = { + "Either Side": ["ctrl+ctrl", "alt+alt", "shift+shift"], + "Left Side": ["left_ctrl+left_ctrl", "left_alt+left_alt", "left_shift+left_shift"], + "Right Side": ["right_ctrl+right_ctrl", "right_alt+right_alt", "right_shift+right_shift"], } # Mode-specific display names (format: {shortcut: {mode: display_name}}) @@ -38,6 +62,30 @@ SHORTCUT_MODE_DISPLAY_NAMES = { "toggle": "Double-tap Shift", "push_to_talk": "Hold Shift", }, + "left_ctrl+left_ctrl": { + "toggle": "Double-tap Left Ctrl", + "push_to_talk": "Hold Left Ctrl", + }, + "left_alt+left_alt": { + "toggle": "Double-tap Left Alt", + "push_to_talk": "Hold Left Alt", + }, + "left_shift+left_shift": { + "toggle": "Double-tap Left Shift", + "push_to_talk": "Hold Left Shift", + }, + "right_ctrl+right_ctrl": { + "toggle": "Double-tap Right Ctrl", + "push_to_talk": "Hold Right Ctrl", + }, + "right_alt+right_alt": { + "toggle": "Double-tap Right Alt", + "push_to_talk": "Hold Right Alt", + }, + "right_shift+right_shift": { + "toggle": "Double-tap Right Shift", + "push_to_talk": "Hold Right Shift", + }, } DEFAULT_SHORTCUT = "ctrl+ctrl" diff --git a/src/vocalinux/ui/keyboard_backends/evdev_backend.py b/src/vocalinux/ui/keyboard_backends/evdev_backend.py index de87d47..7bdff5c 100644 --- a/src/vocalinux/ui/keyboard_backends/evdev_backend.py +++ b/src/vocalinux/ui/keyboard_backends/evdev_backend.py @@ -46,6 +46,12 @@ MODIFIER_KEY_CODES: Dict[str, Set[int]] = { "alt": {KEY_LEFTALT, KEY_RIGHTALT}, "shift": {KEY_LEFTSHIFT, KEY_RIGHTSHIFT}, "super": {KEY_LEFTMETA, KEY_RIGHTMETA}, + "left_ctrl": {KEY_LEFTCTRL}, + "left_alt": {KEY_LEFTALT}, + "left_shift": {KEY_LEFTSHIFT}, + "right_ctrl": {KEY_RIGHTCTRL}, + "right_alt": {KEY_RIGHTALT}, + "right_shift": {KEY_RIGHTSHIFT}, } diff --git a/src/vocalinux/ui/keyboard_backends/pynput_backend.py b/src/vocalinux/ui/keyboard_backends/pynput_backend.py index c76dabf..9ce221f 100644 --- a/src/vocalinux/ui/keyboard_backends/pynput_backend.py +++ b/src/vocalinux/ui/keyboard_backends/pynput_backend.py @@ -24,17 +24,22 @@ from .base import DEFAULT_SHORTCUT, DEFAULT_SHORTCUT_MODE, KeyboardBackend logger = logging.getLogger(__name__) -# Map modifier key names to pynput Key objects -MODIFIER_KEY_MAP = { - "ctrl": keyboard.Key.ctrl if PYNPUT_AVAILABLE else None, - "alt": keyboard.Key.alt if PYNPUT_AVAILABLE else None, - "shift": keyboard.Key.shift if PYNPUT_AVAILABLE else None, - "super": keyboard.Key.cmd if PYNPUT_AVAILABLE else None, # cmd is Super/Windows key -} - -# Map for normalizing left/right variants +MODIFIER_KEY_MAP = {} MODIFIER_NORMALIZE_MAP = {} if PYNPUT_AVAILABLE: + MODIFIER_KEY_MAP = { + "ctrl": keyboard.Key.ctrl, + "alt": keyboard.Key.alt, + "shift": keyboard.Key.shift, + "super": keyboard.Key.cmd, + "left_ctrl": keyboard.Key.ctrl_l, + "left_alt": keyboard.Key.alt_l, + "left_shift": keyboard.Key.shift_l, + "right_ctrl": keyboard.Key.ctrl_r, + "right_alt": keyboard.Key.alt_r, + "right_shift": keyboard.Key.shift_r, + } + MODIFIER_NORMALIZE_MAP = { keyboard.Key.ctrl_l: keyboard.Key.ctrl, keyboard.Key.ctrl_r: keyboard.Key.ctrl, @@ -146,14 +151,19 @@ class PynputKeyboardBackend(KeyboardBackend): def _on_press(self, key) -> None: """Handle key press events.""" try: - normalized_key = self._normalize_modifier_key(key) target_key = self._get_target_key() + is_side_specific = self._modifier_key.startswith(("left_", "right_")) - if normalized_key == target_key: + if is_side_specific: + matched = key == target_key + else: + normalized_key = self._normalize_modifier_key(key) + matched = normalized_key == target_key + + if matched: current_time = time.time() if self._mode == "toggle": - # Check for double-tap if ( current_time - self.last_key_press_time < self.double_tap_threshold and self.double_tap_callback is not None @@ -163,15 +173,14 @@ class PynputKeyboardBackend(KeyboardBackend): self.last_trigger_time = current_time threading.Thread(target=self.double_tap_callback, daemon=True).start() elif self._mode == "push_to_talk": - # Trigger on press if self.key_press_callback is not None: logger.debug(f"Key press {self._modifier_key} detected (pynput)") threading.Thread(target=self.key_press_callback, daemon=True).start() self.last_key_press_time = current_time - # Track current keys (using target key for reference) if target_key and key in self._get_key_variants(self._modifier_key): + normalized_key = self._normalize_modifier_key(key) self.current_keys.add(normalized_key) except Exception as e: @@ -182,12 +191,16 @@ class PynputKeyboardBackend(KeyboardBackend): try: normalized_key = self._normalize_modifier_key(key) target_key = self._get_target_key() + is_side_specific = self._modifier_key.startswith(("left_", "right_")) - # Track current keys self.current_keys.discard(normalized_key) - # Handle push-to-talk release - if self._mode == "push_to_talk" and normalized_key == target_key: + if is_side_specific: + matched = key == target_key + else: + matched = normalized_key == target_key + + if self._mode == "push_to_talk" and matched: if self.key_release_callback is not None: logger.debug(f"Key release {self._modifier_key} detected (pynput)") threading.Thread(target=self.key_release_callback, daemon=True).start() @@ -209,6 +222,12 @@ class PynputKeyboardBackend(KeyboardBackend): "alt": {keyboard.Key.alt, keyboard.Key.alt_l, keyboard.Key.alt_r}, "shift": {keyboard.Key.shift, keyboard.Key.shift_l, keyboard.Key.shift_r}, "super": {keyboard.Key.cmd, keyboard.Key.cmd_l, keyboard.Key.cmd_r}, + "left_ctrl": {keyboard.Key.ctrl_l}, + "left_alt": {keyboard.Key.alt_l}, + "left_shift": {keyboard.Key.shift_l}, + "right_ctrl": {keyboard.Key.ctrl_r}, + "right_alt": {keyboard.Key.alt_r}, + "right_shift": {keyboard.Key.shift_r}, } return variants.get(modifier_name, set()) diff --git a/src/vocalinux/ui/settings_dialog.py b/src/vocalinux/ui/settings_dialog.py index 26dfa2a..2f0e1cc 100644 --- a/src/vocalinux/ui/settings_dialog.py +++ b/src/vocalinux/ui/settings_dialog.py @@ -38,6 +38,7 @@ from ..utils.whispercpp_model_info import get_recommended_model as get_recommend from ..utils.whispercpp_model_info import is_model_downloaded as is_whispercpp_model_downloaded from .keyboard_backends import ( # noqa: E402 SHORTCUT_DISPLAY_NAMES, + SHORTCUT_GROUPS, SHORTCUT_MODES, SUPPORTED_SHORTCUTS, ) @@ -1149,9 +1150,14 @@ class SettingsDialog(Gtk.Dialog): self.shortcut_combo.set_tooltip_text("Select the keyboard shortcut for voice typing") _prevent_scroll_on_hover(self.shortcut_combo) - # Populate shortcut options - for shortcut_id, display_name in SHORTCUT_DISPLAY_NAMES.items(): - self.shortcut_combo.append(shortcut_id, display_name) + # Populate shortcut options grouped by side + for group_label, shortcut_ids in SHORTCUT_GROUPS.items(): + # Add group separator as a disabled label entry + separator_id = f"__separator_{group_label}__" + self.shortcut_combo.append(separator_id, f"── {group_label} ──") + for shortcut_id in shortcut_ids: + display_name = SHORTCUT_DISPLAY_NAMES.get(shortcut_id, shortcut_id) + self.shortcut_combo.append(shortcut_id, display_name) # Load current shortcut from config current_shortcut = self.config_manager.get("shortcuts", "toggle_recognition", "ctrl+ctrl") @@ -1255,6 +1261,13 @@ class SettingsDialog(Gtk.Dialog): if not shortcut_id: return + # Ignore separator entries (used for grouped display) + if shortcut_id.startswith("__separator_"): + # Revert to the previously saved shortcut + current = self.config_manager.get("shortcuts", "toggle_recognition", "ctrl+ctrl") + self.shortcut_combo.set_active_id(current) + return + # Save to config self.config_manager.set("shortcuts", "toggle_recognition", shortcut_id) self.config_manager.save_settings() diff --git a/tests/test_keyboard_shortcuts.py b/tests/test_keyboard_shortcuts.py index 58ec044..ed644aa 100644 --- a/tests/test_keyboard_shortcuts.py +++ b/tests/test_keyboard_shortcuts.py @@ -63,10 +63,38 @@ class TestKeyboardShortcuts(unittest.TestCase): def test_supported_shortcuts(self): """Test that all expected shortcuts are supported.""" - expected_shortcuts = ["ctrl+ctrl", "alt+alt", "shift+shift"] + expected_shortcuts = [ + "ctrl+ctrl", + "alt+alt", + "shift+shift", + "left_ctrl+left_ctrl", + "left_alt+left_alt", + "left_shift+left_shift", + "right_ctrl+right_ctrl", + "right_alt+right_alt", + "right_shift+right_shift", + ] for shortcut in expected_shortcuts: self.assertIn(shortcut, SUPPORTED_SHORTCUTS) + def test_pynput_backend_custom_shortcut_right_side(self): + """Test pynput backend with a right-side shortcut.""" + from vocalinux.ui.keyboard_backends.pynput_backend import PynputKeyboardBackend + + backend = PynputKeyboardBackend(shortcut="right_ctrl+right_ctrl") + self.assertEqual(backend.shortcut, "right_ctrl+right_ctrl") + self.assertEqual(backend.modifier_key, "right_ctrl") + + def test_pynput_backend_set_shortcut_left_alt(self): + """Test changing to a left-side shortcut and verify modifier_key.""" + from vocalinux.ui.keyboard_backends.pynput_backend import PynputKeyboardBackend + + backend = PynputKeyboardBackend() + backend.set_shortcut("left_alt+left_alt") + + self.assertEqual(backend.shortcut, "left_alt+left_alt") + self.assertEqual(backend.modifier_key, "left_alt") + def test_shortcut_display_names(self): """Test that all shortcuts have display names.""" for shortcut in SUPPORTED_SHORTCUTS: @@ -413,6 +441,14 @@ class TestPynputBackend(unittest.TestCase): self.assertEqual(backend.shortcut, "alt+alt") self.assertEqual(backend.modifier_key, "alt") + def test_pynput_backend_custom_shortcut_right_side(self): + """Test pynput backend with right-side specific shortcut.""" + from vocalinux.ui.keyboard_backends.pynput_backend import PynputKeyboardBackend + + backend = PynputKeyboardBackend(shortcut="right_ctrl+right_ctrl") + self.assertEqual(backend.shortcut, "right_ctrl+right_ctrl") + self.assertEqual(backend.modifier_key, "right_ctrl") + def test_pynput_backend_set_shortcut(self): """Test changing shortcut on pynput backend.""" from vocalinux.ui.keyboard_backends.pynput_backend import PynputKeyboardBackend @@ -423,6 +459,16 @@ class TestPynputBackend(unittest.TestCase): self.assertEqual(backend.shortcut, "shift+shift") self.assertEqual(backend.modifier_key, "shift") + def test_pynput_backend_set_shortcut_left_alt(self): + """Test changing to a left-side specific shortcut.""" + from vocalinux.ui.keyboard_backends.pynput_backend import PynputKeyboardBackend + + backend = PynputKeyboardBackend() + backend.set_shortcut("left_alt+left_alt") + + self.assertEqual(backend.shortcut, "left_alt+left_alt") + self.assertEqual(backend.modifier_key, "left_alt") + class TestEvdevBackend(unittest.TestCase): """Test cases for the evdev backend.""" @@ -626,6 +672,27 @@ class TestShortcutParseFunction(unittest.TestCase): with self.assertRaises(ValueError): parse_shortcut("") + def test_parse_shortcut_left_ctrl(self): + """Test parsing left_ctrl+left_ctrl shortcut.""" + from vocalinux.ui.keyboard_backends.base import parse_shortcut + + result = parse_shortcut("left_ctrl+left_ctrl") + self.assertEqual(result, "left_ctrl") + + def test_parse_shortcut_right_alt(self): + """Test parsing right_alt+right_alt shortcut.""" + from vocalinux.ui.keyboard_backends.base import parse_shortcut + + result = parse_shortcut("right_alt+right_alt") + self.assertEqual(result, "right_alt") + + def test_parse_shortcut_left_shift(self): + """Test parsing left_shift+left_shift shortcut.""" + from vocalinux.ui.keyboard_backends.base import parse_shortcut + + result = parse_shortcut("left_shift+left_shift") + self.assertEqual(result, "left_shift") + if __name__ == "__main__": unittest.main() diff --git a/tests/test_settings_shortcuts.py b/tests/test_settings_shortcuts.py index 0a5f125..40045d3 100644 --- a/tests/test_settings_shortcuts.py +++ b/tests/test_settings_shortcuts.py @@ -51,10 +51,11 @@ class TestSettingsDialogShortcutsSection(unittest.TestCase): def test_shortcut_options_populated(self): """Test that shortcut options are populated from SHORTCUT_DISPLAY_NAMES.""" + self.assertIn("for group_label, shortcut_ids in SHORTCUT_GROUPS.items()", self.source_code) self.assertIn( - "for shortcut_id, display_name in SHORTCUT_DISPLAY_NAMES.items()", self.source_code + 'self.shortcut_combo.append(separator_id, f"── {group_label} ──")', self.source_code ) - self.assertIn("self.shortcut_combo.append(shortcut_id, display_name)", self.source_code) + self.assertIn("for shortcut_id in shortcut_ids", self.source_code) def test_shortcut_config_read(self): """Test that shortcut is read from config.""" @@ -111,6 +112,7 @@ class TestSettingsDialogShortcutsSection(unittest.TestCase): # Check for imports (may be multi-line) self.assertIn("from .keyboard_backends import", self.source_code) self.assertIn("SHORTCUT_DISPLAY_NAMES", self.source_code) + self.assertIn("SHORTCUT_GROUPS", self.source_code) self.assertIn("SUPPORTED_SHORTCUTS", self.source_code) @@ -133,7 +135,17 @@ class TestKeyboardBackendsBase(unittest.TestCase): self.assertIsInstance(SHORTCUT_DISPLAY_NAMES, dict) self.assertEqual( set(SHORTCUT_DISPLAY_NAMES.keys()), - {"ctrl+ctrl", "alt+alt", "shift+shift"}, + { + "ctrl+ctrl", + "alt+alt", + "shift+shift", + "left_ctrl+left_ctrl", + "left_alt+left_alt", + "left_shift+left_shift", + "right_ctrl+right_ctrl", + "right_alt+right_alt", + "right_shift+right_shift", + }, ) def test_default_shortcut_defined(self): @@ -151,6 +163,20 @@ class TestKeyboardBackendsBase(unittest.TestCase): self.assertTrue(hasattr(KeyboardBackend, "modifier_key")) self.assertTrue(hasattr(KeyboardBackend, "set_shortcut")) + def test_shortcut_groups_defined(self): + """Test that SHORTCUT_GROUPS is defined with 3 groups and correct keys.""" + from vocalinux.ui.keyboard_backends.base import SHORTCUT_GROUPS + + self.assertIsInstance(SHORTCUT_GROUPS, dict) + self.assertIn("Either Side", SHORTCUT_GROUPS) + self.assertIn("Left Side", SHORTCUT_GROUPS) + self.assertIn("Right Side", SHORTCUT_GROUPS) + self.assertEqual(len(SHORTCUT_GROUPS.keys()), 3) + self.assertEqual(list(SHORTCUT_GROUPS.keys()), ["Either Side", "Left Side", "Right Side"]) + self.assertEqual(len(SHORTCUT_GROUPS["Either Side"]), 3) + self.assertEqual(len(SHORTCUT_GROUPS["Left Side"]), 3) + self.assertEqual(len(SHORTCUT_GROUPS["Right Side"]), 3) + class TestConfigManagerShortcuts(unittest.TestCase): """Test cases for shortcut configuration in config manager."""