diff options
Diffstat (limited to 'src')
| -rw-r--r-- | src/mainwindow.cpp | 75 | ||||
| -rw-r--r-- | src/mainwindow.h | 29 | ||||
| -rw-r--r-- | src/notmuchworker.cpp | 14 | ||||
| -rw-r--r-- | src/threaddigest.h | 25 |
4 files changed, 123 insertions, 20 deletions
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 321c608..eb8f4c6 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -3667,29 +3667,53 @@ bool MainWindow::everySelectedRowIsInATrashFolder() const 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; + // A MESSAGE row is its one file. A CONVERSATION is in the trash 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. + // + // That was the pre-177 rule and it is now wrong in a way that is + // invisible: a partly trashed thread answered on whichever message the + // query returned first, hiding Delete on a conversation with mail + // outside the trash and offering Restore on one that mostly is not. + QStringList paths; + if (m_model->isMessageRow(index)) { + paths = { m_model->messageAt(index).filePath }; + } else { + const ThreadSummary thread = m_model->threadFor(index); + // Known only for the single conversation row the dashboard is + // showing, since that is the one the digest was requested for. + // Anything else falls back to the summary's one path, which is the + // pre-177 answer: wrong in the same partial case, so a multi-row + // selection is left no worse than it was rather than being given a + // second, differently wrong rule of its own. + paths = (!m_conversationPaths.isEmpty() + && m_conversationPathsThreadId == thread.threadId) + ? m_conversationPaths + : QStringList{ thread.firstMessagePath }; + } - const Account account = accountForMessagePath(path); - if (account.maildir.isEmpty() || account.trash.isEmpty()) + if (paths.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; + for (const QString &path : std::as_const(paths)) { + 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; } @@ -4149,6 +4173,10 @@ void MainWindow::onThreadSelected(const QModelIndex ¤t, } m_dashboardThreadId.clear(); + // Stale the moment the dashboard is not showing this thread: leaving them + // would answer a later conversation's question with an earlier one's paths. + m_conversationPathsThreadId.clear(); + m_conversationPaths.clear(); // 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 @@ -4210,6 +4238,13 @@ void MainWindow::onThreadDigestLoaded(const ThreadDigest &digest, if (digest.threadId != m_dashboardThreadId) return; + // Item 178: what Delete and Restore need to judge the CONVERSATION rather + // than its first message. Kept beside the thread id so a late digest for + // another row cannot answer about this one. + m_conversationPathsThreadId = digest.threadId; + m_conversationPaths = digest.messagePaths; + refreshTrashActions(); + m_messageView->showDashboard(digest); } diff --git a/src/mainwindow.h b/src/mainwindow.h index 16bf8c4..d7579f4 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -202,6 +202,16 @@ public: /// command was pushed, which is what "this did nothing" has to assert. int undoDepthForTesting() const { return m_undoStack.count(); } + /// Item 178. Stands in for the digest round trip, which a bare window has + /// no worker to make. Sets what onThreadDigestLoaded() would have set. + void setConversationPathsForTesting(const QString &threadId, + const QStringList &paths) + { + m_conversationPathsThreadId = threadId; + m_conversationPaths = paths; + refreshTrashActions(); + } + /// 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. @@ -1640,6 +1650,25 @@ private: /// which is also set for a thread of one message that renders normally. QString m_dashboardThreadId; + /// Every message path of the conversation the dashboard is showing, keyed + /// by its thread id so a late digest cannot answer about another row. + /// + /// Item 178. Delete and Restore ask whether a row is in the trash, and a + /// conversation is in the trash only when ALL of its messages are; the + /// summary carries ONE path, which was the right answer while a thread row + /// meant its first message (item 108) and stopped being right when item + /// 177 made it mean the conversation. + /// + /// Filled from ThreadDigest, which the selection already requests and + /// which already walks every message, so this costs no query of its own. + /// It is therefore known only for a SINGLE selected conversation row, and + /// everySelectedRowIsInATrashFolder() falls back to the summary's one path + /// when it is absent: the fallback is the pre-177 answer, wrong in exactly + /// the same partial case, so a multi-row selection is no better than + /// before and no worse. + QString m_conversationPathsThreadId; + QStringList m_conversationPaths; + /// The message a dashboard entry asked for and the thread it is in, both /// empty when nothing is waiting. See selectMessageInCurrentThread(). QString m_dashboardSelectMessageId; diff --git a/src/notmuchworker.cpp b/src/notmuchworker.cpp index 821c575..3a0b2c1 100644 --- a/src/notmuchworker.cpp +++ b/src/notmuchworker.cpp @@ -776,6 +776,11 @@ void NotmuchWorker::loadThreadDigest(const QString &threadId, QVector<MessageNode> unread; QVector<qint64> timestamps; + // mailRootOf(), never notmuch_database_get_path(): under a split index the + // latter returns the XAPIAN directory, and a path relative to that matches + // no account (item 124). + const QString dbRoot = QDir(mailRootOf(m_db)).absolutePath(); + notmuch_messages_t *messages = notmuch_thread_get_messages(thread.get()); for (; notmuch_messages_valid(messages); notmuch_messages_move_to_next(messages)) { @@ -786,6 +791,15 @@ void NotmuchWorker::loadThreadDigest(const QString &threadId, ++digest.totalCount; + // Item 178: which FOLDER each message lives in, so Delete and Restore + // can judge a conversation on all of it rather than on whichever + // message the query returned first. Read from the index by the walk + // that is already running. + if (const char *rawName = notmuch_message_get_filename(message)) { + digest.messagePaths.append( + QDir(dbRoot).relativeFilePath(QString::fromUtf8(rawName))); + } + const qint64 when = notmuch_message_get_date(message); timestamps.append(when); diff --git a/src/threaddigest.h b/src/threaddigest.h index e78a2ac..8c68e16 100644 --- a/src/threaddigest.h +++ b/src/threaddigest.h @@ -9,6 +9,7 @@ #include <QMetaType> #include <QPair> #include <QString> +#include <QStringList> #include <QVector> #include "types.h" @@ -41,6 +42,30 @@ struct ThreadDigest int totalCount = 0; + /// Every message's file, RELATIVE to the database path, in the walk's own + /// order. + /// + /// Item 178. Delete and Restore ask whether a row is in the trash, and a + /// CONVERSATION is in the trash only when all of its messages are. + /// ThreadSummary carries one path, which was the right answer while a + /// thread row meant its first message and is not one now, so a partly + /// trashed conversation answered on whichever message the query returned + /// first: Delete hidden on a thread with mail outside the trash, Restore + /// offered on one that mostly is not. + /// + /// Relative and not absolute, for the same reason + /// ThreadSummary::firstMessagePath is: the UI knows an account only by its + /// `maildir`, itself a database-relative prefix, so an absolute path + /// matches no account and silently resolves every row to none. + /// + /// Free, like everything else here: the digest already walks every message + /// for the sender counts, and a filename is served from the INDEX rather + /// than the message file. A message with several files contributes only + /// its first, which is what notmuch_message_get_filename returns; the + /// question is which FOLDER a message lives in, and a caller testing + /// "every path is under trash" is answered correctly by any one of them. + QStringList messagePaths; + /// Always kBuckets entries. A fixed count is what keeps the sparkline's /// geometry testable; a thread spanning five days and one spanning two /// years cannot share a bucket size, so the span is what varies and the |