feat(rules): text mode, and leave untouched rules unwritten

3 files changed, 257 insertions, 1 deletions

diff --git a/src/tagrulesdialog.cpp b/src/tagrulesdialog.cpp
index 797cd92..1d31d07 100644
--- a/src/tagrulesdialog.cpp
+++ b/src/tagrulesdialog.cpp
@@ -162,6 +162,8 @@ TagRulesDialog::TagRulesDialog(QWidget *parent)
 
     connect(m_matchAll, &QRadioButton::toggled,
             this, &TagRulesDialog::syncQueryLine);
+    connect(m_textMode, &QCheckBox::toggled,
+            this, &TagRulesDialog::setTextMode);
     connect(m_addExclusion, &QPushButton::clicked, this, [this] {
         addRow(true);
         syncQueryLine();
@@ -326,6 +328,16 @@ void TagRulesDialog::onSelectionChanged()
     // rule and closing it cannot rewrite the file mailctl also reads.
     m_loadedQuery = RuleQuery::parse(rule.query);
 
+    // A rule the builder cannot show opens as text, and one it can show
+    // returns to the builder. Blocked, because letting setChecked run
+    // setTextMode() here would recompile and overwrite m_query mid-load.
+    {
+        const QSignalBlocker blockTextMode(m_textMode);
+        m_textMode->setChecked(!m_loadedQuery.parsed);
+    }
+    m_builder->setVisible(m_loadedQuery.parsed);
+    m_query->setReadOnly(m_loadedQuery.parsed);
+
     if (m_loadedQuery.parsed)
         rebuildRows(m_loadedQuery);
 
@@ -347,7 +359,16 @@ void TagRulesDialog::applyEditsToCurrentRule()
     rule.enabled = m_enabled->isChecked();
     rule.add = splitTags(m_add->text());
     rule.remove = splitTags(m_remove->text());
-    rule.query = m_query->text().trimmed();
+    if (m_textMode->isChecked()) {
+        rule.query = m_query->text().trimmed();
+    } else {
+        const RuleQuery current = currentQueryFromRows();
+        // Unchanged rows mean the stored string is left exactly as it was
+        // read. Recompiling an untouched rule would churn a file the
+        // companion tool also reads, showing a diff the user never made.
+        if (!(current == m_loadedQuery))
+            rule.query = current.compile();
+    }
     rule.note = m_note->toPlainText();
 
     fillItem(m_list->topLevelItem(index), rule);
@@ -431,6 +452,19 @@ QString TagRulesDialog::queryLineForTest() const
     return m_query->text();
 }
 
+bool TagRulesDialog::textModeForTest() const
+{
+    return m_textMode->isChecked();
+}
+
+void TagRulesDialog::setRowValueForTest(int index, const QString &value)
+{
+    if (index < 0 || index >= m_rows.size())
+        return;
+    m_rows.at(index).value->setText(value);
+    syncQueryLine();
+}
+
 void TagRulesDialog::selectRuleForTest(int index)
 {
     if (index >= 0 && index < m_list->topLevelItemCount())
@@ -587,6 +621,40 @@ RuleQuery TagRulesDialog::currentQueryFromRows() const
     return query;
 }
 
+void TagRulesDialog::setTextMode(bool on)
+{
+    if (on) {
+        // Show what the rows currently mean, then hand the string over.
+        if (m_loadedQuery.parsed)
+            m_query->setText(currentQueryFromRows().compile());
+        m_builder->setVisible(false);
+        m_query->setReadOnly(false);
+        return;
+    }
+
+    // Going back needs the typed query to be representable. If it is not, the
+    // checkbox cannot clear: there are no rows that mean this query.
+    const RuleQuery parsed = RuleQuery::parse(m_query->text().trimmed());
+    if (!parsed.parsed) {
+        const QSignalBlocker block(m_textMode);
+        m_textMode->setChecked(true);
+        QMessageBox::information(
+            this, tr("Cannot show as rows"),
+            tr("This query is more than the builder can show, so it stays "
+               "as text. It is still saved and applied normally."));
+        return;
+    }
+
+    const bool wasReloading = m_reloading;
+    m_reloading = true;
+    rebuildRows(parsed);
+    m_reloading = wasReloading;
+
+    m_loadedQuery = parsed;
+    m_builder->setVisible(true);
+    m_query->setReadOnly(true);
+}
+
 void TagRulesDialog::rebuildRows(const RuleQuery &query)
 {
     while (!m_rows.isEmpty())
diff --git a/src/tagrulesdialog.h b/src/tagrulesdialog.h
index 6676da9..9cee163 100644
--- a/src/tagrulesdialog.h
+++ b/src/tagrulesdialog.h
@@ -64,6 +64,11 @@ public:
     /// Selects the rule at `index` as a click on the list would.
     void selectRuleForTest(int index);
 
+    bool textModeForTest() const;
+    void setRowValueForTest(int index, const QString &value);
+    /// Runs the Save path without showing the dialog.
+    void saveForTest() { onSave(); }
+
 signals:
     /// Asks the owner to run countQueries() through the worker.
     void countsRequested();
@@ -104,6 +109,8 @@ private:
     void updateExclusionsVisibility();
     void syncQueryLine();
 
+    void setTextMode(bool on);
+
     void rebuildRows(const RuleQuery &query);
     void applyTermToRow(Row *row, const RuleTerm &term);
     RuleQuery currentQueryFromRows() const;
diff --git a/tests/test_tagrules.cpp b/tests/test_tagrules.cpp
index fa5c0b5..49abf8a 100644
--- a/tests/test_tagrules.cpp
+++ b/tests/test_tagrules.cpp
@@ -42,6 +42,10 @@ private slots:
     void aNewerVersionIsRefused();
     void openingARuleFillsTheBuilderRows();
     void switchingRulesDoesNotLeakRowsBetweenThem();
+    void openingARuleWithoutEditingLeavesItByteIdentical();
+    void anUnrepresentableRuleOpensInTextMode();
+    void editingARowRewritesTheQuery();
+    void aTextModeRuleStaysTextWhenAnotherRuleIsVisited();
 
 private:
     QString writeRules(const QString &json);
@@ -313,5 +317,182 @@ void TestTagRules::switchingRulesDoesNotLeakRowsBetweenThem()
     QCOMPARE(dialog.queryLineForTest(), QStringLiteral("from:one.example.org"));
 }
 
+void TestTagRules::openingARuleWithoutEditingLeavesItByteIdentical()
+{
+    // Recompiling on open would rewrite the shared file for no reason, and
+    // the companion tool would see a diff the user never made. Semantically
+    // equal is not enough: the bytes must match.
+    QTemporaryDir configHome;
+    QVERIFY(configHome.isValid());
+    qputenv("XDG_CONFIG_HOME", configHome.path().toUtf8());
+    QVERIFY(QDir().mkpath(configHome.filePath(QStringLiteral("mailrules"))));
+
+    const QString stored = configHome.filePath(
+        QStringLiteral("mailrules/rules.json"));
+    QFile out(stored);
+    QVERIFY(out.open(QIODevice::WriteOnly));
+    out.write(R"({
+      "version": 1,
+      "rules": [
+        {"id": "handwritten",
+         "query": "not subject:receipt  and   from:plain.example.net",
+         "add": ["handwritten"], "stage": 50, "enabled": true},
+        {"id": "vendor",
+         "query": "(from:vendor.example.org or from:vendor.example.net) and not subject:receipt",
+         "add": ["vendor"], "stage": 50, "enabled": true},
+        {"id": "plain", "query": "from:plain.example.org",
+         "add": ["plain"], "stage": 50, "enabled": true}
+      ]
+    })");
+    out.close();
+
+    {
+        TagRulesDialog dialog;
+        dialog.selectRuleForTest(2);
+        dialog.selectRuleForTest(1);
+        dialog.selectRuleForTest(0);
+        dialog.saveForTest();
+    }
+
+    TagRules reloaded;
+    reloaded.load(stored);
+    QCOMPARE(reloaded.rules().size(), 3);
+    // Hand-written spacing and an exclusion ahead of the positive term. Both
+    // are things compile() normalises away, and this rule is deliberately the
+    // one left current at Save, since that is the only rule the save path
+    // writes at all. The two below round trip byte for byte on their own, so
+    // neither could catch a save path that recompiles regardless.
+    QCOMPARE(reloaded.rules().at(0).query,
+             QStringLiteral("not subject:receipt  and   "
+                            "from:plain.example.net"));
+    QCOMPARE(reloaded.rules().at(1).query,
+             QStringLiteral("(from:vendor.example.org or "
+                            "from:vendor.example.net) and not subject:receipt"));
+    QCOMPARE(reloaded.rules().at(2).query,
+             QStringLiteral("from:plain.example.org"));
+}
+
+void TestTagRules::anUnrepresentableRuleOpensInTextMode()
+{
+    QTemporaryDir configHome;
+    QVERIFY(configHome.isValid());
+    qputenv("XDG_CONFIG_HOME", configHome.path().toUtf8());
+    QVERIFY(QDir().mkpath(configHome.filePath(QStringLiteral("mailrules"))));
+
+    const QString stored = configHome.filePath(
+        QStringLiteral("mailrules/rules.json"));
+    QFile out(stored);
+    QVERIFY(out.open(QIODevice::WriteOnly));
+    // body: is a perfectly good notmuch prefix this builder does not model.
+    out.write(R"({
+      "version": 1,
+      "rules": [
+        {"id": "deep", "query": "body:receipt",
+         "add": ["deep"], "stage": 50, "enabled": true}
+      ]
+    })");
+    out.close();
+
+    {
+        TagRulesDialog dialog;
+        QVERIFY(dialog.textModeForTest());
+        dialog.saveForTest();
+    }
+
+    // Unrepresentable is not invalid: it must survive a save untouched.
+    TagRules reloaded;
+    reloaded.load(stored);
+    QCOMPARE(reloaded.rules().size(), 1);
+    QCOMPARE(reloaded.rules().at(0).query, QStringLiteral("body:receipt"));
+}
+
+void TestTagRules::editingARowRewritesTheQuery()
+{
+    // The other half of the guarantee: when rows DO change, the stored query
+    // must follow.
+    QTemporaryDir configHome;
+    QVERIFY(configHome.isValid());
+    qputenv("XDG_CONFIG_HOME", configHome.path().toUtf8());
+    QVERIFY(QDir().mkpath(configHome.filePath(QStringLiteral("mailrules"))));
+
+    const QString stored = configHome.filePath(
+        QStringLiteral("mailrules/rules.json"));
+    QFile out(stored);
+    QVERIFY(out.open(QIODevice::WriteOnly));
+    out.write(R"({
+      "version": 1,
+      "rules": [
+        {"id": "vendor", "query": "from:vendor.example.org",
+         "add": ["vendor"], "stage": 50, "enabled": true}
+      ]
+    })");
+    out.close();
+
+    {
+        TagRulesDialog dialog;
+        dialog.setRowValueForTest(0, QStringLiteral("other.example.org"));
+        dialog.saveForTest();
+    }
+
+    TagRules reloaded;
+    reloaded.load(stored);
+    QCOMPARE(reloaded.rules().size(), 1);
+    QCOMPARE(reloaded.rules().at(0).query,
+             QStringLiteral("from:other.example.org"));
+}
+
+void TestTagRules::aTextModeRuleStaysTextWhenAnotherRuleIsVisited()
+{
+    // The cross-rule question, asked directly. Text mode and m_loadedQuery are
+    // per-rule state on a dialog that has one set of widgets, so visiting a
+    // representable rule and coming back must not leave the unrepresentable one
+    // holding the other rule's mode or its parsed query. Getting that wrong
+    // recompiles a query the builder never modelled, which is data loss.
+    QTemporaryDir configHome;
+    QVERIFY(configHome.isValid());
+    qputenv("XDG_CONFIG_HOME", configHome.path().toUtf8());
+    QVERIFY(QDir().mkpath(configHome.filePath(QStringLiteral("mailrules"))));
+
+    const QString stored = configHome.filePath(
+        QStringLiteral("mailrules/rules.json"));
+    QFile out(stored);
+    QVERIFY(out.open(QIODevice::WriteOnly));
+    out.write(R"({
+      "version": 1,
+      "rules": [
+        {"id": "deep", "query": "body:receipt",
+         "add": ["deep"], "stage": 50, "enabled": true},
+        {"id": "plain", "query": "from:plain.example.org",
+         "add": ["plain"], "stage": 50, "enabled": true}
+      ]
+    })");
+    out.close();
+
+    {
+        TagRulesDialog dialog;
+        QVERIFY(dialog.textModeForTest());
+
+        dialog.selectRuleForTest(1);
+        QVERIFY2(!dialog.textModeForTest(),
+                 "a representable rule must return to the builder");
+        QCOMPARE(dialog.queryLineForTest(),
+                 QStringLiteral("from:plain.example.org"));
+
+        dialog.selectRuleForTest(0);
+        QVERIFY2(dialog.textModeForTest(),
+                 "coming back to an unrepresentable rule must be text again");
+        QCOMPARE(dialog.queryLineForTest(), QStringLiteral("body:receipt"));
+
+        dialog.saveForTest();
+    }
+
+    TagRules reloaded;
+    reloaded.load(stored);
+    QCOMPARE(reloaded.rules().size(), 2);
+    QCOMPARE(reloaded.rules().at(0).query, QStringLiteral("body:receipt"));
+    QCOMPARE(reloaded.rules().at(1).query,
+             QStringLiteral("from:plain.example.org"));
+}
+
 QTEST_MAIN(TestTagRules)
 #include "test_tagrules.moc"