From 2b6ecd2a1f624c12c071320ff1ed96404d75f839 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Sat, 31 Oct 2026 12:43:21 +0100 Subject: fix: handle every rule answer, and keep its failure out of errorOccurred A second Apply overwrote the pending token, so the first rule's answer was dropped as stale although its writes had landed: no undo, no pending count, no accounts. A write's answer cannot be superseded, so the token is gone and every ruleApplied() is handled. Pending counts are keyed by generation for the same reason, since a second Apply before the first count returned dropped the first Apply entirely. applyRule() no longer emits errorOccurred(). That path reverted an unrelated in-flight user write and its message was then overwritten by "nothing to change". The failure travels in the reply instead: ruleApplied(description, changes, error). openReadOnly() and writeTags() take an optional error out-parameter so the rule's path stays quiet. The window shows the error, still records and offers undo for whatever landed first, and names the rule from the reply's description. Item 212. Co-Authored-By: Claude Opus 5.5 --- src/mainwindow.cpp | 48 ++++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 24 deletions(-) (limited to 'src/mainwindow.cpp') diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 918bd7b..3ee31aa 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -2827,10 +2827,8 @@ void MainWindow::onRuleCountsReady(const QVector &counts, // Item 212. Apply's count first: it shares this signal and the generation // sequence, and needs no dialog, since the test and a closed dialog's // last Apply both arrive here without one. - if (m_pendingApplyGeneration != 0 - && generation == m_pendingApplyGeneration) { - m_pendingApplyGeneration = 0; - confirmRuleApply(counts.value(0, -1)); + if (m_pendingApplies.contains(generation)) { + confirmRuleApply(m_pendingApplies.take(generation), counts.value(0, -1)); return; } @@ -2863,12 +2861,12 @@ void MainWindow::applyRuleToExistingMail(const TagRule &rule) const QStringList queries{ SearchTerm::changes(rule.query, rule.add, rule.remove) }; - m_pendingApplyRule = rule; - m_pendingApplyGeneration = ++m_ruleCountGeneration; + const quint64 generation = ++m_ruleCountGeneration; + m_pendingApplies.insert(generation, rule); QMetaObject::invokeMethod( m_worker, "requestMessageCounts", Qt::QueuedConnection, Q_ARG(QStringList, queries), - Q_ARG(quint64, m_pendingApplyGeneration)); + Q_ARG(quint64, generation)); } TagCommand *MainWindow::armTagCommandForTesting(const QStringList &add, @@ -2882,9 +2880,8 @@ TagCommand *MainWindow::armTagCommandForTesting(const QStringList &add, return command; } -void MainWindow::confirmRuleApply(int count) +void MainWindow::confirmRuleApply(const TagRule &rule, int count) { - const TagRule rule = m_pendingApplyRule; if (count < 0) { showTransientStatus( tr("Rule \"%1\" could not be counted, so nothing was applied.") @@ -2929,22 +2926,18 @@ void MainWindow::confirmRuleApply(int count) } const QString description = tr("Apply rule \"%1\"").arg(rule.id); - m_pendingRuleToken = ++m_lastRuleToken; QMetaObject::invokeMethod( m_worker, "applyRule", Qt::QueuedConnection, Q_ARG(QString, rule.query), Q_ARG(QStringList, rule.add), - Q_ARG(QStringList, rule.remove), Q_ARG(QString, description), - Q_ARG(quint64, m_pendingRuleToken)); + Q_ARG(QStringList, rule.remove), Q_ARG(QString, description)); announceAction(tr("%1: %n message(s)", "", count).arg(description)); } -void MainWindow::onRuleApplied(quint64 token, - const QVector &changes) +void MainWindow::onRuleApplied(const QString &description, + const QVector &changes, + const QString &error) { - if (token == 0 || token != m_pendingRuleToken) - return; - m_pendingRuleToken = 0; // One undo entry for the whole rule, one command per operation, each // built from the ids that operation MOVED (item 176). The command's first @@ -2967,15 +2960,22 @@ void MainWindow::onRuleApplied(quint64 token, m_undoStack.push(command); } - if (!anyMoved) { - // Another writer got there between the count and the write. The - // ticket bump stops announceAction() claiming "done" over this. + if (anyMoved) + m_undoStack.endMacro(); + + // The ticket bump stops announceAction() claiming "done" over either + // message below. + if (!error.isEmpty()) { ++m_announceTicket; - showTransientStatus( - tr("Rule \"%1\" has nothing to change.").arg(m_pendingApplyRule.id)); - return; + // Not transient: a failed write is state the user has to read. + m_statusLabel->setText(tr("%1 failed: %2").arg(description, error)); + } else if (!anyMoved) { + // Another writer got there between the count and the write. + ++m_announceTicket; + showTransientStatus(tr("%1: nothing to change.").arg(description)); } - m_undoStack.endMacro(); + if (!anyMoved) + return; // Every account, not the ones the model knows: a backfill reaches mail no // row shows, so its accounts cannot be resolved through the model the way -- cgit v1.2.3