diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-04 11:16:52 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-04 12:54:48 +0200 |
| commit | 3b0a52b8620d3cead2f527d108ba64bfec8ee273 (patch) | |
| tree | f231a877f4805a7dbcde4145a7e8ce9443156dc5 /src/mainwindow.cpp | |
| parent | 272cb9de98ea387b1d73c55333e4058d1af5da47 (diff) | |
| download | qtmaildir-3b0a52b8620d3cead2f527d108ba64bfec8ee273.tar.gz qtmaildir-3b0a52b8620d3cead2f527d108ba64bfec8ee273.zip | |
feat(sync): show unsynced edits and offer to sync on exit
Tagging changes the notmuch index at once, but the mail store only hears
about it on the next sync, and nothing said so. Quitting with tagging
outstanding was silent. Items 18 and 19 of the usability backlog, built
together because the second needs the first's counter.
The counter cannot be QUndoStack::isClean(), which is the obvious
candidate and the wrong one: 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. m_pendingEdits is its own count, incremented where a write is
CONFIRMED rather than where one is sent, so an optimistic update the
worker later rejects cannot leave the indicator claiming an edit that
never landed. Only a successful sync resets it: clearing on failure would
assert the changes had reached the mail store when the sync is exactly
what failed to put them there.
It is shown in the status bar, hidden entirely at zero, and described as
a lower bound rather than a guarantee, since an external notmuch run can
carry changes over without this application noticing.
On exit, sync_on_exit in [general] takes ask, always or never. Three
values rather than a bool because "prompt me", "just do it" and "do
nothing" are three behaviours and true/false expresses two; an unknown
value warns by name, since a typo there silently changes what happens to
unsynced work. The prompt offers three buttons for the same reason: a
user who hit Quit by mistake needs a way back that is not "sync". A sync
started at exit holds the window open until it finishes rather than being
killed mid-run, and a sync that FAILS does not quit, because quitting
there would discard the user's choice silently. With no sync command
configured the prompt degrades to a plain warning instead of offering a
sync that cannot run.
This is not a destructive-action confirmation of the kind CLAUDE.md
forbids. Those cover tag mutations, which keep undo instead of a dialog.
This asks about losing work at the one point where undo cannot help.
The tagsApplied lambda became a named slot, which is better structure and
also what lets a test drive it: the worker is deliberately parentless
because it moves to its own thread, so reaching it with findChild to emit
the real signal cannot work, and contorting the test to try was the wrong
instinct. Testing a modal needed its own care. A test that sends a close
event hangs forever if an unexpected dialog opens, because the modal
spins its own event loop; CloseProbe polls for activeModalWidget, closes
it and records that one appeared, turning "a dialog opened" into an
assertion rather than a hang.
Also removes a stray qDebug left in the open_thread action by the earlier
Enter-key investigation, which had reached two commits.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Diffstat (limited to 'src/mainwindow.cpp')
| -rw-r--r-- | src/mainwindow.cpp | 171 |
1 files changed, 153 insertions, 18 deletions
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. |
