diff options
| author | Danilo M. <danix@danix.xyz> | 2026-09-14 13:46:33 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-09-14 13:46:33 +0200 |
| commit | 9fdac493c9a6e35f36dc45ff0e13583ddf88605e (patch) | |
| tree | ff561fba18bd543306b3166c8eb3591cb1d220e3 /src | |
| parent | d714483b6027425923340d1bcfa0263b8e7ac0bc (diff) | |
| download | qtmaildir-9fdac493c9a6e35f36dc45ff0e13583ddf88605e.tar.gz qtmaildir-9fdac493c9a6e35f36dc45ff0e13583ddf88605e.zip | |
feat: a Not spam action
Backlog item 201. A message in the Spam view had no way back out: spam is
one-way and Restore is hidden outside the trash. not_spam moves each message
to the folder its moved-from: origin names, falling back to the account's
inbox (reported) for provider-caught mail with no origin.
The action is labelled "Not junk" on the free Alt+J: every letter of "Not
spam" is taken in the Message menu, and Alt+P (Re&ply) and Alt+S (Mark &spam,
frozen) are unavailable.
restoreResolvedMessages() and the undelete_thread branch are parameterised
with the cleared tag and undo description rather than copied, so Delete and
Not spam cannot drift.
Diffstat (limited to 'src')
| -rw-r--r-- | src/keymap.cpp | 5 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 280 | ||||
| -rw-r--r-- | src/mainwindow.h | 48 |
3 files changed, 249 insertions, 84 deletions
diff --git a/src/keymap.cpp b/src/keymap.cpp index d62ba3e..97399e2 100644 --- a/src/keymap.cpp +++ b/src/keymap.cpp @@ -54,6 +54,11 @@ QStringList KeyMap::knownActions() // is menu-reachable rather than broken. QStringLiteral("empty_spam"), QStringLiteral("spam"), + // Item 201. Mark spam's inverse: a message in the spam folder had no + // way back. No default binding, matching empty_spam: moving mail out of + // a folder is reached from a menu, and since item 132 an unbound action + // is menu-reachable rather than broken. + QStringLiteral("not_spam"), QStringLiteral("toggle_unread"), QStringLiteral("mark_all_read"), QStringLiteral("edit_tags"), diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 4b116ba..383e257 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -1809,6 +1809,18 @@ void MainWindow::registerActions() tr("Move the selected messages to the spam folder"), [this]() { spamSelected(); }); + // Item 201. Mark spam's inverse, and Restore's twin: it moves each message + // back to the folder its `moved-from:` tag names, falling back to the + // account's inbox for mail a provider caught, which carries no origin. + // + // The mnemonic is on "&junk" rather than "spam": Alt+P is already Re&ply + // and Alt+S is a frozen collision, and no letter of "Not spam" is free in + // this menu. `junk` is the theme's own name for the folder and what its + // icon (`mail-mark-notjunk`) draws, so the wording stays honest. + addAction(QStringLiteral("not_spam"), tr("Not &junk"), + tr("Move the selected messages out of the spam folder"), [this]() { + notSpamSelected(); + }); // Item 57. The LABEL is "Important"; the action name and the tag are both // still `flag`/`flagged`, deliberately. The name is what a user writes in // the config's [keys] section, and `flagged` is a notmuch tag that neomutt, @@ -2111,6 +2123,11 @@ const QHash<QString, QPair<QString, QString>> kThemeIcons = { // `bug` first, per the user's choice, with the standard junk name behind // it so a theme without the bug still draws a junk icon. { QStringLiteral("spam"), { QStringLiteral("bug"), QStringLiteral("mail-mark-junk") } }, + // Item 201. Mark spam's inverse. `mail-mark-notjunk` is the theme's own + // name for taking a message back out of the junk folder, it ships in Breeze + // and Adwaita, and nothing else in this table uses it, so no + // noTwoActionsShareAnIcon() exception is needed. + { QStringLiteral("not_spam"), { QStringLiteral("mail-mark-notjunk"), QString() } }, // A STAR, which is what the user asked for and what every other mail // client draws for this. NOT mail-mark-important: Breeze draws that // as an exclamation mark, so on the icon-only message bar the action @@ -2218,6 +2235,9 @@ void MainWindow::buildMenus() // that it exists. messageMenu->addAction(m_actions.value(QStringLiteral("purge"))); messageMenu->addAction(m_actions.value(QStringLiteral("spam"))); + // Beside Mark spam, whose inverse it is, and hidden everywhere Mark spam + // cannot be undone from: outside the spam folder it means nothing. + messageMenu->addAction(m_actions.value(QStringLiteral("not_spam"))); messageMenu->addSeparator(); messageMenu->addAction(m_actions.value(QStringLiteral("toggle_unread"))); messageMenu->addAction(m_actions.value(QStringLiteral("mark_all_read"))); @@ -2296,6 +2316,7 @@ void MainWindow::buildMenus() m_threadContextMenu->addAction(m_actions.value(QStringLiteral("delete"))); m_threadContextMenu->addAction(m_actions.value(QStringLiteral("restore"))); m_threadContextMenu->addAction(m_actions.value(QStringLiteral("spam"))); + m_threadContextMenu->addAction(m_actions.value(QStringLiteral("not_spam"))); m_threadContextMenu->addSeparator(); m_threadContextMenu->addAction(m_actions.value(QStringLiteral("toggle_unread"))); m_threadContextMenu->addAction(m_actions.value(QStringLiteral("flag"))); @@ -2443,6 +2464,13 @@ void MainWindow::populateMessageBar() // Restore alone. The two purges are the same act at two scopes and // need no colour to tell them from each other, only from this one. tinted = { m_actions.value(QStringLiteral("restore")) }; + } else if (everySelectedRowIsInASpamFolder() + && !m_threadView->selectionModel()->selectedRows().isEmpty()) { + // Item 201. Mail in a spam folder affords one thing the ordinary bar + // cannot offer: a way back out. Ordered after the trash branch and + // before the draft one, because a draft thrown into the spam folder is + // no more useful to edit than one thrown into the trash. + messageActions = { m_actions.value(QStringLiteral("not_spam")) }; } else if (currentMessageIsADraft()) { messageActions = { m_actions.value(QStringLiteral("edit_draft")) }; } else { @@ -3853,7 +3881,7 @@ void MainWindow::showThreadContextMenu(const QPoint &pos) m_threadContextMenu->popup(m_threadView->viewport()->mapToGlobal(pos)); } -bool MainWindow::everySelectedRowIsInATrashFolder() const +bool MainWindow::everySelectedRowIsInAFolder(QString Account::*folder) const { const QModelIndexList rows = m_threadView->selectionModel()->selectedRows(); @@ -3861,7 +3889,7 @@ bool MainWindow::everySelectedRowIsInATrashFolder() const return false; for (const QModelIndex &index : rows) { - // A MESSAGE row is its one file. A CONVERSATION is in the trash only + // A MESSAGE row is its one file. A CONVERSATION is in the folder only // when ALL of its messages are (item 178), so it answers on every path // the digest reported rather than on the one the summary carries. // @@ -3894,14 +3922,15 @@ bool MainWindow::everySelectedRowIsInATrashFolder() const return false; const Account account = accountForMessagePath(path); - if (account.maildir.isEmpty() || account.trash.isEmpty()) + const QString named = account.*folder; + if (account.maildir.isEmpty() || named.isEmpty()) return false; // Compared as a path segment, never with startsWith(): `trash-old` // starts with `trash` and is a different folder. The same trap the // attachment-save check records. const QString prefix = account.maildir + QLatin1Char('/') - + account.trash + QLatin1Char('/'); + + named + QLatin1Char('/'); // accountForMessagePath() accepts both shapes, so this must too: a // thread row's path is database-relative and a reply row's // absolute. @@ -3912,9 +3941,20 @@ bool MainWindow::everySelectedRowIsInATrashFolder() const return true; } +bool MainWindow::everySelectedRowIsInATrashFolder() const +{ + return everySelectedRowIsInAFolder(&Account::trash); +} + +bool MainWindow::everySelectedRowIsInASpamFolder() const +{ + return everySelectedRowIsInAFolder(&Account::spam); +} + void MainWindow::refreshTrashActions() { const bool inTrash = everySelectedRowIsInATrashFolder(); + const bool inSpam = everySelectedRowIsInASpamFolder(); const bool haveSelection = !m_threadView->selectionModel()->selectedRows().isEmpty(); @@ -3940,6 +3980,16 @@ void MainWindow::refreshTrashActions() && !m_replySelectionHidesDelete); } + // Item 201. Not spam is Restore's twin, but scoped to the SPAM folder + // rather than the trash: shown when the selection is in a spam folder, and + // hidden on a reply row by the same flag Delete reads. Hidden rather than + // disabled, like Mark spam beside it: offering it where it does not apply + // teaches nothing. + if (auto *notSpam = m_actions.value(QStringLiteral("not_spam"))) { + notSpam->setVisible(haveSelection && inSpam + && !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"))) { @@ -6710,77 +6760,36 @@ void MainWindow::onThreadMessagesResolved(const QStringList &messageIds, } if (requestTag == QStringLiteral("restore_messages")) { - restoreResolvedMessages(messageIds, paths, tags); + restoreResolvedMessages(messageIds, paths, tags, + QStringLiteral("deleted"), tr("Restore"), + true, false); return; } - if (requestTag != QStringLiteral("undelete_thread")) + if (requestTag == QStringLiteral("not_spam_messages")) { + notSpamMessages(messageIds, paths, tags); return; - - // Restore, resolved per message: each one goes back to the folder its own - // `moved-from:` tag names, so a thread whose messages were deleted from - // different folders reassembles correctly rather than collapsing into one. - const QString prefix = QString::fromLatin1(kOriginTagPrefix); - QHash<QString, QStringList> byOrigin; - QStringList unknown; - for (int i = 0; i < messageIds.size(); ++i) { - // Split on TAB, matching resolveThreadMessages(). A space is not a - // safe separator: a folder name containing one produces a tag - // containing one, and splitting there silently truncates the origin - // to its first word. - const QStringList messageTags = - tags.at(i).split(QLatin1Char('\t'), Qt::SkipEmptyParts); - QString origin; - for (const QString &tag : messageTags) { - if (tag.startsWith(prefix)) { - origin = tag.mid(prefix.length()); - break; - } - } - // A message with no `deleted` tag is not in the trash and has nothing - // to come back from. A thread-scoped restore reaches every message, - // including ones the user never deleted, and moving those would drag - // untouched mail out of whatever folder it legitimately sits in. - if (!messageTags.contains(QStringLiteral("deleted"))) - continue; - const Account account = - accountForMessagePath(paths.at(i)); - if (origin.isEmpty() || account.maildir.isEmpty()) { - unknown.append(messageIds.at(i)); - continue; - } - byOrigin[account.maildir + QLatin1Char('/') + origin] - .append(messageIds.at(i)); } - if (!unknown.isEmpty()) { - // No origin recorded: deleted by an older version or tagged by hand. - // The tag comes off so the row stops claiming to be deleted, but no - // file moves, since guessing a folder would put the message somewhere - // the user never had it. - auto *command = new MessageTagCommand(this, unknown, {}, - { QStringLiteral("deleted") }, - tr("Undelete thread")); - awaitTagConfirmation(command); - sendMessageTagChange(unknown, {}, { QStringLiteral("deleted") }, - tr("Undelete thread")); - m_undoStack.push(command); + if (requestTag == QStringLiteral("not_spam_thread")) { + restoreResolvedMessages(messageIds, paths, tags, + QStringLiteral("spam"), tr("Not junk"), + true, false, threadScope); + return; } - for (auto it = byOrigin.cbegin(); it != byOrigin.cend(); ++it) { - // The origin tag is named here, not left as the placeholder: on a - // restore the placeholder would resolve to the folder the message is - // coming FROM, which is the trash, and strip a tag never written. - const QString origin = originTagFor(it.key()); - QStringList remove{ QStringLiteral("deleted") }; - if (!origin.isEmpty()) - remove.append(origin); - sendMove(it.value(), it.key(), {}, remove, tr("Undelete thread"), - false, threadScope); - } + if (requestTag != QStringLiteral("undelete_thread")) + return; - showTransientStatus(tr("%1: %n message(s)", "", messageIds.size()) - .arg(tr("Undelete thread"))); + // Restore, resolved per message: each one goes back to the folder its own + // `moved-from:` tag names, so a thread whose messages were deleted from + // different folders reassembles correctly rather than collapsing into one. + // + // The same walk serves Not spam and both message-scoped callers; the flags + // say what a message with no origin gets, so the four paths cannot drift. + restoreResolvedMessages(messageIds, paths, tags, + QStringLiteral("deleted"), tr("Undelete thread"), + false, true, threadScope); } void MainWindow::untrashThreads(const QStringList &threadIds) @@ -6799,6 +6808,62 @@ void MainWindow::untrashThreads(const QStringList &threadIds) Q_ARG(QString, QStringLiteral("undelete_thread"))); } +void MainWindow::notSpamSelected() +{ + const QModelIndexList rows = + m_threadView->selectionModel()->selectedRows(); + if (rows.isEmpty()) + return; + + // Mark spam's inverse, resolved the same per-row way (item 177): a + // conversation row moves its whole conversation, a thread of one moves its + // message. Both halves run, because a selection really can hold one of + // each; they travel different routes for the reason spamSelected() + // records. + const ActionScope scope = m_model->scopeForSelection(rows); + + if (!scope.threadIds.isEmpty()) + notSpamThreads(scope.threadIds); + + if (scope.messageIds.isEmpty()) + return; + + // Resolved by the WORKER, not read from the model. A row whose move into + // the spam folder has not been re-queried still carries its pre-move tags, + // so reading the origin off the model would find none and send the message + // to the inbox instead of the folder it came from. The same reasoning + // restoreSelectedFromTrash() records. + QMetaObject::invokeMethod( + m_worker, "resolveMessages", Qt::QueuedConnection, + Q_ARG(QStringList, scope.messageIds), + Q_ARG(QString, QStringLiteral("not_spam_messages"))); +} + +void MainWindow::notSpamMessages(const QStringList &messageIds, + const QStringList &paths, + const QStringList &tags) +{ + restoreResolvedMessages(messageIds, paths, tags, QStringLiteral("spam"), + tr("Not junk"), true, false); +} + +void MainWindow::notSpamThreads(const QStringList &threadIds) +{ + if (threadIds.isEmpty()) + return; + + // Repainted synchronously, as the mark-spam direction is: the card is what + // the user watches, and it must not wait for the worker round trip. + for (const QString &threadId : threadIds) + m_model->applyTagChange(threadId, {}, { QStringLiteral("spam") }); + + m_pendingThreadScope = threadIds; + QMetaObject::invokeMethod( + m_worker, "resolveThreadMessages", Qt::QueuedConnection, + Q_ARG(QStringList, threadIds), + Q_ARG(QString, QStringLiteral("not_spam_thread"))); +} + QString MainWindow::inboxFolderFor(const Account &account) const { // Discovered from the account's OWN inbox query, never hardcoded. @@ -6829,7 +6894,12 @@ QString MainWindow::inboxFolderFor(const Account &account) const void MainWindow::restoreResolvedMessages(const QStringList &messageIds, const QStringList &paths, - const QStringList &tags) + const QStringList &tags, + const QString &clearedTag, + const QString &description, + bool moveUnoriginToInbox, + bool skipUnmarked, + const QStringList &wholeThreadIds) { if (messageIds.size() != paths.size() || messageIds.size() != tags.size()) return; @@ -6837,11 +6907,24 @@ void MainWindow::restoreResolvedMessages(const QStringList &messageIds, const QString prefix = QString::fromLatin1(kOriginTagPrefix); QHash<QString, QStringList> byOrigin; QHash<QString, QStringList> byInbox; + QStringList unmarked; QStringList stranded; for (int i = 0; i < messageIds.size(); ++i) { + // Split on TAB, matching resolveThreadMessages(). A space is not a + // safe separator: a folder name containing one produces a tag + // containing one, and splitting there silently truncates the origin + // to its first word. const QStringList messageTags = tags.at(i).split(QLatin1Char('\t'), Qt::SkipEmptyParts); + + // A thread-scoped restore reaches every message, including ones that + // never moved, so it skips the ones that do not carry the tag this + // move clears. The message-scoped callers selected mail that is + // demonstrably IN the folder, so they ask nothing. + if (skipUnmarked && !messageTags.contains(clearedTag)) + continue; + QString origin; for (const QString &tag : messageTags) { if (tag.startsWith(prefix)) { @@ -6851,30 +6934,53 @@ void MainWindow::restoreResolvedMessages(const QStringList &messageIds, } const Account account = accountForMessagePath(paths.at(i)); - if (account.maildir.isEmpty()) { - stranded.append(messageIds.at(i)); - continue; - } - if (origin.isEmpty()) { - // Trashed by another client, so there is no record of where it - // belongs. Inbox is the documented fallback, and it is reported: - // a guess the user is not told about is worse than the guess. - byInbox[account.maildir + QLatin1Char('/') - + account.inboxFolder()] - .append(messageIds.at(i)); - continue; + if (!moveUnoriginToInbox) { + // The thread-scoped restore out of the TRASH is also reached from + // Delete's toggle on stranded mail that sits outside the trash, so + // a message with no origin (or no account) only has its tag taken + // off: guessing a folder would put it somewhere the user never had + // it. + if (origin.isEmpty() || account.maildir.isEmpty()) { + unmarked.append(messageIds.at(i)); + continue; + } + } else { + if (account.maildir.isEmpty()) { + stranded.append(messageIds.at(i)); + continue; + } + if (origin.isEmpty()) { + // In the folder with no record of where it came from, usually + // because a provider put it there. Inbox is the documented + // fallback, and it is reported: a guess the user is not told + // about is worse than the guess. + byInbox[account.maildir + QLatin1Char('/') + + account.inboxFolder()] + .append(messageIds.at(i)); + continue; + } } byOrigin[account.maildir + QLatin1Char('/') + origin] .append(messageIds.at(i)); } + if (!unmarked.isEmpty()) { + // The tag comes off so the row stops claiming to be there, but no file + // moves. The undo is the same tag write, pushed like any other. + auto *command = new MessageTagCommand(this, unmarked, {}, + { clearedTag }, description); + awaitTagConfirmation(command); + sendMessageTagChange(unmarked, {}, { clearedTag }, description); + m_undoStack.push(command); + } + for (auto it = byOrigin.cbegin(); it != byOrigin.cend(); ++it) { // The origin tag is named here rather than left as the placeholder, // which onMessagesMoved() would resolve to the folder the message is - // coming FROM, namely the trash. + // coming FROM, namely the folder it is leaving. const QString origin = originTagFor(it.key()); - QStringList remove{ QStringLiteral("deleted") }; + QStringList remove{ clearedTag }; if (!origin.isEmpty()) remove.append(origin); @@ -6913,7 +7019,8 @@ void MainWindow::restoreResolvedMessages(const QStringList &messageIds, break; } - sendMove(it.value(), it.key(), add, remove, tr("Restore")); + sendMove(it.value(), it.key(), add, remove, description, false, + wholeThreadIds); } for (auto it = byInbox.cbegin(); it != byInbox.cend(); ++it) { @@ -6921,9 +7028,16 @@ void MainWindow::restoreResolvedMessages(const QStringList &messageIds, // message with no origin tag, and the folder it names is the // account's own inbox. So the tag always comes with it. sendMove(it.value(), it.key(), { QStringLiteral("inbox") }, - { QStringLiteral("deleted") }, tr("Restore")); + { clearedTag }, description, false, wholeThreadIds); } + // A thread-scoped move is announced as a count, matching the delete and + // mark-spam directions. The message-scoped callers leave the bar's own + // status alone. + if (!wholeThreadIds.isEmpty()) { + showTransientStatus(tr("%1: %n message(s)", "", messageIds.size()) + .arg(description)); + } if (!byInbox.isEmpty()) { m_statusLabel->setText( tr("%n message(s) had no record of where they came from and were " diff --git a/src/mainwindow.h b/src/mainwindow.h index 478de2d..c3395c9 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -1130,6 +1130,17 @@ private: /// Delete on exactly the mail a trash view is full of. bool everySelectedRowIsInATrashFolder() const; + /// Whether every selected row's file already sits in its account's spam + /// folder. Empty selection answers false. The spam twin of + /// everySelectedRowIsInATrashFolder(), and asked of the PATH for the same + /// reason: provider-caught mail in the spam folder carries no `spam` tag. + bool everySelectedRowIsInASpamFolder() const; + + /// The one walk both predicates above share: \p folder names the Account + /// member (`&Account::trash` or `&Account::spam`) whose folder is asked + /// about. + bool everySelectedRowIsInAFolder(QString Account::*folder) const; + /// Opens the tag dialog on the current selection and applies its result. /// /// The only route to an arbitrary tag: every other tag action writes a @@ -1276,6 +1287,22 @@ private: /// this asks the worker and finishes in onThreadMessagesResolved(). void spamThreads(const QStringList &threadIds); + /// Moves each selected row's message back OUT of the spam folder, to the + /// folder its `moved-from:` tag names. Mark spam's inverse and Restore's + /// twin, offered only on a selection that is in a spam folder. + void notSpamSelected(); + + /// The half of notSpamSelected() that does the work, given the tags and + /// paths the WORKER reported. Forwards to restoreResolvedMessages() with + /// `spam` as the cleared tag, so the two restore directions cannot drift. + void notSpamMessages(const QStringList &messageIds, + const QStringList &paths, + const QStringList &tags); + + /// Moves every message of the named THREADS out of their accounts' spam + /// folders. Asynchronous like spamThreads(), and for the same reason. + void notSpamThreads(const QStringList &threadIds); + /// Runs the thread-scoped delete once the worker has resolved the /// threads to messages. void onThreadMessagesResolved(const QStringList &messageIds, @@ -1360,9 +1387,28 @@ private: /// Moves each resolved message home, using the tags and paths the WORKER /// reported rather than anything the model holds. + /// + /// One walk serves all four restore routes: Delete out of the trash + /// (message and thread scoped) and Not spam out of the spam folder + /// (message and thread scoped). \p clearedTag is the folder's marker tag + /// (`deleted` or `spam`) stripped by the move; \p description names the + /// action in the status line and on the undo entry. + /// + /// \p moveUnoriginToInbox picks what a message with no `moved-from:` + /// origin gets: the inbox, reported (the message-scoped routes and Not + /// spam, where the mail is demonstrably in the folder and must come out), + /// or a tag-only removal with no file move (the thread-scoped TRASH route, + /// which Delete's toggle also reaches on stranded mail sitting outside the + /// trash). \p skipUnmarked drops messages that do not carry \p clearedTag, + /// which only the thread-scoped trash route needs. void restoreResolvedMessages(const QStringList &messageIds, const QStringList &paths, - const QStringList &tags); + const QStringList &tags, + const QString &clearedTag, + const QString &description, + bool moveUnoriginToInbox, + bool skipUnmarked, + const QStringList &wholeThreadIds = {}); /// The messages a resolveMessages() request was made for. QStringList m_pendingRestoreIds; |
