diff options
Diffstat (limited to 'src')
| -rw-r--r-- | src/carddelegate.cpp | 85 | ||||
| -rw-r--r-- | src/carddelegate.h | 18 | ||||
| -rw-r--r-- | src/cardlayout.cpp | 26 | ||||
| -rw-r--r-- | src/cardlayout.h | 13 | ||||
| -rw-r--r-- | src/keymap.cpp | 29 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 460 | ||||
| -rw-r--r-- | src/mainwindow.h | 101 | ||||
| -rw-r--r-- | src/notmuchworker.cpp | 6 | ||||
| -rw-r--r-- | src/tagchip.cpp | 14 | ||||
| -rw-r--r-- | src/tagchip.h | 13 | ||||
| -rw-r--r-- | src/threadlistmodel.cpp | 383 | ||||
| -rw-r--r-- | src/threadlistmodel.h | 120 | ||||
| -rw-r--r-- | src/types.h | 23 |
13 files changed, 1154 insertions, 137 deletions
diff --git a/src/carddelegate.cpp b/src/carddelegate.cpp index d7d06aa..4e27e6e 100644 --- a/src/carddelegate.cpp +++ b/src/carddelegate.cpp @@ -20,6 +20,7 @@ #include "cardlayout.h" #include "marks.h" +#include "tagchip.h" #include "threadlistmodel.h" #include <QApplication> @@ -31,6 +32,12 @@ namespace { +/// How much of a full-size chip's padding a SIBLING chip keeps. +/// +/// Matched to CardLayout::siblingFont()'s own scale, so the chip shrinks as a +/// whole rather than keeping full-size margins around smaller letters. +constexpr qreal kSiblingPaddingScale = 0.70; + CardLayout::Input inputFor(const QModelIndex &index) { CardLayout::Input in; @@ -58,6 +65,39 @@ QRect CardDelegate::expanderRectFor(const QStyleOptionViewItem &option, .expanderRect; } +QSize CardDelegate::chipSize(const QFontMetrics &metrics, const QString &text, + bool own) +{ + // The padding shrinks with the font for a sibling chip. Left fixed it is + // 18px around roughly 30px of text, so the chip stays wide while its + // letters shrink and the tier reads as "same chip, smaller text". + return own ? TagChip::sizeFor(metrics, text) + : TagChip::sizeFor(metrics, text, kSiblingPaddingScale); +} + +QColor CardDelegate::mutedChipColour(const QColor &chipColour) +{ + if (!chipColour.isValid()) + return chipColour; + + // Saturation only, and NOT a blend toward the background. The accent bar + // above records what blending toward Base costs: on a dark theme it lands + // on the background and the thing disappears. A chip is worse, because its + // fill also has to carry legible text on top of it. + // + // Hue is untouched, so a muted `signed` is still recognisably the same + // colour as a full-size `signed` elsewhere in the list. Lightness is + // untouched too, which is what keeps TagColors::textColourOn() picking the + // same text colour: draining saturation alone moves the fill toward grey + // without moving it toward either black or white, so contrast is preserved + // by construction rather than by hoping. + constexpr float kSaturationScale = 0.45f; + + float h = 0, s = 0, l = 0, a = 0; + chipColour.getHslF(&h, &s, &l, &a); + return QColor::fromHslF(h, s * kSaturationScale, l, a); +} + QColor CardDelegate::accentLineColour(const QColor &accountColour) { if (!accountColour.isValid()) @@ -304,20 +344,47 @@ void CardDelegate::paint(QPainter *painter, const QStyleOptionViewItem &option, : ThreadListModel::PillColoursRole) .toList(); - const QFont chipFont = CardLayout::smallFont(chrome.font); - const QFontMetrics chipMetrics(chipFont); + // A thread card draws its own tags at full size and the rest of the + // conversation's smaller and muted (item 111). The count is where the two + // tiers meet; a message row has no such split and reports its whole list. + // + // Shown rather than dropped, at the user's request: a card sits above a + // conversation, so what its siblings carry is worth seeing, just not at + // the same weight. Before the row has been opened everything is in the own + // tier, so a chip SHRINKS when the split becomes known and none vanishes. + const int ownCount = + isMessage ? tags.size() + : index.data(ThreadListModel::PillOwnCountRole).toInt(); + + const QFont ownFont = CardLayout::smallFont(chrome.font); + const QFont siblingFont = CardLayout::siblingFont(chrome.font); + const QFontMetrics ownMetrics(ownFont); + const QFontMetrics siblingMetrics(siblingFont); + painter->save(); - painter->setFont(chipFont); int x = card.tagRect.left(); for (int i = 0; i < tags.size(); ++i) { - const QSize size = TagChip::sizeFor(chipMetrics, tags.at(i)); + const bool own = i < ownCount; + const QFontMetrics &metrics = own ? ownMetrics : siblingMetrics; + + const QSize size = chipSize(metrics, tags.at(i), own); if (x + size.width() > card.tagRect.right()) break; // Out of room; a clipped chip reads as a rendering fault. - const QColor colour = i < colours.size() - ? colours.at(i).value<QColor>() - : QColor(0x55, 0x55, 0x5f); - TagChip::paint(painter, QRect(QPoint(x, card.tagRect.top()), size), - tags.at(i), colour); + + QColor colour = i < colours.size() ? colours.at(i).value<QColor>() + : QColor(0x55, 0x55, 0x5f); + if (!own) + colour = mutedChipColour(colour); + + // Bottom-aligned, so a smaller chip sits on the same baseline as its + // neighbours rather than floating in the middle of the row. Top + // alignment would step the tier down and read as a layout fault. + const int top = card.tagRect.top() + + (ownMetrics.height() - metrics.height()); + + painter->setFont(own ? ownFont : siblingFont); + TagChip::paint(painter, QRect(QPoint(x, top), size), tags.at(i), + colour); x += size.width() + TagChip::kSpacing; } painter->restore(); diff --git a/src/carddelegate.h b/src/carddelegate.h index 74dee8e..1846359 100644 --- a/src/carddelegate.h +++ b/src/carddelegate.h @@ -76,4 +76,22 @@ public: /// /// Falls back to threadLineColour() for a thread with no account tag. static QColor accentLineColour(const QColor &accountColour); + + /// A tag chip's colour, drained for the SIBLING tier (item 111). + /// + /// Saturation only: hue stays so the tag is still recognisable, and + /// lightness stays so `TagColors::textColourOn()` keeps choosing the same + /// text colour and the chip cannot become unreadable. Exposed for a test, + /// since "muted" has to be asserted on rather than eyeballed. + static QColor mutedChipColour(const QColor &chipColour); + + /// The size of one tag chip on a card, for either tier. + /// + /// The delegate's own arithmetic rather than a duplicate of it: a test + /// calling `TagChip::sizeFor` directly proves what that function does and + /// nothing about what the delegate ASKS for, which is where the padding + /// scale is chosen. A mutation dropping the scale at the call site + /// survived exactly that kind of test. + static QSize chipSize(const QFontMetrics &metrics, const QString &text, + bool own); }; diff --git a/src/cardlayout.cpp b/src/cardlayout.cpp index 0febb4b..3719591 100644 --- a/src/cardlayout.cpp +++ b/src/cardlayout.cpp @@ -107,6 +107,32 @@ QFont CardLayout::smallFont(const QFont &cardFont) return small; } +QFont CardLayout::siblingFont(const QFont &cardFont) +{ + // A FRACTION of the card's font, not a fixed number of points off it. + // + // Subtracting one point was the first attempt and the user reported the + // tiers as indistinguishable. The reason is arithmetic: their desktop is + // 14pt, so the two chip tiers were 13 and 12, a 7% step. Subtraction gives + // a step whose size depends on the desktop font, which is exactly backwards + // — it is largest where the text is already small enough to be fragile. + // + // 0.70 of the card font, against smallFont()'s one point off, so on a 14pt + // desktop the tiers are 13 and 10. Chosen with the user against rendered + // sizes rather than picked. + constexpr qreal kSiblingScale = 0.70; + + QFont small = cardFont; + // pointSizeF() returns -1 for a font set in PIXELS, which qt6ct does, and + // scaling -1 asks for an invalid size that Qt silently ignores, leaving + // both tiers identical. Same split as smallFont(), same reason. + if (small.pointSizeF() > 0.0) + small.setPointSizeF(qMax(6.0, cardFont.pointSizeF() * kSiblingScale)); + else if (small.pixelSize() > 0) + small.setPixelSize(qMax(8, qRound(cardFont.pixelSize() * kSiblingScale))); + return small; +} + int CardLayout::heightFor(const QFont &font) { const QFontMetrics metrics(font); diff --git a/src/cardlayout.h b/src/cardlayout.h index 3512242..de4c41f 100644 --- a/src/cardlayout.h +++ b/src/cardlayout.h @@ -160,6 +160,19 @@ struct CardLayout /// column of content. static QFont smallFont(const QFont &cardFont); + /// The font a SIBLING's tag chip is drawn in: a size down again from + /// smallFont(). + /// + /// A card stands for one message but sits above a conversation, and shows + /// both tiers (item 111). Size is what says which is which, so the two + /// must be visibly different; taking one more step from the same base + /// keeps it following the desktop's font rather than being fixed. + /// + /// Floored like smallFont(), and the floor really is reachable: a desktop + /// at the minimum size gives both tiers the same size, which is a legible + /// degradation rather than an illegible chip. + static QFont siblingFont(const QFont &cardFont); + static CardLayout compute(const Input &input, const QRect &rect, const QFont &font); diff --git a/src/keymap.cpp b/src/keymap.cpp index f6ef6a4..c731bbb 100644 --- a/src/keymap.cpp +++ b/src/keymap.cpp @@ -37,6 +37,21 @@ QStringList KeyMap::knownActions() QStringLiteral("edit_tags"), QStringLiteral("tag_rules"), QStringLiteral("flag"), + // The whole-thread counterparts (item 108). The names above act on the + // message a row displays; these act on its entire thread. Separate + // names rather than a scope flag, because a name is what a user writes + // in [keys]: giving `delete` new semantics would silently change an + // existing config, and renaming it would break one that mentions it. + // + // Unbound by default. They are reached through the "Whole thread" + // submenu, and inventing five more default chords for actions most + // users will rarely want is worse than leaving them to bind what they + // use. + QStringLiteral("archive_thread"), + QStringLiteral("delete_thread"), + QStringLiteral("spam_thread"), + QStringLiteral("toggle_unread_thread"), + QStringLiteral("flag_thread"), QStringLiteral("focus_query"), QStringLiteral("complete_query"), QStringLiteral("save_query"), @@ -90,6 +105,20 @@ QList<QPair<QString, QString>> KeyMap::defaultBindings() // action takes the harder chord rather than the easier one. { QStringLiteral("Ctrl+Shift+U"), QStringLiteral("mark_all_read") }, { QStringLiteral("Ctrl+I"), QStringLiteral("flag") }, + // The whole-thread tier (item 108), one modifier out from each + // message-scoped twin: Ctrl+D deletes the message a row displays, + // Ctrl+Alt+D deletes its conversation. + // + // Ctrl+ALT, not Ctrl+Shift. The obvious pairing is taken twice over: + // Ctrl+Shift+S is `spam` and Ctrl+Shift+U is `mark_all_read`, both + // shipped and both in users' fingers. Reusing either would silently + // change what an existing key does, which is the same objection that + // made these separate action names rather than a flag on the old ones. + { 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 // against Ctrl+U: this is the standing version of tagging, applied to diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index fcf97df..ee883b0 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -832,15 +832,14 @@ void MainWindow::registerActions() // independently would leave one keystroke with the selection in two // states, which is worse than either outcome, so undelete only when // every selected thread is already deleted. - const QModelIndexList rows = - m_threadView->selectionModel()->selectedRows(); - bool allDeleted = !rows.isEmpty(); - for (const QModelIndex &index : rows) { - if (!m_model->threadAt(index.row()).isDeleted()) { - allDeleted = false; - break; - } - } + // + // Each row's own state, message or thread: a reply row is asked about + // the MESSAGE it stands for. Asking its thread made Delete one-way on + // a reply, since a message-scoped write never changes the thread's + // tags and the answer therefore stayed "not deleted" however many + // times it was pressed. Item 88 fixed which thread was read here; this + // is about reading a message at all. + const bool allDeleted = everySelectedRowHasTag(QStringLiteral("deleted")); if (allDeleted) tagSelected({}, { QStringLiteral("deleted") }, tr("Undelete")); @@ -866,21 +865,26 @@ void MainWindow::registerActions() }); addAction(QStringLiteral("toggle_unread"), tr("Toggle &unread"), tr("Toggle the unread tag"), [this]() { - // The direction comes from the current row, but the change applies to - // the whole selection, so a mixed selection lands in one consistent - // state rather than each row flipping its own way. - const QModelIndex current = m_threadView->currentIndex(); - if (!current.isValid()) - return; - const ThreadSummary thread = m_model->threadAt(current.row()); + // The state of whatever the rows STAND FOR, which for a reply is the + // message and not its thread. See everySelectedRowHasTag(): reading + // the thread here made the key dead on a reply. + // + // Item 88 fixed WHICH thread this read. That was necessary and not + // sufficient: a reply needs a message read, not a better thread. + // + // Per selection rather than per current row, matching Delete. The old + // comment said the direction came from the current row while the + // change applied to the whole selection, which is the same split that + // makes a mixed selection land in two states. + const bool unread = everySelectedRowHasTag(QStringLiteral("unread")); // An explicit toggle overrides the automatic one. Without this, marking // a thread unread by hand would be undone a moment later by a timer // armed when it was opened, and the key would look broken. m_markReadTimer->stop(); - m_markReadThreadId.clear(); + m_markReadMessageId.clear(); - if (thread.isUnread()) + if (unread) tagSelected({}, { QStringLiteral("unread") }, tr("Mark read")); else tagSelected({ QStringLiteral("unread") }, {}, tr("Mark unread")); @@ -894,6 +898,58 @@ void MainWindow::registerActions() tr("Add or remove any tag on the selected threads"), [this]() { editTagsOnSelection(); }); + + // The whole-thread counterparts (item 108). Separate action NAMES, because + // a name is what a user writes in [keys]: reusing `delete` with new + // semantics would silently change what an existing config does, and + // renaming it would break one that mentions it. These are unbound by + // default; the submenu is how they are reached. + // + // Each one is its message-scoped twin with TagScope::Thread, so the two + // cannot drift in what they write, only in what they write it to. + addAction(QStringLiteral("archive_thread"), tr("&Archive thread"), + tr("Remove inbox from every message of the selected threads"), + [this]() { + tagSelected({}, { QStringLiteral("inbox") }, tr("Archive thread"), + TagScope::Thread); + }); + addAction(QStringLiteral("delete_thread"), tr("&Delete thread"), + tr("Add or remove the deleted tag on whole threads"), [this]() { + if (everySelectedRowHasTag(QStringLiteral("deleted"), TagScope::Thread)) { + tagSelected({}, { QStringLiteral("deleted") }, + tr("Undelete thread"), TagScope::Thread); + } else { + tagSelected({ QStringLiteral("deleted") }, {}, tr("Delete thread"), + TagScope::Thread); + } + }); + addAction(QStringLiteral("spam_thread"), tr("Mark thread as &spam"), + tr("Add spam and remove inbox on whole threads"), [this]() { + 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]() { + // 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); + } + }); + addAction(QStringLiteral("flag_thread"), tr("&Important"), + tr("Mark every message of the selected threads as important"), + [this]() { + tagSelected({ QStringLiteral("flagged") }, {}, + tr("Mark thread important"), TagScope::Thread); + }); addAction(QStringLiteral("tag_rules"), tr("Tagging &rules..."), tr("Edit the rules that tag mail as it arrives"), [this]() { showTagRulesDialog(); @@ -975,7 +1031,7 @@ void MainWindow::registerActions() m_messageView->clear(); showPlaceholderPane(); m_markReadTimer->stop(); - m_markReadThreadId.clear(); + m_markReadMessageId.clear(); }); addAction(QStringLiteral("clear_selection"), tr("Clear &selection"), tr("Blank the message pane and deselect every thread"), @@ -1010,7 +1066,7 @@ void MainWindow::registerActions() m_messageView->clear(); showPlaceholderPane(); m_markReadTimer->stop(); - m_markReadThreadId.clear(); + m_markReadMessageId.clear(); }); addAction(QStringLiteral("select_all"), tr("Select &all threads"), tr("Select every thread in the current result list"), [this]() { @@ -1061,6 +1117,8 @@ void MainWindow::buildMenus() messageMenu->addAction(m_actions.value(QStringLiteral("mark_all_read"))); messageMenu->addAction(m_actions.value(QStringLiteral("edit_tags"))); messageMenu->addAction(m_actions.value(QStringLiteral("flag"))); + messageMenu->addSeparator(); + messageMenu->addMenu(buildThreadActionsMenu(messageMenu)); // Separated from the entries above: those act on the selection, this edits // a rule store shared with mailctl and changes nothing that is on screen. messageMenu->addSeparator(); @@ -1141,6 +1199,20 @@ void MainWindow::buildMenus() { QStringLiteral("zoom_in"), QStringLiteral("zoom-in") }, { QStringLiteral("zoom_out"), QStringLiteral("zoom-out") }, { QStringLiteral("zoom_reset"), QStringLiteral("zoom-original") }, + + // The whole-thread tier (item 108) deliberately SHARES each icon with + // its message-scoped twin. The no-duplicates rule exists because the + // toolbar can be icon-only, where the icon is the entire control; + // these five never reach the toolbar. They live in a submenu whose + // entries always carry text, and "Delete thread" beside the delete + // icon is the honest pairing: the same operation, a wider scope, with + // the words saying which. Inventing five different shapes for the same + // five operations would be less clear, not more. + { 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("flag_thread"), QStringLiteral("mail-mark-important") }, }; for (auto it = themeIcons.cbegin(); it != themeIcons.cend(); ++it) { QAction *action = m_actions.value(it.key()); @@ -1168,6 +1240,8 @@ void MainWindow::buildMenus() m_threadContextMenu->addAction(m_actions.value(QStringLiteral("flag"))); m_threadContextMenu->addAction(m_actions.value(QStringLiteral("edit_tags"))); m_threadContextMenu->addSeparator(); + m_threadContextMenu->addMenu(buildThreadActionsMenu(m_threadContextMenu)); + m_threadContextMenu->addSeparator(); m_threadContextMenu->addAction(m_actions.value(QStringLiteral("select_all"))); m_threadView->setContextMenuPolicy(Qt::CustomContextMenu); @@ -2384,7 +2458,7 @@ void MainWindow::markAllRead() // An automatic mark-read armed for the open thread would fire after this // and push a second, redundant command onto the stack. m_markReadTimer->stop(); - m_markReadThreadId.clear(); + m_markReadMessageId.clear(); const QString description = tr("Mark all read"); sendThreadTagChange(threadIds, {}, { QStringLiteral("unread") }, @@ -2448,16 +2522,16 @@ void MainWindow::onSelectionChanged() // Qt 6.11), so that handler still sees the old count and returns // without loading anything. // - // Compared per row kind. A message row's row number indexes its - // siblings, so threadAt() on one answers about an unrelated thread and - // the comparison below would be against the wrong id. + // Compared per row kind: a message row is identified by its message id + // and a thread row by its thread id, which are different questions. + // threadFor() resolves the thread either way, so the row-number trap + // (item 88) cannot be re-entered here even if this branch changes. const QModelIndex current = m_threadView->currentIndex(); if (current.isValid()) { const bool changed = m_model->isMessageRow(current) ? m_model->messageAt(current).messageId != m_currentMessageId - : m_model->threadAt(current.row()).threadId - != m_currentThreadId; + : m_model->threadFor(current).threadId != m_currentThreadId; if (changed) onThreadSelected(current, QModelIndex()); } @@ -2507,7 +2581,7 @@ void MainWindow::onSelectionChanged() // current, so onThreadSelected never runs and its guard never fires. The // pane and the pending timer have to be dealt with here as well. m_markReadTimer->stop(); - m_markReadThreadId.clear(); + m_markReadMessageId.clear(); m_currentThreadId.clear(); m_currentMessageId.clear(); m_currentMessageThreadId.clear(); @@ -2563,7 +2637,7 @@ void MainWindow::onThreadSelected(const QModelIndex ¤t, // pane that no longer shows the thread. if (m_threadView->selectionModel()->selectedRows().size() > 1) { m_markReadTimer->stop(); - m_markReadThreadId.clear(); + m_markReadMessageId.clear(); m_currentThreadId.clear(); m_currentMessageId.clear(); m_currentMessageThreadId.clear(); @@ -2572,21 +2646,22 @@ void MainWindow::onThreadSelected(const QModelIndex ¤t, return; } - // A message row renders that message ALONE. Checked before threadAt(), - // which takes a top-level row number: a child's row number indexes its - // siblings, so passing it here would silently load whichever thread happens - // to sit at that position in the list. + // A message row renders that message ALONE, so the kind of row still has + // to be checked here: this is a different render path, not a different way + // of naming the same thread. if (m_model->isMessageRow(current)) { const MessageNode node = m_model->messageAt(current); if (node.messageId.isEmpty()) return; - // No mark-read timer for a message row in this pass. Marking one - // message of a thread read is a per-message tag write, and the - // pending-edit map is keyed by thread; item 28 is the record of what - // happens when that count goes wrong. + // Armed for a reply too, since item 87. It deliberately was not + // before, because the write was thread-scoped and reading one reply + // would have marked the whole conversation read. With the write scoped + // to one message that objection is gone, and leaving it unarmed would + // make the message the user is actually reading the one kind that + // never gets marked read. m_markReadTimer->stop(); - m_markReadThreadId.clear(); + m_markReadMessageId.clear(); m_currentThreadId.clear(); m_currentMessageId = node.messageId; @@ -2594,16 +2669,27 @@ void MainWindow::onThreadSelected(const QModelIndex ¤t, // thread it came from is what the refreshed list is checked against. m_currentMessageThreadId = node.threadId; m_messageView->setTags(node.tags); + + // After m_currentMessageId is set: the handler compares against it to + // tell "still showing this" from "the selection moved on". + scheduleMarkRead(node.messageId, node.isUnread()); + QMetaObject::invokeMethod(m_worker, "loadMessage", Qt::QueuedConnection, Q_ARG(QString, node.messageId), Q_ARG(quint64, m_generation)); return; } - const ThreadSummary thread = m_model->threadAt(current.row()); + const ThreadSummary thread = m_model->threadFor(current); m_currentThreadId = thread.threadId; m_messageView->setTags(thread.tags); - scheduleMarkRead(thread); + + // The message the card displays, not the thread. The summary's `unread` is + // a union over the conversation, so this can arm for a thread whose first + // message is already read; the write is scoped to that message either way, + // so the cost is a no-op rather than a wrong write. Narrowing it properly + // needs per-message state in ThreadSummary, which nothing carries yet. + scheduleMarkRead(thread.firstMessageId, thread.isUnread()); // The root card IS the thread's first message, so selecting it renders // that message. Never the whole conversation: that path is gone (item 66). @@ -2654,6 +2740,25 @@ void MainWindow::onMessageLoaded(const QVector<MessageRef> &messages, if (m_currentMessageId.isEmpty()) return; + // The worker's answer is the authority on what THIS message carries, and + // it is the only place that truth arrives. Until it does, a thread row can + // only offer ThreadSummary::tags, which is notmuch's union over the + // conversation: a four-message thread whose third message is signed makes + // the root card and the pane both claim `signed` for a message that is not + // (item 110). Recording it here corrects the card and gives a + // message-scoped write something to update, which is why marking a root + // message read left the row bold before. + // + // A reply already has its own node from the thread tree, and + // setRootMessageTags ignores anything that is not a root. + for (const MessageRef &ref : messages) + m_model->setRootMessageTags(ref.messageId, ref.tags); + + // The pane follows the same correction. setTags() at selection time can + // only have used the union. + if (messages.size() == 1) + m_messageView->setTags(messages.first().tags); + renderMessages(messages); } @@ -2741,7 +2846,12 @@ void MainWindow::renderMessages(const QVector<MessageRef> &messages) void MainWindow::revertPendingTagChange() { - if (m_pendingThreadIds.isEmpty()) + // Either scope can be in flight: a thread-scoped write names threads, a + // message-scoped one names messages, and both are now applied + // optimistically. Checking only the thread ids left a failed message write + // showing its optimistic state for good, with nothing to correct it until + // the next query. + if (m_pendingThreadIds.isEmpty() && m_pendingChange.messageIds.isEmpty()) return; // Put the rows back the way they were. Only the model is touched: the @@ -2750,6 +2860,10 @@ void MainWindow::revertPendingTagChange() m_model->applyTagChange(threadId, m_pendingChange.removed, m_pendingChange.added); } + for (const QString &messageId : m_pendingChange.messageIds) { + m_model->applyMessageTagChange(messageId, m_pendingChange.removed, + m_pendingChange.added); + } // The undo entry describes a change that never landed, so it would apply a // spurious inverse if the user pressed undo. @@ -2805,18 +2919,37 @@ void MainWindow::flushHeldEdits() m_flushGeneration = m_generation; for (const HeldEdit &edit : edits) { - // Take the optimistic update back before sending, because - // sendThreadTagChange() applies it again. applyTagChange() is - // idempotent per tag so the rows do not visibly flicker; without this - // the change is applied twice and a later revert undoes only one of - // them, leaving a row showing a tag the database never got. + // Take the optimistic update back before sending, because the send + // applies it again. Both apply functions are idempotent per tag so the + // rows do not visibly flicker; without this the change is applied + // twice and a later revert undoes only one of them, leaving a row + // showing a tag the database never got. for (const QString &threadId : edit.threadIds) { m_model->applyTagChange(threadId, edit.change.removed, edit.change.added); } + for (const QString &messageId : edit.change.messageIds) { + m_model->applyMessageTagChange(messageId, edit.change.removed, + edit.change.added); + } - sendThreadTagChange(edit.threadIds, edit.change.added, - edit.change.removed, edit.change.description); + // By SCOPE. A held edit is one or the other, never both: a + // message-scoped edit carries no thread ids, so sending it through + // sendThreadTagChange() sent an empty list, which returns immediately. + // The edit was applied to the row, counted as unsynced and then + // dropped without ever being written, which is data loss with a + // pending count claiming the opposite. + // + // Escalating it to its thread instead would be worse: Delete on one + // reply would delete every message in the conversation. + if (!edit.threadIds.isEmpty()) { + sendThreadTagChange(edit.threadIds, edit.change.added, + edit.change.removed, edit.change.description); + } + if (!edit.change.messageIds.isEmpty()) { + sendMessageTagChange(edit.change.messageIds, edit.change.added, + edit.change.removed, edit.change.description); + } } // Held edits stop counting as held; what counts now is whatever @@ -3149,7 +3282,7 @@ void MainWindow::onRowDoubleClicked(const QModelIndex &index) // the recovery selects. What is cancelled is the arming for a row the user // is leaving. m_markReadTimer->stop(); - m_markReadThreadId.clear(); + m_markReadMessageId.clear(); // Reuses the stale-thread recovery outright, which already runs thread:<id>, // expands the thread when the row arrives, selects the target message once @@ -3604,27 +3737,32 @@ void MainWindow::updatePendingIndicator() m_pendingLabel->show(); } -void MainWindow::scheduleMarkRead(const ThreadSummary &thread) +void MainWindow::scheduleMarkRead(const QString &messageId, bool unread) { - // Any pending timer belongs to a thread that is no longer on screen. + // Any pending timer belongs to a message that is no longer on screen. // Stopping unconditionally is what makes this a restart rather than a - // stack: arrowing down ten threads must mark only the one still selected + // stack: arrowing down ten rows must mark only the one still selected // when the timer finally fires. m_markReadTimer->stop(); - m_markReadThreadId.clear(); + m_markReadMessageId.clear(); // Negative disables the behaviour entirely, per the config key. const int delay = m_config.markReadDelayMs(); if (delay < 0) return; - // Nothing to do for a thread that is already read. Checked here rather - // than in the handler so no timer is even armed, which keeps a read thread - // from arming one that would fire into a no-op write. - if (!thread.tags.contains(QStringLiteral("unread"))) + // A row the model cannot name a message for. Marking its thread instead + // would be the escalation item 108 removed. + if (messageId.isEmpty()) + return; + + // Nothing to do for a message that is already read. Checked here rather + // than in the handler so no timer is even armed, which keeps a read + // message from arming one that would fire into a no-op write. + if (!unread) return; - m_markReadThreadId = thread.threadId; + m_markReadMessageId = messageId; // Zero means immediately, and a zero-interval timer still fires through // the event loop rather than reentering the selection handler. @@ -3633,62 +3771,167 @@ void MainWindow::scheduleMarkRead(const ThreadSummary &thread) void MainWindow::markCurrentThreadRead() { - if (m_markReadThreadId.isEmpty()) + if (m_markReadMessageId.isEmpty()) return; // The selection can have moved on between the timer being armed and it - // firing, and the thread can have been marked read by hand in that window. - // Both mean this timer has nothing left to do. - if (m_markReadThreadId != m_currentThreadId) { - m_markReadThreadId.clear(); - return; - } - - const QModelIndex current = m_threadView->currentIndex(); - if (!current.isValid()) { - m_markReadThreadId.clear(); - return; - } - - const ThreadSummary thread = m_model->threadAt(current.row()); - if (thread.threadId != m_markReadThreadId - || !thread.tags.contains(QStringLiteral("unread"))) { - m_markReadThreadId.clear(); + // firing, and the message can have been marked read by hand in that + // window. Both mean this timer has nothing left to do. + // + // Compared against what the PANE is showing rather than against the + // selection: those are the same thing for both kinds of row, and the pane + // is what "the message the user is reading" means. + const QString showing = m_currentMessageId.isEmpty() + ? currentThreadFirstMessageId() + : m_currentMessageId; + if (m_markReadMessageId != showing) { + m_markReadMessageId.clear(); return; } - const QStringList threadIds = { m_markReadThreadId }; - m_markReadThreadId.clear(); + const QStringList messageIds = { m_markReadMessageId }; + m_markReadMessageId.clear(); - // sendThreadTagChange, NOT tagSelected: this deliberately does not go on + // sendMessageTagChange, NOT tagSelected: this deliberately does not go on // the undo stack. The user never took this action, so hijacking Ctrl+Z to // reverse it would undo something they did not do, and toggle_unread // already gives them a direct way to put it back. Decided 2026-08-03. // + // MESSAGE-scoped since item 87. The thread-wide write was coherent while a + // root card rendered the whole conversation; item 66 made it render one + // message and left the write alone, so reading one message marked replies + // read that had never been displayed. maildir.synchronize_flags is on, so + // that reached the server and nothing here could put it back. + // // It still funnels through the one applyTags path, per CLAUDE.md; what // differs is only whether the inverse is pushed, which is a window-level // decision above the worker. - sendThreadTagChange(threadIds, {}, { QStringLiteral("unread") }, - tr("Mark read")); + sendMessageTagChange(messageIds, {}, { QStringLiteral("unread") }, + tr("Mark read")); } -void MainWindow::editTagsOnSelection() +QString MainWindow::currentThreadFirstMessageId() const +{ + // The message a selected THREAD row displays. m_currentThreadId is what + // the pane was opened from, so this resolves through the model rather than + // through the selection, which can have moved. + if (m_currentThreadId.isEmpty()) + return {}; + + for (int row = 0; row < m_model->rowCount(QModelIndex()); ++row) { + const ThreadSummary thread = m_model->threadAt(row); + if (thread.threadId == m_currentThreadId) + return thread.firstMessageId; + } + return {}; +} + +bool MainWindow::everySelectedRowHasTag(const QString &tag, + TagScope scope) const { + // What a toggle asks before choosing its direction, for both Delete and + // Toggle unread. + // + // Per ROW, and each row is asked about what it stands for: a reply row + // reports the message's tags, a thread row the thread's. Asking a reply's + // THREAD is the trap both toggles fell into. The write is message-scoped, + // so it never changes the thread's tags; the thread's answer therefore + // never moves however many times the key is pressed, and the toggle + // becomes one-way. On the second press it re-sends a tag the message + // already has, which is a no-op, and a no-op repaints nothing. + // + // One direction for the WHOLE selection, which is the rule Delete + // established: toggling each row independently would leave one keystroke + // with the selection in two states, which is worse than either outcome. const QModelIndexList rows = m_threadView->selectionModel()->selectedRows(); - if (rows.isEmpty()) { - showTransientStatus(tr("Select a thread first")); - return; + if (rows.isEmpty()) + return false; + + for (const QModelIndex &index : rows) { + QStringList tags; + if (scope == TagScope::Thread) { + tags = m_model->threadFor(index).tags; + } else if (m_model->isMessageRow(index)) { + tags = m_model->messageAt(index).tags; + } else { + // The thread's summary, and this is a KNOWN approximation rather + // than an oversight. A thread row acts on the message its card + // displays, but that message's own tags are never in the model: + // setThreadMessages drops depth 0 because the root row stands for + // it, so there is no node to read and messageById() cannot find + // one. The summary is a union over the thread, so it answers + // "unread" while ANY message is. + // + // The consequence is bounded and only affects the DIRECTION a + // toggle picks, never what it writes: on a thread whose first + // message is read while a later one is not, Toggle unread reads + // the thread as unread and marks the first message read again, a + // no-op. Fixing it properly needs per-message state in + // ThreadSummary, which is the same thing item 87 needs; leave it + // for that item rather than guessing here. + tags = m_model->threadFor(index).tags; + } + if (!tags.contains(tag)) + return false; } + return true; +} + +ThreadSummary MainWindow::threadForCurrentRowForTesting() const +{ + return m_model->threadFor(m_threadView->currentIndex()); +} - // How many of the selected threads carry each tag, which is what tells a - // tag that is on all of them from one that is on some. +QMenu *MainWindow::buildThreadActionsMenu(QWidget *parent) +{ + // Built per call rather than shared. A QMenu belongs to one place in one + // menu tree, and adding the same instance to both the menu bar and the + // context menu gives whichever added it last the object. The ACTIONS are + // shared, which is what has to stay consistent; the menu holding them is + // just a container. + auto *menu = new QMenu(tr("&Whole thread"), parent); + menu->setObjectName(QStringLiteral("threadActionsMenu")); + menu->addAction(m_actions.value(QStringLiteral("archive_thread"))); + 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("flag_thread"))); + return menu; +} + +QHash<QString, int> MainWindow::selectionTagCounts() const +{ + // How many of the selected rows carry each tag, which is what tells a tag + // that is on all of them from one that is on some. The dialog's tri-state + // checkboxes are built from this, so a wrong count offers to remove a tag + // the selection does not have. + // + // threadFor(index), NOT threadAt(index.row()): a reply's row number named + // an unrelated thread, so selecting one counted the tags of whichever + // thread sat at that position in the list (item 88). QHash<QString, int> counts; + const QModelIndexList rows = + m_threadView->selectionModel()->selectedRows(); for (const QModelIndex &index : rows) { - const ThreadSummary thread = m_model->threadAt(index.row()); + const ThreadSummary thread = m_model->threadFor(index); for (const QString &tag : thread.tags) counts[tag] += 1; } + return counts; +} + +void MainWindow::editTagsOnSelection() +{ + const QModelIndexList rows = + m_threadView->selectionModel()->selectedRows(); + if (rows.isEmpty()) { + showTransientStatus(tr("Select a thread first")); + return; + } + + const QHash<QString, int> counts = selectionTagCounts(); // m_knownTags is the same list the query completer uses, so the dialog // offers every tag in the database without a round trip. @@ -3708,7 +3951,7 @@ void MainWindow::editTagsOnSelection() } void MainWindow::tagSelected(const QStringList &add, const QStringList &remove, - const QString &description) + const QString &description, TagScope tagScope) { const QModelIndexList rows = m_threadView->selectionModel()->selectedRows(); @@ -3719,7 +3962,13 @@ void MainWindow::tagSelected(const QStringList &add, const QStringList &remove, // A message row's row number indexes its siblings, so the old // threadAt(index.row()) mapping silently acted on whichever thread sat at // that position in the list. - const ActionScope scope = m_model->scopeFor(rows); + // + // Message scope by default since item 108: a thread row displays one + // message, so acting on it acts on that message. Thread scope is what the + // "Whole thread" actions ask for explicitly. + const ActionScope scope = tagScope == TagScope::Thread + ? m_model->scopeFor(rows) + : m_model->messageScopeFor(rows); if (scope.isEmpty()) return; @@ -3758,10 +4007,33 @@ void MainWindow::sendMessageTagChange(const QStringList &messageIds, if (messageIds.isEmpty()) return; - // No optimistic model update. applyTagChange is keyed by THREAD and would - // repaint the whole row as though every message in it had changed, which - // for a one-message edit is a lie the user would see and then watch - // silently correct itself on the next query. + // Optimistically applied to each MESSAGE's own row. applyTagChange is + // keyed by thread and would repaint the whole card as though every message + // in it had changed, which for a one-message edit is a lie; that is why + // this path had no optimistic update at all, and the cost was that Delete + // and Toggle unread on a reply moved the pending count and changed nothing + // the user could see. The reply's own row is where the feedback belongs. + for (const QString &messageId : messageIds) + m_model->applyMessageTagChange(messageId, add, remove); + + // The strip shows the tags of the message ON DISPLAY, so it has to follow + // an edit to that message rather than waiting for the next selection. The + // thread path has carried this since the strip existed; without it here, a + // message-scoped edit repainted the list row and left the pane's chips + // describing the message as it was, until the user selected away and back. + // + // Keyed on m_currentMessageId, which is set only for a message row, so a + // write to some other reply cannot repaint the open one with its tags. + // + // Read by ID, not from currentIndex(): the two agree today, and a guard + // that depends on them agreeing would put the WRONG message's tags in the + // pane on the day they do not. The id is what the pane is actually + // showing. + if (!m_currentMessageId.isEmpty() + && messageIds.contains(m_currentMessageId)) { + m_messageView->setTags( + m_model->messageById(m_currentMessageId).tags); + } // The accounts this touches, resolved through the containing threads: the // account is a property of the thread, and the sync needs the channel @@ -3821,7 +4093,7 @@ void MainWindow::sendThreadTagChange(const QStringList &threadIds, if (threadIds.contains(m_currentThreadId)) { const QModelIndex current = m_threadView->currentIndex(); if (current.isValid()) - m_messageView->setTags(m_model->threadAt(current.row()).tags); + m_messageView->setTags(m_model->threadFor(current).tags); } // A sync holds notmuch's exclusive write lock, and the worker's read-write diff --git a/src/mainwindow.h b/src/mainwindow.h index 614430f..e741416 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -167,6 +167,37 @@ public: /// command was pushed, which is what "this did nothing" has to assert. int undoDepthForTesting() const { return m_undoStack.count(); } + /// The text of the command on top of the undo stack. + /// + /// A test seam for the DIRECTION a toggle chose. Delete and Undelete both + /// push one command and touch the same rows, so a depth or an id says + /// nothing about which way the toggle went, which is exactly what item 88 + /// got wrong. + QString undoTextForTesting() const { return m_undoStack.undoText(); } + + /// The tag counts the tag dialog would be built from, for the current + /// selection. A test seam: the dialog is modal, so the counts cannot be + /// observed through it. + QHash<QString, int> selectionTagCountsForTesting() const + { + return selectionTagCounts(); + } + + /// The thread the current row belongs to. A test seam for item 88's + /// resolution itself, reachable when the write it guards is not. + ThreadSummary threadForCurrentRowForTesting() const; + + /// Sends a message-scoped tag change directly. A test seam for the cases + /// where driving the action would move the selection, which is sometimes + /// the very thing under test. + void sendMessageTagChangeForTesting(const QStringList &messageIds, + const QStringList &add, + const QStringList &remove, + const QString &description) + { + sendMessageTagChange(messageIds, add, remove, description); + } + /// The ids the last tag change was sent for, and whether they were thread /// ids or message ids. /// @@ -180,6 +211,11 @@ public: return m_pendingChange.messageIds; } + /// The whole change last sent, for tests about WHAT was written rather + /// than what it was written to. The tags are the same under either scope, + /// so a test about a tag name should read this instead of a model row. + TagChange pendingChangeForTesting() const { return m_pendingChange; } + /// The generation a worker reply must carry to be accepted. /// /// A test seam: onQueryFinished() discards a reply whose generation is @@ -563,13 +599,32 @@ private: const QString &description, const std::function<void()> &handler); + /// What a tag action acts on. + /// + /// Since item 108 a thread ROW means the one message its card displays, so + /// Message is the default and Thread is the explicit choice the user makes + /// through the "Whole thread" submenu. Before that there was no choice: + /// a thread row always meant the conversation. + enum class TagScope { + Message, ///< The message each selected row displays. + Thread, ///< Every message of each selected row's thread. + }; + void tagSelected(const QStringList &add, const QStringList &remove, - const QString &description); + const QString &description, + TagScope scope = TagScope::Message); /// Starts, restarts or cancels the mark-read timer for a newly opened - /// thread. Cancels outright for a thread that is not unread, so an already - /// read thread never schedules a write that would change nothing. - void scheduleMarkRead(const ThreadSummary &thread); + /// MESSAGE. Cancels outright for one that is not unread, so an already read + /// message never schedules a write that would change nothing. + /// + /// Takes the id and the state separately because the two come from + /// different places: a reply row has a MessageNode, and a thread row has + /// only its summary, whose `unread` is a union over the conversation. + void scheduleMarkRead(const QString &messageId, bool unread); + + /// The message id of the thread the pane was opened from, or empty. + QString currentThreadFirstMessageId() const; /// Removes `unread` from the thread the timer was armed for, if it is still /// the one on screen. @@ -631,6 +686,29 @@ private: /// /// The only route to an arbitrary tag: every other tag action writes a /// hardcoded name. + /// The "Whole thread" submenu, built fresh for each parent that needs one. + /// + /// A QMenu lives in one menu tree, so the menu bar and the context menu get + /// their own instance. The actions inside are shared, which is what has to + /// stay consistent between them. + QMenu *buildThreadActionsMenu(QWidget *parent); + + /// Per-tag counts across the selected rows, for the tag dialog. + QHash<QString, int> selectionTagCounts() const; + + /// True when every selected row already carries \p tag, which is what a + /// toggle asks before choosing its direction. + /// + /// Under Message scope each row answers about what it STANDS FOR: a reply + /// row about its message, a thread row about the message its card + /// displays. Asking a reply's thread makes a toggle one-way, since the + /// message-scoped write never changes the thread's tags. + /// + /// Under Thread scope a row answers about its whole thread, so the + /// question matches the write the thread actions are about to make. + bool everySelectedRowHasTag(const QString &tag, + TagScope scope = TagScope::Message) const; + void editTagsOnSelection(); /// Set once the user has answered the exit prompt, or once a sync started @@ -981,10 +1059,17 @@ private: /// fires, not each one passed through. QTimer *m_markReadTimer = nullptr; - /// The thread m_markReadTimer will mark read. Compared against the current - /// selection when it fires, so a timer that outlives its thread does - /// nothing rather than marking the wrong one. - QString m_markReadThreadId; + /// The MESSAGE m_markReadTimer will mark read. Compared against what the + /// pane is showing when it fires, so a timer that outlives its message + /// does nothing rather than marking the wrong one. + /// + /// A message id, not a thread id, since item 87. The timer used to mark + /// the whole thread, which was coherent while a root card rendered the + /// whole conversation and stopped being so when item 66 made it render + /// one message: reading one message marked replies read that had never + /// been displayed, and with maildir.synchronize_flags on that reaches the + /// server. + QString m_markReadMessageId; /// Debounces the automatic sync that follows a tag edit (item 71). /// diff --git a/src/notmuchworker.cpp b/src/notmuchworker.cpp index a3a2fd5..b152830 100644 --- a/src/notmuchworker.cpp +++ b/src/notmuchworker.cpp @@ -314,6 +314,9 @@ void NotmuchWorker::runQuery(const QString &query, quint64 generation, if (matched) { summary.firstMessageId = QString::fromUtf8( notmuch_message_get_message_id(message)); + // The card's own tags, beside the thread's union above. + // Same walk, same index read, no extra query. + summary.firstMessageTags = tagsOf(message); break; } } @@ -323,6 +326,9 @@ void NotmuchWorker::runQuery(const QString &query, quint64 generation, if (notmuch_message_t *first = notmuch_messages_get(top)) { summary.firstMessageId = QString::fromUtf8( notmuch_message_get_message_id(first)); + // The card's own tags, beside the thread's union above. + // Same walk, same index read, no extra query. + summary.firstMessageTags = tagsOf(first); } } } diff --git a/src/tagchip.cpp b/src/tagchip.cpp index d770a56..ec83641 100644 --- a/src/tagchip.cpp +++ b/src/tagchip.cpp @@ -29,8 +29,18 @@ namespace TagChip { QSize sizeFor(const QFontMetrics &metrics, const QString &text) { - return QSize(metrics.horizontalAdvance(text) + kPaddingX * 2, - metrics.height() + kPaddingY * 2); + return sizeFor(metrics, text, 1.0); +} + +QSize sizeFor(const QFontMetrics &metrics, const QString &text, qreal scale) +{ + // Floored at 2 a side: the corner radius is half the chip's height, so the + // leftmost and rightmost pixels of the fill are curve rather than usable + // width, and text set flush against it touches the round end. + const int padX = qMax(2, qRound(kPaddingX * scale)); + const int padY = qMax(0, qRound(kPaddingY * scale)); + return QSize(metrics.horizontalAdvance(text) + padX * 2, + metrics.height() + padY * 2); } void paint(QPainter *painter, const QRect &rect, const QString &text, diff --git a/src/tagchip.h b/src/tagchip.h index 5514cae..68e1e55 100644 --- a/src/tagchip.h +++ b/src/tagchip.h @@ -43,6 +43,19 @@ constexpr int kSpacing = 4; QSize sizeFor(const QFontMetrics &metrics, const QString &text); +/// The same, with the padding scaled by \p scale. +/// +/// The padding is a fixed pixel count, which is right for one chip size and +/// wrong the moment there are two: at 1.0 it is 18px around roughly 30px of +/// text on a sibling chip, so the chip stays wide while its text shrinks and +/// the tier reads as "same size, smaller letters" rather than as a smaller +/// chip. Scaling it with the font is what makes the second tier actually look +/// smaller (item 111). +/// +/// Floored at 2px a side, because the corner radius is half the height and a +/// chip with no horizontal padding has its text touching the curve. +QSize sizeFor(const QFontMetrics &metrics, const QString &text, qreal scale); + /// Paints the chip into `rect`, using `text` and `background`. The text colour /// is derived from the fill so it stays legible. void paint(QPainter *painter, const QRect &rect, const QString &text, diff --git a/src/threadlistmodel.cpp b/src/threadlistmodel.cpp index 3e079ed..6ddd85c 100644 --- a/src/threadlistmodel.cpp +++ b/src/threadlistmodel.cpp @@ -347,6 +347,15 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const case DateFormatRole: return m_dateFormat; case Qt::BackgroundRole: + // Doomed first: a reply tagged deleted or spam is on its way out + // and the user has to see that the moment they act, exactly as a + // thread row does. Without this branch a message-scoped Delete + // repainted a reply identically to an undeleted one, so the + // pending count moved and nothing on screen did. + if (node.isDoomed()) + return QBrush(node.isDeleted() ? deletedColour() + : spamColour()); + // Tinted, so an expanded thread reads as one block rather than as // more table rows. Applied per cell here; ThreadListView fills the // same colour across the strip's band so the row does not end up @@ -354,18 +363,47 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const return replyBackground(); case Qt::FontRole: { // A size down from the thread rows, so a thread reads as the - // heading and its replies as the contents. Never bold: an unread - // reply is still subordinate to the thread it belongs to, and the - // thread row above already carries the unread cue for the whole - // conversation. + // heading and its replies as the contents. The size is what keeps + // a reply subordinate; bold on top of it is the unread cue, at the + // user's request on 2026-08-16. + // + // Replies were unbolded deliberately at first, on the reasoning + // that the thread row above already says the conversation has + // unread mail. That is true of the THREAD and useless for the + // reply: once a thread is expanded, the row telling the user which + // messages in it are unread is the only one that can, and dimming + // alone left the user unable to see a read/unread change at all. QFont font = QGuiApplication::font(); if (font.pointSize() > 0) font.setPointSize(qMax(6, font.pointSize() - 1)); else if (font.pixelSize() > 0) font.setPixelSize(qMax(8, font.pixelSize() - 2)); + + // Bold combines with the dimming rather than replacing it: two + // cues for one state, which is what the thread row has had since + // 2026-08-07 and for the same reason. If the desktop's own font is + // configured Bold, setBold() changes nothing and the dimming is + // the whole cue, which CLAUDE.md records as a real configuration + // on this user's machine. + if (node.isUnread()) + font.setBold(true); + + // Struck through when doomed, for the same reason the thread row + // is: the state then survives a screenshot, a colourblind reader, + // and a theme that overrides the background. A reply had neither + // this nor the fill, so a message-scoped Delete was invisible. + if (node.isDoomed()) + font.setStrikeOut(true); return font; } case Qt::ForegroundRole: + // White over the doomed fill, matching the thread row. The dimmed + // read colour is mixed toward the BACKGROUND, so leaving it here + // would compute a grey against the pane's base and then paint it + // over red. + if (node.isDoomed()) + return QBrush(QColor(Qt::white)); + // Dimmed whether read or not, for the same reason as the font: a // reply is subordinate content. An unread one is left undimmed so // it can still be found. @@ -378,7 +416,26 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const if (index.row() >= m_threads.size()) return {}; - const ThreadSummary &thread = m_threads.at(index.row()).summary; + const ThreadNode &rowNode = m_threads.at(index.row()); + + // A card stands for ONE message since item 108, so it must draw that + // message's tags and not the thread's. `ThreadSummary::tags` is notmuch's + // UNION over the conversation: a four-message thread whose third message + // is signed reads as signed, and the card said so about a message that was + // not (item 110). + // + // Only the tags are substituted. Everything else on the card, the subject, + // the authors, the date and the reply count, describes the THREAD and is + // correct as it stands; only the tags were ever the union that lied. + // + // `first.tags` is populated when the message is loaded, which is when the + // user selects the row. Before that the union is the only answer available + // and is what the card shows, which is why an unopened row can still + // display a sibling's mark. Narrowing that further needs per-message state + // in the query itself. + ThreadSummary thread = rowNode.summary; + if (!rowNode.first.messageId.isEmpty()) + thread.tags = rowNode.first.tags; if (role == ThreadIdRole) return thread.threadId; @@ -424,7 +481,8 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const if (role == MessageOwnColoursRole) return QVariantList(); - if (role == PillTagsRole || role == PillColoursRole) { + if (role == PillTagsRole || role == PillColoursRole + || role == PillOwnCountRole) { // Everything the row already says another way is dropped: the account // is the chip in the subject cell, flagged is the star column, // attachment is the paperclip, unread is the row not being dimmed, and @@ -443,16 +501,43 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const QStringLiteral("unread"), }; - QStringList pills; - for (const QString &tag : thread.tags) { - if (hidden.contains(tag) || isDrawnAsAMark(tag) - || TagColors::isAccountTag(tag)) - continue; - pills.append(tag); + const auto pillsFrom = [&](const QStringList &tags) { + QStringList pills; + for (const QString &tag : tags) { + if (hidden.contains(tag) || isDrawnAsAMark(tag) + || TagColors::isAccountTag(tag)) + continue; + pills.append(tag); + } + // Sorted rather than in notmuch's order, which is not guaranteed + // stable: a row whose pills reordered between repaints would + // flicker. + pills.sort(); + return pills; + }; + + // `thread.tags` is the displayed message's own tags once the row has + // been opened, and the thread's union before that (see the + // substitution above). The union is always the full set, so the + // difference is what belongs only to siblings. + QStringList pills = pillsFrom(thread.tags); + const int ownCount = pills.size(); + + // The sibling tier, appended after the message's own. Shown rather + // than dropped at the user's request: a card sits above a + // conversation, so what the rest of it carries is worth seeing, just + // not at the same weight. The delegate draws these smaller and muted. + // + // Empty until the row has been opened, because before that + // `thread.tags` IS the union and the difference is nothing. That is + // what makes a chip shrink rather than appear. + for (const QString &tag : pillsFrom(rowNode.summary.tags)) { + if (!pills.contains(tag)) + pills.append(tag); } - // Sorted rather than in notmuch's order, which is not guaranteed - // stable: a row whose pills reordered between repaints would flicker. - pills.sort(); + + if (role == PillOwnCountRole) + return ownCount; if (role == PillTagsRole) return pills; @@ -598,6 +683,23 @@ QVariant ThreadListModel::data(const QModelIndex &index, int role) const return {}; } +ThreadListModel::ThreadNode +ThreadListModel::nodeFor(const ThreadSummary &summary) +{ + ThreadNode node{ summary, {}, {}, false }; + + // Only when the query actually supplied them. An empty list here would be + // indistinguishable from "this message carries nothing", which would put + // every chip in the sibling tier and mute the whole card. + if (!summary.firstMessageId.isEmpty() + && !summary.firstMessageTags.isEmpty()) { + node.first.messageId = summary.firstMessageId; + node.first.threadId = summary.threadId; + node.first.tags = summary.firstMessageTags; + } + return node; +} + void ThreadListModel::appendBatch(const QVector<ThreadSummary> &batch) { // beginInsertRows with an empty range violates Qt's contract, so the guard @@ -608,7 +710,7 @@ void ThreadListModel::appendBatch(const QVector<ThreadSummary> &batch) const int first = m_threads.size(); beginInsertRows({}, first, first + batch.size() - 1); for (const ThreadSummary &summary : batch) - m_threads.append(ThreadNode{ summary, {}, {}, false }); + m_threads.append(nodeFor(summary)); endInsertRows(); } @@ -657,7 +759,7 @@ void ThreadListModel::reconcile(const QVector<ThreadSummary> &threads) if (it == present.constEnd()) { const int at = qMin(target, m_threads.size()); beginInsertRows({}, at, at); - m_threads.insert(at, ThreadNode{ summary, {}, {}, false }); + m_threads.insert(at, nodeFor(summary)); endInsertRows(); // Every later row shifted by one, and the map is read again on the @@ -707,8 +809,26 @@ void ThreadListModel::reconcile(const QVector<ThreadSummary> &threads) || m_threads.at(row).summary.authors != summary.authors || m_threads.at(row).summary.date != summary.date || m_threads.at(row).summary.totalCount != summary.totalCount - || m_threads.at(row).summary.matchedCount != summary.matchedCount) { + || m_threads.at(row).summary.matchedCount != summary.matchedCount + // The card's OWN message, which can move while the thread's union + // does not: a root read elsewhere leaves the thread unread as long + // as any reply is. Without this the card kept the tags it was + // first given, and the sibling tier with them. + || m_threads.at(row).summary.firstMessageTags + != summary.firstMessageTags) { m_threads[row].summary = summary; + + // The node too, since the card draws its tags from there. Only the + // tags: the node's children and loaded flag are the expansion + // state this whole method exists to preserve, and `first` carries + // no children. + if (!summary.firstMessageId.isEmpty() + && !summary.firstMessageTags.isEmpty()) { + m_threads[row].first.messageId = summary.firstMessageId; + m_threads[row].first.threadId = summary.threadId; + m_threads[row].first.tags = summary.firstMessageTags; + } + emit dataChanged(index(row, 0), index(row, 0)); } } @@ -804,6 +924,108 @@ QString ThreadListModel::threadIdForMessage(const QString &messageId) const return {}; } +void ThreadListModel::setRootMessageTags(const QString &messageId, + const QStringList &tags) +{ + if (messageId.isEmpty()) + return; + + for (int row = 0; row < m_threads.size(); ++row) { + ThreadNode &node = m_threads[row]; + if (node.summary.firstMessageId != messageId + && node.first.messageId != messageId) { + continue; + } + + if (node.first.tags == tags && !node.first.messageId.isEmpty()) + return; // Nothing changed; do not churn the view. + + // Enough of a node for the card to draw from. The rest of the display + // still comes from the summary, which is correct for it: the subject, + // the authors and the date describe the thread, and only the TAGS were + // ever the union that lied about this message. + node.first.messageId = messageId; + node.first.threadId = node.summary.threadId; + node.first.tags = tags; + + const QModelIndex threadIndex = index(row, 0, QModelIndex()); + emit dataChanged(threadIndex, threadIndex); + return; + } +} + +MessageNode ThreadListModel::messageById(const QString &messageId) const +{ + if (messageId.isEmpty()) + return {}; + + for (const ThreadNode &node : m_threads) { + // The root's own message first, and it is not among the children: + // setThreadMessages drops depth 0 because the root row stands for it. + // Searching only the children returned a default-constructed node for + // every root message, and a caller that trusted it set the message + // pane's tag strip to that empty tag list, wiping a strip that had + // been correct. + if (!node.first.messageId.isEmpty() + && node.first.messageId == messageId) { + return node.first; + } + + // Before expansion there is no node, so the answer is assembled from + // the summary: for a thread of one, its tags ARE this message's, since + // a thread's tags are a union over its messages. For a longer thread + // they are a union over messages this one is only part of, which is + // wider than the truth but is also exactly what the card shows, so a + // caller repainting from it stays consistent with the row beside it. + if (node.summary.firstMessageId == messageId) { + MessageNode root; + root.messageId = node.summary.firstMessageId; + root.threadId = node.summary.threadId; + root.subject = node.summary.subject; + root.date = node.summary.date; + root.tags = node.summary.tags; + return root; + } + + for (const MessageNode &child : node.children) { + if (child.messageId == messageId) + return child; + } + } + return {}; +} + +ActionScope ThreadListModel::messageScopeFor( + const QModelIndexList &selection) const +{ + ActionScope scope; + + for (const QModelIndex &index : selection) { + QString messageId; + if (isMessageRow(index)) { + messageId = messageAt(index).messageId; + } else { + if (index.row() < 0 || index.row() >= m_threads.size()) + continue; + // The message the CARD displays, which the query already named. + // Not the loaded children: a thread the user never expanded still + // shows its first message, and this must work without one. + messageId = m_threads.at(index.row()).summary.firstMessageId; + } + + // Skipped rather than widened. Falling back to the thread here would + // silently act on messages the row does not display, which is the + // behaviour item 108 removed. + if (messageId.isEmpty() || scope.messageIds.contains(messageId)) + continue; + + scope.messageIds.append(messageId); + scope.messageCount += 1; + } + + return scope; +} + ActionScope ThreadListModel::scopeFor(const QModelIndexList &selection) const { ActionScope scope; @@ -847,6 +1069,19 @@ ThreadSummary ThreadListModel::threadAt(int row) const return m_threads.at(row).summary; } +ThreadSummary ThreadListModel::threadFor(const QModelIndex &index) const +{ + if (!index.isValid()) + return {}; + + // The parent's row for a message, its own for a thread. Both are top-level + // numbers by the time threadAt() sees them, which is the whole point: the + // conversion happens once, here, instead of at every call site that has to + // remember which kind of row it is holding. + const QModelIndex threadIndex = isMessageRow(index) ? index.parent() : index; + return threadAt(threadIndex.row()); +} + QStringList ThreadListModel::accountKeysForThread(const QString &threadId) const { QStringList keys; @@ -884,7 +1119,117 @@ void ThreadListModel::applyTagChange(const QString &threadId, // The whole card repaints: unread state drives its font, and the tags // it draws on line 3 have just changed. - emit dataChanged(index(row, 0), index(row, 0)); + const QModelIndex threadIndex = index(row, 0); + emit dataChanged(threadIndex, threadIndex); + + // And every LOADED reply, because a thread-scoped write reaches every + // message in the thread. Updating only the summary left an expanded + // thread showing replies that still carried the old tags: marking a + // thread read repainted the card and left its replies bold and + // undimmed, describing a state the database no longer held. They + // corrected themselves on the next query, which is what made it look + // like a repaint bug rather than a stale model. + // + // Only the loaded ones exist to update. An unexpanded thread has no + // child rows, and the replies it does not hold are the database's + // business, not this model's. + QVector<MessageNode> &children = m_threads[row].children; + if (children.isEmpty()) + return; + + for (MessageNode &child : children) { + for (const QString &tag : removed) + child.tags.removeAll(tag); + for (const QString &tag : added) { + if (!child.tags.contains(tag)) + child.tags.append(tag); + } + } + + // One span for the whole expansion rather than a signal per reply: the + // rows are contiguous under this parent and a view coalesces them + // anyway. + emit dataChanged(index(0, 0, threadIndex), + index(children.size() - 1, 0, threadIndex)); + return; + } +} + +void ThreadListModel::applyMessageTagChange(const QString &messageId, + const QStringList &added, + const QStringList &removed) +{ + if (messageId.isEmpty()) return; + + const auto retag = [&](QStringList &tags) { + for (const QString &tag : removed) + tags.removeAll(tag); + for (const QString &tag : added) { + if (!tags.contains(tag)) + tags.append(tag); + } + }; + + for (int row = 0; row < m_threads.size(); ++row) { + ThreadNode &node = m_threads[row]; + const QModelIndex threadIndex = index(row, 0, QModelIndex()); + + // The ROOT card's own message, which is not among the children: + // setThreadMessages drops depth 0 because the root row stands for it. + // Searching only the children meant a write to the message a root card + // displays found nothing and repainted nothing, and item 108 made that + // the ordinary gesture rather than an edge case. + // + // Matched on the summary's id as well as the loaded node's, because the + // node is empty until the thread has been expanded and the user acts on + // unexpanded threads constantly. + const bool isRoot = + node.summary.firstMessageId == messageId + || (!node.first.messageId.isEmpty() + && node.first.messageId == messageId); + if (isRoot) { + // The root's own node, which is what the card draws its tags from + // once the message has been loaded. Seeded from the summary when + // the message has never been loaded, so an edit made before the + // row was ever opened still has somewhere to land; the summary is + // the union, which is the widest honest starting point. + if (node.first.messageId.isEmpty()) { + node.first.messageId = node.summary.firstMessageId; + node.first.threadId = node.summary.threadId; + node.first.tags = node.summary.tags; + } + retag(node.first.tags); + + // The SUMMARY only for a single-message thread. A thread's tags are + // a UNION over its messages: for a thread of one that union IS this + // message, so keeping the two in step is exact; for a longer + // thread, deleting one message does not delete the conversation, + // and the summary must keep describing the conversation because + // that is what the thread-scoped actions and the query read. + // + // The CARD does not depend on this either way: since item 110 it + // draws its tags from first.tags, which was just updated. This + // keeps the summary honest for everything else that reads it. + if (node.summary.totalCount <= 1) + retag(node.summary.tags); + + emit dataChanged(threadIndex, threadIndex); + return; + } + + QVector<MessageNode> &children = node.children; + for (int child = 0; child < children.size(); ++child) { + if (children.at(child).messageId != messageId) + continue; + + retag(children[child].tags); + + // The reply's own row, and only that row. Its chips, its marks, + // its dimming and its doomed fill all read the node's tags. + const QModelIndex replyIndex = index(child, 0, threadIndex); + emit dataChanged(replyIndex, replyIndex); + return; + } } } diff --git a/src/threadlistmodel.h b/src/threadlistmodel.h index 2b8d2b0..717537c 100644 --- a/src/threadlistmodel.h +++ b/src/threadlistmodel.h @@ -65,6 +65,21 @@ public: /// config itself would be a second source of truth. PillColoursRole, + /// How many of PillTagsRole's entries belong to the message the card + /// DISPLAYS, the rest belonging only to its siblings. + /// + /// The card stands for one message but sits above a conversation, so + /// it shows both: the message's own tags first at full size, then the + /// thread's other tags smaller and muted. Without the split a card + /// either claimed a sibling's tag as its own (item 110) or dropped it + /// and looked like it had lost information. + /// + /// Equals the whole list until the row has been opened, since the + /// per-message tags arrive with the message load and before that the + /// union is the only answer there is. Chips therefore SHRINK when the + /// split becomes known; none ever disappears. + PillOwnCountRole, + /// True when the row is a MESSAGE row rather than a thread root. /// Drives both the action scope and whether the view paints a tag /// strip under the row. @@ -225,8 +240,29 @@ public: /// contradict the sort the user selected. void reconcile(const QVector<ThreadSummary> &threads); + /// The thread at a TOP-LEVEL row. + /// + /// **Wrong for any index that might be a reply**, and that is item 88. A + /// tree numbers rows per parent, so a reply's row() indexes its siblings: + /// threadAt(0) on the first reply of any thread returns the FIRST THREAD IN + /// THE LIST, and the caller acts on unrelated mail while every id it + /// compares looks right. Safe only for a row number that came from a loop + /// over rowCount(), never from an index the user selected. + /// + /// Prefer threadFor(index), which cannot be handed the wrong number. ThreadSummary threadAt(int row) const; + /// The thread an index belongs to, whichever kind of row it is. + /// + /// A thread row resolves to itself; a message row resolves through its + /// PARENT rather than through its own row number. This is the accessor + /// every caller holding a QModelIndex wants, and it exists because the + /// row-taking one above silently answers about unrelated mail for a reply. + /// + /// An invalid or unknown index gives a default-constructed summary, whose + /// empty threadId every caller here already treats as "nothing to do". + ThreadSummary threadFor(const QModelIndex &index) const; + /// Fills in a thread's message rows once the worker has walked its tree. /// /// The depth-0 message is dropped: it is the thread's first message and the @@ -249,14 +285,58 @@ public: /// message the user could select is always findable here. QString threadIdForMessage(const QString &messageId) const; - /// Resolves a selection into what an action should touch. + /// Records the tags a MESSAGE really carries, as the worker reported them. + /// + /// Exists because `ThreadSummary::tags` is notmuch's UNION over the + /// thread, which is right for a card standing for a conversation and wrong + /// for one standing for a message. A four-message thread whose third + /// message is signed makes the whole thread read as signed, so the root + /// card and the message pane both claimed a tag the displayed message did + /// not have. + /// + /// Only the ROOT needs this: reply rows already carry their own nodes from + /// setThreadMessages. Calling it for anything else is a no-op. + /// + /// The thread's summary is deliberately NOT rewritten. It describes the + /// conversation, and three unread siblings do not stop being unread + /// because this message was read. + void setRootMessageTags(const QString &messageId, const QStringList &tags); + + /// A loaded message row's node, found by id rather than by position. /// - /// Mixed selections are honoured as given: a thread root and an unrelated - /// reply act on that whole thread and that one message. Nothing is - /// escalated or narrowed silently, which is the point of the scope being - /// visible in the first place. + /// For callers that know WHICH message they mean and must not depend on it + /// being the row the user has selected. Default-constructed when no + /// expanded thread holds it. + MessageNode messageById(const QString &messageId) const; + + /// Resolves a selection into whole THREADS, for the thread-scoped actions. + /// + /// A thread row contributes its thread; a message row still contributes + /// only itself, since a reply's own row cannot be widened into its + /// conversation without escalating silently. Mixed selections are honoured + /// as given: a thread root and an unrelated reply act on that whole thread + /// and that one message. + /// + /// **Not the default any more.** Since item 108 the ordinary actions use + /// messageScopeFor(); this is what the explicit "whole thread" submenu + /// resolves through. ActionScope scopeFor(const QModelIndexList &selection) const; + /// Resolves a selection into individual MESSAGES, which is what the + /// ordinary tag actions act on since item 108. + /// + /// A thread row contributes the ONE message its card displays, not its + /// whole conversation. That is `ThreadSummary::firstMessageId`, carried + /// from the query, so this needs no expansion and no worker round trip. + /// In the Sent view that field is the first MATCHED message rather than + /// the thread's opening one, which is right here for the same reason it is + /// right on the card: both answer "the message this row shows". + /// + /// A thread row whose `firstMessageId` is empty contributes nothing. That + /// is a row the model cannot name a message for, and acting on the whole + /// thread instead would be the silent escalation this exists to remove. + ActionScope messageScopeFor(const QModelIndexList &selection) const; + /// The account keys behind a thread's account tags, for item 49's /// per-account sync. /// @@ -273,6 +353,21 @@ public: void applyTagChange(const QString &threadId, const QStringList &added, const QStringList &removed); + /// The same, for a change scoped to ONE message. + /// + /// Repaints that message's own row and leaves the thread alone. The thread + /// row deliberately does not follow: it stands for the whole conversation, + /// so redrawing it for a one-message edit would claim every message in it + /// had changed. That reasoning is why no optimistic update existed here at + /// all, which left Delete and Toggle unread on a reply moving the pending + /// count and changing nothing on screen. + /// + /// A message id that no expanded thread holds is a no-op: only expanded + /// threads have message rows, so there is nothing to repaint. + void applyMessageTagChange(const QString &messageId, + const QStringList &added, + const QStringList &removed); + private: /// One thread root and the message rows expanded under it. /// @@ -301,6 +396,21 @@ private: bool loaded = false; }; + /// A newly arrived thread, with its card's own message seeded from the + /// query. + /// + /// `ThreadSummary::tags` is notmuch's union over the conversation, and the + /// card stands for ONE message. The worker reads that message's own tags + /// in the same walk that finds its id, so the two tiers are known from the + /// first paint (item 111). Deriving them from the message LOAD instead + /// left every unopened row drawing one tier and correcting itself when the + /// user selected it, which is most of the list. + /// + /// `first` is left empty when the query carried no per-message tags, so + /// anything that supplies only a summary keeps the old behaviour rather + /// than claiming the union as one message's. + static ThreadNode nodeFor(const ThreadSummary &summary); + QVector<ThreadNode> m_threads; const TagColors *m_tagColors = nullptr; QString m_dateFormat; diff --git a/src/types.h b/src/types.h index f4eaeba..409ce79 100644 --- a/src/types.h +++ b/src/types.h @@ -48,6 +48,20 @@ struct ThreadSummary /// with recipients; the two have nothing in common but their position here. QString firstMessageId; + /// The tags of that ONE message, as opposed to `tags` above, which is + /// notmuch's union over the whole thread. + /// + /// A card stands for one message but sits above a conversation, and shows + /// both: its own tags at full size, the thread's others smaller (item + /// 111). Without this the split is unknown until the row is opened and the + /// message loads, so every chip renders as the card's own and then shrinks + /// on selection, which is what the user reported. + /// + /// Free, for the same reason `firstMessageId` is: the walk that finds that + /// message is already happening and this reads the INDEX, not the message + /// file. Do not move it behind a flag by analogy with `recipients`. + QStringList firstMessageTags; + /// Who the thread's messages were sent TO, summarised for one line. /// /// Empty unless the query asked for it, and that is a performance @@ -140,6 +154,15 @@ struct MessageNode { return tags.contains(QStringLiteral("attachment")); } + + bool isDeleted() const { return tags.contains(QStringLiteral("deleted")); } + bool isSpam() const { return tags.contains(QStringLiteral("spam")); } + + /// True while the message is tagged for removal, exactly as the thread + /// predicate of the same name. A reply carries its own fate: a + /// message-scoped Delete tags one message, and the reply's row is the only + /// place the user can see that happen. + bool isDoomed() const { return isDeleted() || isSpam(); } }; /// What an action is about to touch, resolved from the selection. |