From 55705ce53061d365cc71e4ad090efb6530bbd64f Mon Sep 17 00:00:00 2001 From: Xander <1174877+xtrasmal@users.noreply.github.com> Date: Mon, 2 Feb 2026 14:24:23 +0100 Subject: [PATCH 01/12] feat(keybindings): add file-based custom keymap configuration Implements user-facing JSON keymap configuration following the ThemeManager pattern. Users can now customize all keybindings by creating JSON files in ~/.sqlit/keymaps/ and setting custom_keymap in settings.json. Features: - KeymapManager class for loading/validating custom keymaps - FileBasedKeymapProvider implementing KeymapProvider interface - JSON structure with leader_commands and action_keys arrays - Graceful error handling (invalid configs don't crash app) - Comprehensive test suite (11 tests covering all scenarios) - User documentation with examples and troubleshooting - Template keymap showing all default bindings Integration: - Loads during app startup after theme initialization - Settings key: custom_keymap (default/name) - Directory: ~/.sqlit/keymaps/ - Follows established sqlit configuration patterns Files: - sqlit/core/keymap_manager.py: Manager implementation - config/keymap.template.json: User template with examples - docs/custom-keymaps.md: Complete user guide - tests/ui/keybindings/test_keymap_manager.py: Test coverage This enables power users to customize keybindings per CONTRIBUTING.md line 243 which explicitly permits keyboard binding customization. --- config/keymap.template.json | 234 +++++++++++++++ config/settings.template.json | 1 + docs/custom-keymaps.md | 283 ++++++++++++++++++ sqlit/core/__init__.py | 2 + sqlit/core/keymap_manager.py | 304 ++++++++++++++++++++ sqlit/domains/shell/app/main.py | 4 + sqlit/domains/shell/app/startup_flow.py | 3 + tests/ui/keybindings/test_keymap_manager.py | 291 +++++++++++++++++++ 8 files changed, 1122 insertions(+) create mode 100644 config/keymap.template.json create mode 100644 docs/custom-keymaps.md create mode 100644 sqlit/core/keymap_manager.py create mode 100644 tests/ui/keybindings/test_keymap_manager.py diff --git a/config/keymap.template.json b/config/keymap.template.json new file mode 100644 index 00000000..86a35311 --- /dev/null +++ b/config/keymap.template.json @@ -0,0 +1,234 @@ +{ + "_note": "Copy to ~/.sqlit/keymaps/my-custom.json and customize keybindings", + "_usage": "Set 'custom_keymap' in settings.json to 'my-custom' (without .json extension)", + "keymap": { + "leader_commands": [ + { + "_comment": "Leader commands are triggered by pressing the leader key (default: space) followed by another key", + "key": "q", + "action": "quit", + "label": "Quit", + "category": "Actions", + "guard": null, + "menu": "leader" + }, + { + "key": "h", + "action": "show_help", + "label": "Help", + "category": "Actions", + "menu": "leader" + }, + { + "key": "e", + "action": "toggle_explorer", + "label": "Toggle Explorer", + "category": "View", + "menu": "leader" + }, + { + "key": "f", + "action": "toggle_fullscreen", + "label": "Toggle Maximize", + "category": "View", + "menu": "leader" + }, + { + "key": "c", + "action": "show_connection_picker", + "label": "Connect", + "category": "Connection", + "menu": "leader" + }, + { + "key": "x", + "action": "disconnect", + "label": "Disconnect", + "category": "Connection", + "guard": "has_connection", + "menu": "leader" + }, + { + "key": "z", + "action": "cancel_operation", + "label": "Cancel", + "category": "Actions", + "guard": "query_executing", + "menu": "leader" + }, + { + "key": "t", + "action": "change_theme", + "label": "Change Theme", + "category": "Actions", + "menu": "leader" + }, + { + "key": "space", + "action": "telescope", + "label": "Telescope", + "category": "Actions", + "menu": "leader" + }, + { + "key": "slash", + "action": "telescope_filter", + "label": "Telescope Search", + "category": "Actions", + "menu": "leader" + } + ], + "action_keys": [ + { + "_comment": "Action keys are direct keybindings that work in specific contexts", + "key": "space", + "action": "leader_key", + "context": "global", + "guard": null, + "primary": true, + "show": false, + "priority": true + }, + { + "key": "ctrl+q", + "action": "quit", + "context": "global", + "primary": true, + "show": false, + "priority": false + }, + { + "key": "escape", + "action": "cancel_operation", + "context": "global", + "primary": true, + "show": false, + "priority": false + }, + { + "key": "question_mark", + "action": "show_help", + "context": "global", + "primary": true, + "show": false, + "priority": false + }, + { + "_comment": "Query editor keybindings (normal mode)", + "key": "i", + "action": "enter_insert_mode", + "context": "query_normal", + "primary": true, + "show": false, + "priority": false + }, + { + "key": "I", + "action": "prepend_insert_mode", + "context": "query_normal", + "primary": true, + "show": false, + "priority": false + }, + { + "key": "o", + "action": "open_line_below", + "context": "query_normal", + "primary": true, + "show": false, + "priority": false + }, + { + "key": "O", + "action": "open_line_above", + "context": "query_normal", + "primary": true, + "show": false, + "priority": false + }, + { + "key": "enter", + "action": "execute_query", + "context": "query_normal", + "primary": true, + "show": false, + "priority": false + }, + { + "_comment": "Vim cursor movement (normal mode)", + "key": "h", + "action": "cursor_left", + "context": "query_normal", + "primary": true, + "show": false, + "priority": false + }, + { + "key": "j", + "action": "cursor_down", + "context": "query_normal", + "primary": true, + "show": false, + "priority": false + }, + { + "key": "k", + "action": "cursor_up", + "context": "query_normal", + "primary": true, + "show": false, + "priority": false + }, + { + "key": "l", + "action": "cursor_right", + "context": "query_normal", + "primary": true, + "show": false, + "priority": false + }, + { + "_comment": "Query editor keybindings (insert mode)", + "key": "escape", + "action": "exit_insert_mode", + "context": "query_insert", + "primary": true, + "show": false, + "priority": false + }, + { + "key": "ctrl+enter", + "action": "execute_query_insert", + "context": "query_insert", + "primary": true, + "show": false, + "priority": false + }, + { + "_comment": "Navigation between panes", + "key": "e", + "action": "focus_explorer", + "context": "navigation", + "primary": true, + "show": false, + "priority": false + }, + { + "key": "q", + "action": "focus_query", + "context": "navigation", + "primary": true, + "show": false, + "priority": false + }, + { + "key": "r", + "action": "focus_results", + "context": "navigation", + "primary": true, + "show": false, + "priority": false + } + ] + } +} diff --git a/config/settings.template.json b/config/settings.template.json index f09352c1..727ad1b0 100644 --- a/config/settings.template.json +++ b/config/settings.template.json @@ -2,6 +2,7 @@ "_note": "Copy to .sqlit/settings.json (gitignored) and run: sqlit --settings .sqlit/settings.json", "theme": "tokyo-night", "custom_themes": [], + "custom_keymap": "default", "expanded_nodes": [], "process_worker": true, "process_worker_warm_on_idle": true, diff --git a/docs/custom-keymaps.md b/docs/custom-keymaps.md new file mode 100644 index 00000000..6f3c548f --- /dev/null +++ b/docs/custom-keymaps.md @@ -0,0 +1,283 @@ +# Custom Keymaps + +sqlit supports custom keymaps, allowing you to customize all keyboard shortcuts to match your preferences. + +## Quick Start + +1. **Copy the template:** + ```bash + mkdir -p ~/.sqlit/keymaps + cp config/keymap.template.json ~/.sqlit/keymaps/my-custom.json + ``` + +2. **Edit the keymap:** + Open `~/.sqlit/keymaps/my-custom.json` and customize the keybindings. + +3. **Enable your custom keymap:** + Edit `~/.sqlit/settings.json` and set: + ```json + { + "custom_keymap": "my-custom" + } + ``` + Note: Use the filename without the `.json` extension. + +4. **Restart sqlit** to load your custom keymap. + +## Keymap Structure + +Custom keymaps are JSON files with two main sections: + +### Leader Commands + +Leader commands are triggered by pressing the leader key (default: `space`) followed by another key. + +```json +{ + "keymap": { + "leader_commands": [ + { + "key": "q", + "action": "quit", + "label": "Quit", + "category": "Actions", + "guard": null, + "menu": "leader" + } + ] + } +} +``` + +**Fields:** +- `key` (required): The key to press after the leader key +- `action` (required): The action to execute +- `label` (required): Display label in the help menu +- `category` (required): Category for grouping in help menu +- `guard` (optional): Condition that must be met (e.g., `"has_connection"`) +- `menu` (optional): Menu ID (default: `"leader"`) + +### Action Keys + +Action keys are direct keybindings that work in specific contexts. + +```json +{ + "keymap": { + "action_keys": [ + { + "key": "i", + "action": "enter_insert_mode", + "context": "query_normal", + "guard": null, + "primary": true, + "show": false, + "priority": false + } + ] + } +} +``` + +**Fields:** +- `key` (required): The key combination (e.g., `"i"`, `"ctrl+q"`, `"escape"`) +- `action` (required): The action to execute +- `context` (optional): Context where this binding is active (e.g., `"query_normal"`, `"tree"`, `"results"`) +- `guard` (optional): Condition that must be met +- `primary` (optional): Whether this is the primary binding for the action (default: `true`) +- `show` (optional): Whether to show in Textual's binding hints (default: `false`) +- `priority` (optional): Whether to give priority to this binding (default: `false`) + +## Special Key Names + +Some keys use special names in the keymap: + +- `space` - Space bar +- `escape` - Escape key +- `enter` - Enter/Return key +- `backspace` - Backspace key +- `delete` - Delete key +- `tab` - Tab key +- `question_mark` - ? key +- `slash` - / key +- `dollar_sign` - $ key +- `percent_sign` - % key +- `asterisk` - * key +- `ctrl+` - Ctrl key combinations (e.g., `"ctrl+q"`, `"ctrl+enter"`) +- `shift+` - Shift key combinations (e.g., `"shift+tab"`) + +## Common Actions + +### Global Actions +- `quit` - Exit sqlit +- `show_help` - Show help menu +- `cancel_operation` - Cancel current operation +- `leader_key` - Trigger leader menu + +### Navigation Actions +- `focus_explorer` - Focus the database explorer +- `focus_query` - Focus the query editor +- `focus_results` - Focus the results table + +### Connection Actions +- `show_connection_picker` - Show connection picker +- `disconnect` - Disconnect from database +- `new_connection` - Create new connection + +### Query Editor Actions (Normal Mode) +- `enter_insert_mode` - Enter insert mode +- `prepend_insert_mode` - Enter insert mode at line start +- `append_insert_mode` - Enter insert mode after cursor +- `append_line_end` - Enter insert mode at line end +- `open_line_below` - Open new line below and enter insert mode +- `open_line_above` - Open new line above and enter insert mode +- `execute_query` - Execute the query + +### Query Editor Actions (Insert Mode) +- `exit_insert_mode` - Return to normal mode +- `execute_query_insert` - Execute query from insert mode + +### Vim-style Navigation +- `cursor_left`, `cursor_down`, `cursor_up`, `cursor_right` - Move cursor +- `cursor_word_forward` - Move to next word +- `cursor_word_back` - Move to previous word +- `cursor_line_start` - Move to line start +- `cursor_line_end` - Move to line end +- `cursor_last_line` - Move to last line + +### Results Actions +- `view_cell` - View cell value +- `edit_cell` - Edit cell value +- `delete_row` - Delete row +- `results_yank_leader_key` - Trigger results copy menu +- `clear_results` - Clear results +- `results_filter` - Filter results + +## Contexts + +Action keys can be scoped to specific contexts: + +- `global` - Active everywhere +- `query_normal` - Query editor in normal mode +- `query_insert` - Query editor in insert mode +- `tree` - Database explorer tree +- `results` - Results table +- `navigation` - For navigation actions +- `autocomplete` - Autocomplete dropdown + +## Guards + +Guards are conditions that must be met for a keybinding to be active: + +- `has_connection` - Database is connected +- `query_executing` - Query is currently executing + +## Examples + +### Vim-style Leader Key + +Change the leader key from space to comma: + +```json +{ + "keymap": { + "action_keys": [ + { + "key": "comma", + "action": "leader_key", + "context": "global", + "primary": true, + "priority": true + } + ] + } +} +``` + +### Emacs-style Bindings + +Use Ctrl+X Ctrl+C to quit instead of `q`: + +```json +{ + "keymap": { + "leader_commands": [], + "action_keys": [ + { + "key": "ctrl+x", + "action": "leader_key", + "context": "global", + "primary": true, + "priority": true + } + ] + } +} +``` + +Then add a leader command: +```json +{ + "key": "ctrl+c", + "action": "quit", + "label": "Quit", + "category": "Actions", + "menu": "leader" +} +``` + +### Custom Query Execution + +Execute query with Ctrl+Enter in normal mode: + +```json +{ + "keymap": { + "action_keys": [ + { + "key": "ctrl+enter", + "action": "execute_query", + "context": "query_normal", + "primary": true + } + ] + } +} +``` + +## Troubleshooting + +### Keymap not loading + +1. Check that `custom_keymap` in `~/.sqlit/settings.json` matches your filename (without `.json`) +2. Check console output for error messages: `sqlit 2>&1 | grep keymap` +3. Verify JSON syntax with: `python -m json.tool ~/.sqlit/keymaps/my-custom.json` + +### Invalid JSON + +If sqlit reports JSON errors, validate your file: + +```bash +python -m json.tool ~/.sqlit/keymaps/my-custom.json > /dev/null +``` + +### Missing required fields + +Each leader command must have: `key`, `action`, `label`, `category` +Each action key must have: `key`, `action` + +## Resetting to Default + +To revert to the default keymap, set in `~/.sqlit/settings.json`: + +```json +{ + "custom_keymap": "default" +} +``` + +Or remove the `custom_keymap` field entirely. + +## Complete Example + +See `config/keymap.template.json` for a complete example with all common keybindings. diff --git a/sqlit/core/__init__.py b/sqlit/core/__init__.py index 761fd29f..3a70d523 100644 --- a/sqlit/core/__init__.py +++ b/sqlit/core/__init__.py @@ -10,12 +10,14 @@ reset_keymap, set_keymap, ) +from .keymap_manager import KeymapManager from .leader_commands import get_leader_commands from .vim import VimMode __all__ = [ "ActionKeyDef", "InputContext", + "KeymapManager", "KeymapProvider", "LeaderCommandDef", "VimMode", diff --git a/sqlit/core/keymap_manager.py b/sqlit/core/keymap_manager.py new file mode 100644 index 00000000..50f6af4c --- /dev/null +++ b/sqlit/core/keymap_manager.py @@ -0,0 +1,304 @@ +"""Keymap management utilities for sqlit.""" + +from __future__ import annotations + +import json +import sys +from pathlib import Path +from typing import Any + +from sqlit.shared.core.protocols import SettingsStoreProtocol +from sqlit.shared.core.store import CONFIG_DIR + +from .keymap import ( + ActionKeyDef, + KeymapProvider, + LeaderCommandDef, + get_keymap, + set_keymap, +) + +CUSTOM_KEYMAP_SETTINGS_KEY = "custom_keymap" +CUSTOM_KEYMAP_DIR = CONFIG_DIR / "keymaps" +LEADER_COMMAND_FIELDS = {"key", "action", "label", "category", "guard", "menu"} +ACTION_KEY_FIELDS = {"key", "action", "context", "guard", "primary", "show", "priority"} + + +class FileBasedKeymapProvider(KeymapProvider): + """Keymap provider that loads from JSON file.""" + + def __init__( + self, + name: str, + leader_commands: list[LeaderCommandDef], + action_keys: list[ActionKeyDef], + ): + self._name = name + self._leader_commands = leader_commands + self._action_keys = action_keys + + @property + def name(self) -> str: + """Get the keymap name.""" + return self._name + + def get_leader_commands(self) -> list[LeaderCommandDef]: + """Get all leader command definitions.""" + return list(self._leader_commands) + + def get_action_keys(self) -> list[ActionKeyDef]: + """Get all regular action key definitions.""" + return list(self._action_keys) + + +class KeymapManager: + """Centralized keymap handling for the app.""" + + def __init__( + self, + settings_store: SettingsStoreProtocol | None = None, + ) -> None: + from sqlit.domains.shell.store.settings import SettingsStore + + self._settings_store = settings_store or SettingsStore.get_instance() + self._custom_keymap_name: str | None = None + self._custom_keymap_path: Path | None = None + + def initialize(self) -> dict: + """Initialize keymap from settings. + + Returns: + The loaded settings dictionary. + """ + settings = self._settings_store.load_all() + self.load_custom_keymap(settings) + return settings + + def load_custom_keymap(self, settings: dict) -> None: + """Load custom keymap from settings if specified. + + Args: + settings: Settings dictionary containing custom_keymap key. + """ + keymap_name = settings.get(CUSTOM_KEYMAP_SETTINGS_KEY) + if not keymap_name or not isinstance(keymap_name, str): + return + if keymap_name.strip() in ("", "default"): + return + + try: + path = self._resolve_keymap_path(keymap_name.strip()) + self._register_custom_keymap(path, keymap_name.strip()) + except Exception as exc: + print( + f"[sqlit] Failed to load custom keymap '{keymap_name}': {exc}", + file=sys.stderr, + ) + + def _resolve_keymap_path(self, keymap_name: str) -> Path: + """Resolve keymap name to file path. + + Args: + keymap_name: Name of the keymap (without .json extension). + + Returns: + Path to the keymap JSON file. + """ + if keymap_name.startswith(("~", "/")) or Path(keymap_name).is_absolute(): + return Path(keymap_name).expanduser() + + name = Path(keymap_name).stem + file_name = f"{name}.json" + return CUSTOM_KEYMAP_DIR / file_name + + def _register_custom_keymap(self, path: Path, keymap_name: str) -> None: + """Load and register a custom keymap from file. + + Args: + path: Path to the keymap JSON file. + keymap_name: Name of the keymap. + + Raises: + ValueError: If the keymap file is invalid. + """ + path = path.expanduser() + if not path.exists(): + raise ValueError(f"Keymap file not found: {path}") + + keymap = self._load_keymap_from_file(path, keymap_name) + set_keymap(keymap) + self._custom_keymap_name = keymap_name + self._custom_keymap_path = path.resolve() + + def _load_keymap_from_file(self, path: Path, keymap_name: str) -> FileBasedKeymapProvider: + """Load keymap data from JSON file. + + Args: + path: Path to the keymap JSON file. + keymap_name: Name of the keymap. + + Returns: + FileBasedKeymapProvider instance. + + Raises: + ValueError: If the JSON is invalid or missing required fields. + """ + try: + payload = json.loads(path.read_text(encoding="utf-8")) + except Exception as exc: + raise ValueError(f"Failed to read keymap JSON: {exc}") from exc + + if not isinstance(payload, dict): + raise ValueError("Keymap file must contain a JSON object.") + + keymap_data = payload.get("keymap", payload) + if not isinstance(keymap_data, dict): + raise ValueError('Keymap file "keymap" must be a JSON object.') + + leader_commands_data = keymap_data.get("leader_commands", []) + action_keys_data = keymap_data.get("action_keys", []) + + if not isinstance(leader_commands_data, list): + raise ValueError('"leader_commands" must be a list.') + if not isinstance(action_keys_data, list): + raise ValueError('"action_keys" must be a list.') + + leader_commands = self._parse_leader_commands(leader_commands_data) + action_keys = self._parse_action_keys(action_keys_data) + + return FileBasedKeymapProvider(keymap_name, leader_commands, action_keys) + + def _parse_leader_commands(self, data: list[Any]) -> list[LeaderCommandDef]: + """Parse leader commands from JSON data. + + Args: + data: List of leader command dictionaries. + + Returns: + List of LeaderCommandDef instances. + + Raises: + ValueError: If any command is invalid. + """ + commands = [] + for i, item in enumerate(data): + if not isinstance(item, dict): + raise ValueError(f'Leader command at index {i} must be an object.') + + key = item.get("key") + action = item.get("action") + label = item.get("label") + category = item.get("category") + + if not isinstance(key, str) or not key: + raise ValueError(f'Leader command at index {i} missing required "key".') + if not isinstance(action, str) or not action: + raise ValueError(f'Leader command at index {i} missing required "action".') + if not isinstance(label, str) or not label: + raise ValueError(f'Leader command at index {i} missing required "label".') + if not isinstance(category, str) or not category: + raise ValueError(f'Leader command at index {i} missing required "category".') + + guard = item.get("guard") + if guard is not None and not isinstance(guard, str): + raise ValueError(f'Leader command at index {i} "guard" must be a string.') + + menu = item.get("menu", "leader") + if not isinstance(menu, str): + raise ValueError(f'Leader command at index {i} "menu" must be a string.') + + commands.append( + LeaderCommandDef( + key=key, + action=action, + label=label, + category=category, + guard=guard, + menu=menu, + ) + ) + + return commands + + def _parse_action_keys(self, data: list[Any]) -> list[ActionKeyDef]: + """Parse action keys from JSON data. + + Args: + data: List of action key dictionaries. + + Returns: + List of ActionKeyDef instances. + + Raises: + ValueError: If any action key is invalid. + """ + action_keys = [] + for i, item in enumerate(data): + if not isinstance(item, dict): + raise ValueError(f'Action key at index {i} must be an object.') + + key = item.get("key") + action = item.get("action") + + if not isinstance(key, str) or not key: + raise ValueError(f'Action key at index {i} missing required "key".') + if not isinstance(action, str) or not action: + raise ValueError(f'Action key at index {i} missing required "action".') + + context = item.get("context") + if context is not None and not isinstance(context, str): + raise ValueError(f'Action key at index {i} "context" must be a string.') + + guard = item.get("guard") + if guard is not None and not isinstance(guard, str): + raise ValueError(f'Action key at index {i} "guard" must be a string.') + + primary = item.get("primary", True) + if not isinstance(primary, bool): + raise ValueError(f'Action key at index {i} "primary" must be a boolean.') + + show = item.get("show", False) + if not isinstance(show, bool): + raise ValueError(f'Action key at index {i} "show" must be a boolean.') + + priority = item.get("priority", False) + if not isinstance(priority, bool): + raise ValueError(f'Action key at index {i} "priority" must be a boolean.') + + action_keys.append( + ActionKeyDef( + key=key, + action=action, + context=context, + guard=guard, + primary=primary, + show=show, + priority=priority, + ) + ) + + return action_keys + + def get_custom_keymap_name(self) -> str | None: + """Get the name of the currently loaded custom keymap. + + Returns: + Keymap name or None if using default keymap. + """ + return self._custom_keymap_name + + def get_custom_keymap_path(self) -> Path | None: + """Get the path to the currently loaded custom keymap file. + + Returns: + Path to keymap file or None if using default keymap. + """ + return self._custom_keymap_path + + def reset_to_default(self) -> None: + """Reset to the default keymap.""" + from .keymap import reset_keymap + + reset_keymap() + self._custom_keymap_name = None + self._custom_keymap_path = None diff --git a/sqlit/domains/shell/app/main.py b/sqlit/domains/shell/app/main.py index f99ed82e..71230f70 100644 --- a/sqlit/domains/shell/app/main.py +++ b/sqlit/domains/shell/app/main.py @@ -24,6 +24,7 @@ from sqlit.core.input_context import InputContext from sqlit.core.key_router import resolve_action +from sqlit.core.keymap_manager import KeymapManager from sqlit.core.vim import VimMode from sqlit.domains.connections.domain.config import ConnectionConfig from sqlit.domains.connections.providers.model import DatabaseProvider @@ -190,6 +191,9 @@ def __init__( settings_store=self.services.settings_store, override_theme=self.services.runtime.theme, ) + self._keymap_manager = KeymapManager( + settings_store=self.services.settings_store, + ) self._spinner_index: int = 0 self._spinner_timer: Timer | None = None # Schema indexing state diff --git a/sqlit/domains/shell/app/startup_flow.py b/sqlit/domains/shell/app/startup_flow.py index 84c482a7..dc4e9007 100644 --- a/sqlit/domains/shell/app/startup_flow.py +++ b/sqlit/domains/shell/app/startup_flow.py @@ -35,6 +35,9 @@ def run_on_mount(app: AppProtocol) -> None: settings = app._theme_manager.initialize() app._startup_stamp("settings_loaded") + app._keymap_manager.initialize() + app._startup_stamp("keymap_loaded") + app._expanded_paths = set(settings.get("expanded_nodes", [])) if settings.get("debug_events_enabled"): setter = getattr(app, "_set_debug_events_enabled", None) diff --git a/tests/ui/keybindings/test_keymap_manager.py b/tests/ui/keybindings/test_keymap_manager.py new file mode 100644 index 00000000..ab94bc3d --- /dev/null +++ b/tests/ui/keybindings/test_keymap_manager.py @@ -0,0 +1,291 @@ +"""Tests for the KeymapManager.""" + +from __future__ import annotations + +import json +import tempfile +from pathlib import Path + +import pytest + +from sqlit.core.keymap import get_keymap, reset_keymap +from sqlit.core.keymap_manager import KeymapManager + + +class MockSettingsStore: + """Mock settings store for testing.""" + + def __init__(self, settings: dict | None = None): + self.settings = settings or {} + + def load_all(self) -> dict: + return self.settings + + def save_all(self, settings: dict) -> None: + self.settings = settings + + def get(self, key: str, default=None): + return self.settings.get(key, default) + + +@pytest.fixture(autouse=True) +def reset_keymap_after_test(): + """Reset keymap after each test to avoid cross-test pollution.""" + yield + reset_keymap() + + +class TestKeymapManager: + """Test the KeymapManager class.""" + + def test_initialize_with_no_custom_keymap(self): + """Should use default keymap when no custom keymap is specified.""" + settings_store = MockSettingsStore({}) + manager = KeymapManager(settings_store=settings_store) + + settings = manager.initialize() + + assert settings == {} + assert manager.get_custom_keymap_name() is None + assert manager.get_custom_keymap_path() is None + + def test_initialize_with_default_keymap_setting(self): + """Should use default keymap when custom_keymap is set to 'default'.""" + settings_store = MockSettingsStore({"custom_keymap": "default"}) + manager = KeymapManager(settings_store=settings_store) + + settings = manager.initialize() + + assert settings == {"custom_keymap": "default"} + assert manager.get_custom_keymap_name() is None + + def test_load_custom_keymap_from_file(self, tmp_path: Path): + """Should load custom keymap from JSON file.""" + keymap_file = tmp_path / "my-custom.json" + keymap_data = { + "keymap": { + "leader_commands": [ + { + "key": "x", + "action": "quit", + "label": "Exit", + "category": "Actions", + "menu": "leader", + } + ], + "action_keys": [ + { + "key": "ctrl+x", + "action": "quit", + "context": "global", + "primary": True, + "show": False, + "priority": False, + } + ], + } + } + keymap_file.write_text(json.dumps(keymap_data), encoding="utf-8") + + settings_store = MockSettingsStore({"custom_keymap": str(keymap_file)}) + manager = KeymapManager(settings_store=settings_store) + + manager.initialize() + + assert manager.get_custom_keymap_name() == str(keymap_file) + assert manager.get_custom_keymap_path() == keymap_file + + keymap = get_keymap() + assert keymap.leader("quit") == "x" + assert keymap.action("quit") == "ctrl+x" + + def test_load_custom_keymap_with_invalid_json(self, tmp_path: Path, capsys): + """Should handle invalid JSON gracefully.""" + keymap_file = tmp_path / "invalid.json" + keymap_file.write_text("not valid json", encoding="utf-8") + + settings_store = MockSettingsStore({"custom_keymap": str(keymap_file)}) + manager = KeymapManager(settings_store=settings_store) + + manager.initialize() + + captured = capsys.readouterr() + assert "Failed to load custom keymap" in captured.err + assert manager.get_custom_keymap_name() is None + + def test_load_custom_keymap_with_missing_file(self, tmp_path: Path, capsys): + """Should handle missing file gracefully.""" + keymap_file = tmp_path / "nonexistent.json" + + settings_store = MockSettingsStore({"custom_keymap": str(keymap_file)}) + manager = KeymapManager(settings_store=settings_store) + + manager.initialize() + + captured = capsys.readouterr() + assert "Failed to load custom keymap" in captured.err + assert "not found" in captured.err + + def test_reset_to_default(self, tmp_path: Path): + """Should reset to default keymap.""" + keymap_file = tmp_path / "test.json" + keymap_data = { + "keymap": { + "leader_commands": [ + { + "key": "z", + "action": "quit", + "label": "Quit", + "category": "Actions", + } + ], + "action_keys": [], + } + } + keymap_file.write_text(json.dumps(keymap_data), encoding="utf-8") + + settings_store = MockSettingsStore({"custom_keymap": str(keymap_file)}) + manager = KeymapManager(settings_store=settings_store) + manager.initialize() + + assert manager.get_custom_keymap_name() is not None + + manager.reset_to_default() + + assert manager.get_custom_keymap_name() is None + assert manager.get_custom_keymap_path() is None + + def test_parse_leader_commands_with_all_fields(self, tmp_path: Path): + """Should parse leader commands with all optional fields.""" + keymap_file = tmp_path / "full.json" + keymap_data = { + "keymap": { + "leader_commands": [ + { + "key": "x", + "action": "disconnect", + "label": "Disconnect", + "category": "Connection", + "guard": "has_connection", + "menu": "custom", + } + ], + "action_keys": [], + } + } + keymap_file.write_text(json.dumps(keymap_data), encoding="utf-8") + + settings_store = MockSettingsStore({"custom_keymap": str(keymap_file)}) + manager = KeymapManager(settings_store=settings_store) + manager.initialize() + + keymap = get_keymap() + commands = keymap.get_leader_commands() + assert len(commands) == 1 + assert commands[0].key == "x" + assert commands[0].action == "disconnect" + assert commands[0].guard == "has_connection" + assert commands[0].menu == "custom" + + def test_parse_action_keys_with_all_fields(self, tmp_path: Path): + """Should parse action keys with all fields.""" + keymap_file = tmp_path / "actions.json" + keymap_data = { + "keymap": { + "leader_commands": [], + "action_keys": [ + { + "key": "i", + "action": "enter_insert_mode", + "context": "query_normal", + "guard": "not_executing", + "primary": False, + "show": True, + "priority": True, + } + ], + } + } + keymap_file.write_text(json.dumps(keymap_data), encoding="utf-8") + + settings_store = MockSettingsStore({"custom_keymap": str(keymap_file)}) + manager = KeymapManager(settings_store=settings_store) + manager.initialize() + + keymap = get_keymap() + action_keys = keymap.get_action_keys() + assert len(action_keys) == 1 + assert action_keys[0].key == "i" + assert action_keys[0].action == "enter_insert_mode" + assert action_keys[0].context == "query_normal" + assert action_keys[0].guard == "not_executing" + assert action_keys[0].primary is False + assert action_keys[0].show is True + assert action_keys[0].priority is True + + def test_parse_keymap_with_nested_keymap_object(self, tmp_path: Path): + """Should handle both nested and flat keymap structure.""" + keymap_file = tmp_path / "nested.json" + keymap_data = { + "keymap": { + "leader_commands": [ + {"key": "q", "action": "quit", "label": "Quit", "category": "Actions"} + ], + "action_keys": [], + } + } + keymap_file.write_text(json.dumps(keymap_data), encoding="utf-8") + + settings_store = MockSettingsStore({"custom_keymap": str(keymap_file)}) + manager = KeymapManager(settings_store=settings_store) + manager.initialize() + + keymap = get_keymap() + assert keymap.leader("quit") == "q" + + def test_invalid_leader_command_missing_required_field(self, tmp_path: Path, capsys): + """Should error on missing required fields.""" + keymap_file = tmp_path / "invalid.json" + keymap_data = { + "keymap": { + "leader_commands": [ + { + "key": "q", + "action": "quit", + # Missing "label" and "category" + } + ], + "action_keys": [], + } + } + keymap_file.write_text(json.dumps(keymap_data), encoding="utf-8") + + settings_store = MockSettingsStore({"custom_keymap": str(keymap_file)}) + manager = KeymapManager(settings_store=settings_store) + manager.initialize() + + captured = capsys.readouterr() + assert "Failed to load custom keymap" in captured.err + + def test_invalid_action_key_missing_required_field(self, tmp_path: Path, capsys): + """Should error on missing required action key fields.""" + keymap_file = tmp_path / "invalid-action.json" + keymap_data = { + "keymap": { + "leader_commands": [], + "action_keys": [ + { + "key": "i", + # Missing "action" + } + ], + } + } + keymap_file.write_text(json.dumps(keymap_data), encoding="utf-8") + + settings_store = MockSettingsStore({"custom_keymap": str(keymap_file)}) + manager = KeymapManager(settings_store=settings_store) + manager.initialize() + + captured = capsys.readouterr() + assert "Failed to load custom keymap" in captured.err From 4a698441252425b16382e45836330ade045b87ba Mon Sep 17 00:00:00 2001 From: Peter Adams <18162810+Maxteabag@users.noreply.github.com> Date: Sun, 24 May 2026 21:54:31 +0200 Subject: [PATCH 02/12] feat(keybindings): additive custom keymaps with conflict detection Builds on #131 by xtrasmal. Overrides now merge over the defaults rather than replacing the whole map, the footer and help screen reflect custom keys, and a config with conflicting bindings is rejected at startup. Also wires more screens (value view, autocomplete, connection editor, etc.) into the keymap so user overrides actually take effect there too. Co-authored-by: Xander <1174877+xtrasmal@users.noreply.github.com> --- config/keymap.template.json | 235 +------ docs/custom-keymaps.md | 345 ++++------- sqlit/core/keymap.py | 7 +- sqlit/core/keymap_manager.py | 325 ++++++---- .../connections/ui/screens/connection.py | 6 +- .../ui/screens/connection_picker/screen.py | 22 +- .../connections/ui/screens/package_setup.py | 2 +- sqlit/domains/query/ui/mixins/autocomplete.py | 19 +- .../domains/query/ui/screens/query_history.py | 11 +- .../domains/results/ui/screens/value_view.py | 25 +- sqlit/domains/shell/app/startup_flow.py | 6 + sqlit/domains/shell/state/machine.py | 181 +++--- sqlit/domains/shell/ui/screens/help.py | 12 +- sqlit/shared/ui/screens/error.py | 5 +- sqlit/shared/ui/screens/file_picker.py | 5 +- sqlit/shared/ui/widgets_text_area.py | 34 +- tests/ui/keybindings/test_keymap_manager.py | 581 +++++++++++------- 17 files changed, 901 insertions(+), 920 deletions(-) diff --git a/config/keymap.template.json b/config/keymap.template.json index 86a35311..f1081d2f 100644 --- a/config/keymap.template.json +++ b/config/keymap.template.json @@ -1,233 +1,38 @@ { - "_note": "Copy to ~/.sqlit/keymaps/my-custom.json and customize keybindings", - "_usage": "Set 'custom_keymap' in settings.json to 'my-custom' (without .json extension)", + "_note": "Copy to ~/.sqlit/keymaps/.json and set 'custom_keymap' to '' in settings.json.", + "_help": "Only list the bindings you want to change — your entries merge over the defaults. See docs/custom-keymaps.md for the full action list.", "keymap": { "leader_commands": [ { - "_comment": "Leader commands are triggered by pressing the leader key (default: space) followed by another key", - "key": "q", - "action": "quit", - "label": "Quit", - "category": "Actions", - "guard": null, - "menu": "leader" - }, - { - "key": "h", - "action": "show_help", - "label": "Help", - "category": "Actions", - "menu": "leader" - }, - { - "key": "e", - "action": "toggle_explorer", - "label": "Toggle Explorer", - "category": "View", - "menu": "leader" - }, - { - "key": "f", - "action": "toggle_fullscreen", - "label": "Toggle Maximize", - "category": "View", - "menu": "leader" - }, - { - "key": "c", - "action": "show_connection_picker", - "label": "Connect", - "category": "Connection", - "menu": "leader" - }, - { - "key": "x", - "action": "disconnect", - "label": "Disconnect", - "category": "Connection", - "guard": "has_connection", - "menu": "leader" - }, - { - "key": "z", - "action": "cancel_operation", - "label": "Cancel", - "category": "Actions", - "guard": "query_executing", - "menu": "leader" - }, - { - "key": "t", - "action": "change_theme", - "label": "Change Theme", - "category": "Actions", - "menu": "leader" - }, - { - "key": "space", - "action": "telescope", - "label": "Telescope", - "category": "Actions", - "menu": "leader" + "_comment": "Move 'Help' from h to ?", + "key": "question_mark", + "action": "show_help" }, { - "key": "slash", - "action": "telescope_filter", - "label": "Telescope Search", - "category": "Actions", - "menu": "leader" + "_comment": "Add a new leader binding to refresh the tree on R", + "key": "R", + "action": "refresh_tree", + "label": "Refresh tree", + "category": "View" } ], "action_keys": [ { - "_comment": "Action keys are direct keybindings that work in specific contexts", - "key": "space", - "action": "leader_key", - "context": "global", - "guard": null, - "primary": true, - "show": false, - "priority": true - }, - { - "key": "ctrl+q", - "action": "quit", - "context": "global", - "primary": true, - "show": false, - "priority": false - }, - { - "key": "escape", - "action": "cancel_operation", - "context": "global", - "primary": true, - "show": false, - "priority": false - }, - { - "key": "question_mark", - "action": "show_help", - "context": "global", - "primary": true, - "show": false, - "priority": false - }, - { - "_comment": "Query editor keybindings (normal mode)", - "key": "i", - "action": "enter_insert_mode", - "context": "query_normal", - "primary": true, - "show": false, - "priority": false - }, - { - "key": "I", - "action": "prepend_insert_mode", - "context": "query_normal", - "primary": true, - "show": false, - "priority": false - }, - { - "key": "o", - "action": "open_line_below", - "context": "query_normal", - "primary": true, - "show": false, - "priority": false - }, - { - "key": "O", - "action": "open_line_above", - "context": "query_normal", - "primary": true, - "show": false, - "priority": false - }, - { - "key": "enter", - "action": "execute_query", - "context": "query_normal", - "primary": true, - "show": false, - "priority": false - }, - { - "_comment": "Vim cursor movement (normal mode)", - "key": "h", - "action": "cursor_left", - "context": "query_normal", - "primary": true, - "show": false, - "priority": false - }, - { - "key": "j", - "action": "cursor_down", - "context": "query_normal", - "primary": true, - "show": false, - "priority": false - }, - { - "key": "k", - "action": "cursor_up", - "context": "query_normal", - "primary": true, - "show": false, - "priority": false - }, - { - "key": "l", - "action": "cursor_right", - "context": "query_normal", - "primary": true, - "show": false, - "priority": false - }, - { - "_comment": "Query editor keybindings (insert mode)", - "key": "escape", - "action": "exit_insert_mode", - "context": "query_insert", - "primary": true, - "show": false, - "priority": false - }, - { + "_comment": "Use ctrl+enter to execute queries in normal mode (instead of )", "key": "ctrl+enter", - "action": "execute_query_insert", - "context": "query_insert", - "primary": true, - "show": false, - "priority": false - }, - { - "_comment": "Navigation between panes", - "key": "e", - "action": "focus_explorer", - "context": "navigation", - "primary": true, - "show": false, - "priority": false + "action": "execute_query", + "context": "query_normal" }, { - "key": "q", - "action": "focus_query", - "context": "navigation", - "primary": true, - "show": false, - "priority": false + "_comment": "Autocomplete cycling: ctrl+n / ctrl+p (instead of ctrl+j / ctrl+k)", + "key": "ctrl+n", + "action": "autocomplete_next", + "context": "autocomplete" }, { - "key": "r", - "action": "focus_results", - "context": "navigation", - "primary": true, - "show": false, - "priority": false + "key": "ctrl+p", + "action": "autocomplete_prev", + "context": "autocomplete" } ] } diff --git a/docs/custom-keymaps.md b/docs/custom-keymaps.md index 6f3c548f..914b51e9 100644 --- a/docs/custom-keymaps.md +++ b/docs/custom-keymaps.md @@ -1,283 +1,162 @@ # Custom Keymaps -sqlit supports custom keymaps, allowing you to customize all keyboard shortcuts to match your preferences. +sqlit lets you override individual keybindings via a JSON file. Your overrides merge over the built-in defaults, so you only list what you want to change — anything you don't touch keeps its default behaviour. The footer hints and help screen (`?`) automatically reflect your custom keys. -## Quick Start +## Quick start -1. **Copy the template:** +1. Create the keymaps directory and copy the template: ```bash mkdir -p ~/.sqlit/keymaps cp config/keymap.template.json ~/.sqlit/keymaps/my-custom.json ``` -2. **Edit the keymap:** - Open `~/.sqlit/keymaps/my-custom.json` and customize the keybindings. +2. Edit `~/.sqlit/keymaps/my-custom.json` and replace the example overrides with the ones you want. -3. **Enable your custom keymap:** - Edit `~/.sqlit/settings.json` and set: +3. Enable it by adding to `~/.sqlit/settings.json`: ```json - { - "custom_keymap": "my-custom" - } + { "custom_keymap": "my-custom" } ``` - Note: Use the filename without the `.json` extension. + Use the filename without `.json`. To go back to defaults, set `"custom_keymap": "default"` or remove the key. -4. **Restart sqlit** to load your custom keymap. +4. Restart sqlit. -## Keymap Structure +## How merging works -Custom keymaps are JSON files with two main sections: +There are two binding types: -### Leader Commands +| Type | Identity for merging | What "override" means | +|------|----------------------|-----------------------| +| `action_keys` | `(action, context)` | Replaces the default's *primary* key for that action/context | +| `leader_commands` | `(action, menu)` | Replaces the default leader key for that action in that menu | -Leader commands are triggered by pressing the leader key (default: `space`) followed by another key. +If your entry's identity matches a default, the default's primary slot is taken over by your entry. **Secondary defaults survive** — e.g. tree navigation has `j` (primary) plus `down` (alias); rebinding `j` doesn't kill `down`. If your entry doesn't match any default, it's added as a new binding. -```json -{ - "keymap": { - "leader_commands": [ - { - "key": "q", - "action": "quit", - "label": "Quit", - "category": "Actions", - "guard": null, - "menu": "leader" - } - ] - } -} -``` - -**Fields:** -- `key` (required): The key to press after the leader key -- `action` (required): The action to execute -- `label` (required): Display label in the help menu -- `category` (required): Category for grouping in help menu -- `guard` (optional): Condition that must be met (e.g., `"has_connection"`) -- `menu` (optional): Menu ID (default: `"leader"`) +Because identity is `(action, ...)` and not `(key, ...)`, the cleanest way to remap is to specify the action you want to rebind and the new key. Optional fields (`label`, `category`, `guard`, `primary`, `show`, `priority`) are inherited from the matching default — you only need to repeat them when adding a brand-new binding that has no default. -### Action Keys +## Validation at startup -Action keys are direct keybindings that work in specific contexts. +sqlit validates the merged keymap before the app boots. If your overrides introduce a conflict (the same key bound to *different* actions in the same context or menu) the keymap is rejected and sqlit falls back to defaults with a clear stderr message naming the conflicting entries. Default-only overlaps that are disambiguated by runtime state (for example `d` on a connection vs. a folder) are tolerated. -```json -{ - "keymap": { - "action_keys": [ - { - "key": "i", - "action": "enter_insert_mode", - "context": "query_normal", - "guard": null, - "primary": true, - "show": false, - "priority": false - } - ] - } -} -``` +Missing `context` for a context-scoped action is also rejected — silently appending a duplicate global binding is almost never what you wanted. If you really do want a global binding, set `"context": null` explicitly. -**Fields:** -- `key` (required): The key combination (e.g., `"i"`, `"ctrl+q"`, `"escape"`) -- `action` (required): The action to execute -- `context` (optional): Context where this binding is active (e.g., `"query_normal"`, `"tree"`, `"results"`) -- `guard` (optional): Condition that must be met -- `primary` (optional): Whether this is the primary binding for the action (default: `true`) -- `show` (optional): Whether to show in Textual's binding hints (default: `false`) -- `priority` (optional): Whether to give priority to this binding (default: `false`) - -## Special Key Names - -Some keys use special names in the keymap: - -- `space` - Space bar -- `escape` - Escape key -- `enter` - Enter/Return key -- `backspace` - Backspace key -- `delete` - Delete key -- `tab` - Tab key -- `question_mark` - ? key -- `slash` - / key -- `dollar_sign` - $ key -- `percent_sign` - % key -- `asterisk` - * key -- `ctrl+` - Ctrl key combinations (e.g., `"ctrl+q"`, `"ctrl+enter"`) -- `shift+` - Shift key combinations (e.g., `"shift+tab"`) - -## Common Actions - -### Global Actions -- `quit` - Exit sqlit -- `show_help` - Show help menu -- `cancel_operation` - Cancel current operation -- `leader_key` - Trigger leader menu - -### Navigation Actions -- `focus_explorer` - Focus the database explorer -- `focus_query` - Focus the query editor -- `focus_results` - Focus the results table - -### Connection Actions -- `show_connection_picker` - Show connection picker -- `disconnect` - Disconnect from database -- `new_connection` - Create new connection - -### Query Editor Actions (Normal Mode) -- `enter_insert_mode` - Enter insert mode -- `prepend_insert_mode` - Enter insert mode at line start -- `append_insert_mode` - Enter insert mode after cursor -- `append_line_end` - Enter insert mode at line end -- `open_line_below` - Open new line below and enter insert mode -- `open_line_above` - Open new line above and enter insert mode -- `execute_query` - Execute the query - -### Query Editor Actions (Insert Mode) -- `exit_insert_mode` - Return to normal mode -- `execute_query_insert` - Execute query from insert mode - -### Vim-style Navigation -- `cursor_left`, `cursor_down`, `cursor_up`, `cursor_right` - Move cursor -- `cursor_word_forward` - Move to next word -- `cursor_word_back` - Move to previous word -- `cursor_line_start` - Move to line start -- `cursor_line_end` - Move to line end -- `cursor_last_line` - Move to last line - -### Results Actions -- `view_cell` - View cell value -- `edit_cell` - Edit cell value -- `delete_row` - Delete row -- `results_yank_leader_key` - Trigger results copy menu -- `clear_results` - Clear results -- `results_filter` - Filter results - -## Contexts - -Action keys can be scoped to specific contexts: - -- `global` - Active everywhere -- `query_normal` - Query editor in normal mode -- `query_insert` - Query editor in insert mode -- `tree` - Database explorer tree -- `results` - Results table -- `navigation` - For navigation actions -- `autocomplete` - Autocomplete dropdown - -## Guards - -Guards are conditions that must be met for a keybinding to be active: - -- `has_connection` - Database is connected -- `query_executing` - Query is currently executing - -## Examples - -### Vim-style Leader Key - -Change the leader key from space to comma: +### Minimal override examples +Move `Help` from `h` to `?`: ```json -{ - "keymap": { - "action_keys": [ - { - "key": "comma", - "action": "leader_key", - "context": "global", - "primary": true, - "priority": true - } - ] - } -} +{ "key": "question_mark", "action": "show_help" } ``` -### Emacs-style Bindings - -Use Ctrl+X Ctrl+C to quit instead of `q`: - +Use `ctrl+enter` to execute queries in normal mode (overrides the default ``): ```json -{ - "keymap": { - "leader_commands": [], - "action_keys": [ - { - "key": "ctrl+x", - "action": "leader_key", - "context": "global", - "primary": true, - "priority": true - } - ] - } -} +{ "key": "ctrl+enter", "action": "execute_query", "context": "query_normal" } ``` -Then add a leader command: +Add a brand-new binding (no matching default — needs `label` and `category`): ```json -{ - "key": "ctrl+c", - "action": "quit", - "label": "Quit", - "category": "Actions", - "menu": "leader" -} +{ "key": "R", "action": "refresh_tree", "label": "Refresh tree", "category": "View" } ``` -### Custom Query Execution - -Execute query with Ctrl+Enter in normal mode: +## File format ```json { "keymap": { - "action_keys": [ - { - "key": "ctrl+enter", - "action": "execute_query", - "context": "query_normal", - "primary": true - } - ] + "leader_commands": [ /* leader-key bindings */ ], + "action_keys": [ /* direct bindings */ ] } } ``` -## Troubleshooting - -### Keymap not loading - -1. Check that `custom_keymap` in `~/.sqlit/settings.json` matches your filename (without `.json`) -2. Check console output for error messages: `sqlit 2>&1 | grep keymap` -3. Verify JSON syntax with: `python -m json.tool ~/.sqlit/keymaps/my-custom.json` +### `leader_commands` fields + +| Field | Required? | Notes | +|-------|-----------|-------| +| `key` | yes | The key pressed after the leader (e.g. `"q"`, `"question_mark"`) | +| `action` | yes | Action name | +| `menu` | only if not `"leader"` | Submenu (e.g. `"delete"`, `"yank"`, `"g"`) | +| `label` | only for new bindings | Display label; inherited from default when overriding | +| `category` | only for new bindings | Group in command menu; inherited when overriding | +| `guard` | optional | e.g. `"has_connection"`; inherited when overriding | + +### `action_keys` fields + +| Field | Required? | Notes | +|-------|-----------|-------| +| `key` | yes | Key combination (e.g. `"i"`, `"ctrl+q"`, `"escape"`) | +| `action` | yes | Action name | +| `context` | optional | e.g. `"query_normal"`, `"tree"`, `"results"`; `null` means global-no-context | +| `guard` | optional | Inherited when overriding | +| `primary` | optional, default `true` | Footer/help prefers primary keys | +| `show` | optional, default `false` | Show in Textual's binding hints | +| `priority` | optional, default `false` | Wins over child-widget bindings | + +## Special key names + +`space`, `escape`, `enter`, `backspace`, `delete`, `tab`, `question_mark`, `slash`, `dollar_sign`, `percent_sign`, `asterisk`, `ctrl+`, `shift+`. + +## Common actions + +### Global +`quit`, `show_help`, `cancel_operation`, `leader_key` + +### Navigation (context: `navigation`) +`focus_explorer`, `focus_query`, `focus_results` + +### Connection +`show_connection_picker`, `disconnect`, `new_connection` + +### Query editor (context: `query_normal`) +`enter_insert_mode`, `prepend_insert_mode`, `append_insert_mode`, `append_line_end`, +`open_line_below`, `open_line_above`, `execute_query`, +`show_history`, `new_query`, `undo`, `redo`, +`yank_leader_key`, `delete_leader_key`, `change_leader_key`, `g_leader_key`, +`paste` + +### Query editor (context: `query_insert`) +`exit_insert_mode`, `execute_query_insert`, `autocomplete_accept`, +`select_all`, `copy_selection`, `paste` + +### Vim cursor movement (context: `query_normal`) +`cursor_left`, `cursor_down`, `cursor_up`, `cursor_right`, +`cursor_word_forward`, `cursor_WORD_forward`, +`cursor_word_back`, `cursor_WORD_back`, +`cursor_line_start`, `cursor_line_end`, `cursor_last_line`, +`cursor_find_char`, `cursor_find_char_back`, +`cursor_till_char`, `cursor_till_char_back`, +`cursor_matching_bracket` + +### Tree (context: `tree`) +`tree_cursor_up`, `tree_cursor_down`, `tree_filter`, `collapse_tree`, `refresh_tree`, +`select_table`, `new_connection`, `edit_connection`, `delete_connection`, +`duplicate_connection`, `disconnect` + +### Results (context: `results`) +`view_cell`, `view_cell_full`, `edit_cell`, `delete_row`, +`results_yank_leader_key`, `clear_results`, `results_filter`, +`results_cursor_left`, `results_cursor_down`, `results_cursor_up`, `results_cursor_right`, +`next_result_section`, `prev_result_section`, `toggle_result_section` + +### Autocomplete (context: `autocomplete`) +`autocomplete_next`, `autocomplete_prev`, `autocomplete_close` -### Invalid JSON - -If sqlit reports JSON errors, validate your file: +## Contexts -```bash -python -m json.tool ~/.sqlit/keymaps/my-custom.json > /dev/null -``` +`global`, `query_normal`, `query_insert`, `tree`, `tree_filter`, `tree_visual`, +`results`, `results_filter`, `value_view`, `navigation`, `autocomplete`. -### Missing required fields +## Guards -Each leader command must have: `key`, `action`, `label`, `category` -Each action key must have: `key`, `action` +- `has_connection` — a database connection is active +- `query_executing` — a query is currently running -## Resetting to Default +## Troubleshooting -To revert to the default keymap, set in `~/.sqlit/settings.json`: +**Keymap doesn't load.** Make sure `custom_keymap` in `~/.sqlit/settings.json` matches your filename without `.json`. Check the console for `[sqlit] Failed to load custom keymap …`. -```json -{ - "custom_keymap": "default" -} -``` +**Invalid JSON.** Validate with `python -m json.tool ~/.sqlit/keymaps/my-custom.json`. -Or remove the `custom_keymap` field entirely. +**Override is being ignored.** Confirm the `action` and `context`/`menu` exactly match a default. Identity is `(action, context)` for action keys and `(action, menu)` for leader commands — a typo in `context` adds a new binding instead of replacing the default. -## Complete Example +## Reverting -See `config/keymap.template.json` for a complete example with all common keybindings. +Set `"custom_keymap": "default"` in settings, or remove the key entirely. diff --git a/sqlit/core/keymap.py b/sqlit/core/keymap.py index fb2ef92c..86b9b49b 100644 --- a/sqlit/core/keymap.py +++ b/sqlit/core/keymap.py @@ -369,14 +369,19 @@ def _build_action_keys(self) -> list[ActionKeyDef]: # Query (insert mode) ActionKeyDef("escape", "exit_insert_mode", "query_insert"), ActionKeyDef("ctrl+enter", "execute_query_insert", "query_insert"), - ActionKeyDef("tab", "autocomplete_accept", "query_insert"), + # autocomplete_accept lives in the 'autocomplete' context below; + # the query_insert context still acquires it implicitly when the + # dropdown is visible. # Navigation ActionKeyDef("e", "focus_explorer", "navigation"), ActionKeyDef("q", "focus_query", "navigation"), ActionKeyDef("r", "focus_results", "navigation"), # Query (autocomplete) ActionKeyDef("ctrl+j", "autocomplete_next", "autocomplete"), + ActionKeyDef("down", "autocomplete_next", "autocomplete", primary=False), ActionKeyDef("ctrl+k", "autocomplete_prev", "autocomplete"), + ActionKeyDef("up", "autocomplete_prev", "autocomplete", primary=False), + ActionKeyDef("tab", "autocomplete_accept", "autocomplete"), ActionKeyDef("escape", "autocomplete_close", "autocomplete"), # Clipboard (only in insert mode for vim consistency) ActionKeyDef("ctrl+a", "select_all", "query_insert"), diff --git a/sqlit/core/keymap_manager.py b/sqlit/core/keymap_manager.py index 50f6af4c..0e39af0d 100644 --- a/sqlit/core/keymap_manager.py +++ b/sqlit/core/keymap_manager.py @@ -1,9 +1,23 @@ -"""Keymap management utilities for sqlit.""" +"""Keymap management utilities for sqlit. + +User keymaps merge additively over the default keymap: each override +replaces the matching default's primary binding (by ``(action, menu)`` for +leader commands and ``(action, context)`` for action keys). Secondary +aliases — defaults marked ``primary=False`` with the same identity — are +preserved unless the user overrides them explicitly. New entries are +appended. Optional fields are filled in from the matching default when +omitted, so overrides can be as small as ``{"key": "X", "action": "..."}``. + +After merging, the keymap is validated for conflicts (two different actions +bound to the same key in the same context/menu); conflicts abort load with +a clear error so the user can fix their config. +""" from __future__ import annotations import json import sys +from collections import defaultdict from pathlib import Path from typing import Any @@ -12,20 +26,33 @@ from .keymap import ( ActionKeyDef, + DefaultKeymapProvider, KeymapProvider, LeaderCommandDef, - get_keymap, set_keymap, ) CUSTOM_KEYMAP_SETTINGS_KEY = "custom_keymap" CUSTOM_KEYMAP_DIR = CONFIG_DIR / "keymaps" -LEADER_COMMAND_FIELDS = {"key", "action", "label", "category", "guard", "menu"} -ACTION_KEY_FIELDS = {"key", "action", "context", "guard", "primary", "show", "priority"} + + +def build_textual_keymap(provider: KeymapProvider) -> dict[str, str]: + """Build a Textual-compatible keymap dict from a KeymapProvider. + + Groups action keys by action name (across contexts) and joins the keys + with commas — Textual's set_keymap accepts comma-separated key lists. + Use the result with ``App.set_keymap(...)`` so that Bindings with + matching ``id=`` get their keys overridden at runtime. + """ + by_action: dict[str, list[str]] = defaultdict(list) + for ak in provider.get_action_keys(): + if ak.key not in by_action[ak.action]: + by_action[ak.action].append(ak.key) + return {action: ",".join(keys) for action, keys in by_action.items()} class FileBasedKeymapProvider(KeymapProvider): - """Keymap provider that loads from JSON file.""" + """Keymap provider built by merging user overrides onto the defaults.""" def __init__( self, @@ -39,15 +66,12 @@ def __init__( @property def name(self) -> str: - """Get the keymap name.""" return self._name def get_leader_commands(self) -> list[LeaderCommandDef]: - """Get all leader command definitions.""" return list(self._leader_commands) def get_action_keys(self) -> list[ActionKeyDef]: - """Get all regular action key definitions.""" return list(self._action_keys) @@ -65,21 +89,11 @@ def __init__( self._custom_keymap_path: Path | None = None def initialize(self) -> dict: - """Initialize keymap from settings. - - Returns: - The loaded settings dictionary. - """ settings = self._settings_store.load_all() self.load_custom_keymap(settings) return settings def load_custom_keymap(self, settings: dict) -> None: - """Load custom keymap from settings if specified. - - Args: - settings: Settings dictionary containing custom_keymap key. - """ keymap_name = settings.get(CUSTOM_KEYMAP_SETTINGS_KEY) if not keymap_name or not isinstance(keymap_name, str): return @@ -96,31 +110,13 @@ def load_custom_keymap(self, settings: dict) -> None: ) def _resolve_keymap_path(self, keymap_name: str) -> Path: - """Resolve keymap name to file path. - - Args: - keymap_name: Name of the keymap (without .json extension). - - Returns: - Path to the keymap JSON file. - """ if keymap_name.startswith(("~", "/")) or Path(keymap_name).is_absolute(): return Path(keymap_name).expanduser() name = Path(keymap_name).stem - file_name = f"{name}.json" - return CUSTOM_KEYMAP_DIR / file_name + return CUSTOM_KEYMAP_DIR / f"{name}.json" def _register_custom_keymap(self, path: Path, keymap_name: str) -> None: - """Load and register a custom keymap from file. - - Args: - path: Path to the keymap JSON file. - keymap_name: Name of the keymap. - - Raises: - ValueError: If the keymap file is invalid. - """ path = path.expanduser() if not path.exists(): raise ValueError(f"Keymap file not found: {path}") @@ -131,18 +127,6 @@ def _register_custom_keymap(self, path: Path, keymap_name: str) -> None: self._custom_keymap_path = path.resolve() def _load_keymap_from_file(self, path: Path, keymap_name: str) -> FileBasedKeymapProvider: - """Load keymap data from JSON file. - - Args: - path: Path to the keymap JSON file. - keymap_name: Name of the keymap. - - Returns: - FileBasedKeymapProvider instance. - - Raises: - ValueError: If the JSON is invalid or missing required fields. - """ try: payload = json.loads(path.read_text(encoding="utf-8")) except Exception as exc: @@ -155,57 +139,98 @@ def _load_keymap_from_file(self, path: Path, keymap_name: str) -> FileBasedKeyma if not isinstance(keymap_data, dict): raise ValueError('Keymap file "keymap" must be a JSON object.') - leader_commands_data = keymap_data.get("leader_commands", []) - action_keys_data = keymap_data.get("action_keys", []) + user_leader_data = keymap_data.get("leader_commands", []) + user_action_data = keymap_data.get("action_keys", []) - if not isinstance(leader_commands_data, list): + if not isinstance(user_leader_data, list): raise ValueError('"leader_commands" must be a list.') - if not isinstance(action_keys_data, list): + if not isinstance(user_action_data, list): raise ValueError('"action_keys" must be a list.') - leader_commands = self._parse_leader_commands(leader_commands_data) - action_keys = self._parse_action_keys(action_keys_data) + defaults = DefaultKeymapProvider() + base_leader = defaults.get_leader_commands() + base_action = defaults.get_action_keys() - return FileBasedKeymapProvider(keymap_name, leader_commands, action_keys) + # For inheritance lookup: prefer the primary default when multiple + # defaults share an (action, identity). + leader_by_identity: dict[tuple[str, str], LeaderCommandDef] = { + (cmd.action, cmd.menu): cmd for cmd in base_leader + } + action_by_identity: dict[tuple[str, str | None], ActionKeyDef] = {} + for ak in base_action: + existing = action_by_identity.get((ak.action, ak.context)) + if existing is None or (ak.primary and not existing.primary): + action_by_identity[(ak.action, ak.context)] = ak - def _parse_leader_commands(self, data: list[Any]) -> list[LeaderCommandDef]: - """Parse leader commands from JSON data. + # All known contexts per action (across defaults) — used to reject + # overrides that omit context when the action is context-scoped. + contexts_by_action: dict[str, set[str | None]] = defaultdict(set) + for ak in base_action: + contexts_by_action[ak.action].add(ak.context) - Args: - data: List of leader command dictionaries. + user_leader = self._parse_leader_commands(user_leader_data, leader_by_identity) + user_action = self._parse_action_keys( + user_action_data, action_by_identity, contexts_by_action + ) - Returns: - List of LeaderCommandDef instances. + merged_leader = self._merge_leader_commands(base_leader, user_leader) + merged_action = self._merge_action_keys(base_action, user_action) - Raises: - ValueError: If any command is invalid. - """ - commands = [] + self._detect_conflicts(merged_leader, merged_action, user_leader, user_action) + + return FileBasedKeymapProvider(keymap_name, merged_leader, merged_action) + + def _parse_leader_commands( + self, + data: list[Any], + defaults_by_identity: dict[tuple[str, str], LeaderCommandDef], + ) -> list[LeaderCommandDef]: + commands: list[LeaderCommandDef] = [] for i, item in enumerate(data): if not isinstance(item, dict): raise ValueError(f'Leader command at index {i} must be an object.') key = item.get("key") action = item.get("action") - label = item.get("label") - category = item.get("category") - if not isinstance(key, str) or not key: raise ValueError(f'Leader command at index {i} missing required "key".') if not isinstance(action, str) or not action: raise ValueError(f'Leader command at index {i} missing required "action".') + + menu_raw = item.get("menu") + if menu_raw is None: + menu = "leader" + elif isinstance(menu_raw, str) and menu_raw: + menu = menu_raw + else: + raise ValueError(f'Leader command at index {i} "menu" must be a non-empty string.') + + default = defaults_by_identity.get((action, menu)) + + label = item.get("label") + if label is None and default is not None: + label = default.label if not isinstance(label, str) or not label: - raise ValueError(f'Leader command at index {i} missing required "label".') - if not isinstance(category, str) or not category: - raise ValueError(f'Leader command at index {i} missing required "category".') + raise ValueError( + f'Leader command at index {i} ({action!r} in menu {menu!r}) ' + f'has no matching default — "label" is required for new bindings.' + ) - guard = item.get("guard") - if guard is not None and not isinstance(guard, str): - raise ValueError(f'Leader command at index {i} "guard" must be a string.') + category = item.get("category") + if category is None and default is not None: + category = default.category + if not isinstance(category, str) or not category: + raise ValueError( + f'Leader command at index {i} ({action!r} in menu {menu!r}) ' + f'has no matching default — "category" is required for new bindings.' + ) - menu = item.get("menu", "leader") - if not isinstance(menu, str): - raise ValueError(f'Leader command at index {i} "menu" must be a string.') + if "guard" in item: + guard = item.get("guard") + if guard is not None and not isinstance(guard, str): + raise ValueError(f'Leader command at index {i} "guard" must be a string or null.') + else: + guard = default.guard if default is not None else None commands.append( LeaderCommandDef( @@ -220,48 +245,66 @@ def _parse_leader_commands(self, data: list[Any]) -> list[LeaderCommandDef]: return commands - def _parse_action_keys(self, data: list[Any]) -> list[ActionKeyDef]: - """Parse action keys from JSON data. - - Args: - data: List of action key dictionaries. - - Returns: - List of ActionKeyDef instances. - - Raises: - ValueError: If any action key is invalid. - """ - action_keys = [] + def _parse_action_keys( + self, + data: list[Any], + defaults_by_identity: dict[tuple[str, str | None], ActionKeyDef], + contexts_by_action: dict[str, set[str | None]], + ) -> list[ActionKeyDef]: + action_keys: list[ActionKeyDef] = [] for i, item in enumerate(data): if not isinstance(item, dict): raise ValueError(f'Action key at index {i} must be an object.') key = item.get("key") action = item.get("action") - if not isinstance(key, str) or not key: raise ValueError(f'Action key at index {i} missing required "key".') if not isinstance(action, str) or not action: raise ValueError(f'Action key at index {i} missing required "action".') + context_provided = "context" in item context = item.get("context") if context is not None and not isinstance(context, str): - raise ValueError(f'Action key at index {i} "context" must be a string.') + raise ValueError(f'Action key at index {i} "context" must be a string or null.') + + # Reject the silent-no-op trap: user writes a context-scoped action + # without specifying context, which would create a brand-new + # context=None binding while the default stays in place. + known_contexts = contexts_by_action.get(action) + if not context_provided and known_contexts and known_contexts != {None}: + contexts_sorted = sorted(c for c in known_contexts if c is not None) + raise ValueError( + f'Action key at index {i} for action {action!r} is missing "context". ' + f'This action is bound in default context(s): {contexts_sorted}. ' + f'Specify "context" to override one of them, or set it explicitly to null ' + f'to add a global (no-context) binding.' + ) - guard = item.get("guard") - if guard is not None and not isinstance(guard, str): - raise ValueError(f'Action key at index {i} "guard" must be a string.') + default = defaults_by_identity.get((action, context)) - primary = item.get("primary", True) + if "guard" in item: + guard = item.get("guard") + if guard is not None and not isinstance(guard, str): + raise ValueError(f'Action key at index {i} "guard" must be a string or null.') + else: + guard = default.guard if default is not None else None + + primary = item.get("primary") + if primary is None: + primary = default.primary if default is not None else True if not isinstance(primary, bool): raise ValueError(f'Action key at index {i} "primary" must be a boolean.') - show = item.get("show", False) + show = item.get("show") + if show is None: + show = default.show if default is not None else False if not isinstance(show, bool): raise ValueError(f'Action key at index {i} "show" must be a boolean.') - priority = item.get("priority", False) + priority = item.get("priority") + if priority is None: + priority = default.priority if default is not None else False if not isinstance(priority, bool): raise ValueError(f'Action key at index {i} "priority" must be a boolean.') @@ -279,24 +322,86 @@ def _parse_action_keys(self, data: list[Any]) -> list[ActionKeyDef]: return action_keys - def get_custom_keymap_name(self) -> str | None: - """Get the name of the currently loaded custom keymap. - - Returns: - Keymap name or None if using default keymap. + @staticmethod + def _merge_leader_commands( + base: list[LeaderCommandDef], user: list[LeaderCommandDef] + ) -> list[LeaderCommandDef]: + # Leader defaults are unique by (action, menu); drop the matching + # default entirely when the user overrides it. + overridden = {(u.action, u.menu) for u in user} + kept = [cmd for cmd in base if (cmd.action, cmd.menu) not in overridden] + return kept + user + + @staticmethod + def _merge_action_keys( + base: list[ActionKeyDef], user: list[ActionKeyDef] + ) -> list[ActionKeyDef]: + # An action_key override claims the *primary* slot for (action, context). + # Defaults with the same identity that are marked primary=False + # (e.g. arrow-key aliases for j/k movement) survive as aliases. + overridden = {(u.action, u.context) for u in user} + kept = [ + ak for ak in base + if (ak.action, ak.context) not in overridden or not ak.primary + ] + return kept + user + + @staticmethod + def _detect_conflicts( + merged_leader: list[LeaderCommandDef], + merged_action: list[ActionKeyDef], + user_leader: list[LeaderCommandDef], + user_action: list[ActionKeyDef], + ) -> None: + """Raise ValueError on user-introduced bindings that collide. + + Defaults intentionally bind some keys to multiple actions in the + same context (e.g. ``d`` in ``tree`` for both delete_connection + and delete_connection_folder, disambiguated by tree-node state at + runtime). We don't flag those. We *do* flag any conflict that a + user override either created or contributed to — those almost + always indicate a typo or misunderstanding. """ + conflicts: list[str] = [] + + user_leader_slots = {(u.key, u.menu) for u in user_leader} + leader_by_slot: dict[tuple[str, str], set[str]] = defaultdict(set) + for cmd in merged_leader: + leader_by_slot[(cmd.key, cmd.menu)].add(cmd.action) + for (key, menu), actions in sorted(leader_by_slot.items()): + if len(actions) > 1 and (key, menu) in user_leader_slots: + conflicts.append( + f"leader key {key!r} in menu {menu!r} is bound to multiple actions: " + f"{sorted(actions)}" + ) + + user_action_slots = {(u.key, u.context) for u in user_action} + action_by_slot: dict[tuple[str, str | None], set[str]] = defaultdict(set) + for ak in merged_action: + action_by_slot[(ak.key, ak.context)].add(ak.action) + for (key, ctx), actions in sorted( + action_by_slot.items(), key=lambda t: (t[0][0], t[0][1] or "") + ): + if len(actions) > 1 and (key, ctx) in user_action_slots: + conflicts.append( + f"key {key!r} in context {ctx!r} is bound to multiple actions: " + f"{sorted(actions)}" + ) + + if conflicts: + lines = "\n - ".join(conflicts) + raise ValueError( + f"Conflicting keybindings detected ({len(conflicts)}):\n - {lines}\n" + f"Resolve by removing or rebinding the conflicting entries in your keymap." + ) + + def get_custom_keymap_name(self) -> str | None: return self._custom_keymap_name def get_custom_keymap_path(self) -> Path | None: - """Get the path to the currently loaded custom keymap file. - - Returns: - Path to keymap file or None if using default keymap. - """ return self._custom_keymap_path def reset_to_default(self) -> None: - """Reset to the default keymap.""" from .keymap import reset_keymap reset_keymap() diff --git a/sqlit/domains/connections/ui/screens/connection.py b/sqlit/domains/connections/ui/screens/connection.py index dc398188..227a6c90 100644 --- a/sqlit/domains/connections/ui/screens/connection.py +++ b/sqlit/domains/connections/ui/screens/connection.py @@ -50,9 +50,9 @@ class ConnectionScreen(ModalScreen): BINDINGS = [ Binding("escape", "cancel", "Cancel", priority=True), - Binding("ctrl+s", "save", "Save", priority=True), - Binding("ctrl+t", "test_connection", "Test", priority=True), - Binding("ctrl+d", "install_driver", "Install driver", show=False, priority=True), + Binding("ctrl+s", "save", "Save", priority=True, id="connection_save"), + Binding("ctrl+t", "test_connection", "Test", priority=True, id="connection_test"), + Binding("ctrl+d", "install_driver", "Install driver", show=False, priority=True, id="connection_install_driver"), Binding("tab", "next_field", "Next field", priority=True), Binding("shift+tab", "prev_field", "Previous field", priority=True), Binding("down", "focus_tab_content", "Focus content", show=False), diff --git a/sqlit/domains/connections/ui/screens/connection_picker/screen.py b/sqlit/domains/connections/ui/screens/connection_picker/screen.py index 26be90fa..886a22ec 100644 --- a/sqlit/domains/connections/ui/screens/connection_picker/screen.py +++ b/sqlit/domains/connections/ui/screens/connection_picker/screen.py @@ -49,18 +49,18 @@ class ConnectionPickerScreen(ModalScreen): """Modal screen for selecting a connection with fuzzy search.""" BINDINGS = [ - Binding("escape", "cancel_or_close_filter", "Cancel"), - Binding("enter", "select", "Select"), - Binding("s", "save", "Save", show=False), - Binding("n", "new_connection", "New", show=False), - Binding("f", "refresh", "Refresh", show=False), - Binding("slash", "open_filter", "Search", show=False), - Binding("up", "move_up", "Up", show=False), - Binding("down", "move_down", "Down", show=False), + Binding("escape", "cancel_or_close_filter", "Cancel", id="picker_cancel"), + Binding("enter", "select", "Select", id="picker_select"), + Binding("s", "save", "Save", show=False, id="picker_save"), + Binding("n", "new_connection", "New", show=False, id="picker_new_connection"), + Binding("f", "refresh", "Refresh", show=False, id="picker_refresh"), + Binding("slash", "open_filter", "Search", show=False, id="picker_open_filter"), + Binding("up", "move_up", "Up", show=False, id="picker_move_up"), + Binding("down", "move_down", "Down", show=False, id="picker_move_down"), Binding("backspace", "backspace", "Backspace", show=False), - Binding("tab", "switch_tab", "Switch Tab", show=False), - Binding("l", "cloud_logout", "Logout", show=False), - Binding("w", "cloud_switch", "Switch", show=False), + Binding("tab", "switch_tab", "Switch Tab", show=False, id="picker_switch_tab"), + Binding("l", "cloud_logout", "Logout", show=False, id="picker_cloud_logout"), + Binding("w", "cloud_switch", "Switch", show=False, id="picker_cloud_switch"), ] CSS = """ diff --git a/sqlit/domains/connections/ui/screens/package_setup.py b/sqlit/domains/connections/ui/screens/package_setup.py index 29e23021..9787ad3a 100644 --- a/sqlit/domains/connections/ui/screens/package_setup.py +++ b/sqlit/domains/connections/ui/screens/package_setup.py @@ -22,7 +22,7 @@ class PackageSetupScreen(ModalScreen): BINDINGS = [ Binding("enter", "install", "Install", priority=True), - Binding("y", "yank", "Copy"), + Binding("y", "yank", "Copy", id="package_setup_yank"), Binding("escape", "cancel", "Cancel", priority=True), ] diff --git a/sqlit/domains/query/ui/mixins/autocomplete.py b/sqlit/domains/query/ui/mixins/autocomplete.py index 9e6eead7..b2d9ab35 100644 --- a/sqlit/domains/query/ui/mixins/autocomplete.py +++ b/sqlit/domains/query/ui/mixins/autocomplete.py @@ -245,21 +245,30 @@ def on_key(self: AutocompleteMixinHost, event: Any) -> None: dropdown = self.autocomplete_dropdown - if event.key == "down": + from sqlit.core.keymap import get_keymap + keymap = get_keymap() + next_keys = keymap.keys_for_action("autocomplete_next") + prev_keys = keymap.keys_for_action("autocomplete_prev") + accept_keys = keymap.keys_for_action("autocomplete_accept") + close_keys = keymap.keys_for_action("autocomplete_close") + + if event.key in next_keys: dropdown.move_selection(1) event.prevent_default() event.stop() - elif event.key == "up": + elif event.key in prev_keys: dropdown.move_selection(-1) event.prevent_default() event.stop() - elif event.key in ("tab", "enter"): + elif event.key in accept_keys or event.key == "enter": + # 'enter' is hardcoded as a secondary accept key — it doubles as + # newline-insert when there are no suggestions, so accept-on-enter + # only fires when the dropdown has results. if self.vim_mode == VimMode.INSERT and dropdown.filtered_items: self._apply_autocomplete() event.prevent_default() event.stop() - elif event.key == "escape": - # Hide autocomplete AND exit insert mode (go to normal mode) + elif event.key in close_keys: self.action_exit_insert_mode() event.prevent_default() event.stop() diff --git a/sqlit/domains/query/ui/screens/query_history.py b/sqlit/domains/query/ui/screens/query_history.py index 2176eaba..31e3e57d 100644 --- a/sqlit/domains/query/ui/screens/query_history.py +++ b/sqlit/domains/query/ui/screens/query_history.py @@ -23,12 +23,11 @@ class QueryHistoryScreen(ModalScreen): """Modal screen for query history selection.""" BINDINGS = [ - Binding("escape", "cancel", "Cancel", priority=True), - Binding("q", "cancel", "Cancel"), - Binding("enter", "select", "Select"), - Binding("d", "delete", "Delete"), - Binding("asterisk", "toggle_star", "Star"), - Binding("slash", "open_filter", "Filter"), + Binding("escape,q", "cancel", "Cancel", priority=True, id="query_history_cancel"), + Binding("enter", "select", "Select", id="query_history_select"), + Binding("d", "delete", "Delete", id="query_history_delete"), + Binding("asterisk", "toggle_star", "Star", id="query_history_toggle_star"), + Binding("slash", "open_filter", "Filter", id="query_history_open_filter"), ] CSS = """ diff --git a/sqlit/domains/results/ui/screens/value_view.py b/sqlit/domains/results/ui/screens/value_view.py index bcc12d0e..f80a4729 100644 --- a/sqlit/domains/results/ui/screens/value_view.py +++ b/sqlit/domains/results/ui/screens/value_view.py @@ -18,13 +18,17 @@ class ValueViewScreen(ModalScreen): """Modal screen for viewing a single (potentially long) value with tree/syntax toggle.""" + # Each id matches an action name in DefaultKeymapProvider so that + # `App.set_keymap(...)` can remap them per the user's custom keymap. + # `enter` stays unbinded by id because it's a Textual-modal convention, + # not a user-overridable action. BINDINGS = [ - Binding("escape", "dismiss", "Close"), + Binding("escape,q", "dismiss", "Close", id="close_value_view"), Binding("enter", "dismiss", "Close"), - Binding("q", "dismiss", "Close"), - Binding("y", "copy", "Copy"), - Binding("t", "toggle_view", "Toggle View"), - Binding("z", "collapse_all", "Collapse All", show=False), + Binding("y", "copy", "Copy", id="copy_value_view"), + Binding("t", "toggle_view", "Toggle View", id="toggle_value_view_mode"), + Binding("z", "collapse_all", "Collapse All", show=False, id="collapse_all_json_nodes"), + Binding("Z", "expand_all", "Expand All", show=False, id="expand_all_json_nodes"), ] CSS = """ @@ -133,6 +137,17 @@ def action_collapse_all(self) -> None: except Exception: pass + def action_expand_all(self) -> None: + """Expand all tree nodes.""" + if self._is_json and self._tree_mode: + try: + tree = self.query_one("#json-tree-modal", JSONTreeView) + expand_all = getattr(tree, "action_expand_all", None) + if callable(expand_all): + expand_all() + except Exception: + pass + def action_copy(self) -> None: from sqlit.shared.ui.widgets import flash_widget diff --git a/sqlit/domains/shell/app/startup_flow.py b/sqlit/domains/shell/app/startup_flow.py index dc4e9007..d3af09ee 100644 --- a/sqlit/domains/shell/app/startup_flow.py +++ b/sqlit/domains/shell/app/startup_flow.py @@ -36,6 +36,12 @@ def run_on_mount(app: AppProtocol) -> None: app._startup_stamp("settings_loaded") app._keymap_manager.initialize() + # Feed the (possibly user-customized) keymap into Textual so that any + # Binding with id= picks up the user's key. + from sqlit.core.keymap import get_keymap + from sqlit.core.keymap_manager import build_textual_keymap + + app.set_keymap(build_textual_keymap(get_keymap())) app._startup_stamp("keymap_loaded") app._expanded_paths = set(settings.get("expanded_nodes", [])) diff --git a/sqlit/domains/shell/state/machine.py b/sqlit/domains/shell/state/machine.py index e9f4e4bd..031a2176 100644 --- a/sqlit/domains/shell/state/machine.py +++ b/sqlit/domains/shell/state/machine.py @@ -11,12 +11,10 @@ from __future__ import annotations from sqlit.core.input_context import InputContext -from sqlit.core.keymap import format_key from sqlit.core.leader_commands import get_leader_commands from sqlit.core.state_base import ( ActionResult, DisplayBinding, - HelpEntry, State, resolve_display_key, ) @@ -132,11 +130,28 @@ def get_active_state_name(self, app: InputContext) -> str: return state.__class__.__name__ def generate_help_text(self) -> str: - """Generate structured help text with organized sections.""" - from sqlit.core.keymap import format_key + """Generate structured help text with organized sections. + Keys are resolved from the active keymap so custom keybindings show up + here too. Literal fallbacks are kept for sequences that aren't bound to + a single action (command-mode prefixes, composite vim sequences). + """ + from sqlit.core.keymap import format_key, get_keymap + + keymap = get_keymap() leader_key = resolve_display_key("leader_key") or "" + def k(action: str, fallback: str) -> str: + key = keymap.action(action) + return format_key(key) if key else fallback + + def ks(actions_and_fallbacks: list[tuple[str, str]], sep: str = "/") -> str: + return sep.join(k(a, f) for a, f in actions_and_fallbacks) + + def lk(action: str, menu: str, fallback: str) -> str: + key = keymap.leader(action, menu) + return format_key(key) if key else fallback + def section(title: str) -> str: divider = "-" * 62 return f"[bold $primary]{title}[/]\n[dim]{divider}[/]" @@ -155,119 +170,129 @@ def binding(key: str, desc: str, indent: int = 4) -> str: # ═══════════════════════════════════════════════════════════════════ lines.append(section("GLOBAL")) lines.append(binding(":q", "Quit")) - lines.append(binding(f"{leader_key}t", "Change theme")) - lines.append(binding(f"{leader_key}f", "Toggle fullscreen pane")) - lines.append(binding(f"{leader_key}e", "Toggle explorer visibility")) + lines.append(binding(f"{leader_key}{lk('change_theme', 'leader', 't')}", "Change theme")) + lines.append(binding(f"{leader_key}{lk('toggle_fullscreen', 'leader', 'f')}", "Toggle fullscreen pane")) + lines.append(binding(f"{leader_key}{lk('toggle_explorer', 'leader', 'e')}", "Toggle explorer visibility")) lines.append("") # ═══════════════════════════════════════════════════════════════════ # NAVIGATION # ═══════════════════════════════════════════════════════════════════ lines.append(section("NAVIGATION")) - lines.append(binding("e", "Focus Explorer pane")) - lines.append(binding("q", "Focus Query pane")) - lines.append(binding("r", "Focus Results pane")) + lines.append(binding(k("focus_explorer", "e"), "Focus Explorer pane")) + lines.append(binding(k("focus_query", "q"), "Focus Query pane")) + lines.append(binding(k("focus_results", "r"), "Focus Results pane")) lines.append(binding(leader_key, "Open command menu")) - lines.append(binding("?", "Show this help")) + lines.append(binding(k("show_help", "?"), "Show this help")) lines.append("") # ═══════════════════════════════════════════════════════════════════ # EXPLORER # ═══════════════════════════════════════════════════════════════════ lines.append(section("EXPLORER")) - lines.append(binding("j/k", "Move cursor down/up")) + lines.append(binding(ks([("tree_cursor_down", "j"), ("tree_cursor_up", "k")]), "Move cursor down/up")) lines.append(binding("", "Expand node / Connect")) - lines.append(binding("s", "SELECT TOP 100 (on table/view)")) - lines.append(binding("/", "Filter tree")) - lines.append(binding("z", "Collapse all nodes")) - lines.append(binding("f", "Refresh tree")) + lines.append(binding(k("select_table", "s"), "SELECT TOP 100 (on table/view)")) + lines.append(binding(k("tree_filter", "/"), "Filter tree")) + lines.append(binding(k("collapse_tree", "z"), "Collapse all nodes")) + lines.append(binding(k("refresh_tree", "f"), "Refresh tree")) lines.append("") lines.append(subsection("On Connection Node:")) - lines.append(binding("n", "New connection")) - lines.append(binding("e", "Edit connection")) - lines.append(binding("d", "Delete connection")) - lines.append(binding("D", "Duplicate connection")) - lines.append(binding("x", "Disconnect")) + lines.append(binding(k("new_connection", "n"), "New connection")) + lines.append(binding(k("edit_connection", "e"), "Edit connection")) + lines.append(binding(k("delete_connection", "d"), "Delete connection")) + lines.append(binding(k("duplicate_connection", "D"), "Duplicate connection")) + lines.append(binding(k("disconnect", "x"), "Disconnect")) lines.append("") # ═══════════════════════════════════════════════════════════════════ # QUERY EDITOR # ═══════════════════════════════════════════════════════════════════ + g_key = k("g_leader_key", "g") lines.append(section("QUERY EDITOR")) lines.append(subsection("Normal Mode:")) - lines.append(binding("i/I", "Enter INSERT mode")) - lines.append(binding("o/O", "Open line below/above")) - lines.append(binding("C", "Change to line end")) - lines.append(binding("D", "Delete to line end")) - lines.append(binding("/gr", "Execute query")) - lines.append(binding("gt", "Execute as transaction")) - lines.append(binding("", "Query history")) - lines.append(binding("N", "New query (clear)")) - lines.append(binding("u", "Undo")) - lines.append(binding("^r", "Redo")) + lines.append(binding(ks([("enter_insert_mode", "i"), ("prepend_insert_mode", "I")]), "Enter INSERT mode")) + lines.append(binding(ks([("open_line_below", "o"), ("open_line_above", "O")]), "Open line below/above")) + lines.append(binding(k("change_line_end_motion", "C"), "Change to line end")) + lines.append(binding(k("delete_line_end", "D"), "Delete to line end")) + lines.append(binding(f"{k('execute_query', '')}/{g_key}{lk('execute_query', 'g', 'r')}", "Execute query")) + lines.append(binding(f"{g_key}{lk('execute_query_atomic', 'g', 't')}", "Execute as transaction")) + lines.append(binding(k("show_history", ""), "Query history")) + lines.append(binding(k("new_query", "N"), "New query (clear)")) + lines.append(binding(k("undo", "u"), "Undo")) + lines.append(binding(k("redo", "^r"), "Redo")) lines.append("") lines.append(subsection("Insert Mode:")) - lines.append(binding("", "Exit to NORMAL mode")) - lines.append(binding("^", "Execute (stay in INSERT)")) - lines.append(binding("", "Accept autocomplete")) - lines.append(binding("^a", "Select all")) - lines.append(binding("^c", "Copy selection")) - lines.append(binding("^v", "Paste")) + lines.append(binding(k("exit_insert_mode", ""), "Exit to NORMAL mode")) + lines.append(binding(k("execute_query_insert", "^enter"), "Execute (stay in INSERT)")) + lines.append(binding(k("autocomplete_accept", ""), "Accept autocomplete")) + lines.append(binding(k("select_all", "^a"), "Select all")) + lines.append(binding(k("copy_selection", "^c"), "Copy selection")) + lines.append(binding(k("paste", "^v"), "Paste")) lines.append("") + # Operator + motion sequences: resolve the operator key, keep "{motion}" literal. + yank_op = k("yank_leader_key", "y") + del_op = k("delete_leader_key", "d") + chg_op = k("change_leader_key", "c") lines.append(subsection("Vim Operators (Normal Mode):")) - lines.append(binding("y{motion}", "Copy")) - lines.append(binding("d{motion}", "Delete")) - lines.append(binding("c{motion}", "Change (delete + INSERT)")) - lines.append(binding("p", "Paste after cursor")) + lines.append(binding(f"{yank_op}{{motion}}", "Copy")) + lines.append(binding(f"{del_op}{{motion}}", "Delete")) + lines.append(binding(f"{chg_op}{{motion}}", "Change (delete + INSERT)")) + lines.append(binding(k("paste", "p"), "Paste after cursor")) lines.append("") lines.append(subsection("Vim Motions:")) - lines.append(binding("h/j/k/l", "Cursor left/down/up/right")) - lines.append(binding("w/W", "Word forward")) - lines.append(binding("b/B", "Word backward")) - lines.append(binding("0/$", "Line start/end")) - lines.append(binding("gg/G", "File start/end")) - lines.append(binding("f{c}/F{c}", "Find char forward/back")) - lines.append(binding("t{c}/T{c}", "Till char forward/back")) - lines.append(binding("%", "Matching bracket")) + lines.append(binding(ks([("cursor_left", "h"), ("cursor_down", "j"), ("cursor_up", "k"), ("cursor_right", "l")]), "Cursor left/down/up/right")) + lines.append(binding(ks([("cursor_word_forward", "w"), ("cursor_WORD_forward", "W")]), "Word forward")) + lines.append(binding(ks([("cursor_word_back", "b"), ("cursor_WORD_back", "B")]), "Word backward")) + lines.append(binding(ks([("cursor_line_start", "0"), ("cursor_line_end", "$")]), "Line start/end")) + lines.append(binding(f"{g_key}{lk('first_line', 'g', 'g')}/{k('cursor_last_line', 'G')}", "File start/end")) + lines.append(binding(f"{k('cursor_find_char', 'f')}{{c}}/{k('cursor_find_char_back', 'F')}{{c}}", "Find char forward/back")) + lines.append(binding(f"{k('cursor_till_char', 't')}{{c}}/{k('cursor_till_char_back', 'T')}{{c}}", "Till char forward/back")) + lines.append(binding(k("cursor_matching_bracket", "%"), "Matching bracket")) lines.append("") - lines.append(subsection("Text Objects (with i=inner, a=around):")) - lines.append(binding("iw/aw", "Word")) - lines.append(binding('i"/a"', "Double quotes")) - lines.append(binding("i'/a'", "Single quotes")) - lines.append(binding("i)/a)", "Parentheses")) - lines.append(binding("i}/a}", "Braces")) - lines.append(binding("i]/a]", "Brackets")) + # Text objects: inner/around are leader_commands inside operator menus (yank/delete/change). + # Resolve from the yank menu — by convention these stay aligned across menus. + inner = lk("inner", "yank", "i") + around = lk("around", "yank", "a") + lines.append(subsection(f"Text Objects (with {inner}=inner, {around}=around):")) + lines.append(binding(f"{inner}w/{around}w", "Word")) + lines.append(binding(f'{inner}"/{around}"', "Double quotes")) + lines.append(binding(f"{inner}'/{around}'", "Single quotes")) + lines.append(binding(f"{inner})/{around})", "Parentheses")) + lines.append(binding(f"{inner}}}/{around}}}", "Braces")) + lines.append(binding(f"{inner}]/{around}]", "Brackets")) lines.append("") # ═══════════════════════════════════════════════════════════════════ # RESULTS # ═══════════════════════════════════════════════════════════════════ lines.append(section("RESULTS")) - lines.append(binding("h/j/k/l", "Navigate cells")) - lines.append(binding("v", "Preview cell (inline)")) - lines.append(binding("V", "View full cell value")) - lines.append(binding("u", "Generate UPDATE statement")) - lines.append(binding("d", "Generate DELETE statement")) - lines.append(binding("/", "Filter rows")) - lines.append(binding("x", "Clear results")) - lines.append(binding("", "Next result set")) - lines.append(binding("", "Previous result set")) - lines.append(binding("z", "Collapse/expand result")) + lines.append(binding(ks([("results_cursor_left", "h"), ("results_cursor_down", "j"), ("results_cursor_up", "k"), ("results_cursor_right", "l")]), "Navigate cells")) + lines.append(binding(k("view_cell", "v"), "Preview cell (inline)")) + lines.append(binding(k("view_cell_full", "V"), "View full cell value")) + lines.append(binding(k("edit_cell", "u"), "Generate UPDATE statement")) + lines.append(binding(k("delete_row", "d"), "Generate DELETE statement")) + lines.append(binding(k("results_filter", "/"), "Filter rows")) + lines.append(binding(k("clear_results", "x"), "Clear results")) + lines.append(binding(k("next_result_section", ""), "Next result set")) + lines.append(binding(k("prev_result_section", ""), "Previous result set")) + lines.append(binding(k("toggle_result_section", "z"), "Collapse/expand result")) lines.append("") - lines.append(subsection("Copy Menu (y):")) - lines.append(binding("yc", "Copy cell")) - lines.append(binding("yy", "Copy row")) - lines.append(binding("ya", "Copy all")) - lines.append(binding("ye", "Export menu...")) + results_yank = k("results_yank_leader_key", "y") + lines.append(subsection(f"Copy Menu ({results_yank}):")) + lines.append(binding(f"{results_yank}{lk('cell', 'ry', 'c')}", "Copy cell")) + lines.append(binding(f"{results_yank}{lk('row', 'ry', 'y')}", "Copy row")) + lines.append(binding(f"{results_yank}{lk('all', 'ry', 'a')}", "Copy all")) + lines.append(binding(f"{results_yank}{lk('export', 'ry', 'e')}", "Export menu...")) lines.append("") # ═══════════════════════════════════════════════════════════════════ # FILTERING # ═══════════════════════════════════════════════════════════════════ lines.append(section("FILTERING")) - lines.append(binding("/", "Open filter (Explorer/Results)")) - lines.append(binding("", "Apply filter")) - lines.append(binding("", "Close filter")) + lines.append(binding(k("results_filter", "/"), "Open filter (Explorer/Results)")) + lines.append(binding(k("results_filter_accept", ""), "Apply filter")) + lines.append(binding(k("results_filter_close", ""), "Close filter")) lines.append(binding("~prefix", "Fuzzy match mode")) lines.append("") @@ -296,10 +321,10 @@ def binding(key: str, desc: str, indent: int = 4) -> str: lines.append(binding("/", "Search connections")) lines.append(binding("j/k", "Navigate list")) lines.append(binding("", "Connect to selected")) - lines.append(binding("n", "New connection")) - lines.append(binding("e", "Edit connection")) - lines.append(binding("d", "Delete connection")) - lines.append(binding("D", "Duplicate connection")) + lines.append(binding(k("new_connection", "n"), "New connection")) + lines.append(binding(k("edit_connection", "e"), "Edit connection")) + lines.append(binding(k("delete_connection", "d"), "Delete connection")) + lines.append(binding(k("duplicate_connection", "D"), "Duplicate connection")) lines.append(binding("", "Close picker")) lines.append("") diff --git a/sqlit/domains/shell/ui/screens/help.py b/sqlit/domains/shell/ui/screens/help.py index ad579c14..9147d512 100644 --- a/sqlit/domains/shell/ui/screens/help.py +++ b/sqlit/domains/shell/ui/screens/help.py @@ -15,13 +15,11 @@ class HelpScreen(ModalScreen): """Modal screen showing keyboard shortcuts and navigation tips.""" BINDINGS = [ - Binding("escape", "dismiss", "Close"), - Binding("enter", "dismiss", "Close"), - Binding("q", "dismiss", "Close"), - Binding("j", "scroll_down", "Scroll down", show=False), - Binding("k", "scroll_up", "Scroll up", show=False), - Binding("g", "scroll_home", "Scroll to top", show=False), - Binding("G", "scroll_end", "Scroll to bottom", show=False), + Binding("escape,enter,q", "dismiss", "Close", id="help_dismiss"), + Binding("j", "scroll_down", "Scroll down", show=False, id="help_scroll_down"), + Binding("k", "scroll_up", "Scroll up", show=False, id="help_scroll_up"), + Binding("g", "scroll_home", "Scroll to top", show=False, id="help_scroll_home"), + Binding("G", "scroll_end", "Scroll to bottom", show=False, id="help_scroll_end"), ] CSS = """ diff --git a/sqlit/shared/ui/screens/error.py b/sqlit/shared/ui/screens/error.py index f29456ce..ad68ae9d 100644 --- a/sqlit/shared/ui/screens/error.py +++ b/sqlit/shared/ui/screens/error.py @@ -14,9 +14,8 @@ class ErrorScreen(ModalScreen): """Modal screen for displaying error messages.""" BINDINGS = [ - Binding("enter", "close", "Close"), - Binding("escape", "close", "Close"), - Binding("y", "copy_message", "Copy"), + Binding("enter,escape", "close", "Close"), + Binding("y", "copy_message", "Copy", id="error_copy_message"), ] CSS = """ diff --git a/sqlit/shared/ui/screens/file_picker.py b/sqlit/shared/ui/screens/file_picker.py index 2616b407..3ff79159 100644 --- a/sqlit/shared/ui/screens/file_picker.py +++ b/sqlit/shared/ui/screens/file_picker.py @@ -30,9 +30,8 @@ class FilePickerScreen(ModalScreen[str | None]): BINDINGS = [ Binding("escape", "cancel", "Cancel", priority=True), - Binding("tab", "toggle_focus", "Switch focus", show=False), - Binding("shift+tab", "toggle_focus", "Switch focus", show=False), - Binding("s", "save", "Save", show=False), + Binding("tab,shift+tab", "toggle_focus", "Switch focus", show=False), + Binding("s", "save", "Save", show=False, id="file_picker_save"), ] CSS = """ diff --git a/sqlit/shared/ui/widgets_text_area.py b/sqlit/shared/ui/widgets_text_area.py index 7eade929..02d945a0 100644 --- a/sqlit/shared/ui/widgets_text_area.py +++ b/sqlit/shared/ui/widgets_text_area.py @@ -130,33 +130,45 @@ async def _on_key(self, event: Key) -> None: """Intercept clipboard, undo/redo, and Enter keys.""" normalized_key = self._normalize_key(event.key) + from sqlit.core.keymap import get_keymap + keymap = get_keymap() + select_all_keys = keymap.keys_for_action("select_all") + copy_keys = keymap.keys_for_action("copy_selection") + # `paste` is bound in both query_normal (vim "p") and query_insert + # (ctrl+v). Restrict to the modifier-style keys for this handler so + # plain "p" in normal mode still reaches the vim operator. + paste_keys = [ + k for k in keymap.keys_for_action("paste") + if "+" in k or len(k) > 1 + ] + undo_keys = keymap.keys_for_action("undo") + redo_keys = keymap.keys_for_action("redo") + + clipboard_keys = set(select_all_keys + copy_keys + paste_keys) + # Clipboard shortcuts only work in INSERT mode (vim consistency) - if normalized_key in ("ctrl+a", "ctrl+c", "ctrl+v"): + if normalized_key in clipboard_keys: if not self._is_insert_mode(): # Block these in normal mode - use vim commands instead event.prevent_default() event.stop() return - # Handle CTRL+A (select all) - override Emacs beginning-of-line - if normalized_key == "ctrl+a": + if normalized_key in select_all_keys: if hasattr(self.app, "action_select_all"): self.app.action_select_all() event.prevent_default() event.stop() return - # Handle CTRL+C (copy) - override default behavior - if normalized_key == "ctrl+c": + if normalized_key in copy_keys: if hasattr(self.app, "action_copy_selection"): self.app.action_copy_selection() event.prevent_default() event.stop() return - # Handle CTRL+V (paste) - override default behavior - if normalized_key == "ctrl+v": - # Push undo state before paste + if normalized_key in paste_keys: self._push_undo_if_changed() if hasattr(self.app, "action_paste"): self.app.action_paste() @@ -165,16 +177,14 @@ async def _on_key(self, event: Key) -> None: return # Undo/redo work in both modes - # Handle CTRL+Z (undo) - if normalized_key == "ctrl+z": + if normalized_key in undo_keys and "+" in normalized_key: if hasattr(self.app, "action_undo"): self.app.action_undo() event.prevent_default() event.stop() return - # Handle CTRL+Y (redo) - if normalized_key == "ctrl+y": + if normalized_key in redo_keys and "+" in normalized_key: if hasattr(self.app, "action_redo"): self.app.action_redo() event.prevent_default() diff --git a/tests/ui/keybindings/test_keymap_manager.py b/tests/ui/keybindings/test_keymap_manager.py index ab94bc3d..3bb7ac5e 100644 --- a/tests/ui/keybindings/test_keymap_manager.py +++ b/tests/ui/keybindings/test_keymap_manager.py @@ -1,14 +1,13 @@ -"""Tests for the KeymapManager.""" +"""Tests for the KeymapManager (additive-merge model).""" from __future__ import annotations import json -import tempfile from pathlib import Path import pytest -from sqlit.core.keymap import get_keymap, reset_keymap +from sqlit.core.keymap import DefaultKeymapProvider, get_keymap, reset_keymap from sqlit.core.keymap_manager import KeymapManager @@ -28,264 +27,392 @@ def get(self, key: str, default=None): return self.settings.get(key, default) +def _write_keymap(path: Path, payload: dict) -> Path: + path.write_text(json.dumps(payload), encoding="utf-8") + return path + + +def _load(tmp_path: Path, name: str, payload: dict) -> KeymapManager: + file_path = _write_keymap(tmp_path / f"{name}.json", payload) + manager = KeymapManager(settings_store=MockSettingsStore({"custom_keymap": str(file_path)})) + manager.initialize() + return manager + + @pytest.fixture(autouse=True) def reset_keymap_after_test(): - """Reset keymap after each test to avoid cross-test pollution.""" yield reset_keymap() -class TestKeymapManager: - """Test the KeymapManager class.""" - - def test_initialize_with_no_custom_keymap(self): - """Should use default keymap when no custom keymap is specified.""" - settings_store = MockSettingsStore({}) - manager = KeymapManager(settings_store=settings_store) - +class TestKeymapManagerLifecycle: + def test_no_custom_keymap_uses_defaults(self): + manager = KeymapManager(settings_store=MockSettingsStore({})) settings = manager.initialize() - assert settings == {} assert manager.get_custom_keymap_name() is None assert manager.get_custom_keymap_path() is None - def test_initialize_with_default_keymap_setting(self): - """Should use default keymap when custom_keymap is set to 'default'.""" - settings_store = MockSettingsStore({"custom_keymap": "default"}) - manager = KeymapManager(settings_store=settings_store) - - settings = manager.initialize() - - assert settings == {"custom_keymap": "default"} - assert manager.get_custom_keymap_name() is None - - def test_load_custom_keymap_from_file(self, tmp_path: Path): - """Should load custom keymap from JSON file.""" - keymap_file = tmp_path / "my-custom.json" - keymap_data = { - "keymap": { - "leader_commands": [ - { - "key": "x", - "action": "quit", - "label": "Exit", - "category": "Actions", - "menu": "leader", - } - ], - "action_keys": [ - { - "key": "ctrl+x", - "action": "quit", - "context": "global", - "primary": True, - "show": False, - "priority": False, - } - ], - } - } - keymap_file.write_text(json.dumps(keymap_data), encoding="utf-8") - - settings_store = MockSettingsStore({"custom_keymap": str(keymap_file)}) - manager = KeymapManager(settings_store=settings_store) - + def test_default_sentinel_skips_loading(self): + manager = KeymapManager(settings_store=MockSettingsStore({"custom_keymap": "default"})) manager.initialize() + assert manager.get_custom_keymap_name() is None - assert manager.get_custom_keymap_name() == str(keymap_file) - assert manager.get_custom_keymap_path() == keymap_file - - keymap = get_keymap() - assert keymap.leader("quit") == "x" - assert keymap.action("quit") == "ctrl+x" - - def test_load_custom_keymap_with_invalid_json(self, tmp_path: Path, capsys): - """Should handle invalid JSON gracefully.""" - keymap_file = tmp_path / "invalid.json" - keymap_file.write_text("not valid json", encoding="utf-8") - - settings_store = MockSettingsStore({"custom_keymap": str(keymap_file)}) - manager = KeymapManager(settings_store=settings_store) - + def test_invalid_json_is_reported_not_raised(self, tmp_path: Path, capsys): + path = tmp_path / "invalid.json" + path.write_text("not valid json", encoding="utf-8") + manager = KeymapManager(settings_store=MockSettingsStore({"custom_keymap": str(path)})) manager.initialize() - captured = capsys.readouterr() assert "Failed to load custom keymap" in captured.err assert manager.get_custom_keymap_name() is None - def test_load_custom_keymap_with_missing_file(self, tmp_path: Path, capsys): - """Should handle missing file gracefully.""" - keymap_file = tmp_path / "nonexistent.json" - - settings_store = MockSettingsStore({"custom_keymap": str(keymap_file)}) - manager = KeymapManager(settings_store=settings_store) - + def test_missing_file_is_reported_not_raised(self, tmp_path: Path, capsys): + path = tmp_path / "nope.json" + manager = KeymapManager(settings_store=MockSettingsStore({"custom_keymap": str(path)})) manager.initialize() - captured = capsys.readouterr() assert "Failed to load custom keymap" in captured.err assert "not found" in captured.err def test_reset_to_default(self, tmp_path: Path): - """Should reset to default keymap.""" - keymap_file = tmp_path / "test.json" - keymap_data = { - "keymap": { - "leader_commands": [ - { - "key": "z", - "action": "quit", - "label": "Quit", - "category": "Actions", - } - ], - "action_keys": [], - } - } - keymap_file.write_text(json.dumps(keymap_data), encoding="utf-8") - - settings_store = MockSettingsStore({"custom_keymap": str(keymap_file)}) - manager = KeymapManager(settings_store=settings_store) - manager.initialize() - + manager = _load( + tmp_path, + "rebind", + {"keymap": {"leader_commands": [{"key": "Z", "action": "quit"}], "action_keys": []}}, + ) assert manager.get_custom_keymap_name() is not None - manager.reset_to_default() - assert manager.get_custom_keymap_name() is None assert manager.get_custom_keymap_path() is None - def test_parse_leader_commands_with_all_fields(self, tmp_path: Path): - """Should parse leader commands with all optional fields.""" - keymap_file = tmp_path / "full.json" - keymap_data = { - "keymap": { - "leader_commands": [ - { - "key": "x", - "action": "disconnect", - "label": "Disconnect", - "category": "Connection", - "guard": "has_connection", - "menu": "custom", - } - ], - "action_keys": [], - } - } - keymap_file.write_text(json.dumps(keymap_data), encoding="utf-8") - - settings_store = MockSettingsStore({"custom_keymap": str(keymap_file)}) - manager = KeymapManager(settings_store=settings_store) - manager.initialize() - keymap = get_keymap() - commands = keymap.get_leader_commands() - assert len(commands) == 1 - assert commands[0].key == "x" - assert commands[0].action == "disconnect" - assert commands[0].guard == "has_connection" - assert commands[0].menu == "custom" - - def test_parse_action_keys_with_all_fields(self, tmp_path: Path): - """Should parse action keys with all fields.""" - keymap_file = tmp_path / "actions.json" - keymap_data = { - "keymap": { - "leader_commands": [], - "action_keys": [ - { - "key": "i", - "action": "enter_insert_mode", - "context": "query_normal", - "guard": "not_executing", - "primary": False, - "show": True, - "priority": True, - } - ], - } - } - keymap_file.write_text(json.dumps(keymap_data), encoding="utf-8") - - settings_store = MockSettingsStore({"custom_keymap": str(keymap_file)}) - manager = KeymapManager(settings_store=settings_store) - manager.initialize() +class TestAdditiveMerge: + """Overrides should merge over defaults, not replace them.""" - keymap = get_keymap() - action_keys = keymap.get_action_keys() - assert len(action_keys) == 1 - assert action_keys[0].key == "i" - assert action_keys[0].action == "enter_insert_mode" - assert action_keys[0].context == "query_normal" - assert action_keys[0].guard == "not_executing" - assert action_keys[0].primary is False - assert action_keys[0].show is True - assert action_keys[0].priority is True - - def test_parse_keymap_with_nested_keymap_object(self, tmp_path: Path): - """Should handle both nested and flat keymap structure.""" - keymap_file = tmp_path / "nested.json" - keymap_data = { - "keymap": { - "leader_commands": [ - {"key": "q", "action": "quit", "label": "Quit", "category": "Actions"} - ], - "action_keys": [], - } - } - keymap_file.write_text(json.dumps(keymap_data), encoding="utf-8") - - settings_store = MockSettingsStore({"custom_keymap": str(keymap_file)}) - manager = KeymapManager(settings_store=settings_store) - manager.initialize() + def test_unrelated_defaults_remain(self, tmp_path: Path): + """An override of one leader command must not erase the others.""" + defaults = DefaultKeymapProvider() + before = {(c.action, c.menu) for c in defaults.get_leader_commands()} - keymap = get_keymap() - assert keymap.leader("quit") == "q" - - def test_invalid_leader_command_missing_required_field(self, tmp_path: Path, capsys): - """Should error on missing required fields.""" - keymap_file = tmp_path / "invalid.json" - keymap_data = { - "keymap": { - "leader_commands": [ - { - "key": "q", - "action": "quit", - # Missing "label" and "category" - } - ], - "action_keys": [], - } - } - keymap_file.write_text(json.dumps(keymap_data), encoding="utf-8") - - settings_store = MockSettingsStore({"custom_keymap": str(keymap_file)}) - manager = KeymapManager(settings_store=settings_store) - manager.initialize() + _load( + tmp_path, + "small", + {"keymap": {"leader_commands": [{"key": "Z", "action": "quit"}], "action_keys": []}}, + ) + after = {(c.action, c.menu) for c in get_keymap().get_leader_commands()} + assert before == after, "merge must preserve every default binding's identity" + + def test_leader_override_replaces_matching_default(self, tmp_path: Path): + _load( + tmp_path, + "override", + {"keymap": {"leader_commands": [{"key": "Z", "action": "quit"}], "action_keys": []}}, + ) + keymap = get_keymap() + assert keymap.leader("quit", "leader") == "Z" + # Original "q" no longer maps to quit in the leader menu. + assert "quit" not in [c.action for c in keymap.get_leader_commands() if c.key == "q" and c.menu == "leader"] + + def test_action_key_override_replaces_matching_default(self, tmp_path: Path): + _load( + tmp_path, + "override-action", + { + "keymap": { + "leader_commands": [], + "action_keys": [ + {"key": "ctrl+enter", "action": "execute_query", "context": "query_normal"} + ], + } + }, + ) + keymap = get_keymap() + assert keymap.action("execute_query") == "ctrl+enter" + # Original "" no longer maps to execute_query in query_normal. + assert not any( + ak.key == "enter" and ak.action == "execute_query" and ak.context == "query_normal" + for ak in keymap.get_action_keys() + ) + + def test_missing_label_and_category_inherited_from_default(self, tmp_path: Path): + """Overrides that match a default should not require label/category.""" + _load( + tmp_path, + "inherit", + {"keymap": {"leader_commands": [{"key": "Z", "action": "quit"}], "action_keys": []}}, + ) + match = next( + c for c in get_keymap().get_leader_commands() + if c.action == "quit" and c.menu == "leader" + ) + assert match.key == "Z" + assert match.label == "Quit" # inherited + assert match.category == "Actions" # inherited + + def test_new_binding_without_default_requires_label_and_category(self, tmp_path: Path, capsys): + """A brand-new (action, menu) pair must spell out label and category.""" + _load( + tmp_path, + "new-no-meta", + { + "keymap": { + "leader_commands": [{"key": "X", "action": "some_made_up_action"}], + "action_keys": [], + } + }, + ) captured = capsys.readouterr() assert "Failed to load custom keymap" in captured.err - - def test_invalid_action_key_missing_required_field(self, tmp_path: Path, capsys): - """Should error on missing required action key fields.""" - keymap_file = tmp_path / "invalid-action.json" - keymap_data = { - "keymap": { - "leader_commands": [], - "action_keys": [ - { - "key": "i", - # Missing "action" - } - ], - } - } - keymap_file.write_text(json.dumps(keymap_data), encoding="utf-8") - - settings_store = MockSettingsStore({"custom_keymap": str(keymap_file)}) - manager = KeymapManager(settings_store=settings_store) - manager.initialize() - + assert '"label" is required' in captured.err + + def test_new_binding_with_full_fields_is_appended(self, tmp_path: Path): + _load( + tmp_path, + "new-full", + { + "keymap": { + "leader_commands": [ + { + "key": "R", + "action": "refresh_tree", + "label": "Refresh tree", + "category": "View", + } + ], + "action_keys": [], + } + }, + ) + commands = get_keymap().get_leader_commands() + match = [c for c in commands if c.action == "refresh_tree" and c.menu == "leader"] + assert len(match) == 1 and match[0].key == "R" + + def test_optional_action_key_flags_inherited(self, tmp_path: Path): + """primary/show/priority should come from the matching default when omitted.""" + # leader_key in "global" context is primary=True, priority=True by default. + _load( + tmp_path, + "inherit-flags", + { + "keymap": { + "leader_commands": [], + "action_keys": [ + {"key": "comma", "action": "leader_key", "context": "global"} + ], + } + }, + ) + match = next( + ak for ak in get_keymap().get_action_keys() + if ak.action == "leader_key" and ak.context == "global" + ) + assert match.key == "comma" + assert match.priority is True # inherited from default + assert match.primary is True # inherited from default + + +class TestAliasPreservation: + """Secondary defaults (primary=False) should survive when only the primary slot is overridden.""" + + def test_secondary_alias_survives_primary_override(self, tmp_path: Path): + # refresh_tree in 'tree' has 'f' (primary) and 'R' (alias) by default. + _load( + tmp_path, + "alias", + { + "keymap": { + "leader_commands": [], + "action_keys": [ + {"key": "F", "action": "refresh_tree", "context": "tree"} + ], + } + }, + ) + keys = get_keymap().keys_for_action("refresh_tree") + assert "F" in keys, "user override should be present" + assert "R" in keys, "secondary alias 'R' must not be dropped" + assert "f" not in keys, "primary default 'f' should be replaced" + + +class TestConflictDetection: + """User-introduced same-key/different-action collisions abort load.""" + + def test_user_creates_conflict_with_default(self, tmp_path: Path, capsys): + # Default: 'i' → enter_insert_mode in query_normal. User binds 'i' to undo. + _load( + tmp_path, + "conflict", + { + "keymap": { + "leader_commands": [], + "action_keys": [ + {"key": "i", "action": "undo", "context": "query_normal"} + ], + } + }, + ) + captured = capsys.readouterr() + assert "Conflicting keybindings detected" in captured.err + assert "'i'" in captured.err + assert "query_normal" in captured.err + + def test_user_creates_conflict_with_self(self, tmp_path: Path, capsys): + # Two user overrides claiming the same key in the same context. + _load( + tmp_path, + "self-conflict", + { + "keymap": { + "leader_commands": [], + "action_keys": [ + {"key": "ctrl+x", "action": "undo", "context": "query_normal"}, + {"key": "ctrl+x", "action": "redo", "context": "query_normal"}, + ], + } + }, + ) + captured = capsys.readouterr() + assert "Conflicting keybindings detected" in captured.err + + def test_default_conflicts_are_tolerated(self, tmp_path: Path, capsys): + # 'd' in tree maps to both delete_connection and delete_connection_folder + # by default (resolved by tree-node state at runtime). User does NOT touch + # those — loading an unrelated override must not flag pre-existing overlaps. + _load( + tmp_path, + "no-touch", + { + "keymap": { + "leader_commands": [], + "action_keys": [ + {"key": "Z", "action": "undo", "context": "query_normal"} + ], + } + }, + ) + captured = capsys.readouterr() + assert "Conflicting" not in captured.err + + def test_leader_conflict(self, tmp_path: Path, capsys): + # User binds h to two different actions in the same menu. + _load( + tmp_path, + "leader-conflict", + { + "keymap": { + "leader_commands": [ + {"key": "h", "action": "show_help"}, + { + "key": "h", + "action": "refresh_tree", + "label": "Refresh", + "category": "View", + }, + ], + "action_keys": [], + } + }, + ) + captured = capsys.readouterr() + assert "leader key 'h'" in captured.err + + +class TestContextRequired: + """Omitting context for a context-scoped action is a config trap, not a no-op.""" + + def test_missing_context_is_rejected(self, tmp_path: Path, capsys): + _load( + tmp_path, + "no-context", + { + "keymap": { + "leader_commands": [], + "action_keys": [ + {"key": "ctrl+enter", "action": "execute_query"} + ], + } + }, + ) + captured = capsys.readouterr() + assert "missing \"context\"" in captured.err + assert "query_normal" in captured.err + + def test_explicit_null_context_is_allowed(self, tmp_path: Path): + """Setting context to null (when defaults have no null binding) errors only + if a conflict results — explicit null is the user opting into a global + binding. Here we use an action that has no default at all, so it's clean.""" + _load( + tmp_path, + "null-ctx", + { + "keymap": { + "leader_commands": [], + "action_keys": [ + {"key": "ctrl+shift+z", "action": "my_custom_action", "context": None} + ], + } + }, + ) + match = next( + ak for ak in get_keymap().get_action_keys() + if ak.action == "my_custom_action" + ) + assert match.key == "ctrl+shift+z" + assert match.context is None + + +class TestParsing: + def test_nested_keymap_object(self, tmp_path: Path): + _load( + tmp_path, + "nested", + {"keymap": {"leader_commands": [{"key": "Q", "action": "quit"}], "action_keys": []}}, + ) + assert get_keymap().leader("quit") == "Q" + + def test_action_key_with_explicit_fields(self, tmp_path: Path): + # Use a key that doesn't collide with any default in query_normal. + _load( + tmp_path, + "explicit", + { + "keymap": { + "leader_commands": [], + "action_keys": [ + { + "key": "ctrl+i", + "action": "enter_insert_mode", + "context": "query_normal", + "primary": False, + "show": True, + "priority": True, + } + ], + } + }, + ) + match = next( + ak for ak in get_keymap().get_action_keys() + if ak.action == "enter_insert_mode" and ak.context == "query_normal" + and ak.key == "ctrl+i" + ) + assert match.primary is False + assert match.show is True + assert match.priority is True + + def test_missing_action_field_is_rejected(self, tmp_path: Path, capsys): + _load( + tmp_path, + "no-action", + { + "keymap": { + "leader_commands": [], + "action_keys": [{"key": "i"}], + } + }, + ) captured = capsys.readouterr() assert "Failed to load custom keymap" in captured.err + assert "missing required" in captured.err From e522ac72b376e07582dbc89588d6a2b808d1a5e7 Mon Sep 17 00:00:00 2001 From: Peter Adams <18162810+Maxteabag@users.noreply.github.com> Date: Sun, 24 May 2026 23:03:16 +0200 Subject: [PATCH 03/12] refactor(keybindings): state-nested JSON, relocate KeymapManager MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit JSON shape is now a pure key remap: { "keymap": { "action_keys": { "": { "": "" } }, "leader_commands": { "": { "": "" } } } } Unknown (state, action) pairs are rejected with a helpful error. Label, category, guard, primary, show, priority are all inherited from defaults — they are no longer user-facing. KeymapManager moves from sqlit/core/ to sqlit/domains/shell/app/, alongside ThemeManager. settings_store is now a required argument; the lazy import from domains is gone and core no longer depends on domains for this code path. Also fixes a bug where ":" in INSERT mode could be re-dispatched through the action router and start command mode mid-SQL-string; QueryInsertModeState now forbids enter_command_mode. Strips id= attributes that had no matching keymap default and were therefore inert. --- config/keymap.template.json | 47 +- docs/custom-keymaps.md | 204 ++++----- sqlit/core/__init__.py | 4 +- sqlit/core/keymap.py | 19 + sqlit/core/keymap_manager.py | 409 ----------------- .../connections/ui/screens/connection.py | 6 +- .../ui/screens/connection_picker/screen.py | 22 +- .../connections/ui/screens/package_setup.py | 2 +- sqlit/domains/query/state/query_insert.py | 4 + .../domains/query/ui/screens/query_history.py | 10 +- sqlit/domains/shell/app/keymap_manager.py | 358 +++++++++++++++ sqlit/domains/shell/app/main.py | 13 +- sqlit/domains/shell/app/startup_flow.py | 3 +- sqlit/domains/shell/ui/screens/help.py | 10 +- sqlit/shared/ui/screens/error.py | 2 +- sqlit/shared/ui/screens/file_picker.py | 2 +- tests/ui/keybindings/test_keymap_manager.py | 412 ++++++------------ 17 files changed, 667 insertions(+), 860 deletions(-) delete mode 100644 sqlit/core/keymap_manager.py create mode 100644 sqlit/domains/shell/app/keymap_manager.py diff --git a/config/keymap.template.json b/config/keymap.template.json index f1081d2f..c19ac07f 100644 --- a/config/keymap.template.json +++ b/config/keymap.template.json @@ -1,39 +1,24 @@ { "_note": "Copy to ~/.sqlit/keymaps/.json and set 'custom_keymap' to '' in settings.json.", - "_help": "Only list the bindings you want to change — your entries merge over the defaults. See docs/custom-keymaps.md for the full action list.", + "_help": "Only list the bindings you want to change. Each entry maps action → key (or [primary, alias...]) within its state. See docs/custom-keymaps.md for the action/state catalog.", "keymap": { - "leader_commands": [ - { - "_comment": "Move 'Help' from h to ?", - "key": "question_mark", - "action": "show_help" + "action_keys": { + "query_normal": { + "execute_query": "ctrl+enter" }, - { - "_comment": "Add a new leader binding to refresh the tree on R", - "key": "R", - "action": "refresh_tree", - "label": "Refresh tree", - "category": "View" - } - ], - "action_keys": [ - { - "_comment": "Use ctrl+enter to execute queries in normal mode (instead of )", - "key": "ctrl+enter", - "action": "execute_query", - "context": "query_normal" - }, - { - "_comment": "Autocomplete cycling: ctrl+n / ctrl+p (instead of ctrl+j / ctrl+k)", - "key": "ctrl+n", - "action": "autocomplete_next", - "context": "autocomplete" + "autocomplete": { + "autocomplete_next": "ctrl+n", + "autocomplete_prev": "ctrl+p" }, - { - "key": "ctrl+p", - "action": "autocomplete_prev", - "context": "autocomplete" + "tree": { + "tree_cursor_down": ["j", "down"], + "tree_cursor_up": ["k", "up"] + } + }, + "leader_commands": { + "leader": { + "show_help": "question_mark" } - ] + } } } diff --git a/docs/custom-keymaps.md b/docs/custom-keymaps.md index 914b51e9..3d8c562e 100644 --- a/docs/custom-keymaps.md +++ b/docs/custom-keymaps.md @@ -1,6 +1,6 @@ # Custom Keymaps -sqlit lets you override individual keybindings via a JSON file. Your overrides merge over the built-in defaults, so you only list what you want to change — anything you don't touch keeps its default behaviour. The footer hints and help screen (`?`) automatically reflect your custom keys. +sqlit lets you remap the keys that trigger any action. The set of actions and the states they live in is fixed by the app; the keymap JSON is *purely* a key-remapping layer on top of the defaults. ## Quick start @@ -10,153 +10,141 @@ sqlit lets you override individual keybindings via a JSON file. Your overrides m cp config/keymap.template.json ~/.sqlit/keymaps/my-custom.json ``` -2. Edit `~/.sqlit/keymaps/my-custom.json` and replace the example overrides with the ones you want. +2. Edit `~/.sqlit/keymaps/my-custom.json`. Only list the bindings you want to change. 3. Enable it by adding to `~/.sqlit/settings.json`: ```json { "custom_keymap": "my-custom" } ``` - Use the filename without `.json`. To go back to defaults, set `"custom_keymap": "default"` or remove the key. + Use the filename without `.json`. To revert, set `"custom_keymap": "default"` or remove the key. -4. Restart sqlit. +4. Restart sqlit. The footer and help screen (`?`) reflect your custom keys automatically. -## How merging works +## File shape -There are two binding types: +The JSON mirrors the state machine: each binding lives in the state (for action keys) or menu (for leader commands) where it is active. -| Type | Identity for merging | What "override" means | -|------|----------------------|-----------------------| -| `action_keys` | `(action, context)` | Replaces the default's *primary* key for that action/context | -| `leader_commands` | `(action, menu)` | Replaces the default leader key for that action in that menu | +```json +{ + "keymap": { + "action_keys": { + "": { + "": "" + } + }, + "leader_commands": { + "": { + "": "" + } + } + } +} +``` -If your entry's identity matches a default, the default's primary slot is taken over by your entry. **Secondary defaults survive** — e.g. tree navigation has `j` (primary) plus `down` (alias); rebinding `j` doesn't kill `down`. If your entry doesn't match any default, it's added as a new binding. +Values can be a single key string, or a list to provide aliases (the first entry is the primary key shown in footer/help): -Because identity is `(action, ...)` and not `(key, ...)`, the cleanest way to remap is to specify the action you want to rebind and the new key. Optional fields (`label`, `category`, `guard`, `primary`, `show`, `priority`) are inherited from the matching default — you only need to repeat them when adding a brand-new binding that has no default. +```json +"tree": { + "refresh_tree": ["f", "R"] +} +``` -## Validation at startup +A single string *replaces every default key* for that action in that state. A list lets you control the full set of keys explicitly. -sqlit validates the merged keymap before the app boots. If your overrides introduce a conflict (the same key bound to *different* actions in the same context or menu) the keymap is rejected and sqlit falls back to defaults with a clear stderr message naming the conflicting entries. Default-only overlaps that are disambiguated by runtime state (for example `d` on a connection vs. a folder) are tolerated. +## What you can change — and what you cannot -Missing `context` for a context-scoped action is also rejected — silently appending a duplicate global binding is almost never what you wanted. If you really do want a global binding, set `"context": null` explicitly. +You can change **which key** triggers a given action. That is the entire vocabulary. -### Minimal override examples +You cannot — and don't need to — set the action's label, category, guard, priority, or visibility. Those are properties of the action itself, defined in `sqlit/core/keymap.py`. The keymap is strictly a key-rebinding layer. -Move `Help` from `h` to `?`: -```json -{ "key": "question_mark", "action": "show_help" } -``` +You also cannot invent new actions or attach an existing action to a state it doesn't belong to. The loader rejects unknown `(state, action)` pairs with a clear error that lists the known actions for that state. -Use `ctrl+enter` to execute queries in normal mode (overrides the default ``): -```json -{ "key": "ctrl+enter", "action": "execute_query", "context": "query_normal" } -``` +## Validation at startup -Add a brand-new binding (no matching default — needs `label` and `category`): -```json -{ "key": "R", "action": "refresh_tree", "label": "Refresh tree", "category": "View" } -``` +The keymap is checked before the app boots. Failures fall back to defaults and print to stderr: -## File format +- **Unknown action in a state** — sqlit lists the actions that *do* exist in that state. +- **Conflicting bindings** — two different actions claiming the same key in the same state (or the same menu, for leader commands). Default-only overlaps that are disambiguated by runtime state — e.g. `d` on a connection vs. on a folder, both in `tree` — are tolerated; only collisions you introduce are flagged. -```json -{ - "keymap": { - "leader_commands": [ /* leader-key bindings */ ], - "action_keys": [ /* direct bindings */ ] - } -} -``` +## States (for `action_keys`) -### `leader_commands` fields - -| Field | Required? | Notes | -|-------|-----------|-------| -| `key` | yes | The key pressed after the leader (e.g. `"q"`, `"question_mark"`) | -| `action` | yes | Action name | -| `menu` | only if not `"leader"` | Submenu (e.g. `"delete"`, `"yank"`, `"g"`) | -| `label` | only for new bindings | Display label; inherited from default when overriding | -| `category` | only for new bindings | Group in command menu; inherited when overriding | -| `guard` | optional | e.g. `"has_connection"`; inherited when overriding | - -### `action_keys` fields - -| Field | Required? | Notes | -|-------|-----------|-------| -| `key` | yes | Key combination (e.g. `"i"`, `"ctrl+q"`, `"escape"`) | -| `action` | yes | Action name | -| `context` | optional | e.g. `"query_normal"`, `"tree"`, `"results"`; `null` means global-no-context | -| `guard` | optional | Inherited when overriding | -| `primary` | optional, default `true` | Footer/help prefers primary keys | -| `show` | optional, default `false` | Show in Textual's binding hints | -| `priority` | optional, default `false` | Wins over child-widget bindings | +| State | Where it applies | +|-------|------------------| +| `global` | Active everywhere | +| `navigation` | Switching focus between the three panes | +| `tree` | Database explorer tree | +| `tree_filter` | Filter input over the explorer tree | +| `tree_visual` | Tree visual selection mode | +| `query_normal` | Query editor, NORMAL mode | +| `query_insert` | Query editor, INSERT mode | +| `autocomplete` | Autocomplete dropdown is visible | +| `results` | Results table | +| `results_filter` | Filter input over the results table | +| `value_view` | Value-view modal | -## Special key names +## Menus (for `leader_commands`) -`space`, `escape`, `enter`, `backspace`, `delete`, `tab`, `question_mark`, `slash`, `dollar_sign`, `percent_sign`, `asterisk`, `ctrl+`, `shift+`. +`leader` (the top-level command menu), plus the vim-style sub-menus: `delete`, `yank`, `change`, `g`, `gc` (line comments), `ry` (results yank), `rye` (results export), `vy` (value-view yank). -## Common actions +## Action catalog -### Global -`quit`, `show_help`, `cancel_operation`, `leader_key` +The canonical list lives in `sqlit/core/keymap.py` — `DefaultKeymapProvider._build_action_keys()` and `_build_leader_commands()`. Use the in-app help (`?`) or read the source for the full set. The most commonly-rebound ones: -### Navigation (context: `navigation`) -`focus_explorer`, `focus_query`, `focus_results` +**Global** — `leader_key`, `show_help`, `quit`, `cancel_operation`, `enter_command_mode` -### Connection -`show_connection_picker`, `disconnect`, `new_connection` +**Navigation** — `focus_explorer`, `focus_query`, `focus_results` -### Query editor (context: `query_normal`) -`enter_insert_mode`, `prepend_insert_mode`, `append_insert_mode`, `append_line_end`, -`open_line_below`, `open_line_above`, `execute_query`, -`show_history`, `new_query`, `undo`, `redo`, -`yank_leader_key`, `delete_leader_key`, `change_leader_key`, `g_leader_key`, -`paste` +**Tree** — `tree_cursor_up`, `tree_cursor_down`, `select_table`, `tree_filter`, `collapse_tree`, `refresh_tree`, `new_connection`, `edit_connection`, `delete_connection`, `duplicate_connection`, `disconnect` -### Query editor (context: `query_insert`) -`exit_insert_mode`, `execute_query_insert`, `autocomplete_accept`, -`select_all`, `copy_selection`, `paste` +**Query (normal mode)** — `enter_insert_mode`, `prepend_insert_mode`, `append_insert_mode`, `append_line_end`, `open_line_below`, `open_line_above`, `execute_query`, `cursor_left`, `cursor_down`, `cursor_up`, `cursor_right`, `cursor_word_forward`, `cursor_WORD_forward`, `cursor_word_back`, `cursor_WORD_back`, `cursor_line_start`, `cursor_line_end`, `cursor_last_line`, `cursor_find_char`, `cursor_find_char_back`, `cursor_till_char`, `cursor_till_char_back`, `cursor_matching_bracket`, `paste`, `show_history`, `new_query`, `undo`, `redo` -### Vim cursor movement (context: `query_normal`) -`cursor_left`, `cursor_down`, `cursor_up`, `cursor_right`, -`cursor_word_forward`, `cursor_WORD_forward`, -`cursor_word_back`, `cursor_WORD_back`, -`cursor_line_start`, `cursor_line_end`, `cursor_last_line`, -`cursor_find_char`, `cursor_find_char_back`, -`cursor_till_char`, `cursor_till_char_back`, -`cursor_matching_bracket` +**Query (insert mode)** — `exit_insert_mode`, `execute_query_insert`, `autocomplete_accept`, `select_all`, `copy_selection`, `paste` -### Tree (context: `tree`) -`tree_cursor_up`, `tree_cursor_down`, `tree_filter`, `collapse_tree`, `refresh_tree`, -`select_table`, `new_connection`, `edit_connection`, `delete_connection`, -`duplicate_connection`, `disconnect` +**Autocomplete** — `autocomplete_next`, `autocomplete_prev`, `autocomplete_accept`, `autocomplete_close` -### Results (context: `results`) -`view_cell`, `view_cell_full`, `edit_cell`, `delete_row`, -`results_yank_leader_key`, `clear_results`, `results_filter`, -`results_cursor_left`, `results_cursor_down`, `results_cursor_up`, `results_cursor_right`, -`next_result_section`, `prev_result_section`, `toggle_result_section` +**Results** — `view_cell`, `view_cell_full`, `edit_cell`, `delete_row`, `clear_results`, `results_filter`, `results_cursor_left/down/up/right`, `next_result_section`, `prev_result_section`, `toggle_result_section` -### Autocomplete (context: `autocomplete`) -`autocomplete_next`, `autocomplete_prev`, `autocomplete_close` +**Value view** — `close_value_view`, `copy_value_view`, `toggle_value_view_mode`, `collapse_all_json_nodes`, `expand_all_json_nodes` -## Contexts +## Special key names -`global`, `query_normal`, `query_insert`, `tree`, `tree_filter`, `tree_visual`, -`results`, `results_filter`, `value_view`, `navigation`, `autocomplete`. +`space`, `escape`, `enter`, `backspace`, `delete`, `tab`, `question_mark`, `slash`, `dollar_sign`, `percent_sign`, `asterisk`, `ctrl+`, `shift+`. -## Guards +## Example -- `has_connection` — a database connection is active -- `query_executing` — a query is currently running +Rebind autocomplete to `ctrl+n`/`ctrl+p`, swap `h` to `?`, keep the arrow-key aliases for tree navigation, and use `ctrl+enter` to execute queries in normal mode: -## Troubleshooting +```json +{ + "keymap": { + "action_keys": { + "query_normal": { + "execute_query": "ctrl+enter" + }, + "autocomplete": { + "autocomplete_next": "ctrl+n", + "autocomplete_prev": "ctrl+p" + }, + "tree": { + "tree_cursor_down": ["j", "down"], + "tree_cursor_up": ["k", "up"] + } + }, + "leader_commands": { + "leader": { + "show_help": "question_mark" + } + } + } +} +``` -**Keymap doesn't load.** Make sure `custom_keymap` in `~/.sqlit/settings.json` matches your filename without `.json`. Check the console for `[sqlit] Failed to load custom keymap …`. +## Troubleshooting -**Invalid JSON.** Validate with `python -m json.tool ~/.sqlit/keymaps/my-custom.json`. +**Keymap didn't load.** Check `~/.sqlit/settings.json` — `custom_keymap` must match your filename without the `.json`. Check stderr for `[sqlit] Failed to load custom keymap …`. -**Override is being ignored.** Confirm the `action` and `context`/`menu` exactly match a default. Identity is `(action, context)` for action keys and `(action, menu)` for leader commands — a typo in `context` adds a new binding instead of replacing the default. +**"Unknown action" error.** The action doesn't exist in the state you named. The error message lists the known actions for that state — pick one of those, or check the canonical list in `sqlit/core/keymap.py`. -## Reverting +**"Conflicting keybindings" error.** Your overrides bind the same key to two different actions in the same state. Pick a different key, or remove the redundant entry. -Set `"custom_keymap": "default"` in settings, or remove the key entirely. +**Invalid JSON.** Run `python -m json.tool ~/.sqlit/keymaps/my-custom.json` to find the syntax error. diff --git a/sqlit/core/__init__.py b/sqlit/core/__init__.py index 3a70d523..418a57c8 100644 --- a/sqlit/core/__init__.py +++ b/sqlit/core/__init__.py @@ -5,22 +5,22 @@ ActionKeyDef, KeymapProvider, LeaderCommandDef, + build_textual_keymap, format_key, get_keymap, reset_keymap, set_keymap, ) -from .keymap_manager import KeymapManager from .leader_commands import get_leader_commands from .vim import VimMode __all__ = [ "ActionKeyDef", "InputContext", - "KeymapManager", "KeymapProvider", "LeaderCommandDef", "VimMode", + "build_textual_keymap", "format_key", "get_keymap", "get_leader_commands", diff --git a/sqlit/core/keymap.py b/sqlit/core/keymap.py index 86b9b49b..5686c72b 100644 --- a/sqlit/core/keymap.py +++ b/sqlit/core/keymap.py @@ -3,6 +3,7 @@ from __future__ import annotations from abc import ABC, abstractmethod +from collections import defaultdict from dataclasses import dataclass from sqlit.shared.core.debug_events import emit_debug_event @@ -330,6 +331,9 @@ def _build_action_keys(self) -> list[ActionKeyDef]: ActionKeyDef("ctrl+q", "quit", "global"), ActionKeyDef("escape", "cancel_operation", "global"), ActionKeyDef("question_mark", "show_help", "global"), + ActionKeyDef("colon", "enter_command_mode", "global"), + ActionKeyDef("shift+semicolon", "enter_command_mode", "global", primary=False), + ActionKeyDef(":", "enter_command_mode", "global", primary=False), # Query (normal mode) ActionKeyDef("i", "enter_insert_mode", "query_normal"), ActionKeyDef("I", "prepend_insert_mode", "query_normal"), @@ -491,3 +495,18 @@ def emit_keybinding_snapshot(provider: KeymapProvider | None = None) -> None: keymap._action_emitted = True # type: ignore[attr-defined] except Exception: pass + + +def build_textual_keymap(provider: KeymapProvider) -> dict[str, str]: + """Build a Textual-compatible keymap dict from a KeymapProvider. + + Groups action keys by action name (across contexts) and joins the keys + with commas — Textual's set_keymap accepts comma-separated key lists. + Use the result with ``App.set_keymap(...)`` so that Bindings with + matching ``id=`` get their keys overridden at runtime. + """ + by_action: dict[str, list[str]] = defaultdict(list) + for ak in provider.get_action_keys(): + if ak.key not in by_action[ak.action]: + by_action[ak.action].append(ak.key) + return {action: ",".join(keys) for action, keys in by_action.items()} diff --git a/sqlit/core/keymap_manager.py b/sqlit/core/keymap_manager.py deleted file mode 100644 index 0e39af0d..00000000 --- a/sqlit/core/keymap_manager.py +++ /dev/null @@ -1,409 +0,0 @@ -"""Keymap management utilities for sqlit. - -User keymaps merge additively over the default keymap: each override -replaces the matching default's primary binding (by ``(action, menu)`` for -leader commands and ``(action, context)`` for action keys). Secondary -aliases — defaults marked ``primary=False`` with the same identity — are -preserved unless the user overrides them explicitly. New entries are -appended. Optional fields are filled in from the matching default when -omitted, so overrides can be as small as ``{"key": "X", "action": "..."}``. - -After merging, the keymap is validated for conflicts (two different actions -bound to the same key in the same context/menu); conflicts abort load with -a clear error so the user can fix their config. -""" - -from __future__ import annotations - -import json -import sys -from collections import defaultdict -from pathlib import Path -from typing import Any - -from sqlit.shared.core.protocols import SettingsStoreProtocol -from sqlit.shared.core.store import CONFIG_DIR - -from .keymap import ( - ActionKeyDef, - DefaultKeymapProvider, - KeymapProvider, - LeaderCommandDef, - set_keymap, -) - -CUSTOM_KEYMAP_SETTINGS_KEY = "custom_keymap" -CUSTOM_KEYMAP_DIR = CONFIG_DIR / "keymaps" - - -def build_textual_keymap(provider: KeymapProvider) -> dict[str, str]: - """Build a Textual-compatible keymap dict from a KeymapProvider. - - Groups action keys by action name (across contexts) and joins the keys - with commas — Textual's set_keymap accepts comma-separated key lists. - Use the result with ``App.set_keymap(...)`` so that Bindings with - matching ``id=`` get their keys overridden at runtime. - """ - by_action: dict[str, list[str]] = defaultdict(list) - for ak in provider.get_action_keys(): - if ak.key not in by_action[ak.action]: - by_action[ak.action].append(ak.key) - return {action: ",".join(keys) for action, keys in by_action.items()} - - -class FileBasedKeymapProvider(KeymapProvider): - """Keymap provider built by merging user overrides onto the defaults.""" - - def __init__( - self, - name: str, - leader_commands: list[LeaderCommandDef], - action_keys: list[ActionKeyDef], - ): - self._name = name - self._leader_commands = leader_commands - self._action_keys = action_keys - - @property - def name(self) -> str: - return self._name - - def get_leader_commands(self) -> list[LeaderCommandDef]: - return list(self._leader_commands) - - def get_action_keys(self) -> list[ActionKeyDef]: - return list(self._action_keys) - - -class KeymapManager: - """Centralized keymap handling for the app.""" - - def __init__( - self, - settings_store: SettingsStoreProtocol | None = None, - ) -> None: - from sqlit.domains.shell.store.settings import SettingsStore - - self._settings_store = settings_store or SettingsStore.get_instance() - self._custom_keymap_name: str | None = None - self._custom_keymap_path: Path | None = None - - def initialize(self) -> dict: - settings = self._settings_store.load_all() - self.load_custom_keymap(settings) - return settings - - def load_custom_keymap(self, settings: dict) -> None: - keymap_name = settings.get(CUSTOM_KEYMAP_SETTINGS_KEY) - if not keymap_name or not isinstance(keymap_name, str): - return - if keymap_name.strip() in ("", "default"): - return - - try: - path = self._resolve_keymap_path(keymap_name.strip()) - self._register_custom_keymap(path, keymap_name.strip()) - except Exception as exc: - print( - f"[sqlit] Failed to load custom keymap '{keymap_name}': {exc}", - file=sys.stderr, - ) - - def _resolve_keymap_path(self, keymap_name: str) -> Path: - if keymap_name.startswith(("~", "/")) or Path(keymap_name).is_absolute(): - return Path(keymap_name).expanduser() - - name = Path(keymap_name).stem - return CUSTOM_KEYMAP_DIR / f"{name}.json" - - def _register_custom_keymap(self, path: Path, keymap_name: str) -> None: - path = path.expanduser() - if not path.exists(): - raise ValueError(f"Keymap file not found: {path}") - - keymap = self._load_keymap_from_file(path, keymap_name) - set_keymap(keymap) - self._custom_keymap_name = keymap_name - self._custom_keymap_path = path.resolve() - - def _load_keymap_from_file(self, path: Path, keymap_name: str) -> FileBasedKeymapProvider: - try: - payload = json.loads(path.read_text(encoding="utf-8")) - except Exception as exc: - raise ValueError(f"Failed to read keymap JSON: {exc}") from exc - - if not isinstance(payload, dict): - raise ValueError("Keymap file must contain a JSON object.") - - keymap_data = payload.get("keymap", payload) - if not isinstance(keymap_data, dict): - raise ValueError('Keymap file "keymap" must be a JSON object.') - - user_leader_data = keymap_data.get("leader_commands", []) - user_action_data = keymap_data.get("action_keys", []) - - if not isinstance(user_leader_data, list): - raise ValueError('"leader_commands" must be a list.') - if not isinstance(user_action_data, list): - raise ValueError('"action_keys" must be a list.') - - defaults = DefaultKeymapProvider() - base_leader = defaults.get_leader_commands() - base_action = defaults.get_action_keys() - - # For inheritance lookup: prefer the primary default when multiple - # defaults share an (action, identity). - leader_by_identity: dict[tuple[str, str], LeaderCommandDef] = { - (cmd.action, cmd.menu): cmd for cmd in base_leader - } - action_by_identity: dict[tuple[str, str | None], ActionKeyDef] = {} - for ak in base_action: - existing = action_by_identity.get((ak.action, ak.context)) - if existing is None or (ak.primary and not existing.primary): - action_by_identity[(ak.action, ak.context)] = ak - - # All known contexts per action (across defaults) — used to reject - # overrides that omit context when the action is context-scoped. - contexts_by_action: dict[str, set[str | None]] = defaultdict(set) - for ak in base_action: - contexts_by_action[ak.action].add(ak.context) - - user_leader = self._parse_leader_commands(user_leader_data, leader_by_identity) - user_action = self._parse_action_keys( - user_action_data, action_by_identity, contexts_by_action - ) - - merged_leader = self._merge_leader_commands(base_leader, user_leader) - merged_action = self._merge_action_keys(base_action, user_action) - - self._detect_conflicts(merged_leader, merged_action, user_leader, user_action) - - return FileBasedKeymapProvider(keymap_name, merged_leader, merged_action) - - def _parse_leader_commands( - self, - data: list[Any], - defaults_by_identity: dict[tuple[str, str], LeaderCommandDef], - ) -> list[LeaderCommandDef]: - commands: list[LeaderCommandDef] = [] - for i, item in enumerate(data): - if not isinstance(item, dict): - raise ValueError(f'Leader command at index {i} must be an object.') - - key = item.get("key") - action = item.get("action") - if not isinstance(key, str) or not key: - raise ValueError(f'Leader command at index {i} missing required "key".') - if not isinstance(action, str) or not action: - raise ValueError(f'Leader command at index {i} missing required "action".') - - menu_raw = item.get("menu") - if menu_raw is None: - menu = "leader" - elif isinstance(menu_raw, str) and menu_raw: - menu = menu_raw - else: - raise ValueError(f'Leader command at index {i} "menu" must be a non-empty string.') - - default = defaults_by_identity.get((action, menu)) - - label = item.get("label") - if label is None and default is not None: - label = default.label - if not isinstance(label, str) or not label: - raise ValueError( - f'Leader command at index {i} ({action!r} in menu {menu!r}) ' - f'has no matching default — "label" is required for new bindings.' - ) - - category = item.get("category") - if category is None and default is not None: - category = default.category - if not isinstance(category, str) or not category: - raise ValueError( - f'Leader command at index {i} ({action!r} in menu {menu!r}) ' - f'has no matching default — "category" is required for new bindings.' - ) - - if "guard" in item: - guard = item.get("guard") - if guard is not None and not isinstance(guard, str): - raise ValueError(f'Leader command at index {i} "guard" must be a string or null.') - else: - guard = default.guard if default is not None else None - - commands.append( - LeaderCommandDef( - key=key, - action=action, - label=label, - category=category, - guard=guard, - menu=menu, - ) - ) - - return commands - - def _parse_action_keys( - self, - data: list[Any], - defaults_by_identity: dict[tuple[str, str | None], ActionKeyDef], - contexts_by_action: dict[str, set[str | None]], - ) -> list[ActionKeyDef]: - action_keys: list[ActionKeyDef] = [] - for i, item in enumerate(data): - if not isinstance(item, dict): - raise ValueError(f'Action key at index {i} must be an object.') - - key = item.get("key") - action = item.get("action") - if not isinstance(key, str) or not key: - raise ValueError(f'Action key at index {i} missing required "key".') - if not isinstance(action, str) or not action: - raise ValueError(f'Action key at index {i} missing required "action".') - - context_provided = "context" in item - context = item.get("context") - if context is not None and not isinstance(context, str): - raise ValueError(f'Action key at index {i} "context" must be a string or null.') - - # Reject the silent-no-op trap: user writes a context-scoped action - # without specifying context, which would create a brand-new - # context=None binding while the default stays in place. - known_contexts = contexts_by_action.get(action) - if not context_provided and known_contexts and known_contexts != {None}: - contexts_sorted = sorted(c for c in known_contexts if c is not None) - raise ValueError( - f'Action key at index {i} for action {action!r} is missing "context". ' - f'This action is bound in default context(s): {contexts_sorted}. ' - f'Specify "context" to override one of them, or set it explicitly to null ' - f'to add a global (no-context) binding.' - ) - - default = defaults_by_identity.get((action, context)) - - if "guard" in item: - guard = item.get("guard") - if guard is not None and not isinstance(guard, str): - raise ValueError(f'Action key at index {i} "guard" must be a string or null.') - else: - guard = default.guard if default is not None else None - - primary = item.get("primary") - if primary is None: - primary = default.primary if default is not None else True - if not isinstance(primary, bool): - raise ValueError(f'Action key at index {i} "primary" must be a boolean.') - - show = item.get("show") - if show is None: - show = default.show if default is not None else False - if not isinstance(show, bool): - raise ValueError(f'Action key at index {i} "show" must be a boolean.') - - priority = item.get("priority") - if priority is None: - priority = default.priority if default is not None else False - if not isinstance(priority, bool): - raise ValueError(f'Action key at index {i} "priority" must be a boolean.') - - action_keys.append( - ActionKeyDef( - key=key, - action=action, - context=context, - guard=guard, - primary=primary, - show=show, - priority=priority, - ) - ) - - return action_keys - - @staticmethod - def _merge_leader_commands( - base: list[LeaderCommandDef], user: list[LeaderCommandDef] - ) -> list[LeaderCommandDef]: - # Leader defaults are unique by (action, menu); drop the matching - # default entirely when the user overrides it. - overridden = {(u.action, u.menu) for u in user} - kept = [cmd for cmd in base if (cmd.action, cmd.menu) not in overridden] - return kept + user - - @staticmethod - def _merge_action_keys( - base: list[ActionKeyDef], user: list[ActionKeyDef] - ) -> list[ActionKeyDef]: - # An action_key override claims the *primary* slot for (action, context). - # Defaults with the same identity that are marked primary=False - # (e.g. arrow-key aliases for j/k movement) survive as aliases. - overridden = {(u.action, u.context) for u in user} - kept = [ - ak for ak in base - if (ak.action, ak.context) not in overridden or not ak.primary - ] - return kept + user - - @staticmethod - def _detect_conflicts( - merged_leader: list[LeaderCommandDef], - merged_action: list[ActionKeyDef], - user_leader: list[LeaderCommandDef], - user_action: list[ActionKeyDef], - ) -> None: - """Raise ValueError on user-introduced bindings that collide. - - Defaults intentionally bind some keys to multiple actions in the - same context (e.g. ``d`` in ``tree`` for both delete_connection - and delete_connection_folder, disambiguated by tree-node state at - runtime). We don't flag those. We *do* flag any conflict that a - user override either created or contributed to — those almost - always indicate a typo or misunderstanding. - """ - conflicts: list[str] = [] - - user_leader_slots = {(u.key, u.menu) for u in user_leader} - leader_by_slot: dict[tuple[str, str], set[str]] = defaultdict(set) - for cmd in merged_leader: - leader_by_slot[(cmd.key, cmd.menu)].add(cmd.action) - for (key, menu), actions in sorted(leader_by_slot.items()): - if len(actions) > 1 and (key, menu) in user_leader_slots: - conflicts.append( - f"leader key {key!r} in menu {menu!r} is bound to multiple actions: " - f"{sorted(actions)}" - ) - - user_action_slots = {(u.key, u.context) for u in user_action} - action_by_slot: dict[tuple[str, str | None], set[str]] = defaultdict(set) - for ak in merged_action: - action_by_slot[(ak.key, ak.context)].add(ak.action) - for (key, ctx), actions in sorted( - action_by_slot.items(), key=lambda t: (t[0][0], t[0][1] or "") - ): - if len(actions) > 1 and (key, ctx) in user_action_slots: - conflicts.append( - f"key {key!r} in context {ctx!r} is bound to multiple actions: " - f"{sorted(actions)}" - ) - - if conflicts: - lines = "\n - ".join(conflicts) - raise ValueError( - f"Conflicting keybindings detected ({len(conflicts)}):\n - {lines}\n" - f"Resolve by removing or rebinding the conflicting entries in your keymap." - ) - - def get_custom_keymap_name(self) -> str | None: - return self._custom_keymap_name - - def get_custom_keymap_path(self) -> Path | None: - return self._custom_keymap_path - - def reset_to_default(self) -> None: - from .keymap import reset_keymap - - reset_keymap() - self._custom_keymap_name = None - self._custom_keymap_path = None diff --git a/sqlit/domains/connections/ui/screens/connection.py b/sqlit/domains/connections/ui/screens/connection.py index 227a6c90..dc398188 100644 --- a/sqlit/domains/connections/ui/screens/connection.py +++ b/sqlit/domains/connections/ui/screens/connection.py @@ -50,9 +50,9 @@ class ConnectionScreen(ModalScreen): BINDINGS = [ Binding("escape", "cancel", "Cancel", priority=True), - Binding("ctrl+s", "save", "Save", priority=True, id="connection_save"), - Binding("ctrl+t", "test_connection", "Test", priority=True, id="connection_test"), - Binding("ctrl+d", "install_driver", "Install driver", show=False, priority=True, id="connection_install_driver"), + Binding("ctrl+s", "save", "Save", priority=True), + Binding("ctrl+t", "test_connection", "Test", priority=True), + Binding("ctrl+d", "install_driver", "Install driver", show=False, priority=True), Binding("tab", "next_field", "Next field", priority=True), Binding("shift+tab", "prev_field", "Previous field", priority=True), Binding("down", "focus_tab_content", "Focus content", show=False), diff --git a/sqlit/domains/connections/ui/screens/connection_picker/screen.py b/sqlit/domains/connections/ui/screens/connection_picker/screen.py index 886a22ec..26be90fa 100644 --- a/sqlit/domains/connections/ui/screens/connection_picker/screen.py +++ b/sqlit/domains/connections/ui/screens/connection_picker/screen.py @@ -49,18 +49,18 @@ class ConnectionPickerScreen(ModalScreen): """Modal screen for selecting a connection with fuzzy search.""" BINDINGS = [ - Binding("escape", "cancel_or_close_filter", "Cancel", id="picker_cancel"), - Binding("enter", "select", "Select", id="picker_select"), - Binding("s", "save", "Save", show=False, id="picker_save"), - Binding("n", "new_connection", "New", show=False, id="picker_new_connection"), - Binding("f", "refresh", "Refresh", show=False, id="picker_refresh"), - Binding("slash", "open_filter", "Search", show=False, id="picker_open_filter"), - Binding("up", "move_up", "Up", show=False, id="picker_move_up"), - Binding("down", "move_down", "Down", show=False, id="picker_move_down"), + Binding("escape", "cancel_or_close_filter", "Cancel"), + Binding("enter", "select", "Select"), + Binding("s", "save", "Save", show=False), + Binding("n", "new_connection", "New", show=False), + Binding("f", "refresh", "Refresh", show=False), + Binding("slash", "open_filter", "Search", show=False), + Binding("up", "move_up", "Up", show=False), + Binding("down", "move_down", "Down", show=False), Binding("backspace", "backspace", "Backspace", show=False), - Binding("tab", "switch_tab", "Switch Tab", show=False, id="picker_switch_tab"), - Binding("l", "cloud_logout", "Logout", show=False, id="picker_cloud_logout"), - Binding("w", "cloud_switch", "Switch", show=False, id="picker_cloud_switch"), + Binding("tab", "switch_tab", "Switch Tab", show=False), + Binding("l", "cloud_logout", "Logout", show=False), + Binding("w", "cloud_switch", "Switch", show=False), ] CSS = """ diff --git a/sqlit/domains/connections/ui/screens/package_setup.py b/sqlit/domains/connections/ui/screens/package_setup.py index 9787ad3a..29e23021 100644 --- a/sqlit/domains/connections/ui/screens/package_setup.py +++ b/sqlit/domains/connections/ui/screens/package_setup.py @@ -22,7 +22,7 @@ class PackageSetupScreen(ModalScreen): BINDINGS = [ Binding("enter", "install", "Install", priority=True), - Binding("y", "yank", "Copy", id="package_setup_yank"), + Binding("y", "yank", "Copy"), Binding("escape", "cancel", "Cancel", priority=True), ] diff --git a/sqlit/domains/query/state/query_insert.py b/sqlit/domains/query/state/query_insert.py index 2b479440..90e5f467 100644 --- a/sqlit/domains/query/state/query_insert.py +++ b/sqlit/domains/query/state/query_insert.py @@ -24,12 +24,16 @@ def _setup_actions(self) -> None: # Undo/redo self.allows("undo", help="Undo") self.allows("redo", help="Redo") + # enter_command_mode is forbidden so ":" reaches the text area when + # the user is typing a SQL literal — the action router would otherwise + # re-dispatch the keymap entry and start command mode. self.forbids( "focus_explorer", "focus_results", "leader_key", "new_connection", "show_help", + "enter_command_mode", ) def get_display_bindings(self, app: InputContext) -> tuple[list[DisplayBinding], list[DisplayBinding]]: diff --git a/sqlit/domains/query/ui/screens/query_history.py b/sqlit/domains/query/ui/screens/query_history.py index 31e3e57d..48d8c891 100644 --- a/sqlit/domains/query/ui/screens/query_history.py +++ b/sqlit/domains/query/ui/screens/query_history.py @@ -23,11 +23,11 @@ class QueryHistoryScreen(ModalScreen): """Modal screen for query history selection.""" BINDINGS = [ - Binding("escape,q", "cancel", "Cancel", priority=True, id="query_history_cancel"), - Binding("enter", "select", "Select", id="query_history_select"), - Binding("d", "delete", "Delete", id="query_history_delete"), - Binding("asterisk", "toggle_star", "Star", id="query_history_toggle_star"), - Binding("slash", "open_filter", "Filter", id="query_history_open_filter"), + Binding("escape,q", "cancel", "Cancel", priority=True), + Binding("enter", "select", "Select"), + Binding("d", "delete", "Delete"), + Binding("asterisk", "toggle_star", "Star"), + Binding("slash", "open_filter", "Filter"), ] CSS = """ diff --git a/sqlit/domains/shell/app/keymap_manager.py b/sqlit/domains/shell/app/keymap_manager.py new file mode 100644 index 00000000..99096a3a --- /dev/null +++ b/sqlit/domains/shell/app/keymap_manager.py @@ -0,0 +1,358 @@ +"""Loads the user's custom keymap and registers it with the core keymap. + +Follows the same domain-service pattern as +[`ThemeManager`][sqlit.domains.shell.app.theme_manager] — a settings-driven +configuration step run during app startup. Keymaps live in JSON files under +``~/.sqlit/keymaps/`` and are selected by the ``custom_keymap`` setting. + +The JSON is strictly a *key remapping*. The set of actions and the states +they live in are defined in :mod:`sqlit.core.keymap`; the user only +chooses which key(s) trigger each action:: + + { + "keymap": { + "action_keys": { + "": { + "": "" // single key + | ["", "..."] // primary + aliases + } + }, + "leader_commands": { + "": { + "": "" + } + } + } + } + +The loader validates that every ``(state, action)`` and ``(menu, action)`` +pair the user names exists in the defaults; unknown ones abort the load +with a clear error. After merging, the keymap is also validated for +conflicts (two actions claiming the same key in the same state/menu); on +conflict the loader falls back to defaults and prints every collision to +stderr so the user can fix their config. +""" + +from __future__ import annotations + +import json +import sys +from collections import defaultdict +from pathlib import Path +from typing import Any + +from sqlit.core.keymap import ( + ActionKeyDef, + DefaultKeymapProvider, + KeymapProvider, + LeaderCommandDef, + set_keymap, +) +from sqlit.shared.core.protocols import SettingsStoreProtocol +from sqlit.shared.core.store import CONFIG_DIR + +CUSTOM_KEYMAP_SETTINGS_KEY = "custom_keymap" +CUSTOM_KEYMAP_DIR = CONFIG_DIR / "keymaps" + + +class FileBasedKeymapProvider(KeymapProvider): + """Keymap provider built by merging user overrides onto the defaults.""" + + def __init__( + self, + name: str, + leader_commands: list[LeaderCommandDef], + action_keys: list[ActionKeyDef], + ): + self._name = name + self._leader_commands = leader_commands + self._action_keys = action_keys + + @property + def name(self) -> str: + return self._name + + def get_leader_commands(self) -> list[LeaderCommandDef]: + return list(self._leader_commands) + + def get_action_keys(self) -> list[ActionKeyDef]: + return list(self._action_keys) + + +class KeymapManager: + """Loads and applies a custom keymap during app startup.""" + + def __init__(self, settings_store: SettingsStoreProtocol) -> None: + self._settings_store = settings_store + + def initialize(self) -> dict: + settings = self._settings_store.load_all() + self.load_custom_keymap(settings) + return settings + + def load_custom_keymap(self, settings: dict) -> None: + keymap_name = settings.get(CUSTOM_KEYMAP_SETTINGS_KEY) + if not keymap_name or not isinstance(keymap_name, str): + return + if keymap_name.strip() in ("", "default"): + return + + try: + path = self._resolve_keymap_path(keymap_name.strip()) + self._register_custom_keymap(path, keymap_name.strip()) + except Exception as exc: + print( + f"[sqlit] Failed to load custom keymap '{keymap_name}': {exc}", + file=sys.stderr, + ) + + def _resolve_keymap_path(self, keymap_name: str) -> Path: + if keymap_name.startswith(("~", "/")) or Path(keymap_name).is_absolute(): + return Path(keymap_name).expanduser() + + name = Path(keymap_name).stem + return CUSTOM_KEYMAP_DIR / f"{name}.json" + + def _register_custom_keymap(self, path: Path, keymap_name: str) -> None: + path = path.expanduser() + if not path.exists(): + raise ValueError(f"Keymap file not found: {path}") + + keymap = self._load_keymap_from_file(path, keymap_name) + set_keymap(keymap) + + def _load_keymap_from_file(self, path: Path, keymap_name: str) -> FileBasedKeymapProvider: + try: + payload = json.loads(path.read_text(encoding="utf-8")) + except Exception as exc: + raise ValueError(f"Failed to read keymap JSON: {exc}") from exc + + if not isinstance(payload, dict): + raise ValueError("Keymap file must contain a JSON object.") + + keymap_data = payload.get("keymap", payload) + if not isinstance(keymap_data, dict): + raise ValueError('Keymap file "keymap" must be a JSON object.') + + defaults = DefaultKeymapProvider() + base_action = defaults.get_action_keys() + base_leader = defaults.get_leader_commands() + + user_action_overrides = self._parse_action_overrides( + keymap_data.get("action_keys", {}), base_action + ) + user_leader_overrides = self._parse_leader_overrides( + keymap_data.get("leader_commands", {}), base_leader + ) + + merged_action = self._merge_action_keys(base_action, user_action_overrides) + merged_leader = self._merge_leader_commands(base_leader, user_leader_overrides) + + self._detect_conflicts( + merged_leader, + merged_action, + user_leader_overrides, + user_action_overrides, + ) + + return FileBasedKeymapProvider(keymap_name, merged_leader, merged_action) + + # ------------------------------------------------------------------ parsing + + @staticmethod + def _normalize_key_list(value: Any, where: str) -> list[str]: + if isinstance(value, str): + if not value: + raise ValueError(f"{where}: key must be a non-empty string.") + return [value] + if isinstance(value, list): + if not value: + raise ValueError(f"{where}: key list must contain at least one key.") + for k in value: + if not isinstance(k, str) or not k: + raise ValueError(f"{where}: every entry must be a non-empty string.") + return list(value) + raise ValueError(f"{where}: expected a string or list of strings.") + + @staticmethod + def _parse_action_overrides( + data: Any, base: list[ActionKeyDef] + ) -> list[ActionKeyDef]: + if not isinstance(data, dict): + raise ValueError('"action_keys" must be a JSON object keyed by state name.') + + # Catalog of defaults grouped by (action, context) — the primary entry + # carries the canonical guard/show/priority that we inherit for the + # user's rebound keys. + defaults_by_pair: dict[tuple[str, str | None], ActionKeyDef] = {} + for ak in base: + existing = defaults_by_pair.get((ak.action, ak.context)) + if existing is None or (ak.primary and not existing.primary): + defaults_by_pair[(ak.action, ak.context)] = ak + + actions_in_state: dict[str | None, set[str]] = defaultdict(set) + for ak in base: + actions_in_state[ak.context].add(ak.action) + + out: list[ActionKeyDef] = [] + for state, mapping in data.items(): + if not isinstance(state, str) or not state: + raise ValueError('action_keys keys must be non-empty state names.') + if not isinstance(mapping, dict): + raise ValueError(f'action_keys."{state}" must be an object of action → key.') + + for action, keys in mapping.items(): + if not isinstance(action, str) or not action: + raise ValueError(f'action_keys."{state}": action names must be non-empty strings.') + + template = defaults_by_pair.get((action, state)) + if template is None: + suggestions = sorted(actions_in_state.get(state, set())) + hint = ( + f" Known actions in this state: {suggestions}" if suggestions + else f" State {state!r} has no actions in defaults." + ) + raise ValueError( + f"Unknown action {action!r} in state {state!r}.{hint}" + ) + + key_list = KeymapManager._normalize_key_list( + keys, where=f'action_keys."{state}"."{action}"' + ) + for i, key in enumerate(key_list): + out.append( + ActionKeyDef( + key=key, + action=action, + context=state, + guard=template.guard, + primary=(i == 0), + show=template.show, + priority=template.priority, + ) + ) + return out + + @staticmethod + def _parse_leader_overrides( + data: Any, base: list[LeaderCommandDef] + ) -> list[LeaderCommandDef]: + if not isinstance(data, dict): + raise ValueError('"leader_commands" must be a JSON object keyed by menu name.') + + defaults_by_pair: dict[tuple[str, str], LeaderCommandDef] = { + (cmd.action, cmd.menu): cmd for cmd in base + } + actions_in_menu: dict[str, set[str]] = defaultdict(set) + for cmd in base: + actions_in_menu[cmd.menu].add(cmd.action) + + out: list[LeaderCommandDef] = [] + for menu, mapping in data.items(): + if not isinstance(menu, str) or not menu: + raise ValueError('leader_commands keys must be non-empty menu names.') + if not isinstance(mapping, dict): + raise ValueError(f'leader_commands."{menu}" must be an object of action → key.') + + for action, key in mapping.items(): + if not isinstance(action, str) or not action: + raise ValueError(f'leader_commands."{menu}": action names must be non-empty strings.') + + template = defaults_by_pair.get((action, menu)) + if template is None: + suggestions = sorted(actions_in_menu.get(menu, set())) + hint = ( + f" Known actions in this menu: {suggestions}" if suggestions + else f" Menu {menu!r} has no actions in defaults." + ) + raise ValueError( + f"Unknown leader action {action!r} in menu {menu!r}.{hint}" + ) + + # Leader commands are 1:1 — exactly one key per (action, menu). + if not isinstance(key, str) or not key: + raise ValueError( + f'leader_commands."{menu}"."{action}": expected a non-empty key string.' + ) + + out.append( + LeaderCommandDef( + key=key, + action=action, + label=template.label, + category=template.category, + guard=template.guard, + menu=menu, + ) + ) + return out + + # ------------------------------------------------------------------- merge + + @staticmethod + def _merge_action_keys( + base: list[ActionKeyDef], user: list[ActionKeyDef] + ) -> list[ActionKeyDef]: + # User overrides specify the COMPLETE key list for each (action, state) + # they touch — drop every default with that identity, then append. + overridden = {(u.action, u.context) for u in user} + kept = [ak for ak in base if (ak.action, ak.context) not in overridden] + return kept + user + + @staticmethod + def _merge_leader_commands( + base: list[LeaderCommandDef], user: list[LeaderCommandDef] + ) -> list[LeaderCommandDef]: + overridden = {(u.action, u.menu) for u in user} + kept = [cmd for cmd in base if (cmd.action, cmd.menu) not in overridden] + return kept + user + + # --------------------------------------------------------------- conflicts + + @staticmethod + def _detect_conflicts( + merged_leader: list[LeaderCommandDef], + merged_action: list[ActionKeyDef], + user_leader: list[LeaderCommandDef], + user_action: list[ActionKeyDef], + ) -> None: + """Raise ValueError on user-introduced bindings that collide. + + Defaults intentionally bind some keys to multiple actions in the + same state (e.g. ``d`` in ``tree`` for both delete_connection and + delete_connection_folder, disambiguated by tree-node state at + runtime). We don't flag those. We *do* flag any conflict that a + user override either created or contributed to. + """ + conflicts: list[str] = [] + + user_leader_slots = {(u.key, u.menu) for u in user_leader} + leader_by_slot: dict[tuple[str, str], set[str]] = defaultdict(set) + for cmd in merged_leader: + leader_by_slot[(cmd.key, cmd.menu)].add(cmd.action) + for (key, menu), actions in sorted(leader_by_slot.items()): + if len(actions) > 1 and (key, menu) in user_leader_slots: + conflicts.append( + f"leader key {key!r} in menu {menu!r} is bound to multiple actions: " + f"{sorted(actions)}" + ) + + user_action_slots = {(u.key, u.context) for u in user_action} + action_by_slot: dict[tuple[str, str | None], set[str]] = defaultdict(set) + for ak in merged_action: + action_by_slot[(ak.key, ak.context)].add(ak.action) + for (key, ctx), actions in sorted( + action_by_slot.items(), key=lambda t: (t[0][0], t[0][1] or "") + ): + if len(actions) > 1 and (key, ctx) in user_action_slots: + conflicts.append( + f"key {key!r} in state {ctx!r} is bound to multiple actions: " + f"{sorted(actions)}" + ) + + if conflicts: + lines = "\n - ".join(conflicts) + raise ValueError( + f"Conflicting keybindings detected ({len(conflicts)}):\n - {lines}\n" + f"Resolve by removing or rebinding the conflicting entries in your keymap." + ) diff --git a/sqlit/domains/shell/app/main.py b/sqlit/domains/shell/app/main.py index 71230f70..87f302cd 100644 --- a/sqlit/domains/shell/app/main.py +++ b/sqlit/domains/shell/app/main.py @@ -24,7 +24,7 @@ from sqlit.core.input_context import InputContext from sqlit.core.key_router import resolve_action -from sqlit.core.keymap_manager import KeymapManager +from sqlit.domains.shell.app.keymap_manager import KeymapManager from sqlit.core.vim import VimMode from sqlit.domains.connections.domain.config import ConnectionConfig from sqlit.domains.connections.providers.model import DatabaseProvider @@ -512,6 +512,13 @@ def _start_command_mode(self) -> None: self._command_buffer = "" self._update_status_bar() + def action_enter_command_mode(self) -> None: + # Entered by the keymap-bound key (default ":"). The query-insert + # guard lives in _handle_command_input so a user typing ":" in a + # SQL string still inserts it; this method is a no-op alias used + # only to satisfy the action validator for the keymap entry. + self._start_command_mode() + def _exit_command_mode(self) -> None: self._command_mode = False self._command_buffer = "" @@ -573,10 +580,12 @@ def _handle_count_digit(self, event: Key, ctx: InputContext) -> bool: return True def _handle_command_input(self, event: Key, ctx: InputContext) -> bool: + from sqlit.core.keymap import get_keymap from sqlit.core.vim import VimMode if not self._command_mode: - is_command_start = event.character == ":" or event.key in {":", "colon", "shift+semicolon"} + command_keys = get_keymap().keys_for_action("enter_command_mode") + is_command_start = event.key in command_keys or event.character in command_keys if not is_command_start: return False if ctx.focus == "query" and self.vim_mode == VimMode.INSERT: diff --git a/sqlit/domains/shell/app/startup_flow.py b/sqlit/domains/shell/app/startup_flow.py index d3af09ee..a325558f 100644 --- a/sqlit/domains/shell/app/startup_flow.py +++ b/sqlit/domains/shell/app/startup_flow.py @@ -38,8 +38,7 @@ def run_on_mount(app: AppProtocol) -> None: app._keymap_manager.initialize() # Feed the (possibly user-customized) keymap into Textual so that any # Binding with id= picks up the user's key. - from sqlit.core.keymap import get_keymap - from sqlit.core.keymap_manager import build_textual_keymap + from sqlit.core.keymap import build_textual_keymap, get_keymap app.set_keymap(build_textual_keymap(get_keymap())) app._startup_stamp("keymap_loaded") diff --git a/sqlit/domains/shell/ui/screens/help.py b/sqlit/domains/shell/ui/screens/help.py index 9147d512..d6f1fb32 100644 --- a/sqlit/domains/shell/ui/screens/help.py +++ b/sqlit/domains/shell/ui/screens/help.py @@ -15,11 +15,11 @@ class HelpScreen(ModalScreen): """Modal screen showing keyboard shortcuts and navigation tips.""" BINDINGS = [ - Binding("escape,enter,q", "dismiss", "Close", id="help_dismiss"), - Binding("j", "scroll_down", "Scroll down", show=False, id="help_scroll_down"), - Binding("k", "scroll_up", "Scroll up", show=False, id="help_scroll_up"), - Binding("g", "scroll_home", "Scroll to top", show=False, id="help_scroll_home"), - Binding("G", "scroll_end", "Scroll to bottom", show=False, id="help_scroll_end"), + Binding("escape,enter,q", "dismiss", "Close"), + Binding("j", "scroll_down", "Scroll down", show=False), + Binding("k", "scroll_up", "Scroll up", show=False), + Binding("g", "scroll_home", "Scroll to top", show=False), + Binding("G", "scroll_end", "Scroll to bottom", show=False), ] CSS = """ diff --git a/sqlit/shared/ui/screens/error.py b/sqlit/shared/ui/screens/error.py index ad68ae9d..d788e5ad 100644 --- a/sqlit/shared/ui/screens/error.py +++ b/sqlit/shared/ui/screens/error.py @@ -15,7 +15,7 @@ class ErrorScreen(ModalScreen): BINDINGS = [ Binding("enter,escape", "close", "Close"), - Binding("y", "copy_message", "Copy", id="error_copy_message"), + Binding("y", "copy_message", "Copy"), ] CSS = """ diff --git a/sqlit/shared/ui/screens/file_picker.py b/sqlit/shared/ui/screens/file_picker.py index 3ff79159..4302c6c5 100644 --- a/sqlit/shared/ui/screens/file_picker.py +++ b/sqlit/shared/ui/screens/file_picker.py @@ -31,7 +31,7 @@ class FilePickerScreen(ModalScreen[str | None]): BINDINGS = [ Binding("escape", "cancel", "Cancel", priority=True), Binding("tab,shift+tab", "toggle_focus", "Switch focus", show=False), - Binding("s", "save", "Save", show=False, id="file_picker_save"), + Binding("s", "save", "Save", show=False), ] CSS = """ diff --git a/tests/ui/keybindings/test_keymap_manager.py b/tests/ui/keybindings/test_keymap_manager.py index 3bb7ac5e..12f86113 100644 --- a/tests/ui/keybindings/test_keymap_manager.py +++ b/tests/ui/keybindings/test_keymap_manager.py @@ -1,4 +1,4 @@ -"""Tests for the KeymapManager (additive-merge model).""" +"""Tests for the state-nested KeymapManager.""" from __future__ import annotations @@ -7,13 +7,11 @@ import pytest -from sqlit.core.keymap import DefaultKeymapProvider, get_keymap, reset_keymap -from sqlit.core.keymap_manager import KeymapManager +from sqlit.core.keymap import get_keymap, reset_keymap +from sqlit.domains.shell.app.keymap_manager import FileBasedKeymapProvider, KeymapManager class MockSettingsStore: - """Mock settings store for testing.""" - def __init__(self, settings: dict | None = None): self.settings = settings or {} @@ -27,13 +25,13 @@ def get(self, key: str, default=None): return self.settings.get(key, default) -def _write_keymap(path: Path, payload: dict) -> Path: +def _write(path: Path, payload: dict) -> Path: path.write_text(json.dumps(payload), encoding="utf-8") return path def _load(tmp_path: Path, name: str, payload: dict) -> KeymapManager: - file_path = _write_keymap(tmp_path / f"{name}.json", payload) + file_path = _write(tmp_path / f"{name}.json", payload) manager = KeymapManager(settings_store=MockSettingsStore({"custom_keymap": str(file_path)})) manager.initialize() return manager @@ -45,374 +43,230 @@ def reset_keymap_after_test(): reset_keymap() -class TestKeymapManagerLifecycle: - def test_no_custom_keymap_uses_defaults(self): +class TestLifecycle: + def test_no_custom_keymap_leaves_defaults(self): manager = KeymapManager(settings_store=MockSettingsStore({})) - settings = manager.initialize() - assert settings == {} - assert manager.get_custom_keymap_name() is None - assert manager.get_custom_keymap_path() is None + manager.initialize() + assert not isinstance(get_keymap(), FileBasedKeymapProvider) def test_default_sentinel_skips_loading(self): manager = KeymapManager(settings_store=MockSettingsStore({"custom_keymap": "default"})) manager.initialize() - assert manager.get_custom_keymap_name() is None + assert not isinstance(get_keymap(), FileBasedKeymapProvider) - def test_invalid_json_is_reported_not_raised(self, tmp_path: Path, capsys): + def test_invalid_json_falls_back(self, tmp_path: Path, capsys): path = tmp_path / "invalid.json" path.write_text("not valid json", encoding="utf-8") - manager = KeymapManager(settings_store=MockSettingsStore({"custom_keymap": str(path)})) - manager.initialize() + KeymapManager(settings_store=MockSettingsStore({"custom_keymap": str(path)})).initialize() captured = capsys.readouterr() assert "Failed to load custom keymap" in captured.err - assert manager.get_custom_keymap_name() is None + assert not isinstance(get_keymap(), FileBasedKeymapProvider) - def test_missing_file_is_reported_not_raised(self, tmp_path: Path, capsys): - path = tmp_path / "nope.json" - manager = KeymapManager(settings_store=MockSettingsStore({"custom_keymap": str(path)})) - manager.initialize() + def test_missing_file_falls_back(self, tmp_path: Path, capsys): + KeymapManager( + settings_store=MockSettingsStore({"custom_keymap": str(tmp_path / "nope.json")}) + ).initialize() captured = capsys.readouterr() - assert "Failed to load custom keymap" in captured.err assert "not found" in captured.err - def test_reset_to_default(self, tmp_path: Path): - manager = _load( + +class TestSimpleRemap: + """Remapping a single key for an existing (state, action) pair.""" + + def test_single_key_replaces_default(self, tmp_path: Path): + _load( tmp_path, - "rebind", - {"keymap": {"leader_commands": [{"key": "Z", "action": "quit"}], "action_keys": []}}, + "remap", + {"keymap": {"action_keys": {"query_normal": {"execute_query": "ctrl+enter"}}}}, + ) + keymap = get_keymap() + assert keymap.action("execute_query") == "ctrl+enter" + # Old "enter" no longer triggers execute_query in query_normal. + assert not any( + ak.key == "enter" and ak.action == "execute_query" and ak.context == "query_normal" + for ak in keymap.get_action_keys() ) - assert manager.get_custom_keymap_name() is not None - manager.reset_to_default() - assert manager.get_custom_keymap_name() is None - assert manager.get_custom_keymap_path() is None - -class TestAdditiveMerge: - """Overrides should merge over defaults, not replace them.""" + def test_leader_remap(self, tmp_path: Path): + _load( + tmp_path, + "leader", + {"keymap": {"leader_commands": {"leader": {"show_help": "question_mark"}}}}, + ) + assert get_keymap().leader("show_help", "leader") == "question_mark" def test_unrelated_defaults_remain(self, tmp_path: Path): - """An override of one leader command must not erase the others.""" - defaults = DefaultKeymapProvider() - before = {(c.action, c.menu) for c in defaults.get_leader_commands()} - _load( tmp_path, "small", - {"keymap": {"leader_commands": [{"key": "Z", "action": "quit"}], "action_keys": []}}, + {"keymap": {"leader_commands": {"leader": {"quit": "Z"}}}}, ) + keymap = get_keymap() + # toggle_explorer wasn't touched — still on its default 'e'. + assert keymap.leader("toggle_explorer", "leader") == "e" - after = {(c.action, c.menu) for c in get_keymap().get_leader_commands()} - assert before == after, "merge must preserve every default binding's identity" - def test_leader_override_replaces_matching_default(self, tmp_path: Path): +class TestAliases: + """List values let the user keep aliases explicitly.""" + + def test_list_creates_primary_plus_aliases(self, tmp_path: Path): _load( tmp_path, - "override", - {"keymap": {"leader_commands": [{"key": "Z", "action": "quit"}], "action_keys": []}}, + "aliases", + {"keymap": {"action_keys": {"tree": {"refresh_tree": ["F", "ctrl+r"]}}}}, ) - keymap = get_keymap() - assert keymap.leader("quit", "leader") == "Z" - # Original "q" no longer maps to quit in the leader menu. - assert "quit" not in [c.action for c in keymap.get_leader_commands() if c.key == "q" and c.menu == "leader"] - - def test_action_key_override_replaces_matching_default(self, tmp_path: Path): + keys = get_keymap().keys_for_action("refresh_tree") + assert keys[0] == "F", "first entry is primary" + assert "ctrl+r" in keys, "alias is preserved" + # Default 'f' and 'R' both get replaced — user list is authoritative. + assert "f" not in keys + assert "R" not in keys + + def test_single_string_replaces_all_default_aliases(self, tmp_path: Path): + # Default tree_cursor_down has 'j' (primary) and 'down' (alias). + # Use ctrl+j to avoid colliding with other default tree bindings. _load( tmp_path, - "override-action", - { - "keymap": { - "leader_commands": [], - "action_keys": [ - {"key": "ctrl+enter", "action": "execute_query", "context": "query_normal"} - ], - } - }, - ) - keymap = get_keymap() - assert keymap.action("execute_query") == "ctrl+enter" - # Original "" no longer maps to execute_query in query_normal. - assert not any( - ak.key == "enter" and ak.action == "execute_query" and ak.context == "query_normal" - for ak in keymap.get_action_keys() + "single", + {"keymap": {"action_keys": {"tree": {"tree_cursor_down": "ctrl+j"}}}}, ) + keys = get_keymap().keys_for_action("tree_cursor_down") + assert keys == ["ctrl+j"], "single-string override replaces the entire key set" + - def test_missing_label_and_category_inherited_from_default(self, tmp_path: Path): - """Overrides that match a default should not require label/category.""" +class TestStrictness: + """Unknown (state, action) pairs are rejected with a helpful error.""" + + def test_unknown_action_in_state(self, tmp_path: Path, capsys): _load( tmp_path, - "inherit", - {"keymap": {"leader_commands": [{"key": "Z", "action": "quit"}], "action_keys": []}}, - ) - match = next( - c for c in get_keymap().get_leader_commands() - if c.action == "quit" and c.menu == "leader" + "bad", + {"keymap": {"action_keys": {"tree": {"this_action_does_not_exist": "x"}}}}, ) - assert match.key == "Z" - assert match.label == "Quit" # inherited - assert match.category == "Actions" # inherited + captured = capsys.readouterr() + assert "Unknown action 'this_action_does_not_exist' in state 'tree'" in captured.err + assert "Known actions" in captured.err - def test_new_binding_without_default_requires_label_and_category(self, tmp_path: Path, capsys): - """A brand-new (action, menu) pair must spell out label and category.""" + def test_unknown_state(self, tmp_path: Path, capsys): _load( tmp_path, - "new-no-meta", - { - "keymap": { - "leader_commands": [{"key": "X", "action": "some_made_up_action"}], - "action_keys": [], - } - }, + "bad-state", + {"keymap": {"action_keys": {"made_up_state": {"some_action": "x"}}}}, ) captured = capsys.readouterr() - assert "Failed to load custom keymap" in captured.err - assert '"label" is required' in captured.err + assert "Unknown action" in captured.err + assert "made_up_state" in captured.err - def test_new_binding_with_full_fields_is_appended(self, tmp_path: Path): + def test_unknown_leader_action(self, tmp_path: Path, capsys): _load( tmp_path, - "new-full", - { - "keymap": { - "leader_commands": [ - { - "key": "R", - "action": "refresh_tree", - "label": "Refresh tree", - "category": "View", - } - ], - "action_keys": [], - } - }, + "bad-leader", + {"keymap": {"leader_commands": {"leader": {"not_a_leader_action": "x"}}}}, ) - commands = get_keymap().get_leader_commands() - match = [c for c in commands if c.action == "refresh_tree" and c.menu == "leader"] - assert len(match) == 1 and match[0].key == "R" + captured = capsys.readouterr() + assert "Unknown leader action" in captured.err - def test_optional_action_key_flags_inherited(self, tmp_path: Path): - """primary/show/priority should come from the matching default when omitted.""" - # leader_key in "global" context is primary=True, priority=True by default. + def test_empty_key_string(self, tmp_path: Path, capsys): _load( tmp_path, - "inherit-flags", - { - "keymap": { - "leader_commands": [], - "action_keys": [ - {"key": "comma", "action": "leader_key", "context": "global"} - ], - } - }, + "empty", + {"keymap": {"action_keys": {"tree": {"refresh_tree": ""}}}}, ) - match = next( - ak for ak in get_keymap().get_action_keys() - if ak.action == "leader_key" and ak.context == "global" - ) - assert match.key == "comma" - assert match.priority is True # inherited from default - assert match.primary is True # inherited from default - - -class TestAliasPreservation: - """Secondary defaults (primary=False) should survive when only the primary slot is overridden.""" + captured = capsys.readouterr() + assert "key must be a non-empty string" in captured.err - def test_secondary_alias_survives_primary_override(self, tmp_path: Path): - # refresh_tree in 'tree' has 'f' (primary) and 'R' (alias) by default. + def test_empty_key_list(self, tmp_path: Path, capsys): _load( tmp_path, - "alias", - { - "keymap": { - "leader_commands": [], - "action_keys": [ - {"key": "F", "action": "refresh_tree", "context": "tree"} - ], - } - }, + "empty-list", + {"keymap": {"action_keys": {"tree": {"refresh_tree": []}}}}, ) - keys = get_keymap().keys_for_action("refresh_tree") - assert "F" in keys, "user override should be present" - assert "R" in keys, "secondary alias 'R' must not be dropped" - assert "f" not in keys, "primary default 'f' should be replaced" + captured = capsys.readouterr() + assert "key list must contain at least one key" in captured.err -class TestConflictDetection: - """User-introduced same-key/different-action collisions abort load.""" +class TestConflicts: + """User-introduced collisions abort load with a clear error.""" - def test_user_creates_conflict_with_default(self, tmp_path: Path, capsys): + def test_user_vs_default_action_key(self, tmp_path: Path, capsys): # Default: 'i' → enter_insert_mode in query_normal. User binds 'i' to undo. _load( tmp_path, "conflict", - { - "keymap": { - "leader_commands": [], - "action_keys": [ - {"key": "i", "action": "undo", "context": "query_normal"} - ], - } - }, + {"keymap": {"action_keys": {"query_normal": {"undo": "i"}}}}, ) captured = capsys.readouterr() assert "Conflicting keybindings detected" in captured.err - assert "'i'" in captured.err - assert "query_normal" in captured.err + assert "'i'" in captured.err and "query_normal" in captured.err - def test_user_creates_conflict_with_self(self, tmp_path: Path, capsys): - # Two user overrides claiming the same key in the same context. + def test_user_vs_user_action_key(self, tmp_path: Path, capsys): _load( tmp_path, "self-conflict", { "keymap": { - "leader_commands": [], - "action_keys": [ - {"key": "ctrl+x", "action": "undo", "context": "query_normal"}, - {"key": "ctrl+x", "action": "redo", "context": "query_normal"}, - ], + "action_keys": { + "query_normal": { + "undo": "ctrl+x", + "redo": "ctrl+x", + } + } } }, ) captured = capsys.readouterr() assert "Conflicting keybindings detected" in captured.err - def test_default_conflicts_are_tolerated(self, tmp_path: Path, capsys): - # 'd' in tree maps to both delete_connection and delete_connection_folder - # by default (resolved by tree-node state at runtime). User does NOT touch - # those — loading an unrelated override must not flag pre-existing overlaps. + def test_user_vs_default_leader(self, tmp_path: Path, capsys): + # Default: e → toggle_explorer. User binds e → quit. _load( tmp_path, - "no-touch", - { - "keymap": { - "leader_commands": [], - "action_keys": [ - {"key": "Z", "action": "undo", "context": "query_normal"} - ], - } - }, + "leader-conflict", + {"keymap": {"leader_commands": {"leader": {"quit": "e"}}}}, ) captured = capsys.readouterr() - assert "Conflicting" not in captured.err + assert "leader key 'e'" in captured.err - def test_leader_conflict(self, tmp_path: Path, capsys): - # User binds h to two different actions in the same menu. + def test_default_only_overlaps_are_tolerated(self, tmp_path: Path, capsys): + # 'd' in tree binds two actions in defaults (state-guarded at runtime). + # An unrelated user override must not trip on that pre-existing overlap. _load( tmp_path, - "leader-conflict", - { - "keymap": { - "leader_commands": [ - {"key": "h", "action": "show_help"}, - { - "key": "h", - "action": "refresh_tree", - "label": "Refresh", - "category": "View", - }, - ], - "action_keys": [], - } - }, + "untouched", + {"keymap": {"action_keys": {"query_normal": {"undo": "Z"}}}}, ) captured = capsys.readouterr() - assert "leader key 'h'" in captured.err + assert "Conflicting" not in captured.err -class TestContextRequired: - """Omitting context for a context-scoped action is a config trap, not a no-op.""" +class TestMetadataInheritance: + """Action metadata (label, category, guard, priority, show) always comes from defaults.""" - def test_missing_context_is_rejected(self, tmp_path: Path, capsys): + def test_leader_inherits_label_and_category(self, tmp_path: Path): _load( tmp_path, - "no-context", - { - "keymap": { - "leader_commands": [], - "action_keys": [ - {"key": "ctrl+enter", "action": "execute_query"} - ], - } - }, - ) - captured = capsys.readouterr() - assert "missing \"context\"" in captured.err - assert "query_normal" in captured.err - - def test_explicit_null_context_is_allowed(self, tmp_path: Path): - """Setting context to null (when defaults have no null binding) errors only - if a conflict results — explicit null is the user opting into a global - binding. Here we use an action that has no default at all, so it's clean.""" - _load( - tmp_path, - "null-ctx", - { - "keymap": { - "leader_commands": [], - "action_keys": [ - {"key": "ctrl+shift+z", "action": "my_custom_action", "context": None} - ], - } - }, + "leader-meta", + {"keymap": {"leader_commands": {"leader": {"show_help": "question_mark"}}}}, ) match = next( - ak for ak in get_keymap().get_action_keys() - if ak.action == "my_custom_action" - ) - assert match.key == "ctrl+shift+z" - assert match.context is None - - -class TestParsing: - def test_nested_keymap_object(self, tmp_path: Path): - _load( - tmp_path, - "nested", - {"keymap": {"leader_commands": [{"key": "Q", "action": "quit"}], "action_keys": []}}, + c for c in get_keymap().get_leader_commands() + if c.action == "show_help" and c.menu == "leader" ) - assert get_keymap().leader("quit") == "Q" + assert match.key == "question_mark" + assert match.label == "Help" # inherited + assert match.category == "Actions" # inherited - def test_action_key_with_explicit_fields(self, tmp_path: Path): - # Use a key that doesn't collide with any default in query_normal. + def test_action_key_inherits_flags(self, tmp_path: Path): + # leader_key in 'global' is primary=True, priority=True by default. _load( tmp_path, - "explicit", - { - "keymap": { - "leader_commands": [], - "action_keys": [ - { - "key": "ctrl+i", - "action": "enter_insert_mode", - "context": "query_normal", - "primary": False, - "show": True, - "priority": True, - } - ], - } - }, + "flags", + {"keymap": {"action_keys": {"global": {"leader_key": "comma"}}}}, ) match = next( ak for ak in get_keymap().get_action_keys() - if ak.action == "enter_insert_mode" and ak.context == "query_normal" - and ak.key == "ctrl+i" + if ak.action == "leader_key" and ak.context == "global" ) - assert match.primary is False - assert match.show is True + assert match.key == "comma" + assert match.primary is True assert match.priority is True - - def test_missing_action_field_is_rejected(self, tmp_path: Path, capsys): - _load( - tmp_path, - "no-action", - { - "keymap": { - "leader_commands": [], - "action_keys": [{"key": "i"}], - } - }, - ) - captured = capsys.readouterr() - assert "Failed to load custom keymap" in captured.err - assert "missing required" in captured.err From 1ba4bda7dd97b4da9b12b87ab76ab49b942031ed Mon Sep 17 00:00:00 2001 From: Peter Adams <18162810+Maxteabag@users.noreply.github.com> Date: Mon, 25 May 2026 00:23:05 +0200 Subject: [PATCH 04/12] feat(keybindings): surface keymap load errors via app.notify When a custom keymap fails to load (invalid JSON, unknown action, conflicting bindings), the error now shows up as an error-severity notification in the TUI instead of being buried in stderr where a running-in-a-terminal user never sees it. KeymapManager stores the message on self.load_error; startup_flow picks it up after mount and surfaces it via app.notify, falling back to stderr on headless or when notify fails. Mirrors the existing _warn_on_missing_actions / _warn_on_keyring_error pattern. --- docs/custom-keymaps.md | 4 +- sqlit/domains/shell/app/keymap_manager.py | 11 +- sqlit/domains/shell/app/startup_flow.py | 16 +++ tests/ui/keybindings/test_keymap_manager.py | 152 +++++++++++++------- 4 files changed, 128 insertions(+), 55 deletions(-) diff --git a/docs/custom-keymaps.md b/docs/custom-keymaps.md index 3d8c562e..4d83854e 100644 --- a/docs/custom-keymaps.md +++ b/docs/custom-keymaps.md @@ -61,7 +61,7 @@ You also cannot invent new actions or attach an existing action to a state it do ## Validation at startup -The keymap is checked before the app boots. Failures fall back to defaults and print to stderr: +The keymap is checked before the app boots. Failures fall back to defaults; the error surfaces as a Textual notification in the running app, and as a `[sqlit] …` line on stderr when running headless. Possible failures: - **Unknown action in a state** — sqlit lists the actions that *do* exist in that state. - **Conflicting bindings** — two different actions claiming the same key in the same state (or the same menu, for leader commands). Default-only overlaps that are disambiguated by runtime state — e.g. `d` on a connection vs. on a folder, both in `tree` — are tolerated; only collisions you introduce are flagged. @@ -141,7 +141,7 @@ Rebind autocomplete to `ctrl+n`/`ctrl+p`, swap `h` to `?`, keep ## Troubleshooting -**Keymap didn't load.** Check `~/.sqlit/settings.json` — `custom_keymap` must match your filename without the `.json`. Check stderr for `[sqlit] Failed to load custom keymap …`. +**Keymap didn't load.** Check `~/.sqlit/settings.json` — `custom_keymap` must match your filename without the `.json`. The app shows a notification on startup; running headless prints `[sqlit] Failed to load custom keymap …` to stderr. **"Unknown action" error.** The action doesn't exist in the state you named. The error message lists the known actions for that state — pick one of those, or check the canonical list in `sqlit/core/keymap.py`. diff --git a/sqlit/domains/shell/app/keymap_manager.py b/sqlit/domains/shell/app/keymap_manager.py index 99096a3a..14d7c22d 100644 --- a/sqlit/domains/shell/app/keymap_manager.py +++ b/sqlit/domains/shell/app/keymap_manager.py @@ -36,7 +36,6 @@ from __future__ import annotations import json -import sys from collections import defaultdict from pathlib import Path from typing import Any @@ -84,6 +83,10 @@ class KeymapManager: def __init__(self, settings_store: SettingsStoreProtocol) -> None: self._settings_store = settings_store + # Last load error surfaced by startup_flow once the app is mounted, + # so the user sees it in the UI instead of only on stderr. None when + # the most recent load succeeded or no custom keymap was requested. + self.load_error: str | None = None def initialize(self) -> dict: settings = self._settings_store.load_all() @@ -91,6 +94,7 @@ def initialize(self) -> dict: return settings def load_custom_keymap(self, settings: dict) -> None: + self.load_error = None keymap_name = settings.get(CUSTOM_KEYMAP_SETTINGS_KEY) if not keymap_name or not isinstance(keymap_name, str): return @@ -101,10 +105,7 @@ def load_custom_keymap(self, settings: dict) -> None: path = self._resolve_keymap_path(keymap_name.strip()) self._register_custom_keymap(path, keymap_name.strip()) except Exception as exc: - print( - f"[sqlit] Failed to load custom keymap '{keymap_name}': {exc}", - file=sys.stderr, - ) + self.load_error = f"Failed to load custom keymap '{keymap_name}': {exc}" def _resolve_keymap_path(self, keymap_name: str) -> Path: if keymap_name.startswith(("~", "/")) or Path(keymap_name).is_absolute(): diff --git a/sqlit/domains/shell/app/startup_flow.py b/sqlit/domains/shell/app/startup_flow.py index a325558f..02637a3e 100644 --- a/sqlit/domains/shell/app/startup_flow.py +++ b/sqlit/domains/shell/app/startup_flow.py @@ -106,6 +106,7 @@ def run_on_mount(app: AppProtocol) -> None: app._startup_stamp("footer_updated") _warn_on_missing_actions(app, is_headless) _warn_on_keyring_error(app, is_headless) + _warn_on_keymap_error(app, is_headless) if ( app.services.runtime.process_worker and app.services.runtime.process_worker_warm_on_idle @@ -164,6 +165,21 @@ def _warn_on_keyring_error(app: AppProtocol, is_headless: bool) -> None: print(f"[sqlit] {message}", file=sys.stderr) +def _warn_on_keymap_error(app: AppProtocol, is_headless: bool) -> None: + manager = getattr(app, "_keymap_manager", None) + error = getattr(manager, "load_error", None) if manager else None + if not error: + return + message = f"{error}. Defaults are in effect — fix ~/.sqlit/keymaps/ and restart." + if is_headless: + print(f"[sqlit] {message}", file=sys.stderr) + return + try: + app.notify(message, severity="error", timeout=15) + except Exception: + print(f"[sqlit] {message}", file=sys.stderr) + + def apply_mock_settings(app: AppProtocol, settings: dict) -> None: app.services.apply_mock_settings(settings) diff --git a/tests/ui/keybindings/test_keymap_manager.py b/tests/ui/keybindings/test_keymap_manager.py index 12f86113..1ce408c8 100644 --- a/tests/ui/keybindings/test_keymap_manager.py +++ b/tests/ui/keybindings/test_keymap_manager.py @@ -54,20 +54,31 @@ def test_default_sentinel_skips_loading(self): manager.initialize() assert not isinstance(get_keymap(), FileBasedKeymapProvider) - def test_invalid_json_falls_back(self, tmp_path: Path, capsys): + def test_invalid_json_falls_back(self, tmp_path: Path): path = tmp_path / "invalid.json" path.write_text("not valid json", encoding="utf-8") - KeymapManager(settings_store=MockSettingsStore({"custom_keymap": str(path)})).initialize() - captured = capsys.readouterr() - assert "Failed to load custom keymap" in captured.err + manager = KeymapManager(settings_store=MockSettingsStore({"custom_keymap": str(path)})) + manager.initialize() + assert manager.load_error and "Failed to load custom keymap" in manager.load_error assert not isinstance(get_keymap(), FileBasedKeymapProvider) - def test_missing_file_falls_back(self, tmp_path: Path, capsys): - KeymapManager( + def test_missing_file_falls_back(self, tmp_path: Path): + manager = KeymapManager( settings_store=MockSettingsStore({"custom_keymap": str(tmp_path / "nope.json")}) - ).initialize() - captured = capsys.readouterr() - assert "not found" in captured.err + ) + manager.initialize() + assert manager.load_error and "not found" in manager.load_error + + def test_load_error_cleared_on_clean_reinit(self, tmp_path: Path): + path = tmp_path / "bad.json" + path.write_text("not valid json", encoding="utf-8") + manager = KeymapManager(settings_store=MockSettingsStore({"custom_keymap": str(path)})) + manager.initialize() + assert manager.load_error is not None + # Re-running with the default sentinel must clear the prior error. + manager._settings_store = MockSettingsStore({"custom_keymap": "default"}) # type: ignore[attr-defined] + manager.initialize() + assert manager.load_error is None class TestSimpleRemap: @@ -137,70 +148,70 @@ def test_single_string_replaces_all_default_aliases(self, tmp_path: Path): class TestStrictness: """Unknown (state, action) pairs are rejected with a helpful error.""" - def test_unknown_action_in_state(self, tmp_path: Path, capsys): - _load( + def test_unknown_action_in_state(self, tmp_path: Path): + manager = _load( tmp_path, "bad", {"keymap": {"action_keys": {"tree": {"this_action_does_not_exist": "x"}}}}, ) - captured = capsys.readouterr() - assert "Unknown action 'this_action_does_not_exist' in state 'tree'" in captured.err - assert "Known actions" in captured.err + assert manager.load_error is not None + assert "Unknown action 'this_action_does_not_exist' in state 'tree'" in manager.load_error + assert "Known actions" in manager.load_error - def test_unknown_state(self, tmp_path: Path, capsys): - _load( + def test_unknown_state(self, tmp_path: Path): + manager = _load( tmp_path, "bad-state", {"keymap": {"action_keys": {"made_up_state": {"some_action": "x"}}}}, ) - captured = capsys.readouterr() - assert "Unknown action" in captured.err - assert "made_up_state" in captured.err + assert manager.load_error is not None + assert "Unknown action" in manager.load_error + assert "made_up_state" in manager.load_error - def test_unknown_leader_action(self, tmp_path: Path, capsys): - _load( + def test_unknown_leader_action(self, tmp_path: Path): + manager = _load( tmp_path, "bad-leader", {"keymap": {"leader_commands": {"leader": {"not_a_leader_action": "x"}}}}, ) - captured = capsys.readouterr() - assert "Unknown leader action" in captured.err + assert manager.load_error is not None + assert "Unknown leader action" in manager.load_error - def test_empty_key_string(self, tmp_path: Path, capsys): - _load( + def test_empty_key_string(self, tmp_path: Path): + manager = _load( tmp_path, "empty", {"keymap": {"action_keys": {"tree": {"refresh_tree": ""}}}}, ) - captured = capsys.readouterr() - assert "key must be a non-empty string" in captured.err + assert manager.load_error is not None + assert "key must be a non-empty string" in manager.load_error - def test_empty_key_list(self, tmp_path: Path, capsys): - _load( + def test_empty_key_list(self, tmp_path: Path): + manager = _load( tmp_path, "empty-list", {"keymap": {"action_keys": {"tree": {"refresh_tree": []}}}}, ) - captured = capsys.readouterr() - assert "key list must contain at least one key" in captured.err + assert manager.load_error is not None + assert "key list must contain at least one key" in manager.load_error class TestConflicts: """User-introduced collisions abort load with a clear error.""" - def test_user_vs_default_action_key(self, tmp_path: Path, capsys): + def test_user_vs_default_action_key(self, tmp_path: Path): # Default: 'i' → enter_insert_mode in query_normal. User binds 'i' to undo. - _load( + manager = _load( tmp_path, "conflict", {"keymap": {"action_keys": {"query_normal": {"undo": "i"}}}}, ) - captured = capsys.readouterr() - assert "Conflicting keybindings detected" in captured.err - assert "'i'" in captured.err and "query_normal" in captured.err + assert manager.load_error is not None + assert "Conflicting keybindings detected" in manager.load_error + assert "'i'" in manager.load_error and "query_normal" in manager.load_error - def test_user_vs_user_action_key(self, tmp_path: Path, capsys): - _load( + def test_user_vs_user_action_key(self, tmp_path: Path): + manager = _load( tmp_path, "self-conflict", { @@ -214,29 +225,74 @@ def test_user_vs_user_action_key(self, tmp_path: Path, capsys): } }, ) - captured = capsys.readouterr() - assert "Conflicting keybindings detected" in captured.err + assert manager.load_error is not None + assert "Conflicting keybindings detected" in manager.load_error - def test_user_vs_default_leader(self, tmp_path: Path, capsys): + def test_user_vs_default_leader(self, tmp_path: Path): # Default: e → toggle_explorer. User binds e → quit. - _load( + manager = _load( tmp_path, "leader-conflict", {"keymap": {"leader_commands": {"leader": {"quit": "e"}}}}, ) - captured = capsys.readouterr() - assert "leader key 'e'" in captured.err + assert manager.load_error is not None + assert "leader key 'e'" in manager.load_error - def test_default_only_overlaps_are_tolerated(self, tmp_path: Path, capsys): + def test_default_only_overlaps_are_tolerated(self, tmp_path: Path): # 'd' in tree binds two actions in defaults (state-guarded at runtime). # An unrelated user override must not trip on that pre-existing overlap. - _load( + manager = _load( tmp_path, "untouched", {"keymap": {"action_keys": {"query_normal": {"undo": "Z"}}}}, ) - captured = capsys.readouterr() - assert "Conflicting" not in captured.err + assert manager.load_error is None + + +class TestStartupNotification: + """The load error is surfaced to the user via app.notify on startup.""" + + def test_notifies_when_load_error_present(self, tmp_path: Path): + from sqlit.domains.shell.app.startup_flow import _warn_on_keymap_error + + path = tmp_path / "bad.json" + path.write_text("not valid json", encoding="utf-8") + manager = KeymapManager(settings_store=MockSettingsStore({"custom_keymap": str(path)})) + manager.initialize() + assert manager.load_error is not None + + notifications: list[tuple[str, str]] = [] + + class FakeApp: + _keymap_manager = manager + + def notify(self, message: str, *, severity: str = "information", timeout: float = 5) -> None: + notifications.append((severity, message)) + + _warn_on_keymap_error(FakeApp(), is_headless=False) # type: ignore[arg-type] + assert len(notifications) == 1 + severity, message = notifications[0] + assert severity == "error" + assert "Failed to load custom keymap" in message + assert "Defaults are in effect" in message + + def test_silent_when_no_load_error(self, tmp_path: Path): + from sqlit.domains.shell.app.startup_flow import _warn_on_keymap_error + + manager = KeymapManager(settings_store=MockSettingsStore({})) + manager.initialize() + assert manager.load_error is None + + notifications: list[tuple[str, str]] = [] + + class FakeApp: + _keymap_manager = manager + + def notify(self, message: str, *, severity: str = "information", timeout: float = 5) -> None: + notifications.append((severity, message)) + + _warn_on_keymap_error(FakeApp(), is_headless=False) # type: ignore[arg-type] + assert notifications == [] class TestMetadataInheritance: From bb228b2726327204b44e8d5a71b8b0eebfb2003c Mon Sep 17 00:00:00 2001 From: Peter Adams <18162810+Maxteabag@users.noreply.github.com> Date: Mon, 25 May 2026 00:35:47 +0200 Subject: [PATCH 05/12] fix(keybindings): defer keymap-error notification until screen is ready MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit app.notify() raises NoMatches mid-run_on_mount because the notification rack's refresh chain touches widgets (#results-area DataTable) that aren't mounted until after refresh. The notify exception was swallowed and the user saw nothing. Wrap the notify call in call_after_refresh so it fires once the initial paint completes. The existing _warn_on_missing_actions and _warn_on_keyring_error have the same latent bug — left untouched here since they rarely fire in practice. --- sqlit/domains/shell/app/startup_flow.py | 14 ++++++++++---- tests/ui/keybindings/test_keymap_manager.py | 14 ++++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/sqlit/domains/shell/app/startup_flow.py b/sqlit/domains/shell/app/startup_flow.py index 02637a3e..eb80c385 100644 --- a/sqlit/domains/shell/app/startup_flow.py +++ b/sqlit/domains/shell/app/startup_flow.py @@ -174,10 +174,16 @@ def _warn_on_keymap_error(app: AppProtocol, is_headless: bool) -> None: if is_headless: print(f"[sqlit] {message}", file=sys.stderr) return - try: - app.notify(message, severity="error", timeout=15) - except Exception: - print(f"[sqlit] {message}", file=sys.stderr) + # Defer until the screen is composed — calling notify() during + # run_on_mount refreshes the notification rack, which touches widgets + # (`#results-area DataTable`) that aren't mounted yet. + def _show() -> None: + try: + app.notify(message, severity="error", timeout=15) + except Exception: + print(f"[sqlit] {message}", file=sys.stderr) + + app.call_after_refresh(_show) def apply_mock_settings(app: AppProtocol, settings: dict) -> None: diff --git a/tests/ui/keybindings/test_keymap_manager.py b/tests/ui/keybindings/test_keymap_manager.py index 1ce408c8..8cc30f5d 100644 --- a/tests/ui/keybindings/test_keymap_manager.py +++ b/tests/ui/keybindings/test_keymap_manager.py @@ -262,6 +262,7 @@ def test_notifies_when_load_error_present(self, tmp_path: Path): assert manager.load_error is not None notifications: list[tuple[str, str]] = [] + scheduled: list = [] class FakeApp: _keymap_manager = manager @@ -269,7 +270,14 @@ class FakeApp: def notify(self, message: str, *, severity: str = "information", timeout: float = 5) -> None: notifications.append((severity, message)) + def call_after_refresh(self, callback, *args, **kwargs) -> None: + scheduled.append((callback, args, kwargs)) + _warn_on_keymap_error(FakeApp(), is_headless=False) # type: ignore[arg-type] + # notify is deferred via call_after_refresh; run the scheduled callback. + assert len(scheduled) == 1 + callback, args, kwargs = scheduled[0] + callback(*args, **kwargs) assert len(notifications) == 1 severity, message = notifications[0] assert severity == "error" @@ -283,6 +291,7 @@ def test_silent_when_no_load_error(self, tmp_path: Path): manager.initialize() assert manager.load_error is None + scheduled: list = [] notifications: list[tuple[str, str]] = [] class FakeApp: @@ -291,7 +300,12 @@ class FakeApp: def notify(self, message: str, *, severity: str = "information", timeout: float = 5) -> None: notifications.append((severity, message)) + def call_after_refresh(self, callback, *args, **kwargs) -> None: + scheduled.append((callback, args, kwargs)) + _warn_on_keymap_error(FakeApp(), is_headless=False) # type: ignore[arg-type] + # Nothing scheduled or notified when load_error is None. + assert scheduled == [] assert notifications == [] From ed7e8cc2b1f4d12b27d427d816c5f3e972b76845 Mon Sep 17 00:00:00 2001 From: Peter Adams <18162810+Maxteabag@users.noreply.github.com> Date: Mon, 25 May 2026 00:55:22 +0200 Subject: [PATCH 06/12] feat(keybindings): allow unbinding defaults via null MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Setting a key to null/""/[] in the keymap removes the default for that (action, state) — no key triggers it anymore. Makes the natural case where the user wants a key already taken by another default trivially solvable: "query_normal": { "enter_insert_mode": "u", "undo": null } Previously this would fail with "key 'u' is bound to multiple actions" and the user had no in-config way to resolve it. The conflict-error message now points at the null-unbind option, and the modal's "defaults are in effect" hint uses the resolved CUSTOM_KEYMAP_DIR instead of the hardcoded legacy ~/.sqlit/keymaps/ path. --- docs/custom-keymaps.md | 11 +++- sqlit/domains/shell/app/keymap_manager.py | 65 ++++++++++++------ sqlit/domains/shell/app/startup_flow.py | 3 +- tests/ui/keybindings/test_keymap_manager.py | 73 ++++++++++++++++++--- 4 files changed, 121 insertions(+), 31 deletions(-) diff --git a/docs/custom-keymaps.md b/docs/custom-keymaps.md index 4d83854e..b24481b9 100644 --- a/docs/custom-keymaps.md +++ b/docs/custom-keymaps.md @@ -49,7 +49,16 @@ Values can be a single key string, or a list to provide aliases (the first entry } ``` -A single string *replaces every default key* for that action in that state. A list lets you control the full set of keys explicitly. +A single string *replaces every default key* for that action in that state. A list lets you control the full set of keys explicitly. **Set the value to `null` to unbind a default entirely** — useful when you want to claim a key that already has a default action: + +```json +"query_normal": { + "enter_insert_mode": "u", + "undo": null +} +``` + +Here `u` enters insert mode and the default `u → undo` binding is removed (no key triggers undo in `query_normal` anymore). Without the `undo: null`, the loader would reject the keymap because two actions would be claiming `u`. ## What you can change — and what you cannot diff --git a/sqlit/domains/shell/app/keymap_manager.py b/sqlit/domains/shell/app/keymap_manager.py index 14d7c22d..1f341d48 100644 --- a/sqlit/domains/shell/app/keymap_manager.py +++ b/sqlit/domains/shell/app/keymap_manager.py @@ -139,15 +139,19 @@ def _load_keymap_from_file(self, path: Path, keymap_name: str) -> FileBasedKeyma base_action = defaults.get_action_keys() base_leader = defaults.get_leader_commands() - user_action_overrides = self._parse_action_overrides( + user_action_overrides, action_unbinds = self._parse_action_overrides( keymap_data.get("action_keys", {}), base_action ) - user_leader_overrides = self._parse_leader_overrides( + user_leader_overrides, leader_unbinds = self._parse_leader_overrides( keymap_data.get("leader_commands", {}), base_leader ) - merged_action = self._merge_action_keys(base_action, user_action_overrides) - merged_leader = self._merge_leader_commands(base_leader, user_leader_overrides) + merged_action = self._merge_action_keys( + base_action, user_action_overrides, action_unbinds + ) + merged_leader = self._merge_leader_commands( + base_leader, user_leader_overrides, leader_unbinds + ) self._detect_conflicts( merged_leader, @@ -161,24 +165,30 @@ def _load_keymap_from_file(self, path: Path, keymap_name: str) -> FileBasedKeyma # ------------------------------------------------------------------ parsing @staticmethod - def _normalize_key_list(value: Any, where: str) -> list[str]: + def _normalize_key_list(value: Any, where: str) -> list[str] | None: + """Return the user's key list, or None to mean "unbind this action". + + Accepts `null`, `""`, and `[]` as unbind sentinels. + """ + if value is None: + return None if isinstance(value, str): if not value: - raise ValueError(f"{where}: key must be a non-empty string.") + return None return [value] if isinstance(value, list): if not value: - raise ValueError(f"{where}: key list must contain at least one key.") + return None for k in value: if not isinstance(k, str) or not k: raise ValueError(f"{where}: every entry must be a non-empty string.") return list(value) - raise ValueError(f"{where}: expected a string or list of strings.") + raise ValueError(f"{where}: expected a string, list of strings, or null to unbind.") @staticmethod def _parse_action_overrides( data: Any, base: list[ActionKeyDef] - ) -> list[ActionKeyDef]: + ) -> tuple[list[ActionKeyDef], set[tuple[str, str | None]]]: if not isinstance(data, dict): raise ValueError('"action_keys" must be a JSON object keyed by state name.') @@ -196,6 +206,7 @@ def _parse_action_overrides( actions_in_state[ak.context].add(ak.action) out: list[ActionKeyDef] = [] + unbinds: set[tuple[str, str | None]] = set() for state, mapping in data.items(): if not isinstance(state, str) or not state: raise ValueError('action_keys keys must be non-empty state names.') @@ -220,6 +231,9 @@ def _parse_action_overrides( key_list = KeymapManager._normalize_key_list( keys, where=f'action_keys."{state}"."{action}"' ) + if key_list is None: + unbinds.add((action, state)) + continue for i, key in enumerate(key_list): out.append( ActionKeyDef( @@ -232,12 +246,12 @@ def _parse_action_overrides( priority=template.priority, ) ) - return out + return out, unbinds @staticmethod def _parse_leader_overrides( data: Any, base: list[LeaderCommandDef] - ) -> list[LeaderCommandDef]: + ) -> tuple[list[LeaderCommandDef], set[tuple[str, str]]]: if not isinstance(data, dict): raise ValueError('"leader_commands" must be a JSON object keyed by menu name.') @@ -249,6 +263,7 @@ def _parse_leader_overrides( actions_in_menu[cmd.menu].add(cmd.action) out: list[LeaderCommandDef] = [] + unbinds: set[tuple[str, str]] = set() for menu, mapping in data.items(): if not isinstance(menu, str) or not menu: raise ValueError('leader_commands keys must be non-empty menu names.') @@ -270,10 +285,14 @@ def _parse_leader_overrides( f"Unknown leader action {action!r} in menu {menu!r}.{hint}" ) - # Leader commands are 1:1 — exactly one key per (action, menu). - if not isinstance(key, str) or not key: + # null / "" unbinds the default for this (action, menu). + if key is None or key == "": + unbinds.add((action, menu)) + continue + + if not isinstance(key, str): raise ValueError( - f'leader_commands."{menu}"."{action}": expected a non-empty key string.' + f'leader_commands."{menu}"."{action}": expected a key string or null.' ) out.append( @@ -286,25 +305,30 @@ def _parse_leader_overrides( menu=menu, ) ) - return out + return out, unbinds # ------------------------------------------------------------------- merge @staticmethod def _merge_action_keys( - base: list[ActionKeyDef], user: list[ActionKeyDef] + base: list[ActionKeyDef], + user: list[ActionKeyDef], + unbinds: set[tuple[str, str | None]], ) -> list[ActionKeyDef]: # User overrides specify the COMPLETE key list for each (action, state) # they touch — drop every default with that identity, then append. - overridden = {(u.action, u.context) for u in user} + # Unbinds drop the defaults without adding anything. + overridden = {(u.action, u.context) for u in user} | unbinds kept = [ak for ak in base if (ak.action, ak.context) not in overridden] return kept + user @staticmethod def _merge_leader_commands( - base: list[LeaderCommandDef], user: list[LeaderCommandDef] + base: list[LeaderCommandDef], + user: list[LeaderCommandDef], + unbinds: set[tuple[str, str]], ) -> list[LeaderCommandDef]: - overridden = {(u.action, u.menu) for u in user} + overridden = {(u.action, u.menu) for u in user} | unbinds kept = [cmd for cmd in base if (cmd.action, cmd.menu) not in overridden] return kept + user @@ -355,5 +379,6 @@ def _detect_conflicts( lines = "\n - ".join(conflicts) raise ValueError( f"Conflicting keybindings detected ({len(conflicts)}):\n - {lines}\n" - f"Resolve by removing or rebinding the conflicting entries in your keymap." + f'Pick a different key, or unbind a colliding action by setting ' + f'its key to null (e.g. "undo": null).' ) diff --git a/sqlit/domains/shell/app/startup_flow.py b/sqlit/domains/shell/app/startup_flow.py index eb80c385..332ee665 100644 --- a/sqlit/domains/shell/app/startup_flow.py +++ b/sqlit/domains/shell/app/startup_flow.py @@ -170,7 +170,8 @@ def _warn_on_keymap_error(app: AppProtocol, is_headless: bool) -> None: error = getattr(manager, "load_error", None) if manager else None if not error: return - message = f"{error}. Defaults are in effect — fix ~/.sqlit/keymaps/ and restart." + from sqlit.domains.shell.app.keymap_manager import CUSTOM_KEYMAP_DIR + message = f"{error}\nDefaults are in effect — fix {CUSTOM_KEYMAP_DIR} and restart." if is_headless: print(f"[sqlit] {message}", file=sys.stderr) return diff --git a/tests/ui/keybindings/test_keymap_manager.py b/tests/ui/keybindings/test_keymap_manager.py index 8cc30f5d..ea5926cf 100644 --- a/tests/ui/keybindings/test_keymap_manager.py +++ b/tests/ui/keybindings/test_keymap_manager.py @@ -177,23 +177,78 @@ def test_unknown_leader_action(self, tmp_path: Path): assert manager.load_error is not None assert "Unknown leader action" in manager.load_error - def test_empty_key_string(self, tmp_path: Path): + def test_empty_string_in_list_is_rejected(self, tmp_path: Path): manager = _load( tmp_path, - "empty", - {"keymap": {"action_keys": {"tree": {"refresh_tree": ""}}}}, + "empty-in-list", + {"keymap": {"action_keys": {"tree": {"refresh_tree": ["f", ""]}}}}, ) assert manager.load_error is not None - assert "key must be a non-empty string" in manager.load_error + assert "every entry must be a non-empty string" in manager.load_error - def test_empty_key_list(self, tmp_path: Path): + +class TestUnbind: + """null / "" / [] removes a default binding without adding a replacement.""" + + def test_null_unbinds_default(self, tmp_path: Path): + # Default: 'u' → undo in query_normal. User sets it to null. + _load( + tmp_path, + "unbind-null", + {"keymap": {"action_keys": {"query_normal": {"undo": None}}}}, + ) + keymap = get_keymap() + # No key in query_normal triggers undo anymore. + assert not any( + ak.action == "undo" and ak.context == "query_normal" + for ak in keymap.get_action_keys() + ) + + def test_unbind_resolves_user_vs_default_conflict(self, tmp_path: Path): + # User wants 'u' for enter_insert_mode. Without unbinding undo this + # collides with the default 'u' → undo. With it, the user's binding + # works cleanly. manager = _load( tmp_path, - "empty-list", - {"keymap": {"action_keys": {"tree": {"refresh_tree": []}}}}, + "unbind-resolves", + { + "keymap": { + "action_keys": { + "query_normal": { + "enter_insert_mode": "u", + "undo": None, + } + } + } + }, + ) + assert manager.load_error is None + keymap = get_keymap() + assert keymap.action("enter_insert_mode") == "u" + assert not any(ak.action == "undo" and ak.context == "query_normal" + for ak in keymap.get_action_keys()) + + def test_empty_string_also_unbinds(self, tmp_path: Path): + _load( + tmp_path, + "unbind-empty", + {"keymap": {"action_keys": {"query_normal": {"undo": ""}}}}, + ) + assert not any( + ak.action == "undo" and ak.context == "query_normal" + for ak in get_keymap().get_action_keys() + ) + + def test_leader_null_unbinds(self, tmp_path: Path): + _load( + tmp_path, + "unbind-leader", + {"keymap": {"leader_commands": {"leader": {"toggle_explorer": None}}}}, + ) + assert not any( + c.action == "toggle_explorer" and c.menu == "leader" + for c in get_keymap().get_leader_commands() ) - assert manager.load_error is not None - assert "key list must contain at least one key" in manager.load_error class TestConflicts: From 24687dfa8f5a89a9c71e1ac94a927ccb5b1703a1 Mon Sep 17 00:00:00 2001 From: Peter Adams <18162810+Maxteabag@users.noreply.github.com> Date: Mon, 25 May 2026 00:57:21 +0200 Subject: [PATCH 07/12] docs(readme): mention custom keybindings in Configuration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Short pointer in the Configuration section with a minimal example and a link to docs/custom-keymaps.md. Doesn't add it to the main Keybindings table — kept advanced-usage voluntary per CONTRIBUTING.md. --- README.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/README.md b/README.md index 49c0bee9..54a379e1 100644 --- a/README.md +++ b/README.md @@ -241,6 +241,25 @@ Connections and settings are stored in `$XDG_CONFIG_HOME/sqlit/` (default: `~/.c If an older install left files in `~/.sqlit/`, they are moved to the new location automatically on first run. +### Custom keybindings + +Drop a JSON file in `~/.config/sqlit/keymaps/` and point `custom_keymap` in `settings.json` at it. The file lists only the bindings you want to change — anything you don't touch keeps its default. Footer hints and the help screen pick up your custom keys automatically. + +```json +{ + "keymap": { + "action_keys": { + "query_normal": { + "enter_insert_mode": "u", + "undo": null + } + } + } +} +``` + +See [docs/custom-keymaps.md](docs/custom-keymaps.md) for the full action/state catalog and the unbind / alias-list semantics. + ## FAQ ### How are sensitive credentials stored? From d9cd68c9f4cd053848c26702b995c3fd4ab1ca50 Mon Sep 17 00:00:00 2001 From: Peter Adams <18162810+Maxteabag@users.noreply.github.com> Date: Mon, 25 May 2026 09:29:02 +0200 Subject: [PATCH 08/12] docs(keymaps): fix legacy ~/.sqlit/ paths, mention null-unbind in troubleshooting --- docs/custom-keymaps.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/custom-keymaps.md b/docs/custom-keymaps.md index b24481b9..ac2b2cee 100644 --- a/docs/custom-keymaps.md +++ b/docs/custom-keymaps.md @@ -6,13 +6,13 @@ sqlit lets you remap the keys that trigger any action. The set of actions and th 1. Create the keymaps directory and copy the template: ```bash - mkdir -p ~/.sqlit/keymaps - cp config/keymap.template.json ~/.sqlit/keymaps/my-custom.json + mkdir -p ~/.config/sqlit/keymaps + cp config/keymap.template.json ~/.config/sqlit/keymaps/my-custom.json ``` -2. Edit `~/.sqlit/keymaps/my-custom.json`. Only list the bindings you want to change. +2. Edit `~/.config/sqlit/keymaps/my-custom.json`. Only list the bindings you want to change. -3. Enable it by adding to `~/.sqlit/settings.json`: +3. Enable it by adding to `~/.config/sqlit/settings.json`: ```json { "custom_keymap": "my-custom" } ``` @@ -150,10 +150,10 @@ Rebind autocomplete to `ctrl+n`/`ctrl+p`, swap `h` to `?`, keep ## Troubleshooting -**Keymap didn't load.** Check `~/.sqlit/settings.json` — `custom_keymap` must match your filename without the `.json`. The app shows a notification on startup; running headless prints `[sqlit] Failed to load custom keymap …` to stderr. +**Keymap didn't load.** Check `~/.config/sqlit/settings.json` — `custom_keymap` must match your filename without the `.json`. The app shows a notification on startup; running headless prints `[sqlit] Failed to load custom keymap …` to stderr. **"Unknown action" error.** The action doesn't exist in the state you named. The error message lists the known actions for that state — pick one of those, or check the canonical list in `sqlit/core/keymap.py`. -**"Conflicting keybindings" error.** Your overrides bind the same key to two different actions in the same state. Pick a different key, or remove the redundant entry. +**"Conflicting keybindings" error.** Your overrides bind the same key to two different actions in the same state. Pick a different key, or unbind the colliding default by setting its key to `null` (e.g. `"undo": null`). -**Invalid JSON.** Run `python -m json.tool ~/.sqlit/keymaps/my-custom.json` to find the syntax error. +**Invalid JSON.** Run `python -m json.tool ~/.config/sqlit/keymaps/my-custom.json` to find the syntax error. From 708aebb09c00a27c9ffc3afe9a1ee7603d243522 Mon Sep 17 00:00:00 2001 From: Peter Adams <18162810+Maxteabag@users.noreply.github.com> Date: Mon, 25 May 2026 09:34:01 +0200 Subject: [PATCH 09/12] feat(keybindings): autoload ~/.config/sqlit/keymap.json, drop the docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The customization flow is now: drop a single JSON file at ~/.config/sqlit/keymap.json, restart sqlit. No settings.json edit, no mkdir. - KeymapManager picks up the default file automatically when no custom_keymap setting is set. The setting still wins for the power-user case where you want several named keymaps in keymaps/. - README's Custom keybindings subsection is now self-contained (path, JSON shape, list/null semantics, multi-keymap escape hatch). - docs/custom-keymaps.md deleted — the workflow + error messages carry the user without separate documentation. Errors already list valid actions/states on typo and suggest the null unbind on conflict. - keymap.template.json points at the new path. --- README.md | 7 +- config/keymap.template.json | 3 +- docs/custom-keymaps.md | 159 -------------------- sqlit/domains/shell/app/keymap_manager.py | 34 +++-- tests/ui/keybindings/test_keymap_manager.py | 44 ++++++ 5 files changed, 74 insertions(+), 173 deletions(-) delete mode 100644 docs/custom-keymaps.md diff --git a/README.md b/README.md index 54a379e1..daaaabe5 100644 --- a/README.md +++ b/README.md @@ -243,7 +243,7 @@ If an older install left files in `~/.sqlit/`, they are moved to the new locatio ### Custom keybindings -Drop a JSON file in `~/.config/sqlit/keymaps/` and point `custom_keymap` in `settings.json` at it. The file lists only the bindings you want to change — anything you don't touch keeps its default. Footer hints and the help screen pick up your custom keys automatically. +Drop a JSON file at `~/.config/sqlit/keymap.json` listing only the bindings you want to change. No settings edit, no `mkdir`. Footer hints and the help screen pick up your custom keys automatically. ```json { @@ -258,7 +258,10 @@ Drop a JSON file in `~/.config/sqlit/keymaps/` and point `custom_keymap` in `set } ``` -See [docs/custom-keymaps.md](docs/custom-keymaps.md) for the full action/state catalog and the unbind / alias-list semantics. +- Outer keys under `action_keys` are state names (`query_normal`, `tree`, `results`, …); under `leader_commands` they are menu names (`leader`, `delete`, `yank`, …). Mistype one and the loader names the valid options for you. +- The value is a key string, a list `["primary", "alias", …]`, or `null` to unbind that default. +- If you want to keep several named keymaps and swap between them, put them in `~/.config/sqlit/keymaps/` and pick one in `settings.json` as `"custom_keymap": "name"`. +- `config/keymap.template.json` in the repo is a ready-to-edit starting point. ## FAQ diff --git a/config/keymap.template.json b/config/keymap.template.json index c19ac07f..d2becb2b 100644 --- a/config/keymap.template.json +++ b/config/keymap.template.json @@ -1,6 +1,5 @@ { - "_note": "Copy to ~/.sqlit/keymaps/.json and set 'custom_keymap' to '' in settings.json.", - "_help": "Only list the bindings you want to change. Each entry maps action → key (or [primary, alias...]) within its state. See docs/custom-keymaps.md for the action/state catalog.", + "_note": "Copy to ~/.config/sqlit/keymap.json (no subdir) — sqlit loads it automatically. List only the bindings you want to change. See docs/custom-keymaps.md for the action/state catalog.", "keymap": { "action_keys": { "query_normal": { diff --git a/docs/custom-keymaps.md b/docs/custom-keymaps.md deleted file mode 100644 index ac2b2cee..00000000 --- a/docs/custom-keymaps.md +++ /dev/null @@ -1,159 +0,0 @@ -# Custom Keymaps - -sqlit lets you remap the keys that trigger any action. The set of actions and the states they live in is fixed by the app; the keymap JSON is *purely* a key-remapping layer on top of the defaults. - -## Quick start - -1. Create the keymaps directory and copy the template: - ```bash - mkdir -p ~/.config/sqlit/keymaps - cp config/keymap.template.json ~/.config/sqlit/keymaps/my-custom.json - ``` - -2. Edit `~/.config/sqlit/keymaps/my-custom.json`. Only list the bindings you want to change. - -3. Enable it by adding to `~/.config/sqlit/settings.json`: - ```json - { "custom_keymap": "my-custom" } - ``` - Use the filename without `.json`. To revert, set `"custom_keymap": "default"` or remove the key. - -4. Restart sqlit. The footer and help screen (`?`) reflect your custom keys automatically. - -## File shape - -The JSON mirrors the state machine: each binding lives in the state (for action keys) or menu (for leader commands) where it is active. - -```json -{ - "keymap": { - "action_keys": { - "": { - "": "" - } - }, - "leader_commands": { - "": { - "": "" - } - } - } -} -``` - -Values can be a single key string, or a list to provide aliases (the first entry is the primary key shown in footer/help): - -```json -"tree": { - "refresh_tree": ["f", "R"] -} -``` - -A single string *replaces every default key* for that action in that state. A list lets you control the full set of keys explicitly. **Set the value to `null` to unbind a default entirely** — useful when you want to claim a key that already has a default action: - -```json -"query_normal": { - "enter_insert_mode": "u", - "undo": null -} -``` - -Here `u` enters insert mode and the default `u → undo` binding is removed (no key triggers undo in `query_normal` anymore). Without the `undo: null`, the loader would reject the keymap because two actions would be claiming `u`. - -## What you can change — and what you cannot - -You can change **which key** triggers a given action. That is the entire vocabulary. - -You cannot — and don't need to — set the action's label, category, guard, priority, or visibility. Those are properties of the action itself, defined in `sqlit/core/keymap.py`. The keymap is strictly a key-rebinding layer. - -You also cannot invent new actions or attach an existing action to a state it doesn't belong to. The loader rejects unknown `(state, action)` pairs with a clear error that lists the known actions for that state. - -## Validation at startup - -The keymap is checked before the app boots. Failures fall back to defaults; the error surfaces as a Textual notification in the running app, and as a `[sqlit] …` line on stderr when running headless. Possible failures: - -- **Unknown action in a state** — sqlit lists the actions that *do* exist in that state. -- **Conflicting bindings** — two different actions claiming the same key in the same state (or the same menu, for leader commands). Default-only overlaps that are disambiguated by runtime state — e.g. `d` on a connection vs. on a folder, both in `tree` — are tolerated; only collisions you introduce are flagged. - -## States (for `action_keys`) - -| State | Where it applies | -|-------|------------------| -| `global` | Active everywhere | -| `navigation` | Switching focus between the three panes | -| `tree` | Database explorer tree | -| `tree_filter` | Filter input over the explorer tree | -| `tree_visual` | Tree visual selection mode | -| `query_normal` | Query editor, NORMAL mode | -| `query_insert` | Query editor, INSERT mode | -| `autocomplete` | Autocomplete dropdown is visible | -| `results` | Results table | -| `results_filter` | Filter input over the results table | -| `value_view` | Value-view modal | - -## Menus (for `leader_commands`) - -`leader` (the top-level command menu), plus the vim-style sub-menus: `delete`, `yank`, `change`, `g`, `gc` (line comments), `ry` (results yank), `rye` (results export), `vy` (value-view yank). - -## Action catalog - -The canonical list lives in `sqlit/core/keymap.py` — `DefaultKeymapProvider._build_action_keys()` and `_build_leader_commands()`. Use the in-app help (`?`) or read the source for the full set. The most commonly-rebound ones: - -**Global** — `leader_key`, `show_help`, `quit`, `cancel_operation`, `enter_command_mode` - -**Navigation** — `focus_explorer`, `focus_query`, `focus_results` - -**Tree** — `tree_cursor_up`, `tree_cursor_down`, `select_table`, `tree_filter`, `collapse_tree`, `refresh_tree`, `new_connection`, `edit_connection`, `delete_connection`, `duplicate_connection`, `disconnect` - -**Query (normal mode)** — `enter_insert_mode`, `prepend_insert_mode`, `append_insert_mode`, `append_line_end`, `open_line_below`, `open_line_above`, `execute_query`, `cursor_left`, `cursor_down`, `cursor_up`, `cursor_right`, `cursor_word_forward`, `cursor_WORD_forward`, `cursor_word_back`, `cursor_WORD_back`, `cursor_line_start`, `cursor_line_end`, `cursor_last_line`, `cursor_find_char`, `cursor_find_char_back`, `cursor_till_char`, `cursor_till_char_back`, `cursor_matching_bracket`, `paste`, `show_history`, `new_query`, `undo`, `redo` - -**Query (insert mode)** — `exit_insert_mode`, `execute_query_insert`, `autocomplete_accept`, `select_all`, `copy_selection`, `paste` - -**Autocomplete** — `autocomplete_next`, `autocomplete_prev`, `autocomplete_accept`, `autocomplete_close` - -**Results** — `view_cell`, `view_cell_full`, `edit_cell`, `delete_row`, `clear_results`, `results_filter`, `results_cursor_left/down/up/right`, `next_result_section`, `prev_result_section`, `toggle_result_section` - -**Value view** — `close_value_view`, `copy_value_view`, `toggle_value_view_mode`, `collapse_all_json_nodes`, `expand_all_json_nodes` - -## Special key names - -`space`, `escape`, `enter`, `backspace`, `delete`, `tab`, `question_mark`, `slash`, `dollar_sign`, `percent_sign`, `asterisk`, `ctrl+`, `shift+`. - -## Example - -Rebind autocomplete to `ctrl+n`/`ctrl+p`, swap `h` to `?`, keep the arrow-key aliases for tree navigation, and use `ctrl+enter` to execute queries in normal mode: - -```json -{ - "keymap": { - "action_keys": { - "query_normal": { - "execute_query": "ctrl+enter" - }, - "autocomplete": { - "autocomplete_next": "ctrl+n", - "autocomplete_prev": "ctrl+p" - }, - "tree": { - "tree_cursor_down": ["j", "down"], - "tree_cursor_up": ["k", "up"] - } - }, - "leader_commands": { - "leader": { - "show_help": "question_mark" - } - } - } -} -``` - -## Troubleshooting - -**Keymap didn't load.** Check `~/.config/sqlit/settings.json` — `custom_keymap` must match your filename without the `.json`. The app shows a notification on startup; running headless prints `[sqlit] Failed to load custom keymap …` to stderr. - -**"Unknown action" error.** The action doesn't exist in the state you named. The error message lists the known actions for that state — pick one of those, or check the canonical list in `sqlit/core/keymap.py`. - -**"Conflicting keybindings" error.** Your overrides bind the same key to two different actions in the same state. Pick a different key, or unbind the colliding default by setting its key to `null` (e.g. `"undo": null`). - -**Invalid JSON.** Run `python -m json.tool ~/.config/sqlit/keymaps/my-custom.json` to find the syntax error. diff --git a/sqlit/domains/shell/app/keymap_manager.py b/sqlit/domains/shell/app/keymap_manager.py index 1f341d48..b8a45183 100644 --- a/sqlit/domains/shell/app/keymap_manager.py +++ b/sqlit/domains/shell/app/keymap_manager.py @@ -2,8 +2,12 @@ Follows the same domain-service pattern as [`ThemeManager`][sqlit.domains.shell.app.theme_manager] — a settings-driven -configuration step run during app startup. Keymaps live in JSON files under -``~/.sqlit/keymaps/`` and are selected by the ``custom_keymap`` setting. +configuration step run during app startup. The loader looks for, in order: + +1. A named keymap selected by the ``custom_keymap`` setting, resolved + relative to ``~/.config/sqlit/keymaps/.json`` (or an absolute path). +2. ``~/.config/sqlit/keymap.json`` if it exists — picked up automatically + so a user can customize without editing settings.json. The JSON is strictly a *key remapping*. The set of actions and the states they live in are defined in :mod:`sqlit.core.keymap`; the user only @@ -52,6 +56,10 @@ CUSTOM_KEYMAP_SETTINGS_KEY = "custom_keymap" CUSTOM_KEYMAP_DIR = CONFIG_DIR / "keymaps" +# Picked up automatically when no `custom_keymap` setting is present. +# Lets a user customize their bindings by dropping a single file in +# their config dir — no settings edit, no mkdir. +DEFAULT_KEYMAP_FILE = CONFIG_DIR / "keymap.json" class FileBasedKeymapProvider(KeymapProvider): @@ -96,16 +104,22 @@ def initialize(self) -> dict: def load_custom_keymap(self, settings: dict) -> None: self.load_error = None keymap_name = settings.get(CUSTOM_KEYMAP_SETTINGS_KEY) - if not keymap_name or not isinstance(keymap_name, str): - return - if keymap_name.strip() in ("", "default"): + if isinstance(keymap_name, str) and keymap_name.strip() not in ("", "default"): + # Explicit named keymap from settings — power-user path. + try: + path = self._resolve_keymap_path(keymap_name.strip()) + self._register_custom_keymap(path, keymap_name.strip()) + except Exception as exc: + self.load_error = f"Failed to load custom keymap '{keymap_name}': {exc}" return - try: - path = self._resolve_keymap_path(keymap_name.strip()) - self._register_custom_keymap(path, keymap_name.strip()) - except Exception as exc: - self.load_error = f"Failed to load custom keymap '{keymap_name}': {exc}" + # No setting → look for the default file. Silently absent is fine; a + # broken file still raises. + if DEFAULT_KEYMAP_FILE.exists(): + try: + self._register_custom_keymap(DEFAULT_KEYMAP_FILE, DEFAULT_KEYMAP_FILE.name) + except Exception as exc: + self.load_error = f"Failed to load {DEFAULT_KEYMAP_FILE}: {exc}" def _resolve_keymap_path(self, keymap_name: str) -> Path: if keymap_name.startswith(("~", "/")) or Path(keymap_name).is_absolute(): diff --git a/tests/ui/keybindings/test_keymap_manager.py b/tests/ui/keybindings/test_keymap_manager.py index ea5926cf..92acec7a 100644 --- a/tests/ui/keybindings/test_keymap_manager.py +++ b/tests/ui/keybindings/test_keymap_manager.py @@ -187,6 +187,50 @@ def test_empty_string_in_list_is_rejected(self, tmp_path: Path): assert "every entry must be a non-empty string" in manager.load_error +class TestDefaultKeymapFile: + """Drop ~/.config/sqlit/keymap.json with no settings edit → it loads.""" + + def test_default_file_picked_up_automatically(self, tmp_path: Path, monkeypatch): + # Point CONFIG_DIR-derived constants at a fresh dir. + from sqlit.domains.shell.app import keymap_manager as km_mod + default_path = tmp_path / "keymap.json" + default_path.write_text(json.dumps( + {"keymap": {"action_keys": {"query_normal": {"execute_query": "ctrl+enter"}}}} + )) + monkeypatch.setattr(km_mod, "DEFAULT_KEYMAP_FILE", default_path) + + manager = KeymapManager(settings_store=MockSettingsStore({})) + manager.initialize() + assert manager.load_error is None + assert get_keymap().action("execute_query") == "ctrl+enter" + + def test_settings_override_takes_precedence(self, tmp_path: Path, monkeypatch): + # If both default file and `custom_keymap` setting exist, the setting wins. + from sqlit.domains.shell.app import keymap_manager as km_mod + default_path = tmp_path / "keymap.json" + default_path.write_text(json.dumps( + {"keymap": {"action_keys": {"query_normal": {"execute_query": "ctrl+enter"}}}} + )) + monkeypatch.setattr(km_mod, "DEFAULT_KEYMAP_FILE", default_path) + + named = tmp_path / "named.json" + named.write_text(json.dumps( + {"keymap": {"action_keys": {"query_normal": {"execute_query": "ctrl+shift+enter"}}}} + )) + manager = KeymapManager(settings_store=MockSettingsStore({"custom_keymap": str(named)})) + manager.initialize() + assert get_keymap().action("execute_query") == "ctrl+shift+enter" + + def test_no_default_file_and_no_setting_is_silent(self, tmp_path: Path, monkeypatch): + from sqlit.domains.shell.app import keymap_manager as km_mod + # Point at a path that doesn't exist. + monkeypatch.setattr(km_mod, "DEFAULT_KEYMAP_FILE", tmp_path / "nope.json") + manager = KeymapManager(settings_store=MockSettingsStore({})) + manager.initialize() + assert manager.load_error is None + assert not isinstance(get_keymap(), FileBasedKeymapProvider) + + class TestUnbind: """null / "" / [] removes a default binding without adding a replacement.""" From f994e7fd5de55d711deaa73ccbb68969232bc5a6 Mon Sep 17 00:00:00 2001 From: Peter Adams <18162810+Maxteabag@users.noreply.github.com> Date: Mon, 25 May 2026 10:34:05 +0200 Subject: [PATCH 10/12] feat(keybindings): friendly key names, complete template, autoload scaffold MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A user editing keymap.json no longer needs to know Textual's internal key names — `:`, `?`, `/`, `$`, `%`, `*`, `^`, `;`, `@`, `#`, `!`, `&`, `~`, brackets, etc. work directly. The loader expands them to canonical Textual variants on read (e.g. `:` covers `colon`, `shift+semicolon`, and `:` to handle every terminal's emission), so a binding written with the friendly form works everywhere. config/keymap.template.json is now the full default keymap in friendly form (~9KB across every state and menu). A user copies lines they want to change rather than memorizing action names. KeymapManager auto-creates an empty keymap.json scaffold in the config dir on first run so the customization file is discoverable. README points at both files. Conflict detection now ignores user-preserved default overlaps (e.g. the intentional `d` → delete_connection vs delete_connection_folder overlap in `tree`, both kept by state-machine disambiguation) — only new user-introduced collisions are flagged. docs/keymap-actions.md and docs/custom-keymaps.md removed — the template + README + error messages carry the user without a separate documentation file to maintain. --- README.md | 20 +- config/keymap.template.json | 345 +++++++++++++++++++- sqlit/domains/shell/app/keymap_manager.py | 227 ++++++++++--- tests/ui/keybindings/test_keymap_manager.py | 91 +++++- 4 files changed, 603 insertions(+), 80 deletions(-) diff --git a/README.md b/README.md index daaaabe5..7e3f0c35 100644 --- a/README.md +++ b/README.md @@ -243,25 +243,7 @@ If an older install left files in `~/.sqlit/`, they are moved to the new locatio ### Custom keybindings -Drop a JSON file at `~/.config/sqlit/keymap.json` listing only the bindings you want to change. No settings edit, no `mkdir`. Footer hints and the help screen pick up your custom keys automatically. - -```json -{ - "keymap": { - "action_keys": { - "query_normal": { - "enter_insert_mode": "u", - "undo": null - } - } - } -} -``` - -- Outer keys under `action_keys` are state names (`query_normal`, `tree`, `results`, …); under `leader_commands` they are menu names (`leader`, `delete`, `yank`, …). Mistype one and the loader names the valid options for you. -- The value is a key string, a list `["primary", "alias", …]`, or `null` to unbind that default. -- If you want to keep several named keymaps and swap between them, put them in `~/.config/sqlit/keymaps/` and pick one in `settings.json` as `"custom_keymap": "name"`. -- `config/keymap.template.json` in the repo is a ready-to-edit starting point. +Override defaults by editing `keymap.json` in your sqlit config dir — sqlit auto-creates it on first run. Only list the bindings you want to change; everything you don't touch keeps its default. Use a list for aliases, or `null` to unbind a key entirely. [`config/keymap.template.json`](config/keymap.template.json) is the full default keymap — copy lines you want to override, or use it as a reference for action and state names. ## FAQ diff --git a/config/keymap.template.json b/config/keymap.template.json index d2becb2b..4fee0442 100644 --- a/config/keymap.template.json +++ b/config/keymap.template.json @@ -1,22 +1,351 @@ { - "_note": "Copy to ~/.config/sqlit/keymap.json (no subdir) — sqlit loads it automatically. List only the bindings you want to change. See docs/custom-keymaps.md for the action/state catalog.", "keymap": { "action_keys": { + "global": { + "leader_key": "space", + "quit": "ctrl+q", + "cancel_operation": "escape", + "show_help": "?", + "enter_command_mode": ":", + "undo": "ctrl+z", + "redo": "ctrl+y" + }, + "navigation": { + "focus_explorer": "e", + "focus_query": "q", + "focus_results": "r" + }, + "tree": { + "new_connection": "n", + "enter_tree_visual_mode": "v", + "clear_connection_selection": "escape", + "select_table": "s", + "refresh_tree": [ + "f", + "R" + ], + "edit_connection": "e", + "rename_connection_folder": "M", + "delete_connection_folder": [ + "d", + "delete" + ], + "delete_connection": [ + "d", + "delete" + ], + "duplicate_connection": "D", + "move_connection_to_folder": "m", + "disconnect": "x", + "collapse_tree": "z", + "tree_cursor_down": [ + "j", + "down" + ], + "tree_cursor_up": [ + "k", + "up" + ], + "tree_filter": "/" + }, + "tree_filter": { + "tree_filter_close": "escape", + "tree_filter_accept": "enter" + }, + "tree_visual": { + "exit_tree_visual_mode": [ + "escape", + "v" + ] + }, "query_normal": { - "execute_query": "ctrl+enter" + "enter_insert_mode": "i", + "prepend_insert_mode": "I", + "open_line_below": "o", + "open_line_above": "O", + "execute_query": "enter", + "paste": "p", + "yank_leader_key": "y", + "change_leader_key": "c", + "g_leader_key": "g", + "show_history": "backspace", + "new_query": "N", + "delete_leader_key": "d", + "change_line_end_motion": "C", + "delete_line_end": "D", + "undo": "u", + "redo": "ctrl+r", + "cursor_left": "h", + "cursor_down": "j", + "cursor_up": "k", + "cursor_right": "l", + "cursor_word_forward": "w", + "cursor_WORD_forward": "W", + "cursor_word_back": "b", + "cursor_WORD_back": "B", + "cursor_line_start": "0", + "cursor_first_non_blank": "^", + "cursor_line_end": "$", + "cursor_last_line": "G", + "cursor_matching_bracket": "%", + "cursor_find_char": "f", + "cursor_find_char_back": "F", + "cursor_till_char": "t", + "cursor_till_char_back": "T", + "enter_visual_mode": "v", + "enter_visual_line_mode": "V", + "delete_char": "x", + "append_insert_mode": "a", + "append_line_end": "A" + }, + "query_insert": { + "exit_insert_mode": "escape", + "execute_query_insert": "ctrl+enter", + "select_all": "ctrl+a", + "copy_selection": "ctrl+c", + "paste": "ctrl+v" + }, + "query_visual": { + "exit_visual_mode": [ + "escape", + "v" + ], + "switch_to_visual_line_mode": "V", + "visual_yank": "y", + "visual_delete": [ + "d", + "x" + ], + "visual_change": "c", + "visual_execute": "enter", + "cursor_left": [ + "h", + "left" + ], + "cursor_down": [ + "j", + "down" + ], + "cursor_up": [ + "k", + "up" + ], + "cursor_right": [ + "l", + "right" + ], + "cursor_word_forward": "w", + "cursor_WORD_forward": "W", + "cursor_word_back": "b", + "cursor_WORD_back": "B", + "cursor_line_start": "0", + "cursor_first_non_blank": "^", + "cursor_line_end": "$", + "cursor_last_line": "G", + "g_leader_key": "g", + "cursor_matching_bracket": "%", + "cursor_find_char": "f", + "cursor_find_char_back": "F", + "cursor_till_char": "t", + "cursor_till_char_back": "T" + }, + "query_visual_line": { + "exit_visual_line_mode": [ + "escape", + "V" + ], + "switch_to_visual_mode": "v", + "visual_line_yank": "y", + "visual_line_delete": [ + "d", + "x" + ], + "visual_line_change": "c", + "cursor_down": [ + "j", + "down" + ], + "cursor_up": [ + "k", + "up" + ], + "cursor_last_line": "G", + "g_leader_key": "g", + "visual_line_execute": "enter" }, "autocomplete": { - "autocomplete_next": "ctrl+n", - "autocomplete_prev": "ctrl+p" + "autocomplete_next": [ + "ctrl+j", + "down" + ], + "autocomplete_prev": [ + "ctrl+k", + "up" + ], + "autocomplete_accept": "tab", + "autocomplete_close": "escape" }, - "tree": { - "tree_cursor_down": ["j", "down"], - "tree_cursor_up": ["k", "up"] + "results": { + "view_cell": "v", + "view_cell_full": "V", + "edit_cell": "u", + "delete_row": "d", + "results_yank_leader_key": "y", + "clear_results": "x", + "results_filter": "/", + "results_cursor_left": "h", + "results_cursor_down": "j", + "results_cursor_up": "k", + "results_cursor_right": "l", + "rg_leader_key": "g", + "results_cursor_last_row": "G", + "results_page_up": "ctrl+u", + "results_page_down": "ctrl+d", + "results_cursor_first_column": "0", + "results_cursor_last_column": "$", + "results_column_picker": [ + "f", + "F" + ], + "next_result_section": "tab", + "prev_result_section": "shift+tab", + "toggle_result_section": "z" + }, + "results_filter": { + "results_filter_close": "escape", + "results_filter_accept": "enter" + }, + "value_view": { + "close_value_view": [ + "q", + "escape" + ], + "copy_value_view": "y", + "toggle_value_view_mode": "t", + "collapse_all_json_nodes": "z", + "expand_all_json_nodes": "Z" } }, "leader_commands": { "leader": { - "show_help": "question_mark" + "toggle_explorer": "e", + "toggle_fullscreen": "f", + "show_connection_picker": "c", + "disconnect": "x", + "cancel_operation": "z", + "change_theme": "t", + "edit_query_in_editor": "o", + "show_help": "h", + "telescope": "space", + "telescope_filter": "/", + "quit": "q" + }, + "delete": { + "line": "d", + "word": "w", + "WORD": "W", + "word_back": "b", + "WORD_back": "B", + "word_end": "e", + "WORD_end": "E", + "line_start": "0", + "line_end_motion": "$", + "line_end": "D", + "char": "x", + "char_back": "X", + "left": "h", + "down": "j", + "up": "k", + "right": "l", + "to_end": "G", + "find_char": "f", + "find_char_back": "F", + "till_char": "t", + "till_char_back": "T", + "matching_bracket": "%", + "inner": "i", + "around": "a" + }, + "yank": { + "line": "y", + "word": "w", + "WORD": "W", + "word_back": "b", + "WORD_back": "B", + "word_end": "e", + "WORD_end": "E", + "line_start": "0", + "line_end_motion": "$", + "left": "h", + "down": "j", + "up": "k", + "right": "l", + "to_end": "G", + "find_char": "f", + "find_char_back": "F", + "till_char": "t", + "till_char_back": "T", + "matching_bracket": "%", + "inner": "i", + "around": "a" + }, + "change": { + "line": "c", + "word": "w", + "WORD": "W", + "word_back": "b", + "WORD_back": "B", + "word_end": "e", + "WORD_end": "E", + "line_start": "0", + "line_end_motion": "$", + "left": "h", + "down": "j", + "up": "k", + "right": "l", + "to_end": "G", + "find_char": "f", + "find_char_back": "F", + "till_char": "t", + "till_char_back": "T", + "matching_bracket": "%", + "inner": "i", + "around": "a" + }, + "g": { + "first_line": "g", + "word_end_back": "e", + "WORD_end_back": "E", + "comment": "c", + "execute_query": "r", + "execute_single_statement": "s", + "execute_query_atomic": "t" + }, + "gc": { + "line": "c", + "down": "j", + "up": "k", + "to_end": "G", + "selection": "s", + "statement": "S" + }, + "ry": { + "cell": "c", + "row": "y", + "all": "a", + "export": "e" + }, + "rye": { + "csv": "c", + "json": "j" + }, + "rg": { + "first_row": "g" + }, + "vy": { + "value": "y", + "field": "f", + "all": "a" } } } diff --git a/sqlit/domains/shell/app/keymap_manager.py b/sqlit/domains/shell/app/keymap_manager.py index b8a45183..7da686e0 100644 --- a/sqlit/domains/shell/app/keymap_manager.py +++ b/sqlit/domains/shell/app/keymap_manager.py @@ -5,10 +5,14 @@ configuration step run during app startup. The loader looks for, in order: 1. A named keymap selected by the ``custom_keymap`` setting, resolved - relative to ``~/.config/sqlit/keymaps/.json`` (or an absolute path). -2. ``~/.config/sqlit/keymap.json`` if it exists — picked up automatically + relative to ``/keymaps/.json`` (or an absolute path). +2. ``/keymap.json`` if it exists — picked up automatically so a user can customize without editing settings.json. +``CONFIG_DIR`` resolves to ``$SQLIT_CONFIG_DIR`` if set, otherwise +``$XDG_CONFIG_HOME/sqlit`` (defaulting to ``~/.config/sqlit``). See +:mod:`sqlit.shared.core.store`. + The JSON is strictly a *key remapping*. The set of actions and the states they live in are defined in :mod:`sqlit.core.keymap`; the user only chooses which key(s) trigger each action:: @@ -61,6 +65,82 @@ # their config dir — no settings edit, no mkdir. DEFAULT_KEYMAP_FILE = CONFIG_DIR / "keymap.json" +# Friendly literal characters → the canonical Textual key name(s) they +# expand to. Lets the user write `"?"` instead of `"question_mark"` and +# `":"` instead of spelling out the colon's three terminal variants. +# Multi-element values cover the platforms / terminals that emit +# different `event.key` strings for the same physical keypress. +_FRIENDLY_TO_CANONICAL: dict[str, list[str]] = { + "?": ["question_mark"], + "/": ["slash"], + "$": ["dollar_sign"], + "%": ["percent_sign"], + "*": ["asterisk"], + "^": ["circumflex_accent"], + ":": ["colon", "shift+semicolon", ":"], + ";": ["semicolon"], + "@": ["at"], + "#": ["number_sign"], + "!": ["exclamation_mark"], + "&": ["ampersand"], + "~": ["tilde"], + "`": ["grave_accent"], + "(": ["left_parenthesis"], + ")": ["right_parenthesis"], + "[": ["left_square_bracket"], + "]": ["right_square_bracket"], + "{": ["left_curly_bracket"], + "}": ["right_curly_bracket"], + "<": ["less_than_sign"], + ">": ["greater_than_sign"], + "|": ["vertical_line"], + "_": ["underscore"], +} + +# Reverse map (canonical → friendly) for emitting templates and any +# user-facing serialization of the default keymap. Multi-variant +# entries collapse to their first canonical, since the friendly form +# is the same for all of them. +_CANONICAL_TO_FRIENDLY: dict[str, str] = { + canonical: friendly + for friendly, canonicals in _FRIENDLY_TO_CANONICAL.items() + for canonical in canonicals +} + + +def _expand_user_key(key: str) -> list[str]: + """Expand a single user-supplied key string to its canonical Textual form(s). + + Splits off modifier prefixes (``ctrl+``, ``shift+``, ``alt+``, ``cmd+``), + looks up the base character in the friendly-name table, and re-attaches + the modifiers to each canonical variant. Unknown bases pass through + unchanged so Textual's own key names (e.g. ``escape``, ``f5``) keep + working. + """ + parts = key.split("+") + base = parts[-1] + modifiers = parts[:-1] + canonicals = _FRIENDLY_TO_CANONICAL.get(base, [base]) + if not modifiers: + return list(canonicals) + prefix = "+".join(modifiers) + "+" + return [prefix + c for c in canonicals] + + +def canonical_to_friendly(key: str) -> str: + """Convert a canonical Textual key name to its friendly form for display. + + Used when generating the template from defaults. Keeps modifier + prefixes intact and only rewrites the base name. + """ + parts = key.split("+") + base = parts[-1] + modifiers = parts[:-1] + friendly = _CANONICAL_TO_FRIENDLY.get(base, base) + if not modifiers: + return friendly + return "+".join(modifiers) + "+" + friendly + class FileBasedKeymapProvider(KeymapProvider): """Keymap provider built by merging user overrides onto the defaults.""" @@ -113,14 +193,35 @@ def load_custom_keymap(self, settings: dict) -> None: self.load_error = f"Failed to load custom keymap '{keymap_name}': {exc}" return - # No setting → look for the default file. Silently absent is fine; a - # broken file still raises. + # No setting → load the default file, creating an empty scaffold if + # it doesn't exist so the file is discoverable in the user's config + # dir even before they've customized anything. + self._ensure_default_keymap_scaffold() if DEFAULT_KEYMAP_FILE.exists(): try: self._register_custom_keymap(DEFAULT_KEYMAP_FILE, DEFAULT_KEYMAP_FILE.name) except Exception as exc: self.load_error = f"Failed to load {DEFAULT_KEYMAP_FILE}: {exc}" + @staticmethod + def _ensure_default_keymap_scaffold() -> None: + """Create an empty keymap.json on first run so users can discover it.""" + if DEFAULT_KEYMAP_FILE.exists(): + return + try: + DEFAULT_KEYMAP_FILE.parent.mkdir(parents=True, exist_ok=True) + DEFAULT_KEYMAP_FILE.write_text( + json.dumps( + {"keymap": {"action_keys": {}, "leader_commands": {}}}, + indent=2, + ) + "\n", + encoding="utf-8", + ) + except OSError: + # Read-only config dir or similar — silently skip; the user + # can still create the file themselves. + pass + def _resolve_keymap_path(self, keymap_name: str) -> Path: if keymap_name.startswith(("~", "/")) or Path(keymap_name).is_absolute(): return Path(keymap_name).expanduser() @@ -172,6 +273,8 @@ def _load_keymap_from_file(self, path: Path, keymap_name: str) -> FileBasedKeyma merged_action, user_leader_overrides, user_action_overrides, + base_leader, + base_action, ) return FileBasedKeymapProvider(keymap_name, merged_leader, merged_action) @@ -248,18 +351,25 @@ def _parse_action_overrides( if key_list is None: unbinds.add((action, state)) continue - for i, key in enumerate(key_list): - out.append( - ActionKeyDef( - key=key, - action=action, - context=state, - guard=template.guard, - primary=(i == 0), - show=template.show, - priority=template.priority, + # Expand friendly chars (e.g. "?") to their canonical Textual + # variants. The first entry in the original user list is the + # primary; its expansion contributes all primary candidates; + # subsequent user entries are aliases. + first = True + for user_key in key_list: + for canonical in _expand_user_key(user_key): + out.append( + ActionKeyDef( + key=canonical, + action=action, + context=state, + guard=template.guard, + primary=first, + show=template.show, + priority=template.priority, + ) ) - ) + first = False return out, unbinds @staticmethod @@ -309,16 +419,18 @@ def _parse_leader_overrides( f'leader_commands."{menu}"."{action}": expected a key string or null.' ) - out.append( - LeaderCommandDef( - key=key, - action=action, - label=template.label, - category=template.category, - guard=template.guard, - menu=menu, + # Expand friendly chars (`":"` → variant list, `"?"` → "question_mark", …). + for canonical in _expand_user_key(key): + out.append( + LeaderCommandDef( + key=canonical, + action=action, + label=template.label, + category=template.category, + guard=template.guard, + menu=menu, + ) ) - ) return out, unbinds # ------------------------------------------------------------------- merge @@ -354,40 +466,65 @@ def _detect_conflicts( merged_action: list[ActionKeyDef], user_leader: list[LeaderCommandDef], user_action: list[ActionKeyDef], + base_leader: list[LeaderCommandDef], + base_action: list[ActionKeyDef], ) -> None: """Raise ValueError on user-introduced bindings that collide. Defaults intentionally bind some keys to multiple actions in the same state (e.g. ``d`` in ``tree`` for both delete_connection and delete_connection_folder, disambiguated by tree-node state at - runtime). We don't flag those. We *do* flag any conflict that a - user override either created or contributed to. + runtime). We never flag those — and if the user's config preserves + that exact overlap (e.g. they copied the full template verbatim), + we don't flag that either. We *do* flag any conflict the user + actually introduced — a new action joining an existing slot. """ conflicts: list[str] = [] + def _by_slot_leader(commands): + out: dict[tuple[str, str], set[str]] = defaultdict(set) + for cmd in commands: + out[(cmd.key, cmd.menu)].add(cmd.action) + return out + + def _by_slot_action(action_keys): + out: dict[tuple[str, str | None], set[str]] = defaultdict(set) + for ak in action_keys: + out[(ak.key, ak.context)].add(ak.action) + return out + + base_leader_slots = _by_slot_leader(base_leader) + merged_leader_slots = _by_slot_leader(merged_leader) user_leader_slots = {(u.key, u.menu) for u in user_leader} - leader_by_slot: dict[tuple[str, str], set[str]] = defaultdict(set) - for cmd in merged_leader: - leader_by_slot[(cmd.key, cmd.menu)].add(cmd.action) - for (key, menu), actions in sorted(leader_by_slot.items()): - if len(actions) > 1 and (key, menu) in user_leader_slots: - conflicts.append( - f"leader key {key!r} in menu {menu!r} is bound to multiple actions: " - f"{sorted(actions)}" - ) + for slot, actions in sorted(merged_leader_slots.items()): + if len(actions) <= 1 or slot not in user_leader_slots: + continue + # If the actions for this slot match the defaults exactly, the + # user just preserved a pre-existing (state-machine-disambiguated) + # overlap rather than creating a new one. + if actions == base_leader_slots.get(slot): + continue + key, menu = slot + conflicts.append( + f"leader key {key!r} in menu {menu!r} is bound to multiple actions: " + f"{sorted(actions)}" + ) + base_action_slots = _by_slot_action(base_action) + merged_action_slots = _by_slot_action(merged_action) user_action_slots = {(u.key, u.context) for u in user_action} - action_by_slot: dict[tuple[str, str | None], set[str]] = defaultdict(set) - for ak in merged_action: - action_by_slot[(ak.key, ak.context)].add(ak.action) - for (key, ctx), actions in sorted( - action_by_slot.items(), key=lambda t: (t[0][0], t[0][1] or "") + for slot, actions in sorted( + merged_action_slots.items(), key=lambda t: (t[0][0], t[0][1] or "") ): - if len(actions) > 1 and (key, ctx) in user_action_slots: - conflicts.append( - f"key {key!r} in state {ctx!r} is bound to multiple actions: " - f"{sorted(actions)}" - ) + if len(actions) <= 1 or slot not in user_action_slots: + continue + if actions == base_action_slots.get(slot): + continue + key, ctx = slot + conflicts.append( + f"key {key!r} in state {ctx!r} is bound to multiple actions: " + f"{sorted(actions)}" + ) if conflicts: lines = "\n - ".join(conflicts) diff --git a/tests/ui/keybindings/test_keymap_manager.py b/tests/ui/keybindings/test_keymap_manager.py index 92acec7a..1eee35ec 100644 --- a/tests/ui/keybindings/test_keymap_manager.py +++ b/tests/ui/keybindings/test_keymap_manager.py @@ -43,16 +43,40 @@ def reset_keymap_after_test(): reset_keymap() +@pytest.fixture(autouse=True) +def isolate_default_keymap_file(tmp_path_factory, monkeypatch): + """Redirect DEFAULT_KEYMAP_FILE to a per-test tmp dir. + + The loader auto-creates a scaffold keymap.json when one isn't present; + we don't want that writing into the developer's real ~/.config/sqlit. + """ + from sqlit.domains.shell.app import keymap_manager as km_mod + tmp_dir = tmp_path_factory.mktemp("keymap-isolation") + monkeypatch.setattr(km_mod, "DEFAULT_KEYMAP_FILE", tmp_dir / "keymap.json") + + class TestLifecycle: - def test_no_custom_keymap_leaves_defaults(self): + def test_no_custom_keymap_leaves_defaults_intact(self): + # The loader auto-creates an empty scaffold, but defaults are unchanged. manager = KeymapManager(settings_store=MockSettingsStore({})) manager.initialize() - assert not isinstance(get_keymap(), FileBasedKeymapProvider) + assert manager.load_error is None + assert get_keymap().action("enter_insert_mode") == "i" - def test_default_sentinel_skips_loading(self): + def test_default_sentinel_uses_scaffold_only(self): manager = KeymapManager(settings_store=MockSettingsStore({"custom_keymap": "default"})) manager.initialize() - assert not isinstance(get_keymap(), FileBasedKeymapProvider) + assert manager.load_error is None + assert get_keymap().action("enter_insert_mode") == "i" + + def test_scaffold_created_on_first_run(self, tmp_path_factory): + # The autouse fixture redirected DEFAULT_KEYMAP_FILE to a fresh tmp dir. + from sqlit.domains.shell.app import keymap_manager as km_mod + assert not km_mod.DEFAULT_KEYMAP_FILE.exists() + KeymapManager(settings_store=MockSettingsStore({})).initialize() + assert km_mod.DEFAULT_KEYMAP_FILE.exists() + body = json.loads(km_mod.DEFAULT_KEYMAP_FILE.read_text()) + assert body == {"keymap": {"action_keys": {}, "leader_commands": {}}} def test_invalid_json_falls_back(self, tmp_path: Path): path = tmp_path / "invalid.json" @@ -221,14 +245,65 @@ def test_settings_override_takes_precedence(self, tmp_path: Path, monkeypatch): manager.initialize() assert get_keymap().action("execute_query") == "ctrl+shift+enter" - def test_no_default_file_and_no_setting_is_silent(self, tmp_path: Path, monkeypatch): + def test_no_setting_creates_scaffold_and_loads_it(self, tmp_path: Path, monkeypatch): from sqlit.domains.shell.app import keymap_manager as km_mod - # Point at a path that doesn't exist. - monkeypatch.setattr(km_mod, "DEFAULT_KEYMAP_FILE", tmp_path / "nope.json") + target = tmp_path / "fresh-config" / "keymap.json" + monkeypatch.setattr(km_mod, "DEFAULT_KEYMAP_FILE", target) + assert not target.exists() manager = KeymapManager(settings_store=MockSettingsStore({})) manager.initialize() assert manager.load_error is None - assert not isinstance(get_keymap(), FileBasedKeymapProvider) + assert target.exists(), "scaffold must be written on first run" + # Defaults are still in effect since the scaffold is empty. + assert get_keymap().action("enter_insert_mode") == "i" + + +class TestFriendlyKeyNames: + """User can write `:` instead of `colon`, `?` instead of `question_mark`, etc.""" + + def test_question_mark_normalizes(self, tmp_path: Path): + _load( + tmp_path, + "qmark", + {"keymap": {"leader_commands": {"leader": {"show_help": "?"}}}}, + ) + # Internally stored as canonical Textual name. + assert get_keymap().leader("show_help", "leader") == "question_mark" + + def test_colon_expands_to_all_terminal_variants(self, tmp_path: Path): + # `:` is emitted as `colon`, `shift+semicolon`, or `:` depending on + # the terminal/platform — `_expand_user_key` covers all three so the + # binding works everywhere. + _load( + tmp_path, + "colon", + {"keymap": {"action_keys": {"global": {"enter_command_mode": ":"}}}}, + ) + keys = get_keymap().keys_for_action("enter_command_mode") + assert "colon" in keys + assert "shift+semicolon" in keys + assert ":" in keys + + def test_friendly_char_with_modifier(self, tmp_path: Path): + _load( + tmp_path, + "ctrl-slash", + {"keymap": {"action_keys": {"tree": {"tree_filter": "ctrl+/"}}}}, + ) + # ctrl+/ canonicalizes to ctrl+slash. + keys = get_keymap().keys_for_action("tree_filter") + assert "ctrl+slash" in keys + assert "/" not in keys + assert "slash" not in keys + + def test_canonical_names_still_work(self, tmp_path: Path): + # User can still write the canonical Textual name if they want to. + _load( + tmp_path, + "canonical", + {"keymap": {"leader_commands": {"leader": {"show_help": "question_mark"}}}}, + ) + assert get_keymap().leader("show_help", "leader") == "question_mark" class TestUnbind: From 2e7e636e7ac7a703bbbf9c29253bc4b8f5965635 Mon Sep 17 00:00:00 2001 From: Peter Adams <18162810+Maxteabag@users.noreply.github.com> Date: Mon, 25 May 2026 10:52:16 +0200 Subject: [PATCH 11/12] fix(keybindings): dedupe modifiers in friendly-char expansion + drop dead code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_expand_user_key` was naively concatenating the user's modifier prefix onto each canonical variant. For inputs like `shift+:` — where `:` already expands to `shift+semicolon` — this produced `shift+shift+semicolon`, which Textual silently ignores. Now the merge unions user modifiers with whatever the canonical already carries, so each modifier appears once. Edge case: input ending in `+` (e.g. `ctrl++`, meaning ctrl + the plus key) is preserved verbatim — caught by a regression test before pushing. Also removes `canonical_to_friendly` and `_CANONICAL_TO_FRIENDLY` — the template-from-defaults generator they were written for was never wired up, so they were dead code. --- sqlit/domains/shell/app/keymap_manager.py | 54 ++++++++++----------- tests/ui/keybindings/test_keymap_manager.py | 35 +++++++++++++ 2 files changed, 62 insertions(+), 27 deletions(-) diff --git a/sqlit/domains/shell/app/keymap_manager.py b/sqlit/domains/shell/app/keymap_manager.py index 7da686e0..75058ff1 100644 --- a/sqlit/domains/shell/app/keymap_manager.py +++ b/sqlit/domains/shell/app/keymap_manager.py @@ -97,17 +97,6 @@ "_": ["underscore"], } -# Reverse map (canonical → friendly) for emitting templates and any -# user-facing serialization of the default keymap. Multi-variant -# entries collapse to their first canonical, since the friendly form -# is the same for all of them. -_CANONICAL_TO_FRIENDLY: dict[str, str] = { - canonical: friendly - for friendly, canonicals in _FRIENDLY_TO_CANONICAL.items() - for canonical in canonicals -} - - def _expand_user_key(key: str) -> list[str]: """Expand a single user-supplied key string to its canonical Textual form(s). @@ -116,30 +105,41 @@ def _expand_user_key(key: str) -> list[str]: the modifiers to each canonical variant. Unknown bases pass through unchanged so Textual's own key names (e.g. ``escape``, ``f5``) keep working. + + When a canonical variant already carries one of the user's modifiers + (e.g. ``":"`` expands to include ``"shift+semicolon"`` and the user + wrote ``"shift+:"``), we deduplicate so we don't produce a malformed + ``shift+shift+semicolon`` that Textual would silently ignore. """ parts = key.split("+") base = parts[-1] modifiers = parts[:-1] + + # Trailing "+" in input (e.g. "ctrl++") means the literal plus key. + # Splitting leaves an empty base — preserve the user's input verbatim + # rather than trying to canonicalize. + if base == "": + return [key] + canonicals = _FRIENDLY_TO_CANONICAL.get(base, [base]) if not modifiers: return list(canonicals) - prefix = "+".join(modifiers) + "+" - return [prefix + c for c in canonicals] - -def canonical_to_friendly(key: str) -> str: - """Convert a canonical Textual key name to its friendly form for display. - - Used when generating the template from defaults. Keeps modifier - prefixes intact and only rewrites the base name. - """ - parts = key.split("+") - base = parts[-1] - modifiers = parts[:-1] - friendly = _CANONICAL_TO_FRIENDLY.get(base, base) - if not modifiers: - return friendly - return "+".join(modifiers) + "+" + friendly + user_mods = set(modifiers) + expanded: list[str] = [] + seen: set[str] = set() + for canonical in canonicals: + canon_parts = canonical.split("+") + canon_mods = set(canon_parts[:-1]) + canon_base = canon_parts[-1] + # Union the user's modifiers with whatever the canonical already + # has — keeps each modifier exactly once. + merged_mods = sorted(user_mods | canon_mods) + combined = "+".join(merged_mods + [canon_base]) if merged_mods else canon_base + if combined not in seen: + seen.add(combined) + expanded.append(combined) + return expanded class FileBasedKeymapProvider(KeymapProvider): diff --git a/tests/ui/keybindings/test_keymap_manager.py b/tests/ui/keybindings/test_keymap_manager.py index 1eee35ec..8f68e391 100644 --- a/tests/ui/keybindings/test_keymap_manager.py +++ b/tests/ui/keybindings/test_keymap_manager.py @@ -305,6 +305,41 @@ def test_canonical_names_still_work(self, tmp_path: Path): ) assert get_keymap().leader("show_help", "leader") == "question_mark" + def test_literal_plus_via_double_plus_preserved(self): + """`ctrl++` means ctrl + plus key; must round-trip unchanged.""" + from sqlit.domains.shell.app.keymap_manager import _expand_user_key + assert _expand_user_key("ctrl++") == ["ctrl++"] + assert _expand_user_key("++") == ["++"] + + def test_shift_modifier_does_not_double_when_canonical_already_has_shift( + self, tmp_path: Path + ): + """`shift+:` must not expand to `shift+shift+semicolon`. + + The friendly char `:` expands to three canonical variants, one of + which (`shift+semicolon`) already carries a shift prefix. Naively + concatenating the user's `shift+` prefix produces `shift+shift+...`, + which Textual won't match — the binding silently dies. + """ + _load( + tmp_path, + "shift-friendly", + { + "keymap": { + "action_keys": { + "query_normal": {"new_query": "shift+:"} + } + } + }, + ) + keys = get_keymap().keys_for_action("new_query") + # No canonical key should ever start with "shift+shift+". + bad = [k for k in keys if "shift+shift+" in k] + assert not bad, ( + f"shift+ expansion produced malformed canonical(s): {bad}; " + f"all keys: {keys}" + ) + class TestUnbind: """null / "" / [] removes a default binding without adding a replacement.""" From b0a9d4653ac78fafeeed19c2930d87be4c107588 Mon Sep 17 00:00:00 2001 From: Peter Adams <18162810+Maxteabag@users.noreply.github.com> Date: Mon, 25 May 2026 11:02:39 +0200 Subject: [PATCH 12/12] docs(readme): trim Custom keybindings to its essentials --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 7e3f0c35..8ca45e6b 100644 --- a/README.md +++ b/README.md @@ -243,7 +243,7 @@ If an older install left files in `~/.sqlit/`, they are moved to the new locatio ### Custom keybindings -Override defaults by editing `keymap.json` in your sqlit config dir — sqlit auto-creates it on first run. Only list the bindings you want to change; everything you don't touch keeps its default. Use a list for aliases, or `null` to unbind a key entirely. [`config/keymap.template.json`](config/keymap.template.json) is the full default keymap — copy lines you want to override, or use it as a reference for action and state names. +Edit the `keymap.json` file in your sqlit config dir. See [`config/keymap.template.json`](config/keymap.template.json) for the full default keymap. Keymap.json need to only contain the overriding keymaps. ## FAQ