diff options
Diffstat (limited to 'src')
| -rw-r--r-- | src/CMakeLists.txt | 6 | ||||
| -rw-r--r-- | src/composecontext.cpp | 23 | ||||
| -rw-r--r-- | src/keymap.cpp | 12 | ||||
| -rw-r--r-- | src/maildirname.cpp | 61 | ||||
| -rw-r--r-- | src/maildirname.h | 24 | ||||
| -rw-r--r-- | src/main.cpp | 4 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 313 | ||||
| -rw-r--r-- | src/mainwindow.h | 47 | ||||
| -rw-r--r-- | src/messageview.cpp | 2 | ||||
| -rw-r--r-- | src/nmraii.h | 2 | ||||
| -rw-r--r-- | src/notmuchworker.cpp | 205 | ||||
| -rw-r--r-- | src/notmuchworker.h | 26 | ||||
| -rw-r--r-- | src/version.h.in | 23 |
13 files changed, 717 insertions, 31 deletions
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 700b185..4d9dbaa 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -44,6 +44,12 @@ target_include_directories(qtmaildir_lib PUBLIC ${CMAKE_CURRENT_SOURCE_DIR} ${NOTMUCH_INCLUDE_DIR} ${CMAKE_BINARY_DIR}/generated/qtmaildir) +# The counter target rewrites buildnumber.h, which version.h includes, so the +# library must not start compiling before it has run. +if(TARGET qtmaildir_buildnumber) + add_dependencies(qtmaildir_lib qtmaildir_buildnumber) +endif() + target_link_libraries(qtmaildir_lib PUBLIC Qt6::Widgets Qt6::Svg Qt6::WebEngineWidgets PkgConfig::GMIME ${NOTMUCH_LIBRARY} PkgConfig::CMARK_GFM diff --git a/src/composecontext.cpp b/src/composecontext.cpp index d0406fc..7233330 100644 --- a/src/composecontext.cpp +++ b/src/composecontext.cpp @@ -24,6 +24,7 @@ #include "composecontext.h" #include "config.h" +#include "maildirname.h" #include "mimeparser.h" #include <QDir> @@ -491,16 +492,32 @@ ComposeContext ComposeContextBuilder::forDraft(const Config &config, { ComposeContext context; + // Item 163. The caller's path comes from the model, captured when the + // query ran, and mbsync renames an uploaded draft to add its `,U=<uid>` + // infix. Resolving first is what stops a rename from refusing the reopen: + // the refusal happens BEFORE any composer exists, so the user composes + // again into a FRESH window whose autosave has no previous path to unlink, + // and the draft is silently forked into two files with two Message-IDs, + // both of which reach the server. + // + // Returns the path unchanged when nothing was renamed, and empty when the + // file is genuinely gone, which still fails below exactly as before. + const QString resolved = MaildirName::resolveRenamed(path); + MimeParser parser; - const ParsedMessage draft = parser.parse(path); + const ParsedMessage draft = parser.parse(resolved); if (!draft.ok) return context; // Kind::New and empty: the caller reports the failure. context.kind = ComposeContext::Kind::Draft; - context.originalPath = path; + context.originalPath = resolved; // The file this composer OWNS. Without it the first autosave writes a // second draft and leaves this one behind, so one message becomes two. - context.draftPath = path; + // + // The RESOLVED path, never the caller's: seeding the stale one would let + // the reopen succeed and the unlink still miss, which is the same fork + // arriving one step later. + context.draftPath = resolved; const auto addresses = [](const QString &header) { QStringList out; diff --git a/src/keymap.cpp b/src/keymap.cpp index 6cd965a..6605882 100644 --- a/src/keymap.cpp +++ b/src/keymap.cpp @@ -33,6 +33,10 @@ QStringList KeyMap::knownActions() QStringLiteral("delete"), QStringLiteral("restore"), QStringLiteral("cleanup_stranded"), + // Item 118. No default binding, deliberately: this is the one action + // that destroys mail with no undo, and a chord is how it would be run + // by accident. Menu only, which item 132 made a legitimate choice. + QStringLiteral("empty_trash"), QStringLiteral("spam"), QStringLiteral("toggle_unread"), QStringLiteral("mark_all_read"), @@ -52,7 +56,12 @@ QStringList KeyMap::knownActions() QStringLiteral("archive_thread"), QStringLiteral("delete_thread"), QStringLiteral("spam_thread"), - QStringLiteral("toggle_unread_thread"), + // Item 112 split the thread toggle in two. Neither carries a default + // chord, at the user's choice: since item 132 a shortcut is a chosen + // subset rather than a requirement, and Ctrl+Alt+U meant whichever + // direction the union happened to pick, which is what made it wrong. + QStringLiteral("mark_thread_read"), + QStringLiteral("mark_thread_unread"), QStringLiteral("flag_thread"), // Compose and send (item 123). save_message deliberately carries no // default chord: since item 132 a shortcut is a chosen subset rather @@ -177,7 +186,6 @@ QList<QPair<QString, QString>> KeyMap::defaultBindings() { QStringLiteral("Ctrl+Alt+E"), QStringLiteral("archive_thread") }, { QStringLiteral("Ctrl+Alt+D"), QStringLiteral("delete_thread") }, { QStringLiteral("Ctrl+Alt+S"), QStringLiteral("spam_thread") }, - { QStringLiteral("Ctrl+Alt+U"), QStringLiteral("toggle_unread_thread") }, { QStringLiteral("Ctrl+Alt+I"), QStringLiteral("flag_thread") }, { QStringLiteral("Ctrl+T"), QStringLiteral("edit_tags") }, // Shifted against Ctrl+T for the same reason Ctrl+Shift+U is shifted diff --git a/src/maildirname.cpp b/src/maildirname.cpp index 6263aec..9f827fc 100644 --- a/src/maildirname.cpp +++ b/src/maildirname.cpp @@ -20,6 +20,8 @@ #include <QCoreApplication> #include <QDateTime> +#include <QDir> +#include <QFileInfo> #include <QHostInfo> namespace MaildirName { @@ -77,4 +79,63 @@ QString fresh(const QString &oldName) .arg(info); } +QString resolveRenamed(const QString &path) +{ + if (path.isEmpty()) + return QString(); + + // The ordinary case, and the overwhelmingly common one: nothing was + // renamed. One stat, then out. + if (QFileInfo::exists(path)) + return path; + + const QFileInfo info(path); + const QString name = info.fileName(); + + // The unique part mbsync preserves. `<stem>:2,D` becomes + // `<stem>,U=5:2,D`, so the stem ends at whichever of `,` or `:` comes + // first. A name carrying neither is all stem. + int cut = name.size(); + for (const QChar separator : { QLatin1Char(','), QLatin1Char(':') }) { + const int at = name.indexOf(separator); + if (at >= 0 && at < cut) + cut = at; + } + const QString stem = name.left(cut); + if (stem.isEmpty()) + return QString(); + + // One directory, never a recursive walk: a rename keeps the file where it + // was, and a file that changed FOLDERS is a different question that only + // the message id can answer (see NotmuchWorker::moveMessages(), item 162). + const QDir dir(info.absolutePath()); + if (!dir.exists()) + return QString(); + + QString found; + const QFileInfoList entries = + dir.entryInfoList(QDir::Files | QDir::NoDotAndDotDot); + for (const QFileInfo &entry : entries) { + const QString candidate = entry.fileName(); + // Anchored on the stem AND on what follows it, so `...Q2` cannot match + // `...Q23`: the next character must begin the infix or the flags. + if (!candidate.startsWith(stem)) + continue; + const QString rest = candidate.mid(stem.size()); + if (!rest.isEmpty() && !rest.startsWith(QLatin1Char(',')) + && !rest.startsWith(QLatin1Char(':'))) { + continue; + } + + // Two files sharing a stem cannot happen in a correct Maildir. Refuse + // rather than guess: the caller reports "gone", which is honest, where + // a guess could open, move or delete the wrong message. + if (!found.isEmpty()) + return QString(); + found = entry.absoluteFilePath(); + } + + return found; +} + } // namespace MaildirName diff --git a/src/maildirname.h b/src/maildirname.h index f24bc71..255517d 100644 --- a/src/maildirname.h +++ b/src/maildirname.h @@ -38,4 +38,28 @@ namespace MaildirName { /// what a newly composed draft is. QString fresh(const QString &oldName); +/// The file \p path names, or the renamed file that replaced it. +/// +/// Item 163. mbsync renames an uploaded file to add its `,U=<uid>` infix, and +/// anything holding the previous name (the model's `MessageRef::filePath`, a +/// draft's `ComposeContext::draftPath`) then points at a path that no longer +/// exists. Returns \p path unchanged when it is still there, so the ordinary +/// case costs one stat and nothing else. +/// +/// Matched on the UNIQUE STEM, the part before the first `,` or `:`, which +/// mbsync preserves: `<stem>:2,D` becomes `<stem>,U=5:2,D`. That is what makes +/// this safe to do by filename at all. The search is confined to the file's +/// own directory and never recurses, and an ambiguous match (more than one +/// candidate, which a correct Maildir cannot produce) yields nothing rather +/// than guessing. +/// +/// Empty when there is no such file, which every caller must treat as the +/// genuine "it is gone" it is: recovering silently from a real deletion would +/// turn a reportable defect into a wrong answer. +/// +/// This resolves a RENAME, not a MOVE. A file that changed folders is a +/// different question and belongs to whoever knows the message id; +/// `NotmuchWorker::moveMessages()` re-resolves that way for item 162. +QString resolveRenamed(const QString &path); + } // namespace MaildirName diff --git a/src/main.cpp b/src/main.cpp index 2ed8057..a7908d0 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -43,7 +43,7 @@ int main(int argc, char *argv[]) for (int i = 1; i < argc; ++i) { if (std::strcmp(argv[i], "--version") == 0 || std::strcmp(argv[i], "-v") == 0) { - std::printf("qtmaildir %s\n", QTMAILDIR_VERSION); + std::printf("qtmaildir %s\n", QTMAILDIR_VERSION_DISPLAY); return 0; } if (std::strcmp(argv[i], "--help") == 0 @@ -59,7 +59,7 @@ int main(int argc, char *argv[]) "Configuration: ~/.config/qtmaildir/qtmaildir.conf\n" "qtmaildir reads a notmuch-indexed Maildir. It does no network\n" "protocol work: fetching and sending are external commands.\n", - QTMAILDIR_VERSION); + QTMAILDIR_VERSION_DISPLAY); return 0; } } diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index af3b817..89c01eb 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -18,6 +18,8 @@ #include "mainwindow.h" +#include "maildirname.h" + #include <QAction> #include <QApplication> #include <QCloseEvent> @@ -898,6 +900,16 @@ void MainWindow::buildUi() &QItemSelectionModel::selectionChanged, this, &MainWindow::onSelectionChanged); + // The label describes the SELECTION'S STATE, which a write moves without + // touching the selection: marking the current row read has to flip the + // entry to "Mark as unread" with the same row still selected. Keyed on + // the model rather than on each of the six call sites that apply an + // optimistic update, so a new one cannot forget. + connect(m_model, &QAbstractItemModel::dataChanged, this, [this]() { + refreshUnreadAction(); + refreshTrashActions(); + }); + connect(m_threadView, &QAbstractItemView::doubleClicked, this, &MainWindow::onRowDoubleClicked); @@ -1043,8 +1055,13 @@ void MainWindow::openComposerFor(const MessageRef &ref, return; } + // Item 163, the same stale path the pane and the draft reopen hit. Here it + // refuses a Reply or a Forward outright, so the user cannot answer a + // message that is sitting on disk and readable. + const QString originalPath = MaildirName::resolveRenamed(ref.filePath); + MimeParser parser; - const ParsedMessage original = parser.parse(ref.filePath); + const ParsedMessage original = parser.parse(originalPath); if (!original.ok) { showTransientStatus(tr("That message could not be read")); return; @@ -1052,7 +1069,7 @@ void MainWindow::openComposerFor(const MessageRef &ref, ComposeContext context; context.kind = kind; - context.originalPath = ref.filePath; + context.originalPath = originalPath; const bool replyAll = kind == ComposeContext::Kind::ReplyAll; const bool forwarding = kind == ComposeContext::Kind::Forward; @@ -1573,6 +1590,19 @@ void MainWindow::registerActions() [this]() { showStrandedDeletedMail(); }); + // The ONE irreversible action in this application, and the only one that + // asks before it runs (item 118). CLAUDE.md rules out confirmation + // dialogs for mutations because every mutation pushes its inverse onto + // the undo stack; a purge has no inverse, so the rule does not reach it. + // What the rule protects is that the user never loses work to a + // keystroke, which here is what the dialog provides. + // + // No default shortcut, for the same reason: a chord is how this would be + // run by accident. + addAction(QStringLiteral("empty_trash"), tr("Empt&y trash..."), + tr("Permanently delete every message in the trash"), [this]() { + emptyTrash(); + }); addAction(QStringLiteral("spam"), tr("Mark &spam"), tr("Add spam and remove inbox"), [this]() { tagSelected({ QStringLiteral("spam") }, { QStringLiteral("inbox") }, @@ -1677,21 +1707,34 @@ void MainWindow::registerActions() tagSelected({ QStringLiteral("spam") }, { QStringLiteral("inbox") }, tr("Mark thread spam"), TagScope::Thread); }); - addAction(QStringLiteral("toggle_unread_thread"), tr("Toggle &unread"), - tr("Toggle the unread tag on whole threads"), [this]() { + // Two fixed directions rather than one toggle, and the asymmetry with the + // message-scoped twin is the point (item 112). `ThreadSummary::tags` is + // notmuch's UNION over the conversation, so a thread holding even one + // unread message answers "unread" and a toggle reading that predicate + // always chose "mark read": there was no input that reached "mark thread + // unread" on a mixed thread, which is exactly the thread a user wants it + // for. A union is not a state, and a toggle needs a state. + // + // The message-scoped `toggle_unread` stays a toggle, because one message + // has a real two-valued state. Do not unify them. + addAction(QStringLiteral("mark_thread_read"), tr("Mark thread &read"), + tr("Remove the unread tag from every message of the selected " + "threads"), [this]() { + m_markReadTimer->stop(); + m_markReadMessageId.clear(); + tagSelected({}, { QStringLiteral("unread") }, + tr("Mark thread read"), TagScope::Thread); + }); + addAction(QStringLiteral("mark_thread_unread"), tr("Mark thread &unread"), + tr("Add the unread tag to every message of the selected threads"), + [this]() { // Cancels the automatic mark-read for the same reason its // message-scoped twin does: a thread marked unread by hand must not be // undone a moment later by a timer armed when it was opened. m_markReadTimer->stop(); m_markReadMessageId.clear(); - - if (everySelectedRowHasTag(QStringLiteral("unread"), TagScope::Thread)) { - tagSelected({}, { QStringLiteral("unread") }, - tr("Mark thread read"), TagScope::Thread); - } else { - tagSelected({ QStringLiteral("unread") }, {}, - tr("Mark thread unread"), TagScope::Thread); - } + tagSelected({ QStringLiteral("unread") }, {}, + tr("Mark thread unread"), TagScope::Thread); }); addAction(QStringLiteral("flag_thread"), tr("&Important"), tr("Mark every message of the selected threads as important"), @@ -1929,6 +1972,7 @@ void MainWindow::buildMenus() // It replaces the whole view like a filter does, so a sixth button beside // the five filters would read as one of them. messageMenu->addAction(m_actions.value(QStringLiteral("cleanup_stranded"))); + messageMenu->addAction(m_actions.value(QStringLiteral("empty_trash"))); messageMenu->addAction(m_actions.value(QStringLiteral("tag_rules"))); auto *viewMenu = menuBar()->addMenu(tr("&View")); @@ -1989,6 +2033,7 @@ void MainWindow::buildMenus() // nothing, so an icon from the delete family would promise the one // thing it deliberately does not do. { QStringLiteral("cleanup_stranded"), QStringLiteral("system-search") }, + { QStringLiteral("empty_trash"), QStringLiteral("edit-delete-shred") }, { QStringLiteral("undo"), QStringLiteral("edit-undo") }, { QStringLiteral("spam"), QStringLiteral("mail-mark-junk") }, { QStringLiteral("flag"), QStringLiteral("mail-mark-important") }, @@ -2033,7 +2078,8 @@ void MainWindow::buildMenus() { QStringLiteral("archive_thread"), QStringLiteral("mail-archive") }, { QStringLiteral("delete_thread"), QStringLiteral("edit-delete") }, { QStringLiteral("spam_thread"), QStringLiteral("mail-mark-junk") }, - { QStringLiteral("toggle_unread_thread"), QStringLiteral("mail-mark-unread") }, + { QStringLiteral("mark_thread_read"), QStringLiteral("mail-mark-read") }, + { QStringLiteral("mark_thread_unread"), QStringLiteral("mail-mark-unread") }, { QStringLiteral("flag_thread"), QStringLiteral("mail-mark-important") }, // Compose and send (item 123). reply_no_quote SHARES reply's icon for @@ -2439,7 +2485,7 @@ void MainWindow::showAbout() "version 2.</p>" "<p>Developed with AI assistance. All code is reviewed, " "tested and curated by the maintainer.</p>") - .arg(QStringLiteral(QTMAILDIR_VERSION))); + .arg(QStringLiteral(QTMAILDIR_VERSION_DISPLAY))); auto *link = new QLabel( QStringLiteral("<a href='https://danix.xyz/qtmaildir'>" @@ -2511,6 +2557,18 @@ void MainWindow::wireWorker() connect(m_worker, &NotmuchWorker::messagesMovedFrom, this, &MainWindow::onMessagesMoved); + // A purge removes rows rather than changing them, so there is no + // optimistic update to apply: the only honest view is the one the query + // gives now. Without this the list went on showing mail that no longer + // existed until the user refreshed by hand, which is how the user found + // it. + connect(m_worker, &NotmuchWorker::messagesPurged, this, + [this](const QStringList &messageIds) { + showTransientStatus( + tr("Deleted %n message(s) permanently", "", messageIds.size())); + runCurrentQuery(); + }); + connect(m_worker, &NotmuchWorker::threadMessagesResolved, this, &MainWindow::onThreadMessagesResolved); @@ -3461,8 +3519,98 @@ void MainWindow::showThreadContextMenu(const QPoint &pos) m_threadContextMenu->popup(m_threadView->viewport()->mapToGlobal(pos)); } +bool MainWindow::everySelectedRowIsInATrashFolder() const +{ + const QModelIndexList rows = + m_threadView->selectionModel()->selectedRows(); + if (rows.isEmpty()) + return false; + + for (const QModelIndex &index : rows) { + // The row's own file: a reply row's message, a thread row's displayed + // message. Same rule as everySelectedRowHasTag(), and for the same + // reason: a thread row acts on the message its card shows. + const QString path = + m_model->isMessageRow(index) + ? m_model->messageAt(index).filePath + : m_model->threadFor(index).firstMessagePath; + if (path.isEmpty()) + return false; + + const Account account = accountForMessagePath(path); + if (account.maildir.isEmpty() || account.trash.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('/'); + // accountForMessagePath() accepts both shapes, so this must too: a + // thread row's path is database-relative and a reply row's absolute. + if (!path.contains(prefix)) + return false; + } + return true; +} + +void MainWindow::refreshTrashActions() +{ + const bool inTrash = everySelectedRowIsInATrashFolder(); + const bool haveSelection = + !m_threadView->selectionModel()->selectedRows().isEmpty(); + + // Delete on mail already in the trash reported success and did nothing: + // moveMessages() finds the file already in the destination and takes its + // early-return branch, which counts an unsynced change for a move that + // never happened (item 168). + if (auto *del = m_actions.value(QStringLiteral("delete"))) + del->setVisible(!haveSelection || !inTrash); + + // 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"))) + restore->setVisible(!haveSelection || inTrash); +} + +void MainWindow::refreshUnreadAction() +{ + // The user's design (item 112 and its duplicates 99/147): the label says + // which way the action will go, and on a selection with no single state + // the entry is HIDDEN rather than labelled wrongly. The thread submenu is + // then the route, whose entries are absolute and work whatever the mix. + auto *action = m_actions.value(QStringLiteral("toggle_unread")); + if (!action) + return; + + switch (selectionTagPresence(QStringLiteral("unread"))) { + case TagPresence::Every: + action->setVisible(true); + action->setText(tr("Mark as &read")); + action->setStatusTip(tr("Remove the unread tag from the selection")); + break; + case TagPresence::None: + action->setVisible(true); + action->setText(tr("Mark as &unread")); + action->setStatusTip(tr("Add the unread tag to the selection")); + break; + case TagPresence::Mixed: + // No honest label exists, so there is no label to show. Hidden rather + // than disabled, at the user's choice. + action->setVisible(false); + break; + } +} + void MainWindow::onSelectionChanged() { + // Here rather than in the currentRowChanged handler: that signal is + // emitted BEFORE the selection model is updated, so a handler reading + // selectedRows() there sees the PREVIOUS selection and would label the + // action for the rows the user just left (CLAUDE.md, verified Qt 6.11). + refreshUnreadAction(); + refreshTrashActions(); + const QModelIndexList rows = m_threadView->selectionModel()->selectedRows(); const int selected = rows.size(); if (selected == 1) { @@ -3849,11 +3997,22 @@ void MainWindow::renderMessages(const QVector<MessageRef> &messages) const MessageRef &ref = messages.at(i); ThreadRenderItem item; - item.message = parser.parse(ref.filePath); + // Item 163. The model's path was captured when the query ran, and + // mbsync renames an uploaded file to add its `,U=<uid>` infix, so a row + // loaded before that sync names a file that no longer exists. The pane + // then reported the message unreadable while nothing was wrong with it. + // Unchanged when nothing was renamed; empty when the file is genuinely + // gone, which still reports below. + const QString path = MaildirName::resolveRenamed(ref.filePath); + item.message = parser.parse(path); if (!item.message.ok) { // One unreadable message must not lose the rest of the thread, so // it becomes an inline note rather than replacing the whole pane. + // + // Named by the path the model HOLDS, not by the resolved one: when + // resolution failed there is no resolved path, and the stale name + // is what the user can act on. item.message = {}; item.message.ok = true; item.message.from = tr("(unreadable message)"); @@ -4897,6 +5056,16 @@ QString MainWindow::currentThreadFirstMessageId() const bool MainWindow::everySelectedRowHasTag(const QString &tag, TagScope scope) const { + // Kept as the direction question, which only has two answers to give: a + // mixed selection has to go one way, and this says which. The LABEL asks + // selectionTagPresence() instead, because a label can say "these disagree" + // and a direction cannot. + return selectionTagPresence(tag, scope) == TagPresence::Every; +} + +MainWindow::TagPresence MainWindow::selectionTagPresence(const QString &tag, + TagScope scope) const +{ // What a toggle asks before choosing its direction, for both Delete and // Toggle unread. // @@ -4914,8 +5083,9 @@ bool MainWindow::everySelectedRowHasTag(const QString &tag, const QModelIndexList rows = m_threadView->selectionModel()->selectedRows(); if (rows.isEmpty()) - return false; + return TagPresence::None; + int withTag = 0; for (const QModelIndex &index : rows) { QStringList tags; if (scope == TagScope::Thread) { @@ -4961,10 +5131,13 @@ bool MainWindow::everySelectedRowHasTag(const QString &tag, tags = own.messageId.isEmpty() ? summary.firstMessageTags : own.tags; } - if (!tags.contains(tag)) - return false; + if (tags.contains(tag)) + ++withTag; } - return true; + + if (withTag == 0) + return TagPresence::None; + return withTag == rows.size() ? TagPresence::Every : TagPresence::Mixed; } ThreadSummary MainWindow::threadForCurrentRowForTesting() const @@ -4985,7 +5158,8 @@ QMenu *MainWindow::buildThreadActionsMenu(QWidget *parent) menu->addAction(m_actions.value(QStringLiteral("delete_thread"))); menu->addAction(m_actions.value(QStringLiteral("spam_thread"))); menu->addSeparator(); - menu->addAction(m_actions.value(QStringLiteral("toggle_unread_thread"))); + menu->addAction(m_actions.value(QStringLiteral("mark_thread_read"))); + menu->addAction(m_actions.value(QStringLiteral("mark_thread_unread"))); menu->addAction(m_actions.value(QStringLiteral("flag_thread"))); return menu; } @@ -5271,8 +5445,23 @@ void MainWindow::trashMessages(const QStringList &messageIds, return; for (auto it = byTrash.cbegin(); it != byTrash.cend(); ++it) { + // `unread` goes with it (item 168, the user's request). Deleting is a + // decision about the message, so the unread count must not go on + // including what the user threw away. + // + // In the SAME change rather than as a second write, so one undo + // returns the folder and the tag together: TagChange::inverted() + // gives it back only if it travelled with the move. + // + // This rewrites the Maildir filename, because + // maildir.synchronize_flags is true, and so reaches the server on the + // next mbsync. That is the same mechanism the post-new hook REFUSES + // to touch, and the difference is who is acting: the hook tags + // arriving mail unattended, while this is an explicit gesture on a + // message in front of the user. sendMove(it.value(), it.key(), - { QStringLiteral("deleted"), kOriginTagPlaceholder() }, {}, + { QStringLiteral("deleted"), kOriginTagPlaceholder() }, + { QStringLiteral("unread") }, tr("Delete"), false, wholeThreadIds); } @@ -5374,6 +5563,11 @@ void MainWindow::onThreadMessagesResolved(const QStringList &messageIds, const QStringList threadScope = m_pendingThreadScope; m_pendingThreadScope.clear(); + if (requestTag == QStringLiteral("empty_trash")) { + confirmAndPurge(messageIds); + return; + } + if (requestTag == QStringLiteral("delete_thread")) { trashMessages(messageIds, pathById, messageIds.size(), threadScope); return; @@ -5602,6 +5796,83 @@ void MainWindow::restoreSelectedFromTrash() Q_ARG(QString, QStringLiteral("restore_messages"))); } +void MainWindow::purgeForTesting(const QStringList &messageIds) +{ + if (!m_worker || messageIds.isEmpty()) + return; + QMetaObject::invokeMethod(m_worker, "purgeMessages", Qt::QueuedConnection, + Q_ARG(QStringList, messageIds)); +} + +void MainWindow::emptyTrash() +{ + // Scoped to the account selector, like every other account-aware surface: + // the All accounts view empties every configured trash, a selected + // account empties only its own. The user sees which in the dialog. + const QString accountKey = m_accountBox->currentData().toString(); + const QString query = accountKey.isEmpty() + ? m_config.allTrashQuery() + : m_config.account(accountKey).trashQuery(); + + // An account with no trash folder configured produces an EMPTY query, and + // an empty notmuch query matches EVERYTHING. Refusing here rather than + // relying on the worker's own guard, so the message names the cause. + if (query.isEmpty()) { + showTransientStatus(tr("No trash folder is configured")); + return; + } + + if (!m_worker) { + showTransientStatus(tr("Not connected to the mail index")); + return; + } + + // Enumerated before it is counted, and counted from the DATABASE: the + // number in the dialog has to be the number destroyed, and the model + // holds whatever the current view is showing, which is usually not the + // trash at all. + QMetaObject::invokeMethod(m_worker, "resolveQueryMessages", + Qt::QueuedConnection, + Q_ARG(QString, query), + Q_ARG(QString, QStringLiteral("empty_trash"))); +} + +void MainWindow::confirmAndPurge(const QStringList &messageIds) +{ + if (messageIds.isEmpty()) { + showTransientStatus(tr("The trash is already empty")); + return; + } + + const QString accountKey = m_accountBox->currentData().toString(); + const QString where = accountKey.isEmpty() + ? tr("every account") + : m_accountBox->currentText(); + + QMessageBox box(this); + box.setObjectName(QStringLiteral("emptyTrashConfirmation")); + box.setIcon(QMessageBox::Warning); + box.setWindowTitle(tr("Empty trash")); + box.setText(tr("Permanently delete %n message(s) from the trash of %1?", + "", messageIds.size()) + .arg(where)); + // Said plainly, because it is the only place in this application where it + // is true. + box.setInformativeText(tr("This cannot be undone.")); + box.addButton(QMessageBox::Cancel); + QPushButton *confirm = + box.addButton(tr("Delete permanently"), QMessageBox::DestructiveRole); + // Cancel is the default, so Return does not destroy mail. + box.setDefaultButton(QMessageBox::Cancel); + box.exec(); + + if (box.clickedButton() != confirm) + return; + + QMetaObject::invokeMethod(m_worker, "purgeMessages", Qt::QueuedConnection, + Q_ARG(QStringList, messageIds)); +} + void MainWindow::showStrandedDeletedMail() { // Not scoped to the selected account, deliberately. The stranded mail is diff --git a/src/mainwindow.h b/src/mainwindow.h index 951eaa4..a5a8c31 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -169,6 +169,11 @@ public: /// command was pushed, which is what "this did nothing" has to assert. int undoDepthForTesting() const { return m_undoStack.count(); } + /// Runs a purge without the confirmation, which a test cannot drive: a + /// modal blocks the thread it is shown on (item 84). What this exists to + /// cover is what happens AFTER the user confirms. + void purgeForTesting(const QStringList &messageIds); + /// The text of the command on top of the undo stack. /// /// A test seam for the DIRECTION a toggle chose. Delete and Undelete both @@ -887,6 +892,37 @@ private: bool everySelectedRowHasTag(const QString &tag, TagScope scope = TagScope::Message) const; + /// The three-valued version of the question above, which is what a LABEL + /// needs and a toggle's direction does not. + /// + /// `everySelectedRowHasTag` answers yes or no over a reality with three + /// states: every row has the tag, none does, or they disagree. That is + /// enough to choose a direction, since a mixed selection has to go one way + /// or the other, but it cannot name the direction honestly, and item 112 + /// is what happens when a two-valued predicate is asked a three-valued + /// question. + enum class TagPresence { None, Every, Mixed }; + TagPresence selectionTagPresence( + const QString &tag, TagScope scope = TagScope::Message) const; + + /// Relabels the unread action, and hides it when the selection has no + /// single state. Called whenever the selection changes. + void refreshUnreadAction(); + + /// Hides Delete on mail already in the trash, and Restore on mail that + /// was never there (item 168). Each is offered only where it means + /// something, the same rule refreshUnreadAction() applies to the label. + void refreshTrashActions(); + + /// Whether every selected row's file already sits in its account's trash + /// folder. Empty selection answers false. + /// + /// The question is about the PATH, never the `deleted` TAG: a message + /// trashed by another client carries no such tag at all, which is why the + /// trash view is path-based (item 103), and asking the tag would offer + /// Delete on exactly the mail a trash view is full of. + bool everySelectedRowIsInATrashFolder() const; + void editTagsOnSelection(); /// Set once the user has answered the exit prompt, or once a sync started @@ -1015,6 +1051,17 @@ private: /// without moving. void showStrandedDeletedMail(); + /// Asks the worker what is in the trash. The answer arrives at + /// onThreadMessagesResolved() tagged `empty_trash` and goes to + /// confirmAndPurge(): the count in the dialog has to be what will actually + /// be destroyed, so it comes from the database rather than from the model, + /// which holds whatever the current view happens to show. + void emptyTrash(); + + /// The confirmation, and the only one in this application. Destroys + /// nothing if the user declines. + void confirmAndPurge(const QStringList &messageIds); + /// Moves each resolved message home, using the tags and paths the WORKER /// reported rather than anything the model holds. void restoreResolvedMessages(const QStringList &messageIds, diff --git a/src/messageview.cpp b/src/messageview.cpp index 86e40eb..eb0dccd 100644 --- a/src/messageview.cpp +++ b/src/messageview.cpp @@ -544,7 +544,7 @@ void MessageView::showPlaceholder( // uses it: a style sheet or a themed parent can give this pane different // colours from the application. setDocument(HtmlBuilder::buildPlaceholder( - helpers, QStringLiteral(QTMAILDIR_VERSION), + helpers, QStringLiteral(QTMAILDIR_VERSION_DISPLAY), HtmlBuilder::brandPaletteFrom(palette()))); } diff --git a/src/nmraii.h b/src/nmraii.h index 1c3e8f1..925a8e7 100644 --- a/src/nmraii.h +++ b/src/nmraii.h @@ -67,3 +67,5 @@ using NmMessages = NmHandle<notmuch_messages_t, notmuch_messages_destroy>; using NmThread = NmHandle<notmuch_thread_t, notmuch_thread_destroy>; using NmMessage = NmHandle<notmuch_message_t, notmuch_message_destroy>; using NmTags = NmHandle<notmuch_tags_t, notmuch_tags_destroy>; +using NmFilenames = + NmHandle<notmuch_filenames_t, notmuch_filenames_destroy>; diff --git a/src/notmuchworker.cpp b/src/notmuchworker.cpp index 16df4ed..8ab3ab5 100644 --- a/src/notmuchworker.cpp +++ b/src/notmuchworker.cpp @@ -184,6 +184,39 @@ void walkReplies(notmuch_messages_t *messages, int depth, } } +/// Teach the database the current filenames in one Maildir directory. +/// +/// Item 162's recovery step. mbsync renames a file to add its `,U=<uid>` infix +/// and notmuch does not learn the new name until a `notmuch new` runs; this +/// indexes just the one directory rather than waiting for that sweep. +/// +/// Deliberately NOT a full `notmuch new`: that walks the entire Maildir and +/// runs the post-new hook, which tags real mail. This must stay a read of one +/// folder with no side effects beyond the filenames it records. +/// +/// Indexing a file already known under another name ADDS a filename to the +/// same message rather than creating a second message, which is what lets the +/// caller pick the surviving path out of get_filenames(). Errors are ignored +/// on purpose: this is a best-effort repair whose caller reports the failure +/// if the path is still missing afterwards. +void reindexFolder(notmuch_database_t *db, const QString &folder) +{ + const QDir dir(folder); + if (!dir.exists()) + return; + + const QFileInfoList entries = + dir.entryInfoList(QDir::Files | QDir::NoDotAndDotDot); + for (const QFileInfo &entry : entries) { + notmuch_message_t *indexed = nullptr; + notmuch_database_index_file( + db, entry.absoluteFilePath().toUtf8().constData(), nullptr, + &indexed); + if (indexed) + notmuch_message_destroy(indexed); + } +} + /// The Maildir FOLDER a message file sits in, relative to the database root. /// /// `<root>/acct/inbox/cur/12345` becomes `acct/inbox`: the `cur`/`new` segment @@ -255,8 +288,28 @@ QByteArray NotmuchWorker::configPathArg() const bool NotmuchWorker::openReadOnly() { - if (m_db) + if (m_db) { + // A read-only handle is a Xapian SNAPSHOT taken when it was opened, so + // it never observes a write made by another process afterwards. The + // sync script's `notmuch new` is exactly that, which made mail arriving + // while the window was open invisible until the application restarted: + // not only to the post-sync refresh, but to any query the user typed by + // hand, since all of them are answered from the same handle. Item 104. + // + // Reopening here rather than at each call site covers every read path, + // which all begin by asking for the handle. It is cheap and it is what + // notmuch provides the call for; a failure is deliberately NOT fatal, + // since the existing handle is still usable and serving slightly stale + // results beats refusing to answer at all. + const notmuch_status_t status = + notmuch_database_reopen(m_db, NOTMUCH_DATABASE_MODE_READ_ONLY); + if (status != NOTMUCH_STATUS_SUCCESS) { + emit errorOccurred( + QStringLiteral("Cannot refresh notmuch database: %1") + .arg(QString::fromUtf8(notmuch_status_to_string(status)))); + } return true; + } const QByteArray configPath = configPathArg(); char *error = nullptr; @@ -747,7 +800,54 @@ void NotmuchWorker::moveMessages(const QStringList &messageIds, const char *rawName = notmuch_message_get_filename(message.get()); if (!rawName) continue; - const QString from = QString::fromUtf8(rawName); + QString from = QString::fromUtf8(rawName); + + // Item 162. mbsync renames an uploaded file to record the server UID + // (`<name>,U=<uid>:2,<flags>`), and notmuch keeps the pre-`U=` name + // until that sync's `notmuch new` runs. Renaming a path that no longer + // exists fails, and Delete silently does nothing while blaming the + // destination folder for a timing problem. + // + // Refusing while a sync holds the write lock does NOT close this: + // mbsync renames throughout its run without touching notmuch's lock, + // so the damaging window is open when there is nothing to observe. + // Re-resolving is what closes it. + // + // Recovery is by MESSAGE ID, never by scanning the folder: two files + // can carry the same id, and picking the wrong one moves the wrong + // file. notmuch_message_get_filenames() lists every path the database + // holds for this id, so a file that was renamed rather than removed is + // found among them once the folder is reindexed. + if (!QFileInfo::exists(from)) { + // One reindex of the containing folder, which is what teaches + // notmuch the new name. Bounded deliberately: a single attempt, + // and a message that is still missing afterwards falls through to + // the error below, so a genuinely deleted file is still reported + // (item 41's territory) rather than becoming a silent no-op. + const QString folder = QFileInfo(from).absolutePath(); + message.reset(); + reindexFolder(db, folder); + + notmuch_message_t *again = nullptr; + if (notmuch_database_find_message(db, id.toUtf8().constData(), + &again) + == NOTMUCH_STATUS_SUCCESS + && again) { + message.reset(again); + for (NmFilenames names( + notmuch_message_get_filenames(message.get())); + notmuch_filenames_valid(names.get()); + notmuch_filenames_move_to_next(names.get())) { + const QString candidate = QString::fromUtf8( + notmuch_filenames_get(names.get())); + if (QFileInfo::exists(candidate)) { + from = candidate; + break; + } + } + } + } + // The handle is released before the file moves under it. message.reset(); @@ -820,6 +920,99 @@ void NotmuchWorker::moveMessages(const QStringList &messageIds, emit messagesMovedFrom(origins, destFolder); } +void NotmuchWorker::purgeMessages(const QStringList &messageIds) +{ + if (messageIds.isEmpty()) + return; + + // Same handle ordering as applyTags() and moveMessages(): notmuch allows + // one open handle per process, so the read-only one closes first. + close(); + + const QByteArray configPath = configPathArg(); + notmuch_database_t *db = nullptr; + char *error = nullptr; + const notmuch_status_t status = notmuch_database_open_with_config( + nullptr, + NOTMUCH_DATABASE_MODE_READ_WRITE, + configPath.isEmpty() ? nullptr : configPath.constData(), + nullptr, + &db, + &error); + + if (status != NOTMUCH_STATUS_SUCCESS) { + emit errorOccurred( + QStringLiteral("Cannot open database for writing: %1") + .arg(QString::fromUtf8(error ? error + : notmuch_status_to_string(status)))); + free(error); + return; + } + + QStringList purged; + for (const QString &id : messageIds) { + notmuch_message_t *raw = nullptr; + // find_message reports SUCCESS with a null message for an unknown id, + // so both are checked. A stale id does not abort the batch: the live + // ids beside it still have to go. + if (notmuch_database_find_message(db, id.toUtf8().constData(), &raw) + != NOTMUCH_STATUS_SUCCESS || !raw) { + continue; + } + NmMessage message(raw); + + // EVERY file, not just the first. notmuch deduplicates by Message-ID, + // so one message can have several files; unlinking one would leave the + // message alive in the folder the user emptied, which reads as the + // purge having silently skipped it. This is the same one-message, + // many-files property that item 166 turned on. + QStringList files; + for (NmFilenames names(notmuch_message_get_filenames(message.get())); + notmuch_filenames_valid(names.get()); + notmuch_filenames_move_to_next(names.get())) { + files.append(QString::fromUtf8(notmuch_filenames_get(names.get()))); + } + + // The handle is released before the files go out from under it. + message.reset(); + + bool removedAny = false; + for (const QString &file : files) { + // A file already gone is not an ERROR: the index can name a path a + // sync has since removed, and the goal state (no file) is reached + // either way. Reporting it would teach the user to ignore the one + // message that matters here. + // + // It is not a DESTRUCTION either, which is a separate point and + // the one a first version got wrong. The count reaches the user as + // the size of an irreversible act, so it must say what this run + // actually destroyed, not what was already absent when it started. + if (!QFile::exists(file)) { + notmuch_database_remove_message(db, file.toUtf8().constData()); + continue; + } + if (!QFile::remove(file)) { + emit errorOccurred(QStringLiteral("Cannot delete %1") + .arg(QFileInfo(file).fileName())); + continue; + } + removedAny = true; + // The index entry for that path. When the last filename goes, so + // does the message and every tag on it, which is exactly what is + // wanted here and is the thing moveMessages() has to avoid. + notmuch_database_remove_message(db, file.toUtf8().constData()); + } + + if (removedAny) + purged.append(id); + } + + notmuch_database_close(db); + notmuch_database_destroy(db); + + emit messagesPurged(purged); +} + void NotmuchWorker::indexDraftFile(const QString &path, const QString &previousPath) { @@ -925,6 +1118,14 @@ void NotmuchWorker::resolveMessages(const QStringList &messageIds, resolveQuery(terms.join(QStringLiteral(" or ")), requestTag); } +void NotmuchWorker::resolveQueryMessages(const QString &query, + const QString &requestTag) +{ + if (query.isEmpty()) + return; + resolveQuery(query, requestTag); +} + void NotmuchWorker::resolveThreadMessages(const QStringList &threadIds, const QString &requestTag) { diff --git a/src/notmuchworker.h b/src/notmuchworker.h index 3ccf8e5..2efddaa 100644 --- a/src/notmuchworker.h +++ b/src/notmuchworker.h @@ -134,6 +134,22 @@ public slots: /// it, so removing before indexing loses the message's tags. void moveMessages(const QStringList &messageIds, const QString &destFolder); + /// Destroys mail: removes each file from disk and each message from the + /// index. **This is the only irreversible operation in the application** + /// (item 118), which is why it is a separate entry point rather than a + /// flag on moveMessages(): the two look alike and one of them can be + /// undone. + /// + /// Named ids only, never a folder-wide sweep, so the blast radius is + /// whatever the caller enumerated and confirmed. A message with several + /// files loses every file it has, since leaving one behind would leave + /// the message alive in a folder the user emptied. + /// + /// The caller is responsible for confirming: CLAUDE.md rules out + /// confirmation dialogs for mutations because undo replaces them, and + /// this is the one action where undo cannot exist. + void purgeMessages(const QStringList &messageIds); + /// Indexes one freshly written file, so it appears in a `path:` query /// without a full `notmuch new` (item 158). /// @@ -190,6 +206,12 @@ public slots: void resolveMessages(const QStringList &messageIds, const QString &requestTag); + /// The same walk for an arbitrary QUERY, which is what Empty Trash needs: + /// it has to enumerate what it is about to destroy before it can say how + /// much that is, and the answer must not come from the model, which holds + /// whatever the current view happens to be showing. + void resolveQueryMessages(const QString &query, const QString &requestTag); + private: /// The shared walk behind resolveMessages() and resolveThreadMessages(): /// runs `query` and emits threadMessagesResolved() with each match's id, @@ -273,6 +295,10 @@ signals: /// than aborting the batch. void messagesMoved(const QStringList &messageIds, const QString &destFolder); + /// What a purge actually destroyed. Unlike a move there is no new path to + /// observe afterwards, so this is the only report the UI has. + void messagesPurged(const QStringList &messageIds); + /// The same move, reported per message with the folder it came FROM. /// /// Emitted alongside messagesMoved rather than replacing it: that signal's diff --git a/src/version.h.in b/src/version.h.in index 8d76a3a..743adc5 100644 --- a/src/version.h.in +++ b/src/version.h.in @@ -25,3 +25,26 @@ #define QTMAILDIR_VERSION_MINOR @PROJECT_VERSION_MINOR@ #define QTMAILDIR_VERSION_PATCH @PROJECT_VERSION_PATCH@ #define QTMAILDIR_VERSION "@PROJECT_VERSION@" + +/// The version as shown to a person, which in a DEV build carries the build +/// number and in a release build is exactly QTMAILDIR_VERSION. +/// +/// Two separate macros deliberately. The release procedure checks `--version` +/// against a clean X.Y.Z, the SlackBuild builds from a release tarball where +/// no build counter exists, and the window title is a poor place for a number +/// that changes on every rebuild. Anything comparing versions uses +/// QTMAILDIR_VERSION; anything a person reads to answer "which build am I +/// running" uses this one. +/// +/// buildnumber.h is generated at BUILD time, not here: this file is written +/// by configure_file(), which runs once per cmake run, so a counter +/// interpolated into it would sit still across every rebuild, which is the +/// entire thing item 167 is about. It defines QTMAILDIR_BUILD_NUMBER only in +/// a dev build. +#include "buildnumber.h" + +#ifdef QTMAILDIR_BUILD_NUMBER +# define QTMAILDIR_VERSION_DISPLAY QTMAILDIR_VERSION " build " QTMAILDIR_BUILD_NUMBER +#else +# define QTMAILDIR_VERSION_DISPLAY QTMAILDIR_VERSION +#endif |