| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Third entry in the message pane's submenus and in each details row,
greyed rather than hidden when the query bar is empty, so the feature
stays visible to someone exploring a fresh window.
requestSearch refuses an Exclude with no query as well, so the guard
does not rest on the menu's enabled state alone. Mutation checked:
disabling that condition fails the new test on the blocked emit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
The menus cannot read the query bar and must not. MainWindow already
watched textChanged for the Save button; the same lambda now also tells
MessageView, which passes it to the details dialog at construction,
where it cannot go stale.
Nothing consumes it yet.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Four signatures, no behaviour change: the two shipped operations map to
Replace and Narrow. runSearchFromPane becomes a switch and gains the
Exclude arm, which nothing can reach until the menu entry exists.
Seven call sites across three test files moved with it, two more than
the plan predicted: test_messageview and test_mainwindow also drive
these signals directly.
mainwindow.h and messagedetailsdialog.h now include searchterm.h for
the type; messageview.h already did.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Introduced alone, ahead of the four signatures that change to it, so
that change lands as one mechanical commit with no behaviour difference.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Parenthesises both sides, as extend() does: unparenthesised, a
disjunction in the query bar binds so the exclusion covers only its last
term and leaves the excluded mail on screen, with nothing reporting an
error.
An empty existing query returns empty rather than the addition alone,
which is where this deliberately differs from extend(). Excluding from
nothing means the whole Maildir minus one value; the menus will grey the
entry out and this is the second layer.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
The connection is direct, so emitting first runs the query synchronously: the
model clears and the pane blanks while the modal dialog is still up, holding
the m_items it was built from. Closing first leaves no window in which the
dialog describes a thread the pane has already dropped.
|
|
The panes carry a finished query and know nothing of the query bar; the window
sets the field and calls the existing runner, so the account scope and the
generation counter keep working as they do for a typed query.
Narrowing combines here rather than in a pane, because only the window can see
what the bar currently holds. The tag strip's chips join the header, the body
selection and the details dialog as a fourth surface.
Also fixes the details dialog to actually close when a search is chosen: the
comment above the connection already described this requirement, but nothing
called accept() or reject(), so the dialog stayed open, the query ran behind
it, and the modal exec() never returned. This hung the whole test suite on
QT_QPA_PLATFORM=offscreen once a covering test was added.
|
|
A text box could not carry a per-value context menu without parsing displayed
text back into structure, and the user did not want a text box. Each row now
holds its own value, its message index and its query, built from the parsed
message.
Every value label states Qt::PlainText. The QPlainTextEdit this replaced was
plain by design rather than by style: header values come from strangers, and a
QLabel guesses the format under AutoText.
|
|
selectedText() reads the selection with no script injection; JavaScript stays
disabled in the profile. The page's standard menu is kept and the entries are
added to it below a separator.
The quoting is tested through a function taking the text, so it needs no live
web engine: a selection is arbitrary prose and can carry quotes, newlines and
query syntax, none of which notmuch reports as an error.
|
|
The menu lists what is searchable rather than hit-testing which line of a
rich-text label was clicked, which breaks as soon as the label wraps. The
values are collected by the pass that renders the header, so nothing parses
the markup back into structure.
From, To and Cc appear only for a single-message thread, sharing the condition
with the header's own display: a thread's recipient differs message to
message, and the menu must not offer what the header is not stating.
The test fixture's Date: header named the wrong weekday, which Qt::RFC2822Date
rejects outright, so no date offer would have been produced from it.
|
|
The loop was assigning a variable it never read, overwritten on every pass and
used only after, which reads as an accumulator and is not one. The overflow
chip's position is derived where it is used instead.
|
|
chipRectAt() is the single source of a chip's geometry, used by paintEvent and
by the hit test, so the drawn chip and the clickable chip cannot drift.
The +N chip yields nothing: it stands for a list of tags rather than one, so
there is no single value a search could be built from.
|
|
The date search needs it and the logic already existed inside a file-local
function, including the fix for Qt::RFC2822Date rejecting a string that
carries a trailing timezone comment. Extracted rather than rewritten, so the
second caller cannot end up without that fix.
|
|
One place for the query grammar behind every search surface, with no widget
involved so it is tested without a painter or a web engine.
extend() parenthesises both sides. The query bar may hold a hand-written
disjunction, and 'a or b AND c' binds as 'a or (b AND c)', which widens a
search meant to narrow it and reports nothing.
|
|
Right-click a stored saved query and the rules dialog opens on a new
rule carrying its query, with the tags left empty and focused. Generated
entries are excluded: their query is composed from the accounts, so a
rule made from one would freeze a snapshot that goes stale.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
The dialog is non-modal and single-instance, so a second Create tagging
rule reaches one that is already up. Seeding it beats dropping the
request, which would read as a broken menu item.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
The seed is a whole TagRule rather than a query string, so item 78 can
reuse the same path to seed from a sender. It is a pending edit like one
made with Add rule: appended, selected, Add tags focused, and written
only on Save.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
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>
|
|
Saving a generated entry wrote `"query": ""` and `"flat": true` alongside its
generator. Both reload correctly, so nothing was broken, but queries.json is
meant to be hand-edited and each redundant key is one more thing to read past.
A generated entry has no query of its own, and the sent generator already
implies flat.
Written now only when they say something, which is the rule `pinned` and
`account` already followed: `query` is skipped for a generated entry in favour
of `generated`, and `flat` is skipped when the generator implies it.
Omitting `flat` is only safe because loadSavedQueries() reapplies it from the
generator, so the two are coupled: the mutation that stops reapplying it fails
this test and one other, in both suites. That is deliberate, since a round-trip
test can otherwise pass while quietly writing less than it reads.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Item 82. Saving a query worked and nothing else did: changing one field meant
retyping the whole query under the same name, and deleting one meant editing
the file by hand. An action that creates something the UI cannot then change or
remove is incomplete, and the user hit it within minutes of the first hand test.
Right-clicking a saved query, on its button or its menu entry, now offers Edit,
Move to menu / Show as a button, and Delete. Every path funnels through one
replaceSavedQuery(), which matches on the name the dialog was OPENED with rather
than the one it returns, so a rename replaces the entry instead of leaving the
original behind beside a new one, and which merges the stored entry's unknown
fields in a single place rather than in three. Delete confirms first: the rule
against confirmation dialogs covers tag mutations, which the undo stack can take
back, and this writes user config that it cannot.
Two cases the item did not anticipate. A generated entry has no query to edit,
so the dialog shows its composed query read-only rather than offering a field
that changes nothing, and carries `generated` and `flat` through an edit rather
than letting it decay into a plain entry holding a snapshot of what it resolved
to today. And the overwrite notice had to learn to ignore the entry being
edited, since warning that "Inbox" already exists while editing Inbox is noise.
This also fixes a defect that predated it and was already reachable from the
save path. rebuildSavedQueryRow() called deleteLater() on the old row, which
defers destruction to the event loop, so the stale row went on answering
findChild() and every lookup after a rebuild reported the state from before the
edit. Nothing looked wrong on screen, which is why it surfaced only as three
tests failing against a row that had in fact been rebuilt correctly.
Five tests, three mutations. Matching on the returned name fails two, never
writing the file fails three, and dropping the unknown-field merge fails one.
That last one initially proved nothing: it drove UNPIN, which copies the stored
entry and so carries `unknown` along by itself, and passed with the merge
deleted. It now goes through the edit path with a replacement that has none,
which is what the dialog actually returns.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
The user asked whether the default queries could be unified with Sent. The
answer runs the other way: Sent joins the saved queries rather than the saved
queries becoming hardcoded.
Inbox, Unread and Important are complete strings that depend on nothing and can
never go stale, so generating them would buy nothing and would cost the four
things the file just gained: reordering, unpinning, renaming and deleting.
Hardcoding them would also make them undeletable, which is a regression for
anyone who does not want one of them.
Sent is different only in that its query CANNOT be stored: it is composed from
every account's `sent` key, so a stored copy goes stale the moment a folder is
renamed. That is a property of Sent, not of "default queries". Storing the
GENERATOR rather than its output keeps both halves: `"generated": "sent"` still
resolves from the accounts at click time, and the entry is an ordinary row that
can be reordered, renamed, unpinned or removed. The row now follows one rule
instead of carrying one member the user did not own.
Two properties had to travel with the entry. The composed query, resolved
through Config::resolvedQuery() so what lands in the bar is what actually ran;
and FLAT mode, since a sent view lists messages and a threaded one folds every
reply back into the conversation the user sent one message into. The sent
generator implies flat rather than trusting the file to say so, because a
hand-edited row would otherwise produce a threaded sent view.
An unknown generator is reported but the row is KEPT: a later build may know it,
and dropping it here would delete it from the file on the next save, which is
the same data loss the unknown-field handling exists to prevent. A generator
whose accounts configure nothing is skipped entirely, exactly as the hardcoded
button was hidden rather than offering one that finds nothing.
Eight new tests. The four pre-existing Sent tests reach this through migration
and were left alone, which is what proves the migrated path still behaves; the
new ones cover a STORED file, which is the path every launch after the first
takes. Mutations: a generator resolving to nothing fails three, ignoring flat
fails two, and not skipping an empty generator fails one. A rename test guards
the property the change exists for, since anything keyed on the literal name
"Sent" would break it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
document-save is the floppy/disk shape. It means "write a file somewhere",
which leaves the user to guess what is being written, and next to a row of
saved-query buttons it reads as an unrelated control. Saving a query is
bookmarking a search, so bookmark-new is the icon every desktop already uses
for "keep this for later". Verified to resolve with real art in the desktop's
actual theme rather than assumed present.
The button also shows its label now instead of the icon alone. It sits among
text buttons, and an icon on its own next to them reads as a different kind of
control; it is also the one action whose meaning an icon cannot carry, since
"save" is a familiar shape whose question is always "save what?". The toolbar
is unaffected and still follows the desktop's own button style.
The label is the button's own text rather than the action's. "&Save query..."
is menu phrasing, and setDefaultAction copies it verbatim, so the button
rendered an accelerator ampersand and the ellipsis that promises a dialog. The
action keeps both for the menu it lives in, and the test asserts the override
survives setDefaultAction rather than trusting that it does.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
nothing is pinned
The saved-query buttons are the row's content and read as a set; the overflow
menu is a control over that set, so it belongs apart from them rather than
trailing the last button. Moving the stretch above it pushes it to the right
edge.
Doing that exposed a latent defect in the same function. The row hid itself when
its layout held nothing but the stretch, which was written as a count of one and
happened to be right only because the stretch went last. With the stretch moved
the count changes, and the condition as written would have hidden a row holding
only the menu: a config with saved queries but none pinned would have had no
route to any of them, the menu buried along with the row. The check now counts
the content added before the stretch and treats an unpinned query as content in
its own right.
Both are mutation-checked. Putting the stretch back at the end fails the
alignment test, and restoring the old hide condition fails the new one, which
asserts the row survives with nothing but unpinned queries in it. The alignment
is asserted on the layout's own ordering rather than on x coordinates, since a
geometry assertion would also pass for a row that merely ran out of width.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
The spec asked for "a Save query button beside the search bar" and what shipped
was a menu entry and Ctrl+S. The user went looking for the button where the
design said it would be and did not find it.
Saving is a thing you decide on while looking at the results, so it belongs
where the results came from rather than behind a menu or a remembered chord.
The button takes the action through setDefaultAction rather than a second
connect, so it inherits the text, icon, tooltip and enabled state and cannot end
up offering to save an empty query while the menu entry correctly refuses. The
mutation that replaces it with a plain clicked() connection fails the test.
Also records item 82: a saved query cannot be edited, unpinned or deleted from
the UI. Item 23 specified saving and nothing else, and that is exactly what was
built, so the only way to unpin a query is a text editor or retyping it in full
under the same name. An action that creates something the UI cannot then change
or remove is incomplete, and this was found within minutes of the first hand
test. It is filed as a defect rather than an enhancement, and the spec now says
so where a reader would otherwise take the design for complete.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Second half of item 23, on top of the storage change. A query can now be kept
without hand-editing a file, and the row of buttons no longer grows without
bound.
Ctrl+S opens a dialog on whatever is in the query bar, taking a name, an
optional account scope and whether the query is pinned. It preselects the
account already chosen in the dropdown, since that is the scope the user is
looking at, and it says so when a name is about to replace an existing query
rather than refusing the name: overwriting a saved query on purpose is a normal
edit, and the only thing worth preventing is doing it without noticing. Saving
over an entry keeps the stored entry's unknown fields rather than the dialog's
fresh value, so a field written by a later build survives being edited here.
The saved queries move to a row of their own beneath the query bar, pinned ones
as buttons and the rest behind a More queries menu that only exists when
something is in it. The ponytail note that stood in the query row predicted
exactly this: an unbounded list of buttons sharing the row squeezed the field.
Sent moves down with them and is still not a saved query, for the reason already
recorded there.
A saved query's account scope goes through the account DROPDOWN rather than
being baked into the query text. runQuery() already wraps the query in the
selected account's path, so pre-scoping here would apply it twice, and setting
the dropdown also shows the user which scope they are in. An unscoped query
clears the selection rather than inheriting whatever the last one left, which is
the same defect the rules preview had.
Seven tests, three mutations. Ignoring the pinned flag fails two of them,
pre-scoping the text instead of setting the dropdown fails two, and letting an
unscoped query inherit the previous account fails one. The menu-absence test
initially passed against no implementation at all, since it only asserted a
widget was missing; it now proves the row was populated first, which is the
guard that class of test needs.
Two existing invariants caught real omissions rather than needing adjustment:
every registered action must appear in KeyMap::knownActions(), which is what
gives it a configurable binding, and every action needs its own icon.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
First half of item 23. The storage moves out of the [queries] INI section into
~/.config/qtmaildir/queries.json; the UI that writes it comes next.
The INI could not express order. QSettings reads a section through childKeys(),
which sorts alphabetically and never follows the file, so the saved-query
buttons could not be arranged and config.cpp carried a comment saying a
hand-rolled parser would be needed to change that. queries.json is an ordered
array and nothing sorts it on load. That also makes room for the two fields the
save dialog needs: pinned, which decides whether a query is a button or a menu
entry, and account, which scopes it.
account stores the account KEY, not the maildir path, so it does not duplicate
config that already lives in the account section and go stale when the user
edits it. Config::resolvedQuery() composes through Account::scopedQuery(), whose
parentheses are load-bearing: path:... and a or b binds as (path:... and a) or
b, so an unparenthesised disjunction escapes its scope and matches every
account. A key naming an account that no longer exists resolves to the bare
query rather than a scope built from an empty maildir, which would be path:"/**"
and match everything.
Migration reads [queries] once, when queries.json is absent, marks every entry
pinned so the query row does not empty on the first launch after an upgrade, and
leaves the INI section untouched. Stripping it would mean rewriting a
hand-edited file with QSettings, which drops comments and key order across the
whole file.
The format follows rules.json in shape only: a version and unknown fields
preserved at both levels, so a file written by a later build survives a save
from this one. None of its two-implementation machinery is here, because queries
have exactly one reader; the version constant says so where a future reader will
look. A file whose version this build does not know is refused AND blocks the
save, so a newer document is never overwritten with a lossy reading of itself.
Twelve tests, each checked against a mutation that puts the corresponding bug
back: sorting on load fails three of them, stripping the INI section after
migration fails the byte-identical assertion, concatenating the scope without
parentheses fails the disjunction test, and dropping unknown-field preservation
fails the round trip. The migration test compares the INI file's BYTES rather
than re-reading it through QSettings, which would have passed against a rewrite
that kept every value while dropping the comments.
startup_query still resolves by name, but its fallback now returns the first
entry in the user's own order rather than the alphabetically first one. That is
user-visible for a config whose startup_query matches nothing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Item 77. The dialog could say how many messages a rule matched and not
which ones. A Preview in list button now runs the selected rule's query
in the main window; the dialog stays open, since comparing the rule
against its results is the point.
Two constraints from the backlog entry, both now asserted and both
mutation-checked.
The query runs exactly as stored, with no tag:new and no wrapping
parentheses. The post-new hook adds those when it applies a rule, and a
preview that copied them would match nothing outside a sync window,
since tag:new is set only on mail that has just arrived.
The account selector is cleared first. runQuery() wraps the bar's text
in the selected account's scope, and a rule query usually names its own
path already, so previewing one with an account selected would scope it
twice and show an empty list, which reads as "this rule collects no
mail".
The second mutation only fails once the test's config has an account to
select: with the default empty config the selector sits on "All
accounts" anyway, and asserting that a preview leaves it there passed
against the mutation. Recorded in the test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Item 80. A rule with eight From conditions left the list showing about
one and a half rows.
The list was added with stretch 1 and the form below it with none, which
looks decisive and is not: a stretch factor only distributes space above
each widget's minimum, and the form's minimum grew with every condition
row, so each row came straight out of the list. The builder asked for
120px with one row and 414px with eight.
A QSplitter now divides the list from the editor, so the balance is the
user's and is saved beside the column widths, and the condition rows sit
in a QScrollArea capped at 190px so the editor cannot grow without bound
however the splitter is set. The scroll area is what text mode hides;
hiding the builder inside it would leave an empty frame.
Three measures were tried in the test before one told the bug and the
fix apart, and two passed against broken code: the dialog's
minimumSizeHint does not track form rows and read 580 either way, and a
qMin against the scroll area's own hint read small whether or not the
cap was set, since an uncapped maximumHeight is QWIDGETSIZE_MAX. What
survives mutation is the editor pane's minimum inside the splitter, plus
the cap read directly, and both are asserted. A row's size hint is
invalid until the event loop runs, so the test calls processEvents after
selecting a rule or it measures the same height twice.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
The geometry was saved from closeEvent, and neither dialog button sends
one: Cancel calls reject(), Save calls accept(), and only the window
manager's X button produces a QCloseEvent. So the size and the column
widths were kept for the one route out of three that a user almost never
takes, and a resize followed by Cancel came back forgotten.
The save moves to a done(int) override, which both buttons funnel
through and which QWidget::close() also reaches.
The test that covered this passed against the bug because it asserted
with close(). It now drives all three routes rather than trusting one to
stand for the others, and shows the dialog before the close leg:
close() on a widget that was never visible returns early without
reaching done(), so that assertion would otherwise prove nothing.
Both traps recorded in CLAUDE.md, since neither is specific to this
dialog.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Item 75. saveGeometry() and the rule list header's saveState() go to
uistate.conf under keys of their own, written on closeEvent so a size
survives Cancel as well as Save. The 760x520 resize stays as the
first-run fallback.
The backlog's approach was wrong on one point and a test caught it. It
said to drop the resizeColumnToContents calls once a saved header state
exists, which fixes the restore and leaves the original defect standing:
with nothing saved, a width the user had just dragged was still
discarded by the next add or delete. Each column is instead auto-sized
once, on its first fill, after which the width belongs to the user
however it was set. Two flags, because the count column is filled later
by a reply from the worker.
The window stays a QDialog. Making it a top-level window needs the
unsaved-edit story that being modal currently sidesteps, and that is its
own decision rather than part of this item.
Both tests redirect XDG_STATE_HOME as well as XDG_CONFIG_HOME, so they
cannot write the real uistate.conf. The geometry is asserted on the
stored value rather than the reopened frame, per item 46: the offscreen
platform does not honour a resize.
Also corrects setFolders' doc comment, which still described the folder
list as coming from Config.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
The dropdown was built from config, which names one subtree per account
and nothing below it, so it offered five entries and no way to say
Drafts or Sent. A rule wants to target those as often as a whole
account.
NotmuchWorker gains requestFolders/foldersReady, walking the tree from
notmuch_database_get_path() and listing every directory holding cur/.
It belongs there because the database root is notmuch's database.path
and the worker owns the only handle that can answer for it; putting the
root in config would be the second source of truth the design refuses.
From the disk rather than from the index: a folder mbsync created and
nothing has landed in yet is still a folder a rule may target, and a
list derived from indexed message paths would not offer it.
The two tests build their own fixture rather than extending the shared
one, which needs a nested folder and would otherwise move seven count
assertions in unrelated tests. Mutation-checked: flattening the walk to
non-recursive fails the listing test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Ticking "Edit as text" was a one-way trip: the only way back to the rows
was closing the dialog and reopening it. The checkbox was parented to the
builder widget and sat on the match row, and switching to text mode hides
that widget, so the toggle disappeared along with the rows it governs.
Move it to the query row, which is visible in both modes.
The existing tests all passed against this, because they drove the toggle
through setChecked and then asserted on the checked STATE. A hidden
checkbox reports its state perfectly well, so every one of those
assertions held while the widget was unreachable. The new test asks the
question that matters, whether the toggle would be on screen, and it uses
isVisibleTo since nothing is isVisible on a dialog that was never shown.
Worth recording how close the mutation check came to endorsing this too.
Reparenting the checkbox alone left it in the query row's layout, so it
stayed visible and the test still passed. Only restoring the full shipped
shape, parent and layout together, reproduced the fault and failed the
test. A mutation that does not reproduce the original bug proves nothing
about the test that is meant to catch it.
The spec's layout sketch carried the same error and is corrected, with
the reason, so the next reader does not reintroduce it.
|
|
|
|
Leaving text mode with a query the builder cannot represent has to
refuse, since there are no rows that mean that query. It announced this
with a QMessageBox, which made the branch untestable: a modal blocks the
test that reaches it, so the one path that can strand a user was the one
path shipping unverified.
Say it in the warning label the dialog already has instead. That also
suits the moment better, since it does not interrupt someone mid-edit to
tell them something the label can hold while they keep typing, and it
matches how the tag dialog reports a bad tag.
Returning to the rows now calls showWarnings(), because the refusal
writes into the same label the load warnings use and a stale complaint
would otherwise outlive the query that caused it.
The test drives the refusal and the recovery, and asserts the warning
appears and then clears. Verified by mutation: letting the checkbox clear
regardless fails it.
warningTextForTest uses isVisibleTo rather than isVisible. Every child of
a dialog that was never shown reports isVisible() false, so the seam
would have reported no warning whatever the label held, which is a probe
that cannot see the thing it checks.
|
|
|
|
Opening the tagging rules dialog and pressing Save destroyed the first
rule in the list, without any editing. The rule lost its query and its
tags, then vanished entirely on the next load, since a rule with an empty
query is dropped as malformed.
Reproduced against the released tag rather than the branch, in a
throwaway worktree at 9585674 with a two-rule fixture: constructing the
dialog and running its save path left one rule of two.
onSelectionChanged blocked signals for the note widget only, while
m_enabled::toggled two lines later reached applyEditsToCurrentRule, which
writes every field from widgets the loader has not filled yet. The
existing comment there shows the hazard was known for one widget and not
extended to the other.
The fix landed with the builder work: the reloading flag now covers the
whole load, and switchingRulesDoesNotLeakRowsBetweenThem is the
regression test, verified by mutation to fail without the guard.
The live rules file had one casualty, the account rule sitting first in
the list, with both its query and its tags empty while every sibling was
intact. Restored from the shell backup that the earlier migration kept
and verified through mailctl's own reader. The rule had stopped tagging,
but only one message had arrived meanwhile; that message is now tagged
and the account is complete again at 14969 of 14969.
|
|
Selecting a rule now parses its stored query and rebuilds the builder
rows from it, and a row edit compiles back onto the query line and into
the working copy.
Populating the form was already able to write the rule just loaded over
whichever rule is current: m_enabled's toggled runs
applyEditsToCurrentRule while m_query still holds the previous rule's
text, which emptied the first rule's query on open. The existing
m_reloading guard now covers the whole load rather than one signal
blocker on the note, which also covers the combo boxes rebuildRows
populates.
|
|
|
|
|
|
|
|
|
|
The draft compile() quoted every Is/IsNot term, which contradicted the
same task's own assertion that a negated tag compiles to .
The implementer resolved it in the direction the tests specify, and the
resolution is right: notmuch reads tag:inbox and tag:"inbox" identically,
counting 5322 either way against the live index, so quoting a tag would
change the stored string without changing what it matches. That breaks
the byte-for-byte round trip this type exists to guarantee.
Restate the comment as the rule rather than as a note about what a test
expects, correct the plan's draft so the remaining tasks do not inherit
the contradiction, and warn the parser task that a quoted tag must not be
read back as a quoting operator.
|
|
|
|
|
|
Counts are generation-stamped and dropped when stale or when the dialog
has closed: counting every rule against a cold index takes seconds, so
an in-flight reply outliving its dialog is ordinary rather than rare.
The stamp is its own counter, not m_generation as drafted. That one is
the QUERY generation, compared against directly by every thread, tree
and message load, so bumping it to count rules would discard whatever
the user was opening at the time and blank the message pane for an
unrelated reason.
Registering an action obliges two more entries, both enforced by tests:
the name in KeyMap::knownActions(), and a default binding, since every
action carries one. Ctrl+Shift+T, shifted against Ctrl+T for edit_tags
the way Ctrl+Shift+U is shifted against Ctrl+U.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Edits land on a working copy and reach the file only on Save. The
dialog never opens a notmuch database of its own: it publishes the
queries it wants counted and MainWindow runs them through the worker,
because the worker owns the only handle.
Two departures from the drafted version, both of which lost edits.
QPlainTextEdit has no editingFinished, so the note reached the working
copy only for whichever row was current at Save; it is driven from
textChanged instead, with the selection handler blocking the signal so
loading a rule cannot write itself back over the one now current. And
reloadList()'s setCurrentItem emits currentItemChanged, so New and Copy
repopulated the form from m_working before the pending edit had been
flushed into it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
requestCounts counts threads, which is right for the placeholder pane.
A tagging rule tags messages, so a dry run over rules needs the message
count or it understates every rule that matches part of a thread.
|
|
The same ~/.config/mailrules/rules.json mailctl reads, parsed here with
QJsonDocument and written atomically with QSaveFile. Fields this version
does not understand round-trip untouched, which is what keeps the format
neutral between the two tools.
Mutation-checked: removing the unknown-field write fails
unknownFieldsSurviveASave.
|
|
Items 70 and 69, the second folded into the first as item 70's own size note
predicted it should be.
The panes drew their state marks as font glyphs: U+1F4CE for an attachment and
U+2605 for a flagged thread, each with a fallback for a font that cannot render
it. Both fell back to "*", so on such a font a flagged thread and one carrying
an attachment were indistinguishable, which is a defect the fallback introduced
rather than prevented. What a mark looks like was also the desktop's decision
rather than this application's, and the panes are exactly where it should not
be: the user asked for the toolbar and menus to keep following their icon theme
while the panes stop.
Six marks now ship in assets/icons/marks/: flagged, attachment, passed, replied
and the two expander triangles. QIcon::fromTheme still resolves every toolbar
and menu icon and was not touched.
Licensing chose the shapes. The look came from a GPL3 icon theme, and this
project is GPLv2-only, which are incompatible: GPLv2's "no further
restrictions" clause bars shipping GPL3 assets in a v2-only work. The six were
drawn fresh in the same idiom instead, with no path data copied. The idiom is
generic: solid single-path silhouettes at 16x16 with no strokes.
They are compiled in as string literals rather than loaded from a .qrc.
src/CMakeLists.txt already records why resources belong to the executable: a
qrc in the static library registers itself from a global initialiser the linker
drops. The tests link the library, so a resource-based mark would be missing
exactly where it needs asserting. assets/icons/marks/ stays the editable
source.
One asset serves both palettes. Every payload paints with fill="currentColor",
which QSvgRenderer renders black rather than resolving, so Marks::pixmap
composites the wanted colour with CompositionMode_SourceIn. A mark then takes
the card's own pen colour and follows selection and the read/unread dimming
without a second variant to keep in step.
CardLayout reserves a rect per mark and CardDelegate paints into it. The marks
were glyphs inside the subject STRING, so their width came free from the text
metrics; as icons the geometry has to know they are there or the subject runs
underneath them. The expander pill had the same trap, its triangle being a
glyph in expanderLabel(), and now reserves that width explicitly.
Item 69's part: passed and replied were words in the tag strip and are marks
beside the subject now. The message pane's header carries the flagged and
attachment marks next to the subject, per the user's decision that the right
pane needs those two and only outside the message area.
A duplicate that no test caught is worth recording. Every geometry assertion
passed while a card showed passed as BOTH an arrow and a green tag chip: the
chip filter had no reason to know a mark had appeared. It was found by
rendering real cards to an image and looking at them. isDrawnAsAMark() is now
one list consulted by both PillTagsRole and MessageOwnTagsRole, since two
copies drifting apart is how a tag ends up drawn twice on one row and not at
all on another.
Fourteen tests: nine in test_marks, four in test_cardlayout, one in
test_threadlistmodel. Mutation-checked at four points, each failing a test: the
subject ignoring the marks, the flag not indenting the subject, the pill
forgetting the triangle's width, and the recolour composite removed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Item 71. A tag edit reached the notmuch index at edit time and then sat there
until the user clicked Sync or their cron job fired, so "mark all read" updated
the view while the change itself waited, sometimes for ten minutes.
A confirmed edit now arms a debounce that runs the existing sync path. The delay
is auto_sync_delay_ms in [general], defaulting to 2000, and follows
mark_read_delay_ms exactly, including that zero and negative are not errors:
zero syncs on the next trip through the event loop, and any negative value
disables the behaviour, which is the switch for a user who wants only their cron
job.
It is armed from onTagsApplied, where a write is confirmed and the pending count
is already current, rather than where one is sent: a sync scheduled for a write
the worker went on to reject would run for nothing. A debounce rather than a
schedule, restarted by each edit, because "mark all read" confirms one write per
thread in the view and an arm-per-edit timer would be the storm of syncs the
debounce exists to prevent. Nothing is armed when no sync command is configured
or when the pending count is zero, the case where an edit was netted against its
own inverse. When the timer fires with a sync already running, local or cron, it
skips rather than queues: mbsync's own answer to a second run is to fail on it,
and the edits stay pending rather than being lost.
Also fixes a pane blanked out from under the reader, found by hand testing this
feature. onSyncFinished called runCurrentQuery() where the cron path calls
refreshCurrentQuery(), and a re-run clears the model, the undo stack and the
message pane. The stale-thread notice handles a thread that stops matching the
query and has since item 35, but a re-run left nothing for it to describe. The
two paths had no reason to differ; before this item a local sync only followed a
click on Sync, so the difference went unnoticed. Reading a message in the Unread
view, having it marked read, and watching the pane go blank two seconds later is
what surfaced it.
Its test asserts on the undo stack rather than the pane: both paths issue a
queued query test_mainwindow has no worker to answer, so the pane ends up blank
either way and an assertion on it would pass against both, while the undo stack
is cleared by one and kept by the other.
Nine tests, four in test_config and five in test_mainwindow, each
mutation-checked: removing the schedule call, honouring a negative delay,
dropping the nothing-pending guard, dropping the already-running guard, and
restoring runCurrentQuery() each fail a test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
