From b01b44812ff29b2befc81cabd6ab61dc515c6da3 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Sat, 8 Aug 2026 10:37:48 +0200 Subject: feat(ui): load a thread's replies when its row is expanded Replies are fetched on expansion rather than with the query: walking the reply tree of every thread in a 10k-thread result would cost more than the query and almost none of it would be looked at. hasChildren is what makes that lazy loading work, and its absence would have shipped the feature unreachable. rowCount is 0 until the worker has walked the thread, so a view left to infer the expander from rowCount alone draws none, the user can never expand, and the replies are never requested. It answers from the summary's totalCount before loading and from the children afterwards, so a thread whose count included duplicates stops offering an expander that opens onto nothing. onThreadTreeLoaded reads the thread id from the reply rather than remembering it from the request. Two expansions can be in flight at once, and pairing them by order would attach one thread's replies to the other. --- src/mainwindow.cpp | 40 ++++++++++++++++++++++++++++++++++++++++ src/mainwindow.h | 7 +++++++ src/threadlistmodel.cpp | 29 +++++++++++++++++++++++++++++ src/threadlistmodel.h | 10 ++++++++++ tests/test_threadlistmodel.cpp | 41 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 127 insertions(+) diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 25b7a50..44feb0b 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -606,6 +606,12 @@ void MainWindow::buildUi() m_threadView->setColumnWidth(ThreadListModel::AuthorsColumn, 180); m_threadView->setColumnWidth(ThreadListModel::SubjectColumn, 520); + // Replies are loaded when a thread is expanded, not with the query. + // Walking the reply tree of every thread in a 10k-thread result would cost + // far more than the query itself and almost none of it would be looked at. + connect(m_threadView, &QTreeView::expanded, + this, &MainWindow::onThreadExpanded); + connect(m_threadView->selectionModel(), &QItemSelectionModel::currentRowChanged, this, &MainWindow::onThreadSelected); @@ -1234,6 +1240,8 @@ void MainWindow::wireWorker() this, &MainWindow::onThreadsReady); connect(m_worker, &NotmuchWorker::queryFinished, this, &MainWindow::onQueryFinished); + connect(m_worker, &NotmuchWorker::threadTreeLoaded, + this, &MainWindow::onThreadTreeLoaded); connect(m_worker, &NotmuchWorker::threadLoaded, this, &MainWindow::onThreadLoaded); connect(m_worker, &NotmuchWorker::errorOccurred, @@ -1603,6 +1611,38 @@ void MainWindow::onThreadSelected(const QModelIndex ¤t, Q_ARG(quint64, m_generation)); } +void MainWindow::onThreadExpanded(const QModelIndex &index) +{ + if (!index.isValid() || m_model->isMessageRow(index)) + return; + + const QString threadId = + m_model->data(index, ThreadListModel::ThreadIdRole).toString(); + if (threadId.isEmpty()) + return; + + QMetaObject::invokeMethod(m_worker, "loadThreadTree", Qt::QueuedConnection, + Q_ARG(QString, threadId), + Q_ARG(QString, m_lastQuery), + Q_ARG(quint64, m_generation)); +} + +void MainWindow::onThreadTreeLoaded(const QVector &nodes, + quint64 generation) +{ + // The same generation guard every other worker reply carries: an expansion + // whose query has since been replaced must not insert rows into the new + // result, where that thread may not even appear. + if (generation != m_generation || nodes.isEmpty()) + return; + + // Every node in one reply belongs to one thread, so the first one names it. + // Read from the node rather than remembered from the request: two + // expansions can be in flight at once, and pairing them by order would + // attach one thread's replies to the other. + m_model->setThreadMessages(nodes.first().threadId, nodes); +} + void MainWindow::onThreadLoaded(const QVector &messages, quint64 generation) { diff --git a/src/mainwindow.h b/src/mainwindow.h index 0c6865b..fc980a0 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -164,6 +164,13 @@ private slots: /// the click lands inside. void showThreadContextMenu(const QPoint &pos); void onThreadLoaded(const QVector &messages, quint64 generation); + + /// Asks the worker for a thread's reply tree when its row is expanded. + void onThreadExpanded(const QModelIndex &index); + + /// Fills in the expanded thread's message rows. + void onThreadTreeLoaded(const QVector &nodes, + quint64 generation); void onWorkerError(const QString &message); void onSyncFinished(bool success, int exitCode); diff --git a/src/threadlistmodel.cpp b/src/threadlistmodel.cpp index 0658b90..df083fc 100644 --- a/src/threadlistmodel.cpp +++ b/src/threadlistmodel.cpp @@ -151,6 +151,35 @@ int ThreadListModel::rowCount(const QModelIndex &parent) const return m_threads.at(parent.row()).children.size(); } +bool ThreadListModel::hasChildren(const QModelIndex &parent) const +{ + if (!parent.isValid()) + return !m_threads.isEmpty(); + + // A message row is always a leaf. Reply depth is drawn from the node's own + // depth, not from further nesting, so nothing hangs under a reply. + if (parent.parent().isValid()) + return false; + + if (parent.column() != 0) + return false; + + if (parent.row() < 0 || parent.row() >= m_threads.size()) + return false; + + const ThreadNode &node = m_threads.at(parent.row()); + + // Once loaded the children are the truth, including "there are none", which + // is how a thread whose totalCount counted duplicates stops offering an + // expander that opens onto nothing. + if (node.loaded) + return !node.children.isEmpty(); + + // Before loading, the summary's count is all there is. A thread of one + // message has no replies and must not offer an expander. + return node.summary.totalCount > 1; +} + int ThreadListModel::columnCount(const QModelIndex &parent) const { // Every level has the same columns. Returning 0 for a valid parent, as the diff --git a/src/threadlistmodel.h b/src/threadlistmodel.h index bc9fcbf..e39ade6 100644 --- a/src/threadlistmodel.h +++ b/src/threadlistmodel.h @@ -134,6 +134,16 @@ public: int rowCount(const QModelIndex &parent = {}) const override; int columnCount(const QModelIndex &parent = {}) const override; + + /// Whether a thread row should offer an expander. + /// + /// Answered from totalCount rather than from the loaded children, and that + /// is what makes lazy loading possible at all: rowCount is 0 until the + /// worker has walked the thread, so a view left to infer this from rowCount + /// alone draws no expander, the user can never expand, and the replies are + /// never asked for. The count is already in the summary, so this costs + /// nothing. + bool hasChildren(const QModelIndex &parent = {}) const override; QVariant data(const QModelIndex &index, int role) const override; QVariant headerData(int section, Qt::Orientation orientation, int role) const override; diff --git a/tests/test_threadlistmodel.cpp b/tests/test_threadlistmodel.cpp index 74b8dd1..3d131cf 100644 --- a/tests/test_threadlistmodel.cpp +++ b/tests/test_threadlistmodel.cpp @@ -32,6 +32,7 @@ private slots: void repliesBecomeChildRowsUnderTheirThread(); void messageRowsShowTheirOwnSenderAndSubject(); void reloadingAThreadReplacesItsRepliesRatherThanRepeatingThem(); + void anUnexpandedMultiMessageThreadOffersAnExpander(); void scopeFollowsTheSelectedRowKind(); void scopeCountsEveryMessageOfAnUnexpandedThread(); void scopeHonoursAMixedSelectionWithoutEscalating(); @@ -196,6 +197,46 @@ void TestThreadListModel::reloadingAThreadReplacesItsRepliesRatherThanRepeatingT Q_UNUSED(tester); } +void TestThreadListModel::anUnexpandedMultiMessageThreadOffersAnExpander() +{ + // This is what makes lazy loading work at all. rowCount is 0 until the + // worker has walked the thread, so a view inferring the expander from + // rowCount alone draws none, the user can never expand, and the replies are + // never requested. hasChildren answers from the summary's count instead. + ThreadListModel model; + ThreadSummary many = makeThread(QStringLiteral("t1"), + QStringLiteral("Has replies")); + many.totalCount = 4; + ThreadSummary lone = makeThread(QStringLiteral("t2"), + QStringLiteral("Single message")); + lone.totalCount = 1; + model.appendBatch({ many, lone }); + + const QModelIndex withReplies = model.index(0, 0, QModelIndex()); + const QModelIndex single = model.index(1, 0, QModelIndex()); + + // Guard: neither is expanded, so this really is the unloaded case. + QCOMPARE(model.rowCount(withReplies), 0); + QCOMPARE(model.rowCount(single), 0); + + QVERIFY(model.hasChildren(withReplies)); + QVERIFY(!model.hasChildren(single)); + + // Once loaded the children are the truth, including "there are none": a + // thread whose count included duplicates must stop offering an expander + // that opens onto nothing. + model.setThreadMessages(QStringLiteral("t1"), + { makeNode(QStringLiteral("m0@example.org"), 0) }); + QVERIFY(!model.hasChildren(withReplies)); + + // A message row is always a leaf. + model.setThreadMessages(QStringLiteral("t1"), + { makeNode(QStringLiteral("m0@example.org"), 0), + makeNode(QStringLiteral("m1@example.org"), 1) }); + QVERIFY(model.hasChildren(withReplies)); + QVERIFY(!model.hasChildren(model.index(0, 0, withReplies))); +} + void TestThreadListModel::scopeFollowsTheSelectedRowKind() { ThreadListModel model; -- cgit v1.2.3