diff options
| -rw-r--r-- | CHANGELOG.md | 7 | ||||
| -rw-r--r-- | README.md | 15 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md | 46 | ||||
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 45 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 55 | ||||
| -rw-r--r-- | src/mainwindow.h | 9 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 196 | ||||
| -rw-r--r-- | translations/qtmaildir_it_IT.ts | 12 |
8 files changed, 322 insertions, 63 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index e1378fc..32c1a34 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,13 @@ point at which they are stable. themes draw as an exclamation mark; that was invisible while it only ever appeared beside its own label, and became an info glyph the moment it reached the icon-only message bar. +- **Sync follows the account you are looking at.** With an account selected in + the dropdown, Sync collects that account rather than every one; with All + accounts selected it collects everything, as before. Any account with + unsynced edits is always included, whichever one is on screen, so choosing a + narrower view cannot leave a change stranded. The status line now names what + a run covers instead of saying only "Syncing...", so a narrowed run is + visible as one. ### Fixed @@ -655,11 +655,16 @@ Two things any replacement has to get right, both learned the hard way: ### Per-account sync -When tag changes are outstanding, a sync passes only the affected accounts' -channel names to the command. When nothing is outstanding the run is a plain -fetch and no names are passed, so every account is synced: narrowing a fetch to -wherever the last edit happened to be would quietly stop collecting mail -everywhere else. +A sync covers the account selected in the dropdown, plus any account with +outstanding tag changes. With **All accounts** selected and nothing +outstanding, no names are passed and the run is a plain fetch over everything: +narrowing a fetch to wherever the last edit happened to be would quietly stop +collecting mail everywhere else. + +The two are a union rather than a choice, so an edit is never left behind by +looking somewhere else. Editing mail in `personal` and then switching the +dropdown to `work` syncs both: `work` because it is on screen, `personal` +because it owes a write. The status line names what a run covers as it starts. The name passed is the **mbsync channel**, which is not always the account's section key. `[account.mail-first.last]` may well be the channel diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md index 6613059..bcb6431 100644 --- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md +++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md @@ -10232,3 +10232,49 @@ sends, and asserts the row arrives with no further gesture: no second against the defect. Mutation-checked by disabling the connection, which fails on the row count. +## 101. Sync is account-aware for edits but not for the account the user is looking at + +**Done 2026-09-08.** The user chose BOTH halves the entry offered, and then +narrowed the first one: not a second action beside Sync, but the existing Sync +made account-aware. "If 'All Accounts' is selected, Sync works as today, if +'work' is selected, hitting Sync works only for 'work'." + +**What was built.** `pendingSyncChannels()` now reads the account dropdown as +well as `m_editedAccounts`, and the two are a UNION rather than one replacing +the other. That union is the whole safety property and is the constraint the +original entry named: looking at one account while having edited another is +ordinary, and a run that dropped the edited account's channel would strand that +write with nothing on screen to say so. All accounts is the empty key and +narrows nothing, so a full fetch is still what an unselected window asks for, +which is what keeps item 49's reasoning intact. The existing fallback survives +untouched: an account whose section names no channel still widens the run to +everything rather than being silently skipped. + +The visibility half is `syncStartedText()`, called from `setSyncBusy()`. A run +used to open with "Syncing..." whether it covered one account or all of them, +which is precisely the invisibility the entry complained about; it now reads +"Syncing all accounts..." or "Syncing work-channel, personal-channel...". The +exit paths pass no channels and overwrite the label with their own wording +immediately, so the parameter is defaulted rather than threaded through them. + +**Four tests**, three on the channel resolution and one on the generated +string. `syncNarrowsToTheSelectedAccount` asserts the list is EMPTY before the +gesture, which matters more than it looks: empty is what a full fetch looks +like, so a test starting from a narrowed state could not tell the fix from a +window that had never widened. `aNarrowedSyncStillCarriesAnEditFromAnotherAccount` +is the union, and it is the one that fails under the plausible wrong +implementation. `theStatusLineNamesWhatASyncWillCover` asserts on the generated +string rather than on a label after a real run, which would be a test of +QProcess timing. + +Mutation-checked twice: ignoring the selection fails all three channel tests, +and replacing the pending set instead of unioning with it fails the third one +alone. That second mutation is the one worth having, since it is the version +that looks correct and loses a write. + +**One thing deliberately not done.** The separator in the channel list is a +plain `", "`, not a translated string. `lupdate` picked it up as translatable +when it was written with `tr()`, and a comma joins a list the same way in +Italian; a translatable separator is a string for a translator to get wrong for +no gain. + 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 681200e..47c46fb 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 @@ -167,7 +167,7 @@ taking that too literally. | 98 | "Important" adds the tag but cannot remove it, unlike every other toggle | defect | XS | **done** 2026-08-17, unreleased. Calls `everySelectedRowHasTag()`, as the entry required. Its reply test needed THREE different states (list-first thread, the reply's own thread, the reply) before it could tell the two wrong answers apart; with the reply defaulted to its thread's state the item 105 mutation stayed green, measured | | 99 | The unread action is labelled "Toggle unread" whichever way it will go | presentation | S | **done 2026-08-25**, unreleased, with 112: the user's note is ONE design across both. The label names the direction it will go, and the entry is hidden on a selection with no single state. `refreshUnreadAction()` reads the new three-valued `selectionTagPresence()` | | 100 | The message pane offers Back, Forward, Reload and Save page, none of which mean anything | defect | XS | **done** 2026-08-17, unreleased. `MessageView::removeBrowserActions()` filters the standard menu by `pageAction()` POINTER, never by text; `ViewSource` went with them, and stranded separators are swept | -| 101 | Sync is account-aware for edits but not for the account the user is looking at | workflow | S | open; item 49 built the edit half deliberately. Needs a decision, see the entry | +| 101 | Sync is account-aware for edits but not for the account the user is looking at | workflow | S | **done 2026-09-08**, unreleased. The user chose both halves the entry offered and narrowed the first: no second action, the existing Sync reads the account dropdown. Selected account narrows the run, All accounts is a full fetch as before, and the edited accounts are a UNION with the selection so a narrowed run cannot strand a write. The status line names what a run covers. See the closed entry | | 102 | The rules table shows no note, so the field explaining a rule is invisible until it is opened | workflow | XS | **done** 2026-08-17, unreleased. A Note column before `ColumnCount`, so the appended Matches column stays last. Found a second defect on the way: `restoreState` REFUSES a header state with a different column count, and the sized flags were being set regardless | | 103 | What Delete does to mail on the server is undocumented and unverified | clarification | S+M | done; Delete moves to the account trash, with Restore and a stranded-mail cleanup. Section in the closed file | | 104 | Mail visible in Thunderbird never reaches qtmaildir | defect | XS | **done 2026-08-25**, hand-tested. The worker never reopened its read-only notmuch handle, so no query saw mail indexed after startup. Confirmed on a sync run from the application that added 20 messages: they appeared without a restart | @@ -484,49 +484,6 @@ literals `lupdate` can see; a string built by concatenation is not translatable. **Size: S.** Mostly the mixed-selection and toolbar decisions, not the code. -## 101. Sync is account-aware for edits but not for the account the user is looking at - -**Observed (user, from the notes):** "sync button should be account-aware." - -**Cause (verified in the code).** `MainWindow::pendingSyncChannels()` -(`src/mainwindow.cpp:3550`) resolves channels from `m_editedAccounts`, the set of -accounts the user has made EDITS in, and from nothing else. The account dropdown -is not consulted. With nothing pending it returns empty on purpose, and -`mailsync.sh` turns that into `mbsync -a`, every channel. - -**Item 49 built exactly this and the reasoning still holds.** The comment states -it: with nothing pending the run is a FETCH, and narrowing a fetch to wherever -the last edit happened would "quietly stop collecting mail everywhere else". -Fetching is global by nature; carrying edits is not. - -**So this needs a decision, not a fix.** The note does not say which of two -things the user means, and they are different features: - -*Sync only the selected account, on demand.* A deliberate "sync this account" -that ignores the pending set, presumably beside the existing Sync rather than -replacing it. Useful when one account is slow and the user wants their mail from -another one now. The risk is the one item 49 named: a button that looks like -Sync and quietly does not collect the rest of the mail. - -*Show which accounts a sync will cover.* No behaviour change at all, just making -the existing account-awareness visible, since today the user cannot tell whether -a run is narrowed or full. The status bar already names each channel as mbsync -reaches it (item 42), so most of this exists. - -**Constraints.** - -- The account dropdown is a VIEW filter. Making it also steer sync couples two - things the user may reasonably want apart: looking at one account while - fetching all of them is the normal case, not an edge case. -- Whatever narrows a run must still carry every pending edit, or an edit is - stranded with nothing on screen to say so. `pendingSyncChannels()` already - falls back to a full sync when it cannot resolve a channel for an edited - account, and that safety must survive. -- An account with no `[account.<key>]` section, or one whose section names no - channel, has no channel to sync. The fallback covers it today. - -**Size: S** for the on-demand button, XS for the visibility half. Ask which. - ## 113. No way to see a message's HTML source **Observed (user, 2026-08-17):** reviewing item 100's removals, "view source diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 41dbe9c..e7cbf33 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -5452,7 +5452,7 @@ void MainWindow::feedSyncPhase(const QString &chunk) m_statusLabel->setText(m_syncPhase.statusText()); } -void MainWindow::setSyncBusy(bool busy) +void MainWindow::setSyncBusy(bool busy, const QStringList &channels) { m_localSyncBusy = busy; updateSyncControls(); @@ -5462,7 +5462,22 @@ void MainWindow::setSyncBusy(bool busy) // output by then. Setting the label is still right, since the tracker has // nothing to say until a line it recognises arrives. if (busy && m_syncPhase.statusText().isEmpty()) - m_statusLabel->setText(tr("Syncing...")); + m_statusLabel->setText(syncStartedText(channels)); +} + +QString MainWindow::syncStartedText(const QStringList &channels) const +{ + // Item 101's visibility half. The run is already account-aware and the user + // could not tell: the same three dots appeared whether mbsync was about to + // collect one account or all of them. Naming the channels makes a narrowed + // run legible at the moment it starts, which is what the item asked for. + // + // Empty means every channel, since that is exactly what pendingSyncChannels() + // returns for a full fetch and what mailsync.sh turns into `mbsync -a`. + if (channels.isEmpty()) + return tr("Syncing all accounts..."); + + return tr("Syncing %1...").arg(channels.join(QStringLiteral(", "))); } void MainWindow::updateSyncControls() @@ -5515,11 +5530,15 @@ void MainWindow::startSync() m_syncPhase.reset(); m_syncLineBuffer.clear(); - if (!m_sync->start(pendingSyncChannels())) { + // Read once and passed on, rather than asked for twice: the label must + // describe the run that actually started, and a second call could disagree + // with the first if the selection moved in between. + const QStringList channels = pendingSyncChannels(); + if (!m_sync->start(channels)) { showTransientStatus(tr("Sync already running")); return; } - setSyncBusy(true); + setSyncBusy(true, channels); } void MainWindow::scheduleAutoSync() @@ -5607,16 +5626,34 @@ void MainWindow::recordPendingEdit(const QString &messageId, const QString &tag, QStringList MainWindow::pendingSyncChannels() const { - // Nothing pending means this run is a FETCH, and a fetch must cover every - // account: narrowing it to wherever the last edit happened to be would + // The account the user is LOOKING at narrows the run, per item 101 and the + // user's decision on it: one Sync action, steered by the dropdown. All + // accounts is the empty key and narrows nothing, which is what keeps item + // 49's property that a fetch collects everywhere by default. + // + // This is deliberately a UNION with the pending set below, never a + // replacement for it. Looking at one account while having edited another is + // ordinary rather than an edge case, and a run that dropped the edited + // account's channel would strand that write with nothing on screen to say + // so. + const QString selected = m_accountBox + ? m_accountBox->currentData().toString() + : QString(); + + QSet<QString> wanted = m_editedAccounts; + if (!selected.isEmpty()) + wanted.insert(selected); + + // Nothing pending and nothing selected means this run is a FETCH over + // everything: narrowing it to wherever the last edit happened to be would // quietly stop collecting mail everywhere else. Empty is the signal for // that, and MailSync::start() appends nothing. - if (m_editedAccounts.isEmpty()) + if (wanted.isEmpty()) return {}; QStringList channels; for (const Account &account : m_config.accounts()) { - if (m_editedAccounts.contains(account.key)) + if (wanted.contains(account.key)) channels.append(account.syncChannel()); } @@ -5624,7 +5661,7 @@ QStringList MainWindow::pendingSyncChannels() const // channel, and syncing a subset that omits it would leave its edits behind // with nothing to say so. Fall back to a full sync, which is correct if // wasteful; the alternative is silently stranding an edit. - if (channels.size() != m_editedAccounts.size()) + if (channels.size() != wanted.size()) return {}; // Stable order so a run is reproducible and the log reads the same way diff --git a/src/mainwindow.h b/src/mainwindow.h index 3829ae2..55d67ae 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -1012,7 +1012,14 @@ private: /// the script's output is unstructured, so a bar that filled from left to /// right would be inventing a fraction nobody knows. An indeterminate one /// says "working, duration unknown", which is the truth. - void setSyncBusy(bool busy); + /// \p channels is the list the run was started with, used only for the + /// opening status line: empty means every account. Defaulted because the + /// exit paths overwrite the label with their own wording immediately after. + void setSyncBusy(bool busy, const QStringList &channels = {}); + + /// The opening status line for a run over \p channels. Item 101's + /// visibility half: a narrowed run must say so. + Q_INVOKABLE QString syncStartedText(const QStringList &channels) const; /// Reassembles lines from a sync output chunk and updates the status label /// when the phase or its detail changes. diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 1c54ebb..b7f7846 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -438,6 +438,10 @@ private slots: void anUnknownExternalStateClearsNothing(); void aSuccessfulCronSyncDrainsTheEditedAccounts(); void aCronSyncDoesNotClearAnEditMadeWhileItRan(); + void syncNarrowsToTheSelectedAccount(); + void syncCoversEveryAccountWhenAllIsSelected(); + void aNarrowedSyncStillCarriesAnEditFromAnotherAccount(); + void theStatusLineNamesWhatASyncWillCover(); void everyActionCarriesAnIcon(); void everyActionIsReachableFromAMenu(); @@ -7496,6 +7500,198 @@ void TestMainWindow::aCronSyncDoesNotClearAnEditMadeWhileItRan() "an edit made after the sync ended was swallowed by it"); } +// Item 101. + +namespace { + +/// Writes a config with two accounts, each naming its own sync channel, so a +/// narrowed run can be told from a full one by the channel list alone. +void loadTwoAccountConfig(Config &config, const QTemporaryDir &dir, + const QString &logPath) +{ + const QString path = dir.filePath(QStringLiteral("qtmaildir.conf")); + QFile file(path); + QVERIFY(file.open(QIODevice::WriteOnly | QIODevice::Text)); + file.write(QStringLiteral("[sync]\nlog=%1\nstatus=%2\n\n" + "[account.work]\n" + "maildir=work-mail\n" + "channel=work-channel\n\n" + "[account.personal]\n" + "maildir=personal-mail\n" + "channel=personal-channel\n") + .arg(logPath, dir.filePath(QStringLiteral("no-status.json"))) + .toUtf8()); + file.close(); + + config.load(path); + QCOMPARE(config.accounts().size(), 2); +} + +/// Records one edit against \p accountKey, by giving the model a thread tagged +/// for that account and sending a tag change over it. This is the route a real +/// edit takes, so m_editedAccounts is populated the way production populates it. +void recordEditForAccount(MainWindow &window, const QString &threadId, + const QString &accountKey) +{ + auto *model = window.findChild<ThreadListModel *>(); + QVERIFY(model); + ThreadSummary thread; + thread.threadId = threadId; + thread.subject = QStringLiteral("Subject"); + thread.totalCount = 1; + thread.firstMessageId = threadId + QStringLiteral("-m1"); + thread.tags = { QStringLiteral("account-") + accountKey, + QStringLiteral("inbox") }; + model->appendBatch({ thread }); + QCOMPARE(model->accountKeysForThread(threadId), QStringList{ accountKey }); + + QVERIFY(QMetaObject::invokeMethod( + &window, "sendThreadTagChange", + Q_ARG(QStringList, QStringList{ threadId }), + Q_ARG(QStringList, QStringList{ QStringLiteral("flagged") }), + Q_ARG(QStringList, QStringList{}), + Q_ARG(QString, QStringLiteral("Flag")))); +} + +/// QVERIFY2 expands to a bare `return`, so the invoke check cannot live in a +/// value-returning helper. Out-param instead, called through the macro below. +void readSyncChannels(MainWindow &window, QStringList &channels) +{ + QVERIFY2(QMetaObject::invokeMethod(&window, "pendingSyncChannels", + Q_RETURN_ARG(QStringList, channels)), + "pendingSyncChannels() could not be invoked"); +} + +#define SYNC_CHANNELS(window, out) \ + QStringList out; \ + readSyncChannels((window), (out)) + +} // namespace + +void TestMainWindow::syncNarrowsToTheSelectedAccount() +{ + // The user's decision on item 101: one Sync action, steered by the account + // dropdown. Selecting an account and pressing Sync collects that account + // only, rather than every channel. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + const QString logPath = dir.filePath(QStringLiteral("mailsync.log")); + writeSyncLog(logPath, QStringLiteral("OK")); + + Config config; + loadTwoAccountConfig(config, dir, logPath); + MainWindow window(config); + + // The guard, and it matters: an empty list is what a FULL sync looks like, + // so a test starting from a narrowed state could not tell the fix from a + // window that had never widened. Nothing is selected and nothing is edited, + // so this must be the full-fetch signal before the gesture. + SYNC_CHANNELS(window, before); + QVERIFY2(before.isEmpty(), + "a fresh window with All accounts selected did not ask for a " + "full fetch"); + + window.selectAccountForTesting(QStringLiteral("work")); + + SYNC_CHANNELS(window, narrowed); + QCOMPARE(narrowed, QStringList{ QStringLiteral("work-channel") }); +} + +void TestMainWindow::syncCoversEveryAccountWhenAllIsSelected() +{ + // The other half of the same rule, and the one that keeps item 49's + // property: All accounts means a full fetch, which mailsync.sh turns into + // mbsync -a. Asserted after a narrowing so it proves the selection is read + // each time rather than latched once. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + const QString logPath = dir.filePath(QStringLiteral("mailsync.log")); + writeSyncLog(logPath, QStringLiteral("OK")); + + Config config; + loadTwoAccountConfig(config, dir, logPath); + MainWindow window(config); + + window.selectAccountForTesting(QStringLiteral("work")); + SYNC_CHANNELS(window, narrowed); + QCOMPARE(narrowed, QStringList{ QStringLiteral("work-channel") }); + + window.selectAccountForTesting(QString()); + + SYNC_CHANNELS(window, widened); + QVERIFY2(widened.isEmpty(), + "going back to All accounts left the run narrowed"); +} + +void TestMainWindow::aNarrowedSyncStillCarriesAnEditFromAnotherAccount() +{ + // Item 101's standing constraint, and the reason the selection is a UNION + // with the pending set rather than a replacement for it. Looking at one + // account while having edited another is ordinary, and a run that dropped + // the other account's channel would strand that edit with nothing on + // screen to say so. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + const QString logPath = dir.filePath(QStringLiteral("mailsync.log")); + writeSyncLog(logPath, QStringLiteral("OK")); + + Config config; + loadTwoAccountConfig(config, dir, logPath); + MainWindow window(config); + + // Edit in personal, then go and look at work. + recordEditForAccount(window, QStringLiteral("t1"), + QStringLiteral("personal")); + SYNC_CHANNELS(window, edited); + QCOMPARE(edited, QStringList{ QStringLiteral("personal-channel") }); + + window.selectAccountForTesting(QStringLiteral("work")); + + // Both: work because it is on screen, personal because it owes a write. + const QStringList expected{ QStringLiteral("personal-channel"), + QStringLiteral("work-channel") }; + SYNC_CHANNELS(window, both); + QCOMPARE(both, expected); +} + +void TestMainWindow::theStatusLineNamesWhatASyncWillCover() +{ + // Item 101's visibility half. The run was already account-aware before this + // item and the user could not tell, since a narrowed run and a full one + // showed the same three dots. Asserted on the generated STRING, which has a + // right answer, rather than on the label after a real run, which would be a + // test of QProcess timing. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + const QString logPath = dir.filePath(QStringLiteral("mailsync.log")); + writeSyncLog(logPath, QStringLiteral("OK")); + + Config config; + loadTwoAccountConfig(config, dir, logPath); + MainWindow window(config); + + QString text; + + // Empty is the full-fetch signal, so it must not read as a channel list. + QVERIFY(QMetaObject::invokeMethod(&window, "syncStartedText", + Q_RETURN_ARG(QString, text), + Q_ARG(QStringList, QStringList{}))); + QCOMPARE(text, QStringLiteral("Syncing all accounts...")); + + QVERIFY(QMetaObject::invokeMethod( + &window, "syncStartedText", Q_RETURN_ARG(QString, text), + Q_ARG(QStringList, QStringList{ QStringLiteral("work-channel") }))); + QCOMPARE(text, QStringLiteral("Syncing work-channel...")); + + const QStringList two{ QStringLiteral("personal-channel"), + QStringLiteral("work-channel") }; + QVERIFY(QMetaObject::invokeMethod(&window, "syncStartedText", + Q_RETURN_ARG(QString, text), + Q_ARG(QStringList, two))); + QCOMPARE(text, + QStringLiteral("Syncing personal-channel, work-channel...")); +} + // Items 56 and 57. void TestMainWindow::everyActionIsReachableFromAMenu() diff --git a/translations/qtmaildir_it_IT.ts b/translations/qtmaildir_it_IT.ts index 4daea21..c0e336d 100644 --- a/translations/qtmaildir_it_IT.ts +++ b/translations/qtmaildir_it_IT.ts @@ -739,6 +739,14 @@ Il messaggio È stato inviato. Non inviarlo di nuovo.</translation> <translation>&Elimina conversazione</translation> </message> <message> + <source>Syncing all accounts...</source> + <translation>Sincronizzazione di tutti gli account...</translation> + </message> + <message> + <source>Syncing %1...</source> + <translation>Sincronizzazione di %1...</translation> + </message> + <message> <source>Undelete thread</source> <translation>Ripristina conversazione</translation> </message> @@ -1409,10 +1417,6 @@ Il messaggio È stato inviato. Non inviarlo di nuovo.</translation> <source>%1, waiting for the running sync to finish</source> <translation>%1, in attesa che termini la sincronizzazione in corso</translation> </message> - <message> - <source>Syncing...</source> - <translation>Sincronizzazione in corso...</translation> - </message> <message numerus="yes"> <source>%n unsynced change(s)</source> <translation> |