From a8d84d57b7f3b43fdc0460eb252da45c76ab2baa Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Sat, 31 Oct 2026 11:42:24 +0100 Subject: feat: an optional allow_archive field on a tagging rule Both readers learn it together, as the rule format requires. No version bump: the field is optional, and each writer emits it only on a rule that removes inbox, so a file without one saves unchanged. Co-Authored-By: Claude Opus 5.5 --- assets/hooks/mailrules.py | 9 +++++- assets/hooks/test_mailrules.py | 35 ++++++++++++++++++++ src/tagrules.cpp | 15 ++++++++- src/tagrules.h | 9 ++++++ tests/test_tagrules.cpp | 73 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 139 insertions(+), 2 deletions(-) diff --git a/assets/hooks/mailrules.py b/assets/hooks/mailrules.py index dbb80a5..fbd615b 100755 --- a/assets/hooks/mailrules.py +++ b/assets/hooks/mailrules.py @@ -41,7 +41,8 @@ DEFAULT_STAGE = 50 # Fields this version understands. Anything else in a rule object is kept in # `unknown` and written back untouched, which is what makes the file neutral # rather than this tool's file that another program may read. -KNOWN_KEYS = {"id", "stage", "enabled", "add", "remove", "query", "note"} +KNOWN_KEYS = {"id", "stage", "enabled", "add", "remove", "query", "note", + "allow_archive"} # An id is a handle, not a display name: a UI selects on it and a diff tracks # it. Tags may contain '/' and may be renamed; ids may not. @@ -57,6 +58,7 @@ class Rule: stage: int = DEFAULT_STAGE enabled: bool = True note: str = "" + allow_archive: bool = False unknown: dict = field(default_factory=dict) @@ -194,6 +196,10 @@ def _rule_to_dict(rule): "query": rule.query, "note": rule.note, } + # Written only where it does something, matching src/tagrules.cpp, so a + # file without an archiving rule round-trips unchanged (item 213). + if rule.allow_archive and "inbox" in rule.remove: + out["allow_archive"] = True out.update(rule.unknown) return out @@ -257,5 +263,6 @@ def _parse_rule(obj, index, seen, warnings): stage=stage, enabled=bool(obj.get("enabled", True)), note=obj.get("note", "") if isinstance(obj.get("note", ""), str) else "", + allow_archive=obj.get("allow_archive") is True, unknown={k: v for k, v in obj.items() if k not in KNOWN_KEYS}, ) diff --git a/assets/hooks/test_mailrules.py b/assets/hooks/test_mailrules.py index b1f31c9..ae8fe04 100755 --- a/assets/hooks/test_mailrules.py +++ b/assets/hooks/test_mailrules.py @@ -266,6 +266,41 @@ def test_tag_arguments(): assert mailrules.tag_arguments(rule) == ["+one", "+two", "-three"] +def test_allow_archive_round_trips_and_is_omitted_when_false(): + """Item 213. Known, so not carried as unknown; written only when true.""" + with tempfile.TemporaryDirectory() as tmp: + path = write_rules(tmp, { + "version": 1, + "rules": [ + {"id": "promo", "add": ["promo"], "remove": ["inbox"], + "query": "from:a@example.com", "allow_archive": True}, + {"id": "plain", "add": ["x"], "query": "from:b@example.com"}, + ], + }) + store = mailrules.load(path) + assert store.rules[0].allow_archive is True + assert store.rules[1].allow_archive is False + assert "allow_archive" not in store.rules[0].unknown + + mailrules.save(store, path) + raw = json.loads(path.read_text()) + assert raw["rules"][0]["allow_archive"] is True + assert "allow_archive" not in raw["rules"][1] + + +def test_allow_archive_on_a_rule_keeping_inbox_is_accepted_and_ignored(): + """Never a reason to drop a rule: the hook runs unattended.""" + with tempfile.TemporaryDirectory() as tmp: + path = write_rules(tmp, { + "version": 1, + "rules": [{"id": "x", "add": ["x"], "query": "from:a@example.com", + "allow_archive": True}], + }) + store = mailrules.load(path) + assert len(store.rules) == 1 + assert store.warnings == [] + + def run_all(): for name, fn in sorted(globals().items()): if name.startswith("test_") and callable(fn): diff --git a/src/tagrules.cpp b/src/tagrules.cpp index 0049278..c25ed59 100644 --- a/src/tagrules.cpp +++ b/src/tagrules.cpp @@ -41,7 +41,7 @@ bool isKnownKey(const QString &key) QStringLiteral("id"), QStringLiteral("stage"), QStringLiteral("enabled"), QStringLiteral("add"), QStringLiteral("remove"), QStringLiteral("query"), - QStringLiteral("note"), + QStringLiteral("note"), QStringLiteral("allow_archive"), }; return known.contains(key); } @@ -256,6 +256,8 @@ void TagRules::load(const QString &path) rule.stage = object.value(QStringLiteral("stage")).toInt(kDefaultStage); rule.enabled = object.value(QStringLiteral("enabled")).toBool(true); rule.note = object.value(QStringLiteral("note")).toString(); + rule.allowArchive = + object.value(QStringLiteral("allow_archive")).toBool(false); for (auto it = object.begin(); it != object.end(); ++it) { if (!isKnownKey(it.key())) @@ -267,6 +269,13 @@ void TagRules::load(const QString &path) } } +bool TagRules::hookWouldSkip(const TagRule &rule) +{ + if (rule.remove.contains(QStringLiteral("unread"))) + return true; + return rule.remove.contains(QStringLiteral("inbox")) && !rule.allowArchive; +} + bool TagRules::save(const QString &path) const { const QString target = path.isEmpty() ? defaultPath() : path; @@ -285,6 +294,10 @@ bool TagRules::save(const QString &path) const QJsonArray::fromStringList(rule.remove)); object.insert(QStringLiteral("query"), rule.query); object.insert(QStringLiteral("note"), rule.note); + // Written only where it does something, so a file without an + // archiving rule saves byte-identical (item 213). + if (rule.allowArchive && rule.remove.contains(QStringLiteral("inbox"))) + object.insert(QStringLiteral("allow_archive"), true); for (auto it = rule.unknown.begin(); it != rule.unknown.end(); ++it) object.insert(it.key(), it.value()); array.append(object); diff --git a/src/tagrules.h b/src/tagrules.h index 4d3f0ab..e2c8667 100644 --- a/src/tagrules.h +++ b/src/tagrules.h @@ -39,6 +39,10 @@ struct TagRule QStringList remove; int stage = 50; ///< Ascending. Account tags 10, topic rules 50. bool enabled = true; + /// Lets the post-new hook remove `inbox` from arriving mail for this rule + /// (item 213). Meaningless, and cleared on save, unless `remove` holds + /// `inbox`. `unread` stays refused regardless. + bool allowArchive = false; /// Fields this version of qtmaildir does not understand, kept verbatim and /// written back on save. Without this, one save from here silently strips @@ -100,6 +104,11 @@ public: /// could reach the file and never come back. static QStringList validate(const QList &rules); + /// Whether the post-new hook will skip this rule whole. Mirrors + /// PROTECTED_REMOVALS in assets/hooks/post-new: `unread` always, `inbox` + /// unless the rule opts in with allowArchive. + static bool hookWouldSkip(const TagRule &rule); + /// No file yet, as distinct from a file that would not load. A fresh /// install is not an error and must not be reported as one. bool missing() const { return m_missing; } diff --git a/tests/test_tagrules.cpp b/tests/test_tagrules.cpp index c831864..860d433 100644 --- a/tests/test_tagrules.cpp +++ b/tests/test_tagrules.cpp @@ -35,6 +35,9 @@ class TestTagRules : public QObject Q_OBJECT private slots: + void allowArchiveRoundTripsAndIsOmittedWhenFalse(); + void allowArchiveIsDroppedFromARuleThatKeepsInbox(); + void hookWouldSkipMirrorsThePostNewRefusal(); void aRuleLoadsWithEveryField(); void absentFieldsTakeTheirDefaults(); void aMalformedRuleIsDroppedWithAWarning(); @@ -1505,5 +1508,75 @@ void TestTagRules::previewClearsTheAccountScope() "is wrapped twice and matches nothing"); } +void TestTagRules::allowArchiveRoundTripsAndIsOmittedWhenFalse() +{ + // Item 213. Optional, so a file with no archiving rule must save + // unchanged: no "allow_archive": false appears from nowhere. + const QString path = writeRules(R"({ + "version": 1, + "rules": [ + {"id": "promo", "add": ["promo"], "remove": ["inbox"], + "query": "from:a@example.org", "allow_archive": true}, + {"id": "plain", "add": ["x"], "query": "from:b@example.org"} + ] + })"); + + TagRules rules; + rules.load(path); + QCOMPARE(rules.rules().size(), 2); + QVERIFY(rules.rules().at(0).allowArchive); + QVERIFY(!rules.rules().at(1).allowArchive); + // Known now, so it is not ALSO carried as an unknown field. + QVERIFY(!rules.rules().at(0).unknown.contains(QStringLiteral("allow_archive"))); + + QVERIFY(rules.save(path)); + QFile file(path); + QVERIFY(file.open(QIODevice::ReadOnly)); + const QJsonArray saved = QJsonDocument::fromJson(file.readAll()) + .object().value(QStringLiteral("rules")).toArray(); + QCOMPARE(saved.at(0).toObject().value(QStringLiteral("allow_archive")), + QJsonValue(true)); + QVERIFY(!saved.at(1).toObject().contains(QStringLiteral("allow_archive"))); +} + +void TestTagRules::allowArchiveIsDroppedFromARuleThatKeepsInbox() +{ + // A flag that does nothing is cleared by the WRITER, never rejected by a + // reader: the hook must not drop a whole rule over a harmless flag. + TagRule rule; + rule.id = QStringLiteral("x"); + rule.query = QStringLiteral("from:a@example.org"); + rule.add = { QStringLiteral("x") }; + rule.allowArchive = true; + + TagRules rules; + rules.setRules({ rule }); + QVERIFY(TagRules::validate(rules.rules()).isEmpty()); + const QString path = m_dir.filePath(QStringLiteral("cleared.json")); + QVERIFY(rules.save(path)); + + TagRules reloaded; + reloaded.load(path); + QVERIFY(!reloaded.rules().at(0).allowArchive); +} + +void TestTagRules::hookWouldSkipMirrorsThePostNewRefusal() +{ + // Mirrors PROTECTED_REMOVALS in assets/hooks/post-new, which the Python + // suite tests against the hook itself. + TagRule rule; + rule.remove = { QStringLiteral("inbox") }; + QVERIFY(TagRules::hookWouldSkip(rule)); + rule.allowArchive = true; + QVERIFY(!TagRules::hookWouldSkip(rule)); + + rule.remove = { QStringLiteral("unread") }; + QVERIFY(TagRules::hookWouldSkip(rule)); // never opted out + + rule.remove = { QStringLiteral("promo") }; + rule.allowArchive = false; + QVERIFY(!TagRules::hookWouldSkip(rule)); +} + QTEST_MAIN(TestTagRules) #include "test_tagrules.moc" -- cgit v1.2.3