diff options
| -rw-r--r-- | README.md | 21 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 55 | ||||
| -rw-r--r-- | src/config.cpp | 19 | ||||
| -rw-r--r-- | src/config.h | 10 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 171 | ||||
| -rw-r--r-- | src/mainwindow.h | 32 | ||||
| -rw-r--r-- | tests/test_config.cpp | 47 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 163 |
8 files changed, 498 insertions, 20 deletions
@@ -119,6 +119,11 @@ identity. ; with Ctrl+U. Arrowing quickly through a list marks only the thread you stop ; on, never the ones you pass through. ; mark_read_delay_ms = 2000 +; Optional. What to do about unsynced tag changes when you quit. "ask" (the +; default) offers to sync, quit anyway, or stay; "always" syncs without asking +; and quits when it finishes; "never" quits silently. A sync that fails never +; closes the window, so a failure cannot discard the changes quietly. +; sync_on_exit = ask [completion] ; Optional. Extra content types offered after mimetype:, APPENDED to the @@ -243,6 +248,22 @@ an attachment, so it is visible without opening the thread. It comes from the `attachment` tag notmuch applies while indexing, not from parsing the message, and costs no extra query. +## Unsynced changes + +Tagging changes the notmuch index at once, but the mail store only learns about +it on the next sync. The status bar therefore counts the tag changes made here +that a sync has not yet carried over, and clears the count when one succeeds. A +**failed** sync leaves the count standing, since the changes really are still +unsynced. + +The count is a lower bound rather than a guarantee: an external `notmuch new` +from your own cron can carry changes over without this application noticing. + +Quitting with changes outstanding asks what to do, controlled by +`[general] sync_on_exit`. See the configuration block above for the three +values. When a sync started at exit fails, the window stays open and says so +rather than quitting as though it had worked. + ## Message details The strip above the message pane says what it can say without guessing. A 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 7fedaf3..8f19d6b 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 @@ -56,8 +56,8 @@ taking that too literally. | 15 | Attachments are parsed but unreachable from the UI | information | M | **done** | | 16 | Delete on an already-deleted thread should undelete | behavior | S | open | | 17 | No completion for tags in the query bar | workflow | M | **done** | -| 18 | No visual cue that there are unsynced edits | feedback | S | open | -| 19 | No prompt to sync on exit when edits are pending | behavior | S | open | +| 18 | No visual cue that there are unsynced edits | feedback | S | **done** | +| 19 | No prompt to sync on exit when edits are pending | behavior | S | **done** | | 20 | Thread view does not match the user's mental model | presentation | ? | open, unspecified | | 21 | Default shortcuts are not sensible enough | discoverability | S | open | | 22 | Translatability audit and i18n wiring | correctness | M | open | @@ -847,6 +847,27 @@ resets. survives. Then sync and confirm it clears. Then make the sync fail and confirm it does not. +### Outcome (done) + +Built as specced. `m_pendingEdits` counts confirmed mutations, incremented in +`onTagsApplied()` and reset only by `onSyncFinished(true, ...)`. The count is +shown as a permanent status-bar widget, hidden entirely at zero. + +**Counted where a write is confirmed, not where one is sent.** An optimistic +update the worker later rejects must not leave the indicator claiming an edit +that never landed, so the increment sits in the `tagsApplied` handler. + +That handler was a lambda; it is now a named slot, which is both better +structure and what lets a test drive it. An earlier draft tried to reach the +worker with `findChild` to emit the real signal: the worker is deliberately +parentless because it moves to its own thread, so that cannot work, and +contorting the test to reach it was the wrong instinct. What matters is the +counter's behaviour, not the signal's origin. + +**Both tests were verified by breaking the code.** Clearing the count on any +sync outcome is caught, and so is clearing it where the undo stack is cleared, +which is the naive design this item exists to avoid. + ## 19. No prompt to sync on exit when edits are pending **Observed (user, 2026-08-04):** quitting with unsynced edits is silent. The @@ -891,6 +912,36 @@ for the reason item 18 documents. each `sync_on_exit` value and confirm the behavior matches. Then quit with a deliberately broken sync command and confirm the app neither hangs nor lies. +### Outcome (done) + +Built as specced, with the user confirming the three-value key over their +original on/off idea. `closeEvent()` consults the count, and a sync started for +exit holds the window open until `finished` arrives rather than being killed +mid-run. + +**A failed exit-sync does not quit.** It restores the window, shows the log and +says what happened. Quitting there would discard the user's choice silently, +which is the exact failure the prompt exists to prevent. + +**With no sync command configured the prompt degrades** to a warning offering +Discard or Cancel, rather than offering a sync that cannot run. +`MailSync::isAvailable()` is already false in that case. + +**Testing a modal needs care, and the first attempt was wrong.** A test that +sends a close event hangs forever if an unexpected dialog opens, because the +modal spins its own event loop. The first version appeared to pass in 19 +seconds; it had actually opened a real dialog on the user's screen, and the +user dismissed it. `CloseProbe` now polls for `activeModalWidget()`, closes it, +and records that one appeared, turning "a dialog opened" into an assertion +instead of a hang or a prompt for whoever is watching. + +**One mutation test was a dud and is worth recording.** Removing the `Never` +guard from `closeEvent` did NOT make the test fail, because the branches below +match only `Ask` and `Always`, so `Never` fell through to closing anyway. The +guard is redundant belt-and-braces rather than load-bearing. Confirming the +probe really detects a prompt needed the config mutated to `ask` instead, which +does fail it. + ## 20. Thread view does not match the user's mental model **Observed (user, 2026-08-04), in passing while deciding item 2:** "My view for diff --git a/src/config.cpp b/src/config.cpp index f5d2d15..8267835 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -98,6 +98,25 @@ void Config::load(const QString &path) // Zero and negative are NOT errors and must not be clamped. Zero means mark // read at once, and any negative value means never, which is how the // behaviour is turned off. + // Three values, not a bool: "prompt me", "just do it" and "do nothing" are + // three distinct behaviours and true/false can only express two of them. + const QString syncExit = + settings.value(QStringLiteral("sync_on_exit"), + QStringLiteral("ask")).toString().trimmed().toLower(); + if (syncExit == QStringLiteral("ask")) { + m_syncOnExit = SyncOnExit::Ask; + } else if (syncExit == QStringLiteral("always")) { + m_syncOnExit = SyncOnExit::Always; + } else if (syncExit == QStringLiteral("never")) { + m_syncOnExit = SyncOnExit::Never; + } else { + // Naming the accepted values, since a typo here silently changes what + // happens to unsynced work at exit. + addProblem(QStringLiteral("Unknown sync_on_exit '%1'; expected ask, " + "always or never. Using ask.") + .arg(syncExit)); + } + const QVariant markRead = settings.value(QStringLiteral("mark_read_delay_ms")); if (markRead.isValid()) { bool ok = false; diff --git a/src/config.h b/src/config.h index 7c6eb63..ba9b7f6 100644 --- a/src/config.h +++ b/src/config.h @@ -102,6 +102,15 @@ public: /// once it is known. The manual trigger works regardless. bool completionOnFocus() const { return m_completionOnFocus; } + /// What to do about unsynced edits when the window closes. + enum class SyncOnExit { + Ask, ///< Prompt, offering to sync, quit anyway, or stay. The default. + Always, ///< Sync without asking, then quit once it finishes. + Never, ///< Quit silently, which is the behaviour before this existed. + }; + + SyncOnExit syncOnExit() const { return m_syncOnExit; } + /// How long an opened thread stays unread before it is marked read. /// /// Three meanings, all deliberate: a positive value is the delay in @@ -143,6 +152,7 @@ private: qreal m_messageZoom = 1.0; bool m_completionOnFocus = false; int m_markReadDelayMs = 2000; + SyncOnExit m_syncOnExit = SyncOnExit::Ask; QList<CompletionEntry> m_extraMimetypes; QString m_startupQuery = QStringLiteral("Unread"); diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 45db452..d8fba3e 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -145,6 +145,84 @@ void MainWindow::saveUiState() const void MainWindow::closeEvent(QCloseEvent *event) { + // A sync started for exit is still running: hold the window open. Its + // finished signal closes us, and asking again here would stack prompts. + if (m_syncingForExit) { + event->ignore(); + return; + } + + if (!m_closeApproved && m_pendingEdits > 0 + && m_config.syncOnExit() != Config::SyncOnExit::Never) { + + // Not a destructive-action confirmation, which CLAUDE.md forbids for + // tag mutations. Those get undo instead. This asks about LOSING work at + // the one point where undo cannot help, which is the opposite case. + const bool canSync = m_sync && m_sync->isAvailable(); + + if (!canSync) { + // Degrade to a warning rather than offering a sync that cannot run. + const auto answer = QMessageBox::warning( + this, tr("Unsynced changes"), + tr("%n tag change(s) have not been synced, and no sync command " + "is configured. Quit anyway?", "", m_pendingEdits), + QMessageBox::Discard | QMessageBox::Cancel, + QMessageBox::Cancel); + if (answer == QMessageBox::Cancel) { + event->ignore(); + return; + } + } else if (m_config.syncOnExit() == Config::SyncOnExit::Ask) { + // Three buttons, not two: a user who hit Quit by mistake needs a + // way back that is not "sync". + QMessageBox box(this); + box.setIcon(QMessageBox::Question); + box.setWindowTitle(tr("Unsynced changes")); + box.setText(tr("%n tag change(s) have not been synced.", "", + m_pendingEdits)); + box.setInformativeText(tr("Sync before quitting?")); + QPushButton *sync = + box.addButton(tr("Sync and quit"), QMessageBox::AcceptRole); + QPushButton *quit = + box.addButton(tr("Quit anyway"), QMessageBox::DestructiveRole); + box.addButton(QMessageBox::Cancel); + box.setDefaultButton(sync); + box.exec(); + + if (box.clickedButton() == sync) { + if (m_sync->start()) { + m_syncingForExit = true; + m_statusLabel->setText(tr("Syncing before quitting...")); + event->ignore(); + return; + } + // Could not start after all: say so and stay, rather than + // quitting as though the sync had happened. + QMessageBox::warning(this, tr("Sync failed"), + tr("The sync could not be started, so " + "your changes are still unsynced.")); + event->ignore(); + return; + } + if (box.clickedButton() != quit) { + event->ignore(); // Cancel, or the dialog was dismissed. + return; + } + } else if (m_config.syncOnExit() == Config::SyncOnExit::Always) { + if (m_sync->start()) { + m_syncingForExit = true; + m_statusLabel->setText(tr("Syncing before quitting...")); + event->ignore(); + return; + } + QMessageBox::warning(this, tr("Sync failed"), + tr("The sync could not be started, so your " + "changes are still unsynced.")); + event->ignore(); + return; + } + } + saveUiState(); QMainWindow::closeEvent(event); } @@ -243,6 +321,14 @@ void MainWindow::buildUi() m_statusLabel = new QLabel(this); statusBar()->addWidget(m_statusLabel); + // Beside the sync status rather than as a widget competing with it: the two + // say related things and reading them apart would be worse than reading + // them together. + m_pendingLabel = new QLabel(this); + m_pendingLabel->setObjectName(QStringLiteral("pendingEdits")); + m_pendingLabel->hide(); + statusBar()->addPermanentWidget(m_pendingLabel); + // Query row. auto *queryRow = new QHBoxLayout; m_accountBox = new QComboBox(central); @@ -422,10 +508,6 @@ void MainWindow::registerActions() }); addAction(QStringLiteral("open_thread"), tr("&Open thread"), tr("Focus the thread list"), [this]() { - qDebug("[MW] open_thread action TRIGGERED (focus=%s)", - QApplication::focusWidget() - ? QApplication::focusWidget()->metaObject()->className() - : "none"); m_threadView->setFocus(); }); addAction(QStringLiteral("archive"), tr("&Archive"), @@ -739,20 +821,7 @@ void MainWindow::wireWorker() // A confirmed write clears the pending revert: without this, a later // unrelated error would roll back a change that actually succeeded. connect(m_worker, &NotmuchWorker::tagsApplied, - this, [this](const TagChange &change) { - m_pendingChange = {}; - m_pendingThreadIds.clear(); - - // A tag the user has just created is the one they are most likely to - // type again, so do not wait for the next sync to offer it. A set - // membership test, not a query. - for (const QString &tag : change.added) { - if (!m_knownTags.contains(tag)) { - requestAllTags(); - break; - } - } - }); + this, &MainWindow::onTagsApplied); m_workerThread.start(); @@ -939,16 +1008,82 @@ void MainWindow::onWorkerError(const QString &message) void MainWindow::onSyncFinished(bool success, int exitCode) { if (success) { + // Only a SUCCESSFUL sync clears the count. Clearing on failure would + // assert the edits had reached the mail store when the sync is exactly + // what failed to put them there. + m_pendingEdits = 0; + updatePendingIndicator(); + m_statusLabel->setText(tr("Sync complete")); + + if (m_syncingForExit) { + // The work is safely across, so finish the quit the user asked for. + m_syncingForExit = false; + m_closeApproved = true; + close(); + return; + } + runCurrentQuery(); // A sync is the usual way new tags enter the database. requestAllTags(); } else { m_statusLabel->setText(tr("Sync failed (exit %1)").arg(exitCode)); m_syncLog->show(); + + if (m_syncingForExit) { + // Do NOT quit: the edits are still unsynced and quitting now would + // discard the user's choice silently, which is the failure the + // whole prompt exists to prevent. Leave the window open with the + // log showing, so they can see what went wrong and decide. + m_syncingForExit = false; + QMessageBox::warning( + this, tr("Sync failed"), + tr("The sync failed (exit %1), so your changes are still " + "unsynced. The window has been left open.").arg(exitCode)); + } + } +} + +void MainWindow::onTagsApplied(const TagChange &change) +{ + m_pendingChange = {}; + m_pendingThreadIds.clear(); + + // Counted here, where a write is CONFIRMED, rather than where one is sent: + // an optimistic update the worker later rejects must not leave the + // indicator claiming an edit that never landed. + ++m_pendingEdits; + updatePendingIndicator(); + + // A tag the user has just created is the one they are most likely to type + // again, so do not wait for the next sync to offer it. A set membership + // test, not a query. + for (const QString &tag : change.added) { + if (!m_knownTags.contains(tag)) { + requestAllTags(); + break; + } } } +void MainWindow::updatePendingIndicator() +{ + if (m_pendingEdits <= 0) { + m_pendingLabel->hide(); + return; + } + + // "Changes" and not "mutations": the unit the user thinks in is the tagging + // they did, not the writes it became. + m_pendingLabel->setText(tr("%n unsynced change(s)", "", m_pendingEdits)); + m_pendingLabel->setToolTip( + tr("Tag changes made here that a sync has not yet carried to the mail " + "store. An external notmuch run can clear them without this count " + "noticing.")); + m_pendingLabel->show(); +} + void MainWindow::scheduleMarkRead(const ThreadSummary &thread) { // Any pending timer belongs to a thread that is no longer on screen. diff --git a/src/mainwindow.h b/src/mainwindow.h index 0eea164..b0a4cfd 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -88,6 +88,10 @@ private slots: void onThreadLoaded(const QVector<MessageRef> &messages, quint64 generation); void onWorkerError(const QString &message); void onSyncFinished(bool success, int exitCode); + + /// A tag mutation the worker has confirmed reached the database. Counts it + /// as unsynced, since reaching the index is not reaching the mail store. + void onTagsApplied(const TagChange &change); void onAllTagsReady(const QStringList &tags); private: @@ -127,6 +131,19 @@ private: /// the one on screen. void markCurrentThreadRead(); + /// Redraws the unsynced-edits indicator from m_pendingEdits. + void updatePendingIndicator(); + + /// Set once the user has answered the exit prompt, or once a sync started + /// for exit has finished. Stops closeEvent asking a second time, and is + /// what lets the deferred close through. + bool m_closeApproved = false; + + /// True while a sync started by the exit prompt is running. The window + /// stays open until it finishes: killing the process mid-sync is exactly + /// the loss the prompt exists to prevent. + bool m_syncingForExit = false; + /// Sends a tag change for a set of threads without touching the undo stack. /// Both tagSelected() and ThreadTagCommand route through this. void sendThreadTagChange(const QStringList &threadIds, @@ -158,6 +175,10 @@ private: QComboBox *m_accountBox = nullptr; QPushButton *m_syncButton = nullptr; QLabel *m_statusLabel = nullptr; + + /// Says how many tag changes have not been seen to reach the mail store. + /// Hidden entirely at zero rather than reading "0 unsynced", which is noise. + QLabel *m_pendingLabel = nullptr; QPlainTextEdit *m_syncLog = nullptr; /// Action name (as used in [keys]) to the QAction implementing it. Owned @@ -178,6 +199,17 @@ private: QString m_lastQuery; QString m_currentThreadId; + /// Confirmed tag mutations not yet known to have reached the mail store. + /// + /// A count of its own rather than QUndoStack::isClean(), which cannot serve + /// here: the undo stack is CLEARED on every query, since its entries refer + /// to rows the new result set discards. Tag a thread, run any query, and the + /// stack is empty while the change is still unsynced. + /// + /// A lower bound on what is outstanding, never a guarantee: the user's cron + /// can run notmuch new without the application noticing. + int m_pendingEdits = 0; + /// Marks the open thread read once it has been on screen long enough. /// /// Single-shot and RESTARTED on every selection change, never stacked: diff --git a/tests/test_config.cpp b/tests/test_config.cpp index a09738b..4d92891 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -45,6 +45,9 @@ private slots: void markReadDelayIsActuallyRead(); void markReadDelayAcceptsZeroAndNegative(); void markReadDelayRejectsGarbage(); + void syncOnExitDefaultsToAsk(); + void syncOnExitReadsAllThreeValues(); + void syncOnExitWarnsOnGarbage(); void extraMimetypesAppendToBuiltins(); void extraMimetypeDescriptionMayContainComma(); void malformedExtraMimetypeIsSkipped(); @@ -429,6 +432,50 @@ void TestConfig::markReadDelayRejectsGarbage() QVERIFY(!config.problems().isEmpty()); } +void TestConfig::syncOnExitDefaultsToAsk() +{ + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral("[general]\n"))); + QCOMPARE(config.syncOnExit(), Config::SyncOnExit::Ask); + QVERIFY(config.problems().isEmpty()); +} + +void TestConfig::syncOnExitReadsAllThreeValues() +{ + // A bool could only carry two of these. Each is a distinct behaviour at + // exit, so each has to round-trip. + const QList<QPair<QString, Config::SyncOnExit>> cases = { + { QStringLiteral("ask"), Config::SyncOnExit::Ask }, + { QStringLiteral("always"), Config::SyncOnExit::Always }, + { QStringLiteral("never"), Config::SyncOnExit::Never }, + // Case and surrounding space are the user's, not the parser's problem. + { QStringLiteral(" Always "), Config::SyncOnExit::Always }, + }; + + for (const auto &testCase : cases) { + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral("[general]\nsync_on_exit=%1\n") + .arg(testCase.first))); + QCOMPARE(config.syncOnExit(), testCase.second); + QVERIFY2(config.problems().isEmpty(), + qPrintable(QStringLiteral("'%1' warned").arg(testCase.first))); + } +} + +void TestConfig::syncOnExitWarnsOnGarbage() +{ + // Silently falling back would change what happens to unsynced work without + // telling the user, so a typo has to be named. + QTemporaryDir dir; + Config config; + config.load(writeIni(dir, QStringLiteral("[general]\n" + "sync_on_exit=maybe\n"))); + QCOMPARE(config.syncOnExit(), Config::SyncOnExit::Ask); + QVERIFY(!config.problems().isEmpty()); +} + void TestConfig::extraMimetypesAppendToBuiltins() { QTemporaryDir dir; diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index fb33462..d307a99 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -20,8 +20,10 @@ #include <QAction> #include <QApplication> +#include <QCloseEvent> #include <QDir> #include <QKeyEvent> +#include <QLabel> #include <QLineEdit> #include <QFile> #include <QSettings> @@ -35,6 +37,7 @@ #include "keymap.h" #include "mainwindow.h" #include "messageview.h" +#include "notmuchworker.h" #include "threadlistmodel.h" /// MainWindow is mostly wiring, and the parts that need a real database are @@ -59,6 +62,10 @@ private slots: void markReadTimerRestartsRatherThanStacking(); void markReadTimerIsNotArmedForAReadThread(); void markReadCanBeDisabled(); + void pendingEditCountSurvivesAQuery(); + void aFailedSyncDoesNotClearThePendingCount(); + void closingWithNoPendingEditsDoesNotPrompt(); + void syncOnExitNeverClosesSilently(); }; void TestMainWindow::everyKnownActionIsRegistered() @@ -428,6 +435,162 @@ void TestMainWindow::markReadCanBeDisabled() "a negative mark_read_delay_ms must disable the timer"); } +void TestMainWindow::pendingEditCountSurvivesAQuery() +{ + // The defining property, and the reason this is a counter of its own rather + // than QUndoStack::isClean(): the undo stack is cleared on every query, + // because its entries refer to rows the new result set discards. Tag a + // thread, run any query, and the stack is empty while the change is still + // sitting unsynced in the database. + const Config config; + MainWindow window(config); + + auto *label = window.findChild<QLabel *>(QStringLiteral("pendingEdits")); + QVERIFY(label); + QVERIFY2(label->isHidden(), "the indicator must start hidden at zero"); + + // Confirm a write the way the worker really does, by emitting the signal + // the window listens to. No test-only entry point on MainWindow. + TagChange change; + change.added = { QStringLiteral("deleted") }; + change.description = QStringLiteral("Delete"); + QVERIFY(QMetaObject::invokeMethod(&window, "onTagsApplied", + Q_ARG(TagChange, change))); + + QVERIFY2(!label->isHidden(), "a confirmed edit must show the indicator"); + const QString afterEdit = label->text(); + QVERIFY(!afterEdit.isEmpty()); + + // Now run a query, which clears the undo stack. The indicator must not + // follow it down. + window.findChild<QLineEdit *>()->setText(QStringLiteral("tag:inbox")); + QMetaObject::invokeMethod(&window, "runCurrentQuery"); + + QVERIFY2(!label->isHidden(), + "the indicator was cleared by a query, so it is tracking the undo " + "stack rather than unsynced state"); + QCOMPARE(label->text(), afterEdit); +} + +void TestMainWindow::aFailedSyncDoesNotClearThePendingCount() +{ + // A failed sync means the edits are still unsynced. Clearing here would + // assert the opposite, and the user would quit believing their tagging had + // been carried over. + const Config config; + MainWindow window(config); + + auto *label = window.findChild<QLabel *>(QStringLiteral("pendingEdits")); + QVERIFY(label); + + TagChange change; + change.added = { QStringLiteral("flagged") }; + QVERIFY(QMetaObject::invokeMethod(&window, "onTagsApplied", + Q_ARG(TagChange, change))); + QVERIFY(!label->isHidden()); + const QString afterEdit = label->text(); + + QMetaObject::invokeMethod(&window, "onSyncFinished", + Q_ARG(bool, false), Q_ARG(int, 1)); + QVERIFY2(!label->isHidden(), "a FAILED sync cleared the pending count"); + QCOMPARE(label->text(), afterEdit); + + // A successful one does clear it, so this is not "never clears". + QMetaObject::invokeMethod(&window, "onSyncFinished", + Q_ARG(bool, true), Q_ARG(int, 0)); + QVERIFY2(label->isHidden(), "a successful sync must clear the indicator"); +} + +/// Closes a window and reports whether it accepted, failing rather than hanging +/// if a modal appears. +/// +/// A modal spins its own event loop, so a test that simply sends a close event +/// blocks forever when a dialog it did not expect opens. This arms a timer that +/// closes any active modal and records that one was there, which turns "a +/// dialog appeared" into an assertion instead of a hung run. +struct CloseProbe +{ + bool accepted = false; + bool sawModal = false; + + void run(MainWindow *window) + { + QTimer poll; + poll.setInterval(50); + int ticks = 0; + QObject::connect(&poll, &QTimer::timeout, [this, &poll, &ticks]() { + if (QWidget *modal = QApplication::activeModalWidget()) { + sawModal = true; + modal->close(); + poll.stop(); + return; + } + if (++ticks > 20) // one second is ample for a synchronous close + poll.stop(); + }); + poll.start(); + + QCloseEvent event; + QApplication::sendEvent(window, &event); + accepted = event.isAccepted(); + poll.stop(); + } +}; + +void TestMainWindow::closingWithNoPendingEditsDoesNotPrompt() +{ + // Nothing outstanding means nothing to ask about. If a prompt fires here it + // is keying off something other than there being work to lose, and every + // quit would carry a dialog. + const Config config; + MainWindow window(config); + + CloseProbe probe; + probe.run(&window); + + QVERIFY2(!probe.sawModal, "a clean window prompted on close"); + QVERIFY2(probe.accepted, "a clean window refused to close"); +} + +void TestMainWindow::syncOnExitNeverClosesSilently() +{ + // "never" is the behaviour that existed before the prompt did, and it has + // to stay reachable for anyone who does not want to be asked. It must hold + // whether or not a sync command is configured, so this covers both: the + // no-command path has its own dialog, and "never" must skip that one too. + QTemporaryDir dir; + const QString path = dir.filePath(QStringLiteral("qtmaildir.conf")); + { + QFile file(path); + QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text)); + file.write("[general]\nsync_on_exit=never\n\n[sync]\ncommand=/bin/true\n"); + } + + Config config; + config.load(path); + QCOMPARE(config.syncOnExit(), Config::SyncOnExit::Never); + + MainWindow window(config); + + // Give it something to lose, so this is not passing for the same reason + // the previous test does. + TagChange change; + change.added = { QStringLiteral("deleted") }; + QVERIFY(QMetaObject::invokeMethod(&window, "onTagsApplied", + Q_ARG(TagChange, change))); + auto *label = window.findChild<QLabel *>(QStringLiteral("pendingEdits")); + QVERIFY(label); + QVERIFY(!label->isHidden()); + + CloseProbe probe; + probe.run(&window); + + QVERIFY2(!probe.sawModal, + "sync_on_exit=never prompted anyway"); + QVERIFY2(probe.accepted, + "sync_on_exit=never must close without prompting"); +} + // Constructing a MainWindow needs a QApplication and a platform plugin. The // test has no display under ctest, so it runs offscreen unless the caller // asked for something else. |