| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
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 thread MOVES every message now, and a move needs message ids and
file paths that the UI does not hold: a thread the user never expanded has
no nodes in the model for its replies, so those live only in the database.
resolveThreadMessages() answers with both, in one combined query, for the
same reason applyTagsToThreads() resolves threads here rather than in the
UI: a query per thread reopens the same Xapian cursor once per selected
row.
Paths come back RELATIVE to the database root, matching
ThreadSummary::firstMessagePath, because the UI knows accounts only by
their maildir, itself a database-relative prefix. Without them the caller
would know which messages to move and not where any of them belongs.
Tags come back too, because Restore reads a message's `deleted-from:` tag
to decide where to send it and an unexpanded thread's messages have no node
to read tags from.
They are joined by a TAB, not a space. A notmuch tag may absolutely contain
a space: the folder "Inbox/SlackBuilds users" produces
`deleted-from:Inbox/SlackBuilds users`, and splitting that on spaces
truncated the folder to "Inbox/SlackBuilds". Restore then moved the
messages into a folder of that name, CREATING it, so real messages ended up
in a directory mbsync does not sync and read as missing. Under `Create
Both` that folder can propagate to the mail server. A tab cannot appear in
a tag, since notmuch's own dump format is line-based and
whitespace-delimited.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
validateTagName() ran on the removal list as well as the addition list, so
a tag whose name contains a space could be seen on a message and never
deleted: the one dialog that could clear it refused the only text that
names it, and it did so with a modal warning, so the dialog would not even
close.
Whether a tag SHOULD exist is a separate question from whether the user may
delete one that already does, and the answer to the second is always yes.
Validation now runs on additions only, which is where the rule earns its
keep: it stops a troublesome name being created.
Reached by a real Maildir folder named "Inbox/SlackBuilds users", whose
origin tag carried the space through. Only the TYPED route was ever
blocked; unchecking the tag in the list appends to the removal list after
validation has run and worked throughout. The test asserts both routes for
that reason, and asserts that ADDING a spaced tag is still refused, since
the fix must not weaken the rule it narrows.
The natural mutation for this test hangs rather than fails: restoring the
validation raises a modal warning with nothing to dismiss it. The test
avoids calling accept() on the add-rejection case and asks the validator
directly, and the mutation check drops the tag silently instead.
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>
|
|
The first mutation here that is not a notmuch tag. Indexes the new path
before dropping the old one, since removing the last filename for a
message id deletes the database entry and every tag on it.
|
|
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 trash key is mandatory: Delete moves a file into it, so an
account without one cannot delete at all. Report it as a config
problem naming the account and the key, rather than degrading
Delete silently, per the existing "a warning the user cannot act on
teaches them to ignore warnings" rule (item 83).
Several existing test fixtures loaded accounts with no trash key and
asserted zero problems/warnings; added trash=Trash to those where it
was incidental to what the test actually covers.
|
|
|
|
Nine tasks, TDD throughout. The ordering mutation in task 4 is the
load-bearing check: indexing the new path before removing the old one is
what preserves a message's tags, and the wrong order passes every other
test while silently destroying them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Item 103's measurement is done, so its section carries the finding and the
three constraints that decide whether to open the spec, rather than the
design inline.
Item 118 is blocked on 103 and is the first action that would destroy mail
with no undo, which is why it is filed separately rather than folded in.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Measures what Delete does today (a notmuch tag and nothing else, verified
against the tag-to-flag table and a probe on a throwaway database) and
specifies the move-to-trash behaviour that replaces it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
|
|
The reconciliation against the user's own notes found ZERO unrecorded
entries, the first clean pass: the 2026-08-16 sweep added items 98 to 104 and
those absorbed the whole current "Not done yet" list.
Closed this session, sections moved to the closed-items file: 98, 100, 102.
Dropped:
- 78, at the user's request. Never a defect. Items 85, 23 and 81 already give
the whole journey (right-click a value, search it, save the query, make a
rule from it); this was only a shortcut across it, and the entry had already
said to gather usage evidence first. That evidence never appeared.
- 116, the same day it was raised, and its section is kept for the process
failure rather than the non-bug. Copy image was reported as copying markup
instead of pixels. Two explanations were eliminated by real evidence, and
the conclusion drawn was that something more interesting must be wrong; the
actual answer was that the measurement distinguishing them was broken. A
wl-paste reading taken minutes after the copy showed text flavours only, was
explicitly labelled unreliable in the entry, and was then reasoned from
anyway. Run immediately after a copy it reports image/png and 30 more, and
pasting into GIMP immediately works. A caveat that does not stop the
reasoning it qualifies is decoration.
Opened:
- 112, Toggle unread on a whole thread cannot reach "all unread" on a
partly-read thread. The direction comes from notmuch's UNION over the
thread, so one unread message anywhere makes the action pick "mark read" and
no input reaches the other branch. Third defect from that union after 110.
- 113, view source as our own plain-text dialog.
- 114, Save image is offered and does nothing: no downloadRequested handler
exists anywhere. The user corrected the first proposal, which would have
refused remote images on security grounds; once remote content is granted
the bytes are already fetched, so saving them is a local copy and blocking
it protects nothing.
- 115, no confirmation when a copy succeeds.
- 117, the pane offers no Select all. NOT caused by item 100: verified against
a build with that filter reverted. Three wrong theories preceded that
measurement, and the lesson is one item 100 had already written down: a menu
built by hand proves nothing about the menu Chromium builds.
The changelog's Unreleased section gains Important-as-a-toggle, the rules
Note column, the menu fix, and two Upgrading notes.
|
|
Item 108 added five whole-thread actions and their submenu last session and
never refreshed the translation, so fifteen strings had no Italian: every
entry under "Whole thread", every undo text those actions push, and their
tooltips. lrelease silently DROPS an unfinished string and ships the source
text instead, so an Italian UI showed an English submenu and English undo
entries with nothing reporting a problem.
Found by running lupdate after adding this session's own strings: it reported
19 new, of which only three were mine. The gap is the reason ctest -R
translations exists, and it did not catch this because the .ts file was never
regenerated after item 108 landed.
Also translates the Config warning about an unparseable `language` value,
which had been untranslated since the i18n audit.
lrelease now reports 373 finished, 0 unfinished. The submenu was read in a
running LANG=it_IT build and confirmed correct.
|
|
The `note` field explains why a rule is shaped the way it is, and it was
reachable only by selecting the rule and reading the editor form, which is the
wrong way round for the one field that says what a rule is for.
Note is the LAST column, after Matches, at the user's request: a note is prose
and the widest thing in the table, so it belongs where it can run on without
pushing the narrow columns off screen. That is fiddlier than it looks, because
"Matches" is not in the Column enum at all: it is appended past the end at
index ColumnCount. Note therefore has to be declared before ColumnCount and
still draw after it, and setColumnCount takes a new ColumnTotal rather than
ColumnCount + 1. Both columns hold text, so a mix-up puts the counts under
Note and looks entirely plausible; the test asserts the counts land under
Matches as well as asserting the header order, since the header assertion
alone passes with the two swapped.
The cell is simplified(), because a note is free text and a newline truncates
a tree row at it. The full text is the cell's tooltip and is untouched in the
editor.
Also fixes a defect found on the way, which is not in the backlog entry.
QHeaderView::restoreState REFUSES a state saved with a different column count,
returning false and leaving the header untouched, which is what every existing
uistate.conf now does. The restore path set m_columnsSized and
m_countColumnSized regardless, spending the one auto-size each column gets on
a restore that did nothing: the new Note column would have opened at its
default width, once, permanently. Now guarded on the return value. Upgrading
costs one reset of this dialog's column widths, which is unavoidable, since
the saved state genuinely describes a table that no longer exists.
Backlog item 102.
|
|
The pane's context menu started from QWebEngineView::createStandardContextMenu()
and kept it whole, so it offered Back, Forward, Reload and Save page. None of
them can apply: every message is rendered with setHtml() from memory, so there
is no history to go back to and nothing to reload, and the request interceptor
blocks everything by default. They were inert as well as meaningless.
removeBrowserActions() matches on the QAction pointer returned by
page->action(), never on the entry's text, which is translated: a text match
would work in English and fail in every other locale, which is a defect no
test written in English would catch. Removing entries also strands separators
at the edges or doubles them up, which reads as a menu that lost something, so
the filter sweeps them; Qt offers nothing for this.
View source is deliberately NOT filtered. It was removed with the other four
at first, which was an overreach: the user asked for four and view-source has
a real document and a real use. Chromium's own entry cannot work here either,
since it navigates to view-source:<url> and MessagePage refuses that, so
backlog item 113 implements it as our own plain-text dialog.
The test builds a menu by hand, which is right for testing the filter and
proves nothing about what Chromium's real menu contains. That limit is stated
at the test, and is why it does not assert on SelectAll: the real menu has
never offered it, verified by hand against a build with this filter reverted
(backlog item 117).
Backlog item 100.
|
|
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>
|
|
The packaging under assets/slackbuild/ had been stale since 0.7.0 and was
deliberately not SBo compliant, carrying a _danix tag and a txz package type
because it was never an SBo submission. It is not what the user builds from, so
it lived here collecting drift: it predated the translations added in 0.23.0 and
would have shipped a package without them.
It now lives in the my-slackbuilds repo under qtmaildir/, rewritten from the SBo
cmake template rather than carried across, at 0.24.0 and building from the
GitHub release tarball rather than a cgit snapshot whose checksum can change
when the tarball is regenerated. That repo has the workflow for bumping it and
an nvchecker stanza that picks up releases from here.
README points at the repo instead of describing a local directory, and keeps the
build invocation so the section still tells a Slackware reader what to do.
CLAUDE.md's release procedure drops the "do not bump this as a side effect"
warning, which has nothing left to warn about.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
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>
|
|
The entry said a double-click runs a query naming the row, an id: for a message
and a thread: for a thread. That is wrong for a reply, which the user wants to
drill to its THREAD with itself selected, not to itself alone. The view is
always the whole thread, expanded; only which message the pane shows changes.
It also assumed a thread: query would show the conversation. Nothing in the tree
auto-expands, so it lands on one collapsed card and the replies still need a
click.
Both are already solved by recoverStaleThread(), which runs the query, expands
the thread, selects the target reply when the replies arrive, and falls back to
the thread when the message has gone. Item 91 is that mechanism triggered by a
gesture rather than by the stale-thread notice, so the approach is to reuse it
rather than write a second selection-after-query path.
Recorded alongside: a double-click delivers a single click first, which arms the
mark-read timer, so the handler must cancel it rather than marking a message
read that the user only passed through.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
91 needed one decision, how the user leaves a drill-down, and the answer is that
nothing is built for it: "I didn't think of a back action, usually I'd go back
to a known list like unread or inbox at that point." The filter buttons already
are that and are one click away in every view, so the Back action, the history
stack and the restore-the-previous-query scheme are all unnecessary. That also
retires the undo-stack concern: a drill-down clears the stack exactly as a typed
query does, which is the behaviour the user already expects from the query bar.
The item is now fully specified and ready to build at S.
92 is postponed at the user's request: "I don't see the utility, so I don't
really know how to answer." The clarification that preceded it named the
per-message version, which needs provenance nothing records, costs a format
change across both repos against a hook running on real mail every ten minutes,
and stays blank on every message already tagged. A feature whose requester
cannot say what it is for should not be built. The cheaper substitute is
recorded beside it for if the question ever turns out to be "why does this
message carry this tag", which rules.json can answer at read time with nothing
stored.
Section moved to the closed-items file on this commit, per CLAUDE.md.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Both were "open, unspecified" and could not be planned from the backlog alone.
91 is not a second window. The user meant a drill-down to the selected thread
or message, the same result an `id:` query gives, reached by double-clicking
instead of typing. That removes the constraint the entry was mostly made of,
since no second QWebEngineView and no extra render process are involved, and
takes it from ? to S. What is left to decide is how the user gets back, and
that runQuery() clears the undo stack, which a gesture-triggered query would do
silently.
92 is the expensive half of the two it held: the user wants a per-message hint,
rule-written against hand-applied, and doubts its utility in the same sentence.
Nothing records that today, so it is a format change across both repos before
it is any pixels, and it stays blank on every message already tagged. Recorded
beside it is the cheaper question that may be the real one, "which rule would
tag this message", answerable from rules.json at read time with no stored
provenance and no hook change.
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>
|
|
|
|
The four filter buttons gave no sign of which one you were looking at, so
the row said what you could do and never where you were.
The active filter is drawn as a checked QToolButton, which lets the style
paint its own pressed look: a hand-picked highlight colour would have to be
picked once per theme and would still be wrong under a third.
The check state is derived from the query TEXT rather than from the last
button clicked, which is the whole design decision. A record of what was
pressed goes on lying the moment the query is edited into something else,
where a highlight that follows the query clears itself and lights again
when a filter's query is typed by hand. It is resolved against the account
box, so changing account recomputes it rather than dropping it: the same
filter under two accounts is two different query strings and both are
still "Inbox".
Buttons are held in a hash keyed by generator, cleared at the top of the
row build because the row is rebuilt wholesale on every saved-query edit
and stale entries would dangle. The connections are owned by the row
widget, so a rebuild takes them with it rather than leaving a second copy
firing at deleted buttons.
Unread opens already highlighted, which is correct rather than incidental:
startup_query defaults to it, so the window opens on that view. The test
asserts it, so the assertions that follow are known to be a change of
state rather than a button that happened to start unchecked.
Mutation checked against the design that was rejected: deriving the state
from the click instead of the query fails all three tests, each naming the
behaviour it protects.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
The interface language followed the environment and nothing else, so
choosing it meant setting LANG for the whole application. [general]
language overrides it in both directions: it selects Italian on an English
desktop, and en_US forces English on an Italian one.
A short code or a full locale name both work, since Qt resolves "it" to
it_IT when the QLocale is built and QTranslator::load falls back from
qtmaildir_it_IT to qtmaildir_it. "system" is the default written down, so
the default can be expressed rather than only reached by deleting the key.
Validated on the locale NAME rather than on whether a translation loads,
because those are different questions and only one is an error. QLocale
accepts any string and degrades an unrecognised one to C rather than
failing, so `language = itallian` loads no translation and is otherwise
indistinguishable from asking for English on purpose; meanwhile
`language = en_US` legitimately loads nothing, English being the source
language and shipping no .qm. Checking the name separates the typo from
the deliberate choice, and the typo is reported.
The translator is now installed after Config is loaded, since the config
is what chooses it. The cost is that config warnings are generated before
the translator exists and are therefore built in English; retranslating
them would mean re-running load(), and a warning about the config file is
the one string a user can still act on in either language.
Verified against the real loader across six configurations, under both
LANG=en_US and LANG=it_IT: short and full codes select Italian, system and
an absent key follow the environment, en_US forces English whatever the
environment says, and a bad name reports a problem and falls back. Two
mutations checked: dropping the name validation and treating "system" as a
locale name each fail a test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Nothing loaded a translation before this: no QTranslator, no .ts file and
no build rule, so every string was English whatever the locale said. The
language now comes from the environment, LANG=it_IT.UTF-8, and any other
locale runs in English as before.
The audit found that the tr() discipline was largely holding, and found
eight strings that could never be translated into any language. kFields[]
in tagrulesdialog.cpp declared the rule-builder field labels with
QT_TR_NOOP inside an anonymous namespace, where lupdate reports "tr()
cannot be called without context" and extracts nothing, while the use site
calls TagRulesDialog::tr() on them at runtime. From, To, Cc, Subject, Tag,
Folder, Attachment and Date: the whole vocabulary of the rule builder,
absent from every translation file that could ever exist. The source
compiles and reads correctly; only lupdate reveals it.
Q_DECLARE_TR_FUNCTIONS is not the fix for that case, though it is the fix
for a free function calling tr(). Measured against lupdate: a class
carrying the macro beside the array still extracts 0 strings, because the
context must be attached to the literal itself. QT_TRANSLATE_NOOP names it
explicitly and matches the tr() that already reads them, so the use site
needed no change.
Twenty configuration and keybinding warnings were not translatable either.
They are user-facing, reaching the status label and the "Configuration
problems" dialog. Config already had the tr() macro; KeyMap needed it.
Translating the filter labels then broke startup_query, found in hand
testing: a filter's name is a translated label, so `startup_query = Inbox`
matched nothing where the filter shows as "In arrivo". The application
opened a different view and reported the user's own working config as
invalid. Resolution matches the generator as well now, which is stored in
queries.json and identical in every locale; the translated name still
works. The regression test installs a real QTranslator rather than a stub,
since the bug lives in the gap between the stored string and the displayed
one, and it writes a queries.json because the warning it asserts on is
guarded by a non-empty saved-query list: without one the branch never runs
and the test passes against a broken check.
main.cpp's --help and --version stay bare printf, as they run before
QApplication exists and no translator could serve them.
Verified per the backlog's own standard, that lupdate output is the
evidence rather than reading: 355 strings extracted with zero context
warnings, where before there were 327 with eight; lrelease reporting 355
finished and 0 unfinished; the built .qm loaded in a standalone probe
printing "From -> Da" and both Italian plural forms; and the install rule
placing it where main.cpp looks. test_translations guards it and was
mutation checked, failing on an emptied translation and naming the defect
when QT_TRANSLATE_NOOP is reverted.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
|
|
startup_account matches Account::key, the [account.<key>] suffix, and not the
mbsync channel. The two genuinely differ in a real setup: a section key may
carry dots that the channel does not.
The dot is the part worth pinning. QSettings treats "/" as a group separator,
which is why account sections use a dot at all; a dot INSIDE the key is a
different case, and a silent mismatch would report "not a configured account"
and quietly start on All accounts. Verified against a standalone probe before
this test was written.
The second half asserts the channel is rejected, so the two names cannot be
interchangeable in one direction only.
|
|
Answers "start me in work - Inbox rather than All accounts - Inbox". The key
names an account by its [account.<key>] suffix and the dropdown is set to it
before the startup query runs; because a built-in filter composes with the
dropdown, that is the whole mechanism and the key never reaches a query
builder. Validated on load: a name matching no account is reported and cleared,
since the dropdown has no entry for it and would sit on All accounts without
saying why.
Which side applies the scope depends on what the startup entry is, and getting
it wrong is silent in both directions. A generated filter comes back from
resolvedQuery() already scoped, so letting runQuery() apply the dropdown again
gives path:"work/**" and (path:"work/**" and (tag:inbox)). A saved query does
not, because resolvedQuery() ignores the account key for one, so claiming it
was already scoped leaves it unscoped with the dropdown pointing at Work.
The first of those shipped in this session's working tree and passed its test,
because the assertion used contains() and the double-scoped string contains the
scope too. It asserts the exact query now. The second was found by writing the
test for the case rather than by reading, and is covered by
aStartupAccountAlsoScopesASavedStartupQuery.
The README's startup_query documentation was wrong on two counts after the
previous commit: the fallback is the Unread filter rather than the first query
in the file, and the name can now match a built-in filter.
|
|
Two defects, both reachable only after item 93.
startupSavedQuery() searched the saved queries alone, so a startup_query of
"Inbox" matched nothing once Inbox shipped as a built-in filter and the
duplicated entry was removed from queries.json. It then fell back to
m_savedQueries.first(), which is an arbitrary choice that used to look
reasonable while every install carried an Inbox entry: with the duplicates gone
it opened on a leftover search for one sender, and an empty queries.json opened
on nothing at all. The search now covers the saved queries first, so the user's
own entry wins a name collision, then the built-in filters; the fallback is the
Unread filter, which is always present.
The default startup name has always been "Unread" and now resolves for the
first time: before this it named nothing unless the user happened to have such
an entry.
The constructor also read startup.query directly, and a generated entry stores
no query at all, so even a matching filter opened an empty bar. It resolves
through Config::resolvedQuery() now, unscoped, since the account dropdown
starts on "All accounts".
Icons per the user's choices: a star for Important rather than the flag
action's own icon, since on the query row an icon reads as a category rather
than as an instruction, and mail-folder-sent rather than mail-sent.
Three tests changed rather than adapted, because their premises were the
defect. Two asserted the first-saved-query fallback.
aCronSyncDoesNotRefreshBeforeAnyQueryHasRun assumed a fresh window had run no
query, which is no longer true; it is now
aCronSyncRefreshesTheLastRunQueryNotTheQueryBar and asserts the property that
actually matters on a cron timer, through a new lastRunQueryForTesting() seam,
since a legitimate refresh bumps the generation and the counter cannot tell the
two apart.
|
|
Item 57 renamed the `flag` action to "Important" in 0.14.0, chosen over
"Starred" partly because &I was free where &S collided with Mark spam. Item 93
then shipped the filter for the same tag as "Flagged", so one window offered
both names for one thing. The generator keeps its own name, `flagged`: that
string is stored in queries.json and matched against a closed set, so it is
wire format rather than a label.
The filters are QToolButtons now, like the Save button at the other end of the
row, carrying a themed icon with the text beside it. Icon AND text for the
reason the Save button already records: this row is a row of text buttons, so
an icon alone reads as a different kind of control than it is. Theme icons
rather than the shipped SVGs in Marks, because item 70's split is that the
panes are ours and the chrome is the system's, and the query row is chrome.
mail-mark-important matches the `flag` action's own icon, since the filter
finds what the action marks.
The icon test asserts a NAME was requested rather than that the icon resolved:
QIcon::fromTheme returns null where no icon theme is installed, so isNull()
would fail for a reason unrelated to this code. Dropping the setIcon call fails
it.
Widening the buttons to QToolButton broke eleven tests that reached them
through findChild<QPushButton *>, which does not match a sibling type. The
helpers and the filter lookups take QAbstractButton; savedQueryButton() stays
on QPushButton, since the user's own queries really are those.
|
|
|
|
Items 93, 95 and 90. The query row starts with Unread, Inbox, Flagged and Sent,
shipped by the application rather than pinned by the user, and they compose with
the account dropdown instead of resetting it. Item 90 is absorbed: the button
that cleared the account selection is no longer a saved query at all.
Item 95 was found by hand testing this and is not caused by it: no entry in the
saved query overflow menu had ever been runnable, because Qt emits no triggered
for an action that owns a submenu.
|
|
Item 93 ships four built-in filters composing with the account dropdown, and
absorbs item 90. Item 95 is the overflow-menu defect found while hand testing
it: pre-existing, not caused by 93, and recorded as its own item rather than
folded in.
Two traps go to CLAUDE.md because they are still true of the code. Qt emits no
triggered for a QAction owning a menu, which is why every entry in the saved
query menu had always been inert. And a generator must be asked for one
account's query rather than having its all-accounts query wrapped, since the
wrap returns the right rows by accident of path: being hierarchical and a
row-count test cannot tell the two apart.
A third goes beside the existing rendering-probe warnings: visualRect reports a
real height for a row scrolled out of the viewport, so a non-empty check passes
while the pixel loop measures nothing and blames the wrong defect.
The Upgrading note covers what a user sees: the row starts with four shipped
buttons, a duplicate name means one of theirs is now beside a built-in, and
their migrated Sent entry is unpinned for them rather than deleted.
|
|
An unpinned query could not be run. Its menu action carried both a triggered
connection and a submenu of edit actions, and Qt does not emit triggered for an
action that owns a menu: clicking the entry only opened the submenu, so the
connection had never fired.
It shipped unnoticed because the menu was the rarely-used half while the user's
queries were pinned buttons. Item 93 moved every query into the menu, which is
how it surfaced, and item 94 makes the menu their only home, so this is now the
path that has to work.
Running is an item inside the submenu, first and above a separator, with the
edit actions below it. The entry keeps its submenu because an unpinned query
must still be editable and deletable.
The test asserts the Run item exists and is first, then that triggering it
reaches the query, then that Edit and Delete survived beside it. Restoring the
old wiring fails it on the first of those, naming the Qt behaviour rather than
just reporting a wrong query string.
|
|
Item 93, the UI half. Unread, Inbox, Flagged and Sent are buttons the
application ships, sitting first on the row, ahead of the user's pinned saved
queries.
runFilter() is runSavedQuery()'s opposite in the one way that matters: it READS
the account box and never writes it. That is item 90's defect. A filter narrows
what the user is already looking at, so the dropdown is its input rather than
something it resets on the way past. A saved query keeps setting the account
from what it stored, because it is a destination and states its own scope.
runQuery() gains an AccountScope parameter. A filter's text arrives already
resolved in the selected account's scope, and scoping it again would put
path:"work/Sent/**" inside path:"work/**".
Two migration changes, both of which unpin rather than delete:
- Sent is no longer migrated from the INI into queries.json. The built-in
filter covers it, and migrating one too would put two Sent buttons on the
row, one editable and one not.
- A stored entry naming a known generator is unpinned on load, which is what
every install upgraded through 0.19.0 carries. It keeps its name and its
generator and moves to the menu. Deleting it would be data loss on a file
whose readers are supposed to preserve what they do not own.
The test suite needed the same distinction the design makes.
savedQueryButtonLabels() now skips the filters, and savedQueryButton(window,
label) replaces five positional row->findChild<QPushButton *>() lookups that
were silently returning Unread.
One rendering probe had to be fixed rather than adapted.
replyRowsKeepTheirTextUnderTheThreadLine resized the window to 300px, and four
more buttons pushed the reply row below the viewport: the pixel loop then ran
zero times and reported "0 pixels, the row was painted over", which is a
different defect from the one it exists to catch. It gets 600px and a guard
asserting the row is really inside the viewport, so the next person to shrink
it gets told the truth. Verified by putting 300 back: the guard names the row
at 83..165 in an 82px viewport.
|
|
Item 93, the Config half. Four built-in filters, Unread, Inbox, Flagged and
Sent, as generated entries in kQueryGenerators, which was already a closed set
validated on load for Sent alone.
resolvedQuery() gains an overload taking an account key, and that is what makes
a filter compose with the account dropdown instead of fighting it. A generator
is asked for the account's OWN query rather than having its all-accounts query
wrapped in a scope: wrapping gives
path:"a/**" and (path:"a/Sent/**" or path:"b/Sent/**")
which returns the right rows only because path: is hierarchical, so a row-count
test passes against it. The tests assert on the query string for that reason,
and the mutation putting the wrap back fails two of them.
An ordinary saved query ignores the account key and keeps resolving through its
own stored account, which is the behaviour item 90 leaves alone.
matchNothingQuery() exists because an empty query means "match everything" to
notmuch: an account configuring no sent folder would otherwise give a button
labelled Sent that shows the entire Maildir.
Config gains Q_DECLARE_TR_FUNCTIONS for the filter names, which are button
labels. The generator names are not translated: they are matched against the
closed set and stored in queries.json, so translating them would make a file
written in one locale unreadable in another.
No UI yet, and no migration: the query row still builds from pinned saved
queries.
|
|
The user's end state for the query row is built-in filters only, with every
saved query living in the menu. At that point SavedQuery::pinned has nothing
left to decide.
Blocked on 93 and deliberately separate from it: the four buttons have to be
lived with first, and if one of them is wrong, pinning is the escape hatch,
which has to still exist to be used.
Recorded as a user-visible removal rather than a cleanup. pinned shipped in
0.18.0 as a checkbox in SaveQueryDialog and a right-click action, so removing
it is a minor bump with an Upgrading note. The stored field is a separate
decision from the UI, and leaving it in queries.json unread is both cheaper
and reversible.
The spec for 93 gains the ordering rule this resolves: filters first in fixed
order, the user's pinned queries after them, and nothing configurable, since
the mixed row exists only until 94 lands.
|
|
Explaining item 90 to the user produced a reframing rather than a fix. The
buttons and the "more queries" menu are two different kinds of thing sharing
one mechanism: a filter narrows whatever the user is looking at and should
compose with the account dropdown, while a saved query is a self-contained
destination entitled to set the account itself.
Nothing ships as a default today, so the buttons are whatever the user pinned,
which the queries.json migration did to every [queries] entry. That drift is
the defect.
Item 93 ships four built-in filters, Unread, Inbox, Flagged and Sent, as
generated entries in the closed kQueryGenerators set that already exists for
Sent. The user's own pinned queries are unpinned rather than deleted once the
buttons are confirmed working.
Three findings from reading the code, all in the spec. A generator must answer
per account rather than having its all-accounts query wrapped in a scope, or
Sent becomes path:"a/**" and (path:"a/Sent/**" or path:"b/Sent/**"), which
returns the right rows only because path: is hierarchical. Sent is flat and
the other three are not, so the four match in scope and not in view mode. And
m_accountBox has no signal connected to it, which is now a decision rather
than an omission: changing the account runs nothing, the button is the verb.
Item 90's section moves to the closed file, kept in full because its cause and
the rules preview that motivated the reset are still true of the code.
|
|
Item 74. runQuery() set the status bar once and only queryFinished cleared
it, so the bar kept claiming a query was running for the whole walk while
rows were visibly arriving behind it. Measured cold against a 1.1 GB index:
the first batch reaches the model at 642 ms and the walk finishes at 5714 ms,
so five seconds of a slow query read as a frozen one.
onThreadsReady now sets the bar from the model's own row count after each
batch, which is the number of rows the user can actually see. No timing
changes; this only stops the bar from lying.
The refresh branch returns before the new line, so a background refresh stays
silent exactly as onQueryFinished already keeps it. That silence has its own
test, which fails when the write is moved above the guard.
beginRefreshForTesting() is a new seam: refreshCurrentQuery() returns early
without a worker and a bare window has none, so a test cannot otherwise reach
the refresh path.
|
|
The 2026-08-15 pass over ~/Documents/Obsidian/note/notes on qtmaildir.md
found four entries with no item here, and one already-closed item with a
case that was never verified by hand.
89: runAutoSync() returns without re-arming the timer when a sync is
already in flight, so an edit made after the running mbsync passed that
account waits for a manual sync or cron. The second half of the same
complaint, rows leaving the list mid-interaction, is a design question
and is recorded as one rather than guessed at.
90: runSavedQuery() clears the account box for a query that names no
account. The line is deliberate and its comment says why, so this needs
a decision rather than a fix.
91 and 92 are unspecified, as the notes themselves say.
Item 66's row gains the one case its fix should already cover: a
single-message id: query whose card would not open is the same empty
MessageIdRole failure, unverified against an id: query.
|
|
Item 66 turns out not to have been the defect it was filed as. The pane
was never blank: an unexpanded thread root rendered the CONVERSATION,
and the same click rendered one message once the thread had been opened,
because the model learned the root's message id only when the replies
arrived. The user's step-by-step account is what separated the two
halves; two probes against a real database had failed to reproduce the
blank pane because there was none.
Closed by carrying firstMessageId in the query and removing the
conversation view, which the user asked for after being told the stubs
not expanding was itself a defect and that the feature was being judged
in a broken state.
Two defects came out of it and are open. 87: auto mark-read still marks
a whole thread, coherent while a root rendered the conversation and not
any more. 88: threadAt(current.row()) answers about the wrong thread for
a reply row, because a tree numbers rows per parent. 87 is blocked on 88
and the entry says why: a fix for 87 was written, mutation-checked,
shipped and reverted the same evening after it marked an unrelated
message read.
CLAUDE.md gains the row-number trap as its own entry rather than leaving
it implied by the item 20 note, plus the rule that a test for a write
path must exercise the reply case: the reverted fix was green because it
asserted on a root selection, the one case where row() is correct. The
cid-prefixing note is corrected to say every caller now passes one
message while explaining why the prefixing stays.
The changelog carries a Removed entry and an Upgrading note, including
that mark_read_delay_ms accepts a negative value to disable auto
mark-read entirely, verified against config.h.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Reverts bde7409 and 66f1159. The user hit the worst possible symptom:
clicking one message marked a DIFFERENT, unrelated message read.
The cause is in markCurrentThreadRead, which reads
m_model->threadAt(current.row()). CLAUDE.md records this exact trap: a
tree numbers rows PER PARENT, so a reply's row() indexes its siblings
and threadAt() on it answers about an unrelated thread near the top of
the list. The guards then compared the right ids against the wrong
thread and let a write through for whatever message the timer's state
named.
That fault predates these commits, but they made it reachable and
harmful: while the write was thread-scoped the mismatch was mostly
masked, and scoping it to a single message turned it into "a random
message is now read".
Reverting rather than fixing forward. Marking the wrong mail read syncs
out to the server and cannot be undone from here, so the safe state is
the previous behaviour, which is too broad but predictable. The item 66
work in 4a4f82f stands: a thread root still renders one message.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Follows the message-scoped mark-read. The user reported the write going
out with nothing visible changing: the status bar counted an unsynced
edit while the card stayed bold and the message pane still showed the
`unread` tag, until the next query corrected it.
sendMessageTagChange made no optimistic model update on purpose, because
applyTagChange is keyed by THREAD and repainting a whole row for a
one-message edit would claim every reply had changed too. That trade is
right for an explicit tag edit and wrong for auto mark-read, where the
visible change IS the feature and the delay exists to deliver it.
ThreadListModel::applyMessageTagChange updates the message wherever it
is held, as a child row and as `first`, and lets the thread's summary
follow only when the answer is unambiguous: a thread reads as unread
while ANY message does, so the tag is cleared from the thread only when
no other message still carries it. For an unexpanded multi-message
thread the per-message tags are not loaded, so the summary is left for
the next query rather than guessed at.
Mutation checked: without the call the card holds `unread` for the full
timeout.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
