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
This commit is contained in:
@@ -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",
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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},
|
||||
}
|
||||
|
||||
|
||||
|
||||
@@ -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())
|
||||
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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."""
|
||||
|
||||
Reference in New Issue
Block a user