diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-18 12:35:05 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-18 12:41:45 +0200 |
| commit | 601159309118cf65c73f5f50bb3cf216be9f1cbb (patch) | |
| tree | 9cbf374538002193dcd373a46f7825a219617efb /src/mainwindow.cpp | |
| parent | 4583de009571aaa674e7d161d31ec640860787e1 (diff) | |
| download | qtmaildir-601159309118cf65c73f5f50bb3cf216be9f1cbb.tar.gz qtmaildir-601159309118cf65c73f5f50bb3cf216be9f1cbb.zip | |
feat(trash): restore mail from the trash view
Task 6. Delete moved mail into the trash and the only ways back out were a
second press of Delete or Ctrl+Z, both of which act on a row the user has
to have deleted in this session. Browsing the trash and putting something
back needed an action of its own.
`restore` is enabled from the QUERY, not from the selection's tags. The
trash view is path-based precisely so that mail trashed by another client
appears in it, and such a message carries no tag of ours: deciding from
`tag:deleted` would disable Restore on exactly the messages that most need
it. isShowingTrash() compares the current query against the trash
generator's own, for both the per-account and the all-accounts scope, so it
follows the account dropdown like every other filter.
A message with NO origin tag is the foreign-trashed case, and it is why
this is not simply restoreSelected() under a new name. The two callers want
opposite things from a missing origin, which `fallbackToInbox` selects.
From the trash view the message is demonstrably in the trash and refusing
to move it leaves the user looking at mail they cannot get out, so it goes
to the inbox and the status bar says so. From a second press of Delete the
message is not in the trash at all and merely wears a stale `deleted` tag
from an older version or a hand-written notmuch command; moving that to the
inbox would relocate mail the user never asked to move, so the tag comes
off and the file stays put.
The inbox FOLDER is a new optional per-account `inbox` key, defaulting to
"Inbox". It is configurable rather than hardcoded because the name is not
ours to assume: naming a folder that does not exist CREATES it, beside the
real one, and under mbsync's `Create Both` that folder reaches the mail
server. That is not hypothetical, it is what a truncated origin folder did
to real mail while this branch was being tested. Unlike `trash` the key is
optional, since the default is right for any ordinary Maildir and a wrong
value here only affects the fallback.
Ctrl+R, which was free. The action is only enabled in the trash view, so
the key is inert elsewhere rather than doing something surprising. It sits
in the Message menu beside Delete and in the thread context menu, greyed
outside the trash rather than hidden: an action that vanishes teaches
nothing, while a disabled entry with its shortcut beside it says both that
it exists and where it applies.
**Adding an action is FIVE places, not four.** knownActions(),
defaultBindings() and the icon table are each enforced by a test that fails
loudly, and being REACHABLE is a fifth that nothing checked: this shipped
registered, bound, iconned, correctly enabled, and present in no menu at
all, which a green suite reported as complete. Ctrl+R is not a shortcut
anyone guesses, so it was effectively invisible.
restoreIsReachableWithoutTheKeyboard() closes that, and deliberately
excludes the context menu from its menu-bar assertion, since findChildren
returns both and one check would otherwise satisfy the other.
Four tests, each mutation-checked. Two worth keeping: the hardcoded "Inbox"
mutation fails against the fixture's lowercase folders exactly as it would
against a Maildir that spells its inbox differently, and the reachability
mutation reproduces the keyboard-only state this shipped in.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Diffstat (limited to 'src/mainwindow.cpp')
| -rw-r--r-- | src/mainwindow.cpp | 135 |
1 files changed, 126 insertions, 9 deletions
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 361c0d4..da7128f 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -849,6 +849,10 @@ void MainWindow::registerActions() else trashSelected(); }); + addAction(QStringLiteral("restore"), tr("&Restore from trash"), + tr("Move the selected messages out of the trash"), [this]() { + restoreSelected(true); + }); addAction(QStringLiteral("spam"), tr("Mark &spam"), tr("Add spam and remove inbox"), [this]() { tagSelected({ QStringLiteral("spam") }, { QStringLiteral("inbox") }, @@ -1136,6 +1140,11 @@ void MainWindow::buildMenus() auto *messageMenu = menuBar()->addMenu(tr("&Message")); messageMenu->addAction(m_actions.value(QStringLiteral("archive"))); messageMenu->addAction(m_actions.value(QStringLiteral("delete"))); + // Beside Delete, whose inverse it is. Greyed outside the trash view + // rather than hidden: an action that vanishes teaches nothing, while a + // disabled entry with its shortcut beside it says both that it exists and + // where it applies. + messageMenu->addAction(m_actions.value(QStringLiteral("restore"))); messageMenu->addAction(m_actions.value(QStringLiteral("spam"))); messageMenu->addSeparator(); messageMenu->addAction(m_actions.value(QStringLiteral("toggle_unread"))); @@ -1193,6 +1202,9 @@ void MainWindow::buildMenus() // control: two buttons with different consequences looked identical. { QStringLiteral("archive"), QStringLiteral("mail-archive") }, { QStringLiteral("delete"), QStringLiteral("edit-delete") }, + // The inverse of delete, and the theme's own name for it: the icon + // every desktop uses for taking something back out of the wastebasket. + { QStringLiteral("restore"), QStringLiteral("edit-undelete") }, { QStringLiteral("undo"), QStringLiteral("edit-undo") }, { QStringLiteral("spam"), QStringLiteral("mail-mark-junk") }, { QStringLiteral("flag"), QStringLiteral("mail-mark-important") }, @@ -1259,6 +1271,7 @@ void MainWindow::buildMenus() m_threadContextMenu->setObjectName(QStringLiteral("threadContextMenu")); m_threadContextMenu->addAction(m_actions.value(QStringLiteral("archive"))); 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->addSeparator(); m_threadContextMenu->addAction(m_actions.value(QStringLiteral("toggle_unread"))); @@ -2459,8 +2472,40 @@ void MainWindow::onQueryFinished(int total, quint64 generation) applyPendingRecovery(); } +bool MainWindow::isShowingTrash() const +{ + // Compared against the trash GENERATOR's query, not against the word + // "trash" or against a tag. The trash view is path-based so that mail + // trashed by another client shows up in it; deciding this from + // `tag:deleted` instead would disable Restore on exactly the messages + // that most need it, which is the case Restore's fallback exists for. + // + // Both scopes, because the view composes with the account dropdown like + // every other filter: one account's trash, or all of them. + const QString query = m_lastQuery.trimmed(); + if (query.isEmpty()) + return false; + + const QString all = m_config.allTrashQuery().trimmed(); + if (!all.isEmpty() && query == all) + return true; + + for (const Account &account : m_config.accounts()) { + const QString trash = account.trashQuery().trimmed(); + if (!trash.isEmpty() && query == trash) + return true; + } + return false; +} + void MainWindow::updateViewWideActions() { + // Only meaningful on mail that is actually in a trash folder. An enabled + // action that does nothing is worse than an absent one, and Restore + // outside the trash has nothing to restore from. + if (QAction *action = m_actions.value(QStringLiteral("restore"))) + action->setEnabled(isShowingTrash()); + // Threads arrive in batches of kBatchSize, so before the query reports its // total the model holds only what has landed. An action that says "all" // must not run against a partial set and silently skip the rest, and a @@ -4458,7 +4503,35 @@ void MainWindow::restoreSelectedThreads() Q_ARG(QString, QStringLiteral("undelete_thread"))); } -void MainWindow::restoreSelected() +QString MainWindow::inboxFolderFor(const Account &account) const +{ + // Discovered from the account's OWN inbox query, never hardcoded. + // + // The casing is not ours to assume: the real Maildir has `Inbox` and a + // test fixture has `inbox`, and picking either would create a SECOND + // folder beside the real one on whichever side disagreed. That is exactly + // the failure a truncated origin folder caused on real mail this morning, + // and under mbsync's `Create Both` such a folder can reach the server. + // + // The inbox query is a generated `path:"<maildir>/<folder>/**"`, so the + // folder name is the part between the account prefix and the glob. + const QString query = account.inboxQuery(); + const QString prefix = + QStringLiteral("path:\"") + account.maildir + QLatin1Char('/'); + const QString suffix = QStringLiteral("/**\""); + if (query.startsWith(prefix) && query.endsWith(suffix)) { + const int from = prefix.length(); + const int length = query.length() - from - suffix.length(); + if (length > 0) + return query.mid(from, length); + } + + // No inbox configured for this account. `Inbox` is the Maildir + // convention and is what mbsync's own `Inbox` directive defaults to. + return QStringLiteral("Inbox"); +} + +void MainWindow::restoreSelected(bool fallbackToInbox) { const QModelIndexList rows = m_threadView->selectionModel()->selectedRows(); @@ -4496,14 +4569,58 @@ void MainWindow::restoreSelected() } if (!unknown.isEmpty()) { - // No origin recorded, which is the case for mail 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: guessing a folder would - // put the message somewhere the user never had it. - sendMessageTagChange(unknown, {}, { QStringLiteral("deleted") }, - tr("Undelete")); - m_undoStack.push(new MessageTagCommand( - this, unknown, {}, { QStringLiteral("deleted") }, tr("Undelete"))); + // No origin recorded. Two quite different situations reach here and + // they want opposite things, which is what `fallbackToInbox` selects. + // + // From the TRASH VIEW the message is demonstrably in the trash, put + // there by another client, and refusing to move it leaves the user + // looking at a message they cannot get out. Inbox is the documented + // fallback, and it is reported, because a guess the user is not told + // about is worse than the guess itself. + // + // From a second press of Delete the message is NOT in the trash: it is + // sitting wherever it always was, wearing a stale `deleted` tag from + // an older version or from a hand-written notmuch command. Moving it + // to the inbox there would relocate mail the user never asked to move. + // The tag comes off and the file stays put. + if (fallbackToInbox) { + QHash<QString, QStringList> byInbox; + QStringList stranded; + for (const QString &messageId : unknown) { + const Account account = + accountForMessagePath(m_model->messageById(messageId).filePath); + if (account.maildir.isEmpty()) { + stranded.append(messageId); + continue; + } + byInbox[account.maildir + QLatin1Char('/') + + inboxFolderFor(account)] + .append(messageId); + } + + for (auto it = byInbox.cbegin(); it != byInbox.cend(); ++it) { + sendMove(it.value(), it.key(), {}, + { QStringLiteral("deleted") }, tr("Restore")); + } + + if (!byInbox.isEmpty()) { + m_statusLabel->setText( + tr("%n message(s) had no record of where they came from " + "and were moved to the inbox.", "", + int(unknown.size() - stranded.size()))); + } + if (!stranded.isEmpty()) { + m_statusLabel->setText( + tr("%n message(s) could not be restored: they belong to no " + "configured account.", "", int(stranded.size()))); + } + } else { + sendMessageTagChange(unknown, {}, { QStringLiteral("deleted") }, + tr("Undelete")); + m_undoStack.push(new MessageTagCommand( + this, unknown, {}, { QStringLiteral("deleted") }, + tr("Undelete"))); + } } for (auto it = byOrigin.cbegin(); it != byOrigin.cend(); ++it) { |
