aboutsummaryrefslogtreecommitdiffstats
path: root/src
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-28 17:56:25 +0200
committerDanilo M. <danix@danix.xyz>2026-08-28 17:56:25 +0200
commita346bef6c08e9725f3ab41f8237f01e9eb69eea7 (patch)
treeca3ea16c23da8971d6b9f8f7d4867845e5c25e0c /src
parentf934029d3333396bde6ec306ab300641929c7cdc (diff)
downloadqtmaildir-a346bef6c08e9725f3ab41f8237f01e9eb69eea7.tar.gz
qtmaildir-a346bef6c08e9725f3ab41f8237f01e9eb69eea7.zip
feat: judge a row's membership on the thread's union
Closes item 170 under item 177. A conversation belongs to a view while any of its messages match, so reading one message of a thread no longer takes the conversation out of the Unread view. The current row is never evicted, and an automatic write defers its eviction until the selection moves.
Diffstat (limited to 'src')
-rw-r--r--src/mainwindow.cpp179
-rw-r--r--src/mainwindow.h33
-rw-r--r--src/threadlistmodel.cpp54
-rw-r--r--src/threadlistmodel.h27
4 files changed, 293 insertions, 0 deletions
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index c8aa811..c781d77 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -3847,6 +3847,11 @@ void MainWindow::onSelectionChanged()
// 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).
+ //
+ // Before anything else reads the model: the user has moved off whatever
+ // row they were on, so a row held back by syncViewMembership() leaves now.
+ flushDeferredEviction();
+
refreshUnreadAction();
refreshScopedActionLabels();
refreshTrashActions();
@@ -5365,8 +5370,15 @@ void MainWindow::markCurrentThreadRead()
// It still funnels through the one applyTags path, per CLAUDE.md; what
// differs is only whether the inverse is pushed, which is a window-level
// decision above the worker.
+ //
+ // Flagged as AUTOMATIC for syncViewMembership(): the user did not ask for
+ // this write, so the row it changes must not be taken out from under them.
+ // A write they DID ask for evicts at once; the distinction is who
+ // initiated it, not what it does (item 177).
+ m_automaticWrite = true;
sendMessageTagChange(messageIds, {}, { QStringLiteral("unread") },
tr("Mark read"));
+ m_automaticWrite = false;
}
QString MainWindow::currentThreadFirstMessageId() const
@@ -5605,6 +5617,40 @@ void MainWindow::sendMessageTagChange(const QStringList &messageIds,
m_model->messageById(m_currentMessageId).tags);
}
+ // A row that no longer belongs in the view LEAVES it, rather than sitting
+ // there repainted until the next query. Beside the repaint above and
+ // before the sync hold below, since a held edit is applied optimistically
+ // too and its row is just as wrong to keep.
+ //
+ // The threads the touched messages belong to, with no filter on which
+ // message the card draws: a row is the conversation, so it is the UNION
+ // that decides, and the union is what removeThreadsWithoutTag() reads. A
+ // message the model does not hold names no thread, and is what the refresh
+ // inside the sync is for.
+ //
+ // Named only when this write MOVED the union, which is the one case a
+ // message edit can. applyMessageTagChange() keeps the summary in step for
+ // a thread of one, where the union IS the message, and deliberately leaves
+ // a longer thread's summary alone because one message's edit does not
+ // describe the conversation. Putting a longer thread up for eviction here
+ // would judge it on a union this write never touched: a stale answer, and
+ // wrong in both directions. A conversation leaves the view when a
+ // THREAD-scoped write empties its union, which is the other call site.
+ QStringList touchedThreads;
+ bool aRowIsMissing = false;
+ for (const QString &messageId : messageIds) {
+ const QString threadId = m_model->threadIdForMessage(messageId);
+ if (threadId.isEmpty()) {
+ aRowIsMissing = true;
+ continue;
+ }
+ if (m_model->threadCountFor(threadId) > 1)
+ continue;
+ if (!touchedThreads.contains(threadId))
+ touchedThreads.append(threadId);
+ }
+ syncViewMembership(touchedThreads, aRowIsMissing, add, remove);
+
// The accounts this touches, resolved through the containing threads: the
// account is a property of the thread, and the sync needs the channel
// whether one message moved or seven.
@@ -6488,6 +6534,130 @@ void MainWindow::sendMove(const QStringList &messageIds,
Q_ARG(QString, destFolder));
}
+void MainWindow::syncViewMembership(const QStringList &threadIds,
+ bool aRowIsMissing,
+ const QStringList &added,
+ const QStringList &removed)
+{
+ // Item 177. The optimistic REPAINT has always been universal; the
+ // optimistic MEMBERSHIP was not, and lived on the move path alone. So
+ // marking a thread read in the Unread view repainted its row and left it
+ // in a list defined by `tag:unread` that it no longer matched, until the
+ // next query or sync took it away.
+ //
+ // Membership is the UNION, one rule and no exceptions: a thread belongs
+ // to a view while any of its messages match it. The judgement itself is
+ // in removeThreadsWithoutTag(), which reads `summary.tags`; what happens
+ // here is only deciding WHICH rows to put to it and WHEN.
+ //
+ // Guarded on the VIEW's own tag, resolved from the query rather than
+ // assumed: a plain `tag:<x>` query is the only shape whose membership one
+ // tag decides. A path query (Trash, Sent, Drafts) is unaffected by a tag
+ // going away, and an arbitrary query the user typed cannot be reasoned
+ // about at all, so both are left alone and correct at the next sync.
+ // Without that guard, marking read in an `id:` view would empty the list.
+ //
+ // The exposure this accepts, deliberately and unchanged from the move
+ // path: revertPendingTagChange() repaints a rejected write but cannot
+ // REINSERT a row, so a write that fails leaves the row gone until the next
+ // query. Waiting for confirmation instead would give back exactly the lag
+ // this removes, and a rejected tag write is the rare case while the lag
+ // was every keystroke.
+ const QString viewTag = viewFilterTag();
+ if (viewTag.isEmpty())
+ return;
+
+ // The INVERSE, which the model cannot do on its own: a row that starts
+ // matching cannot be inserted optimistically, since the model holds no
+ // summary for a thread the query never returned. A refresh is what
+ // expresses it, exactly as the trash view already does after a restore.
+ // It matters most for UNDO: undoing a mark-read in the Unread view adds
+ // the tag back, and without this the row stayed gone, which would make an
+ // undone action invisible in the view it was undone in.
+ // refreshCurrentQuery() clears nothing, so the selection, the expansions
+ // and the undo stack all survive.
+ //
+ // Only when the row is genuinely ABSENT, which is the whole cost of the
+ // branch. Adding the view's tag to a row still in the list is the ordinary
+ // case, and refreshing there re-runs the query on every such keystroke.
+ if (added.contains(viewTag)) {
+ if (aRowIsMissing)
+ refreshCurrentQuery();
+ return;
+ }
+
+ if (!removed.contains(viewTag))
+ return;
+
+ // A row is never evicted while the user is sitting on it. The automatic
+ // mark-read fires two seconds after selection, so evicting on it takes the
+ // row out from under them, with a context menu possibly open on it, before
+ // they can mark it spam or important. The row leaves when the selection
+ // moves, which flushDeferredEviction() does, so the view still empties as
+ // they work. A write the user ASKED for evicts at once: the distinction is
+ // who initiated it, not what it does.
+ QStringList onScreen;
+ if (m_automaticWrite) {
+ const QModelIndexList selectedRows =
+ m_threadView->selectionModel()->selectedRows();
+ for (const QModelIndex &row : selectedRows)
+ onScreen.append(m_model->threadFor(row).threadId);
+ const QModelIndex current = m_threadView->currentIndex();
+ if (current.isValid())
+ onScreen.append(m_model->threadFor(current).threadId);
+ }
+
+ QStringList evictable;
+ for (const QString &threadId : threadIds) {
+ if (onScreen.contains(threadId)) {
+ if (!m_deferredEvictions.contains(threadId))
+ m_deferredEvictions.append(threadId);
+ continue;
+ }
+ evictable.append(threadId);
+ }
+
+ m_model->removeThreadsWithoutTag(evictable, viewTag);
+}
+
+void MainWindow::flushDeferredEviction()
+{
+ // The rows that stopped matching while the user was on them, taken out now
+ // that they have moved on. Re-checked against the model rather than
+ // trusted: the tag may have come back (an undo, a sync), in which case
+ // removeThreadsWithoutTag() correctly keeps the row.
+ if (m_deferredEvictions.isEmpty())
+ return;
+
+ const QString viewTag = viewFilterTag();
+ if (viewTag.isEmpty()) {
+ m_deferredEvictions.clear();
+ return;
+ }
+
+ // Every thread the user is on, from the SELECTION rather than from
+ // currentIndex(): a click calls select() before setCurrentIndex(), so at
+ // the moment selectionChanged arrives the current index still names the
+ // row being left, and reading it would hold the eviction back for ever.
+ QStringList onScreen;
+ const QModelIndexList rows =
+ m_threadView->selectionModel()->selectedRows();
+ for (const QModelIndex &row : rows)
+ onScreen.append(m_model->threadFor(row).threadId);
+
+ QStringList ready;
+ QStringList stillSelected;
+ for (const QString &threadId : m_deferredEvictions) {
+ if (onScreen.contains(threadId))
+ stillSelected.append(threadId);
+ else
+ ready.append(threadId);
+ }
+ m_deferredEvictions = stillSelected;
+
+ m_model->removeThreadsWithoutTag(ready, viewTag);
+}
+
QString MainWindow::viewFilterTag() const
{
const QString query = m_queryEdit->text().trimmed();
@@ -6658,6 +6828,15 @@ void MainWindow::sendThreadTagChange(const QStringList &threadIds,
m_messageView->setTags(m_model->threadFor(current).tags);
}
+ // Same as the message path: a thread whose union stops matching the view
+ // leaves it now rather than at the next query. A thread-scoped write moves
+ // the summary, which is what the membership judgement reads, so a thread
+ // marked read is emptied of `unread` in one step and correctly evicted.
+ bool aRowIsMissing = false;
+ for (const QString &threadId : threadIds)
+ aRowIsMissing = aRowIsMissing || !m_model->hasThread(threadId);
+ syncViewMembership(threadIds, aRowIsMissing, add, remove);
+
// A sync holds notmuch's exclusive write lock, and the worker's read-write
// open BLOCKS on it rather than failing: measured 9.158s against a 12s
// hold, returning SUCCESS. Sending now would freeze the worker thread for
diff --git a/src/mainwindow.h b/src/mainwindow.h
index fa413fe..0999ef3 100644
--- a/src/mainwindow.h
+++ b/src/mainwindow.h
@@ -235,6 +235,16 @@ public:
sendMessageTagChange(messageIds, add, remove, description);
}
+ /// Sends a thread-scoped tag change directly. The counterpart seam to
+ /// sendMessageTagChangeForTesting, for the same reason.
+ void sendThreadTagChangeForTesting(const QStringList &threadIds,
+ const QStringList &add,
+ const QStringList &remove,
+ const QString &description)
+ {
+ sendThreadTagChange(threadIds, add, remove, description);
+ }
+
/// The ids the last tag change was sent for, and whether they were thread
/// ids or message ids.
///
@@ -1191,6 +1201,29 @@ private:
/// firing in a view it cannot judge.
QString viewFilterTag() const;
+ /// Keeps the current view's membership in step with a write: drops a row
+ /// whose UNION has stopped matching, and refreshes when one starts
+ /// matching, which the model cannot express on its own. No-op unless the
+ /// view has a filter tag and the write touched it.
+ void syncViewMembership(const QStringList &threadIds,
+ bool aRowIsMissing,
+ const QStringList &added,
+ const QStringList &removed);
+
+ /// Removes the rows that stopped matching the view while the user was
+ /// sitting on them. Called when the selection moves.
+ void flushDeferredEviction();
+
+ /// Threads that have stopped matching the view but were selected when it
+ /// happened, so evicting them would have moved the list under the user.
+ QStringList m_deferredEvictions;
+
+ /// True only while an AUTOMATIC write is being sent. A write the user
+ /// asked for evicts its row at once; one a timer made must not, or the row
+ /// leaves under an open context menu two seconds after it was selected.
+ /// The distinction is who initiated it, not what it does (item 177).
+ bool m_automaticWrite = false;
+
void onMessagesMoved(const QMap<QString, QString> &originByMessageId,
const QString &destFolder);
diff --git a/src/threadlistmodel.cpp b/src/threadlistmodel.cpp
index fd4899b..0559694 100644
--- a/src/threadlistmodel.cpp
+++ b/src/threadlistmodel.cpp
@@ -206,6 +206,34 @@ void ThreadListModel::setTrashView(bool trash)
}
}
+void ThreadListModel::removeThreadsWithoutTag(const QStringList &threadIds,
+ const QString &tag)
+{
+ if (tag.isEmpty() || threadIds.isEmpty())
+ return;
+
+ // Backwards for the same reason the sweeping form below is: each removal
+ // renumbers everything after it.
+ for (int row = m_threads.size() - 1; row >= 0; --row) {
+ const ThreadNode &node = m_threads.at(row);
+ if (!threadIds.contains(node.summary.threadId))
+ continue;
+
+ // The SUMMARY, which is notmuch's union over the conversation, and
+ // never `first.tags`. A thread belongs to a view while ANY of its
+ // messages match it (item 177), so reading the message a 44-message
+ // card happens to draw must not evict the conversation while two of
+ // its replies are still unread. `first.tags` is right for what the
+ // card PAINTS and wrong for whether the row belongs here at all.
+ if (node.summary.tags.contains(tag))
+ continue;
+
+ beginRemoveRows({}, row, row);
+ m_threads.remove(row);
+ endRemoveRows();
+ }
+}
+
void ThreadListModel::removeThreadsWithoutTag(const QString &tag)
{
if (tag.isEmpty() || m_threads.isEmpty())
@@ -995,6 +1023,15 @@ MessageNode ThreadListModel::messageAt(const QModelIndex &index) const
QString ThreadListModel::threadIdForMessage(const QString &messageId) const
{
for (const ThreadNode &node : m_threads) {
+ // The ROOT first. A thread's first message is not among its children
+ // (item 109: setThreadMessages drops depth 0 because the root row
+ // stands for it), so a search over children alone answers "no thread"
+ // for every thread row's own message, which is the id an ordinary tag
+ // action resolves to since item 108.
+ if (node.first.messageId == messageId
+ || node.summary.firstMessageId == messageId) {
+ return node.summary.threadId;
+ }
for (const MessageNode &child : node.children) {
if (child.messageId == messageId)
return node.summary.threadId;
@@ -1003,6 +1040,23 @@ QString ThreadListModel::threadIdForMessage(const QString &messageId) const
return {};
}
+bool ThreadListModel::hasThread(const QString &threadId) const
+{
+ return std::any_of(m_threads.cbegin(), m_threads.cend(),
+ [&threadId](const ThreadNode &node) {
+ return node.summary.threadId == threadId;
+ });
+}
+
+int ThreadListModel::threadCountFor(const QString &threadId) const
+{
+ for (const ThreadNode &node : m_threads) {
+ if (node.summary.threadId == threadId)
+ return node.summary.totalCount;
+ }
+ return 0;
+}
+
MessageNode ThreadListModel::messageById(const QString &messageId) const
{
if (messageId.isEmpty())
diff --git a/src/threadlistmodel.h b/src/threadlistmodel.h
index a528c7e..93474ac 100644
--- a/src/threadlistmodel.h
+++ b/src/threadlistmodel.h
@@ -236,6 +236,21 @@ public:
/// CURRENT VIEW requires, so a caller passes what the query filters on and
/// nothing else.
void removeThreadsWithoutTag(const QString &tag);
+
+ /// Drops only the NAMED threads, and only those whose UNION no longer
+ /// carries \p tag.
+ ///
+ /// Two differences from the sweeping form, both deliberate. It judges the
+ /// named rows alone, because the sweeping form is right after a move (the
+ /// query itself is what changed) and wrong after a tag write: a list can
+ /// legitimately hold rows that never matched, and one write must not evict
+ /// rows it did not touch. And it judges `summary.tags`, which is notmuch's
+ /// union over the conversation, never the displayed message's own tags: a
+ /// thread belongs to a view while ANY of its messages match it (item 177),
+ /// so reading one message of a 44-message thread leaves the conversation
+ /// in the Unread view and reading the last one takes it out.
+ void removeThreadsWithoutTag(const QStringList &threadIds,
+ const QString &tag);
bool flatMode() const { return m_flatMode; }
QModelIndex index(int row, int column,
@@ -326,6 +341,18 @@ public:
/// message the user could select is always findable here.
QString threadIdForMessage(const QString &messageId) const;
+ /// Whether a row for this thread is still in the list.
+ bool hasThread(const QString &threadId) const;
+
+ /// How many messages the thread holds, from its summary, or 0 when the
+ /// list has no row for it.
+ ///
+ /// The question a message-scoped write asks before judging membership: a
+ /// thread of one has a union that IS its message, so the write moved it
+ /// and the row can be judged, while a longer thread's union is untouched
+ /// and judging it would be judging a stale answer.
+ int threadCountFor(const QString &threadId) const;
+
/// A loaded message row's node, found by id rather than by position.
///
/// For callers that know WHICH message they mean and must not depend on it