diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-07 18:29:58 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-07 18:29:58 +0200 |
| commit | fbc65d2413190faa873d6b0b5cb9ed31ebba453b (patch) | |
| tree | 3a85bbd06e153bac58e6d0fbed6364a50458b0d7 | |
| parent | 48c243d7aee3382713f7a4653be2179d313b432c (diff) | |
| download | qtmaildir-fbc65d2413190faa873d6b0b5cb9ed31ebba453b.tar.gz qtmaildir-fbc65d2413190faa873d6b0b5cb9ed31ebba453b.zip | |
feat(ui): add a Maildir overview under Help
Nothing in the UI reported database-level facts: every query gave a
thread count for that query, and nothing said how much mail there is
overall. A dialog under Help now shows messages, threads and tags from
notmuch, plus the account list from config, since notmuch does not model
accounts at all.
A separate worker call rather than a reuse of requestCounts, which counts
threads to match the row count of a query. This counts messages, which is
what a user means by "how much mail is in here". The test pins 4 messages
in 3 threads against the fixture and fails if they are ever made equal,
so routing both through one count cannot pass unnoticed.
Every field starts at -1 and renders as "unknown" when notmuch could not
answer it. Printing 0 would say the Maildir is empty, and telling someone
their mail is gone is the worst way to report an index that failed to
open.
The dialog opens showing "Counting..." rather than blocking, since
counting every message is not free on a large database. That makes two
lifetimes matter: the reply can arrive after the dialog is closed, so the
label is a QPointer, and the dialog can be closed and reopened while a
count runs, so a generation counter drops the older answer. The test
drains DeferredDelete before firing the late reply, because close()
deletes through deleteLater and without that the dangling case is never
actually reached.
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 36 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 93 | ||||
| -rw-r--r-- | src/mainwindow.h | 33 | ||||
| -rw-r--r-- | src/notmuchworker.cpp | 42 | ||||
| -rw-r--r-- | src/notmuchworker.h | 17 | ||||
| -rw-r--r-- | src/types.h | 14 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 97 | ||||
| -rw-r--r-- | tests/test_notmuchworker.cpp | 44 |
8 files changed, 375 insertions, 1 deletions
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 2e0930c..1cc75a7 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 @@ -82,7 +82,7 @@ taking that too literally. | 31 | The quit prompt has no highlighted default button | discoverability | XS | **done** | | 32 | Esc does not blank the right pane | workflow | XS | **done** | | 33 | Status bar messages never expire | feedback | S | **done** | -| 34 | No overview of the Maildir itself | information | M | open | +| 34 | No overview of the Maildir itself | information | M | **done** | | 35 | No refresh of the thread list after a sync | workflow | M | open | | 36 | `test_mainwindow` cannot reach the worker | testing | S | open, on demand | | 37 | The worker stalls on a tag edit made during a background sync | correctness | S | **done** | @@ -2001,6 +2001,40 @@ quietly reinterpret what it returns. **Still true from the constraints above:** the account count comes from config, never from notmuch, and the tag list is already fetched for the completer. +### Outcome (done 2026-08-07) + +A dialog under Help, as decided. `requestDatabaseStats` answers messages, +threads and tags in one round trip; the account list comes from `Config`. + +**A separate worker call, not a reuse of `requestCounts`**, exactly as the +revision above warned. That one counts THREADS to match the row count of a +query; this one counts MESSAGES, which is what a user means by "how much mail +is in here". The test asserts 4 messages in 3 threads against the fixture and +fails if they are ever made equal, so a later "simplification" that routes both +through one count cannot pass. + +**Unknown is not zero.** Every field starts at -1, and a field notmuch could not +answer renders as "unknown". Printing 0 would say the Maildir is empty, which is +a claim, and telling someone their mail is gone is the worst available way to +report an index that failed to open. Verified by mutation: removing the guard +puts a literal `-1` on screen. + +**The dialog opens before the answer arrives**, showing "Counting...". Counting +every message is not free on a large database, and a dialog that blocks first is +worse than one that fills in. + +Two lifetime problems follow from that, both handled and both tested: + +- The reply can arrive after the dialog is closed. The label is held in a + `QPointer`, since `WA_DeleteOnClose` means a raw pointer dangles for exactly + as long as the count takes, which is when the user is most likely to have + given up and closed it. The test drains `DeferredDelete` before firing the + late reply, because `close()` deletes through `deleteLater` and without the + drain the case being tested is not the one that occurs. +- The dialog can be closed and reopened while a count runs, so a generation + counter drops the older answer rather than filling in the newer dialog with + numbers that predate the reopen. + ## 35. No refresh of the thread list after a sync **Observed (user, 2026-08-04):** "auto refresh list after sync." diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 3f8a53d..0ce94a8 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -288,6 +288,7 @@ MainWindow::MainWindow(const Config &config, QWidget *parent) qRegisterMetaType<ThreadSummary>(); qRegisterMetaType<MessageRef>(); qRegisterMetaType<TagChange>(); + qRegisterMetaType<DatabaseStats>(); qRegisterMetaType<QVector<ThreadSummary>>(); qRegisterMetaType<QVector<MessageRef>>(); @@ -885,6 +886,13 @@ void MainWindow::buildMenus() auto *shortcuts = helpMenu->addAction(tr("&Keyboard shortcuts")); connect(shortcuts, &QAction::triggered, this, &MainWindow::showShortcutReference); + // A dialog the user asks for, per item 34: counting every message is not + // free on a large database, so this must not be anything that refreshes on + // its own. + auto *maildirInfo = helpMenu->addAction(tr("&Maildir overview")); + maildirInfo->setObjectName(QStringLiteral("maildirOverview")); + connect(maildirInfo, &QAction::triggered, + this, &MainWindow::showMaildirOverview); auto *about = helpMenu->addAction(tr("&About")); connect(about, &QAction::triggered, this, &MainWindow::showAbout); @@ -1031,6 +1039,89 @@ void MainWindow::showShortcutReference() dialog.exec(); } +void MainWindow::showMaildirOverview() +{ + auto *dialog = new QDialog(this); + dialog->setWindowTitle(tr("Maildir overview")); + dialog->setObjectName(QStringLiteral("maildirOverviewDialog")); + // Deleted on close, which is what makes m_overviewCounts a QPointer: the + // worker's reply can arrive after the user has dismissed it. + dialog->setAttribute(Qt::WA_DeleteOnClose); + + auto *counts = new QLabel(dialog); + counts->setObjectName(QStringLiteral("maildirCounts")); + counts->setTextFormat(Qt::RichText); + // Shown as pending rather than as zero. The dialog opens before the answer + // arrives, and a zero would read as "no mail", which is a claim rather than + // an absence of one. + counts->setText(tr("<b>Counting...</b>")); + m_overviewCounts = counts; + + // From config, never from notmuch, which does not model accounts at all. + // That is the whole reason per-account subdirectories are configured. + QString accountText; + const QList<Account> accounts = m_config.accounts(); + accountText += tr("<b>%n account(s)</b>", "", int(accounts.size())); + if (!accounts.isEmpty()) { + accountText += QStringLiteral("<ul>"); + for (const Account &account : accounts) { + // Account names are user-written config, and this label is rich + // text, so they are escaped like any other untrusted value. + const QString label = account.label.isEmpty() ? account.key + : account.label; + accountText += QStringLiteral("<li>%1</li>") + .arg(label.toHtmlEscaped()); + } + accountText += QStringLiteral("</ul>"); + } + + auto *accountLabel = new QLabel(accountText, dialog); + accountLabel->setObjectName(QStringLiteral("maildirAccounts")); + accountLabel->setTextFormat(Qt::RichText); + + auto *buttons = new QDialogButtonBox(QDialogButtonBox::Close, dialog); + connect(buttons, &QDialogButtonBox::rejected, dialog, &QDialog::reject); + + auto *layout = new QVBoxLayout(dialog); + layout->addWidget(counts); + layout->addWidget(accountLabel); + layout->addStretch(); + layout->addWidget(buttons); + + // Asked for when the dialog opens and never on a timer: counting every + // message in a large database is not free, which is the constraint that + // made this a dialog rather than a status-bar field. + QMetaObject::invokeMethod(m_worker, "requestDatabaseStats", + Qt::QueuedConnection, + Q_ARG(quint64, ++m_statsGeneration)); + + dialog->show(); +} + +void MainWindow::onDatabaseStatsReady(const DatabaseStats &stats, + quint64 generation) +{ + // Closed and reopened while the count ran: this answer belongs to the old + // dialog. The QPointer covers "closed", this covers "closed and reopened". + if (generation != m_statsGeneration) + return; + + if (!m_overviewCounts) + return; + + // A field notmuch could not answer stays unknown. Printing 0 would say the + // database is empty, which is the opposite of "we could not tell". + const auto number = [](int value) { + return value < 0 ? tr("unknown") : QLocale().toString(value); + }; + + m_overviewCounts->setText( + tr("<b>%1</b> messages in <b>%2</b> threads<br>" + "<b>%3</b> tags") + .arg(number(stats.messages), number(stats.threads), + number(stats.tags))); +} + void MainWindow::showAbout() { QDialog dialog(this); @@ -1098,6 +1189,8 @@ void MainWindow::wireWorker() this, &MainWindow::onAllTagsReady); connect(m_worker, &NotmuchWorker::countsReady, this, &MainWindow::onCountsReady); + connect(m_worker, &NotmuchWorker::databaseStatsReady, + this, &MainWindow::onDatabaseStatsReady); // A confirmed write clears the pending revert: without this, a later // unrelated error would roll back a change that actually succeeded. diff --git a/src/mainwindow.h b/src/mainwindow.h index 8043727..579597c 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -20,6 +20,7 @@ #include <QHash> #include <QMainWindow> +#include <QPointer> #include <QThread> #include <QUndoCommand> #include <QUndoStack> @@ -133,6 +134,13 @@ public: /// stale, so a test standing in for the worker has to know the current one. quint64 currentGenerationForTesting() const { return m_generation; } + /// The generation a database-stats reply must carry to be accepted. + /// + /// A test seam, for the same reason as the one above: onDatabaseStatsReady + /// discards a reply belonging to a dialog that has since been closed and + /// reopened, so a test standing in for the worker needs the current value. + quint64 statsGenerationForTesting() const { return m_statsGeneration; } + protected: void closeEvent(QCloseEvent *event) override; @@ -189,6 +197,10 @@ private slots: /// requestPlaceholderCounts() asked for them. void onCountsReady(const QVector<int> &counts, quint64 generation); + /// Fills in the overview dialog's counts when the worker answers. Does + /// nothing if the dialog has since been closed. + void onDatabaseStatsReady(const DatabaseStats &stats, quint64 generation); + /// Runs a query the user clicked on the placeholder pane. void onPlaceholderQueryRequested(const QString &query); @@ -225,6 +237,16 @@ private: void showShortcutReference(); void showAbout(); + /// The Maildir overview (item 34): what notmuch knows about the database, + /// plus the account list, which comes from config since notmuch does not + /// model accounts at all. + /// + /// Opens immediately showing the counts as pending and fills them in when + /// the worker answers, rather than blocking: counting every message is not + /// free on a large database and a dialog that hangs first is worse than one + /// that populates. + void showMaildirOverview(); + /// Creates a QAction, binds it to the sequence KeyMap holds for `name`, /// and registers it. `name` is the action name used in [keys]. QAction *addAction(const QString &name, const QString &text, @@ -422,6 +444,17 @@ private: /// attention, so it must survive until the next successful run. bool m_lastSyncFailed = false; + /// The overview dialog's counts label while that dialog is open, null + /// otherwise. A QPointer because the dialog is deleted on close and the + /// worker's reply can arrive afterwards: a raw pointer would dangle for + /// exactly as long as the count takes on a large database, which is + /// precisely when the user is most likely to close it first. + QPointer<QLabel> m_overviewCounts; + + /// Discriminates a stats reply from a dialog that has since been closed + /// and reopened, so an old answer cannot fill in a newer dialog. + quint64 m_statsGeneration = 0; + /// Holds the sync log and its close button, so the pane can be dismissed. QWidget *m_syncLogPane = nullptr; QPlainTextEdit *m_syncLog = nullptr; diff --git a/src/notmuchworker.cpp b/src/notmuchworker.cpp index 973d110..752a52c 100644 --- a/src/notmuchworker.cpp +++ b/src/notmuchworker.cpp @@ -363,6 +363,48 @@ void NotmuchWorker::requestAllTags(quint64 generation) emit allTagsReady(result, generation); } +void NotmuchWorker::requestDatabaseStats(quint64 generation) +{ + if (!openReadOnly()) + return; + + DatabaseStats stats; + + // "*" is notmuch's match-everything query. Counting messages and threads + // needs two calls on it: the numbers differ by the reply depth of the + // database and there is no single call that yields both. + NmQuery all(notmuch_query_create(m_db, "*")); + if (all) { + unsigned int messages = 0; + if (notmuch_query_count_messages(all.get(), &messages) + == NOTMUCH_STATUS_SUCCESS) { + stats.messages = static_cast<int>(messages); + } + } + + // A second query object rather than reusing the one above: notmuch caches + // results on a query, and counting both ways from one has bitten people. + NmQuery allThreads(notmuch_query_create(m_db, "*")); + if (allThreads) { + unsigned int threads = 0; + if (notmuch_query_count_threads(allThreads.get(), &threads) + == NOTMUCH_STATUS_SUCCESS) { + stats.threads = static_cast<int>(threads); + } + } + + // Already enumerated for the completer, so this costs nothing extra. + NmTags tags(notmuch_database_get_all_tags(m_db)); + if (tags) { + int count = 0; + for (; notmuch_tags_valid(tags.get()); notmuch_tags_move_to_next(tags.get())) + ++count; + stats.tags = count; + } + + emit databaseStatsReady(stats, generation); +} + void NotmuchWorker::requestCounts(const QStringList &queries, quint64 generation) { if (!openReadOnly()) diff --git a/src/notmuchworker.h b/src/notmuchworker.h index 05c1d3a..7d6a587 100644 --- a/src/notmuchworker.h +++ b/src/notmuchworker.h @@ -83,6 +83,19 @@ public slots: /// refreshing one nobody is looking at is work for nothing. void requestCounts(const QStringList &queries, quint64 generation); + /// Database-level facts for the Maildir overview (item 34): total messages, + /// total threads, and the number of tags. + /// + /// **Messages, not threads**, which is what distinguishes this from + /// requestCounts above. That one answers "how many rows will this query + /// produce" and counts threads to match the list; this one describes the + /// database, where the message total is the number a user means by "how + /// much mail is in here". + /// + /// Counting every message is not free on a large database, so this is + /// called when the dialog is opened and never on a timer. + void requestDatabaseStats(quint64 generation); + signals: void threadsReady(const QVector<ThreadSummary> &threads, quint64 generation); void queryFinished(int totalThreads, quint64 generation); @@ -95,6 +108,10 @@ signals: /// positional correspondence the caller relies on always holds. void countsReady(const QVector<int> &counts, quint64 generation); + /// Fields left at -1 are ones notmuch could not answer, which the dialog + /// renders as unknown rather than as zero. + void databaseStatsReady(const DatabaseStats &stats, quint64 generation); + void errorOccurred(const QString &message); private: diff --git a/src/types.h b/src/types.h index 59d1d06..999d3f3 100644 --- a/src/types.h +++ b/src/types.h @@ -78,6 +78,20 @@ struct TagChange } }; +/// Database-level facts for the Maildir overview. +/// +/// Every field is -1 until answered, so a dialog opened against a database that +/// cannot be read shows "unknown" rather than a confident zero. A zero is a +/// claim, and "no mail at all" is exactly the wrong thing to tell someone whose +/// index failed to open. +struct DatabaseStats +{ + int messages = -1; ///< Every message notmuch has indexed. + int threads = -1; ///< Every thread. Differs from messages by reply depth. + int tags = -1; ///< Distinct tag names in the database. +}; + Q_DECLARE_METATYPE(ThreadSummary) Q_DECLARE_METATYPE(MessageRef) Q_DECLARE_METATYPE(TagChange) +Q_DECLARE_METATYPE(DatabaseStats) diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 2625bf5..ef6f99d 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -78,6 +78,8 @@ private slots: void theStatusBarReportsAMultiRowSelection(); void clearSelectionBlanksThePaneAndDeselects(); void clearPaneLeavesTheSelectionAlone(); + void maildirOverviewShowsUnknownRatherThanZero(); + void maildirOverviewIgnoresAStaleReply(); void theThreadListOffersAContextMenu(); void aSecondRowBlanksThePaneNotOnlyAThird(); void aLocalSyncIsNotReportedAsABackgroundOne(); @@ -1179,6 +1181,101 @@ void TestMainWindow::clearPaneLeavesTheSelectionAlone() QCOMPARE(view->selectionModel()->selectedRows().size(), 1); } +void TestMainWindow::maildirOverviewShowsUnknownRatherThanZero() +{ + // A field notmuch could not answer must not render as 0. "0 messages" says + // the Maildir is empty, which is a claim; the truth is that the count + // failed, and telling someone their mail is gone is the worst available + // way to report an unreadable index. + const Config config; + MainWindow window(config); + + auto *action = window.findChild<QAction *>(QStringLiteral("maildirOverview")); + QVERIFY2(action, "no maildirOverview action"); + action->trigger(); + + auto *counts = window.findChild<QLabel *>(QStringLiteral("maildirCounts")); + QVERIFY2(counts, "the overview dialog has no counts label"); + + // The worker never answers in this fixture, so drive the slot directly + // with the all-unknown stats a failed open produces. + QMetaObject::invokeMethod( + &window, "onDatabaseStatsReady", Qt::DirectConnection, + Q_ARG(DatabaseStats, DatabaseStats{}), + Q_ARG(quint64, window.statsGenerationForTesting())); + + QVERIFY2(counts->text().contains(QStringLiteral("unknown")), + qPrintable(QStringLiteral("counts label says '%1'") + .arg(counts->text()))); + QVERIFY2(!counts->text().contains(QStringLiteral(">0<")), + qPrintable(QStringLiteral("an unanswered count rendered as zero: " + "'%1'").arg(counts->text()))); + + // WA_DeleteOnClose, so closing is what frees it. Left open, each test + // leaks a window for the rest of the run. + counts->window()->close(); +} + +void TestMainWindow::maildirOverviewIgnoresAStaleReply() +{ + // Counting every message is slow enough that closing and reopening the + // dialog while one runs is realistic. The older answer must not fill in the + // newer dialog, or the numbers silently predate whatever prompted the + // reopen. + const Config config; + MainWindow window(config); + + auto *action = window.findChild<QAction *>(QStringLiteral("maildirOverview")); + QVERIFY(action); + action->trigger(); + + const quint64 stale = window.statsGenerationForTesting(); + + // Reopening bumps the generation, which is what makes the first reply old. + action->trigger(); + QVERIFY2(window.statsGenerationForTesting() != stale, + "reopening the dialog did not bump the generation, so a reply for " + "the previous one cannot be told apart"); + + auto *counts = window.findChild<QLabel *>(QStringLiteral("maildirCounts")); + QVERIFY(counts); + const QString before = counts->text(); + + DatabaseStats old; + old.messages = 4321; + old.threads = 999; + old.tags = 42; + QMetaObject::invokeMethod(&window, "onDatabaseStatsReady", + Qt::DirectConnection, + Q_ARG(DatabaseStats, old), + Q_ARG(quint64, stale)); + + QCOMPARE(counts->text(), before); + QVERIFY2(!counts->text().contains(QStringLiteral("4321")), + "a reply for the previous dialog filled in the current one"); + + QPointer<QLabel> watch(counts); + counts->window()->close(); + + // WA_DeleteOnClose deletes through deleteLater, so the label outlives + // close() until the event loop runs. Drain it, or the "reply after the + // dialog is gone" case below is not actually being tested. + QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete); + QVERIFY2(watch.isNull(), + "the dialog was not destroyed, so the case below is not the one " + "this test means to exercise"); + + // The QPointer's reason for being: counting a large database takes long + // enough that closing the dialog first is ordinary, and the reply then + // arrives for a label that has been deleted. A raw pointer would dangle + // here, so this must not crash. + QMetaObject::invokeMethod(&window, "onDatabaseStatsReady", + Qt::DirectConnection, + Q_ARG(DatabaseStats, old), + Q_ARG(quint64, + window.statsGenerationForTesting())); +} + void TestMainWindow::theThreadListOffersAContextMenu() { // Right-click is the other half of discoverability: until now every tag diff --git a/tests/test_notmuchworker.cpp b/tests/test_notmuchworker.cpp index be62ad3..b419915 100644 --- a/tests/test_notmuchworker.cpp +++ b/tests/test_notmuchworker.cpp @@ -60,6 +60,8 @@ private slots: void requestCountsAnswersOneCountPerQuery(); void requestCountsKeepsPositionOnAnInvalidQuery(); + void requestDatabaseStatsCountsMessagesNotThreads(); + void requestDatabaseStatsOnUnreadableConfigEmitsError(); private: /// Tags of one message, read back through a fresh worker query. @@ -523,5 +525,47 @@ void TestNotmuchWorker::requestCountsKeepsPositionOnAnInvalidQuery() QCOMPARE(counts.at(2), 3); } +void TestNotmuchWorker::requestDatabaseStatsCountsMessagesNotThreads() +{ + NotmuchWorker worker(m_fixture.configPath()); + QSignalSpy spy(&worker, &NotmuchWorker::databaseStatsReady); + + worker.requestDatabaseStats(11); + + QCOMPARE(spy.count(), 1); + QCOMPARE(spy.at(0).at(1).value<quint64>(), quint64(11)); + const auto stats = spy.at(0).at(0).value<DatabaseStats>(); + + // The fixture holds four messages in three threads: thread A is a message + // and its reply. **That difference is the whole point of this call.** + // requestCounts() counts threads, to match the row count of a query; this + // one counts messages, which is what a user means by "how much mail". A + // reimplementation that reused the thread count would report 3 here and be + // confidently wrong under the label "messages". + QCOMPARE(stats.messages, 4); + QCOMPARE(stats.threads, 3); + QVERIFY2(stats.messages != stats.threads, + "messages and threads are equal, so this fixture cannot prove the " + "two counts are distinct: add a reply to it"); + + // Every tag the fixture creates, plus notmuch's own. + QVERIFY(stats.tags > 0); +} + +void TestNotmuchWorker::requestDatabaseStatsOnUnreadableConfigEmitsError() +{ + // Fails closed like every other entry point. The dialog then shows its + // fields as unknown rather than as zero, since "no mail at all" is the + // wrong thing to tell someone whose index failed to open. + NotmuchWorker worker(QStringLiteral("/nonexistent/qtmaildir-test/config")); + QSignalSpy ready(&worker, &NotmuchWorker::databaseStatsReady); + QSignalSpy errors(&worker, &NotmuchWorker::errorOccurred); + + worker.requestDatabaseStats(1); + + QCOMPARE(errors.size(), 1); + QVERIFY(ready.isEmpty()); +} + QTEST_MAIN(TestNotmuchWorker) #include "test_notmuchworker.moc" |
