diff options
| -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 |