summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-20 17:31:50 +0200
committerDanilo M. <danix@danix.xyz>2026-08-20 17:31:50 +0200
commit81c0086a06d4b13478475c2fa05f040c0511bfba (patch)
tree31b754b931b6553594d8d857870064592c5b6443
parentc06809634534ed3b6eb81a1648119428567e71a4 (diff)
downloadqtmaildir-81c0086a06d4b13478475c2fa05f040c0511bfba.tar.gz
qtmaildir-81c0086a06d4b13478475c2fa05f040c0511bfba.zip
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.
-rw-r--r--CLAUDE.md8
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md4
-rw-r--r--tests/CMakeLists.txt1
-rw-r--r--tests/test_mainwindow.cpp17
4 files changed, 8 insertions, 22 deletions
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
diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
index 3d5bcc3..01be5d0 100644
--- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
+++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
@@ -201,9 +201,9 @@ taking that too literally.
| 129 | No inline images in a composed message | v2 | M | open, 2026-08-20, from the item 123 brainstorm. **Blocked on 123.** Wanted by the user. `cid:` from the HTML part with `multipart/related` nested inside the alternative, the most nesting-heavy part of MIME assembly, and markdown offers no syntax for it |
| 130 | A message cannot be attached to another message directly | v2 | S | open, 2026-08-20, from the item 123 brainstorm. **Blocked on 123.** A `message/rfc822` part, which GMime builds natively. The manual route exists from 123's first commit: `save_message` writes the `.eml` and it is attached as a file |
| 131 | The markdown dialect and extensions are fixed | v2 | S | open, 2026-08-20, from the item 123 brainstorm. **Blocked on 123.** Configurable in the shape Hugo's config uses. Deliberately fixed initially: CommonMark plus autolink, strikethrough and tasklist |
-| 132 | Every action must have a shortcut, and that no longer serves | policy | S | open, 2026-08-20, raised by the user during the item 123 brainstorm. `everyActionHasAShortcut` was written when the action list was short; item 123 adds six more, and each new action consumes a chord whether or not anyone would press it. Replacement is the shape `everyActionIsReachableFromAMenu()` already has: menu reachability required, shortcuts a chosen subset. **Not** done inside 123, which would confuse two changes |
+| 132 | Every action must have a shortcut, and that no longer serves | policy | S | done, 2026-08-20. `everyActionHasAShortcut` is deleted and nothing replaces it: `everyActionIsReachableFromAMenu()` is the required rule and a shortcut is now a chosen subset. Nothing else needed changing, since `showShortcutReference()` already printed `(unbound)` for an empty sequence. Verified by unbinding `tag_rules` and running the suite green, which would have failed before |
| 133 | The composer shows no markdown syntax highlighting | v2 | S | open, 2026-08-20, from the item 123 brainstorm. **Blocked on 123.** A `QSyntaxHighlighter` over the composer's editor, so `**bold**` reads as bold while the buffer stays plain markdown. Standard Qt, no dependency. Deliberately after 123's formatting toolbar: agreeing with the grammar about nesting and about code spans suppressing what is inside them is the expensive part, and the toolbar is what makes the feature usable |
-| 134 | The busy indicator is built inline and is about to be built twice | maintenance | S | open, 2026-08-20, raised by the user during the item 123 brainstorm. An indeterminate `QProgressBar` beside a status label, built inline in `MainWindow` as `m_syncProgress`, and item 123's composer needs the same pairing. Extract a widget class and convert `MainWindow` to it. It must expose BOTH modes, not just the indeterminate one `MainWindow` happens to need: item 123's send popup drains a determinate bar during its undo countdown and switches the same widget to indeterminate when the command starts. **Not blocked on 123**, and better done first: if 123 lands first it creates the class itself and this row closes with it. The user's position is general, build once and reuse, so the inline habit `CLAUDE.md` records is a description of the code rather than a rule to follow |
+| 134 | The busy indicator is built inline and is about to be built twice | maintenance | S | done, 2026-08-20, af902e0. `BusyIndicator` (`src/busyindicator.h`) carries both modes: `MainWindow` uses the indeterminate one, and item 123's send popup takes the determinate half for its undo countdown, switching the same widget over when the command starts. Only the BAR was extracted, not the status label this row paired with it. `m_statusLabel` has 34 uses across `MainWindow` for transient messages, selection counts and sync phases, so it belongs to the window rather than to the indicator, and the send popup owns its own phase text |
Sizes are rough: XS under an hour, S a sitting, M a session.
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index f9bc7d1..d1d8a29 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -65,6 +65,7 @@ add_qtmaildir_test(tagdialog)
add_qtmaildir_test(tagrules)
add_qtmaildir_test(rulequery)
add_qtmaildir_test(searchterm)
+add_qtmaildir_test(busyindicator)
add_qtmaildir_test(tagstrip)
add_qtmaildir_test(messagedetailsdialog)
add_qtmaildir_test(translations)
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index 0deaec3..4d70a29 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -178,7 +178,6 @@ private slots:
void noTestCanSeeTheRealLockTable();
void everyKnownActionIsRegistered();
void everyRegisteredActionIsKnown();
- void everyActionHasAShortcut();
void configuredBindingReachesTheAction();
void cidPrefixesAreBangFree();
void cidPrefixesAreDistinctPerMessage();
@@ -489,22 +488,6 @@ void TestMainWindow::everyRegisteredActionIsKnown()
}
}
-void TestMainWindow::everyActionHasAShortcut()
-{
- // An action with no binding is unreachable from the keyboard. Every one
- // of them carries a default, so an empty shortcut means the default table
- // and the action list have drifted apart.
- const Config config;
- MainWindow window(config);
-
- for (const QString &name : window.registeredActionNames()) {
- const QAction *action = window.findChild<QAction *>(name);
- QVERIFY2(action, qPrintable(QStringLiteral("no QAction named '%1'").arg(name)));
- QVERIFY2(!action->shortcut().isEmpty(),
- qPrintable(QStringLiteral("action '%1' has no shortcut").arg(name)));
- }
-}
-
void TestMainWindow::configuredBindingReachesTheAction()
{
// The whole point of [keys]: a user's override must end up on the QAction,