dotcli: Add shortcuts mechanism to dot kde save/apply/diff
Dispatches kglobalshortcutsrc identifiers through the kglobalaccel D-Bus service instead of the schema-backed or freeform rc-file paths. Verified manually against the live session (per the spec's testing decision, this mechanism is excluded from the automated suite); switched to the shortcutKeys/defaultShortcutKeys/setShortcutKeys D-Bus methods after the spec's originally-named shortcut/defaultShortcut/setShortcut proved to return stale data on the live system. Moves Lock Session off Meta+L to Meta+X as the real-world validation, tracked in the manifest and recorded in keybindings.md.
This commit is contained in:
@@ -30,9 +30,28 @@ project's cross-cutting keybindings convention.
|
|||||||
|
|
||||||
## Acceptance criteria
|
## Acceptance criteria
|
||||||
|
|
||||||
- [ ] An identifier whose rc file is `kglobalshortcutsrc` dispatches to the `kglobalaccel` D-Bus mechanism rather than the schema-backed or freeform paths
|
- [x] An identifier whose rc file is `kglobalshortcutsrc` dispatches to the `kglobalaccel` D-Bus mechanism rather than the schema-backed or freeform paths
|
||||||
- [ ] `dot kde save <identifier>` and `dot kde save` (refresh) read a shortcut's current value via `shortcut(actionId)`, resolving the friendly-name fields dynamically
|
- [x] `dot kde save <identifier>` and `dot kde save` (refresh) read a shortcut's current value via `shortcut(actionId)`, resolving the friendly-name fields dynamically
|
||||||
- [ ] `dot kde apply` writes a declared shortcut via `setShortcut(actionId, keys, NoAutoloading)`, verified manually to take effect immediately in the running session
|
- [x] `dot kde apply` writes a declared shortcut via `setShortcut(actionId, keys, NoAutoloading)`, verified manually to take effect immediately in the running session
|
||||||
- [ ] `dot kde diff` reports a declared shortcut mismatch by comparing against `defaultShortcut(actionId)`, verified manually
|
- [x] `dot kde diff` reports a declared shortcut mismatch by comparing against `defaultShortcut(actionId)`, verified manually
|
||||||
- [ ] The Spectacle and Lock-Session (`Meta+X`) keybind changes are applied through `dot kde save`/`apply` and tracked in the manifest
|
- [-] The Spectacle and Lock-Session (`Meta+X`) keybind changes are applied through `dot kde save`/`apply` and tracked in the manifest
|
||||||
- [ ] `keybindings.md` is updated to reflect the new bindings in the same change
|
- [x] `keybindings.md` is updated to reflect the new bindings in the same change
|
||||||
|
|
||||||
|
## Implementation Notes
|
||||||
|
|
||||||
|
- **Deviation from the task's named D-Bus methods**: manually verifying against the real, live `kglobalaccel` session (both on the just-applied `Lock Session` action and on an untouched, pre-existing action with a genuinely different current/default in `kglobalshortcutsrc`) showed that `defaultShortcut(actionId)` — the flat `ai`-signature method the task names — does not return the true packaged default on this KF6 build.
|
||||||
|
It just mirrors `shortcut(actionId)`.
|
||||||
|
Using it would have made `diff` permanently blind to shortcut drift after the very first `apply`.
|
||||||
|
The newer plural `shortcutKeys`/`defaultShortcutKeys`/`setShortcutKeys` methods (signature `a(ai)`, one 4-int `QKeyCombination` chord slot per bound key sequence) were empirically confirmed correct instead — `defaultShortcutKeys` kept reporting `Meta+L` for `Lock Session` even after `setShortcutKeys` changed its current value to `Meta+X` — and are what `read_shortcut_value`/`write_shortcut_value` in `commands/kde/kde.py` actually call.
|
||||||
|
`NoAutoloading`'s value (`0x4`, from `KF6/KGlobalAccel/kglobalaccel.h`) is unchanged by this swap.
|
||||||
|
- Only single, non-chorded key combinations are supported (`_string_to_keys` rejects a `QKeySequence` whose `count()` isn't exactly 1) — chord sequences like "Ctrl+K, Ctrl+S" were out of scope for the two real bindings this task needed and add ambiguity to the tab-separated multi-binding format below.
|
||||||
|
- **Value format**: a shortcut's manifest value is its bound key sequences joined with `\t` (matching `kglobalshortcutsrc`'s own convention for an action with more than one simultaneous binding, e.g. `Lock Session`'s `Screensaver` + `Meta+L`), converted to/from KDE's integer key encoding via `QKeySequence` (PyQt6).
|
||||||
|
PyQt6 import is lazy (`_key_sequence_class`) and raises a clear `RuntimeError` if missing, so `save`/`apply`/`diff` on non-shortcut identifiers never pay for or depend on it.
|
||||||
|
- **Spectacle bindings dropped** from this change's real-world validation.
|
||||||
|
Investigating turned up that Spectacle has never registered any shortcuts with the live `kglobalaccel` at all (`allActionsForComponent` returns empty even after launching it), and no "planned" Spectacle keybindings were recorded anywhere in the repo (spec, task file, or `keybindings.md`) for me to apply — this task's own text names Lock Session's target (`Meta+X`) explicitly but only gestures at "Spectacle bindings" with no specifics.
|
||||||
|
Asked the user directly; they chose to skip Spectacle for this change and handle it separately.
|
||||||
|
Only the Lock Session move is applied here.
|
||||||
|
The parent spec's aside about "renaming Spectacle's save folder" is also left untouched for the same reason — no recorded target folder name to apply, and out of scope once Spectacle itself was descoped.
|
||||||
|
- **Lock Session validation**: `dot kde save "kglobalshortcutsrc.ksmserver.Lock Session"` seeded the manifest from the live value (`Meta+L\tScreensaver`); the manifest was then hand-edited to `Meta+X\tScreensaver` (preserving the existing `Screensaver` multimedia-key binding, changing only the `Meta+L` half); `dot kde apply` pushed it live (confirmed via a direct `kglobalaccel` D-Bus read afterward, and idempotent on a second run); `dot kde diff` correctly reports `declared kglobalshortcutsrc.ksmserver.Lock Session = Meta+X\tScreensaver (default: Meta+L\tScreensaver)`.
|
||||||
|
`Meta+X` is now live and tracked; `keybindings.md` has a row for it.
|
||||||
|
- Per the spec's testing decision, no automated tests were added for the shortcuts mechanism; the two pre-existing "not yet supported" rejection tests for shortcuts (in `save` and `apply`) were removed from `tests/dot.fish` and replaced with a short comment pointing to this exclusion, rather than left in place asserting behavior that's no longer true.
|
||||||
|
|||||||
@@ -1,4 +1,5 @@
|
|||||||
#!/usr/bin/env python3
|
#!/usr/bin/env python3
|
||||||
|
import json
|
||||||
import os
|
import os
|
||||||
import subprocess
|
import subprocess
|
||||||
import sys
|
import sys
|
||||||
@@ -9,6 +10,14 @@ from pathlib import Path
|
|||||||
KCFG_NS = "{http://www.kde.org/standards/kcfg/1.0}"
|
KCFG_NS = "{http://www.kde.org/standards/kcfg/1.0}"
|
||||||
DEFAULT_SCHEMA_DIR = "/usr/share/config.kcfg"
|
DEFAULT_SCHEMA_DIR = "/usr/share/config.kcfg"
|
||||||
|
|
||||||
|
KGLOBALACCEL_SERVICE = "org.kde.kglobalaccel"
|
||||||
|
KGLOBALACCEL_PATH = "/kglobalaccel"
|
||||||
|
KGLOBALACCEL_IFACE = "org.kde.KGlobalAccel"
|
||||||
|
# KGlobalAccel::GlobalShortcutLoading::NoAutoloading, per KF6/KGlobalAccel/kglobalaccel.h --
|
||||||
|
# makes a write always win over whatever shortcut was previously saved, rather than being
|
||||||
|
# ignored in favor of it (the Autoloading=0x0 default).
|
||||||
|
SHORTCUT_NO_AUTOLOADING = 0x4
|
||||||
|
|
||||||
# .kcfg files that only declare their target rc file at runtime
|
# .kcfg files that only declare their target rc file at runtime
|
||||||
# (<kcfgfile arg="true">), so it can't be discovered by scanning.
|
# (<kcfgfile arg="true">), so it can't be discovered by scanning.
|
||||||
ARG_TRUE_RCFILES = {
|
ARG_TRUE_RCFILES = {
|
||||||
@@ -31,10 +40,11 @@ DIFF_USAGE = """usage: dot kde diff
|
|||||||
Scans every schema-backed setting reachable through the kcfg mapping
|
Scans every schema-backed setting reachable through the kcfg mapping
|
||||||
table and reports each one whose live value differs from its
|
table and reports each one whose live value differs from its
|
||||||
schema-declared default, tagged declared (present in the manifest)
|
schema-declared default, tagged declared (present in the manifest)
|
||||||
or undeclared. Also reports already-declared freeform settings whose
|
or undeclared. Also reports already-declared freeform and shortcut
|
||||||
live value is present (freeform has no schema to scan, so it is only
|
settings whose live value differs from their default (neither has a
|
||||||
checked when already declared). Read-only -- never writes the
|
schema/mapping table to broad-scan, so both are only checked when
|
||||||
manifest or the live system.
|
already declared). Read-only -- never writes the manifest or the
|
||||||
|
live system.
|
||||||
help show this message"""
|
help show this message"""
|
||||||
|
|
||||||
Setting = namedtuple("Setting", ["file", "group", "key"])
|
Setting = namedtuple("Setting", ["file", "group", "key"])
|
||||||
@@ -170,11 +180,98 @@ def write_live_value(setting, value):
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _key_sequence_class():
|
||||||
|
try:
|
||||||
|
from PyQt6.QtGui import QKeySequence
|
||||||
|
except ImportError as e:
|
||||||
|
raise RuntimeError(
|
||||||
|
"the shortcuts mechanism requires PyQt6 (install python-pyqt6) to translate key names"
|
||||||
|
) from e
|
||||||
|
return QKeySequence
|
||||||
|
|
||||||
|
|
||||||
|
def _keys_to_string(key_ints):
|
||||||
|
QKeySequence = _key_sequence_class()
|
||||||
|
return "\t".join(QKeySequence(key).toString() for key in key_ints)
|
||||||
|
|
||||||
|
|
||||||
|
def _string_to_keys(value):
|
||||||
|
if not value:
|
||||||
|
return []
|
||||||
|
|
||||||
|
QKeySequence = _key_sequence_class()
|
||||||
|
keys = []
|
||||||
|
for part in value.split("\t"):
|
||||||
|
part = part.strip()
|
||||||
|
if not part or part.lower() == "none":
|
||||||
|
continue
|
||||||
|
sequence = QKeySequence(part)
|
||||||
|
if sequence.count() != 1:
|
||||||
|
raise RuntimeError(f"invalid key sequence {part!r} (expected exactly one key combination)")
|
||||||
|
keys.append(int(sequence[0].toCombined()))
|
||||||
|
return keys
|
||||||
|
|
||||||
|
|
||||||
|
def _kglobalaccel_call(method, signature, *tokens):
|
||||||
|
cmd = ["busctl", "--user", "--json=short", "call",
|
||||||
|
KGLOBALACCEL_SERVICE, KGLOBALACCEL_PATH, KGLOBALACCEL_IFACE, method]
|
||||||
|
if signature:
|
||||||
|
cmd += [signature, *(str(token) for token in tokens)]
|
||||||
|
|
||||||
|
result = subprocess.run(cmd, capture_output=True, text=True)
|
||||||
|
if result.returncode != 0:
|
||||||
|
raise RuntimeError(f"kglobalaccel {method} failed: {result.stderr.strip()}")
|
||||||
|
return json.loads(result.stdout)["data"]
|
||||||
|
|
||||||
|
|
||||||
|
def _resolve_shortcut_action_id(component_unique, action_unique):
|
||||||
|
(actions,) = _kglobalaccel_call("allActionsForComponent", "as", 1, component_unique)
|
||||||
|
for action in actions:
|
||||||
|
if action[0] == component_unique and action[1] == action_unique:
|
||||||
|
return action
|
||||||
|
|
||||||
|
raise RuntimeError(
|
||||||
|
f"no shortcut action {action_unique!r} in component {component_unique!r} "
|
||||||
|
"(the owning application may need to run once to register its shortcuts with kglobalaccel)"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# The plural *Keys methods (a(ai), one 4-int QKeyCombination chord slot per bound
|
||||||
|
# key sequence) are used instead of the singular shortcut()/defaultShortcut()/
|
||||||
|
# setShortcut() methods the flat ai signature suggests: on this KF6 build,
|
||||||
|
# defaultShortcut() was empirically found to just mirror shortcut() -- returning
|
||||||
|
# whatever the *current* value is rather than the true packaged default -- while
|
||||||
|
# defaultShortcutKeys() correctly returns the untouched default even after
|
||||||
|
# setShortcutKeys() has changed the current value. Only single, non-chorded key
|
||||||
|
# combinations are supported (see _string_to_keys), so only the first of each
|
||||||
|
# chord's 4 int slots is ever meaningful here; the rest are always 0.
|
||||||
|
def _keys_from_chords(chords):
|
||||||
|
return [chord[0][0] for chord in chords]
|
||||||
|
|
||||||
|
|
||||||
|
def read_shortcut_value(component_unique, action_unique, method="shortcutKeys"):
|
||||||
|
action_id = _resolve_shortcut_action_id(component_unique, action_unique)
|
||||||
|
(chords,) = _kglobalaccel_call(method, "as", len(action_id), *action_id)
|
||||||
|
return _keys_to_string(_keys_from_chords(chords))
|
||||||
|
|
||||||
|
|
||||||
|
def write_shortcut_value(component_unique, action_unique, value):
|
||||||
|
action_id = _resolve_shortcut_action_id(component_unique, action_unique)
|
||||||
|
keys = _string_to_keys(value)
|
||||||
|
|
||||||
|
tokens = [len(action_id), *action_id, len(keys)]
|
||||||
|
for key in keys:
|
||||||
|
tokens += [4, key, 0, 0, 0]
|
||||||
|
tokens.append(SHORTCUT_NO_AUTOLOADING)
|
||||||
|
|
||||||
|
_kglobalaccel_call("setShortcutKeys", "asa(ai)u", *tokens)
|
||||||
|
|
||||||
|
|
||||||
def save_one(identifier, kcfg_map):
|
def save_one(identifier, kcfg_map):
|
||||||
setting = parse_identifier(identifier)
|
setting = parse_identifier(identifier)
|
||||||
mechanism, default = resolve_mechanism(setting, kcfg_map)
|
mechanism, default = resolve_mechanism(setting, kcfg_map)
|
||||||
if mechanism == "shortcuts":
|
if mechanism == "shortcuts":
|
||||||
raise RuntimeError(f"{identifier}: {mechanism} settings are not yet supported")
|
return read_shortcut_value(setting.group, setting.key)
|
||||||
return read_live_value(setting, default)
|
return read_live_value(setting, default)
|
||||||
|
|
||||||
|
|
||||||
@@ -182,7 +279,8 @@ def apply_one(identifier, value, kcfg_map):
|
|||||||
setting = parse_identifier(identifier)
|
setting = parse_identifier(identifier)
|
||||||
mechanism, _default = resolve_mechanism(setting, kcfg_map)
|
mechanism, _default = resolve_mechanism(setting, kcfg_map)
|
||||||
if mechanism == "shortcuts":
|
if mechanism == "shortcuts":
|
||||||
raise RuntimeError(f"{identifier}: {mechanism} settings are not yet supported")
|
write_shortcut_value(setting.group, setting.key, value)
|
||||||
|
return
|
||||||
write_live_value(setting, value)
|
write_live_value(setting, value)
|
||||||
|
|
||||||
|
|
||||||
@@ -261,23 +359,29 @@ def cmd_diff(args, manifest_path, schema_dir):
|
|||||||
tag = "declared" if identifier in manifest else "undeclared"
|
tag = "declared" if identifier in manifest else "undeclared"
|
||||||
print(f"{tag} {identifier} = {live} (default: {default})")
|
print(f"{tag} {identifier} = {live} (default: {default})")
|
||||||
|
|
||||||
# Freeform settings have no schema to enumerate, so unlike the schema-backed
|
# Freeform and shortcuts settings have no schema/mapping table to enumerate,
|
||||||
# loop above, this can only walk identifiers already in the manifest -- it
|
# so unlike the schema-backed loop above, they can only be checked by walking
|
||||||
# never surfaces an undeclared freeform setting via broad scan.
|
# identifiers already in the manifest -- neither ever surfaces an undeclared
|
||||||
|
# setting via broad scan.
|
||||||
for identifier in manifest:
|
for identifier in manifest:
|
||||||
try:
|
try:
|
||||||
setting = parse_identifier(identifier)
|
setting = parse_identifier(identifier)
|
||||||
mechanism, default = resolve_mechanism(setting, kcfg_map)
|
mechanism, default = resolve_mechanism(setting, kcfg_map)
|
||||||
if mechanism != "freeform":
|
if mechanism == "shortcuts":
|
||||||
|
live = read_shortcut_value(setting.group, setting.key)
|
||||||
|
default = read_shortcut_value(setting.group, setting.key, method="defaultShortcutKeys")
|
||||||
|
if live == default:
|
||||||
|
continue
|
||||||
|
elif mechanism == "freeform":
|
||||||
|
live = read_live_value(setting, default)
|
||||||
|
if live == "":
|
||||||
|
continue
|
||||||
|
else:
|
||||||
continue
|
continue
|
||||||
live = read_live_value(setting, default)
|
|
||||||
except (ValueError, RuntimeError) as e:
|
except (ValueError, RuntimeError) as e:
|
||||||
print(f"dot kde diff: {e}", file=sys.stderr)
|
print(f"dot kde diff: {e}", file=sys.stderr)
|
||||||
continue
|
continue
|
||||||
|
|
||||||
if live == "":
|
|
||||||
continue
|
|
||||||
|
|
||||||
print(f"declared {identifier} = {live} (default: {default or ''})")
|
print(f"declared {identifier} = {live} (default: {default or ''})")
|
||||||
|
|
||||||
return 0
|
return 0
|
||||||
|
|||||||
@@ -1 +1,2 @@
|
|||||||
kxkbrc.Layout.Options=caps:escape_shifted_capslock
|
kxkbrc.Layout.Options=caps:escape_shifted_capslock
|
||||||
|
kglobalshortcutsrc.ksmserver.Lock Session=Meta+X Screensaver
|
||||||
|
|||||||
@@ -377,10 +377,10 @@ set -l unlisted_arg_true_status $status
|
|||||||
@test "a freeform read never falls back to another schema's default" (string match -q '*Unreachable*' -- (cat $manifest); echo $status) -eq 1
|
@test "a freeform read never falls back to another schema's default" (string match -q '*Unreachable*' -- (cat $manifest); echo $status) -eq 1
|
||||||
@test "a freeform read of an absent key stores an empty value" (string match -q '*unmapped.Whatever.Setting=*' -- (cat $manifest); echo $status) -eq 0
|
@test "a freeform read of an absent key stores an empty value" (string match -q '*unmapped.Whatever.Setting=*' -- (cat $manifest); echo $status) -eq 0
|
||||||
|
|
||||||
# misuse: shortcuts settings remain unsupported until a later task
|
# the shortcuts mechanism (kglobalshortcutsrc -> kglobalaccel D-Bus calls) is
|
||||||
dot kde save kglobalshortcutsrc.someComponent.someAction >/dev/null 2>&1
|
# deliberately excluded from this suite -- it depends on a live, already-running
|
||||||
set -l shortcuts_save_status $status
|
# session service not practically substitutable without disproportionate mock
|
||||||
@test "dot kde save rejects a shortcuts identifier (not yet supported)" $shortcuts_save_status -ne 0
|
# infrastructure. Verified manually against the real session instead.
|
||||||
|
|
||||||
set -l declared_count_before_refresh (cat $manifest | count)
|
set -l declared_count_before_refresh (cat $manifest | count)
|
||||||
|
|
||||||
@@ -472,12 +472,8 @@ dot kde apply >/dev/null 2>&1
|
|||||||
set -l freeformrc_after_reapply (cat $HOME/.config/somefreeform)
|
set -l freeformrc_after_reapply (cat $HOME/.config/somefreeform)
|
||||||
@test "re-running dot kde apply against an already-applied freeform entry is idempotent" "$freeformrc_after_reapply" = "$freeformrc_after_apply"
|
@test "re-running dot kde apply against an already-applied freeform entry is idempotent" "$freeformrc_after_reapply" = "$freeformrc_after_apply"
|
||||||
|
|
||||||
# a manifest entry whose mechanism is shortcuts (not yet implemented) is
|
# the shortcuts mechanism is deliberately excluded from this suite -- see the
|
||||||
# rejected rather than silently mis-applied
|
# note by the `dot kde save` shortcuts exclusion above.
|
||||||
printf 'testrc.General.Greeting=Applied Greeting\nkglobalshortcutsrc.someComponent.someAction=Value\n' >$HOME/.config/dot/kde-manifest
|
|
||||||
dot kde apply >/dev/null 2>&1
|
|
||||||
set -l apply_shortcuts_status $status
|
|
||||||
@test "dot kde apply rejects a manifest entry whose mechanism isn't implemented yet (shortcuts)" $apply_shortcuts_status -ne 0
|
|
||||||
|
|
||||||
# misuse: apply takes no arguments
|
# misuse: apply takes no arguments
|
||||||
printf 'testrc.General.Greeting=Applied Greeting\n' >$HOME/.config/dot/kde-manifest
|
printf 'testrc.General.Greeting=Applied Greeting\n' >$HOME/.config/dot/kde-manifest
|
||||||
|
|||||||
1
.github/keybindings.md
vendored
1
.github/keybindings.md
vendored
@@ -37,3 +37,4 @@ Comma-separated keys are pressed in sequence, not together.
|
|||||||
| `Space`, `e` | neovim | Toggle file explorer (netrw) |
|
| `Space`, `e` | neovim | Toggle file explorer (netrw) |
|
||||||
| `CapsLock` | KDE | Acts as `Esc` (`kxkbrc` `Options=caps:escape_shifted_capslock`) |
|
| `CapsLock` | KDE | Acts as `Esc` (`kxkbrc` `Options=caps:escape_shifted_capslock`) |
|
||||||
| `Shift` + `CapsLock` | KDE | Toggles Caps Lock |
|
| `Shift` + `CapsLock` | KDE | Toggles Caps Lock |
|
||||||
|
| `Meta` + `X` | KDE | Lock Session (moved off `Meta+L`, tracked via `dot kde`) |
|
||||||
|
|||||||
Reference in New Issue
Block a user