From 0bbb2d2c8d75c2b4392b17214ec3ec3768305ba1 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Sat, 8 Aug 2026 11:13:10 +0200 Subject: feat(ui): render a single message when its row is selected loadMessage queries by id: and returns one MessageRef, always matched, since the user asked for that message by clicking its row and a stub would answer the wrong question. An unknown id emits an empty vector rather than an error: a stale row after a reindex is an ordinary race, not a failure worth the status bar. The signal fires even when empty so the UI handler runs instead of waiting for a reply that never comes. The branch in onThreadSelected is placed BEFORE threadAt(), which is the whole trap. threadAt takes a top-level row number and a child's row number indexes its siblings, so handing a message row's number to it loads whichever thread happens to sit at that position. Mutation-checked: with the branch disabled the test reports thread 't1' for a reply belonging to 't2', a wrong answer plausible enough to survive review. m_currentMessageId and m_currentThreadId are mutually exclusive and each clears the other, so a queued reply can tell which kind of selection it belongs to. onMessageLoaded carries a third guard onThreadLoaded does not need: a reply landing after the selection moved to a thread row would render one message where the conversation belongs. No mark-read timer for a message row in this pass. Marking one message of a thread read is a per-message tag write and the pending-edit map is keyed by thread; item 28 is the record of what happens when that count goes wrong. --- src/mainwindow.cpp | 52 +++++++++++++++++++++++++++++++++++++++++++++++++++ src/mainwindow.h | 9 +++++++++ src/notmuchworker.cpp | 48 +++++++++++++++++++++++++++++++++++++++++++++++ src/notmuchworker.h | 8 ++++++++ 4 files changed, 117 insertions(+) (limited to 'src') diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index d092f92..f125779 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -857,6 +857,7 @@ void MainWindow::registerActions() // thread straight back, which is the queued-reply race documented in // CLAUDE.md. m_currentThreadId.clear(); + m_currentMessageId.clear(); m_messageView->clear(); showPlaceholderPane(); m_markReadTimer->stop(); @@ -890,6 +891,7 @@ void MainWindow::registerActions() m_threadView->setCurrentIndex(QModelIndex()); m_currentThreadId.clear(); + m_currentMessageId.clear(); m_messageView->clear(); showPlaceholderPane(); m_markReadTimer->stop(); @@ -1257,6 +1259,8 @@ void MainWindow::wireWorker() this, &MainWindow::onQueryFinished); connect(m_worker, &NotmuchWorker::threadTreeLoaded, this, &MainWindow::onThreadTreeLoaded); + connect(m_worker, &NotmuchWorker::messageLoaded, + this, &MainWindow::onMessageLoaded); connect(m_worker, &NotmuchWorker::threadLoaded, this, &MainWindow::onThreadLoaded); connect(m_worker, &NotmuchWorker::errorOccurred, @@ -1580,6 +1584,7 @@ void MainWindow::onSelectionChanged() m_markReadTimer->stop(); m_markReadThreadId.clear(); m_currentThreadId.clear(); + m_currentMessageId.clear(); m_messageView->clear(); showPlaceholderPane(); } @@ -1611,12 +1616,39 @@ void MainWindow::onThreadSelected(const QModelIndex ¤t, m_markReadTimer->stop(); m_markReadThreadId.clear(); m_currentThreadId.clear(); + m_currentMessageId.clear(); m_messageView->clear(); showPlaceholderPane(); return; } + // A message row renders that message ALONE. Checked before threadAt(), + // which takes a top-level row number: a child's row number indexes its + // siblings, so passing it here would silently load whichever thread happens + // to sit at that position in the list. + if (m_model->isMessageRow(current)) { + const MessageNode node = m_model->messageAt(current); + if (node.messageId.isEmpty()) + return; + + // No mark-read timer for a message row in this pass. Marking one + // message of a thread read is a per-message tag write, and the + // pending-edit map is keyed by thread; item 28 is the record of what + // happens when that count goes wrong. + m_markReadTimer->stop(); + m_markReadThreadId.clear(); + + m_currentThreadId.clear(); + m_currentMessageId = node.messageId; + m_messageView->setTags(node.tags); + QMetaObject::invokeMethod(m_worker, "loadMessage", Qt::QueuedConnection, + Q_ARG(QString, node.messageId), + Q_ARG(quint64, m_generation)); + return; + } + const ThreadSummary thread = m_model->threadAt(current.row()); + m_currentMessageId.clear(); m_currentThreadId = thread.threadId; m_messageView->setTags(thread.tags); scheduleMarkRead(thread); @@ -1626,6 +1658,26 @@ void MainWindow::onThreadSelected(const QModelIndex ¤t, Q_ARG(quint64, m_generation)); } +void MainWindow::onMessageLoaded(const QVector &messages, + quint64 generation) +{ + // The same two guards onThreadLoaded carries. A stale generation means the + // query moved on, and a reply landing after the selection grew past one row + // would paint a message back over a deliberately blanked pane. + if (generation != m_generation || messages.isEmpty()) + return; + if (m_threadView->selectionModel()->selectedRows().size() > 1) + return; + + // A third guard this one needs and onThreadLoaded does not: a reply that + // lands after the selection moved to a THREAD row would render one message + // where the whole conversation belongs. + if (m_currentMessageId.isEmpty()) + return; + + onThreadLoaded(messages, generation); +} + void MainWindow::onThreadExpanded(const QModelIndex &index) { if (!index.isValid() || m_model->isMessageRow(index)) diff --git a/src/mainwindow.h b/src/mainwindow.h index fc980a0..5b7e39e 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -171,6 +171,10 @@ private slots: /// Fills in the expanded thread's message rows. void onThreadTreeLoaded(const QVector &nodes, quint64 generation); + + /// Renders the single message a message row asked for. + void onMessageLoaded(const QVector &messages, + quint64 generation); void onWorkerError(const QString &message); void onSyncFinished(bool success, int exitCode); @@ -503,6 +507,11 @@ private: QString m_lastQuery; QString m_currentThreadId; + /// The message a MESSAGE row is showing, empty whenever the pane holds a + /// whole thread. The two are mutually exclusive and each clears the other, + /// so a late reply can tell which kind of selection it belongs to. + QString m_currentMessageId; + /// The selection count last written to the status bar, so it can be taken /// back without clobbering a message some other action put there. QString m_selectionMessage; diff --git a/src/notmuchworker.cpp b/src/notmuchworker.cpp index 47bbb62..7b999cf 100644 --- a/src/notmuchworker.cpp +++ b/src/notmuchworker.cpp @@ -335,6 +335,54 @@ void NotmuchWorker::loadThreadTree(const QString &threadId, emit threadTreeLoaded(nodes, generation); } +void NotmuchWorker::loadMessage(const QString &messageId, quint64 generation) +{ + if (!openReadOnly()) + return; + + // id: is an exact-match prefix, and the id is quoted because a message id + // can legitimately contain characters notmuch's parser would otherwise read + // as query syntax. + const QString query = QStringLiteral("id:\"%1\"").arg(messageId); + NmQuery nmQuery(notmuch_query_create(m_db, query.toUtf8().constData())); + if (!nmQuery) { + emit errorOccurred( + QStringLiteral("Cannot load message %1").arg(messageId)); + return; + } + + notmuch_messages_t *rawMessages = nullptr; + if (notmuch_query_search_messages(nmQuery.get(), &rawMessages) + != NOTMUCH_STATUS_SUCCESS) { + emit errorOccurred( + QStringLiteral("Cannot search message %1").arg(messageId)); + return; + } + NmMessages messages(rawMessages); + + QVector result; + if (notmuch_messages_valid(messages.get())) { + NmMessage message(notmuch_messages_get(messages.get())); + if (message) { + MessageRef ref; + ref.messageId = QString::fromUtf8( + notmuch_message_get_message_id(message.get())); + ref.filePath = QString::fromUtf8( + notmuch_message_get_filename(message.get())); + ref.tags = tagsOf(message.get()); + + // Always matched: the user asked for this message by clicking its + // row, so rendering it as a stub would answer the wrong question. + ref.matched = true; + result.append(ref); + } + } + + // Emitted even when empty, so the UI's handler runs and can decide what to + // do rather than waiting for a reply that never comes. + emit messageLoaded(result, generation); +} + void NotmuchWorker::applyTagsToThreads(const QStringList &threadIds, const QStringList &add, const QStringList &remove, diff --git a/src/notmuchworker.h b/src/notmuchworker.h index 99cad04..df1799d 100644 --- a/src/notmuchworker.h +++ b/src/notmuchworker.h @@ -70,6 +70,13 @@ public slots: void loadThreadTree(const QString &threadId, const QString &matchQuery, quint64 generation); + /// Loads ONE message, for a message row selected in the list. + /// + /// Emits messageLoaded with an empty vector when the id is unknown, which + /// is an ordinary race after a reindex rather than an error worth + /// reporting. + void loadMessage(const QString &messageId, quint64 generation); + /// Applies tag changes. Opens the database read-write, applies, and closes /// immediately: notmuch's write lock is exclusive process-wide, so holding /// it would block the user's cron `notmuch new`. @@ -117,6 +124,7 @@ signals: void threadLoaded(const QVector &messages, quint64 generation); void threadTreeLoaded(const QVector &nodes, quint64 generation); + void messageLoaded(const QVector &messages, quint64 generation); void tagsApplied(const TagChange &change); void allTagsReady(const QStringList &tags, quint64 generation); -- cgit v1.2.3