| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Sorting was hardcoded NEWEST_FIRST. Two orders only: notmuch's other two are
MESSAGE_ID and UNSORTED, neither of which is an order a human wants, and sorting
by sender or subject would have to happen in the model after results arrive,
which fights the batching that makes a large query paint immediately.
loadThread keeps OLDEST_FIRST unconditionally: a thread reads chronologically
whichever way the list is sorted.
|
|
A reply card shows only these. The alternative, a reply's full tag set, was
rejected on measurement rather than taste: in the user's database 7 of 48691
messages carry unread and 75 carry flagged, both already drawn another way, and
every other tag is applied per thread and identical on all its messages. Full
sets would repeat the thread's chips down the whole expansion, which is the
striping the row-wide strip was built to avoid.
|
|
theImportantActionStillWritesTheFlaggedTag arrived from master, where the
thread list is a QTableView, onto a branch where item 20 made it a
QTreeView. findChild<QTableView*> returned null and the test failed on its
first QVERIFY, before reaching anything it was written to check.
Uses the branch's selectThreadRow helper, which is what every other test
here already calls: QTreeView has no selectRow of its own.
|
|
CLAUDE.md described a QTableView over a table model, which has not been true
since the view port. Updated with the traps the port produced, each of which
shipped a plausible-looking broken build before being caught:
- A tree numbers rows per parent, so nothing may be keyed on a row NUMBER.
- drawBranches runs before the row's cells, so an expander on a content
column is painted over by the delegate's background.
- setRootIsDecorated(false) removes the style's HIT AREA along with its
indicator, leaving a glyph that renders and does nothing.
- isExpanded and setExpanded are keyed on column 0.
- A reply's indent must beat the account chip's width, and visualRect
reports the indent correctly even when nothing is visibly indented.
- paintEvent runs after the cells, so a full-row fill erases their text.
Also the notmuch ownership rule, which is a double-free if undone: messages
reached through a thread are freed with it, so walkReplies holds them raw
against this file's own RAII convention.
Item 20 is marked built, not done, and item 53 records why. The user's verdict
on the finished result was that the table view does not fit the use, said with
every cue in and working. That is a design finding rather than a defect: the
item shipped exactly what its four decisions specified, and all four were the
user's own choices. Recording it as a defect would misattribute the cause;
recording nothing would leave the next session building on a rejected design.
Item 53 carries the cause verified in code rather than guessed. A message row
fills the same five columns as a thread row (threadlistmodel.cpp:275-283
mirroring :428-431), so replies land on the same rigid column boundaries as the
threads around them, and the eye reads columns before indentation or tint. The
reference the user gave has no column rules through its reply rows at all, and
that absence is the one thing three added cues cannot supply.
|
|
tagSelected resolved rows to threads with threadAt(index.row()), which is wrong
for a message row: a child's row number indexes its siblings, so acting on a
reply tagged whichever thread sat at that position in the list. It now routes
through ThreadListModel::scopeFor, and a message row's change is sent as message
ids down applyTags with its own MessageTagCommand for undo.
MessageTagCommand stores message ids where ThreadTagCommand stores thread ids,
and that difference is the point rather than an inconsistency: re-resolving the
thread on undo would restore tags across every sibling the action never touched.
sendMessageTagChange deliberately skips the optimistic model update.
applyTagChange is keyed by thread and would repaint the whole row as though
every message in it had changed, which for a one-message edit is a lie the user
watches correct itself on the next query. It keeps the two things that are NOT
optional: the edited-account set, resolved through the containing thread since
the account is a property of the thread, and holding the edit when a sync holds
notmuch's write lock, since the worker's read-write open blocks rather than
failing.
The scope is now stated before an action and after it, naming both the message
count and whether a whole thread went. This is what stands in for the
confirmation dialog CLAUDE.md rules out: undo is the safety net, and undo is
only usable if the user can tell that something larger than they meant has just
happened. Selecting a single message reports no count at all, since reading one
message is not a bulk action.
A mutation that routed message rows down the thread path SURVIVED the whole
suite: undo depth and status text are identical either way while every sibling
gets tagged. anActionOnAMessageRowTagsThatMessageNotTheThread exists because
that gap was found, and asserts on the ids actually sent.
anActionOnAThreadRowSaysItHitTheWholeThread reads the status bar BEFORE draining
the event loop. This binary has no worker, backlog item 36, so the queued write
reaches a database that has never heard of the thread and answers with
errorOccurred, which overwrites the status bar: draining first asserts on that
error and fails against correct code.
|
|
loadMessage queries by id: and returns one MessageRef, always matched, since the
user asked for that message by clicking its row and a stub would answer the
wrong question. An unknown id emits an empty vector rather than an error: a
stale row after a reindex is an ordinary race, not a failure worth the status
bar. The signal fires even when empty so the UI handler runs instead of waiting
for a reply that never comes.
The branch in onThreadSelected is placed BEFORE threadAt(), which is the whole
trap. threadAt takes a top-level row number and a child's row number indexes its
siblings, so handing a message row's number to it loads whichever thread happens
to sit at that position. Mutation-checked: with the branch disabled the test
reports thread 't1' for a reply belonging to 't2', a wrong answer plausible
enough to survive review.
m_currentMessageId and m_currentThreadId are mutually exclusive and each clears
the other, so a queued reply can tell which kind of selection it belongs to.
onMessageLoaded carries a third guard onThreadLoaded does not need: a reply
landing after the selection moved to a thread row would render one message where
the conversation belongs.
No mark-read timer for a message row in this pass. Marking one message of a
thread read is a per-message tag write and the pending-edit map is keyed by
thread; item 28 is the record of what happens when that count goes wrong.
|
|
Indentation alone still read as a table, which was the user's original
complaint about the whole item. Three cues now say the rows belong to the
thread above them: a spine down the left of the expanded block with a stub out
to each reply, a background tint, and text a size down and undimmed only when
unread.
Both colours are mixed from the palette rather than fixed, the same rule
readColour follows: a tint that reads as grouping on a light theme is invisible
or muddy on a dark one. The tint is deliberately near the threshold of noticing,
since it sits beside the deleted and spam fills, which carry real meaning and
must stay the loudest thing in the list.
The spine is accumulated across the visible reply rows and drawn once after the
loop. Drawn per row it left a gap at every row boundary and read as a column of
dashes rather than as the structure holding the block together.
Two bugs fixed here, both mine, both from the previous commit:
Clicking the expander did nothing. setRootIsDecorated(false), needed to stop
the style painting its own indicator under ours, also removed the style's hit
area, so the glyph rendered perfectly and was inert. ThreadListView handles the
press itself now, over the strip the delegate reserves, leaving the rest of the
subject cell to select the row.
Every click then expanded rather than toggling, because isExpanded and
setExpanded are keyed on column 0 and were being asked about the subject-column
index, which always answers false.
Visible, clickable and toggling are three separate properties and a test for
one passes against the other two being broken: the pixel test proved the
triangle was drawn while it could not be clicked, and the first click test
proved it opened while it could never close. The test now clicks twice and
asserts open then closed.
replyRowsKeepTheirTextUnderTheThreadLine covers the other trap. paintEvent runs
AFTER the cells, so the first version of the tint filled the whole reply row and
erased the sender and subject the delegate had just drawn: zero surviving text
pixels, a block of blank tinted rows. The fill and the stub stay in the band
below the text, where the tag strip lives on thread rows.
|
|
Both were reported from the running application after the previous commit
claimed them working, and the tests that passed could not see either fault.
The expander took four attempts, each of which looked right in code:
- QTreeView::drawBranches is the documented hook and does not work here. It
runs BEFORE the row's cells, so with the expander on a content column the
delegate's own background paints over it. A 60-pixel triangle survived as
8, indistinguishable from the theme's near-invisible dot.
- Sizing the glyph from the row rather than the branch rect put most of it
outside that rect.
- Moving it into SubjectDelegate but calling it from only the no-chip branch
left every real row without one, since every real row has an account chip
and takes the other branch.
It is now drawn by the delegate, which owns the cell and paints after the
background, from both branches, with setRootIsDecorated(false) so the style
does not draw its dot underneath.
The indent was 20px and invisible for a reason the geometry could not show: a
thread row draws an account chip before its subject and a reply row does not, so
a reply's text already starts about a chip's width LEFT of its thread's. The
indent has to beat that before any nesting reads at all, hence 72px.
The indent test asserted on visualRect, which was correctly indented the whole
time, and so passed against a build with no visible nesting. It now measures
where the TEXT lands, accounting for the chip, and fails at 20px. The new
expander test counts painted pixels of the glyph colour against a control row
with no replies, and fails when the call is dropped from either branch.
|
|
Replies are fetched on expansion rather than with the query: walking the reply
tree of every thread in a 10k-thread result would cost more than the query and
almost none of it would be looked at.
hasChildren is what makes that lazy loading work, and its absence would have
shipped the feature unreachable. rowCount is 0 until the worker has walked the
thread, so a view left to infer the expander from rowCount alone draws none, the
user can never expand, and the replies are never requested. It answers from the
summary's totalCount before loading and from the children afterwards, so a
thread whose count included duplicates stops offering an expander that opens
onto nothing.
onThreadTreeLoaded reads the thread id from the reply rather than remembering it
from the request. Two expansions can be in flight at once, and pairing them by
order would attach one thread's replies to the other.
|
|
The strip survived the port because every geometry call it needs exists on both
classes. What did not survive is anything keyed on a row NUMBER: a tree numbers
rows per parent, so row 0 exists once per expanded thread and the old flat 0..N
walk would paint the first thread's strip over every one of them. The walk now
goes by index, and the alternating colour follows visual position rather than
index.row() for the same reason.
QTableView::isRowSelected(int) has no QTreeView equivalent; isSelected on the
index replaces it. MainWindow loses verticalHeader and selectRow, so row height
comes from uniformRowHeights and three helpers replace the row arithmetic.
next_thread and prev_thread now resolve the containing thread first: in a tree
current.row() + 1 is the next SIBLING, which under an expanded thread is the
next reply, not the next thread.
Two test defects found by mutation and worth recording, since both produced a
green suite over a broken assertion:
The indent test asserted on column 0. A QTreeView indents only the column
holding the expander, verified against Qt 6.11: with setTreePosition(4), column
0 reports the same left edge for a thread and its reply while column 4 reports
420 against 440. It was failing against a correctly indented tree.
The strip test passed with the view's skip deleted, because the real model
already returns no pills for a child row, so the view's guard was never the
thing under test. It now runs against a stub model that hands pills to every
row, which leaves the view's skip as the only thing that can keep replies clean.
That rewrite then failed for a third reason: without the delegates MainWindow
installs, rows take the default height, the band is measured against
SubjectDelegate::rowHeightFor and overflows into the row below, and the thread's
own strip paints across the reply. Reads exactly like a missing skip and is not
one.
|
|
ActionScope is what an action is about to touch, resolved from the selection in
one place so no call site reinvents the mapping. A thread root contributes the
whole thread, a message row contributes one message, and messageCount is what
the status bar reports.
The count comes from totalCount, not from the loaded children. A thread that was
never expanded still has all of its messages, and counting only the rows that
happen to be on screen would understate what the action does: mutation-checked,
and the wrong version reports 1 message where 7 are about to be tagged.
A mixed selection is honoured as given rather than escalated to thread scope or
narrowed to message scope. Silently widening it would defeat the reason the
scope is shown at all.
|
|
setThreadMessages drops the depth-0 message: it is the thread's first message
and the root row already stands for it. Keeping it would show a thread of seven
as one root and seven children, contradicting the reply count the row
advertises. Calling again replaces rather than appends, so a thread reloaded
after a sync does not list its replies twice.
A message row reports its own sender and subject, not the thread's. That is the
mistake worth guarding: the thread's author summary usually contains the first
sender too, so reading it renders something plausible for the root's own reply
and wrong for every other one. Mutation-checked, and the wrong version returns
'Alice' where 'Bob' belongs.
Child rows carry no tag pills. The strip is a row-wide band of the thread's
tags; one under each reply would stripe the list and repeat identical tags down
the expansion.
|
|
A table cannot indent or expand, so message rows need a tree. This task changes
only the base class and the index plumbing: no children are produced yet, so the
30 pre-existing tests in test_threadlistmodel are the regression net proving a
thread row still behaves exactly as it did, and QAbstractItemModelTester checks
the index/parent round trip a hand-written assertion would miss.
Two things the table version could leave wrong and a tree cannot. columnCount
returned 0 for a valid parent, which would give message rows no columns and
render them blank. And rowCount now answers only for column 0, since a tree
takes one set of children per row and offering them under every column draws an
expander in each.
The model stays two levels deep even though replies carry a reply depth of their
own. The visual nesting past the first level comes from that depth, not from
further parent-child structure, so no index calculation has to recurse.
|
|
loadThread could not be extended to do this. It walks
notmuch_query_search_messages, and a message obtained that way returns NULL from
notmuch_message_get_replies (notmuch.h:1617-1628), so that walk cannot produce
reply depth at all. The tree comes from notmuch_thread_get_toplevel_messages
instead, and the pane keeps the flat list it wants.
walkReplies takes raw notmuch_message_t*, against this file's rule that every
handle is RAII-owned. Messages reached through a thread are freed with it
(notmuch.h:1637), so an NmMessage wrapper would destroy memory the thread frees
again. The NmThread in the caller is what keeps them alive.
Every message in the thread gets a node regardless of the query: the list is
where the reply count is read, and hiding unmatched replies would make that
count disagree with the rows under it.
Both tests mutation-checked. Flattening depth fails the depth assertion, and
skipping the thread walk fails it too, so neither passes against the two
mistakes the notmuch API invites.
|
|
A message row has to be drawn without opening the message, so it needs sender,
subject and date. MessageRef carries none of them: it exists for rendering a
thread into the pane and holds only id, path, tags and matched.
depth defaults to 0, the thread's first message, which the root row stands for
rather than a child row. threadId is carried so a batch of nodes names the
thread it belongs to without the caller tracking it alongside.
|
|
The item sat as 'open, unspecified' since 2026-08-04, recording only that the
user's mental model differed from what was built. Described now from three
screenshots plus four decisions: the left pane gains message rows, the root row
IS the thread's first message, replies indent by true reply depth, and action
scope follows the selected row kind.
No confirmation dialog, per this project's standing rule. The hazard the design
introduces is ambiguity rather than destruction, since deleted is a tag and
every mutation is invertible, so the scope is named in the status bar before and
after the action instead.
Sized L, the largest item in the backlog and the first to warrant a branch.
The plan records four API facts verified against the installed headers, each
contradicting the obvious approach: replies are unreachable from a query walk,
thread-derived messages must not be RAII-wrapped, QTreeView lacks
isRowSelected(int), and a tree numbers rows per parent.
|
|
The user asked to be able to abandon this cleanly. Task 0 now branches
`card-list` off a rebased copy rather than rebasing `item-20-message-rows` in
place, which would have destroyed the record of what the rejected presentation
looked like.
Nothing in the plan touches master, stated as a requirement rather than left to
convention, with the escape hatch written out: `git checkout master` is the
whole of the undo. Task 0 gains a step that verifies master and the original
branch are both where they started, since a rebase onto the wrong branch is
silent and every later task assumes master is the thing to fall back to.
Task 10 now ends by explicitly forbidding a merge. This design was reached by
rejecting a previous one that was finished, tested and green, so a passing suite
is not what decides it: the user looks at the cards. That is the one instruction
most likely to be lost between sessions, and it is the one that matters.
|
|
The plan left the account chip unspecified, because where it sits on a card was
never decided and inventing a place would have been a guess. The user's answer
replaces it rather than placing it: a coloured bar down the card's left edge,
the reply spines inheriting that colour, and matching swatches in the account
dropdown.
It is a net simplification. The chip ate a third of line 2 on every card to
repeat a name the user already knows, which is the table-of-records texture item
53 is about; the bar says the same thing in a few pixels and leaves line 2 to
the subject.
Three details that are easy to get wrong and are specified rather than left to
the implementer:
- The raw account colour is never drawn as a line. It is chosen to be a
chip's FILL, with text drawn on top in whatever stays legible against it. A
thin line on the pane's own background has a different job: followable down
a long expansion without competing with the senders beside it. The accent
blends toward QPalette::Base by the same 0.35 weight threadLineColour()
already uses, keeping the hue and dropping the shout. The dropdown swatch
does use the raw colour, being a filled patch rather than a line.
- A reply resolves its THREAD's colour by walking to the root.
AccountColourRole is empty on a message row, so a spine reading its own
index would fall back to the neutral line under an accented root and break
the continuous edge the design is built on.
- Reply cards carry no bar of their own. Two vertical lines a few pixels
apart in one gutter is what option B looked like, and the spine already
carries the accent.
colourFor() never failing is kept deliberately: an account with no colour= key
gets a stable colour derived from its tag name, so adding an account and
forgetting to colour it degrades to something usable rather than to nothing.
kAccentWidth ships at 3px as a starting value. Whether five accounts are
tellable apart at that width, on this user's screen and theme, is not decidable
from a mockup or a test, so Task 10 gains a step that settles it against real
cards, in both themes, with the guidance to widen the bar before touching the
user's own colour choices.
|
|
Eleven tasks against the card-list spec, TDD throughout, each with its own
mutation check.
Three decisions the plan makes that the spec did not:
- Task 0 rebases the branch first. item-20-message-rows is 15 commits behind
master and conflicts in eight files, one of which carries the item 55
splitter fix in the same file this plan rewrites heavily. Resolving that
while also building a feature would mean debugging a merge and a feature at
once, so it is its own task with its own verification step.
- CardLayout is a separate file from CardDelegate, holding all the geometry
and touching no painter or widget. That is a direct response to the
rendering-probe lessons in CLAUDE.md: a delegate test can be defeated by a
blank render, a pure function cannot.
- Tasks 2, 5 and 6 knowingly leave the build red between commits, since
removing a column across three files cannot be atomic without one enormous
commit. The plan says so at each one and warns against merging mid-plan.
Also recorded: the account chip is specified but has no task, because its
placement on a card was never specified and inventing one would be a guess.
|
|
Both items the spec left open are settled, and one of them turned out to be a
defect rather than the addition it was filed as.
Item 20 deferred "move between messages without returning to the list" as an
addition on top. It is a repair: next_thread and prev_thread are
selectRow(current.row() + 1), and a tree numbers rows per parent, so from the
last reply of an expanded thread row+1 names a sibling that does not exist and
the action silently does nothing. rowCount() with no argument counts top-level
threads, compounding it. Recorded as item 60, since it exists on the branch
whether or not the card list is built, and it is a fresh instance of the exact
rule that branch's own commit message states.
The user asked for arrow keys to skip within a thread and Shift+arrows to skip
between threads, scoped so the keys still work normally in the message pane,
menus and entry bars. Two findings changed the shape of that:
- Up/Down need no binding and no code. QTreeView's built-in navigation walks
VISIBLE rows, so it already steps into an expanded thread's replies, and
being the view's own key handling rather than a shortcut it is inert
whenever focus is elsewhere. The requested focus behaviour is automatic.
- Arrow keys must never become keymap actions. Every action is a QAction with
WindowShortcut, dispatched before the focused widget sees the key, and Qt
withholds only plain LETTERS from editable widgets. Return already proved
this by breaking the query bar and needing a per-widget ShortcutOverride
filter scoped to one widget and one key. Up as a window shortcut would break
the query bar, the tag dialog and the web view at once.
Shift+Up/Down was rejected for thread-jumping: it is the built-in
extend-selection that multi-row tagging and item 20's action scope depend on.
Alt+Up/Down instead, verified free across the whole keymap.
The way back to a whole thread after clicking a reply is the root card, which is
always visible above its replies. Escape is deliberately not overloaded, since
it already means clear-selection with clear-pane on Shift+Escape, and a third
meaning is the half-an-action problem item 50 exists to fix.
The remaining open item is only whether the message pane should change to match,
which stays out of scope: this spec is the left pane.
|
|
Item 53 recorded that message rows read as a table and left the approach
unspecified, with four directions ranging from spanning columns on reply rows to
abandoning message rows entirely. The user's answer is wider than all four: the
column grid is wrong for the WHOLE left pane, threads included.
Threads and replies both become cards in a single column, three lines each, at
one uniform height. Sender and date, then the subject with the flag, attachment
and reply-count marks inline, then the tag chips. Replies indent by depth with a
continuous spine, capped at depth 4.
Three decisions worth their reasoning, since each closed an option that looked
cheaper:
- Uniform height keeps setUniformRowHeights(true), which is the single
cheapest property of the design. A blank third line under untagged cards
buys constant sizeHint arithmetic everywhere else.
- Uncapped indent with a horizontally scrollable pane was asked for and
rejected: it reopens item 51 in a worse form. Cards are viewport width, so
the pane has no horizontal scroll range at all, and item 51 is resolved for
free rather than fought.
- A reply's line 3 shows only the tags its thread does not have. The full
per-message set was rejected on measurement, not taste: of 48691 messages in
the user's database, 7 carry unread and 75 carry flagged, and both are
already shown as the sender's weight and the mark on line 2. Everything else
is applied per thread and identical on every message in it, so full sets
would render blank on nearly every reply and identical chips on the rest.
The design is a net removal. ThreadListView::paintEvent, the tag strip's band
arithmetic, SubjectDelegate, RowStyleDelegate, the five Column enumerators and
the decorative header all go; one CardDelegate paints the whole card. That
retires the two bug classes CLAUDE.md documents for the strip, a deleted row cut
in half and every other row showing a bare stripe, both of which existed because
the strip spanned cells it did not own.
The column header was decorative, so a sort dropdown adds a feature rather than
replacing one. Two entries only, newest and oldest, passed to notmuch. Sorting
by sender or subject would have to happen in the model after results arrive,
which fights the batching that makes a 10k-thread query paint immediately.
Item 51 is marked resolved by 53 rather than left as separate work.
|
|
Both backlog entries stated a cause that measurement contradicted, and
both would have misled the next reader into fixing the wrong thing.
Item 55 blamed the thread view's size hint, computed as the sum of its
fixed column widths, and placed the fault on first run. The hint is
256px, not 886: a QTableView does not put its column sum in its size
hint, so that mechanism never applied. The trigger is the restore, in a
window narrower than the one the position was saved in.
Item 58 claimed the zoom range is unenforced and the first render
unusable. MessageView::clampZoom() has always enforced exactly the
README's 0.5 and 3.0; only the report was missing.
|
|
A splitter position is saved in pixels, so one saved in a wide window
does not fit a narrower one: QSplitter::restoreState() honours the first
pane's saved size verbatim and gives the second whatever is left. A real
1285/1252 split restored into a 1136px window left the message pane 29px
wide, a sliver of rendered mail beside a full-width thread list. The
wider the window ever was, the worse the next narrower session.
Fixed with a minimum width on the pane and setCollapsible(1, false),
which covers the restore and the equivalent drag. A restore-time repair
running from showEvent() was written first and deleted: with the floor
in place it was mutation-tested to be redundant, since the minimum width
constrains restoreState() as much as it constrains a drag.
The floor is 300px rather than a bare "visible" width, because it is
reached only when a position does not fit and should land somewhere mail
is readable; at 200 the placeholder's own text wraps every few words.
The test asserts against the pane's own minimumWidth() rather than a
repeated literal, with a guard on the floor itself so it cannot pass
against a lowered one.
Note for later work in test_mainwindow: the offscreen platform chooses
the window width itself and has been seen to choose differently between
two runs of the same binary (1181 and 779), and it ignores resize(),
setFixedWidth() and a resize of the splitter on a shown window. Any
assertion on a pane ratio, or on the second pane's pixels, is measuring
that choice rather than this code.
Backlog item 55, whose recorded cause blamed the thread view's size hint
and first-run layout. Measured, that hint is 256px, not the 886 the item
computed from the column widths, and a freshly built window splits
correctly; the entry has been corrected in place.
|
|
The documented 0.5 to 3.0 range was already enforced, by
MessageView::clampZoom(), so message_zoom = 500 rendered at 3.0 rather
than unusably. What was missing is the report: the key parses, so
nothing ever told the user that the value in their file is not the value
on screen.
Reported rather than clamped a second time. MessageView owns the bounds
and does the work; a copy of the range in Config would be free to drift
from the one that matters, so config.cpp reports against kMinZoom and
kMaxZoom directly. This is where it differs from toolbar_icon_size,
which has no widget-side enforcement to defer to.
Backlog item 58, whose recorded cause was wrong on this point and has
been corrected in place.
|
|
The repo reached v0.12.0 with fourteen tags and zero GitHub Releases,
because every release had stopped after commit, tag and push. The user
noticed and asked for the backfill, then asked that it not happen again.
Records the whole procedure rather than the missing step alone, since
nothing had written it down: changelog section, version bump, signed
tag, push to both remotes, then `gh release create` with the body taken
from that version's changelog section rather than composed fresh.
Also records that the version choice follows semver on the user-visible
surface pre-1.0, which is why 0.12.0 was minor rather than patch, and
that the stale SlackBuild version is deliberately not part of this.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
One fix: Archive and Mark all read shipped 0.12.0 sharing an icon,
which the same release's toolbar change made visible by removing the
text labels that had been disambiguating them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Item 59, reported by the user against 0.12.0. Archive and Mark all read
both used `mail-mark-read`, so with the toolbar following a desktop set
to icon-only the two buttons were indistinguishable, despite doing
different things: archive removes `inbox` from the selection, mark all
read removes `unread` from the whole view.
Introduced by item 56 in the same session. `archive` had that name from
before, when only eight actions carried icons, and item 56 assigned it
to `mark_all_read` as well without checking the table for duplicates.
`mail-archive` is also the more accurate name, since `mail-mark-read`
describes read state rather than archiving.
The test asserts the class rather than the reported pair: a hand-written
table of twenty-four names has more plausible duplicates in it, so
noTwoActionsShareAnIcon compares every action against every other. It
compares cacheKey() rather than the theme name, because two different
names resolving to the same art are equally ambiguous on screen, and it
guards on every action having an icon first, since on a theme that
resolves nothing the comparison loop would never run and pass vacuously.
Mutation-checked with a different collision.
Item 56's own probe is what let this through: it verified every name
resolves to non-null art, which is true of two names resolving to the
same art. Resolving and being distinguishable are separate properties.
The replacement was picked by rendering both at 24px and comparing the
images.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Collects the per-account sync, the cron-sync indicator fix, and the
toolbar and icon work that had been sitting unreleased since 0.11.0.
A minor bump rather than a patch: two user-visible behaviours changed.
The Flag action is now called Important, and the toolbar follows the
desktop's own button style instead of always showing text beside icons.
Both are documented under Upgrading, along with the saved-query label
that a user may want to rename to match.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Follow-up to item 56. With the toolbar now following the desktop's
button style, an "icon only" desktop makes the icon the whole control,
and this style reports PM_ToolBarIconSize as 16px, which is a small
target for a button with no text beside it.
A [general] toolbar_icon_size key, 16 to 64, defaulting to 24 rather
than to the style's own metric. Setting it to 16 restores the theme's
value.
Clamped and reported, unlike message_zoom, which documents a 0.5 to 3.0
range in the README and enforces none of it. Both ends here break the
UI that would be used to fix them: too small is an invisible icon, too
large is a toolbar taller than the window.
The unenforced message_zoom range is recorded as item 58 rather than
fixed here, since it is a separate defect that predates this change.
Also documents in the README that saved-query button labels are the key
names from the user's own [queries] section, which is why the "Flagged"
button still read that way after the action was renamed: it is a user's
query name, not a string this code owns. The sample config now shows
`Important = tag:flagged` to teach the wording the UI uses.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Items 56 and 57, done together because both touch the action registry.
56. The complaint was inconsistency, not absence: eight of twenty-four
actions had themed icons, so two adjacent entries in one menu disagreed
and the toolbar laid out an empty slot for the other sixteen. The
themeIcons table now covers them all. The fifteen names added were
probed against a live icon theme first rather than taken from the
freedesktop spec on faith, and the existing null-icon guard still lets a
theme that lacks one fall back to text.
The second half of the note asked that buttons honour the desktop's
"Icon only" setting. They could not: the hardcoded setToolButtonStyle
overrode it whatever the user had chosen. It now reads
SH_ToolButtonStyle. Dropping the call entirely was tried and rejected,
since a bare QToolBar defaults to ToolButtonIconOnly rather than to the
platform hint, which ignores the setting just as thoroughly the other
way. This is a visible change: on a desktop set to "Icon only" the
toolbar now shows icons without text.
57. "Important" over "Starred", the user's pick; the Message menu
already has "Mark &spam", so "Starred" would have needed an accelerator
from inside the word. Changed the action text, its status tip, the undo
description and the star column's tooltip, which still read "Flagged".
The tag stays `flagged`. It is wire format that neomutt, the user's
saved queries and ThreadSummary::isFlagged() all read, and following the
label through to the tag would rewrite the mail store and desynchronise
every other tool over the same Maildir. The action name stays `flag`
too, since that is the key users write in [keys].
Four tests. everyActionCarriesAnIcon names every action missing one and
guards against passing on an empty list; it reported all sixteen before
the change. theImportantActionStillWritesTheFlaggedTag asserts on the
tag the model actually received, and mutating it to `important` fails
that test plus two pre-existing held-edit tests.
Also adds the changelog entry for the cron-sync indicator fix, which the
commit that made it omitted.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Two notes added since this morning's pass, both presentation.
56: icons are inconsistent across the interface. The mechanism is not
missing, it is partial: the themeIcons table covers eight actions and
about twenty others get nothing, so adjacent menu entries disagree. The
names are not the obstacle either, probed against this desktop's theme
and every name a full set would need resolves. The second half of the
note, that buttons should honour "Icon only", is a real override: the
hardcoded setToolButtonStyle defeats the desktop's own setting.
57: rename the Flag action to Important or Starred. Label only. The
notmuch tag stays `flagged`, since it is wire format that neomutt and
the user's saved queries also read. The star glyph already ships in the
list column. Recorded that "Important" can take a free accelerator while
"Starred" collides with Mark &spam.
The first draft of 56 claimed there were no icons at all, which was
wrong: it missed the table at mainwindow.cpp:902 by grepping for the
call site rather than reading the setup. Corrected before commit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Item 54. A sync fired by the user's cron carries tag edits to the mail
store exactly as a local one does, but only onSyncFinished() cleared the
pending state, so the indicator kept reporting work that had already
shipped and the exit prompt asked to sync for it. Verified against a
real cron run: 31 changes, cleared with no manual sync.
The window cannot see an external run's exit status, and /proc/locks
carries no outcome. It does not need to: mailsync.sh already ends every
run with a "RUN END ... status=OK" banner in its log, which outlives the
process that wrote it. MailSync::lastRunOutcome() reads a bounded tail
of that file and takes the last completed marker, so no change to the
script and no optimistic guessing were needed.
Only a definite OK clears anything. A failed run, a missing or
unreadable log, and a State::Unknown lock reading all leave the count
alone: over-reporting costs a redundant sync, under-reporting costs the
user their edits.
m_editedAccounts is drained in the same place, before flushHeldEdits()
and matching the local path's ordering. Item 49 uses it to choose which
mbsync channels a run syncs, and a count that reached zero while the set
stayed full would look correct and still sync the wrong channels.
The log path comes from a new optional [sync] log key, defaulting to
where the script writes, so a test never reads the developer's own log.
Two notes on the verification, both recorded in the backlog:
- A timing probe endorsed a tail read that was not happening. The
first version of the huge-log test required the call under 100 ms
and passed with the seek deleted, because reading 10 MB is fast
either way. Replaced with an assertion on content.
- Every fixture was invented and the first batch had the wrong
timestamp format, since the script uses date -Iseconds. The tests
passed anyway, because the parser keys on the prefix and the status
token. One test now builds the banner the way the script does.
The before-flushHeldEdits ordering has no test: without a held lock the
flush is a no-op, so both orderings pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
The backlog is downstream of the user's Obsidian notes and had drifted.
Three notes had no entry; two became items and one turned out to be a
question with an answer already in the tree.
54, a defect: edits applied before a cron sync still counted as pending.
The count is cleared only in the local sync-finished handler, so a run
the window did not start left the indicator claiming work that had
already shipped, and the exit prompt asked to sync for it. Adjacent to
item 49, whose account set goes stale the same way.
55: in a window narrower than the thread view's column widths there is
no space left for the splitter's stretch factor to distribute, so the
message pane collapses to nothing on first run.
The third note asked whether a build can skip the tests. It can:
QTMAILDIR_BUILD_TESTS=OFF, which the SlackBuild already passes.
Causes verified in the code rather than copied from the notes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
The work for item 20 lives on the branch item-20-message-rows, parked and not
merged while the user explores a layout other than the table. Without these
documents on master the session-start backlog read would miss item 53 entirely
and item 20 would still read as unspecified, so the next session would either
re-ask a question already answered or start designing against a rejected design.
Docs only. No code from the branch is merged here, and the entries say so in
three places: a note above the status table, item 20's own section, and a banner
on the plan document telling a future agent not to execute it again.
File and line references were checked against master rather than copied. Item
53's citation of the column-grid cause pointed at threadlistmodel.cpp:275-283,
which on master is unrelated font handling; it now names the branch and the
function instead of a line range that would send a reader to the wrong code.
CLAUDE.md and CHANGELOG.md were deliberately NOT brought across. Both describe a
QTreeView and features master does not have, and a CLAUDE.md that misdescribes
the architecture is worse than one that is merely out of date.
|
|
printf does, and the habit transfers silently. In generated CSS it is
quietly destructive: a percentage written %% reaches the browser malformed,
and a browser drops that one declaration and renders the rest, so the pane
still paints and nothing looks broken. The 0.11.0 placeholder lost its
mask, its glow and both radial gradients this way.
Records the review lesson alongside it, which generalizes further than the
rule: a geometry probe endorsed that layout because it measured only
properties carrying no percentage. A probe that cannot see the thing that
breaks reports success forever.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
A sync ran mbsync -a regardless of what changed, so tagging mail in one
account fetched all of them. The account set was not a parameter anywhere
on the path: MailSync::start() took no arguments and the script hardcoded
-a, so nothing between a tag edit and mbsync carried which account changed.
Track which accounts have edits and pass their mbsync channels through to
the script, which now takes channel names and falls back to -a when given
none. An empty set means all accounts, per the request: a sync with nothing
pending is a fetch, and narrowing that to wherever the last edit landed
would quietly stop collecting mail everywhere else.
The channel is a new optional per-account key rather than the section key.
The two names genuinely diverge, because a QSettings section key may carry
dots that the channel does not, and mbsync treats an unknown channel as
fatal rather than skipping it, so key-as-channel would fail those accounts'
syncs outright rather than degrade. It defaults to the key, so accounts
whose two names already agree need no config change.
The edited-account set is deliberately not netted the way the pending-edit
map is: that map tracks the index, where a tag removed and re-added leaves
nothing outstanding, while this tracks the mail store, where both writes
have already renamed files that mbsync still has to propagate. It is also
snapshotted before flushHeldEdits(), which inserts into it synchronously
rather than on a queued reply, so a successful sync cannot clear accounts
whose edits it never carried.
Closes item 49.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
The empty message pane now carries the application's own identity and the
counts worth knowing, and Escape finally does what it does everywhere else.
Collects the placeholder pane, the Maildir overview, the Escape change,
and the thread-list work that had been sitting unreleased since 0.10.0.
|
|
Nothing in the UI reported database-level facts: every query gave a
thread count for that query, and nothing said how much mail there is
overall. A dialog under Help now shows messages, threads and tags from
notmuch, plus the account list from config, since notmuch does not model
accounts at all.
A separate worker call rather than a reuse of requestCounts, which counts
threads to match the row count of a query. This counts messages, which is
what a user means by "how much mail is in here". The test pins 4 messages
in 3 threads against the fixture and fails if they are ever made equal,
so routing both through one count cannot pass unnoticed.
Every field starts at -1 and renders as "unknown" when notmuch could not
answer it. Printing 0 would say the Maildir is empty, and telling someone
their mail is gone is the worst way to report an index that failed to
open.
The dialog opens showing "Counting..." rather than blocking, since
counting every message is not free on a large database. That makes two
lifetimes matter: the reply can arrive after the dialog is closed, so the
label is a QPointer, and the dialog can be closed and reopened while a
count runs, so a generation counter drops the older answer. The test
drains DeferredDelete before firing the late reply, because close()
deletes through deleteLater and without that the dangling case is never
actually reached.
|
|
Blanking the pane while leaving the row highlighted reads as half an
action, and deselecting is what Escape means nearly everywhere else. The
user asked for two actions rather than a changed one, so clear_pane keeps
its behaviour and moves to Shift+Esc; clear_selection takes Escape and
does both. Shift+Esc rather than unbound because every action carries a
default and a test enforces it.
Clearing the selection re-adopts the thread it just cleared, unless done
in exactly the right way. clearSelection() leaves currentIndex() valid,
so onSelectionChanged takes its "one or fewer rows" branch, sees a
current row whose id differs from m_currentThreadId, and calls
onThreadSelected for it. Clearing the selection before blanking lets that
run while the id still matches, so nothing reloads, and clearing current
stops a later collapse-to-one-row reaching the same row. All four
arrangements were tried; only this one passes.
The first version of the test could not distinguish any of them. It
asserted showingPlaceholder(), which passes regardless because this
fixture has no worker, so loadThread never replies and the pane is never
repainted. currentThreadId() and currentIndex() are observable without
one, and asserting those is what made the test discriminate.
|
|
ctest sets no QT_QPA_PLATFORM, so the suite's verdict depended on how it
was invoked: green for anyone exporting `offscreen` by hand, red under
ctest in the same tree. That cost a wrong diagnosis before the cause was
found, an unrelated change blamed for a failure that reproduces on a
clean checkout.
The Wayland warning blames a missing transientParent, which is
misleading. Instrumenting the test showed the popup viewport arriving as
1278x0. The zero height is why the grab returned a null pixmap, but the
width is the worse half: this test sizes a line edit to 550px and exists
to prove a description survives a popup that size, so a working grab
would have measured a popup twice that wide and passed while proving
nothing. Offscreen gives 548x40, the geometry the test means.
Pinning the platform is therefore the correct fix rather than an opt-out.
It is set in add_qtmaildir_test so it covers every test, including the
two others that create widgets, and anything added later.
The test now also asserts its own geometry, since CMake only governs
ctest and the binary is often run directly. A bare !shot.isNull() said
nothing useful; it now reports "popup viewport has no height (1278x0)"
or names a popup too wide to be the case under test. Both guards were
verified by mutation.
|
|
The notes are the upstream source and keep growing, so the backlog goes
stale on its own between sessions. This pass found three unrecorded
entries, each with its cause verified in the code rather than copied from
the note:
- 49, sync always runs every account regardless of what changed. The
account set is not a parameter anywhere on the path, and the shipped
script hardcodes the whole-store sweep. Needs a decision first about
whether an account key is also an mbsync channel name.
- 50, Esc blanks the pane but leaves the row selected. Built as specified
by item 32; the user now wants both, as two actions rather than one.
- 51, clicking a subject scrolls the list sideways. Ordinary view
auto-scroll, cosmetic.
52 came from the work itself rather than from the notes: the suite's
verdict depends on how it is invoked. test_querycompleter grabs a popup
and Wayland refuses a grabbing popup for a window that has never received
input, so it fails under ctest and passes when run with the offscreen
platform by hand. It cost a wrong diagnosis on the day it was found, an
unrelated change blamed for a failure that reproduces on a clean
checkout, because the comparison was run under two different plugins.
Recorded with the transientParent fix preferred over forcing the
platform, since a test that runs under the real plugin is worth more than
one that opts out.
|
|
An empty right pane said nothing, and multi-select made it a routine
sight. It now carries the wordmark, thread counts that run their query
when clicked, and a sync line that appears only when something needs
attention.
Rendered into the existing web view as a third document shape, so there
is one document path and one set of security rules. The brand palette is
a deliberate exception to deriving colours from the desktop theme, since
a logo is brand rather than chrome; the theme still picks which of the
two sets is used.
Counts refresh when the pane is about to show rather than in the
background: one goes stale the moment a tag is edited, and refreshing one
nobody is looking at is work for nothing. A generation counter discards a
superseded reply, and a late answer cannot repaint over an opened thread.
The helper lines are real links because JavaScript is off in this
profile. The handler is gated on the placeholder actually being
displayed, so the same URL inside a message body is dropped: a stranger's
mail must not drive the thread list, even to run a harmless query.
Three defects found while building, all silent:
- Every CSS percentage was invalid. QString::arg does not collapse "%%"
into "%", so the document carried "50%%" and the browser dropped each
declaration holding one, disabling the mask, the glow and both radial
gradients while still rendering something plausible. Substitution is by
named token now, which cannot collide with a percent sign.
- A geometry probe endorsed the layout while that was live, because it
measured only properties without percentages.
- The font test passed against a build with one face missing, since the
other satisfied both of its checks on its own.
The mockup's light values needed correcting against a real pane: the grid
vanished at a 2% luminance step on white, and the glow subtracts light
there rather than adding it, washing the pane. Strength only, not hue.
|
|
The blank-pane item was designed with the user this session but not
built, and the decisions would otherwise have to be re-derived.
The important correction is to the item's own source material: it is
built from the user's HTML mockups, not from the PNGs this section
originally recorded. Those PNGs were rendered from that HTML, and it
ships a complete brand palette for both light and dark. Two parts of
the mockup cannot survive the port, and both are deliberate properties
of this application rather than problems to fix: its Google Fonts
import is blocked by the interceptor, and its layout script cannot run
because JavaScript is off.
Also records that the selected-thread count, which this section called
the half with real value, was rejected by the user on the grounds that
the status bar already shows it. The helpers are unread, flagged and
inbox counts plus a sync line that appears only when something needs
attention.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Groundwork for item 30, the blank right pane. Nothing references these
yet; the pane itself is the next piece of work.
The pane is drawn from the user's own HTML mockup, which sets its
wordmark in Oxanium and its text in IBM Plex Sans and pulls both from
Google Fonts. That @import cannot work here: the web view's interceptor
blocks every request by default, deliberately, and neither font is
installed on the development machine either. Shipping them is what
makes the pane look the same on every machine.
Both are SIL OFL 1.1, which permits redistribution and is compatible
with the GPL, and each carries its licence text beside it. The README
records the pairing.
Both are subsets, since they are embedded into the rendered document as
data URIs and the full families would dwarf the page using them.
Oxanium is instanced to weight 800 and cut to the nine characters of
the wordmark: 43K to 1.2K. IBM Plex Sans is cut to Latin-1 plus common
punctuation rather than to the exact strings in use: 525K to 13K. The
wider cut is deliberate, because that font carries interface text that
will change, and a subset matching only today's wording would break
silently the moment a string is edited. Coverage of both was verified
against the strings they have to draw.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
A session was spent chasing a defect that did not exist, because three
separate rendering probes returned confidently wrong results and each
was believed until contradicted. The specific failure modes are worth
recording, since the next attempt will otherwise rediscover them:
counting lit pixels cannot tell bold from regular in either direction,
viewport()->render() returns blank images in several ordinary
situations, and a "saturated pixel" threshold catches the antialiased
edge of the selection highlight and passes whatever the code does.
The bug that started it was not in the code at all: the desktop's Qt
font was configured Bold in qt6ct, so every row rendered bold and
setBold() changed nothing.
Also records ThreadListView in the architecture diagram, with the three
consequences of painting a row-wide strip in a per-cell view that are
easiest to undo by accident.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
The thread list was uniform and cramped: every row one line tall, with
nothing to say what a thread was about before opening it. Rows are now
roughly double height, carrying a strip of tag chips beneath the text,
with alternating row colours and a star column for flagged threads
beside the existing paperclip.
The strip is painted by the VIEW rather than by a delegate, which is
why ThreadListView exists. A delegate is handed one cell's rectangle
and cannot paint outside its column, so a strip drawn from the subject
column stops at that column's edge, losing the last tags of a
well-tagged thread, and starts at its left edge, putting the chips
under the subject instead of under the row.
Tags the row already shows another way are left out: inbox as
structure, unread as the dimming, flagged as the star, attachment as
the paperclip, and the account as the chip in the subject cell. Sorted,
since notmuch's order is not guaranteed stable and a row whose chips
reordered between repaints would flicker.
Six defects were introduced and fixed on the way here, all of them one
consequence: a QTableView paints per cell, and a row-wide strip is not
a cell. SubjectDelegate installed view-wide drew the account chip into
every column, since AccountLabelRole belongs to the row; it is split
into RowStyleDelegate for every column and SubjectDelegate for the
subject alone, with a Q_ASSERT guarding that. Row height returned from
sizeHint did nothing, because a table takes one height per row. The
strip painted from x=0 over the marker columns, via a protected
viewportMargins() that returns 0. Measuring the text band and the strip
with one font put the pills over the date. Alternating colours and the
selection are per-cell too, so the band showed bare viewport background
until the view filled it, honouring the model's own BackgroundRole
first so a deleted row is not cut in half. And that fill spanned the
full width, cutting the centred marker glyphs at their midpoint.
Closes item 5.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Bold was unread's only cue, and it renders identically to regular on the
user's system: confirmed by eye against a bare QTableView holding a
plain QStandardItemModel, with no code from this project involved. The
fault is in Qt or fontconfig, below this application, and nothing in the
model could ever have reached it. Read and unread mail looked exactly
alike.
The emphasis is inverted instead. Unread rows keep the palette's own
text colour and read rows are dimmed toward the background, so the cue
rides on Qt::ForegroundRole, which the delegate already honours, and
costs no column. It also suits the real ratio, measured at 99 unread
against 4220 read: dimming the bulk is calmer than highlighting it. The
dim colour is derived from the palette, never hardcoded, per the rule
item 12 established. Bold is kept for systems where it works, but
nothing depends on it now.
That exposed a second defect, visible the moment it shipped. Qt resolves
ForegroundRole into the palette and then prefers it over
HighlightedText, so a model-supplied colour wins on a SELECTED row too.
The dim is blended against the unselected background, so a selected read
row painted grey on the selection colour, near unreadable.
SubjectDelegate::initStyleOption now reverses that, and the delegate is
installed view-wide rather than on the subject column alone, so every
column gets the same handling instead of three of them keeping Qt's
ordering.
The guarding tests state the property rather than the mechanism: strip
the font from the model's answer and the two states must still differ.
A test asserting only that bold is set passes on a system where bold
paints like regular, which is exactly how this survived. The selection
test renders two rows identical but for the unread tag, selects both,
and requires zero differing pixels.
Part of item 5; the density work and the star column remain.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
The stylesheet hardcoded #bbb, #555, #000, #666, #ddd and #4a6f8a, and
set no background at all, so on a dark desktop plain-text mail rendered
as black on white inside a dark window and the web view's own default
showed through.
The colours now come from a Palette struct derived from QPalette, and
are passed into the builder rather than read from qApp inside it, so
the stylesheet can be tested against a known palette with no running
application. Base and Text rather than Window and WindowText: the pane
is a content surface like a text edit, and on many themes those differ.
The secondary colours are blends of text and background, not fixed
greys. That is the part that makes it work both ways round, since a
#555 chosen to read as subtle on white is nearly invisible on #2b2b2b.
The quote colour keeps its hue, because "this is quoted" is carried by
being a different colour rather than a dimmer one, but is pulled toward
the background so it stays readable instead of glowing on dark.
A sender's own HTML is deliberately left alone, and a test asserts that
so it cannot drift: rewriting a sender's styling would break layouts
that depend on it, and a newsletter setting a white background is
entitled to stay white. This themes the plain-text render and the
chrome around messages, nothing more.
MessageView passes its own widget palette rather than the
application's, since a style sheet or a themed parent can give the pane
different colours from qApp, and re-renders on PaletteChange: the
document's colours are baked into its stylesheet at build time, so
unlike a widget it does not restyle itself when the desktop theme
changes.
The load-bearing test asserts the negative, that no hex colour appears
in the style block which the palette did not supply. A test checking
only that the palette's colours are present passes with a leftover
literal still there, and one leftover literal is the whole defect.
Confirmed by mutation.
Closes item 12.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
An action removing "unread" from every thread in the current view,
on the toolbar, the Message menu and Ctrl+Shift+U. It deliberately
ignores the selection, which makes it the one action in the window
that does, and it routes through the same funnel as every other tag
change, so it is one write rather than one per thread.
Disabled until the query reports its total. Threads arrive in batches,
so before then the model holds only what has landed, and an action
saying "all" must not silently skip the rest. A greyed control says
"not yet" without needing a dialog or a stall the user cannot see.
The state is also set at registration, since QAction starts enabled
and a window that has not run a query has nothing to act on.
Two things came out differently from the plan, both forced by existing
code. It carries a default binding, because everyActionHasAShortcut
requires every registered action to have one: an unbound action is
unreachable from the keyboard, and that invariant is deliberate, so the
action was given Ctrl+Shift+U rather than the invariant relaxed. And
only the threads that are actually unread are sent, because sending the
rest would inflate the pending-edit count with writes that change
nothing, and the quit prompt reads that count. A view with nothing
unread does nothing, pushes no command and says so: an undo entry that
restores nothing is worse than none, since it absorbs a Ctrl+Z meant
for the previous action.
undoDepthForTesting() is new and exists for a reason worth recording:
undo->isEnabled() cannot answer "was a command pushed", because the
undo QAction is always enabled and tests canUndo() when triggered. The
first version of the no-op test asserted on it and passed against a
mutant with the unread filter removed.
Closes item 43.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
"Syncing..." was set once and never updated, so a run that takes over a
minute reported nothing about what it was doing.
The original diagnosis in the backlog was half wrong, and two further
wrong ones were made and discarded before the real cause: plain
"mbsync -a" prints NOTHING until it exits, then a single summary line.
Measured on a real run, one line at 11:11:08 then 73 within the second
11:11:33, at the end of a 46-second run. So there was no stream to read
for the part of a sync that takes time. It is not buffering, so stdbuf
changes nothing, and the account name is not unavailable either, which
was the second wrong conclusion.
mbsync -V is what changes both: it announces each channel as it reaches
it, which is at once the progress and the account name originally
asked for. The shipped script now passes it.
SyncPhaseTracker derives a short status from the output as it streams:
the channel being synced, the summary counts when mbsync ends, then the
notmuch reindex. It lives beside MailSync rather than in the window so
the matching rules are one testable thing, and it holds no widget.
Matching is loose and case-insensitive, since the wording varies by
version, and nothing in it decides success or failure: the exit status
remains the only authority on that.
Lines are reassembled in MainWindow before being fed, because
QProcess::readAll() splits wherever it happens to and a half-line would
match nothing. Every status is sanitised and truncated: the channel
name comes from a config file this app does not own, and a long one
must not stretch the status bar.
Two defects in existing code, fixed with it. setSyncBusy(true) ran
after start(), so a fast run's output arrived before the per-run reset
and wiped its own phase. And a first draft deferred phases while a
transient message showed, which let a "Background sync completed"
message armed before the sync began suppress the whole run: a running
sync's state outranks an expiring event message.
Verified by replaying real captured mbsync -V output through the
tracker, not only against fixtures. The MainWindow test paces its
script with sleeps, since a script that prints everything at once
arrives in one readyRead and makes every intermediate phase
unobservable.
Closes item 42.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
