| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
The action carried mail-mark-important, which Breeze and several other themes
draw as an exclamation mark rather than a star. The filter button for the same
tag has used `starred` since a15505d, where the comment records the user asking
for a star when item 57 renamed the action, and the two were allowed to differ
on the reasoning that a query-row icon reads as a category while an action icon
reads as a verb.
That reasoning held only while the action appeared beside its own label. Item
189 put it on the icon-only message bar, where the icon IS the control, and it
read as an info glyph. Both are `starred` now.
The label stays "Important" and the tag stays `flagged`; only the picture
changes. `starred` sits under status/ rather than actions/ in the icon spec,
which needs no fallback: an unresolved name already leaves the action with text
alone, and it resolves in the user's own theme, verified.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NY6poqw199LfFaXe5BHKNe
|
|
Follows items 185 and 186, which established the pane's bar as where actions
on the displayed message live. Star and Archive are both selection-scoped and
fit that rule with nothing to decide; Archive leaves the main toolbar the way
Delete did, since the same icon in two places reads as two controls when the
toolbar is icon-only.
Ordered by what they do rather than by where they came from: answering the
message, then filing it, then destroying it, so the destructive button is not
between two that are not.
Mark all read deliberately stays on the main toolbar, at the user's decision.
It is the one action in this window that ignores the selection and acts on
every row in the view, so a bar whose every other entry acts on the one
displayed message is exactly where it must not be.
Item 140's toolbar test named archive as an example of a list-wide action.
That was never true of it, only untested, and this item reclassifies it: the
test now asserts archive LEFT the toolbar and keeps its guard on
mark_all_read, which is the action that genuinely is list-wide.
Closes item 189.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NY6poqw199LfFaXe5BHKNe
|
|
The pane's bar offered Reply and Forward on a message the user had thrown
away, which are the two things a trashed message is least likely to want,
while Restore and the purges lived only in menus.
The bar now has a third branch, asked before the draft one: a deleted draft
must come out of the trash before it can be edited. It is keyed on the
SELECTION being in a trash folder, the same predicate the menu entries use,
rather than on the trash VIEW, which disagree on mail reached from a search.
It carries Restore, Delete permanently and Empty trash, and only Restore is
tinted: the two purges are one act at two scopes and need no colour to tell
them from each other, only from the one action that gives mail back.
Delete moves here from the main toolbar in the same change (item 186). It
acts on the displayed message, like Reply and Forward, so it belongs on the
pane's bar by the rule items 139 to 141 settled for those two. It stays in
the Message menu and the context menu.
Delete permanently is new. It is Empty trash scoped to the selection, the
same purgeMessages() call with the ids resolved from the selection rather
than from a query, so it inherits both of that action's safeguards: it
confirms, naming the count, and it carries no default shortcut. One combined
thread:/id: query resolves a mixed selection, so a conversation and a reply
selected together still ask once.
The bar is refilled when the conversation digest arrives as well as on
selection, since a conversation's trash-ness is not known until every path
has been reported.
Closes items 185 and 186.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NY6poqw199LfFaXe5BHKNe
|
|
Item 125, the half that was genuinely missing.
Most of this item was already built and the row was stale. The exit-75
branch in onSyncFinished() predates this session and does what the entry
asks: the spinner clears, the skip is reported as neither success nor
failure, m_lastSyncFailed stays put, the log pane is not raised, the lock
latch is handed back to the external monitor, and the sync-on-exit case
has its own dialog. Item 174 then added the external half, a `skipped`
state a run the application did not start can be seen to have produced.
What nothing covered was the RE-ARM, and it is the symptom the item was
filed for. runAutoSync() re-arms when it declines to START, which is item
89 and covers a sync skipped before launching. A run that LAUNCHES, finds
the lock held and exits 75 reaches onSyncFinished() instead, and that
branch armed nothing: the edit stayed pending with nothing scheduled to
carry it, waiting for a manual sync or the next cron tick. That is "a
held edit waits for a completion that never comes".
scheduleAutoSync() in the skip branch. It re-checks the delay, the sync
command and the pending count on the way in, so it cannot arm a sync for
nothing, and against a long external run it re-arms once per debounce
interval until the lock clears.
This is the half the status file could not reach, and the distinction is
worth keeping: that file says what a run DID, this is what the
application does NEXT.
The test records a real pending edit first, since runAutoSync() correctly
declines when there is nothing to carry and a fixture without one would
arm nothing for a legitimate reason. It failed before the fix and is
mutation-checked.
Suite: 43 of 44, with undoMovesTheMessageBack failing as it does on
master (item 136).
|
|
Item 174, and half of item 125.
The premise was corrected before any code. The note asks for an external
`notmuch new` to clear the pending count; it must not. That count means
tag mutations not yet known to have reached the MAIL STORE, which is the
server: an edit is in notmuch the moment it is made, and what is
outstanding is mbsync pushing the renamed Maildir files. `notmuch new`
re-indexes local files and pushes nothing, so clearing on it would tell
the user their work was safe to quit on while it was still local. The
entry's own proposal to watch notmuch_database_get_revision() was
rejected for the same reason: a revision moves when mail ARRIVES too, and
in neither case does it say anything about the server.
What was actually wrong was the reporting channel. The application
inferred a finished run from an inode in /proc/locks and from grepping
the log for its RUN END banner, which made a human-readable line into
wire format and could not say WHICH channels a run carried. The local
sync path has always narrowed its clear to the accounts it carried; the
external path could not, and cleared everything, so an edit to an
account a run never touched was reported as delivered.
So the script reports instead of leaving evidence to be inferred. It
writes ~/.local/state/qtmaildir/syncstatus.json atomically at the end of
every run, including a skip, naming the channels, both exit statuses and
a state of ok, failed or skipped. MailSync::readStatus() reads it,
MainWindow prefers it over the log banner and narrows the clear through
Account::syncChannel(). A skipped run clears nothing, which is item 125's
first half: the application can now see that a run happened and carried
nothing. The log banner and lastRunOutcome() stay as the fallback for a
missing file, which is what a first run after upgrading looks like.
This is the user's own framing of the scope: the script was written for
another system and adapted, and is now qtmaildir's only consumer, so it
serves the application rather than the reverse. Two facts made it safe to
act on: their crontab runs mailsync.sh and nothing else touches mail, and
~/bin/mailsync.sh is a symlink into this repo, so an edit is live on the
next tick.
Two bugs found while wiring it in, both recorded in the closed item.
A test read the developer's real sync state, twice: a [sync] section
naming only `log` leaves syncStatus() defaulting to the real file, so two
tests asserting that a FAILED run leaves the count alone read the last
real cron run, found ok, and cleared. Pinning only `status` has the
mirror problem. noSyncTestReadsTheRealSyncState() is the guard, modelled
on noTestCanSeeTheRealLockTable().
And Qt::ISODate carries no milliseconds. The status file is preferred
only when it describes THIS run, compared against when the lock appeared,
so a stale success cannot outrank a fresh failure; but the script writes
date -Iseconds, and a round trip of "now" comes back 329 ms behind,
measured. A fast sync's own file therefore parsed as stale and fell back
to the log, with nothing failing to say so. One second of slack matches
the precision the format carries.
Design: docs/superpowers/specs/2026-08-29-sync-status-file-design.md
Suite: 43 of 44, with undoMovesTheMessageBack failing as it does on
master (item 136).
|
|
Item 182, found by hand: a thread of 9 messages with 5 unread, marked
read while a sync was running, reported "<subject>: mark as read" and
then reported the same work again when the sync finished. The user read
it as double reporting.
Not a double write, and the mail was correct. It is one action reported
twice because the FIRST report was the wrong one.
A sync holds notmuch's exclusive write lock and the worker's read-write
open blocks on it rather than failing, so an edit made during a sync is
held and sent when the lock frees. All three hold branches say exactly
that, in a label chosen deliberately: NOT transient, because it
describes state lasting until the sync ends, and a message that expired
would leave rows showing a tag the database has not got and no
explanation of why.
That label never survived. Every caller announced the action itself a
line later through showTransientStatus(), which overwrote it, so the
user was told the write had happened and the hold was never mentioned.
The flush at the end of the sync then reported the same work again and
read as a duplicate rather than as its completion.
announceAction() asks whether a sync holds the lock and, when one does,
sets a non-transient label naming the action AND the wait. The action is
still named because that announcement is what stands in for the
confirmation dialog this project rules out: it is how a user tells that
something larger than they meant has just happened, so the hold is added
to it rather than replacing it. The flush message is untouched and is
the only signal that held work actually landed, whose absence was item
106.
The test drives toggle_unread, the route the user took, and asserts both
halves: the text mentions the sync, and it still says what is waiting.
Asserting only the first would pass against an announcement that dropped
the action entirely. Mutation-checked by forcing the non-held branch,
which fails with the exact text the user reported.
The new string is translated, since one that misses the Italian ships as
English inside an otherwise Italian UI: lupdate found it with no context
warnings, lrelease reports 552 finished and 0 unfinished.
Suite: 42 of 43, with undoMovesTheMessageBack failing as it does on
master (item 136).
|
|
Item 181, from the user's notes: "the thread dashboard doesn't update
live with the modifications applied to the list pane. If I mark the
thread as read, the dash still reports N unread".
ThreadDashboard draws a ThreadDigest, which the worker builds from the
index and which reached the pane only when a conversation was selected.
A tag write updated the model optimistically and repainted the card
beside it, and nothing touched the digest, so the pane went on reporting
the unread count, the progress bar and the Waiting-for-you list the
conversation had when it was opened.
Reachable from the dashboard's own Mark all read button, which is the
worst version of it: the number sits directly above the button that
fails to move it.
refreshDashboardDigest() re-asks the worker for the digest of the
conversation on display, and returns at once when the pane is showing
anything else. It bumps m_digestGeneration like any other request, so
the guards in onThreadDigestLoaded() discard a reply that arrives after
the user has moved on. No placeholder digest, unlike the selection path:
the pane already holds this conversation, and blanking it to re-fill it
would flicker the whole dashboard for a change to one number.
Called from onTagsApplied(), where a write is CONFIRMED, and not from
the two write funnels. The first attempt put it beside the optimistic
model update by analogy with every other optimistic repaint, and that
analogy does not hold here: the digest is rebuilt from the index, so a
refresh queued beside the write reaches the worker before the write does
and answers from the state before it. The test failed identically to no
fix at all.
Every write rather than a chosen subset, at the user's decision:
narrowing it to the writes that change what the dashboard happens to
draw today is a list the dashboard can outgrow silently, and this costs
a round trip only while a conversation is on screen. Re-requested rather
than edited in place, because the digest is a derived summary and
recomputing it here would be a second place that has to agree with the
worker about what a write did.
The test is worker-backed over a real two-message conversation and is
driven through the mark_all_read action rather than the private funnel,
which is the path the dashboard's own button takes. It asserts the pane
carries the unread state before the write, so the assertion after it
means something.
Suite: 42 of 43, with undoMovesTheMessageBack failing as it does on
master (item 136).
|
|
Item 178. everySelectedRowIsInATrashFolder() read
ThreadSummary::firstMessagePath for any row that was not a message row.
That was correct while a thread row MEANT that message (item 108) and
stopped being correct when item 177 made it mean the conversation. A
conversation is in the trash only when ALL of its messages are, so a
partly trashed thread answered on whichever message the query returned
first: Delete could be hidden on a conversation that still had mail
outside the trash, and Restore offered on one that mostly did not.
Not data-affecting. Both actions are no-ops in the wrong direction:
Delete on already-trashed mail takes moveMessages()' already-there
branch, and Restore on mail that was never trashed finds nothing to
move.
qtmaildir cannot produce such a thread itself, since Delete is absent on
a reply row and Restore is thread-scoped. Two things outside it can:
another client trashing a single message, and a reply arriving after the
conversation was trashed.
ThreadDigest already walks every message of the selected conversation
for its sender counts, and a filename is served from the index like
everything else in it, so the paths ride along on a request the
selection already makes rather than costing a walk on every query.
ThreadDigest::messagePaths is relative to the mail root, for the reason
firstMessagePath records: an absolute path matches no account and
silently resolves every row to none. MainWindow keeps them beside the
dashboard's thread id and clears them when the dashboard is left, so a
late digest cannot answer about another row.
One limit, stated in the code rather than hidden. The digest is
requested only for a single selected conversation row, so that is the
only case with a real answer; any other selection falls back to the
summary's one path. That fallback IS the pre-177 answer and is wrong in
exactly the same partial case, which is the point: a multi-row selection
is left no worse than it was, rather than given a second, differently
wrong rule of its own. Making it exhaustive costs a per-query walk over
every message, which is what this avoids.
Two tests, both mutation-checked. The worker test puts its two messages
in different folders, since two in one folder answer identically
whichever way the code resolves them. The window test asserts both
directions, so a fix that simply hid Delete everywhere would fail it,
and sets totalCount explicitly: a summary left at the default is a
message row, and the test would otherwise exercise the other branch and
pass for the wrong reason.
Suite: 42 of 43, with undoMovesTheMessageBack failing as it does on
master (item 136).
|
|
A thread row has no message to render, so the pane shows the conversation
instead. A thread of one message still opens its message on one click, and
the automatic mark-read is not armed for a row that displays nothing.
|
|
Closes item 176. applyTags reports the messages whose tags really moved, and
a command stores that rather than what it asked for, so undoing a mark-read
no longer marks the whole conversation unread.
|
|
Closes item 170 under item 177. A conversation belongs to a view while any
of its messages match, so reading one message of a thread no longer takes
the conversation out of the Unread view. The current row is never evicted,
and an automatic write defers its eviction until the selection moves.
|
|
Item 112 hid the toggle whenever the selection disagreed, because a union
is not a state and no honest label existed for it. That was affordable
because the "Whole thread" submenu sat beside it carrying two absolute
entries, which worked whatever the mix.
Item 177 deletes that submenu: the row decides the scope, so a second set
of actions is a second answer to a settled question. Hiding the toggle
then leaves the commonest conversation in the mailbox with no key at all.
The rule is a catch-all instead. Any unread message, a mixed conversation
included, reads "Mark thread as read" and marks every message read; only a
fully read selection reads "Mark thread as unread". Two presses therefore
reach either state from anywhere, which is what makes one key enough.
The write direction moves with the label. Computing it from
everySelectedRowHasTag() while the label promised "read" would mark a
mixed conversation unread, which is the item 112 report happening again
from the other end; the mutation putting that back fails the new test.
The three-valued selectionTagPresence() is unchanged and still asked, since
Every and Mixed differ for other callers. Only this label collapses them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PEK9z5D3oa1nVmJ6xpQhBs
|
|
The five *_thread actions and their submenu are gone: the row's identity is
what decides the scope, so a second set of actions was a second answer to a
settled question. mark_thread_unread went with them, being the sixth entry in
the same submenu. tagSelected() loses its TagScope parameter, and
everySelectedRowHasTag() its own, so the direction and the write ask the same
question of the same object. ThreadListModel::scopeFor() and messageScopeFor()
are deleted; scopeForSelection() is the one resolver.
Labels name the scope. Archive, Delete, Restore, Spam, Important and the
unread toggle all say "thread" on a conversation row, and Delete, Restore and
Archive are ABSENT on a reply: a single reply cannot be removed from a
conversation.
Compose follows the same rule. Forward, Save, Reply-all and Reply without
quoting disappear on a conversation row, which shows no message to act on, and
Reply becomes "Reply to this thread": reply-all, quoting nothing, threaded off
the conversation's NEWEST message so the answer lands at its end rather than
forking the discussion at its opening post. That id is not in the model, since
an unexpanded conversation holds no nodes for its replies, so it comes from
resolveThreadMessages(); resolveQuery() states its newest-first sort rather
than inheriting notmuch's default.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012iDeN6C7y97nHYPvP6ST4L
|
|
Items 110 and 111 reconciled a card that showed one message with a row that
was a thread. The row is the conversation now, so the union is simply what
it means: the first-message substitution, PillOwnCountRole and the seeded
first node all go.
|
|
Item 119, and item 146 which is the same request recorded again. The status
bar's count answers "is my work safe to quit on" and could not say what the
work was.
The label opens a read-only list on a click. A QLabel has no clicked signal,
so the press is taken by MainWindow's existing event filter rather than by
replacing the label with a flat QToolButton, which would have brought the
style's button metrics into a status bar the label already sits correctly
in. The pointing-hand cursor is the affordance, since a status-bar label has
room for nothing else.
The layout is the user's own: a message appears once with its actions
beneath it. PendingChangesDialog::rowsFor() does the grouping over a run of
rows sharing an id, which the snapshot has already ordered, so the actions
under one message keep the order they were made in.
Read-only, deliberately. Retrying or discarding a change from here would be
a new mutation path with its own undo question, and the count exists to be
understood rather than edited.
Three rules the tests pin, each of which is a way the list could disagree
with the count it was opened from:
- Grouping must not collapse: two actions on one message are two rows.
- A thread row stays thread-scoped and reports how many messages it covered.
- An id the index no longer holds still opens a run of its own, showing that
its subject is unknown rather than folding its actions under the message
above it. This is why the row carries startsMessage rather than inferring
it from a non-empty subject.
The queued call carrying QStringList, QList<bool> and QList<int> is covered
by a test that drives it across a real thread, since a container whose
metatype does not resolve is dropped at runtime and the slot runs with a
default. Both survive on Qt 6.11; the test is what says so, and what would
fail if that changed.
Italian ships with it: five new strings, lupdate clean, lrelease 522
finished and 0 unfinished.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P88Q3MCSCSQxKDy7pmXh9F
|
|
Item 119, first half: the data the list behind the unsynced-changes count
is built from, with no dialog and no worker, so the rules it has to follow
are testable on their own.
pendingChangeSnapshot() gathers the three queues the count sums into
PendingChange rows. Two properties are the whole point.
Scope follows the ACTION, not the storage. A held thread edit stays one
thread row, because a `*_thread` action made it and reporting its messages
instead would claim the user acted on each one; a netted tag edit and a held
move are message rows. The queues already encode that distinction, so
nothing is expanded and nothing is escalated.
The rows are grouped by id, so a message with several outstanding actions
appears once with its actions beneath it, which is the layout the user
asked for. The sort is stable, so those actions keep the order they were
made in; QHash has none of its own, and without it the list would reshuffle
between openings.
A snapshot, taken once and frozen. Subjects are empty here and filled by the
resolve step to come.
m_pendingTagEdits gains the action name beside the direction it already
kept. The direction alone was enough to count with; a list has to say what
each change was, and only the action that made it knows. It is carried from
TagChange::description rather than derived from the tag, so there is no
second table of tag names to labels to drift from the first.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P88Q3MCSCSQxKDy7pmXh9F
|
|
Item 119's stated blocker, removed by finding out what it held: nothing.
pendingEditCount() summed four sources, three of which can name the messages
they hold and one of which was a bare int. That int counted confirmed
changes carrying no message ids, on the reasoning that an edit which cannot
be netted must still register rather than be lost. It was what made the
count impossible to open and list, since a dialog would have shown three
groups and then owed the user a remainder it could not describe.
The remainder is empty. NotmuchWorker::applyTags() is the only emitter of
tagsApplied(), and its first statement returns on an empty id list, which is
the exact condition the counter required. applyTagsToThreads() resolves
threads to message ids through a query and errors out when that comes back
empty, so it can only ever hand applyTags() a non-empty list.
Measured rather than read. A qFatal in the branch fired in 4 of 70
test_mainwindow cases, all four building a TagChange by hand and invoking
the slot directly with no worker involved; an assertion before the worker's
own emit never fired across the whole suite, worker-backed tests included.
The worker's guard stays and is pinned where it lives, by
applyTagsWithNoIdsDoesNothing() in test_notmuchworker. The MainWindow test
that asserted the deleted branch is replaced by one for the consequence: a
change reaching the indicator names its messages, and an edit with its
inverse nets back to nothing, which is the property a growing-only counter
could never have. Three tests that leaned on the counter to show the
indicator now carry message ids, as a real edit always does.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P88Q3MCSCSQxKDy7pmXh9F
|
|
Restore moved the file back to the inbox folder and left it invisible: the
message carried no `inbox` tag, so the Inbox view could not see it, and the
user reported restoring a message and losing it.
Delete strips `inbox` so a deleted message leaves that view, which makes
restoring it the other half of the same change. restoreResolvedMessages()
already meant to add the tag back, and the comment above the branch
describes exactly this failure, but the comparison deciding it read
`origin`, which four lines earlier had been reassigned from the bare folder
name to the finished tag. `deleted-from:Inbox` never equals `Inbox` however
an account spells its inbox, so the branch was dead and the tag never came
back.
The destination folder is taken from the move's own key instead, which is
what the surrounding code already builds and what the comment says is being
compared.
The existing test passed against this throughout. It asserted the file
moved, the origin tag came off and `deleted` came off, all of which were
true; nothing asserted the tag that decides whether the user can see the
message afterwards. It does now, and fails against the old comparison.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P88Q3MCSCSQxKDy7pmXh9F
|
|
|
|
|
|
Item 68, which turned out to be three things once its premise was
measured. The note asked to extend a "passed" subject rule to "Fw:";
there was no subject rule, and the correlation it rested on did not
exist. What did exist was a gap nobody had reported.
Reply and forward now flag their source. The Maildir R and P flags,
which every other client sets and notmuch reads back as "replied" and
"passed", had never been written here: measured on the developer's
index, all 317 "replied" and all 6 "passed" came from other clients.
ComposeWindow emits sourceMessageAnswered after a successful send and
MainWindow routes it through sendMessageTagChange, message-scoped and
off the undo stack, for the reason auto mark-read is: the flag records
that the mail went, and the send cannot be undone.
ComposeContext carries sourceMessageId rather than reusing inReplyTo,
which is deliberately empty on a forward so the recipient's client does
not file it under the thread it left. Keying on it made the "passed"
half dead code that compiled and never fired. A resumed draft is
excluded: its kind records how the file was opened, not what the user is
doing, so flagging on it would set R from a guess.
A received forward gets its own mark. Derived from the subject at paint
time, storing nothing and reaching no server, because "passed" means "I
forwarded this" and setting it from a guess would assert something false
on 222 existing messages. subjectIsForwarded() shares forwardSubject()'s
prefix table so the two cannot disagree, strips a Re: chain first, and
takes extra locale spellings from [general] forward_prefixes, which
extends the built-in table rather than replacing it.
A mutation survived the first round and corrected a claim in the code:
QRegularExpression::escape already makes a punctuation prefix inert, so
the word guard is not about pattern validity. It stops a configured "-"
matching "-: x". The comment and test say that now.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LXCZFLXbAii5n5wtovpdhh
|
|
Item 168, found by the user while hand-testing 118: Delete could be
triggered on a message already in the trash. Not dangerous, which is how
it survived. moveMessages() finds the file already in the destination
and takes its early-return branch, so the message is reported as moved,
an unsynced change is counted, and nothing happened. Restore had the
mirror of the same problem, added unconditionally to both menus and so
offered on mail that was never deleted.
Each is now hidden where it has no meaning, which is the rule item 112
established for the unread entry. The question is about the PATH, never
the deleted tag: a message trashed by another client carries no such
tag, which is why the trash view is path-based, and asking the tag would
hide Delete on exactly the mail a trash view is full of.
Delete also removes unread now, at the user's request on the same
tangent. It travels inside the same sendMove() call rather than as a
second write, so one undo returns the folder and the tag together. This
rewrites the Maildir filename, because maildir.synchronize_flags is
true, and so reaches the server: the same mechanism the post-new hook
refuses to touch, and the difference is that the hook acts unattended on
arriving mail while this is an explicit gesture on a message in front of
the user.
A mutation survived the first round and found a real hole: comparing the
prefix without its trailing separator passed every test, because no
fixture had a folder whose name starts with the trash folder's. Under it
Delete silently vanished from mail in acct/trash-old, which is not the
trash. The fixture carries that row now and all three properties are
mutation-checked. The suite is 37 of 38, the failure being item 136 on
an unrelated path.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HFuRPtzFrSxCQjFk6tq7gD
|
|
Item 118, unblocked by 103. Message > Empty trash..., scoped to the
account selector, with no default shortcut.
purgeMessages() is a separate worker entry point from moveMessages()
rather than a flag on it, because the two look alike and only one can be
undone. It takes named ids, never a folder sweep, so the blast radius is
what the dialog enumerated and the user confirmed, and it deletes every
file of a message: notmuch deduplicates by Message-ID, so leaving one
behind leaves the message alive in the folder the user emptied.
It confirms, naming the count and the account, defaulting to Cancel.
That breaks CLAUDE.md's no-confirmation rule deliberately and the rule
now records it as its single exception, in the same paragraph: a purge
has no inverse to push onto the undo stack, so the protection the rule
provides has to come from somewhere, and the dialog is where.
Two defects found rather than reasoned. The count claimed messages whose
files were already gone, overstating an irreversible action; an absent
file is correctly not an error, but that is not the same as destroyed.
And the user's hand test found the list still showing mail that no
longer existed: a purge removes rows rather than changing them, so there
is no optimistic update to apply and nothing was connected to
messagesPurged at all. It re-runs the current query now.
Verified against the live index after the user emptied one real
account's trash: zero files on disk, zero in the index. The suite is 37
of 38, the failure being item 136 on an unrelated path. Ten new strings
translated, lrelease reports 0 unfinished.
Item 168 is filed from the same hand test, on Delete being offered on
mail already in the trash.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HFuRPtzFrSxCQjFk6tq7gD
|
|
Item 112, and 99 and 147 with it: the user's note is one design across
all three. A union is not a state. ThreadSummary::tags is notmuch's
union over the conversation, so a thread holding even one unread message
answered "unread" and the thread toggle always chose "mark read". There
was no input that reached "mark thread unread" on a mixed thread, which
is the thread a user wants it for.
The thread toggle becomes two absolute actions, mark_thread_read and
mark_thread_unread. Neither takes a default chord, at the user's choice:
Ctrl+Alt+U meant whichever direction the union picked, and since item
132 a shortcut is a chosen subset rather than a requirement. It is now
unbound.
The message-scoped toggle stays a toggle, because one message has a real
two-valued state, and its label now names the direction it will go. On a
selection with no single state the entry is hidden rather than labelled
wrongly, chosen over disabling it; the thread submenu is the route then,
and its entries are absolute.
selectionTagPresence() is the three-valued predicate that needed to
exist. everySelectedRowHasTag() delegates to it and keeps its two-valued
answer, which is all a direction needs; a label needs the third value.
The refresh is keyed on the model's dataChanged as well as on the
selection, so a write moves the label without reselecting and none of
the six optimistic-update call sites has to remember.
Three mutations fail: restoring the union predicate reports the user's
original symptom, showing the action on a mixed selection, and dropping
the dataChanged refresh. The suite is 37 of 38, the failure being item
136 on an unrelated path. Four new strings translated, lrelease reports
0 unfinished.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HFuRPtzFrSxCQjFk6tq7gD
|
|
The version alone cannot tell one build of an unreleased X.Y.Z from
another, and the user rebuilds and hand-tests unreleased builds daily.
They chose a counter over a git description: what they want to know is
that the binary is newer than the one they were running, not which
commit it came from.
QTMAILDIR_BUILD_NUMBER is a cmake option, ON by default, that runs
cmake/BuildNumber.cmake as a build step to increment a counter and write
buildnumber.h. It had to be a build step: configure_file runs once per
cmake run, so a counter interpolated into version.h.in would sit still
across exactly the rebuilds this exists to distinguish, which is why
version.h.in includes a second generated header rather than carrying the
number itself.
Two macros, and the split is load-bearing. QTMAILDIR_VERSION stays a
clean X.Y.Z and keeps the window title, applicationVersion and anything
that might ever compare versions; QTMAILDIR_VERSION_DISPLAY carries the
number and goes to the three surfaces the user picked, --version and
--help, the About dialog, and the placeholder pane. The window title was
offered and declined, since the number would then be in every
screenshot.
The counter lives in the build directory and is not tracked, so it
cannot conflict on a pull or leave the tree dirty; a fresh build
directory restarts at 1, which is honest, because it is a different
build tree. A release passes -DQTMAILDIR_BUILD_NUMBER=OFF and the header
is written empty. The SlackBuild in the my-slackbuilds repo needs that
flag and is a separate commit there.
Verified by running it, since none of this is reachable from a C++ test:
three consecutive builds reported build 2, 3 and 4, and a separate
Release configure with the option OFF reported a clean 0.27.0. Passing
the flag to a tree that does not have the option yet is an unused-cli
warning and exit 0, so the SlackBuild change is safe before 0.28.0
ships. The suite is 37 of 38, the one failure being item 136 on an
unrelated path.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HFuRPtzFrSxCQjFk6tq7gD
|
|
Item 163. mbsync renames an uploaded file to add its `,U=<uid>` infix,
and the model's `MessageRef::filePath` was captured when the query ran,
so a row loaded before that sync names a file that no longer exists.
MimeParser then honestly reports a message it cannot open.
MaildirName::resolveRenamed() answers the filesystem question: returns
the path unchanged when it still exists, otherwise looks in that one
directory for the file whose unique stem matches. mbsync preserves the
stem (`<stem>:2,D` becomes `<stem>,U=5:2,D`), which is what makes this
safe to do by filename at all. It never recurses, never crosses a folder
boundary, and refuses an ambiguous match rather than guessing, since
opening or moving the wrong message is worse than reporting none.
It lives in MaildirName because that namespace already owns the `,U=`
infix and is a pure-value unit testable without a widget. A file that
changed FOLDERS is a different question that only the message id can
answer, and NotmuchWorker::moveMessages() re-resolves that way already.
Three call sites, all of which held a stale path:
- The message pane, which reported "(unreadable message)" over a file
that was on disk and readable. Cosmetic and self-repairing.
- Reply and Forward, refused outright, so the user could not answer a
message that was sitting there.
- The draft reopen, and this is the half that costs data. The refusal
happens BEFORE any composer exists, so the user composes again into a
fresh window whose autosave has no previous path to unlink. The old
revision survives, each save mints a new Message-ID, and both files
reach the server. The unlink machinery was correct throughout and
never ran.
forDraft() seeds draftPath from the RESOLVED path, never the caller's:
seeding the stale one would let the reopen succeed and the unlink still
miss, which is the same fork arriving one step later.
Covered by five unit tests on the resolver, including the two that keep
it honest (a genuinely missing file yields nothing, and a neighbouring
message is never matched), and by an integration test that renames the
draft the way mbsync does and asserts the file COUNT, which is the shape
the fork actually takes. Both mutation-checked; the integration test
fails with the reported symptom when the resolution is removed.
The stable-Message-ID question is deliberately untouched: it is what
turns a stale path into two server-side messages rather than one
replaced file, and it wants its own item.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UUQS6n3cmsFrsjCNmwNtf8
|
|
Autosave writes the draft to the Maildir drafts folder and stops, while
the Drafts view is a notmuch path: query, so a freshly saved draft was
invisible until notmuch new ran. saveDraftNow() now emits draftSaved, and
MainWindow connects it to a new NotmuchWorker::indexDraftFile() that
indexes the one file the way moveMessages() does, with the previous
revision removed so a rewrite leaves no ghost.
The send path unlinks a draft that was indexed while being composed, so
draftRemoved -> removeIndexedFile() drops its entry too.
Measured: notmuch_database_index_file assigns NO tags (unlike notmuch
new, which adds draft inbox unread), so no tag-stripping is needed and the
draft cannot leak into a tag:inbox view.
Item 158.
|
|
Item 94. The query row is the six built-in filters (Unread, Inbox,
Important, Sent, Drafts, Trash), which compose with the account
dropdown, and every saved query lives in the More queries menu. Nothing
has to decide which of the user's queries get button space, which is the
question item 93 would otherwise have had to answer.
SavedQuery::pinned is gone from the struct, the reader, the writer, the
save dialog's checkbox and the pin/unpin context action.
The stored key is stripped rather than left ignored, at the user's
choice. That has one non-obvious requirement: `pinned` stays named in
loadSavedQueries' `known` list precisely so it is NOT collected as an
unknown field, since those are preserved and written straight back. A
mutation removing that name puts the key in the file for ever.
Confirmed with the user before starting that the built-in set covers
their use, since removing pinning removes the escape hatch this item was
blocked on.
Tests: four pinning tests replaced by two on the new rule, four more
converted from buttons to menu entries. migrationPinsEveryEntry and
aStoredGeneratedQueryIsUnpinnedNotDropped are rewritten around the
property that outlived the flag rather than deleted: an entry must be
KEPT, which is what both assertions were really guarding.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEcn3u19xPqv6ggD15PG4c
|
|
Item 157, the half item 153 did not close. A draft was editable by
double-click and by a Message-menu entry, neither of which is where the
user looks while reading one. populateMessageBar() swaps the reply pair
for edit_draft on a displayed draft.
Three things came out of hand-testing it, each invisible to the tests
written before them.
The bar keyed on currentIndex(), which a query leaves valid on a row of
the discarded result, so it kept the draft button after switching to the
inbox and the reply pair after switching to drafts. This is item 150's
trap one level up. It answers from m_currentMessageId/m_currentThreadId
now, which every blanking route clears, refilled from
showPlaceholderPane(), the one site all five of those routes share.
That exposed a defect predating the bar: updateComposeActions() ran only
from the two selection handlers, so Reply and Forward stayed enabled over
a blank pane. Invisible while they sat on the main toolbar among
always-on actions.
The bar is hidden over an empty pane, so it comes and goes with the
subject and the details button rather than hovering over the logo. That
in turn broke the showing half: setBarActions() runs before showThread()
fills m_items, so the first message opened after a blanking left the bar
hidden and the second showed it from stale items, one selection behind
for the life of the view. updateHeader() shows it, beside the details
button it rides with.
The test missed the last one by asserting before the render landed,
measuring the placeholder; it waits on showingPlaceholder() now.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEcn3u19xPqv6ggD15PG4c
|
|
Item 153. DraftStore had a write() and no reader, and nothing opened a
composer from an existing message, so a draft rendered like ordinary mail
and could never be finished or sent.
ComposeContextBuilder::forDraft() reads one back. A new Kind::Draft seeds
every field verbatim: the subject takes no Re:/Fwd: prefix, and the body
goes in exactly as it was left, with none of seedBody()'s quote framing. It
is reachable by double-click and by an edit_draft action in the Message
menu.
Three things the shape of this depends on.
A resumed draft must OWN its file. Maildir has no in-place edit, so an
autosave writes a new file and unlinks the old one; a composer that did not
know its own path would leave the original behind and one message would
become two. ComposeContext::draftPath carries it into m_draftPath, which the
autosave already knew how to replace.
MimeParser had no bcc, and nothing had ever needed one. MessageBuilder
writes Bcc into the draft file deliberately and explains why, so a resumed
draft that ignored it would drop every blind recipient from the message the
user then finishes and sends, reporting nothing.
edit_draft is gated on the file being inside a configured drafts folder,
matched on the PATH. A `draft` tag is not enough: notmuch surfaces the
Maildir D flag as one, and a message flagged by another client sits in the
inbox. Offered on ordinary mail, the composer would own a file it did not
write and the first autosave would delete a received message.
And a live defect found on the way, which is most of why this took as long
as it did. updateComposeActions() ran only from onSelectionChanged. Both
signals fire for an ordinary click, so nothing had noticed; but running a
query and setting the current index emits currentRowChanged ALONE, so the
enablement was computed against the previously selected row. Edit draft
stayed disabled on a draft selected that way, and the reply family had the
same blind spot with no test that could see it. Now connected to both.
Reading currentRowChanged is safe here for the reason CLAUDE.md gives: it
answers "which row is current", and no count is read.
WorkerBackedWindow::AccountSpec gains a drafts field, which the two new
tests need and which no fixture could express before.
|
|
Items 138 and 148.
The query row carried Unread, Inbox, Important, Sent and Trash, and no
Drafts, though the composer has been autosaving into each account's drafts
folder since compose shipped. Reaching them meant typing a query by hand.
Smaller than its size suggested: Account::draftsQuery() and
Config::allDraftsQuery() already existed for the placeholder pane's drafts
count, and builtinFilters() derives the row from kQueryGenerators, so the
work was the generator entry, two resolvedQuery branches, a label and an
icon.
It follows TRASH rather than Sent. Folder-matched like both, because `draft`
is a Maildir flag notmuch surfaces as a tag while the folder is what the
user means and what the composer actually writes into. But NOT flat: Sent is
flat so a thread cannot fold the user's own message back into the
conversation it answers, and a draft reply belongs with its conversation for
the same reason a trashed message does.
An account with no drafts folder shows no button, per item 103's rule. The
existing row test surfaced that by failing until its fixture configured one,
which is the rule working rather than a defect.
Ctrl+W closes the composer, which bound nothing at all: the only way out was
the title bar. The action is parented to the composer, so it is a
WindowShortcut dispatched to the active one only and the main window's
namespace is untouched, exactly like the formatting shortcuts. It calls
close() rather than doing anything of its own, since closeEvent() already
decides whether the draft is saved and a second route out that skipped it
would lose the message.
The Italian gains "Bozze"; lrelease reports 478 finished, 0 unfinished.
|
|
Three corrections from looking at the built bar.
Compose returns to the main toolbar. The split this was built to, "about a
message" against "about the list", does not survive contact: what matters is
what the action NEEDS. Reply and Forward are meaningless without a message on
display, while Compose needs none and is disabled only when no account can
send. So the pane's bar holds exactly the two actions that depend on what it
is showing, and Compose sits with the window-wide ones.
The bar moves below the subject and details rows, directly above the web
view. At the top of the pane it read as window chrome rather than as
belonging to the message. The transient notice bars stay above it: they
explain the message rather than offer an action on it.
Its icons were the style's own default, 16px, which is tiny beside a 32px
toolbar. They are now 7/8 of toolbar_icon_size, which is the 28 the user
asked for at their 32, derived rather than hardcoded so the relation holds
if that key changes. The test asserts the relation as well as the value,
since a bare 28 would stop meaning anything the moment the key moved.
m_headerLabel gains an object name so the placement test can find the row it
must sit below.
|
|
Items 139, 140 and 141, built together because the seam between them is
wasted work: 140 needs a container and 141 is that container.
The main toolbar had grown to mix two scopes. Sync, Archive, Delete, Mark
all read and Undo act on the list or the selection; Compose, Reply and
Forward are about one message. With everything in one row the distinction
was invisible, and Forward was on no toolbar at all, reachable only from the
Message menu, which is item 139.
Compose, Reply and Forward now sit on a bar above the message pane, and
LEAVE the main toolbar rather than gaining a second home: that is what makes
the toolbar's remaining contents mean one thing. Toggle HTML joins them at
the right end, separated by an expanding spacer, since changing how a
message is displayed is a different scope from acting on it. That layout was
the open design question item 141 recorded, and it was settled with the user
rather than guessed.
The actions are MainWindow's own QAction objects shown a second time, never
copies: a duplicate would carry its own enablement and drift from the menu
entry updateComposeActions() keeps in step. MessageView::setBarActions() is
the seam, so the pane still knows nothing about the window's action map.
Two things worth recording:
QToolBar has no addStretch(), so the separation is an expanding spacer
widget. A test asserting only on action ORDER passes with that spacer
deleted, measured, so it asserts on the spacer's size policy instead.
noTwoActionsShareAnIcon looked up the toolbar with an unnamed
findChild<QToolBar*>(). There are two toolbars now, so it is pinned to
main_toolbar: pointed at the pane's bar it would have asserted that a
thread action is absent from a bar that never holds any, and passed while
the rule it exists for went unchecked.
|
|
A composer is deliberately parentless, so that it appears in the task
switcher and stays usable while the main window is. Qt therefore does not
take it down with that window, and being a live top-level it kept the process
alive: the main window vanished, the composer stayed on screen with nothing
behind it, and closing it then raised the unsaved-edits dialog for a session
the user had already ended.
The quit path already ASKED about those edits and saved them. What it never
did was close the windows afterwards.
Closing rather than deleting: WA_DeleteOnClose is set on every composer, so
close() is what frees them, and it lets ComposeWindow::closeEvent() run its
own draft handling on the way out. Iterating a copy of the list, since
closing runs the `closed` handler and that mutates m_composers.
Placed last, after every route that turns back has returned: reaching it means
the application really is quitting.
The test opens TWO composers, so a fix that closed only the last one cannot
pass it. Mutation-checked by removing the loop.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q2koFevoSxTLhfexJTZWQd
|
|
The reply family is disabled on mail that arrived at an account with no
send_command, behind a ribbon in MessageView naming the account and the key
to add. save_message is deliberately never disabled: it is the escape hatch
for exactly that case.
The ribbon is a WIDGET in the pane's layout, never markup inside the web
view. Composing HTML from configuration into the one document that renders
input from strangers is the wrong direction, and the header row is already a
widget for the same reason.
Compose itself is disabled only when NO account can send, and that state is
not warned about at startup: an installation with no send_command anywhere
is a valid read-only installation.
Every reply resolves through messageScopeFor(), not threadFor(): a thread
row means the one message its card shows. Replying to a thread is
meaningless; a reply answers a message. The context is built from the
DATABASE rather than the model, the rule Restore already follows, because a
row whose state has not been re-queried carries stale values and a reply
built from one would carry the wrong recipients.
The mail root crosses from the worker as its own signal. There was no route
for it at all: mailRootOf() is file-static in notmuchworker.cpp, and item
124 records that composing a destination from database.path writes into the
Xapian tree under a split index. The test uses NotmuchFixture::splitIndex(),
the only layout where the two accessors disagree.
A thread row's path is RELATIVE to the mail root while a message row's is
absolute, so the account lookup matched nothing and the reply family was
dead on mail from an account that could send. Found by the positive guard
test rather than the negative one, which passed throughout for the wrong
reason.
The quit path checks the failed-save case FIRST. In the ordinary case
nothing is lost by saving; there, saving is what is already not working, so
the dialog says plainly that quitting loses that text rather than offering a
save that will fail again. Both dialogs name the composers, and the ordinary
one asks once whatever the count, because three modals in a row is worse
than a coarse answer. Its wording says drafts already saved stay in the
folder, so Discard cannot read as 'delete my three messages'.
The Save loop holds QPointers, not raw pointers. A deleteLater() posted
while a nested exec() runs IS processed by that nested loop, measured in a
standalone program: the guard nulls before the modal returns. Closing a
composer while the quit dialog is up therefore freed a window the loop then
called saveDraftNow() on, crashing at the exact moment the application
promised to preserve that text.
A compose request that matches nothing clears itself and says so. It was
cleared only on a match, so a message deleted between selection and Reply
left the request armed for the session: Reply did nothing, and the next
ordinary click on that message opened a composer nobody asked for while the
pane stayed blank.
Forward carries the original's attachments, which the context has always had
a field for and nothing ever filled, and seeds its HTML toggle from
[compose] send_html. Only Reply seeds that from the original.
save_message keeps its filename inside the chosen directory and no longer
overwrites a file already there. The check was correct and untested: the
test asserted through Attachment's helpers rather than through the function
production calls, so deleting the containment check outright left it green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TvwDptMWxjqhbCmjxwcSZ2
|
|
Handlers are empty for now; this commit is the registration, so the three
coverage tests guard every later task rather than being satisfied at the end.
Two corrections to the spec, both found in the code rather than assumed. It
calls for a new top-level Message menu and one already exists, so these join
it; two menus named Message would be a defect. And it says every action
needs a binding, which item 132 changed while this was being planned:
save_message ships with no chord, since it is the rarely-used escape hatch
and menu reachability is now the rule that must hold.
reply_no_quote shares reply's icon and is added to the no-duplicate-icons
exception list for the same reason the five thread actions are: it never
reaches the toolbar, and a menu entry always carries its text. That list is
renamed menuOnlySharedIconActions, after the property that earns the
exemption rather than the tier that first needed it.
Bindings are provisional. The user intends to rework them, and Ctrl+Alt+R
for reply_no_quote is an imperfect fit since that tier elsewhere means a
wider scope rather than a variant.
The six labels went through a mnemonic pass that nothing enforced before.
Four of them collided inside the Message menu on first writing, and the
whole class was invisible to a green suite: Qt does not error on a duplicate
mnemonic, it cycles the highlight instead of activating, so the key simply
stops working. Item 57 had already decided this rule by rejecting a label
that would have collided, but it lived in prose and in one test's comment,
which is precisely why it was broken again here.
noMenuHasTwoEntriesSharingAMnemonic() enforces it now, scoped per menu since
a mnemonic resolves among the open menu's entries, and keyed on
QKeySequence::mnemonic() rather than on parsing & by hand, because && is a
literal ampersand and only Qt answers which key it will dispatch. Three
pre-existing collisions are a named freeze list rather than a silent fix or
a narrowed test: Alt+R three ways and Alt+S twice in Message, Alt+O in View.
Renaming entries a user has had in their fingers since 0.1.0 belongs to the
shortcuts rework, and the freeze is written as exact groups so a new entry
joining any of them still fails.
Two of the test's own design choices came from mutation checks that failed
for the right reason while reporting the wrong thing. Reporting collisions
as pairs was order-dependent, so a new colliding entry re-keyed a frozen
pair and the fresh defect read as "a frozen collision no longer happens";
matching frozen entries by whole string broke the same way, since a growing
group stopped matching its frozen text. It reports whole groups and matches
on menu plus key.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FXF741wz4SY7j5dqvAxMU5
|
|
The status bar's sync bar was a bare QProgressBar configured inline in
MainWindow, and item 123's send popup needs the same thing again. It also
needs the half MainWindow does not use: the popup drains a determinate bar
through its cancellable countdown and switches THE SAME widget to
indeterminate when send_command starts and the duration stops being
knowable. Building that inline a second time is what this removes.
BusyIndicator carries both modes. setBusy() resets the range as well as the
visibility, so the switch out of the countdown cannot leave the bar drawing
its last fraction, and setProgress() treats a total of zero as busy rather
than passing it through: setRange(0, 0) IS the indeterminate range, so a
zero total would otherwise hand the caller an animating bar while it
believed it had drawn an empty one.
Only the bar is extracted, not the status label the backlog row mentions
beside it. m_statusLabel has 34 uses across MainWindow for transient
messages, selection counts and sync phases; it belongs to the window rather
than to the indicator, and the send popup owns its own phase text.
The hidden-on-construction test needs a shown parent, which cost a mutation
to find. Measured against a standalone Qt program: a parentless widget
reports isVisible() false and isHidden() true whether or not hide() was ever
called, so both obvious assertions passed against a constructor with the
hide() deleted. What differs is WA_WState_ExplicitShowHide, and the
behaviour it produces appears only once a parent is shown, which is how the
status bar holds this widget.
All five mutations checked and killed: the zero-total guard, the range reset
in setBusy(), the value clamp, the show() in setProgress() and the hide() in
the constructor.
|
|
Reported from a hand test: Restore moved the message correctly and the row it
came from sat in the trash list until the Trash filter was clicked again.
The trash view is path-based, so a restored message stops matching the query
the list was built from. That is a state no tag change can express, and nothing
in onMessagesMoved() removes a row, deliberately: in an ordinary view a deleted
message's card should stay put, since one deleted message does not doom the
conversation.
refreshCurrentQuery(), not runCurrentQuery(). The refresh runs immediately
after the undo entry is pushed, and re-running the query outright clears the
undo stack, which would make Restore the one mutation in the window with no way
back. Gated on isShowingTrash() rather than on the destination, because a
Delete is a move too and reaches the same slot.
Three tests, each catching a different mutation: the row leaves, undo survives
the refresh and still moves the file back, and a delete outside the trash view
leaves its row alone.
The third one was wrong on its first draft and passed against the mutation it
existed to catch. It used a `tag:inbox` view, which looks ordinary but which a
deleted message keeps matching, since Delete adds `deleted` and the origin tag
and removes nothing. A path query on the inbox folder is the honest instrument:
the file really leaves, so the row survives only because nothing refreshed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Every version before item 103 tagged a message `deleted` and left its file
exactly where it was, so deleted mail accumulated in the inboxes with only a
chip to say otherwise. `Find stranded deleted mail` runs the query that finds
it: tagged `deleted`, and not inside any configured trash folder.
It reports and moves nothing. Acting on its own would be a bulk delete with no
selection behind it, and the user asked for something they could come back to
and review. Repeatable rather than a startup migration, for the same reason:
mail reaches this state again whenever another client tags without moving.
A menu entry only, at the user's request, so it cannot be confused with the
Trash filter beside the other four.
Also adds everyActionIsReachableFromAMenu(), which asserts the fifth
registration site nothing enforced. CLAUDE.md documents four places; a menu is
the fifth, and `restore` shipped on this branch reachable by a chord and by
nothing a user could see. The new test found three more of the same:
open_thread, clear_pane and clear_selection were all keyboard-only. All three
now sit in the View menu.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Del is the key a user reaches for and Ctrl+D is not a guess anyone makes.
Both are bound; Del is listed FIRST because that is the one the menus
advertise.
Bare, which is safe here for a reason that does not generalise to other
bare keys. A QAction shortcut is dispatched before the focused widget sees
the key, and Qt withholds only plain LETTERS from editable widgets, so by
the argument that made bare Return break the query bar this should delete
mail while the user edits a query. It does not: QLineEdit accepts the
ShortcutOverride for Delete itself, because Delete is one of its own
editing keys, which Return is not. Measured with and without an explicit
filter, the action fires 0 times either way, so no filter is added.
theDeleteKeyEditsTextInTheQueryBar() pins that Qt behaviour, since the
binding rests on it.
**Two defects surfaced from the second binding, both real.**
An action can now have more than one default, and KeyMap did not allow for
it. sequenceFor() decided "is this a built-in?" by comparing against
defaultSequenceFor(), which returns only the FIRST default, so the second
looked like a user override and won the "a user binding beats the default"
rule. The menus advertised Ctrl+D to a user who had configured nothing, and
sequenceFor() and defaultSequenceFor() disagreed about an untouched action.
isDefaultBinding() asks whether a sequence is ANY of the action's defaults;
when two defaults tie, the one defaultBindings() lists first wins, which is
the author's stated preference rather than an alphabetical accident.
And Restore read each message's origin tag FROM THE MODEL. The model's tags
come from the query, so a row whose delete has not been re-queried still
carries its pre-delete tags: measured `[inbox,unread]` on a message already
sitting in the trash, one run in three. No origin tag was found, the
message took the no-origin branch, and Restore moved it to the INBOX
instead of the folder it came from, silently, with the origin tag left
behind as the only evidence. A restore has to be right about the
destination or it is worse than doing nothing.
The trash-view Restore now resolves its messages against the DATABASE
first, through a new NotmuchWorker::resolveMessages(). That and
resolveThreadMessages() share one walk, resolveQuery(), rather than
growing a near-duplicate: they differ only in whether the terms are `id:`
or `thread:`. restoreSelectedThreads() already worked this way; this is the
same reasoning applied to the message-scoped path.
The flake was found by running one test five times rather than trusting a
single green, and the fix verified the same way: 5 of 5, then the full
suite three times over.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Task 6. Delete moved mail into the trash and the only ways back out were a
second press of Delete or Ctrl+Z, both of which act on a row the user has
to have deleted in this session. Browsing the trash and putting something
back needed an action of its own.
`restore` is enabled from the QUERY, not from the selection's tags. The
trash view is path-based precisely so that mail trashed by another client
appears in it, and such a message carries no tag of ours: deciding from
`tag:deleted` would disable Restore on exactly the messages that most need
it. isShowingTrash() compares the current query against the trash
generator's own, for both the per-account and the all-accounts scope, so it
follows the account dropdown like every other filter.
A message with NO origin tag is the foreign-trashed case, and it is why
this is not simply restoreSelected() under a new name. The two callers want
opposite things from a missing origin, which `fallbackToInbox` selects.
From the trash view the message is demonstrably in the trash and refusing
to move it leaves the user looking at mail they cannot get out, so it goes
to the inbox and the status bar says so. From a second press of Delete the
message is not in the trash at all and merely wears a stale `deleted` tag
from an older version or a hand-written notmuch command; moving that to the
inbox would relocate mail the user never asked to move, so the tag comes
off and the file stays put.
The inbox FOLDER is a new optional per-account `inbox` key, defaulting to
"Inbox". It is configurable rather than hardcoded because the name is not
ours to assume: naming a folder that does not exist CREATES it, beside the
real one, and under mbsync's `Create Both` that folder reaches the mail
server. That is not hypothetical, it is what a truncated origin folder did
to real mail while this branch was being tested. Unlike `trash` the key is
optional, since the default is right for any ordinary Maildir and a wrong
value here only affects the fallback.
Ctrl+R, which was free. The action is only enabled in the trash view, so
the key is inert elsewhere rather than doing something surprising. It sits
in the Message menu beside Delete and in the thread context menu, greyed
outside the trash rather than hidden: an action that vanishes teaches
nothing, while a disabled entry with its shortcut beside it says both that
it exists and where it applies.
**Adding an action is FIVE places, not four.** knownActions(),
defaultBindings() and the icon table are each enforced by a test that fails
loudly, and being REACHABLE is a fifth that nothing checked: this shipped
registered, bound, iconned, correctly enabled, and present in no menu at
all, which a green suite reported as complete. Ctrl+R is not a shortcut
anyone guesses, so it was effectively invisible.
restoreIsReachableWithoutTheKeyboard() closes that, and deliberately
excludes the context menu from its menu-bar assertion, since findChildren
returns both and one check would otherwise satisfy the other.
Four tests, each mutation-checked. Two worth keeping: the hardcoded "Inbox"
mutation fails against the fixture's lowercase folders exactly as it would
against a Maildir that spells its inbox differently, and the reachability
mutation reproduces the keyboard-only state this shipped in.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Item 103's implementation was committed unreviewed and never hand-tested.
Reviewing it, and then hand-testing it against real mail, found seven
defects. Six of them lose or corrupt state and none was caught by the
suite, which was green throughout.
**Undo pushed a command instead of consuming one.** onMessagesMoved()
pushed a MoveCommand for every confirmed move, including the move an undo
had just made, so undoText went "Delete", "Undo Delete", "Undo Undo
Delete". A second press of undo re-deleted the message the first had
rescued. PendingMove carries a fromUndo flag, which has to survive the
queued round trip and so cannot be a window-wide "am I undoing" flag.
**Held moves were invisible to the quit guard.** pendingEditCount() summed
the held tag edits and not the held moves, so a Delete pressed during a
sync left the count at zero: the indicator stayed hidden and closeEvent()'s
guard never fired, discarding the move on quit with no prompt. That is item
106's data loss with a worse shape, because a dropped move leaves the file
in the folder the user asked it out of.
**Two moves to one folder dropped the second's tags.** m_pendingMoves was
keyed on the destination, so two Deletes in one account before the first
confirmation both named `acct/Trash` and the second insert overwrote the
first. That file reached the trash carrying neither `deleted` nor
`deleted-from:`, unrestorable and invisible to a `tag:deleted` query. It is
a FIFO now: the worker moves one batch at a time and emits in request
order, so position alone matches a confirmation to its request.
**Second Delete left the origin tag behind.** The restore passed the origin
PLACEHOLDER in its removal list, and onMessagesMoved() resolves that from
the folder the worker reports, which on a restore is the trash. It asked to
remove `deleted-from:Trash`, a tag never written, while the real
`deleted-from:inbox` was never named. A restore does not need the
placeholder: it already read the origin to decide where to send the file.
originTagFor() is now the one derivation both sides use.
**Ctrl+Z left it behind too**, for a different reason: MoveCommand was
constructed with the unresolved pending.add. The command carries the
resolved tags now, and is pushed per origin group rather than once per
batch, because the placeholder resolves to a different tag per origin.
**A thread root re-deleted itself.** everySelectedRowHasTag() asked a
thread row about its THREAD's tags, which notmuch gives as a union. Delete
the root of a three-message thread and the replies are untouched, so the
union carries no `deleted` and a second press ran Delete again: the message
moved trash-to-trash and came out with `deleted`, `deleted-from:inbox` AND
`deleted-from:Trash`, with no way back. The union was a documented
approximation, called bounded because the worst case for a TAG toggle was
re-applying a tag the message already had. A MOVE re-applies the move.
Resolved through messageById(), NOT through ThreadSummary::firstMessageTags,
which is the value the query delivered and is never refreshed by an
optimistic update: after a delete the node reads `deleted` while the
summary still reads `unread`.
**Delete thread never moved anything.** It was left calling tagSelected()
when Delete became a move, so a whole conversation sat in the inbox wearing
a `deleted` chip. It moves every message now, each with its own origin, so
a thread spanning folders reassembles on restore. A reply row resolves to
its own thread through selectedThreadIds(): scopeFor() reports a reply
under messageIds and leaves threadIds empty, which made a thread action on
a reply row do nothing at all.
**And the root card did not repaint** until it was clicked, while its
replies did. sendMove() had no optimistic update at all, so nothing moved
until the worker answered; and applyMessageTagChange() deliberately leaves
a multi-message thread's SUMMARY alone, which is correct for a one-message
edit and wrong for a thread-scoped one. The replies have nodes and
repainted; the root card reads the summary. The thread paths repaint
synchronously with applyTagChange() before the worker is asked, which also
keeps the toggle's direction readable for the next press.
Every fix carries a test and every test was mutation-checked. Three false
greens were found while writing them and are recorded at their assertions:
a disjunction that emptied on the wrong term, a QTRY_VERIFY(rowCount() == 0)
satisfied by the interval before the worker answers, and a query issued
before the confirming write had landed. Absence is asked of notmuch
directly through a new notmuchCount() helper for that reason.
Two bare-window tests moved off assertions about synchronous pending writes
onto the model, since the thread actions now round-trip through the worker.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Delete added the `deleted` tag and moved nothing, so deleted mail sat in
the inbox indefinitely with only a chip saying otherwise. It now moves the
file into the account's trash, records where it came from, and moves it
back on undo.
The origin is derived in the WORKER, not in the UI, because nowhere else
knows it. A Maildir filename does not record the folder a message came
from and notmuch cannot answer once the file has moved, so the moment the
old filename exists inside moveMessages() is the only place it can be
read. It travels back on a new messagesMovedFrom() signal, and the UI
turns it into a `deleted-from:<folder>` tag that Restore reads days later.
The account is resolved from the message's PATH rather than from its
account tag: that tag is optional config, so resolving through it would
silently make an account undeletable. That needed ThreadSummary to carry
the first message's path, since an unexpanded thread row is the ordinary
case and held no path at all. It is reported relative to the database
root, because the UI knows accounts only by their maildir, itself a
database-relative prefix.
accountForMessagePath() accepts both an absolute and a relative path, and
that is load-bearing rather than defensive: a thread row's path is
relative while a reply row's is absolute, since MimeParser has to open it.
Matching only one form left Delete on a reply resolving to no account and
moving nothing, which is the thread-row/reply-row asymmetry this file has
been bitten by before.
Tags are applied only once the worker CONFIRMS the move. Tagging first
would leave a message marked deleted in a folder it never left when a
rename fails, which is the half-done state this removes. A move made
during a sync is held in its own queue and flushed like a tag edit: the
existing queue carries tag changes only, so a move pushed through it would
apply `deleted` and never move the file.
An account with no trash configured reports through the status bar and
tags nothing, as a second line of defence behind the config-load warning.
Six existing tests used `delete` as a stand-in for a message-scoped tag
action on bare windows with no account; they move to `spam` and
`delete_thread`, which stayed tag-only, keeping the property each was
actually testing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Adds trash as a fifth built-in query filter beside Unread, Inbox,
Important and Sent, composing per-account exactly as Sent does:
Config::resolvedQuery() asks each account for its own trashQuery()
rather than wrapping the all-accounts union, and an account with no
trash folder resolves to matchNothingQuery() rather than "match
everything".
Also gives the Trash button a toolbar icon (user-trash) and a trash
key to the mainwindow fixture that asserts every filter button carries
one; without it the button is skipped from the row entirely (no
account configured a trash folder), and the existing icon test found
no button to check.
|
|
The `flag` action only ever added the `flagged` tag, so pressing Ctrl+I on a
thread or message that was already important re-applied a tag it already had.
Re-applying a tag changes nothing and repaints nothing, so the key read as
dead, and removing `flagged` meant opening the tag dialog.
It now reads the current state and picks a direction, exactly as `delete` and
`toggle_unread` beside it do. One direction is chosen for the whole selection:
it unmarks only when every selected row is already important, so a single
keystroke cannot leave a selection in two states.
The direction comes from everySelectedRowHasTag(), never a hand-rolled loop.
Two separate bugs went into that helper on 2026-08-16 (items 88 and 105), and
a copy of the then-current `delete` loop would have inherited both: resolving
a reply's row number against the top-level list, and asking a reply's THREAD
where the write is message-scoped, which makes a toggle one-way.
The reply test needs THREE different states to mean anything: the first thread
in the list unflagged, the reply's own thread flagged, and the reply itself
unflagged. With the reply left in its thread's state, the mutation putting
item 105's bug back stayed green, measured. The fixture helper defaults
replyTags to the thread's, so a test that does not pass them explicitly
asserts nothing about scope.
Backlog item 98.
|
|
A thread's card has rendered one message since item 66, but every tag
action still acted on the entire conversation. Delete, Archive,
Important, Mark spam and Toggle unread now act on the message the card
shows; the whole-thread versions move to a "Whole thread" submenu in the
Message menu and the thread list's context menu, on Ctrl+Alt+<key>.
Closes items 87, 88, 105, 106, 107, 108, 109, 110 and 111.
The defects fixed along the way, several found by reading rather than by
report:
- threadAt(current.row()) answered about the wrong thread for a reply
row, because a tree numbers rows per parent. The audit found four live
sites, not the one reported: Delete and Toggle unread each chose their
DIRECTION from an unrelated thread, and the tag dialog counted the
wrong thread's tags. threadFor(index) replaces them.
- A message-scoped write made no optimistic model update and no reply
row carried a doomed cue, so acting on a reply moved the pending-edit
count and changed nothing on screen.
- Both toggles read the state of a reply's THREAD, which a
message-scoped write never changes, so they were one-way: the second
press re-sent a tag the message already had.
- flushHeldEdits() re-sent only thread-scoped edits, so a tag change
made on one message during a sync was applied to the row, counted as
unsynced, and then dropped without ever being written.
- applyTagChange() updated a thread's summary but not its loaded
replies, leaving an expanded thread's rows describing a state the
database no longer held.
- A thread's first message is not among its children, so both
message-scoped lookups missed it: acting on a root card repainted
nothing and emptied the message pane's chip row.
- ThreadSummary::tags is notmuch's union over the thread, so a card
standing for one message drew tags belonging to its siblings. The
worker now reads that message's own tags in the walk that already
finds its id, so the split is known before a row is ever opened.
The card shows both tiers: its own message's tags at full size, the rest
of the conversation's smaller and muted, so nothing appears to vanish
when a row is selected.
Auto mark-read is message-scoped as a result, and now arms for a reply,
which it never did. With maildir.synchronize_flags on, the old
thread-wide write reached the server for mail that had never been
displayed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Double-clicking any row drills into its thread: the list becomes that thread
alone, expanded, and the pane shows the double-clicked row's own message. A
reply therefore opens its WHOLE thread with itself selected, never itself alone,
which is what the user asked for and is not the obvious reading of "open it by
itself".
This is recoverStaleThread() triggered by a gesture. That function already ran
thread:<id>, expanded the thread when the row arrived, selected the target
message once the replies landed, and fell back to the root when the message had
gone; all three cases are existing paths through it, so the new code resolves a
row to a thread id and a message id and hands both over.
The row is reached through the INDEX and never through index.row(): a tree
numbers rows per parent, so threadAt(row) on a reply answers about an unrelated
thread. That is item 88's trap, avoided here by construction.
The first click of a double-click arms the mark-read timer, and the handler
cancels it, because a gesture that navigates must not mutate mail. The timer is
armed again for whichever row the recovery lands on, so only the arming for the
row being left is cancelled. Its test asserts the timer was active beforehand,
so it cannot pass by the timer never having been armed at all.
The expander keeps its own double-click: ThreadListView::mousePressEvent accepts
a press inside its rect and returns, so Qt never pairs one into a double-click
there.
Nothing is built for getting back. The filter buttons already are that, per the
user.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
An edit made while a sync is running is held rather than sent, because the
worker's read-write open blocks on notmuch's exclusive lock. At sync end
onExternalSyncStateChanged() refreshed the list first and flushed the held
edits afterwards, so the refresh read a database that still carried the old
tag, reconciled it into the model, and overwrote the optimistic update the hold
had deliberately left applied. The flush then wrote the tag correctly.
The database ended up right and the list ended up wrong, with nothing scheduled
to re-read it, which is why it looked like the edit had been lost. Reported by
hand: a message read during a sync went back to unread when the sync finished.
The flush moves ahead of the refresh and keeps both properties it already had.
It stays outside the Idle branch, so edits held when /proc/locks becomes
unreadable are not stranded waiting for an Idle that never comes, and it stays
after the status-bar retire, so its own "N held changes sent" message survives.
Both orders leave identical end state, so the first version of the test passed
against the defect: after the handler returns the queue is empty and the write
has been sent whichever ran first. flushGenerationForTesting() stamps the query
generation at flush time, which is what separates them, and the test fails
against the old order with Actual: 3, Expected: 2.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
runAutoSync() returned without rescheduling when a sync was already in flight.
The comment defending it argued the edits were not lost, because they reached
the mail store at edit time and the running sync was "very likely" to carry
them. Very likely is not always: an edit made after mbsync has already passed
that account's mailbox is not carried by it, the timer had fired, nothing
re-armed it, and the pending count sat non-zero until a manual sync or the next
cron run.
Skipping is unchanged and still required by item 71: the cron job holds the same
lock and mbsync fails on a second concurrent run. What changes is that the skip
schedules another attempt. scheduleAutoSync() re-checks the delay, the sync
command and the pending count on the way in, so this cannot arm a sync for
nothing, and against a long external sync it re-arms once per debounce interval,
which is a timer rather than a sync.
The test fires the timer by hand and asserts it is active again afterwards, at
the configured interval rather than a shorter one, with the pending indicator
still showing. It fails against the old skip path.
Item 89's other half is dropped rather than built. The list churn it described
is a tag-defined view working as intended: a thread that loses `unread` leaves
the Unread view, and the user resolved it by living in the Inbox view instead.
Three designs were drafted before asking and none is worth building.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Running a query blanks the message pane but left m_currentThreadId,
m_currentMessageId and m_currentMessageThreadId naming the thread that had
been showing. Both selection handlers compare a newly selected row against
those to decide whether it is already on display, so a result containing that
same thread was recognised as "already showing" and onThreadSelected() was
never called. The card painted as selected, the status bar reported one
thread, and the pane stayed on the placeholder.
This is why it looked like an `id:` query defect. The id is copied out of the
details dialog of the message being read, so that thread is current at the
moment the query replaces the view. Any query returning a different thread
hides the fault entirely.
Filed as the unverified half of item 66 and assumed to be the same
empty-MessageIdRole failure. It is not: 66's fix was correct and this
reproduced against it, so it is recorded as item 96. Four hypotheses were
eliminated by measurement first: the row does carry the message id, the
account-scoped query does return it, MimeParser parses the reported message
(ok, 40701 bytes of HTML), and both real ids resolve bare and quoted.
The regression test's first query must open the SAME thread the second one
returns; with two different threads it passes against the defect, which is how
the first version of it was green. Reverting the fix fails it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
