aboutsummaryrefslogtreecommitdiffstats
path: root/src
diff options
context:
space:
mode:
Diffstat (limited to 'src')
-rw-r--r--src/mainwindow.cpp48
-rw-r--r--src/mainwindow.h27
-rw-r--r--src/notmuchworker.cpp49
-rw-r--r--src/notmuchworker.h30
4 files changed, 89 insertions, 65 deletions
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<int> &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<TagChange> &changes)
+void MainWindow::onRuleApplied(const QString &description,
+ const QVector<TagChange> &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
diff --git a/src/mainwindow.h b/src/mainwindow.h
index f962089..2766db2 100644
--- a/src/mainwindow.h
+++ b/src/mainwindow.h
@@ -864,7 +864,14 @@ private slots:
/// meanwhile clears that queue, and the backfill's confirmation was then
/// credited to the user's command, whose undo inverted it over thousands
/// of messages.
- void onRuleApplied(quint64 token, const QVector<TagChange> &changes);
+ ///
+ /// Every answer is handled: there is no token to judge one stale by,
+ /// because a write's answer cannot be superseded. Two Applies in flight
+ /// both landed, and each needs its undo and its pending count. Messages
+ /// name the rule from `description`, which the reply carries. A non-empty
+ /// `error` is shown, and whatever landed before it is still recorded.
+ void onRuleApplied(const QString &description,
+ const QVector<TagChange> &changes, const QString &error);
private:
/// What onTagsApplied() does for any confirmed write, apart from crediting
@@ -874,7 +881,7 @@ private:
/// The count for applyRuleToExistingMail() arrived: confirm, then send
/// the rule to the worker's applyRule().
- void confirmRuleApply(int count);
+ void confirmRuleApply(const TagRule &rule, int count);
void buildUi();
@@ -1856,16 +1863,12 @@ private:
/// counter.
quint64 m_ruleCountGeneration = 0;
- /// The rule awaiting its change count, and the generation that count
- /// will carry. Shares m_ruleCountGeneration's sequence, so a pending
- /// Apply and the dialog's Count matches can never be confused.
- TagRule m_pendingApplyRule;
- quint64 m_pendingApplyGeneration = 0;
-
- /// The token the outstanding applyRule() will answer with, 0 when none.
- /// A reply carrying any other token is stale and dropped.
- quint64 m_pendingRuleToken = 0;
- quint64 m_lastRuleToken = 0;
+ /// The rules awaiting their change count, keyed by the generation each
+ /// count will carry. A map, not one slot: a second Apply before the first
+ /// count returns must not make the first look stale. Shares
+ /// m_ruleCountGeneration's sequence, so an Apply and the dialog's Count
+ /// matches can never be confused.
+ QHash<quint64, TagRule> m_pendingApplies;
/// The generation of a REFRESH query, run after a sync to bring the list
/// up to date without disturbing it.
diff --git a/src/notmuchworker.cpp b/src/notmuchworker.cpp
index e5f5afa..ebe4851 100644
--- a/src/notmuchworker.cpp
+++ b/src/notmuchworker.cpp
@@ -390,7 +390,7 @@ QByteArray NotmuchWorker::configPathArg() const
return m_configPath.isEmpty() ? QByteArray() : m_configPath.toLocal8Bit();
}
-bool NotmuchWorker::openReadOnly()
+bool NotmuchWorker::openReadOnly(QString *error)
{
if (m_db) {
// A read-only handle is a Xapian SNAPSHOT taken when it was opened, so
@@ -407,7 +407,7 @@ bool NotmuchWorker::openReadOnly()
// results beats refusing to answer at all.
const notmuch_status_t status =
notmuch_database_reopen(m_db, NOTMUCH_DATABASE_MODE_READ_ONLY);
- if (status != NOTMUCH_STATUS_SUCCESS) {
+ if (status != NOTMUCH_STATUS_SUCCESS && !error) {
emit errorOccurred(
QStringLiteral("Cannot refresh notmuch database: %1")
.arg(QString::fromUtf8(notmuch_status_to_string(status))));
@@ -416,19 +416,26 @@ bool NotmuchWorker::openReadOnly()
}
const QByteArray configPath = configPathArg();
- char *error = nullptr;
+ char *openError = nullptr;
const notmuch_status_t status = notmuch_database_open_with_config(
nullptr, // let config decide path
NOTMUCH_DATABASE_MODE_READ_ONLY,
configPath.isEmpty() ? nullptr : configPath.constData(),
nullptr,
&m_db,
- &error);
+ &openError);
if (status != NOTMUCH_STATUS_SUCCESS) {
- emit errorOccurred(QStringLiteral("Cannot open notmuch database: %1")
- .arg(QString::fromUtf8(error ? error : notmuch_status_to_string(status))));
- free(error);
+ const QString message =
+ QStringLiteral("Cannot open notmuch database: %1")
+ .arg(QString::fromUtf8(openError
+ ? openError
+ : notmuch_status_to_string(status)));
+ if (error)
+ *error = message;
+ else
+ emit errorOccurred(message);
+ free(openError);
m_db = nullptr;
return false;
}
@@ -1092,19 +1099,20 @@ void NotmuchWorker::applyTagsToThreads(const QStringList &threadIds,
void NotmuchWorker::applyRule(const QString &query, const QStringList &add,
const QStringList &remove,
- const QString &description, quint64 token)
+ const QString &description)
{
QVector<TagChange> changes;
- if (!openReadOnly()) {
- emit ruleApplied(token, changes);
+ QString error;
+ if (!openReadOnly(&error)) {
+ emit ruleApplied(description, changes, error);
return;
}
QStringList messageIds;
if (!collectMessageIds(m_db, SearchTerm::changes(query, add, remove),
&messageIds)) {
- emit errorOccurred(QStringLiteral("Cannot resolve the rule's query"));
- emit ruleApplied(token, changes);
+ emit ruleApplied(description, changes,
+ QStringLiteral("Cannot resolve the rule's query"));
return;
}
@@ -1117,12 +1125,12 @@ void NotmuchWorker::applyRule(const QString &query, const QStringList &add,
for (TagChange &op : ops) {
QStringList changed;
// An empty list writes nothing; writeTags() is not even asked to open.
- if (!messageIds.isEmpty() && !writeTags(op, &changed))
- break; // errorOccurred() already emitted; report what landed
+ if (!messageIds.isEmpty() && !writeTags(op, &changed, &error))
+ break; // report what landed before the failure, and the failure
op.messageIds = changed;
changes.append(op);
}
- emit ruleApplied(token, changes);
+ emit ruleApplied(description, changes, error);
}
void NotmuchWorker::applyTags(const TagChange &change)
@@ -1146,7 +1154,8 @@ void NotmuchWorker::applyTags(const TagChange &change)
change.description });
}
-bool NotmuchWorker::writeTags(const TagChange &change, QStringList *changedIds)
+bool NotmuchWorker::writeTags(const TagChange &change, QStringList *changedIds,
+ QString *errorOut)
{
// The read-only handle must be closed first: notmuch allows only one open
@@ -1174,10 +1183,14 @@ bool NotmuchWorker::writeTags(const TagChange &change, QStringList *changedIds)
//
// The stall a running sync DOES cause is avoided upstream, in
// MainWindow, by not sending the write at all while the lock is held.
- emit errorOccurred(
+ const QString message =
QStringLiteral("Cannot open database for writing: %1")
.arg(QString::fromUtf8(error ? error
- : notmuch_status_to_string(status))));
+ : notmuch_status_to_string(status)));
+ if (errorOut)
+ *errorOut = message;
+ else
+ emit errorOccurred(message);
free(error);
return false;
}
diff --git a/src/notmuchworker.h b/src/notmuchworker.h
index a175d7a..63c5203 100644
--- a/src/notmuchworker.h
+++ b/src/notmuchworker.h
@@ -246,10 +246,11 @@ public slots:
///
/// Answers with ruleApplied() ALWAYS, even when nothing moved, and never
/// with tagsApplied(): the window builds the undo from the answer rather
- /// than from the shared confirmation queue.
+ /// than from the shared confirmation queue. Never with errorOccurred()
+ /// either: that path reverts whatever unrelated user write is in flight,
+ /// so a failure travels in the reply.
void applyRule(const QString &query, const QStringList &add,
- const QStringList &remove, const QString &description,
- quint64 token);
+ const QStringList &remove, const QString &description);
/// Resolves whole threads to the message ids and file paths they contain.
///
@@ -398,10 +399,12 @@ signals:
/// applyRule()'s answer: one TagChange per single-tag operation, in the
/// order applyRule() documents, each carrying the ids that operation
- /// actually moved (possibly none). `token` is echoed from the request.
- /// Emitted even after a write error, carrying what was written before it,
- /// so the window can still offer undo for that part.
- void ruleApplied(quint64 token, const QVector<TagChange> &changes);
+ /// actually moved (possibly none). `description` is echoed from the
+ /// request, so the window names the right rule. `error` is empty on
+ /// success; on a failure, `changes` holds what was written before it, so
+ /// the window can still offer undo for that part.
+ void ruleApplied(const QString &description,
+ const QVector<TagChange> &changes, const QString &error);
/// One file entered or left the index outside a query (item 192).
///
@@ -497,13 +500,18 @@ signals:
void errorOccurred(const QString &message);
private:
- bool openReadOnly();
+ /// With `error` given, a failure is written there instead of emitted, and
+ /// the non-fatal reopen failure is ignored; applyRule() reports in its
+ /// own reply.
+ bool openReadOnly(QString *error = nullptr);
void close();
/// applyTags() without the signal: writes `change` and fills `changedIds`
- /// with the messages whose tags really moved. False, with errorOccurred()
- /// emitted, when the database could not be opened for writing.
- bool writeTags(const TagChange &change, QStringList *changedIds);
+ /// with the messages whose tags really moved. False when the database
+ /// could not be opened for writing, with the reason in `error` when given
+ /// and through errorOccurred() otherwise.
+ bool writeTags(const TagChange &change, QStringList *changedIds,
+ QString *error = nullptr);
QByteArray configPathArg() const;