feat(compose): register the six compose actions, item 123

Handlers are empty for now; this commit is the registration, so the three
coverage tests guard every later task rather than being satisfied at the end.

Two corrections to the spec, both found in the code rather than assumed. It
calls for a new top-level Message menu and one already exists, so these join
it; two menus named Message would be a defect. And it says every action
needs a binding, which item 132 changed while this was being planned:
save_message ships with no chord, since it is the rarely-used escape hatch
and menu reachability is now the rule that must hold.

reply_no_quote shares reply's icon and is added to the no-duplicate-icons
exception list for the same reason the five thread actions are: it never
reaches the toolbar, and a menu entry always carries its text. That list is
renamed menuOnlySharedIconActions, after the property that earns the
exemption rather than the tier that first needed it.

Bindings are provisional. The user intends to rework them, and Ctrl+Alt+R
for reply_no_quote is an imperfect fit since that tier elsewhere means a
wider scope rather than a variant.

The six labels went through a mnemonic pass that nothing enforced before.
Four of them collided inside the Message menu on first writing, and the
whole class was invisible to a green suite: Qt does not error on a duplicate
mnemonic, it cycles the highlight instead of activating, so the key simply
stops working. Item 57 had already decided this rule by rejecting a label
that would have collided, but it lived in prose and in one test's comment,
which is precisely why it was broken again here.

noMenuHasTwoEntriesSharingAMnemonic() enforces it now, scoped per menu since
a mnemonic resolves among the open menu's entries, and keyed on
QKeySequence::mnemonic() rather than on parsing & by hand, because && is a
literal ampersand and only Qt answers which key it will dispatch. Three
pre-existing collisions are a named freeze list rather than a silent fix or
a narrowed test: Alt+R three ways and Alt+S twice in Message, Alt+O in View.
Renaming entries a user has had in their fingers since 0.1.0 belongs to the
shortcuts rework, and the freeze is written as exact groups so a new entry
joining any of them still fails.

Two of the test's own design choices came from mutation checks that failed
for the right reason while reporting the wrong thing. Reporting collisions
as pairs was order-dependent, so a new colliding entry re-keyed a frozen
pair and the fresh defect read as "a frozen collision no longer happens";
matching frozen entries by whole string broke the same way, since a growing
group stopped matching its frozen text. It reports whole groups and matches
on menu plus key.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FXF741wz4SY7j5dqvAxMU5

1 files changed, 191 insertions, 4 deletions

diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index 4d70a29..0ad45ee 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -331,6 +331,7 @@ private slots:
 
     void everyActionCarriesAnIcon();
     void everyActionIsReachableFromAMenu();
+    void noMenuHasTwoEntriesSharingAMnemonic();
     void theToolbarDoesNotOverrideTheDesktopButtonStyle();
     void theImportantActionIsLabelledImportant();
     void theImportantActionStillWritesTheFlaggedTag();
@@ -6444,6 +6445,185 @@ void TestMainWindow::everyActionIsReachableFromAMenu()
                             .arg(unreachable.join(QStringLiteral(", ")))));
 }
 
+void TestMainWindow::noMenuHasTwoEntriesSharingAMnemonic()
+{
+    // The sibling of everyActionIsReachableFromAMenu(), and it exists because
+    // the rule it enforces had lived only in prose and in one other test's
+    // COMMENT, and was duly broken the first time a batch of entries was added
+    // to a menu (item 123: `&Reply` against the pre-existing `&Restore from
+    // trash`, both Alt+R).
+    //
+    // Qt does not error on a duplicate mnemonic. It CYCLES between the
+    // colliding entries instead of activating either, so the key silently
+    // stops working and merely moves a highlight. That is worse than it
+    // sounds in the Message menu, where `restore` is deliberately greyed
+    // outside the trash view: the ordinary case was pressing Alt+R and landing
+    // on a disabled entry.
+    //
+    // Item 57 already decided this is a property rather than a taste. It
+    // rejected the label "Starred" for `flag` precisely because it would have
+    // collided with `Mark &spam`, and theImportantActionIsLabelledImportant()
+    // pins the surviving label with that reasoning in its comment. A decision
+    // recorded only in prose is one nobody re-derives.
+    //
+    // Scoped PER MENU, which is what the collision actually is: a mnemonic is
+    // resolved among the entries of the menu that is open, so the same letter
+    // in File and in View is not a conflict.
+    const Config config;
+    MainWindow window(config);
+
+    auto *bar = window.menuBar();
+    QVERIFY(bar);
+
+    // The menu bar's own top-level titles are one such scope too, so the walk
+    // starts by treating the bar as a menu and then descends.
+    QList<QPair<QString, QList<QAction *>>> scopes;
+    scopes.append({ QStringLiteral("the menu bar"), bar->actions() });
+
+    QList<QMenu *> pending;
+    const auto topLevel = bar->actions();
+    for (QAction *action : topLevel) {
+        if (action->menu())
+            pending.append(action->menu());
+    }
+    QVERIFY2(!pending.isEmpty(), "the menu bar holds no menus");
+
+    while (!pending.isEmpty()) {
+        QMenu *menu = pending.takeFirst();
+        const auto entries = menu->actions();
+        scopes.append({ menu->title(), entries });
+        for (QAction *entry : entries) {
+            if (QMenu *sub = entry->menu())
+                pending.append(sub);
+        }
+    }
+
+    // The four collisions that PREDATE this test, measured by running it
+    // against the tree before item 123 touched any label. They are listed
+    // rather than fixed, and rather than being hidden by narrowing the test,
+    // because renaming a shipped menu entry is the user's call and not a
+    // test's: three of them are in menus a user has had in their fingers
+    // since 0.1.0.
+    //
+    // Listed as exact pairs, not as "ignore Alt+R", so this is a freeze and
+    // not an amnesty: a NEW entry colliding on any of these same keys still
+    // fails, because its pair is not on this list. Fixing one is then a
+    // one-line deletion here, which is the point of writing them out.
+    // Written as the FULL GROUP of labels sharing one key in one menu, not as
+    // a pair. A pair is keyed on which entry the walk happened to see first,
+    // so adding a colliding entry ABOVE a frozen one silently re-pairs it and
+    // the new defect gets reported as "a frozen collision no longer happens",
+    // which names the wrong thing entirely. Measured: reinstating `&Reply`
+    // did exactly that before this was changed. A group is order-independent,
+    // so a new entry grows the group and fails as a new collision.
+    static const QStringList knownPreExistingCollisions = {
+        QStringLiteral("&Message: Alt+R shared by \"&Restore from trash\", \"Mark all &read\", \"Tagging &rules...\""),
+        QStringLiteral("&Message: Alt+S shared by \"Mark &spam\", \"Find &stranded deleted mail\""),
+        QStringLiteral("&View: Alt+O shared by \"&Open thread\", \"Zoom &out\""),
+    };
+
+    QStringList collisions;
+    int compared = 0;
+
+    for (const auto &scope : scopes) {
+        // Keyed on the mnemonic Qt itself derives, not on a hand-parsed '&'.
+        // The question is which key Qt will dispatch, and only Qt answers it:
+        // "&&" is a literal ampersand and carries no mnemonic at all.
+        //
+        // A QMap rather than a QHash so the groups come out in a stable key
+        // order, which is what lets the frozen list above be written once and
+        // stay matching.
+        QMap<QString, QStringList> byMnemonic;
+        for (QAction *entry : scope.second) {
+            if (entry->isSeparator())
+                continue;
+            const QKeySequence mnemonic = QKeySequence::mnemonic(entry->text());
+            if (mnemonic.isEmpty())
+                continue;
+            ++compared;
+            byMnemonic[mnemonic.toString(QKeySequence::NativeText)]
+                .append(QStringLiteral("\"%1\"").arg(entry->text()));
+        }
+
+        for (auto it = byMnemonic.cbegin(); it != byMnemonic.cend(); ++it) {
+            if (it.value().size() < 2)
+                continue;
+            // Names the menu, the key and EVERY label in the group, so a
+            // future failure says what to rename without anyone going looking.
+            collisions.append(QStringLiteral("%1: %2 shared by %3")
+                                  .arg(scope.first, it.key(),
+                                       it.value().join(QStringLiteral(", "))));
+        }
+    }
+
+    // The guard, and it is not ceremonial: every assertion below is a loop
+    // that reports success when it runs zero times. A walk that found no
+    // mnemonics at all would pass this test against any label whatsoever.
+    QVERIFY2(compared > 20,
+             qPrintable(QStringLiteral("only %1 menu entries carried a "
+                                       "mnemonic, so this probe measured "
+                                       "almost nothing")
+                            .arg(compared)));
+
+    // Matched on the menu and key only, with the labels compared separately
+    // below. Comparing whole strings made a GROWING group read as a frozen one
+    // disappearing: adding `&Reply` took Alt+R from three labels to four, the
+    // frozen three-label string stopped matching, and the failure said "this
+    // collision no longer happens" about the very key that had just got worse.
+    // Measured twice, once per attempt, which is why the two questions are
+    // asked separately.
+    const auto scopeAndKey = [](const QString &collision) {
+        return collision.left(collision.indexOf(QStringLiteral(" shared by ")));
+    };
+
+    QHash<QString, QString> frozen;
+    for (const QString &known : knownPreExistingCollisions)
+        frozen.insert(scopeAndKey(known), known);
+
+    QStringList unexpected;
+    QSet<QString> stillPresent;
+    for (const QString &collision : collisions) {
+        const QString key = scopeAndKey(collision);
+        const auto known = frozen.constFind(key);
+        if (known == frozen.constEnd()) {
+            // A collision on a key nothing froze: entirely new.
+            unexpected.append(collision);
+            continue;
+        }
+        stillPresent.insert(key);
+        if (*known != collision) {
+            // The key was already colliding, but the CAST has changed, which
+            // for a frozen entry means an entry joined it. Reported as the
+            // new collision it is, naming both what was frozen and what is
+            // there now.
+            unexpected.append(
+                QStringLiteral("%1 (frozen as [%2], now [%3])")
+                    .arg(key, *known, collision));
+        }
+    }
+
+    // A frozen entry that has since been FIXED must not stay on the list
+    // silently, or the list becomes a place stale claims accumulate.
+    QStringList stale;
+    for (const QString &known : knownPreExistingCollisions) {
+        if (!stillPresent.contains(scopeAndKey(known)))
+            stale.append(known);
+    }
+    QVERIFY2(stale.isEmpty(),
+             qPrintable(QStringLiteral("%1 frozen collision(s) no longer "
+                                       "happen, so delete them from "
+                                       "knownPreExistingCollisions: %2")
+                            .arg(stale.size())
+                            .arg(stale.join(QStringLiteral("; ")))));
+
+    QVERIFY2(unexpected.isEmpty(),
+             qPrintable(QStringLiteral("%1 menu mnemonic collision(s), where "
+                                       "Qt cycles the highlight instead of "
+                                       "activating: %2")
+                            .arg(unexpected.size())
+                            .arg(unexpected.join(QStringLiteral("; ")))));
+}
+
 void TestMainWindow::everyActionCarriesAnIcon()
 {
     // Item 56. The complaint was inconsistency, not absence: eight actions had
@@ -6967,15 +7147,22 @@ void TestMainWindow::noTwoActionsShareAnIcon()
     // the words saying which. Giving them five invented shapes would be less
     // clear than the pairing.
     //
+    // reply_no_quote joined them in item 123 for exactly the same reason: it
+    // shares reply's icon, it is a menu entry that always carries its text,
+    // and it is not on the toolbar. The list is therefore no longer only the
+    // thread tier, which is why it is named for the PROPERTY that earns the
+    // exemption rather than for the tier that first needed it.
+    //
     // Named as an exception list rather than by asking the toolbar what it
     // holds, so that PUTTING one of these on the toolbar fails this test
     // rather than silently passing it.
-    static const QStringList menuOnlyThreadActions = {
+    static const QStringList menuOnlySharedIconActions = {
         QStringLiteral("archive_thread"),
         QStringLiteral("delete_thread"),
         QStringLiteral("spam_thread"),
         QStringLiteral("toggle_unread_thread"),
         QStringLiteral("flag_thread"),
+        QStringLiteral("reply_no_quote"),
     };
 
     const Config config;
@@ -6986,7 +7173,7 @@ void TestMainWindow::noTwoActionsShareAnIcon()
     // may sit on the toolbar.
     auto *toolBar = window.findChild<QToolBar *>();
     QVERIFY(toolBar);
-    for (const QString &name : menuOnlyThreadActions) {
+    for (const QString &name : menuOnlySharedIconActions) {
         auto *action = window.findChild<QAction *>(name);
         QVERIFY2(action, qPrintable(QStringLiteral("no action named %1").arg(name)));
         QVERIFY2(!toolBar->actions().contains(action),
@@ -7006,7 +7193,7 @@ void TestMainWindow::noTwoActionsShareAnIcon()
         QVERIFY2(action, qPrintable(QStringLiteral("no action named %1").arg(name)));
         if (!action->icon().isNull())
             ++withIcons;
-        if (menuOnlyThreadActions.contains(name))
+        if (menuOnlySharedIconActions.contains(name))
             continue;
         if (action->icon().isNull())
             continue;
@@ -7033,7 +7220,7 @@ void TestMainWindow::noTwoActionsShareAnIcon()
 
     // And the exception list did not swallow the comparison itself.
     QCOMPARE(compared, KeyMap::knownActions().size()
-                           - menuOnlyThreadActions.size());
+                           - menuOnlySharedIconActions.size());
 
     QVERIFY2(collisions.isEmpty(),
              qPrintable(QStringLiteral("actions sharing one icon: %1")