aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/mainwindow.cpp52
-rw-r--r--src/mainwindow.h9
-rw-r--r--src/notmuchworker.cpp48
-rw-r--r--src/notmuchworker.h8
-rw-r--r--tests/test_mainwindow.cpp72
-rw-r--r--tests/test_notmuchworker.cpp38
6 files changed, 227 insertions, 0 deletions
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index 8bab2e8..d9aa57a 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -876,6 +876,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();
@@ -909,6 +910,7 @@ void MainWindow::registerActions()
m_threadView->setCurrentIndex(QModelIndex());
m_currentThreadId.clear();
+ m_currentMessageId.clear();
m_messageView->clear();
showPlaceholderPane();
m_markReadTimer->stop();
@@ -1321,6 +1323,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,
@@ -1644,6 +1648,7 @@ void MainWindow::onSelectionChanged()
m_markReadTimer->stop();
m_markReadThreadId.clear();
m_currentThreadId.clear();
+ m_currentMessageId.clear();
m_messageView->clear();
showPlaceholderPane();
}
@@ -1675,12 +1680,39 @@ void MainWindow::onThreadSelected(const QModelIndex &current,
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);
@@ -1690,6 +1722,26 @@ void MainWindow::onThreadSelected(const QModelIndex &current,
Q_ARG(quint64, m_generation));
}
+void MainWindow::onMessageLoaded(const QVector<MessageRef> &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 7014855..a4eca20 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<MessageNode> &nodes,
quint64 generation);
+
+ /// Renders the single message a message row asked for.
+ void onMessageLoaded(const QVector<MessageRef> &messages,
+ quint64 generation);
void onWorkerError(const QString &message);
void onSyncFinished(bool success, int exitCode);
@@ -514,6 +518,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<MessageRef> 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<MessageRef> &messages, quint64 generation);
void threadTreeLoaded(const QVector<MessageNode> &nodes,
quint64 generation);
+ void messageLoaded(const QVector<MessageRef> &messages, quint64 generation);
void tagsApplied(const TagChange &change);
void allTagsReady(const QStringList &tags, quint64 generation);
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index cee32c6..3a375d4 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -101,6 +101,7 @@ private slots:
void noTagStripIsPaintedUnderAMessageRow();
void replyRowsKeepTheirTextUnderTheThreadLine();
void clickingTheExpanderTogglesTheThread();
+ void selectingAMessageRowTargetsThatMessageNotItsThread();
void markAllReadIsDisabledUntilTheQueryFinishes();
void markAllReadActsOnEveryRowAndUndoesInOneStep();
void markAllReadDoesNothingWhenNothingIsUnread();
@@ -800,6 +801,77 @@ void TestMainWindow::aThreadWithRepliesDrawsAVisibleExpander()
QCOMPARE(control, 0);
}
+void TestMainWindow::selectingAMessageRowTargetsThatMessageNotItsThread()
+{
+ // test_mainwindow has no worker (backlog item 36), so this cannot assert on
+ // what the pane renders. What it CAN assert is the decision the UI makes:
+ // a message row must stop tracking a current thread, or a reply arriving
+ // for either kind of selection cannot tell which one it belongs to.
+ //
+ // The trap this covers is specific. 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 in the list. Row 0 under a thread is a plausible-looking wrong
+ // answer, which is why the fixture puts the reply under the SECOND thread.
+ const Config config;
+ MainWindow window(config);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<QTreeView *>();
+ QVERIFY(view);
+
+ ThreadSummary first = makeThread(QStringLiteral("t1"), {});
+ ThreadSummary second = makeThread(QStringLiteral("t2"), {});
+ second.totalCount = 2;
+ model->appendBatch({ first, second });
+
+ MessageNode root;
+ root.messageId = QStringLiteral("m0@example.org");
+ root.threadId = QStringLiteral("t2");
+ root.depth = 0;
+ MessageNode reply;
+ reply.messageId = QStringLiteral("m1@example.org");
+ reply.threadId = QStringLiteral("t2");
+ reply.depth = 1;
+ model->setThreadMessages(QStringLiteral("t2"), { root, reply });
+
+ window.resize(1400, 300);
+ window.show();
+ QVERIFY(QTest::qWaitForWindowExposed(&window));
+
+ // Start on a thread row, so the transition to a message row is what is
+ // being observed rather than the initial state.
+ const QModelIndex threadRow = model->index(1, 0, QModelIndex());
+ selectThreadRow(view, 1);
+ QApplication::processEvents();
+ QCOMPARE(window.currentThreadId(), QStringLiteral("t2"));
+
+ view->expand(threadRow);
+ QApplication::processEvents();
+
+ const QModelIndex messageRow = model->index(0, 0, threadRow);
+ QVERIFY(messageRow.isValid());
+ QVERIFY2(model->isMessageRow(messageRow),
+ "the fixture did not produce a message row, so this test would "
+ "assert nothing about one");
+
+ view->selectionModel()->select(
+ messageRow,
+ QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows);
+ view->setCurrentIndex(messageRow);
+ QApplication::processEvents();
+
+ // The thread is no longer what the pane is about. Left set, a late
+ // loadThread reply would repaint the whole conversation over the single
+ // message the user asked for.
+ QVERIFY2(window.currentThreadId().isEmpty(),
+ qPrintable(QStringLiteral("selecting a reply left the current "
+ "thread set to '%1': the pane is still "
+ "tracking the conversation")
+ .arg(window.currentThreadId())));
+}
+
void TestMainWindow::clickingTheExpanderTogglesTheThread()
{
// The glyph being VISIBLE and the glyph being CLICKABLE are separate
diff --git a/tests/test_notmuchworker.cpp b/tests/test_notmuchworker.cpp
index 0f47c55..c84e262 100644
--- a/tests/test_notmuchworker.cpp
+++ b/tests/test_notmuchworker.cpp
@@ -58,6 +58,8 @@ private slots:
void requestAllTagsReturnsSortedTags();
void requestAllTagsOnUnreadableConfigEmitsError();
+ void loadMessageReturnsOnlyThatMessage();
+ void loadMessageOnAnUnknownIdReturnsNothing();
void loadThreadTreeReportsReplyDepth();
void loadThreadTreeCarriesTheFactsARowNeeds();
@@ -161,6 +163,42 @@ QStringList TestNotmuchWorker::tagsOf(const QString &messageId)
return {};
}
+void TestNotmuchWorker::loadMessageReturnsOnlyThatMessage()
+{
+ // a2 is a reply in a two-message thread. Selecting a reply row must render
+ // that message alone; loadThread would hand back the whole thread and the
+ // pane would show the conversation the user was trying to look inside.
+ NotmuchWorker worker(m_fixture.configPath());
+ QSignalSpy loaded(&worker, &NotmuchWorker::messageLoaded);
+ worker.loadMessage(QStringLiteral("a2@example.org"), 1);
+
+ QCOMPARE(loaded.count(), 1);
+ const auto messages = loaded.first().at(0).value<QVector<MessageRef>>();
+
+ QCOMPARE(messages.size(), 1);
+ QCOMPARE(messages.first().messageId, QStringLiteral("a2@example.org"));
+ QVERIFY(!messages.first().filePath.isEmpty());
+
+ // matched, so the pane renders it expanded rather than as a stub. The user
+ // asked for this message by clicking it, which is as matched as it gets.
+ QVERIFY(messages.first().matched);
+}
+
+void TestNotmuchWorker::loadMessageOnAnUnknownIdReturnsNothing()
+{
+ // Empty rather than an error: a stale row after a reindex is an ordinary
+ // race, not a failure worth a message in the status bar.
+ NotmuchWorker worker(m_fixture.configPath());
+ QSignalSpy loaded(&worker, &NotmuchWorker::messageLoaded);
+ QSignalSpy errors(&worker, &NotmuchWorker::errorOccurred);
+
+ worker.loadMessage(QStringLiteral("nonexistent@example.org"), 1);
+
+ QCOMPARE(loaded.count(), 1);
+ QVERIFY(loaded.first().at(0).value<QVector<MessageRef>>().isEmpty());
+ QCOMPARE(errors.count(), 0);
+}
+
void TestNotmuchWorker::loadThreadTreeReportsReplyDepth()
{
// Thread A is a root plus one reply carrying In-Reply-To, which is what