diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-14 11:01:58 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-14 11:01:58 +0200 |
| commit | b08923df88de7ba03135234aaf3c602f23e49e03 (patch) | |
| tree | 5cc74979efc3470e948bcdfb1b7343e880254c07 /src/tagrules.h | |
| parent | 27838b47bae379578f458ebaefa192b422b55e24 (diff) | |
| download | qtmaildir-b08923df88de7ba03135234aaf3c602f23e49e03.tar.gz qtmaildir-b08923df88de7ba03135234aaf3c602f23e49e03.zip | |
fix(rules): stop a rule with a spaced name from vanishing on save
A rule named "justeat orders" in the field labelled Name was written to
rules.json correctly and then dropped by every reader, because load()
required ^[a-z0-9][a-z0-9-]*$ and the save path validated nothing. The
rule stayed in the file, invisible in the dialog, never applied by the
post-new hook, and the next save from the dialog would have deleted it
outright.
The asymmetry was the defect, not the pattern. TagRules::validate() is
now the single predicate: the dialog refuses to save against it, and
load() uses it to repair rather than drop, so a rule that fails is
visible and fixable instead of silently discarded.
- The typed name is sanitised into an id when the field is committed,
so the field shows what will reach the file. uniqueId() suffixes a
collision, since sanitising is many-to-one and can manufacture the
duplicate that load() then drops.
- An already-legal id is never rewritten, including one like "a---b"
that sanitising would otherwise collapse. Rewriting valid ids would
churn a file mailctl also reads.
- A bad id loads repaired, with the warning kept: what is on disk is
not what the hook runs until the file is saved back.
Deliberately not mirrored into mailrules.py. The hook tags real mail
unattended every ten minutes, where silently renaming an id is worse
than dropping the rule; the file converges as soon as the dialog saves.
No format change, so no version bump and no two-repo commitment.
The load warning was not missing: it had been showing "1 rule could not
be read and was skipped" on every open, in the same font and colour as
the intro prose two lines above it, and read as more explanation. It is
now a red banner beside Save, with an icon and a dismiss button, and it
says the rules need attention rather than that they were skipped, which
is no longer true. Dismissal is per-appearance only; a persistent one
would re-hide the problem that went unnoticed for a session.
Both new dialog tests were confirmed to fail with the sanitiser
reverted, and the banner's styling, position and dismissal each fail
under mutation. 20 of 20 suites green, 34 tests in test_tagrules.
Closes item 83.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Diffstat (limited to 'src/tagrules.h')
| -rw-r--r-- | src/tagrules.h | 25 |
1 files changed, 25 insertions, 0 deletions
diff --git a/src/tagrules.h b/src/tagrules.h index 8f75b38..4d3f0ab 100644 --- a/src/tagrules.h +++ b/src/tagrules.h @@ -75,6 +75,31 @@ public: QStringList warnings() const { return m_warnings; } + /// The one predicate. load() drops or repairs by it, the dialog refuses to + /// save against it, and mailrules.py enforces the same pattern in the + /// companion repo. Anything failing this is invisible to the post-new hook. + static bool isValidId(const QString &id); + + /// A typed name reduced to a legal id: lowercased, every run of anything + /// else collapsed to one dash, dashes trimmed off both ends. + /// + /// Returns an EMPTY string when nothing legal survives ("!!!"), because an + /// empty id is not writable and the caller must decide the fallback rather + /// than have one invented here. Already-legal ids pass through untouched, + /// so loading a good file never rewrites it. + static QString sanitiseId(const QString &name); + + /// sanitiseId plus a numeric suffix when the result is already taken. + /// Sanitising is many-to-one, so it manufactures duplicates that load() + /// would then drop; this is what stops the second rule becoming the first. + static QString uniqueId(const QString &name, const QStringList &taken); + + /// Every reason these rules would not survive a reload, one string each, + /// empty when they all would. Written for the save path: the defect this + /// answers is that save() wrote anything and load() validated, so a rule + /// could reach the file and never come back. + static QStringList validate(const QList<TagRule> &rules); + /// 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; } |
