diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-07 12:03:14 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-07 12:03:14 +0200 |
| commit | 5a4d8f5f021dc98b2a7cc471125aa3040c02675c (patch) | |
| tree | 181eccf726124f39ae6a1c14734cfa18ac42ed9f | |
| parent | 0a9ef3c77c7c593f3568f25761aad5d1f55e0e33 (diff) | |
| download | qtmaildir-5a4d8f5f021dc98b2a7cc471125aa3040c02675c.tar.gz qtmaildir-5a4d8f5f021dc98b2a7cc471125aa3040c02675c.zip | |
feat(tags): mark every thread in the view read, in one undoable step
An action removing "unread" from every thread in the current view,
on the toolbar, the Message menu and Ctrl+Shift+U. It deliberately
ignores the selection, which makes it the one action in the window
that does, and it routes through the same funnel as every other tag
change, so it is one write rather than one per thread.
Disabled until the query reports its total. Threads arrive in batches,
so before then the model holds only what has landed, and an action
saying "all" must not silently skip the rest. A greyed control says
"not yet" without needing a dialog or a stall the user cannot see.
The state is also set at registration, since QAction starts enabled
and a window that has not run a query has nothing to act on.
Two things came out differently from the plan, both forced by existing
code. It carries a default binding, because everyActionHasAShortcut
requires every registered action to have one: an unbound action is
unreachable from the keyboard, and that invariant is deliberate, so the
action was given Ctrl+Shift+U rather than the invariant relaxed. And
only the threads that are actually unread are sent, because sending the
rest would inflate the pending-edit count with writes that change
nothing, and the quit prompt reads that count. A view with nothing
unread does nothing, pushes no command and says so: an undo entry that
restores nothing is worse than none, since it absorbs a Ctrl+Z meant
for the previous action.
undoDepthForTesting() is new and exists for a reason worth recording:
undo->isEnabled() cannot answer "was a command pushed", because the
undo QAction is always enabled and tests canUndo() when triggered. The
first version of the no-op test asserted on it and passed against a
mutant with the unread filter removed.
Closes item 43.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| -rw-r--r-- | CHANGELOG.md | 33 | ||||
| -rw-r--r-- | README.md | 1 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 30 | ||||
| -rw-r--r-- | src/keymap.cpp | 5 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 74 | ||||
| -rw-r--r-- | src/mainwindow.h | 26 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 143 |
7 files changed, 311 insertions, 1 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fb7650..94ae6c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,39 @@ point at which they are stable. ## [Unreleased] +### Added + +- **Mark all read**, on the toolbar, the Message menu and `Ctrl+Shift+U`. It + acts on every thread in the current view rather than the selection, as one + write and one undo entry, so a single `Ctrl+Z` puts back a view of 400 + threads. It stays disabled until the query has reported its total: threads + arrive in batches, and an action that says "all" must not run against + whatever happened to have loaded. A view with nothing unread does nothing and + says so, rather than pushing an undo entry that restores nothing. +- **The status bar says which account is syncing**, then that notmuch is + reindexing, instead of "Syncing..." for the whole run. The account name and + the progress both come from mbsync's own output as it streams. + +### Changed + +- **The sync script runs `mbsync -V`.** Without it mbsync prints nothing at all + until it exits, then a single summary line, so a run of over a minute was + silent and there was nothing for the status bar to report. This is not a + buffering problem and `stdbuf` does not help. + +### Fixed + +- **A message whose HTML body carries a `Content-Id` renders**, instead of + opening blank with the app reporting no HTML part. A content id makes a part + referenceable, not undisplayable, and setting one on the body is legal and + common in bulk-sender output. +- **Removing a tag suggests only the tags the selected threads carry**, rather + than every tag in the database. Adding still reaches the whole vocabulary, + since naming a tag that does not exist yet is what that field is for. +- **A sync that finishes quickly no longer loses its own progress.** The + per-run reset happened after the process launched, so a run that delivered + its output before control returned wiped the state those lines had produced. + ## [0.10.0] - 2026-08-06 Tag edits no longer stall the window when a background sync is running, and @@ -376,6 +376,7 @@ Defaults, all rebindable through `[keys]`: | `Ctrl+D` | `delete` | Add `deleted` | | `Ctrl+Shift+S` | `spam` | Add `spam`, remove `inbox` | | `Ctrl+U` | `toggle_unread` | Toggle `unread` | +| `Ctrl+Shift+U` | `mark_all_read` | Remove `unread` from every thread in the view | | `Ctrl+I` | `flag` | Add `flagged` | | `Ctrl+L` | `focus_query` | Focus and select the query bar | | `Ctrl+Space` | `complete_query` | Focus the query bar and offer completions | diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md index 9424ea4..2bcdd0a 100644 --- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md +++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md @@ -90,7 +90,7 @@ taking that too literally. | 40 | No live filter over the current view | workflow | M | open | | 41 | A message whose HTML body carries a `Content-Id` renders blank | correctness | S | **done** | | 42 | "Syncing..." says nothing about what is being synced | feedback | S | **done** | -| 43 | No "Mark all read" for the current view | workflow | S | open | +| 43 | No "Mark all read" for the current view | workflow | S | **done** | | 44 | No way to manage the filters applied at sync time | workflow | ? | open, unspecified | | 45 | Two Sync buttons, and only one of them works properly | correctness | S | **done** | | 46 | `uiStateSurvivesARestart` fails under the offscreen platform | testing | XS | **done** | @@ -2192,6 +2192,34 @@ query, per `CLAUDE.md`, so marking many threads read is one write, not N. - The pending-edit count must move by the real number of threads changed, or the quit prompt understates the work at risk. +**Resolved (2026-08-07).** The scope question above was decided by the user: +the action is **disabled until the query reports its total**, rather than +acting on a partial set or stalling on a wait. `m_queryComplete` gates it, +cleared in `runCurrentQuery()` and set in `onQueryFinished()`. A greyed control +says "not yet" without a dialog, and the honesty constraint is satisfied by +construction rather than by wording. + +Two things came out differently from the plan, both forced by existing code. + +- **It carries a default binding, `Ctrl+Shift+U`**, shifted against `Ctrl+U` + for `toggle_unread`. The intent was toolbar and menu only, but + `everyActionHasAShortcut` requires every registered action to have one: an + unbound action is unreachable from the keyboard, and an empty shortcut means + the action list and the default table have drifted apart. The invariant is + deliberate, so the action was given a binding rather than the invariant being + relaxed. +- **Only the threads that are actually unread are sent.** Sending every row + would inflate the pending-edit count with writes that change nothing, and the + quit prompt reads that count. A view with nothing unread does nothing at all, + pushes no command, and says so: an undo entry that restores nothing is worse + than none, since it absorbs a Ctrl+Z meant for the previous action. + +**A test-seam note worth keeping.** `undo->isEnabled()` cannot answer "was a +command pushed": the undo `QAction` is always enabled and tests `canUndo()` +when triggered. A first version of the no-op test asserted on it and passed +against a mutant with the unread filter removed. `undoDepthForTesting()` exists +because of that, and the mutation is caught now. + ## 44. No way to manage the filters applied at sync time **Observed (user, 2026-08-05):** "manage filters to be applied when syncing (view diff --git a/src/keymap.cpp b/src/keymap.cpp index f27f9a9..a16a700 100644 --- a/src/keymap.cpp +++ b/src/keymap.cpp @@ -31,6 +31,7 @@ QStringList KeyMap::knownActions() QStringLiteral("delete"), QStringLiteral("spam"), QStringLiteral("toggle_unread"), + QStringLiteral("mark_all_read"), QStringLiteral("edit_tags"), QStringLiteral("flag"), QStringLiteral("focus_query"), @@ -67,6 +68,10 @@ QList<QPair<QString, QString>> KeyMap::defaultBindings() { QStringLiteral("Ctrl+D"), QStringLiteral("delete") }, { QStringLiteral("Ctrl+Shift+S"), QStringLiteral("spam") }, { QStringLiteral("Ctrl+U"), QStringLiteral("toggle_unread") }, + // Shifted against Ctrl+U, which toggles unread on the selection: this + // is the same idea applied to the whole view, and the wider-reaching + // action takes the harder chord rather than the easier one. + { QStringLiteral("Ctrl+Shift+U"), QStringLiteral("mark_all_read") }, { QStringLiteral("Ctrl+I"), QStringLiteral("flag") }, { QStringLiteral("Ctrl+T"), QStringLiteral("edit_tags") }, { QStringLiteral("Ctrl+L"), QStringLiteral("focus_query") }, diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index a3b5496..bb11261 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -677,6 +677,11 @@ void MainWindow::registerActions() else tagSelected({ QStringLiteral("unread") }, {}, tr("Mark unread")); }); + addAction(QStringLiteral("mark_all_read"), tr("Mark all &read"), + tr("Remove the unread tag from every thread in this view"), + [this]() { + markAllRead(); + }); addAction(QStringLiteral("edit_tags"), tr("Edit &tags..."), tr("Add or remove any tag on the selected threads"), [this]() { editTagsOnSelection(); @@ -768,6 +773,13 @@ void MainWindow::registerActions() // silently dead. KeyMap warns about unknown names, but only a check here // catches the reverse: a known action nothing implements. Q_ASSERT(m_actions.size() == KeyMap::knownActions().size()); + + // QAction starts enabled, so the view-wide actions have to be put into + // their real state here rather than waiting for the first query: a window + // that has not run one yet has an empty model and no complete result set, + // and offering "Mark all read" against nothing is a live control that does + // nothing. + updateViewWideActions(); } void MainWindow::buildMenus() @@ -791,6 +803,7 @@ void MainWindow::buildMenus() messageMenu->addAction(m_actions.value(QStringLiteral("spam"))); messageMenu->addSeparator(); messageMenu->addAction(m_actions.value(QStringLiteral("toggle_unread"))); + messageMenu->addAction(m_actions.value(QStringLiteral("mark_all_read"))); messageMenu->addAction(m_actions.value(QStringLiteral("edit_tags"))); messageMenu->addAction(m_actions.value(QStringLiteral("flag"))); @@ -876,6 +889,7 @@ void MainWindow::buildMenus() toolBar->addSeparator(); toolBar->addAction(m_actions.value(QStringLiteral("archive"))); toolBar->addAction(m_actions.value(QStringLiteral("delete"))); + toolBar->addAction(m_actions.value(QStringLiteral("mark_all_read"))); toolBar->addSeparator(); toolBar->addAction(m_actions.value(QStringLiteral("undo"))); } @@ -1102,6 +1116,11 @@ void MainWindow::runCurrentQuery() m_statusLabel->setText(tr("Searching...")); + // The result set is incomplete from here until queryFinished arrives, so + // anything claiming to act on the whole view must wait. + m_queryComplete = false; + updateViewWideActions(); + QMetaObject::invokeMethod(m_worker, "runQuery", Qt::QueuedConnection, Q_ARG(QString, query), Q_ARG(quint64, m_generation)); @@ -1124,6 +1143,61 @@ void MainWindow::onQueryFinished(int total, quint64 generation) // nothing. m_defaultStatus = tr("%n thread(s)", "", total); m_statusLabel->setText(m_defaultStatus); + + // The model now holds every row the query matched, so "the whole view" is + // a thing that can honestly be acted on. + m_queryComplete = true; + updateViewWideActions(); +} + +void MainWindow::updateViewWideActions() +{ + // Threads arrive in batches of kBatchSize, so before the query reports its + // total the model holds only what has landed. An action that says "all" + // must not run against a partial set and silently skip the rest, and a + // disabled control says so without a dialog. + if (QAction *action = m_actions.value(QStringLiteral("mark_all_read"))) + action->setEnabled(m_queryComplete && m_model->rowCount() > 0); +} + +void MainWindow::markAllRead() +{ + // Every row, not the selection: this is the one action in the window that + // deliberately ignores what is selected. + QStringList threadIds; + const int rows = m_model->rowCount(); + threadIds.reserve(rows); + for (int row = 0; row < rows; ++row) { + const ThreadSummary thread = m_model->threadAt(row); + // Only the threads that would actually change. Sending the rest would + // inflate the pending-edit count with writes that do nothing, and the + // quit prompt reads that count. + if (thread.isUnread()) + threadIds.append(thread.threadId); + } + + if (threadIds.isEmpty()) { + showTransientStatus(tr("Nothing unread in this view")); + return; + } + + // An automatic mark-read armed for the open thread would fire after this + // and push a second, redundant command onto the stack. + m_markReadTimer->stop(); + m_markReadThreadId.clear(); + + const QString description = tr("Mark all read"); + sendThreadTagChange(threadIds, {}, { QStringLiteral("unread") }, + description); + + // ONE command for the batch, exactly as tagSelected does: a user who marks + // 400 threads read expects a single Ctrl+Z to put them back. + m_undoStack.push(new ThreadTagCommand(this, threadIds, {}, + { QStringLiteral("unread") }, + description)); + + showTransientStatus( + tr("%1: %n thread(s)", "", threadIds.size()).arg(description)); } void MainWindow::showThreadContextMenu(const QPoint &pos) diff --git a/src/mainwindow.h b/src/mainwindow.h index 3984e78..adb973c 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -119,6 +119,19 @@ public: static void setLocksPathForTesting(const QString &path); static QString locksPath(); + /// How many commands are on the undo stack. + /// + /// A test seam. The undo QAction is always enabled and checks canUndo() + /// when triggered, so its enabled state says nothing about whether a + /// command was pushed, which is what "this did nothing" has to assert. + int undoDepthForTesting() const { return m_undoStack.count(); } + + /// The generation a worker reply must carry to be accepted. + /// + /// A test seam: onQueryFinished() discards a reply whose generation is + /// stale, so a test standing in for the worker has to know the current one. + quint64 currentGenerationForTesting() const { return m_generation; } + protected: void closeEvent(QCloseEvent *event) override; @@ -232,6 +245,14 @@ private: /// when the phase or its detail changes. void feedSyncPhase(const QString &chunk); + /// Removes `unread` from every thread in the current view, as one write and + /// one undo entry, ignoring the selection. + void markAllRead(); + + /// Enables or disables the actions that claim to act on a whole view, + /// according to whether the result set is complete. + void updateViewWideActions(); + /// Applies the sync progress bar and button state from BOTH sync sources. /// /// One function of both, never two assignments: with a local and a @@ -383,6 +404,11 @@ private: QStringList m_knownTags; quint64 m_generation = 0; + + /// True once the running query has reported its total, so the model holds + /// the whole result set rather than the batches that have arrived so far. + /// Gates mark_all_read, which cannot honestly say "all" before then. + bool m_queryComplete = false; QString m_lastQuery; QString m_currentThreadId; diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 02490a5..1fdeaf2 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -81,6 +81,9 @@ private slots: void aSkippedLocalSyncStillReportsTheOtherRunFinishing(); void anUnobservableLockTableLeavesTheSyncButtonUsable(); void theStatusBarFollowsTheSyncPhase(); + void markAllReadIsDisabledUntilTheQueryFinishes(); + void markAllReadActsOnEveryRowAndUndoesInOneStep(); + void markAllReadDoesNothingWhenNothingIsUnread(); void theSyncActionIsDisabledWhileABackgroundSyncHoldsTheLock(); void escapeBlanksTheMessagePane(); void deleteTogglesOnAnAlreadyDeletedThread(); @@ -367,6 +370,146 @@ static ThreadSummary makeThread(const QString &id, const QStringList &tags) return thread; } +void TestMainWindow::markAllReadIsDisabledUntilTheQueryFinishes() +{ + // Threads arrive in batches, so acting mid-load would silently skip + // whatever had not arrived. Rather than acting on part of the view and + // calling it "all", or stalling on a wait the user cannot see, the action + // is simply unavailable until the result set is complete. + const Config config; + MainWindow window(config); + + auto *action = window.findChild<QAction *>(QStringLiteral("mark_all_read")); + QVERIFY2(action, "no mark_all_read action registered"); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + + // A query in flight: rows are arriving but the worker has not said it is + // done, so the action must stay out of reach. + // A query is needed for runCurrentQuery to do anything: it returns early + // on an empty one, which would leave the flag untouched. + window.findChild<QLineEdit *>()->setText(QStringLiteral("tag:inbox")); + QMetaObject::invokeMethod(&window, "runCurrentQuery"); + model->appendBatch({ makeThread(QStringLiteral("t1"), + { QStringLiteral("unread") }) }); + QVERIFY2(!action->isEnabled(), + "the action was live while the query was still loading"); + + // The generation must match or the reply is discarded as stale, which is + // how a superseded query is ignored everywhere else in this window. + const quint64 generation = window.currentGenerationForTesting(); + QMetaObject::invokeMethod(&window, "onQueryFinished", + Q_ARG(int, 1), Q_ARG(quint64, generation)); + QVERIFY2(action->isEnabled(), + "the action stayed disabled after the query finished"); +} + +void TestMainWindow::markAllReadActsOnEveryRowAndUndoesInOneStep() +{ + // Every row in the view, not just the selected ones, and one undo entry for + // the batch: a user who marks 400 threads read expects one Ctrl+Z to be + // enough. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTableView *>(); + QVERIFY(view); + auto *action = window.findChild<QAction *>(QStringLiteral("mark_all_read")); + QVERIFY(action); + + // A query is needed for runCurrentQuery to do anything: it returns early + // on an empty one, which would leave the flag untouched. + window.findChild<QLineEdit *>()->setText(QStringLiteral("tag:inbox")); + QMetaObject::invokeMethod(&window, "runCurrentQuery"); + model->appendBatch({ makeThread(QStringLiteral("t1"), + { QStringLiteral("unread") }), + makeThread(QStringLiteral("t2"), + { QStringLiteral("unread") }), + makeThread(QStringLiteral("t3"), + { QStringLiteral("unread"), + QStringLiteral("flagged") }) }); + QMetaObject::invokeMethod(&window, "onQueryFinished", Q_ARG(int, 3), + Q_ARG(quint64, + window.currentGenerationForTesting())); + + // One row selected, to prove the action ignores the selection rather than + // acting on it. + view->selectRow(0); + + action->trigger(); + + for (int row = 0; row < 3; ++row) { + QVERIFY2(!model->threadAt(row).tags.contains(QStringLiteral("unread")), + qPrintable(QStringLiteral("row %1 kept its unread tag") + .arg(row))); + } + // An unrelated tag on a row is untouched: only unread is removed. + QVERIFY(model->threadAt(2).tags.contains(QStringLiteral("flagged"))); + + // ONE undo entry for the whole batch, not one per thread. Asserted as a + // depth, since triggering undo once and finding everything restored would + // also pass if three commands had been pushed and the model happened to + // recover on the first. + QCOMPARE(window.undoDepthForTesting(), 1); + + auto *undo = window.findChild<QAction *>(QStringLiteral("undo")); + QVERIFY(undo); + undo->trigger(); + + for (int row = 0; row < 3; ++row) { + QVERIFY2(model->threadAt(row).tags.contains(QStringLiteral("unread")), + qPrintable(QStringLiteral("row %1 was not restored by one undo") + .arg(row))); + } +} + +void TestMainWindow::markAllReadDoesNothingWhenNothingIsUnread() +{ + // No write, no undo entry, and no pending edit for a view that is already + // read: an undo entry that restores nothing is worse than none, since it + // absorbs a Ctrl+Z the user meant for their previous action. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *action = window.findChild<QAction *>(QStringLiteral("mark_all_read")); + QVERIFY(action); + + // A query is needed for runCurrentQuery to do anything: it returns early + // on an empty one, which would leave the flag untouched. + window.findChild<QLineEdit *>()->setText(QStringLiteral("tag:inbox")); + QMetaObject::invokeMethod(&window, "runCurrentQuery"); + model->appendBatch({ makeThread(QStringLiteral("t1"), + { QStringLiteral("flagged") }), + makeThread(QStringLiteral("t2"), {}) }); + QMetaObject::invokeMethod(&window, "onQueryFinished", Q_ARG(int, 2), + Q_ARG(quint64, + window.currentGenerationForTesting())); + + QCOMPARE(window.undoDepthForTesting(), 0); + + action->trigger(); + + // The real assertion: no command was pushed. Checking only that the tags + // did not change would pass against a version that sent a no-op write for + // every row, which still costs an undo entry and a pending edit each. The + // undo QAction cannot answer this: it is always enabled and tests canUndo() + // when triggered. + QVERIFY2(window.undoDepthForTesting() == 0, + "an undo entry was pushed for a view with nothing unread"); + QVERIFY(model->threadAt(0).tags.contains(QStringLiteral("flagged"))); + + // And it says so rather than appearing to have done something. + auto *status = window.findChild<QLabel *>(QStringLiteral("statusMessage")); + QVERIFY(status); + QVERIFY2(status->text().contains(QStringLiteral("Nothing unread")), + qPrintable(status->text())); +} + void TestMainWindow::markReadTimerRestartsRatherThanStacking() { // The plan's hard requirement: arrowing quickly down a list must not mark |
