From d714483b6027425923340d1bcfa0263b8e7ac0bc Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 14 Sep 2026 13:03:35 +0200 Subject: fix: gate spam like delete, and keep one origin in the model --- src/config.cpp | 9 +++++---- src/mainwindow.cpp | 43 ++++++++++++++++++++++++++++++++++++++++++- src/mainwindow.h | 24 ++++++++++++++++++++---- 3 files changed, 67 insertions(+), 9 deletions(-) (limited to 'src') diff --git a/src/config.cpp b/src/config.cpp index 390848a..184c756 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -68,8 +68,8 @@ const QStringList kQueryGenerators = { QStringLiteral("unread"), QStringLiteral("spam") }; /// The tag a generator matches, for the three filters that are a plain tag -/// query. Empty for "sent", "drafts" and "trash", which compose from each -/// account's folder instead and are handled separately. +/// query. Empty for "sent", "drafts", "trash" and "spam", which compose from +/// each account's folder instead and are handled separately. QString generatorTag(const QString &generator) { if (generator == QStringLiteral("unread")) @@ -85,8 +85,9 @@ QString generatorTag(const QString &generator) /// user's own message back into the conversation it answers, and "drafts" is /// worse: a thread row stands for its first matched message, which for a draft /// reply is the message being replied TO, so the draft itself is unreachable. -/// "trash" stays threaded, since a deleted message still belongs to its -/// conversation. Closed set, and the one place the three views are decided. +/// "trash" and "spam" stay threaded, since a deleted or spammed message still +/// belongs to its conversation. Closed set, and the one place the three views +/// are decided. bool generatorIsFlat(const QString &generator) { return generator == QStringLiteral("sent") diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 476ff18..4b116ba 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -3931,6 +3931,15 @@ void MainWindow::refreshTrashActions() && !m_replySelectionHidesDelete); } + // Mark spam follows Delete exactly: one reply cannot be moved out of its + // conversation, and the trash view does not afford it. Same two hides, ORed + // rather than fought over, so it reads the same flag instead of walking the + // selection a second time. + if (auto *spam = m_actions.value(QStringLiteral("spam"))) { + spam->setVisible((!haveSelection || !inTrash) + && !m_replySelectionHidesDelete); + } + // The mirror, which shipped beside it: Restore was added unconditionally // to both menus and so was offered on mail that was never deleted. if (auto *restore = m_actions.value(QStringLiteral("restore"))) { @@ -6466,6 +6475,29 @@ QString MainWindow::originTagFor(const QString &dbRelativeFolder) const return QString(kOriginTagPrefix) + accountRelative; } +QStringList MainWindow::originTagsToStrip(const QStringList &messageIds, + const QStringList &added) const +{ + const bool writingOrigin = + std::any_of(added.cbegin(), added.cend(), [](const QString &tag) { + return tag == kOriginTagPlaceholder() + || tag.startsWith(QLatin1String(kOriginTagPrefix)); + }); + if (!writingOrigin) + return {}; + + QStringList stripped; + for (const QString &messageId : messageIds) { + for (const QString &tag : m_model->messageById(messageId).tags) { + if (tag.startsWith(QLatin1String(kOriginTagPrefix)) + && !added.contains(tag) && !stripped.contains(tag)) { + stripped.append(tag); + } + } + } + return stripped; +} + void MainWindow::trashThreads(const QStringList &threadIds) { if (threadIds.isEmpty()) @@ -7331,6 +7363,11 @@ void MainWindow::sendMove(const QStringList &messageIds, if (tag != kOriginTagPlaceholder()) displayRemove.append(tag); } + // The model's half of the worker's overwrite rule. This move is writing a + // new origin, so every `moved-from:` the model still holds is stale and + // goes with it; otherwise the confirmed update below appends the new tag + // beside the old one and restoreSelected() reads whichever comes first. + displayRemove += originTagsToStrip(messageIds, add); // A thread-scoped move already repainted its rows in // trashThreads() / untrashThreads(), synchronously, before // the worker was asked to resolve the threads at all. Repeating it here @@ -7586,7 +7623,11 @@ void MainWindow::onMessagesMoved(const QMap &originByMessageId }; const QStringList resolvedAdd = resolve(pending.add); - const QStringList resolvedRemove = resolve(pending.remove); + QStringList resolvedRemove = resolve(pending.remove); + // The confirmed half of the same rule, and the one that matters when + // the optimistic update was skipped (a whole-thread move): the new + // origin replaces any other the model still holds. + resolvedRemove += originTagsToStrip(it.value(), resolvedAdd); sendMessageTagChange(it.value(), resolvedAdd, resolvedRemove, pending.description); diff --git a/src/mainwindow.h b/src/mainwindow.h index a7fbf26..478de2d 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -1107,12 +1107,13 @@ private: SelectionKind selectionKind() const; /// Whether the selection holds a reply row, which is what hides Delete, - /// Restore and Archive (item 177). + /// Restore, Archive and Mark spam: all conversation-level acts, and a + /// single reply cannot be removed from its thread (item 177). /// /// Written by refreshScopedActionLabels() and read by - /// refreshTrashActions(), which runs after it and owns the same two - /// actions' visibility. A flag rather than a second walk over the - /// selection, so the two cannot answer differently. + /// refreshTrashActions(), which runs after it and owns the same actions' + /// visibility. A flag rather than a second walk over the selection, so the + /// two cannot answer differently. bool m_replySelectionHidesDelete = false; /// Hides Delete on mail already in the trash, and Restore on mail that @@ -1393,6 +1394,21 @@ private: /// and a restore stripped a tag that had never been written. QString originTagFor(const QString &dbRelativeFolder) const; + /// The `moved-from:` tags these messages currently carry that a new origin + /// must replace, read from the MODEL. + /// + /// Mirrors the worker's overwrite rule (NotmuchWorker::applyTags): a + /// message carries exactly one origin, so writing a new one strips any + /// other. The worker does it against the database; this does it against + /// the optimistic model, where restoreSelected() reads the origin back and + /// a stale second tag would send the message to the wrong folder. + /// + /// Returns nothing unless `added` is writing a new origin, which is either + /// a resolved `moved-from:` tag or the unresolved placeholder the + /// optimistic update drops before painting. + QStringList originTagsToStrip(const QStringList &messageIds, + const QStringList &added) const; + /// The account whose maildir contains `path`, or an invalid account when /// no configured maildir does. /// -- cgit v1.2.3