aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-26 19:16:37 +0200
committerDanilo M. <danix@danix.xyz>2026-08-26 19:16:37 +0200
commit6ee94127510862139e89e8d653cd4994e27e5ffe (patch)
treee8fefe65eac56d126b78240e7dc5a1af1eddaf6d
parent06b0435830daaed49a2d5231dcb6f02ff0124d5d (diff)
downloadqtmaildir-6ee94127510862139e89e8d653cd4994e27e5ffe.tar.gz
qtmaildir-6ee94127510862139e89e8d653cd4994e27e5ffe.zip
feat: snapshot the pending changes as rows
Item 119, first half: the data the list behind the unsynced-changes count is built from, with no dialog and no worker, so the rules it has to follow are testable on their own. pendingChangeSnapshot() gathers the three queues the count sums into PendingChange rows. Two properties are the whole point. Scope follows the ACTION, not the storage. A held thread edit stays one thread row, because a `*_thread` action made it and reporting its messages instead would claim the user acted on each one; a netted tag edit and a held move are message rows. The queues already encode that distinction, so nothing is expanded and nothing is escalated. The rows are grouped by id, so a message with several outstanding actions appears once with its actions beneath it, which is the layout the user asked for. The sort is stable, so those actions keep the order they were made in; QHash has none of its own, and without it the list would reshuffle between openings. A snapshot, taken once and frozen. Subjects are empty here and filled by the resolve step to come. m_pendingTagEdits gains the action name beside the direction it already kept. The direction alone was enough to count with; a list has to say what each change was, and only the action that made it knows. It is carried from TagChange::description rather than derived from the tag, so there is no second table of tag names to labels to drift from the first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P88Q3MCSCSQxKDy7pmXh9F
-rw-r--r--src/mainwindow.cpp58
-rw-r--r--src/mainwindow.h42
-rw-r--r--src/types.h39
-rw-r--r--tests/test_mainwindow.cpp85
4 files changed, 215 insertions, 9 deletions
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index 59edf53..0c6092e 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -18,6 +18,8 @@
#include "mainwindow.h"
+#include <algorithm>
+
#include "maildirname.h"
#include <QAction>
@@ -4440,9 +4442,9 @@ void MainWindow::onTagsApplied(const TagChange &change)
// message are two independent changes and must not cancel each other.
for (const QString &messageId : change.messageIds) {
for (const QString &tag : change.added)
- recordPendingEdit(messageId, tag, true);
+ recordPendingEdit(messageId, tag, true, change.description);
for (const QString &tag : change.removed)
- recordPendingEdit(messageId, tag, false);
+ recordPendingEdit(messageId, tag, false, change.description);
}
updatePendingIndicator();
@@ -4989,7 +4991,7 @@ void MainWindow::runAutoSync()
}
void MainWindow::recordPendingEdit(const QString &messageId, const QString &tag,
- bool added)
+ bool added, const QString &action)
{
const QString key = messageId + QLatin1Char('\n') + tag;
@@ -4998,12 +5000,12 @@ void MainWindow::recordPendingEdit(const QString &messageId, const QString &tag,
// long session of tagging and untagging.
const auto existing = m_pendingTagEdits.constFind(key);
if (existing != m_pendingTagEdits.constEnd()) {
- if (*existing != added)
+ if (existing->added != added)
m_pendingTagEdits.erase(m_pendingTagEdits.find(key));
return;
}
- m_pendingTagEdits.insert(key, added);
+ m_pendingTagEdits.insert(key, PendingEdit{ added, action });
}
QStringList MainWindow::pendingSyncChannels() const
@@ -5060,6 +5062,52 @@ int MainWindow::pendingEditCount() const
return m_pendingTagEdits.size() + held + heldMoves;
}
+QVector<PendingChange> MainWindow::pendingChangeSnapshot() const
+{
+ QVector<PendingChange> rows;
+
+ // The netted per-(message, tag) edits. The key is `messageId\ntag`, built
+ // by recordPendingEdit(), so the id is everything before the first
+ // newline: a TAG may contain almost anything, but a message id cannot
+ // contain a newline and neither separator can be confused for the other.
+ for (auto it = m_pendingTagEdits.cbegin(); it != m_pendingTagEdits.cend();
+ ++it) {
+ const QString id = it.key().section(QLatin1Char('\n'), 0, 0);
+ rows.append(PendingChange{ id, false, it->action, QString(), -1 });
+ }
+
+ // Held THREAD edits, which stay thread-scoped: a `*_thread` action is what
+ // made them, and reporting the messages instead would claim the user acted
+ // on each one. One row per thread the edit named, since a single edit can
+ // cover a multi-row selection.
+ for (const HeldEdit &edit : m_heldEdits) {
+ for (const QString &threadId : edit.threadIds) {
+ rows.append(PendingChange{ threadId, true, edit.change.description,
+ QString(), -1 });
+ }
+ }
+
+ // Held MOVES, which are message-scoped. A move is not a tag change and is
+ // queued separately for that reason, but it is the same kind of row here:
+ // one message, one action the user took.
+ for (const HeldMove &move : m_heldMoves) {
+ for (const QString &messageId : move.messageIds)
+ rows.append(PendingChange{ messageId, false, move.description,
+ QString(), -1 });
+ }
+
+ // Grouped by id so a message with several outstanding actions appears
+ // ONCE with its actions beneath it, which is the layout the user asked
+ // for. A stable sort, so the actions under one message keep the order
+ // they were made in rather than an arbitrary one; QHash has no order of
+ // its own, so without this the list reshuffles between openings.
+ std::stable_sort(rows.begin(), rows.end(),
+ [](const PendingChange &a, const PendingChange &b) {
+ return a.id < b.id;
+ });
+ return rows;
+}
+
void MainWindow::updatePendingIndicator()
{
const int pending = pendingEditCount();
diff --git a/src/mainwindow.h b/src/mainwindow.h
index 25a9a7f..532a5ea 100644
--- a/src/mainwindow.h
+++ b/src/mainwindow.h
@@ -105,6 +105,24 @@ public:
/// the worker, which test_mainwindow has no database to drive.
bool hasEditAwaitingSend() const { return !m_heldEdits.isEmpty(); }
+ /// Every outstanding change, as rows, for the list behind the count.
+ ///
+ /// A SNAPSHOT: taken once when the user opens the list and never refreshed
+ /// under them. Subjects are empty here, filled by the resolve step, so
+ /// this is testable with no worker and no database.
+ ///
+ /// Scope follows the ACTION. The three queues already encode it: a held
+ /// thread edit carries thread ids because a `*_thread` action made it,
+ /// while a netted tag edit and a held move both carry message ids. Nothing
+ /// is expanded, and nothing is escalated.
+ /// Net changes the index holds that a sync has not carried over.
+ ///
+ /// Public beside pendingChangeSnapshot(), which must agree with it: the
+ /// count the user clicks is the count the list has to account for.
+ int pendingEditCount() const;
+
+ QVector<PendingChange> pendingChangeSnapshot() const;
+
/// Whether the undo stack still holds anything. Exposed so a test can show
/// that a rejected write did not take unrelated history down with it.
bool canUndo() const { return m_undoStack.canUndo(); }
@@ -853,11 +871,14 @@ private:
/// Records one confirmed (message, tag) change, cancelling it against an
/// opposite change already outstanding for the same pair.
+ ///
+ /// `action` is the name the user would recognise, carried through so the
+ /// list behind the count can say what each change was. It is the
+ /// TagChange's own description rather than anything derived from the tag.
void recordPendingEdit(const QString &messageId, const QString &tag,
- bool added);
+ bool added, const QString &action);
+
- /// Net changes the index holds that a sync has not carried over.
- int pendingEditCount() const;
/// Shows or hides the "syncing" state: the progress bar and a disabled
/// Sync button.
@@ -1539,7 +1560,20 @@ private:
/// value true for added and false for removed; a pair that reverts is
/// erased rather than stored, so an edit and its inverse leave nothing
/// behind and the map cannot grow without bound.
- QHash<QString, bool> m_pendingTagEdits;
+ /// What one pending (message, tag) edit is: its direction, and the name of
+ /// the action that made it.
+ ///
+ /// The direction alone was enough while this only had to be counted. The
+ /// list behind the count (item 119) has to SAY what each change was, and
+ /// only the action that made it knows: `+deleted` is a Delete and
+ /// `-unread` is a Mark read, but deriving that here would be a second
+ /// table of tag names to labels, drifting from the one the actions already
+ /// pass as TagChange::description.
+ struct PendingEdit {
+ bool added = false;
+ QString action; ///< Translated, from TagChange::description.
+ };
+ QHash<QString, PendingEdit> m_pendingTagEdits;
/// Marks the open thread read once it has been on screen long enough.
///
diff --git a/src/types.h b/src/types.h
index a0f772b..c78ab76 100644
--- a/src/types.h
+++ b/src/types.h
@@ -264,6 +264,45 @@ struct TagChange
}
};
+/// One outstanding change, for the list behind the unsynced-changes count.
+///
+/// A SNAPSHOT taken when the user opens the list, then frozen: the count they
+/// clicked is the count the list accounts for, and a dialog left open for
+/// twenty minutes must not keep rewriting itself under them.
+///
+/// The scope follows the ACTION, never the storage. A thread action names its
+/// thread and reports how many messages it covered at snapshot time; a
+/// message action names its message. That distinction is already kept, since a
+/// held thread edit carries thread ids and everything else carries message
+/// ids, so nothing has to be expanded to reconstruct it.
+struct PendingChange
+{
+ /// The message or thread this change is about. Wire format, for resolving
+ /// a subject; never shown.
+ QString id;
+
+ /// True when `id` is a THREAD id and the change covers the conversation.
+ bool isThread = false;
+
+ /// What the user did, translated and ready to show ("Delete", "Mark
+ /// read"). Built where the change is recorded, since only there is the
+ /// direction of a tag write still known.
+ QString action;
+
+ /// The subject, filled by the resolve step. Empty until then, and left
+ /// empty for an id the index no longer holds: the row still appears, since
+ /// dropping it would make the list disagree with the count.
+ QString subject;
+
+ /// How many messages a thread change covered, at snapshot time. -1 for a
+ /// message change and for a thread whose resolve found nothing.
+ ///
+ /// At snapshot time and not at write time: a held thread edit applies when
+ /// the sync ends, and a reply landing in between makes the real number
+ /// larger. The number describes what the user is looking at.
+ int messageCount = -1;
+};
+
/// Database-level facts for the Maildir overview.
///
/// Every field is -1 until answered, so a dialog opened against a database that
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index a120ce6..be19652 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -416,6 +416,8 @@ private slots:
void anEditUndoneNettsBackToZero();
void aDifferentTagOnTheSameMessageStillCounts();
void everyPendingChangeCanNameItsMessages();
+ void theSnapshotGroupsActionsUnderTheirMessage();
+ void theSnapshotKeepsAThreadActionThreadScoped();
void anEditDuringABackgroundSyncIsNotSentYet();
void aHeldEditIsSentWhenTheBackgroundSyncEnds();
void aHeldEditCountsAsUnsynced();
@@ -6489,6 +6491,89 @@ void TestMainWindow::everyPendingChangeCanNameItsMessages()
"an edit and its inverse left the indicator claiming work");
}
+void TestMainWindow::theSnapshotGroupsActionsUnderTheirMessage()
+{
+ // The layout the user asked for: a message appears ONCE with its actions
+ // beneath it. That is a property of the row ORDER, so it is asserted on
+ // the snapshot rather than on a rendered dialog.
+ const Config config;
+ MainWindow window(config);
+
+ // Two actions on one message, one on another, interleaved so a snapshot
+ // that simply reported insertion order would fail.
+ const auto apply = [&window](const QString &id, const QString &tag,
+ const QString &description) {
+ TagChange change;
+ change.messageIds = { id };
+ change.added = { tag };
+ change.description = description;
+ QVERIFY(QMetaObject::invokeMethod(&window, "onTagsApplied",
+ Q_ARG(TagChange, change)));
+ };
+ apply(QStringLiteral("b@example.org"), QStringLiteral("flagged"),
+ QStringLiteral("Mark important"));
+ apply(QStringLiteral("a@example.org"), QStringLiteral("deleted"),
+ QStringLiteral("Delete"));
+ apply(QStringLiteral("b@example.org"), QStringLiteral("spam"),
+ QStringLiteral("Mark spam"));
+
+ const QVector<PendingChange> rows = window.pendingChangeSnapshot();
+ QCOMPARE(rows.size(), 3);
+
+ // One message per contiguous run: b's two actions are adjacent, so the
+ // dialog can draw the subject once and the actions under it.
+ QCOMPARE(rows.at(0).id, QStringLiteral("a@example.org"));
+ QCOMPARE(rows.at(1).id, QStringLiteral("b@example.org"));
+ QCOMPARE(rows.at(2).id, QStringLiteral("b@example.org"));
+
+ // Each row says what the user did, in the words the action itself used.
+ QCOMPARE(rows.at(0).action, QStringLiteral("Delete"));
+ QVERIFY(rows.at(1).action != rows.at(2).action);
+
+ // And every row here is message-scoped: none of these was a thread action.
+ for (const PendingChange &row : rows)
+ QVERIFY(!row.isThread);
+}
+
+void TestMainWindow::theSnapshotKeepsAThreadActionThreadScoped()
+{
+ // Scope follows the ACTION, not the storage. A held thread edit stays one
+ // thread row: reporting its messages instead would claim the user acted on
+ // each one, and the count they clicked would disagree with the list.
+ //
+ // Driven through the held queue because that is the only thing that
+ // carries thread ids; a confirmed edit is message-scoped by construction.
+ const Config config;
+ MainWindow window(config);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ auto *view = window.findChild<QTreeView *>();
+ QVERIFY(model && view);
+ model->appendBatch({ makeThread(QStringLiteral("t1"), {}) });
+ selectThreadRow(view, 0);
+
+ // A cron sync takes the lock, which is what makes the edit HELD rather
+ // than sent, and a held edit is the only thing that carries thread ids.
+ QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged",
+ Q_ARG(SyncMonitor::State,
+ SyncMonitor::State::Running));
+
+ auto *action = window.findChild<QAction *>(QStringLiteral("flag_thread"));
+ QVERIFY(action);
+ action->trigger();
+ QVERIFY(window.hasEditAwaitingSend());
+
+ const QVector<PendingChange> rows = window.pendingChangeSnapshot();
+ QCOMPARE(rows.size(), 1);
+ QVERIFY2(rows.at(0).isThread,
+ "a thread action was reported as a message change");
+ QCOMPARE(rows.at(0).id, QStringLiteral("t1"));
+
+ // The count and the list agree, which is the property the whole dialog
+ // rests on.
+ QCOMPARE(rows.size(), window.pendingEditCount());
+}
+
// Item 37. A tag edit made while a background sync holds notmuch's write lock
// used to stall the worker: the read-write open BLOCKS until the lock frees
// (measured 9.158s against a 12s hold, returning NOTMUCH_STATUS_SUCCESS), so