From 81c0086a06d4b13478475c2fa05f040c0511bfba Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Thu, 20 Aug 2026 17:31:50 +0200 Subject: test(keys): a shortcut is a chosen subset, not a requirement, item 132 everyActionHasAShortcut() was written when the action list was short and every action plausibly deserved a chord. Item 123 adds six more, and under that rule each one consumes a key sequence whether or not anyone would ever press it. Rarely-used actions were being given chords to satisfy a test rather than because a user wanted them. everyActionIsReachableFromAMenu() is the rule that actually matters, and it already has the right shape: it is what stops an action shipping invisible, which is the defect item 103 found when `restore` was reachable by a chord and by nothing a user could see. Discoverability comes from the menu. A shortcut is an accelerator for the things done often. Nothing replaces the deleted test and nothing else needed changing: showShortcutReference() already prints `(unbound)` for an empty sequence, so the code anticipated this and only the test forbade it. Verified rather than assumed: with `tag_rules` unbound in defaultBindings(), an action that is registered, menu-reachable and carries an icon but has no chord at all, the full suite passes. Before this commit it failed. CLAUDE.md's "adding an action is FIVE places" paragraph is updated, including its count of how many are test-enforced, which drops from four to three. --- CLAUDE.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'CLAUDE.md') diff --git a/CLAUDE.md b/CLAUDE.md index a6e19e5..b065741 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -626,11 +626,13 @@ a union over the conversation, so it can arm for a thread whose displayed message is already read. The write is still scoped to that message, so the cost is a no-op rather than a wrong write. -**Adding an action is FIVE places, and four of them are enforced by tests that +**Adding an action is FIVE places, and three of them are enforced by tests that fail in confusing ways.** `KeyMap::knownActions()` (a `Q_ASSERT` in the constructor fires otherwise, and it surfaces in whichever suite happens to build -a `MainWindow` first — `test_tagrules` did), `defaultBindings()` (every action -must be keyboard-reachable), the icon table (every action must carry one), and +a `MainWindow` first — `test_tagrules` did), `defaultBindings()` (OPTIONAL +since item 132: a shortcut is a chosen subset, not a requirement, so an action +nobody would press a chord for simply gets no entry and the shortcut reference +prints it as `(unbound)`), the icon table (every action must carry one), and a MENU. The no-duplicate-icons rule is narrowed to actions that can reach the toolbar, by a named exception list; the five thread actions share their twins' icons because a submenu entry always carries text, and the test asserts none of -- cgit v1.2.3