diff options
| -rw-r--r-- | CHANGELOG.md | 18 | ||||
| -rw-r--r-- | README.md | 8 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 48 | ||||
| -rw-r--r-- | src/config.cpp | 17 | ||||
| -rw-r--r-- | src/config.h | 15 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 82 | ||||
| -rw-r--r-- | src/mainwindow.h | 19 | ||||
| -rw-r--r-- | tests/test_config.cpp | 80 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 252 |
9 files changed, 537 insertions, 2 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 2603b4f..2eda4e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,24 @@ point at which they are stable. ## [Unreleased] +### Added + +- A tag change now syncs itself out, about two seconds after you stop making + changes, instead of waiting for the Sync button or your cron job. The delay is + a debounce, so tagging several threads in a row produces one sync rather than + one per thread, and a sync already running is never interrupted or queued + behind. Set `auto_sync_delay_ms` in `[general]` to change the delay, or to any + negative value to turn the behaviour off and get the previous one back. + +### Fixed + +- A sync you started no longer clears the message pane and the undo stack. It + now reconciles the thread list the way a background sync already did, so a + message stays on screen and open while the list updates around it. If the + thread has stopped matching the current query, which is what happens when the + message you are reading in Unread gets marked read, the pane keeps showing it + and offers "Show it anyway" instead of going blank. + ## [0.15.0] - 2026-08-11 Sent mail becomes a place you can go. A Sent button beside Inbox, Unread and @@ -124,6 +124,14 @@ 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. How long to wait after a tag change before syncing it out for you, +; in milliseconds. Defaults to 2000. The delay is a debounce, so tagging several +; threads in a row produces one sync after you stop, not one per thread. Zero +; syncs immediately; any negative value turns this off, leaving changes for the +; Sync button or your own cron job, which is how every release before 0.16.0 +; behaved. A sync already running, including one started by cron, is never +; interrupted or queued behind: the change simply stays pending. +; auto_sync_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 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 9957ba8..2663b44 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 @@ -127,7 +127,7 @@ taking that too literally. | 68 | A forwarded subject gets no `passed` tag | workflow | S | open; no subject rule exists, measured 2026-08-11. Decision needed: display mark (XS) or write the flag (S, syncs out) | | 69 | `passed` and `replied` read as words where every other state is a glyph | presentation | S | open; depends on 68 for what `passed` means | | 70 | Pane icons are a private set where the main window uses the system theme | presentation | M | open | -| 71 | A toolbar action does not sync, so the edit sits until the next cron run | workflow | S | open; user decision on the delay | +| 71 | A toolbar action does not sync, so the edit sits until the next cron run | workflow | S | **done** 2026-08-11; 2s default, `auto_sync_delay_ms` | | 72 | No khard/khal integration | workflow | ? | open, unspecified; the user places it after send, so v2 at the earliest | | 73 | This backlog is past four thousand lines | maintenance | S | open | @@ -4447,6 +4447,52 @@ lock is held. The undo stack has to survive it, which is what item 35 built. **Size: S** once the delay is chosen. +**Done 2026-08-11.** The delay is 2000ms by default, chosen by the user, and +configurable as `auto_sync_delay_ms` in `[general]`. It follows +`mark_read_delay_ms` exactly, including that zero and negative are not errors: +zero syncs on the next trip through the event loop and any negative value +restores the pre-0.16.0 behaviour, which is the switch for a user who wants only +their cron job. + +Armed from `onTagsApplied`, where a write is CONFIRMED and the pending count is +already current, rather than where one is sent: a sync scheduled for a write the +worker went on to reject would run for nothing. It is a debounce and not a +schedule, restarted by each confirmed edit, because "mark all read" confirms one +write per thread in the view and an arm-per-edit timer would be the storm of +syncs the debounce exists to prevent. Nothing is armed when the delay is +negative, when no sync command is configured, or when the pending count is zero, +which is the case where an edit was netted against its own inverse (item 28). + +The constraints held: `runAutoSync` skips rather than queues when +`m_externalSyncBusy` or a local sync is running, and the edits stay pending +rather than being lost. Item 35's refresh keeps the undo stack. + +Four tests in `test_mainwindow` and four in `test_config`, each mutation-checked: +removing the `scheduleAutoSync()` call, honouring a negative delay, dropping the +nothing-pending guard and dropping the already-running guard each failed a test. +**Follow-up, found by hand testing the same day.** Reading a message in the +Unread view, the automatic mark-read tagged it, the automatic sync fired two +seconds later, and the message pane went blank. The stale-thread notice (item 35) +exists for exactly this and was not the problem: `onSyncFinished` called +`runCurrentQuery()` where the cron path calls `refreshCurrentQuery()`, and a +re-run clears the model, the undo stack and the pane, so there was nothing left +for the notice to describe. The two paths had no reason to differ; before item 71 +a local sync followed only a click on Sync, where blanking was at least +explicable, so the difference went unnoticed. `onSyncFinished` now refreshes. + +Its test asserts on the UNDO STACK, not on the pane. Both paths issue a queued +query that `test_mainwindow` has no worker to answer, so the pane ends up blank +either way and an assertion on it passes against both; the undo stack is cleared +by one and kept by the other, so it names which path ran. Mutation-checked by +restoring `runCurrentQuery()`. + +Two traps met while writing them and worth keeping. The helper first wrote +`general/auto_sync_delay_ms` and the key silently matched nothing, leaving the +default in place, exactly the QSettings `[general]` behaviour recorded in +CLAUDE.md. And a debounce assertion comparing `remainingTime()` before and after +with `>` is FLAKY, since both reads can land in the same millisecond; assert the +remaining time went back up near the full interval instead. + ## 72. No khard/khal integration **Observed (user, from the notes):** "investigate khard/khal integration (light diff --git a/src/config.cpp b/src/config.cpp index a81651f..b1f730c 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -271,6 +271,23 @@ void Config::load(const QString &path) } } + // Item 71. Same shape as mark_read_delay_ms above, including that zero and + // negative are not errors: 0 syncs on the next trip through the event loop + // and negative disables the automatic sync entirely, which is how a user who + // wants only their cron job turns this off. + const QVariant autoSync = settings.value(QStringLiteral("auto_sync_delay_ms")); + if (autoSync.isValid()) { + bool ok = false; + const int value = autoSync.toString().toInt(&ok); + if (ok) { + m_autoSyncDelayMs = value; + } else { + addProblem(QStringLiteral("Auto-sync delay '%1' is not a number; " + "using the default.") + .arg(autoSync.toString())); + } + } + // [completion] is an ordinary section, so this one DOES take its prefix. // ',' separates entries and '|' separates a value from its description: // two different characters because QSettings splits comma lists itself, diff --git a/src/config.h b/src/config.h index 4092141..ea0b055 100644 --- a/src/config.h +++ b/src/config.h @@ -213,6 +213,20 @@ public: /// the behaviour so a thread stays unread until toggled by hand. int markReadDelayMs() const { return m_markReadDelayMs; } + /// How long to wait after a tag edit before syncing it out on the user's + /// behalf. Item 71. + /// + /// Same three meanings as markReadDelayMs() above, and deliberately so: a + /// positive value is the debounce in milliseconds, 0 syncs on the next trip + /// through the event loop, and any negative value disables the behaviour so + /// edits wait for a manual sync or the user's cron job, which is what every + /// release before this one did. + /// + /// Defaults to 2000. The delay is a debounce, not a schedule: each edit + /// restarts it, so a burst of tagging produces one sync after the burst + /// rather than one per tag. + int autoSyncDelayMs() const { return m_autoSyncDelayMs; } + /// User-supplied mimetype completions, APPENDED to the built-in list. /// Appending rather than replacing means a typo cannot leave completion /// worse off than the defaults. Mimetypes are the only completion list a @@ -250,6 +264,7 @@ private: qreal m_messageZoom = 1.0; bool m_completionOnFocus = false; int m_markReadDelayMs = 2000; + int m_autoSyncDelayMs = 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 824bdca..04e951d 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -485,6 +485,13 @@ void MainWindow::buildUi() connect(m_markReadTimer, &QTimer::timeout, this, &MainWindow::markCurrentThreadRead); + m_autoSyncTimer = new QTimer(this); + // Named for the same reason: a test can assert that an edit armed the + // debounce without waiting out the delay or starting a real mbsync. + m_autoSyncTimer->setObjectName(QStringLiteral("autoSyncTimer")); + m_autoSyncTimer->setSingleShot(true); + connect(m_autoSyncTimer, &QTimer::timeout, this, &MainWindow::runAutoSync); + // The pane and its close button travel together: a QPlainTextEdit has // nowhere to put one, and a pane that appears on a failed sync and can // never be dismissed is worse than one that does not appear at all. @@ -2194,7 +2201,22 @@ void MainWindow::onSyncFinished(bool success, int exitCode) return; } - runCurrentQuery(); + // refreshCurrentQuery(), NOT runCurrentQuery(). A sync this window + // started is not a query the user asked to re-run: runCurrentQuery() + // clears the model, the undo stack and the message pane, so a sync + // landing while a message was open read the user out of it. The cron + // path has reconciled instead since item 35, and there was never a + // reason for the two to differ. + // + // Item 71 is what made it matter. A local sync used to happen only + // when the user clicked Sync, where blanking was at least explicable; + // the automatic one fires two seconds after a tag edit, which is + // precisely when the user is still reading the message they tagged. + // Reconciling keeps the pane, and updateStaleThreadNotice() then offers + // "Show it anyway" for a thread that has stopped matching the query, + // which is the reported case: reading in Unread, the thread is marked + // read, and it no longer belongs to the view it was opened from. + refreshCurrentQuery(); // A sync is the usual way new tags enter the database. requestAllTags(); } else if (exitCode == kSyncSkippedExitCode) { @@ -2280,6 +2302,12 @@ void MainWindow::onTagsApplied(const TagChange &change) updatePendingIndicator(); + // Item 71. Armed here, where a write is CONFIRMED and the pending count is + // already up to date, for the same reason recordPendingEdit() is called + // here: a sync scheduled for a write the worker went on to reject would run + // for nothing. + scheduleAutoSync(); + // 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. @@ -2678,6 +2706,58 @@ void MainWindow::startSync() setSyncBusy(true); } +void MainWindow::scheduleAutoSync() +{ + // Negative disables the behaviour entirely, per the config key, and that is + // the pre-0.16.0 behaviour: edits wait for a manual sync or the user's cron + // job. Checked before anything else so a disabled delay arms nothing. + const int delay = m_config.autoSyncDelayMs(); + if (delay < 0) + return; + + // No sync command means the Sync action is already disabled and startSync() + // would only put "No sync command configured" in the status bar. Arming a + // timer to say that on a delay, for something the user did not ask for, is + // worse than staying quiet. + if (!m_sync || !m_sync->isAvailable()) + return; + + // Nothing outstanding, nothing to carry. An edit netted against its own + // inverse leaves the count at zero (item 28), and syncing for it would run + // mbsync over a mail store that is already where the server left it. + if (pendingEditCount() == 0) + return; + + // Restart, not stack. Tagging a multi-row selection confirms one write per + // thread and "mark all read" confirms one per thread in the view, so an + // armed-per-edit timer would be exactly the storm of syncs a debounce is + // for. The last edit of a burst decides when the single sync happens. + m_autoSyncTimer->start(delay); +} + +void MainWindow::runAutoSync() +{ + // The user can have synced by hand, or undone the edit, in the delay. Both + // leave nothing to carry, and re-checking here rather than trusting the arm + // is what makes the debounce safe to restart freely. + if (pendingEditCount() == 0) + return; + + // Skip rather than queue when a sync is already in flight, which item 71 + // requires: the cron job holds the same lock, and mbsync's own answer to a + // second run is to fail on it. The edits are not lost by skipping. They stay + // pending, and the sync already running is very likely to carry them, since + // they reached the mail store at edit time. + // + // m_externalSyncBusy covers the cron job SyncMonitor can see. A lock taken + // between that poll and now is not visible here, and does not need to be: + // MailSync::start() fails on a second run and startSync() reports it. + if (m_externalSyncBusy || (m_sync && m_sync->isRunning())) + return; + + startSync(); +} + void MainWindow::recordPendingEdit(const QString &messageId, const QString &tag, bool added) { diff --git a/src/mainwindow.h b/src/mainwindow.h index 0912520..19635dd 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -390,6 +390,17 @@ private: /// Redraws the unsynced-edits indicator from pendingEditCount(). void updatePendingIndicator(); + /// Arms the debounce that syncs a confirmed tag edit out on its own + /// (item 71). Does nothing when the delay is negative, when no sync command + /// is configured, or when nothing is actually pending. + void scheduleAutoSync(); + + /// Starts the debounced automatic sync, unless a sync is already running + /// (local or external) or the edits it would carry are already gone. + /// + /// Q_INVOKABLE so a test can fire the debounce without waiting it out. + Q_INVOKABLE void runAutoSync(); + /// Records one confirmed (message, tag) change, cancelling it against an /// opposite change already outstanding for the same pair. void recordPendingEdit(const QString &messageId, const QString &tag, @@ -756,6 +767,14 @@ private: /// nothing rather than marking the wrong one. QString m_markReadThreadId; + /// Debounces the automatic sync that follows a tag edit (item 71). + /// + /// Single-shot and RESTARTED by every confirmed edit, for the same reason + /// m_markReadTimer is: tagging a multi-row selection confirms one write per + /// thread, and one sync per thread is exactly what a debounce exists to + /// prevent. + QTimer *m_autoSyncTimer = nullptr; + /// The optimistic update awaiting confirmation, kept so a worker error can /// put the model back. Only the most recent one: mutations are sent from /// the UI thread one user action at a time. diff --git a/tests/test_config.cpp b/tests/test_config.cpp index dad4ecd..95398e6 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -35,6 +35,10 @@ private slots: void toolbarIconSizeIsActuallyRead(); void toolbarIconSizeIsClampedAndReported(); void toolbarIconSizeRejectsGarbage(); + void autoSyncDelayDefaultsTo2000(); + void autoSyncDelayIsActuallyRead(); + void autoSyncDelayKeepsZeroAndNegative(); + void autoSyncDelayRejectsGarbage(); void accountWithoutMaildirIsRejected(); void scopedQueryWrapsCorrectly(); void absentSyncCommandIsNoticeNotProblem(); @@ -268,6 +272,82 @@ void TestConfig::toolbarIconSizeRejectsGarbage() QVERIFY(!config.problems().isEmpty()); } +void TestConfig::autoSyncDelayDefaultsTo2000() +{ + QTemporaryDir dir; + const QString path = writeIni(dir, QStringLiteral("[general]\n")); + + Config config; + config.load(path); + + QCOMPARE(config.autoSyncDelayMs(), 2000); + QVERIFY(config.problems().isEmpty()); +} + +void TestConfig::autoSyncDelayIsActuallyRead() +{ + // [general] keys are read WITHOUT the general/ prefix. A key that silently + // matched nothing would leave the 2000 default in place and pass every + // behavioural test in test_mainwindow, since those arm the timer at the + // default anyway. + QTemporaryDir dir; + const QString path = writeIni(dir, QStringLiteral( + "[general]\n" + "auto_sync_delay_ms = 500\n" + )); + + Config config; + config.load(path); + + QCOMPARE(config.autoSyncDelayMs(), 500); + QVERIFY(config.problems().isEmpty()); +} + +void TestConfig::autoSyncDelayKeepsZeroAndNegative() +{ + // Neither is an error and neither may be clamped: 0 means sync on the next + // trip through the event loop, and negative disables the automatic sync, + // which is the only way to get the pre-0.16.0 behaviour back. Clamping + // either to the default would take that switch away. + QTemporaryDir dir; + const QString zero = writeIni(dir, QStringLiteral( + "[general]\n" + "auto_sync_delay_ms = 0\n" + )); + + Config immediate; + immediate.load(zero); + QCOMPARE(immediate.autoSyncDelayMs(), 0); + QVERIFY(immediate.problems().isEmpty()); + + QTemporaryDir dir2; + const QString off = writeIni(dir2, QStringLiteral( + "[general]\n" + "auto_sync_delay_ms = -1\n" + )); + + Config disabled; + disabled.load(off); + QCOMPARE(disabled.autoSyncDelayMs(), -1); + QVERIFY(disabled.problems().isEmpty()); +} + +void TestConfig::autoSyncDelayRejectsGarbage() +{ + // Falls back to the default and says so, matching mark_read_delay_ms. + QTemporaryDir dir; + const QString path = writeIni(dir, QStringLiteral( + "[general]\n" + "auto_sync_delay_ms = soon\n" + )); + + Config config; + config.load(path); + + QCOMPARE(config.autoSyncDelayMs(), 2000); + QVERIFY(!config.problems().isEmpty()); +} + void TestConfig::accountWithoutMaildirIsRejected() { QTemporaryDir dir; diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 818e5b4..18fe8c1 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -81,6 +81,11 @@ private slots: void returnInTheQueryBarRunsTheQueryNotOpenThread(); void markReadTimerRestartsRatherThanStacking(); void markReadTimerIsNotArmedForAReadThread(); + void aConfirmedEditArmsTheAutoSync(); + void autoSyncDebouncesABurstOfEdits(); + void autoSyncIsNotArmedWhenDisabledOrWithNothingPending(); + void autoSyncSkipsWhileABackgroundSyncIsRunning(); + void aSuccessfulSyncRefreshesRatherThanRerunningTheQuery(); void markReadCanBeDisabled(); void pendingEditCountSurvivesAQuery(); void aFailedSyncDoesNotClearThePendingCount(); @@ -3544,6 +3549,253 @@ void TestMainWindow::theSyncActionIsDisabledWhileABackgroundSyncHoldsTheLock() MainWindow::setLocksPathForTesting(QStringLiteral("/proc/locks")); } +// Item 71. A confirmed tag edit arms a debounce that syncs it out, so an edit +// no longer waits for a manual sync or the user's cron job. +// +// All four of these assert on the TIMER rather than on a sync actually running. +// Starting a real one from a test would launch the configured command, and the +// thing worth guarding here is the decision to sync, not QProcess. +static QString writeSyncConfig(QTemporaryDir &dir, const QString &extra = {}) +{ + QDir().mkpath(dir.filePath(QStringLiteral("qtmaildir"))); + const QString conf = dir.filePath(QStringLiteral("qtmaildir/qtmaildir.conf")); + QSettings s(conf, QSettings::IniFormat); + // /bin/true exists, so Config keeps it and MailSync reports available. A + // config with no command disables the automatic sync by design, which would + // make every one of these tests pass against a stub. + s.setValue(QStringLiteral("sync/command"), QStringLiteral("/bin/true")); + // NOT "general/auto_sync_delay_ms": QSettings' INI backend treats a section + // literally named [general] as its own fallback section and strips it, so a + // prefixed lookup silently matches nothing. Writing it prefixed here left + // the default in place and the -1 case read 2000. + if (!extra.isEmpty()) + s.setValue(QStringLiteral("auto_sync_delay_ms"), extra); + s.sync(); + return conf; +} + +void TestMainWindow::aConfirmedEditArmsTheAutoSync() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + config.load(writeSyncConfig(dir)); + QCOMPARE(config.autoSyncDelayMs(), 2000); + + MainWindow window(config); + auto *timer = window.findChild<QTimer *>(QStringLiteral("autoSyncTimer")); + QVERIFY2(timer, "no autoSyncTimer to observe"); + QVERIFY2(!timer->isActive(), "the debounce is armed before any edit"); + + TagChange change; + change.messageIds = { QStringLiteral("m1") }; + change.added = { QStringLiteral("flagged") }; + QVERIFY(QMetaObject::invokeMethod(&window, "onTagsApplied", + Q_ARG(TagChange, change))); + + QVERIFY2(timer->isActive(), "a confirmed edit did not arm the automatic sync"); + QCOMPARE(timer->interval(), 2000); +} + +void TestMainWindow::autoSyncDebouncesABurstOfEdits() +{ + // The point of the debounce. "Mark all read" confirms one write per thread, + // and one sync per thread is what this prevents. A timer that STACKED would + // still be active here, so the assertion is on the count of timers and on + // the remaining interval having been reset, not merely on isActive(). + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + config.load(writeSyncConfig(dir)); + + MainWindow window(config); + auto *timer = window.findChild<QTimer *>(QStringLiteral("autoSyncTimer")); + QVERIFY(timer); + + TagChange first; + first.messageIds = { QStringLiteral("m1") }; + first.added = { QStringLiteral("flagged") }; + QVERIFY(QMetaObject::invokeMethod(&window, "onTagsApplied", + Q_ARG(TagChange, first))); + QVERIFY(timer->isActive()); + + // Long enough that a restart is unambiguous. Comparing remainingTime() + // before and after with ">" was tried and is FLAKY: the two reads can land + // in the same millisecond bucket, and the test then fails against correct + // code. Assert instead that the remaining time went back up near the full + // interval, which a stacked or un-restarted timer cannot produce. + QTest::qWait(500); + const int afterWait = timer->remainingTime(); + QVERIFY2(afterWait < 1800, "the timer did not start counting down"); + + TagChange second; + second.messageIds = { QStringLiteral("m2") }; + second.added = { QStringLiteral("flagged") }; + QVERIFY(QMetaObject::invokeMethod(&window, "onTagsApplied", + Q_ARG(TagChange, second))); + + QVERIFY2(timer->remainingTime() > 1800, + "the second edit did not restart the debounce, so a burst of edits " + "syncs on the schedule of the FIRST one"); + QCOMPARE(window.findChildren<QTimer *>(QStringLiteral("autoSyncTimer")).size(), + 1); +} + +void TestMainWindow::autoSyncIsNotArmedWhenDisabledOrWithNothingPending() +{ + // A negative delay is the switch that restores the pre-0.16.0 behaviour, so + // it must arm nothing at all. + QTemporaryDir off; + QVERIFY(off.isValid()); + Config disabled; + disabled.load(writeSyncConfig(off, QStringLiteral("-1"))); + QCOMPARE(disabled.autoSyncDelayMs(), -1); + + MainWindow disabledWindow(disabled); + auto *disabledTimer = + disabledWindow.findChild<QTimer *>(QStringLiteral("autoSyncTimer")); + QVERIFY(disabledTimer); + + TagChange change; + change.messageIds = { QStringLiteral("m1") }; + change.added = { QStringLiteral("flagged") }; + QVERIFY(QMetaObject::invokeMethod(&disabledWindow, "onTagsApplied", + Q_ARG(TagChange, change))); + QVERIFY2(!disabledTimer->isActive(), + "auto_sync_delay_ms = -1 still armed a sync"); + + // An edit netted against its own inverse leaves nothing outstanding (item + // 28), and syncing for it would run mbsync over an unchanged mail store. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + config.load(writeSyncConfig(dir)); + + MainWindow window(config); + auto *timer = window.findChild<QTimer *>(QStringLiteral("autoSyncTimer")); + QVERIFY(timer); + + TagChange added; + added.messageIds = { QStringLiteral("m1") }; + added.added = { QStringLiteral("unread") }; + QVERIFY(QMetaObject::invokeMethod(&window, "onTagsApplied", + Q_ARG(TagChange, added))); + QVERIFY2(timer->isActive(), "the first edit did not arm anything, so the " + "netting assertion below proves nothing"); + + timer->stop(); + TagChange undone; + undone.messageIds = { QStringLiteral("m1") }; + undone.removed = { QStringLiteral("unread") }; + QVERIFY(QMetaObject::invokeMethod(&window, "onTagsApplied", + Q_ARG(TagChange, undone))); + QVERIFY2(!timer->isActive(), + "an edit and its inverse left nothing pending but still armed a sync"); +} + +void TestMainWindow::autoSyncSkipsWhileABackgroundSyncIsRunning() +{ + // Item 71 requires skipping rather than queueing: the cron job holds the + // same lock and mbsync's answer to a second run is to fail on it. The edits + // are not lost, they stay pending. + // + // The locks path is redirected so this does not depend on whether a real + // sync is running on the machine, which is item 61's failure mode. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + const QString locks = dir.filePath(QStringLiteral("locks")); + { + QFile f(locks); + QVERIFY(f.open(QIODevice::WriteOnly)); + } + MainWindow::setLocksPathForTesting(locks); + + Config config; + config.load(writeSyncConfig(dir)); + + MainWindow window(config); + auto *timer = window.findChild<QTimer *>(QStringLiteral("autoSyncTimer")); + QVERIFY(timer); + + QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged", + Q_ARG(SyncMonitor::State, + SyncMonitor::State::Running)); + + TagChange change; + change.messageIds = { QStringLiteral("m1") }; + change.added = { QStringLiteral("flagged") }; + QVERIFY(QMetaObject::invokeMethod(&window, "onTagsApplied", + Q_ARG(TagChange, change))); + + // Armed, because the edit is real and will still need carrying. The skip + // belongs to the moment the timer FIRES, not to arming it. + QVERIFY2(timer->isActive(), "the edit did not arm the debounce at all"); + + auto *label = window.findChild<QLabel *>(QStringLiteral("pendingEdits")); + QVERIFY(label); + QVERIFY(!label->isHidden()); + + QVERIFY(QMetaObject::invokeMethod(&window, "runAutoSync")); + + // The edit is still pending: a skipped sync must not clear the indicator, + // which is the fault that would leave the user quitting on unsynced work. + QVERIFY2(!label->isHidden(), + "a skipped automatic sync cleared the pending indicator"); + + MainWindow::setLocksPathForTesting(QStringLiteral("/proc/locks")); +} + +void TestMainWindow::aSuccessfulSyncRefreshesRatherThanRerunningTheQuery() +{ + // Reported by hand against item 71: reading a message in the Unread view, + // the automatic mark-read tags it, the automatic sync fires two seconds + // later, and the message pane went blank because the thread had stopped + // matching "tag:unread". + // + // The cause was not the stale-thread notice, which handles exactly this and + // has since item 35. It was that onSyncFinished() called runCurrentQuery() + // where the cron path calls refreshCurrentQuery(): a re-run clears the + // model, the undo stack and the pane, so there was nothing left for the + // notice to describe. Before item 71 a local sync only ever followed a + // click on Sync, which is why the difference went unnoticed. + // + // Asserted on the UNDO STACK rather than on the pane. Both paths issue a + // queued query this test has no worker to answer, so the pane ends up blank + // either way and an assertion on it would pass against both. The undo stack + // is cleared by runCurrentQuery() and deliberately kept by + // refreshCurrentQuery(), so it names which path actually ran. + const Config config; + MainWindow window(config); + + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + auto *view = window.findChild<QTreeView *>(); + QVERIFY(view); + + window.findChild<QLineEdit *>()->setText(QStringLiteral("tag:unread")); + QMetaObject::invokeMethod(&window, "runCurrentQuery"); + model->appendBatch({ makeThread(QStringLiteral("t1"), + { QStringLiteral("unread") }) }); + QMetaObject::invokeMethod(&window, "onQueryFinished", Q_ARG(int, 1), + Q_ARG(quint64, + window.currentGenerationForTesting())); + + // A real edit, so there is something on the undo stack to lose. + selectThreadRow(view, 0); + auto *markRead = window.findChild<QAction *>(QStringLiteral("mark_all_read")); + QVERIFY(markRead); + markRead->trigger(); + QCOMPARE(window.undoDepthForTesting(), 1); + + QMetaObject::invokeMethod(&window, "onSyncFinished", + Q_ARG(bool, true), Q_ARG(int, 0)); + + QVERIFY2(window.undoDepthForTesting() == 1, + "a successful sync cleared the undo stack, so it re-ran the query " + "instead of refreshing it, and a message open in the pane is read " + "out from under the user"); +} + void TestMainWindow::theStatusBarFollowsTheSyncPhase() { // Item 42: "Syncing..." said nothing about what was happening, while the |