| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Opening the tagging rules dialog and pressing Save destroyed the first
rule in the list, without any editing. The rule lost its query and its
tags, then vanished entirely on the next load, since a rule with an empty
query is dropped as malformed.
Reproduced against the released tag rather than the branch, in a
throwaway worktree at 9585674 with a two-rule fixture: constructing the
dialog and running its save path left one rule of two.
onSelectionChanged blocked signals for the note widget only, while
m_enabled::toggled two lines later reached applyEditsToCurrentRule, which
writes every field from widgets the loader has not filled yet. The
existing comment there shows the hazard was known for one widget and not
extended to the other.
The fix landed with the builder work: the reloading flag now covers the
whole load, and switchingRulesDoesNotLeakRowsBetweenThem is the
regression test, verified by mutation to fail without the guard.
The live rules file had one casualty, the account rule sitting first in
the list, with both its query and its tags empty while every sibling was
intact. Restored from the shell backup that the earlier migration kept
and verified through mailctl's own reader. The rule had stopped tagging,
but only one message had arrived meanwhile; that message is now tagged
and the account is complete again at 14969 of 14969.
|
|
The spec listed from:(((( among the queries the parser must reject, and
the plan's Task 6 asserted that rejection. Probing the built parser shows
it accepts the query as a From row whose value is the literal text, and
compiles it back byte for byte.
That is correct behaviour, not a leak in the strictness rule. notmuch
reads those parens as characters to search for rather than as grouping,
so the query is meaningful and the row displaying it tells the truth.
Rejecting it would buy nothing and would push a representable rule into
text mode.
The distinction the documents were missing: a parenthesis inside a VALUE
is not a shape question at all, only a parenthesis in grouping position
is. Restate both documents accordingly, and replace the assertion with a
round-trip one, which is the property that actually matters here.
|
|
The draft compile() quoted every Is/IsNot term, which contradicted the
same task's own assertion that a negated tag compiles to .
The implementer resolved it in the direction the tests specify, and the
resolution is right: notmuch reads tag:inbox and tag:"inbox" identically,
counting 5322 either way against the live index, so quoting a tag would
change the stored string without changing what it matches. That breaks
the byte-for-byte round trip this type exists to guarantee.
Restate the comment as the rule rather than as a note about what a test
expects, correct the plan's draft so the remaining tasks do not inherit
the contradiction, and warn the parser task that a quoted tag must not be
read back as a quoting operator.
|
|
Twelve tasks against the design approved today, TDD throughout: RuleQuery
comes first as a value type with no widget dependency, tested on exact
strings, and the dialog is wired to it only once parsing and compiling
round-trip.
Two tasks carry the guarantees the design was shaped around rather than
merely testing behaviour. Task 7 round-trips every query shape present in
the live rules file and pins it with a mutation check, since a compile
that differs by one paren would rewrite a file a second tool reads. Task
6 pins whole-query rejection, because a parser that salvages the part it
understands is how a not clause goes missing and a live mail filter
silently widens.
Queries in the tests are generic placeholders. The shapes are what is
under test and they survive substitution intact.
|
|
The standing backlog reconciliation found four unrecorded entries in the
user's notes, all fallout from item 44's rules dialog now that it is in
daily use: the window forgets its geometry and column widths (75), every
field is free text (76), a rule cannot be previewed against the thread
list (77), and there is no way to build a rule from something visible in
a message (78). Each cause is verified in the code rather than copied
from the note.
Item 76 then went through a brainstorming pass and has a design. The
shape is Thunderbird's filter window, which the user supplied as the
reference: field and operator dropdowns, +/- buttons per row, an all/any
radio, and a separate "but not" block.
The structural point is that Thunderbird owns its filter format and this
project does not. The storage is a notmuch query string shared with
mailctl and executed by the post-new hook, so the builder is a view over
a string rather than a store. That decides the rest: the stored format
is untouched and this stays a single-repo change; a query the builder
cannot represent still opens, saves and runs, in a text mode every rule
carries; and the string is rewritten only when the rows actually
changed, compared against the parsed value rather than tracked with a
dirty flag, which Qt sets during programmatic population.
Measured against the seventeen rules in the live store, sixteen are flat
and one nests an or group inside an and chain, which is what the
exclusion block exists for. The parser is strict by design: it
recognises a query whole or rejects it whole, because a lenient parser
that salvages what it understands is how a not clause gets dropped and a
filter silently widens.
|
|
Item 61. An init() fixture gives every test its own empty lock table in a
QTemporaryDir, so no test observes the machine's real sync state. The
failure was never intermittent in the usual sense: 0 failures in 30 runs
with no lock held, 30 in 30 with one held. It presented as three tests
failing that never mention syncing, and cost three misdiagnoses.
The three tests that already used the seam each restored "/proc/locks"
when finished, which was itself the defect: it handed the real table to
whichever test ran next, so one test opting in re-exposed all the others.
Those restores are gone and cleanup() leaves the temporary path in place.
noTestCanSeeTheRealLockTable guards the fixture, since a silent revert
would go back to failing for reasons no assertion mentions.
Verified with the lock deliberately held: 3 failures before, 119/119
after, full suite 19/19. Mutation-checked by disabling the fixture, where
the guard fails first and a real test fails behind it.
|
|
The tagging rules moved from the shell post-new hook to a shared JSON
store both qtmaildir and mailctl read. Seventeen real rules were
converted, each keeping its shell comment as a note, and the conversion
was proved against the real index before anything was installed.
Four findings are recorded in CLAUDE.md rather than only here, because
they will outlive the item: a stored query carries no scope and the hook
parenthesises it (a disjunction would otherwise escape tag:new and match
everything); notmuch's parser rejects almost nothing, so a test asserting
a provoked query failure fails against correct code; rule counts must
count messages rather than threads; and a count request must not bump the
query generation, which would blank the message pane.
|
|
Fifteen tasks across two repositories, TDD throughout. Tasks 1-9 build
the format, the post-new hook and mailctl's read-only rules command;
10-13 add qtmaildir's reader and dialog; 14 migrates the real rules and
15 closes the backlog item.
One deviation from the spec, recorded at the top of the plan: the spec
called for a countRules worker slot, but requestCounts already exists
and counts threads. A tagging rule tags messages, so the plan adds a
general requestMessageCounts instead of a rule-specific slot.
|
|
Item 44 sat as "open, unspecified" because nothing in this application
applies rules at sync time, and the item could not be planned until it
was known whether such rules existed anywhere. They do: the notmuch
post-new hook holds hand-written `notmuch tag` lines scoped to tag:new,
carrying their reasoning in shell comments.
The design moves them to a tool-neutral JSON store that both qtmaildir
and mailctl read, with unknown fields preserved across a write by either
tool so neither owns the format. A rule carries no scope, so the same
rule serves the hook, a dry run and a future backfill.
Also in this pass:
- Item 61's cause is established, not open. It is the user's cron sync
holding the mbsync lock: 0 failures in 30 runs with no lock held, 30
in 30 with one held. The fix is item 38's existing seam applied across
the suite. The document still said "not established" and proposed a
load hypothesis that had already failed to reproduce.
- Item 74 records the first-start latency measured this session. The
delay is the notmuch index paging in from disk, 5714 ms cold against
154 ms warm for the same 4444-thread query, and is not addressable
here. What it did expose is a real defect: the status bar holds
"Searching..." for the whole walk while rows are already arriving.
|
|
Items 70 and 69, the second folded into the first as item 70's own size note
predicted it should be.
The panes drew their state marks as font glyphs: U+1F4CE for an attachment and
U+2605 for a flagged thread, each with a fallback for a font that cannot render
it. Both fell back to "*", so on such a font a flagged thread and one carrying
an attachment were indistinguishable, which is a defect the fallback introduced
rather than prevented. What a mark looks like was also the desktop's decision
rather than this application's, and the panes are exactly where it should not
be: the user asked for the toolbar and menus to keep following their icon theme
while the panes stop.
Six marks now ship in assets/icons/marks/: flagged, attachment, passed, replied
and the two expander triangles. QIcon::fromTheme still resolves every toolbar
and menu icon and was not touched.
Licensing chose the shapes. The look came from a GPL3 icon theme, and this
project is GPLv2-only, which are incompatible: GPLv2's "no further
restrictions" clause bars shipping GPL3 assets in a v2-only work. The six were
drawn fresh in the same idiom instead, with no path data copied. The idiom is
generic: solid single-path silhouettes at 16x16 with no strokes.
They are compiled in as string literals rather than loaded from a .qrc.
src/CMakeLists.txt already records why resources belong to the executable: a
qrc in the static library registers itself from a global initialiser the linker
drops. The tests link the library, so a resource-based mark would be missing
exactly where it needs asserting. assets/icons/marks/ stays the editable
source.
One asset serves both palettes. Every payload paints with fill="currentColor",
which QSvgRenderer renders black rather than resolving, so Marks::pixmap
composites the wanted colour with CompositionMode_SourceIn. A mark then takes
the card's own pen colour and follows selection and the read/unread dimming
without a second variant to keep in step.
CardLayout reserves a rect per mark and CardDelegate paints into it. The marks
were glyphs inside the subject STRING, so their width came free from the text
metrics; as icons the geometry has to know they are there or the subject runs
underneath them. The expander pill had the same trap, its triangle being a
glyph in expanderLabel(), and now reserves that width explicitly.
Item 69's part: passed and replied were words in the tag strip and are marks
beside the subject now. The message pane's header carries the flagged and
attachment marks next to the subject, per the user's decision that the right
pane needs those two and only outside the message area.
A duplicate that no test caught is worth recording. Every geometry assertion
passed while a card showed passed as BOTH an arrow and a green tag chip: the
chip filter had no reason to know a mark had appeared. It was found by
rendering real cards to an image and looking at them. isDrawnAsAMark() is now
one list consulted by both PillTagsRole and MessageOwnTagsRole, since two
copies drifting apart is how a tag ends up drawn twice on one row and not at
all on another.
Fourteen tests: nine in test_marks, four in test_cardlayout, one in
test_threadlistmodel. Mutation-checked at four points, each failing a test: the
subject ignoring the marks, the flag not indenting the subject, the pill
forgetting the triangle's width, and the recolour composite removed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Item 71. A tag edit reached the notmuch index at edit time and then sat there
until the user clicked Sync or their cron job fired, so "mark all read" updated
the view while the change itself waited, sometimes for ten minutes.
A confirmed edit now arms a debounce that runs the existing sync path. The delay
is auto_sync_delay_ms in [general], defaulting to 2000, and follows
mark_read_delay_ms exactly, including that zero and negative are not errors:
zero syncs on the next trip through the event loop, and any negative value
disables the behaviour, which is the switch for a user who wants only their cron
job.
It is armed from onTagsApplied, where a write is confirmed and the pending count
is already current, rather than where one is sent: a sync scheduled for a write
the worker went on to reject would run for nothing. A debounce rather than a
schedule, restarted by each edit, because "mark all read" confirms one write per
thread in the view and an arm-per-edit timer would be the storm of syncs the
debounce exists to prevent. Nothing is armed when no sync command is configured
or when the pending count is zero, the case where an edit was netted against its
own inverse. When the timer fires with a sync already running, local or cron, it
skips rather than queues: mbsync's own answer to a second run is to fail on it,
and the edits stay pending rather than being lost.
Also fixes a pane blanked out from under the reader, found by hand testing this
feature. onSyncFinished called runCurrentQuery() where the cron path calls
refreshCurrentQuery(), and a re-run clears the model, the undo stack and the
message pane. The stale-thread notice handles a thread that stops matching the
query and has since item 35, but a re-run left nothing for it to describe. The
two paths had no reason to differ; before this item a local sync only followed a
click on Sync, so the difference went unnoticed. Reading a message in the Unread
view, having it marked read, and watching the pane go blank two seconds later is
what surfaced it.
Its test asserts on the undo stack rather than the pane: both paths issue a
queued query test_mainwindow has no worker to answer, so the pane ends up blank
either way and an assertion on it would pass against both, while the undo stack
is cleared by one and kept by the other.
Nine tests, four in test_config and five in test_mainwindow, each
mutation-checked: removing the schedule call, honouring a negative delay,
dropping the nothing-pending guard, dropping the already-running guard, and
restoring runCurrentQuery() each fail a test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
The session's backlog reconciliation against the user's notes found nothing
unrecorded, but did find item 67's status cell still reading "open" after the
work shipped in 0.15.0 (72812c0).
The section's Approach also proposed tag:draft as the obvious drafts query,
which the implementation rejected: it counts 0 against the real database and no
draft-ish tag exists there at all, so a tag-based line would have been a
permanent zero that reads as working code. Both lines are folder-composed
instead. Recording that, since the Approach as written would otherwise send the
next reader down the path it was already measured out of.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Eight entries from the notes had no item here. Appended as 66 to 73 with
each cause verified in the code rather than copied from the note: a blank
message pane on a first click (66), missing sent and drafts counts (67),
a forwarded-subject tag (68), passed and replied as words rather than
glyphs (69), pane icons against system icons (70), no sync after a
toolbar action (71), khard/khal (72), and this document's own size (73).
Item 68 arrived as "the passed tag appears for Fwd: but not Fw:, expand
it". Measured against the real database, that correlation does not
exist: 6 messages carry the tag in total, 194 Fwd: subjects carry none,
and every tagged message has P in its Maildir flags. The tag is the
Maildir P flag translated by notmuch under maildir.synchronize_flags,
written by whichever client forwarded the message. Nothing anywhere
reads a subject line, so there is no rule to expand. The section now
carries the measurements and costs the two real options, a display-only
mark against writing the flag out to 222 messages, and stays open
pending that decision.
|
|
Adds a `sent` key to [account.*] naming that account's sent folder, and a
Sent button beside the saved queries that composes its query from every
account carrying one. An account without the key is omitted silently, as a
real account may keep no sent mail locally. With no account selected the
button spans all of them; selecting one narrows it through the existing
scope wrap rather than a second path.
Composed at run time rather than shipped as a [queries] entry. A saved query
is one fixed string: it cannot narrow to the selected account, and it goes
stale the moment an account is added or a provider renames a folder.
The design and the measurements behind it are in
docs/superpowers/specs/2026-08-11-sent-mail-design.md. Three things there are
worth repeating here.
The composed path is QUOTED, and that is load-bearing. A real provider nests
its sent folder under a bracketed parent, and "[" and "]" are Xapian syntax:
unquoted, the query parses rather than matches and returns nothing while
looking entirely plausible. Composition happens in one place so there is one
chance to get it right, and a bracketed path is pinned in a test.
Recipients are opt-in per query, which is a performance contract rather than
a preference. notmuch_message_get_header(m, "To") is not served from the
index, it reads the message file: folding every thread of a 4411-thread
inbox took 38.2 seconds against 251 ms for the 601-thread sent view. The
worker skips the walk entirely unless asked, and the refresh path carries the
same flag so a background sync cannot blank the column mid-read. Always
folding is mutation-tested: the data would be right and only the cost wrong,
which nothing else here would notice.
The messages reached through the thread are owned by it and freed with it, so
recipientsOf() holds them raw and finishes while the thread is alive, exactly
as walkReplies does. An NmMessage wrapper there is a double-free.
Sent mail is presented flat, and the pane follows. A message you sent
otherwise drags in the replies you received, so a view labelled Sent shows
conversations rather than what you sent. ThreadListModel::setFlatMode() makes
hasChildren() and ReplyCountRole answer differently and changes nothing else;
runQuery() sets it on EVERY run, so any other query restores the tree on its
way through and the flag cannot outlive the button that set it. The pane
needed its own fix for the same reason: the single-message path depends on a
field only filled when a thread is expanded, which never happens in a flat
list, so loadThread() gained matchedOnly and drops the messages that did not
match instead of rendering them as stubs.
Recipients replace the sender through the existing SendersRole rather than a
new one, so the delegate needs no branch and cannot disagree with the model
about which name a row shows. It falls back to the sender when a To header is
absent or unparseable, since a blank where a name belongs reads as a
rendering fault.
Address parsing uses GMime: a display name may contain a comma, so
"Rossi, Mario" <m@example.org>, info@example.net is two addresses and
splitting reports three. internet_address_list_parse returns NULL for an
empty string, which is a crash if unguarded.
Backlog item 63.
|
|
The backlog entry could not be planned because it turned on a fact about the
user's mail rather than a design choice. Measured against the real database: no
`sent` tag exists at all, so the XS branch is dead. Every account keeps sent
mail in a folder, and the folders disagree across three shapes, with one
account having no sent folder whatsoever. That rules out a `<maildir>/Sent`
convention and forces a per-account key.
Decisions recorded: a per-account `sent` key composed at run time rather than a
shipped [queries] entry, which cannot narrow to the selected account and goes
stale when an account is added; an account without the key omitted silently, as
`drafts` already is; all-accounts combines and one account scopes through the
existing wrap; and recipients on the card in a Sent view, since the sender is
the user on every row.
Three constraints came from probing rather than memory. A bracketed provider
path contains `[` and `]`, which are Xapian syntax, so quoting is load-bearing
and the composed query returns its real count only because each path is quoted.
notmuch has get_authors and no recipients call at any level, so the To summary
must be folded per message in the worker, under the same thread-ownership rule
as walkReplies. And GMime's internet_address_list_parse returns NULL for an
empty string, which is a crash if unguarded; it also parses a comma inside a
quoted display name correctly, so comma-splitting is out.
Size revised M, up from S: the recipients half is a new ThreadSummary field, a
worker-side per-message walk, the first GMime address parsing in this codebase,
and a card that has to know which view it is in.
The spec lives in its own file rather than inline. The backlog is past four
thousand lines and its job is to say what is open and how big, which a hundred
lines of design buries. The entry keeps the finding and the size and points at
the spec, following item 53. "Adding to this document" now states the split
rule so the next specified item does not land inline again.
Account keys and folder names in the spec are placeholders. The counts and the
folder shapes are the real measurement and are what the design turns on.
Backlog item 63.
|
|
Adds [general] date_format, a QDateTime pattern for the date a thread card
shows. Absent or empty means the system locale's short format, which is what
every other application on the desktop uses and stays the default.
The format reaches the LAYOUT, not only the painter. CardLayout::compute()
reserves the date's width from widestDateSample(), so a pattern that arrived
only at the drawText call would be elided into a rect sized for the old
format, which is the clipping the bold-font fault already produced once. It
rides on CardLayout::Input and defaults to an empty string, leaving every
existing call site unchanged. Confirmed by mutation: making the width ignore
the format fails the test.
widestDateSample() memoised its result in a static, which would have sized
every format after the first from whichever arrived first. It is a plain call
now, costing one QLocale lookup per row, the same as formatting the date.
Validation rejects only a pattern whose output is CONSTANT, found by
formatting two different instants and comparing. QDateTime::toString() treats
nearly every letter as a field, so "banana" formats as "bpmnpmnpm" and
"hello" as "22ello": nonsense, but they vary with the instant, and a check
claiming to find "no date field" cannot reject them. What harms the user is
the pattern that prints the same text on every card, and that is what is
refused, with the value named in the message.
The model supplies the pattern through DateFormatRole for the same reason it
supplies the tag colours: it is the one object here holding config, and a
delegate reading config itself would be a second source of truth.
Backlog item 62.
|
|
The Sync button used mail-receive, a mailbox glyph, which reads as "mail"
rather than "fetch again". The toolbar follows the desktop's tool button
style, so on an icon-only desktop the icon is the whole control and has to
carry the meaning by itself.
view-refresh is the standard freedesktop name for the action. The existing
noTwoActionsShareAnIcon test covers the collision risk that the 0.12.0
Archive/Mark-all-read defect came from, and passes.
Also records the backlog reconciliation this came from: items 64 and 65,
appended from the user's notes with their causes verified in code. 65 is
"full code review and optimization", which names no symptom or measurement
and is filed unspecified rather than given a design.
Backlog item 64.
|
|
The thread list now updates itself when a sync finishes, whether it is
empty or populated. New threads appear where the sort puts them, threads
that stopped matching leave, and threads whose state changed repaint.
Refreshing used to mean re-running the query, which cleared the model,
the selection, the message pane and the undo stack, so 0.8.0 declined to
do it on a cron timer and asked the user to press Enter instead. The
result was a list that quietly disagreed with the database: mail indexed
by cron never appeared, and an Unread view read to the end sat empty in
front of it.
ThreadListModel::reconcile() diffs a result against the current rows by
thread id instead, so a surviving thread keeps its row, its persistent
index and its loaded replies. Order comes from the result and is never
imposed here, which is what makes the sort dropdown authoritative.
The undo constraint this was sized around did not exist: no undo entry
was ever keyed on a row. ThreadTagCommand stores thread ids and
MessageTagCommand stores message ids, and applyTagChange() looks its
target up by id, so an entry already survived its rows leaving the view.
A thread read out of the current view now leaves the list, which is
correct and would otherwise strand the reader, so MessageView grows a
notice saying the open thread no longer matches, with a button that
re-queries it. Recovery lists the whole conversation, expands it, and
restores the message that was on screen rather than reopening at the
first one.
Ten defects were found building this, nine of them by hand testing:
- SyncMonitor::start() polls synchronously, so an idle lock file emits
stateChanged(Idle) from inside buildUi() and the first handler to
touch a widget segfaults before the window exists.
- QTreeView sets a current index when it takes focus with none set, and
current drives loading, so new mail opened itself and was marked read
without the user having looked at it. Selection is now required.
- The notice outlived what it described, both when the pane was blanked
and when another message replaced it.
- Retiring the "Background sync completed" message left the bar claiming
a sync was still running: silent means saying nothing new, not leaving
a stale claim on screen.
- A thread root sets both the thread id and the message id, so treating
the message id as the message-row case discarded it for the commonest
way to open a thread.
- A freshly queried root does not know its own first message until the
tree loads, so recovery selected nothing and left the pane blank.
- A user query mid-recovery had its result hijacked by the pending
selection.
- MessageView emitted the recovery signal with its own members, so a
direct connection handed MainWindow references that runCurrentQuery()
then cleared by blanking the pane. The ids went empty mid-slot and no
recovery ever ran. Every test passed against this, because reaching a
slot through invokeMethod copies its arguments.
A Qt signal argument is a reference until something copies it. Emitting
a member to a slot that can re-enter the emitter is a use-after-write,
and it presents as a wrong value rather than as a crash.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Two tests fail about one full run in twenty, and neither is caused by the card
list: a worktree at f72dba9, before any of that work, failed 3 of 12 runs, and
neither test was touched by it.
Recorded with what was measured and what was not. The cause is NOT established:
synthetic CPU load did not reproduce it, so the trigger is narrower than "the
machine is busy", and the entry says so rather than shipping a plausible guess
for the next reader to trust.
|
|
Three passages claimed the code was not on master, which stopped being true at
the merge. The distinction worth keeping is which branch a reference points at:
item 20's file and line references still mean `item-20-message-rows` at 029a50e,
kept unmerged as the record of the presentation the user rejected, while master
carries the card list built on top of it.
|
|
The architecture section described ThreadListView as existing to paint a strip
across columns. That was true until this change and is now the opposite of true:
it survives only for the expander hit-test. Kept as one paragraph of history,
since it explains the file's shape, but no longer stated as current behaviour.
Two traps are recorded inverted rather than deleted, because the rule survived
its own reason changing. The reply indent is still asserted on where the TEXT
lands, but where visualRect lies has flipped: it used to report an indent the
text did not have, and now reports none while the text is indented. And Q_ENUM
is documented as insufficient for a queued Q_ARG, which cost a silently dropped
sort argument.
Item 60's recorded cause was wrong and is corrected in place. It was read off
master, where the row arithmetic really is current.row() + 1; the branch had
already fixed it a commit earlier with threadRowOf(). The entry stays, with the
correction, because the reasoning was sound and the tests it demanded now exist.
Items 20, 51 and 53 are marked built on the branch rather than done. Nothing is
merged and the user has not seen it, which is the whole point of Task 10.
|
|
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.
|
|
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>
|
|
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>
|
|
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.
|
|
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>
|
|
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>
|
|
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>
|
|
Confirmed on real mail: a message whose text/html part carries a
Content-Id now renders. Item 41's status drops the caveat about the
reported message not being relocated.
Finding an affected message is harder than it looks, which is why the
search is worth recording rather than rederiving. A Content-Id on an
inline image is the common case and always worked, so a plain grep for
the header returns mostly noise. The narrow case is a Content-Id in the
same header block as a text/html Content-Type, which needs a small awk
pass over the message files. That found 96 in one inbox, all bulk
senders.
Also records that file: is not a notmuch search term, so a matched path
cannot be turned into a query directly; the Message-ID header is the
way across to an id: query.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
collectParts() filed any part with a Content-Id into inlineParts and
returned before the text/plain and text/html branches. Setting a
Content-Id on the text/html body is legal and common in bulk-sender
output, and such a message parsed with both body slots empty, so
hasHtml() was false, HtmlBuilder fell through to an empty plain body,
and the pane rendered nothing. Both halves of the report, the blank
message and "no HTML part", came from that one ordering.
A content id makes a part referenceable, not undisplayable. The two are
independent. The branch now registers the part and falls through rather
than returning, so the body still fills its slot. Registering first
keeps a part that is both the body and a cid: target reachable under
its id for any sibling referencing it.
Content-Disposition is deliberately not used as the discriminator: it
is absent far more often than it is correct, and a body part commonly
carries none. The existing attachment check remains the only test for
"not a body", and the first-one-wins isEmpty() guard still stops an
inline image displacing a real body, since an image matches neither
text branch.
Verified against a hand-written fixture whose text/html part carries a
Content-Id, asserting the body renders, the id still resolves, and the
sibling image is unaffected. Load-bearing by mutation: restoring the
early return fails the test. The user could not relocate the message
that prompted the report, so the end-to-end path is unconfirmed.
Closes item 41.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Both TagDialog fields built their completer from knownTags, the whole
database's tag list, so removing a tag offered every tag in existence
rather than the handful the selected threads actually carry.
The candidates were already in the dialog: currentTags, used until now
only to render the checkbox list. The constructor now walks two
(field, vocabulary) pairs instead of two fields sharing one list, with
knownTags for Add and currentTags.keys() for Remove. On a multi-thread
selection that is the union, not the intersection, since removing a tag
two of three threads carry is a meaningful request.
The setWidget and per-token prefix machinery is untouched: these fields
hold a comma-separated list, and QLineEdit::setCompleter is the trap
this dialog already works around. Only the candidate list changed.
Completion stays a suggestion, never a whitelist, so a tag absent from
the candidates still applies.
Tests type keys rather than using setText, which does not drive a
completer at all. Verified load-bearing by mutation: reverting the
Remove vocabulary to knownTags fails the new test.
Closes item 48.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
The remove field's completer is built from knownTags, the whole database's
tag list, because TagDialog builds both fields' completers in one loop. Only
the tags a thread already carries can be removed from it.
The dialog is already given currentTags and uses it to render the existing
tags, so the candidate list is in hand and this needs no new plumbing.
|
