| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
The dropdown was built from config, which names one subtree per account
and nothing below it, so it offered five entries and no way to say
Drafts or Sent. A rule wants to target those as often as a whole
account.
NotmuchWorker gains requestFolders/foldersReady, walking the tree from
notmuch_database_get_path() and listing every directory holding cur/.
It belongs there because the database root is notmuch's database.path
and the worker owns the only handle that can answer for it; putting the
root in config would be the second source of truth the design refuses.
From the disk rather than from the index: a folder mbsync created and
nothing has landed in yet is still a folder a rule may target, and a
list derived from indexed message paths would not offer it.
The two tests build their own fixture rather than extending the shared
one, which needs a nested folder and would otherwise move seven count
assertions in unrelated tests. Mutation-checked: flattening the walk to
non-recursive fails the listing test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Ticking "Edit as text" was a one-way trip: the only way back to the rows
was closing the dialog and reopening it. The checkbox was parented to the
builder widget and sat on the match row, and switching to text mode hides
that widget, so the toggle disappeared along with the rows it governs.
Move it to the query row, which is visible in both modes.
The existing tests all passed against this, because they drove the toggle
through setChecked and then asserted on the checked STATE. A hidden
checkbox reports its state perfectly well, so every one of those
assertions held while the widget was unreachable. The new test asks the
question that matters, whether the toggle would be on screen, and it uses
isVisibleTo since nothing is isVisible on a dialog that was never shown.
Worth recording how close the mutation check came to endorsing this too.
Reparenting the checkbox alone left it in the query row's layout, so it
stayed visible and the test still passed. Only restoring the full shipped
shape, parent and layout together, reproduced the fault and failed the
test. A mutation that does not reproduce the original bug proves nothing
about the test that is meant to catch it.
The spec's layout sketch carried the same error and is corrected, with
the reason, so the next reader does not reintroduce it.
|
|
|
|
Leaving text mode with a query the builder cannot represent has to
refuse, since there are no rows that mean that query. It announced this
with a QMessageBox, which made the branch untestable: a modal blocks the
test that reaches it, so the one path that can strand a user was the one
path shipping unverified.
Say it in the warning label the dialog already has instead. That also
suits the moment better, since it does not interrupt someone mid-edit to
tell them something the label can hold while they keep typing, and it
matches how the tag dialog reports a bad tag.
Returning to the rows now calls showWarnings(), because the refusal
writes into the same label the load warnings use and a stale complaint
would otherwise outlive the query that caused it.
The test drives the refusal and the recovery, and asserts the warning
appears and then clears. Verified by mutation: letting the checkbox clear
regardless fails it.
warningTextForTest uses isVisibleTo rather than isVisible. Every child of
a dialog that was never shown reports isVisible() false, so the seam
would have reported no warning whatever the label held, which is a probe
that cannot see the thing it checks.
|
|
|
|
Selecting a rule now parses its stored query and rebuilds the builder
rows from it, and a row edit compiles back onto the query line and into
the working copy.
Populating the form was already able to write the rule just loaded over
whichever rule is current: m_enabled's toggled runs
applyEditsToCurrentRule while m_query still holds the previous rule's
text, which emptied the first rule's query on open. The existing
m_reloading guard now covers the whole load rather than one signal
blocker on the note, which also covers the combo boxes rebuildRows
populates.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
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.
|
|
requestCounts counts threads, which is right for the placeholder pane.
A tagging rule tags messages, so a dry run over rules needs the message
count or it understates every rule that matches part of a thread.
|
|
The same ~/.config/mailrules/rules.json mailctl reads, parsed here with
QJsonDocument and written atomically with QSaveFile. Fields this version
does not understand round-trip untouched, which is what keeps the format
neutral between the two tools.
Mutation-checked: removing the unknown-field write fails
unknownFieldsSurviveASave.
|
|
Items 70 and 69, the second folded into the first as item 70's own size note
predicted it should be.
The panes drew their state marks as font glyphs: U+1F4CE for an attachment and
U+2605 for a flagged thread, each with a fallback for a font that cannot render
it. Both fell back to "*", so on such a font a flagged thread and one carrying
an attachment were indistinguishable, which is a defect the fallback introduced
rather than prevented. What a mark looks like was also the desktop's decision
rather than this application's, and the panes are exactly where it should not
be: the user asked for the toolbar and menus to keep following their icon theme
while the panes stop.
Six marks now ship in assets/icons/marks/: flagged, attachment, passed, replied
and the two expander triangles. QIcon::fromTheme still resolves every toolbar
and menu icon and was not touched.
Licensing chose the shapes. The look came from a GPL3 icon theme, and this
project is GPLv2-only, which are incompatible: GPLv2's "no further
restrictions" clause bars shipping GPL3 assets in a v2-only work. The six were
drawn fresh in the same idiom instead, with no path data copied. The idiom is
generic: solid single-path silhouettes at 16x16 with no strokes.
They are compiled in as string literals rather than loaded from a .qrc.
src/CMakeLists.txt already records why resources belong to the executable: a
qrc in the static library registers itself from a global initialiser the linker
drops. The tests link the library, so a resource-based mark would be missing
exactly where it needs asserting. assets/icons/marks/ stays the editable
source.
One asset serves both palettes. Every payload paints with fill="currentColor",
which QSvgRenderer renders black rather than resolving, so Marks::pixmap
composites the wanted colour with CompositionMode_SourceIn. A mark then takes
the card's own pen colour and follows selection and the read/unread dimming
without a second variant to keep in step.
CardLayout reserves a rect per mark and CardDelegate paints into it. The marks
were glyphs inside the subject STRING, so their width came free from the text
metrics; as icons the geometry has to know they are there or the subject runs
underneath them. The expander pill had the same trap, its triangle being a
glyph in expanderLabel(), and now reserves that width explicitly.
Item 69's part: passed and replied were words in the tag strip and are marks
beside the subject now. The message pane's header carries the flagged and
attachment marks next to the subject, per the user's decision that the right
pane needs those two and only outside the message area.
A duplicate that no test caught is worth recording. Every geometry assertion
passed while a card showed passed as BOTH an arrow and a green tag chip: the
chip filter had no reason to know a mark had appeared. It was found by
rendering real cards to an image and looking at them. isDrawnAsAMark() is now
one list consulted by both PillTagsRole and MessageOwnTagsRole, since two
copies drifting apart is how a tag ends up drawn twice on one row and not at
all on another.
Fourteen tests: nine in test_marks, four in test_cardlayout, one in
test_threadlistmodel. Mutation-checked at four points, each failing a test: the
subject ignoring the marks, the flag not indenting the subject, the pill
forgetting the triangle's width, and the recolour composite removed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Item 71. A tag edit reached the notmuch index at edit time and then sat there
until the user clicked Sync or their cron job fired, so "mark all read" updated
the view while the change itself waited, sometimes for ten minutes.
A confirmed edit now arms a debounce that runs the existing sync path. The delay
is auto_sync_delay_ms in [general], defaulting to 2000, and follows
mark_read_delay_ms exactly, including that zero and negative are not errors:
zero syncs on the next trip through the event loop, and any negative value
disables the behaviour, which is the switch for a user who wants only their cron
job.
It is armed from onTagsApplied, where a write is confirmed and the pending count
is already current, rather than where one is sent: a sync scheduled for a write
the worker went on to reject would run for nothing. A debounce rather than a
schedule, restarted by each edit, because "mark all read" confirms one write per
thread in the view and an arm-per-edit timer would be the storm of syncs the
debounce exists to prevent. Nothing is armed when no sync command is configured
or when the pending count is zero, the case where an edit was netted against its
own inverse. When the timer fires with a sync already running, local or cron, it
skips rather than queues: mbsync's own answer to a second run is to fail on it,
and the edits stay pending rather than being lost.
Also fixes a pane blanked out from under the reader, found by hand testing this
feature. onSyncFinished called runCurrentQuery() where the cron path calls
refreshCurrentQuery(), and a re-run clears the model, the undo stack and the
message pane. The stale-thread notice handles a thread that stops matching the
query and has since item 35, but a re-run left nothing for it to describe. The
two paths had no reason to differ; before this item a local sync only followed a
click on Sync, so the difference went unnoticed. Reading a message in the Unread
view, having it marked read, and watching the pane go blank two seconds later is
what surfaced it.
Its test asserts on the undo stack rather than the pane: both paths issue a
queued query test_mainwindow has no worker to answer, so the pane ends up blank
either way and an assertion on it would pass against both, while the undo stack
is cleared by one and kept by the other.
Nine tests, four in test_config and five in test_mainwindow, each
mutation-checked: removing the schedule call, honouring a negative delay,
dropping the nothing-pending guard, dropping the already-running guard, and
restoring runCurrentQuery() each fail a test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Item 67. The pane counted unread, flagged and inbox from three fixed
tag: queries. Sent and drafts cannot join that list as tags: tag:draft
counts 0 against a real database and no draft-ish tag exists in it at
all, so a tag-based line would be a permanent zero that reads as working
code. Both are composed from each account's folder keys instead, the
same way the Sent view already composes its query.
The drafts key was parsed and documented as unused in v1. Composing
drafts is still v2; counting them is not, so Account::draftsQuery() and
Config::allDraftsQuery() now mirror the sent pair. The shared body moved
into folderQuery() and joinAccountQueries(), so the load-bearing quoting
(a provider nests both folders under a bracketed parent, and [ and ] are
Xapian syntax) and the bare-"or" guard exist once rather than once per
folder type.
The fixed array is gone rather than extended. It held queries and labels
in two lists indexed in parallel, which is a hazard that grows with the
list: an entry inserted in one and not the other prints a real number
against the wrong name and looks entirely plausible. placeholderLines()
carries each query beside the callable that labels it, so the two cannot
drift, and the count reply stays paired by position as the worker
requires.
A line is omitted when no account configures that folder rather than
shown as 0, following item 63: a missing folder is a real configuration,
and "0 sent" claims the user has sent nothing.
Measured against the real config: 4 sent terms over 601 threads, 5
drafts terms over 3, the extra drafts term coming from the one account
that configures drafts and no sent, which is what proves the two are
collected independently.
Four tests here and four in test_config, mutation-checked at three
points: dropping the drafts line, an off-by-one in the label pairing,
and removing the -1 guard for an uncountable query. Each mutation fails
a test.
|
|
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.
|
|
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 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>
|
|
The five real account colours are all mid-tone, because they were chosen as
CHIP fills with legible text on top, and three pixels of a mid-tone colour
barely registers beside a card's own background. The bar now lifts saturation
and lightness to a floor.
A floor, not a repaint: a colour already past it is returned untouched, so a
deliberately vivid choice is preserved and only the muted ones move. Hue is
never altered, because hue is the entire information the bar carries and a
shifted one would stop matching the account's chip and its dropdown swatch.
0.65 and 0.50 were chosen by rendering all five accounts as 3px bars against
both a dark and a light card background and looking at them. Higher pushed the
weakest of them, a green at S 0.33, toward a neon that no longer matched its own
chip; lower left it where it started.
accentLineColour had no test at all until now, which is how two earlier versions
of it shipped wrong: one blended toward Base until it WAS the background, the
other passed a chip colour through unchanged. test_carddelegate covers the lift,
the floor's idempotence, hue preservation across all five accounts, and the
fallback for a thread with no account tag.
|
|
A reply in a thread with no usable In-Reply-To carries depth 0, because that is
how notmuch reports every message of such a thread. CardLayout read depth 0 as
"not nested", so those replies drew flush against their own thread with no
spine, while a nested thread's replies indented normally: the list showed two
different shapes for the same relationship, side by side.
A MESSAGE row is nested at least one level whatever depth it reports. Being a
child row IS the nesting; the depth only says how much further to go.
This is the third fault from the same root. The depth numbering was trusted to
mean structure when it only ever meant "how notmuch happened to thread this":
first it hid a flat thread's replies entirely, then it left the first message
unreachable, and now it drew the survivors without their indent.
|
|
Four faults from the first hand test, two of them behavioural.
An expander that opened onto nothing. setThreadMessages kept only nodes with
depth > 0, and notmuch_thread_get_toplevel_messages returns every message at
depth 0 when a thread carries no usable In-Reply-To, so a flat thread
contributed no children while its card still advertised the count. Measured in
the user's database: of 396 inbox threads three are flat, one of them nine
messages long, and every two-message thread of that kind was affected, which is
exactly why the fault looked like "the expander only works with more than one
reply". The rule is now position, not depth: every message except the first,
which is the root card itself. That is also the correct rule rather than a
workaround, since the row under the root is the second message however notmuch
chose to nest it.
The thread's first message was unreachable. Selecting a root card loaded the
whole thread, so the pane showed every message with only the last expanded, and
no row in the list offered the first one: the reply rows are messages two
onward. The root card now renders its own message, which is what the card
already claims to be. It keeps its thread id, unlike the message-row path, so
mark-read and the tag-change repaint still work; that is asserted, because
clearing it is the obvious way to write this and silently disables both. Before
the replies are loaded the model has no first message to name and the whole
thread stays the honest answer.
Dates ignored the locale. One hardcoded "yyyy-MM-dd hh:mm" produced a US-looking
format on an Italian desktop; QLocale::system() now formats it, and the width
reserved for the date comes from the same function so a longer locale cannot
clip.
The expander was a bare number on the card's own background. It is a pill now,
carrying "3 replies" (and "1 reply", singular), sized from the label actually
drawn and measured in both glyph states so it does not resize under the pointer
on click. Its fill is blended from Text toward Base rather than taken from
QPalette::Button, which is #2b2b2b against a Base of #2b2b2b on the user's
theme: byte identical, so the pill was invisible. A theme may make any two roles
equal; a blend is defined against the surface it sits on and cannot collide with
it. Checked by rendering both a dark and a light palette and looking.
|
|
Both found by rendering real cards to an image and looking at them, not by any
assertion. The suite was green through both.
The date lost the leading digit of its year on every UNREAD card. The layout
reserves the date's width from the font it is handed, which is the view's
regular font, while the delegate paints with the bold one the model supplies for
unread: 154px reserved against 170px needed. CardLayout now measures the date
bold whatever font it is given, so the reserved width cannot be narrower than
what is drawn. A few pixels are wasted on a read card, which is the cheap side of
the trade.
The accent bar was painted correctly and was invisible. Blending the account
colour 0.35 toward the palette's Base, as the plan specified, is a fraction OF
THE ACCOUNT COLOUR, so on a dark theme it produced (0.18, 0.22, 0.26) against a
Base of (0.169, 0.169, 0.169): the background. The blend is dropped entirely.
An account colour is already chosen to be a chip's fill carrying legible text,
so it is muted to begin with, and nothing is drawn on the bar that needs that
contrast. The spine keeps a blend, at 0.55, because it runs the full height of
every reply in an expansion and is a different problem from a 3px edge marker.
The bar is still faint at 3px on a dark theme, since the account colours are
chosen as chip fills. Whether kAccentWidth needs raising cannot be settled
without the user's own accounts, screen and theme; that is Task 10's open
question and it is left open.
|
|
Two entries, straight to notmuch. This adds a feature rather than replacing one:
the column header was decorative and nothing implemented click-to-sort, so
removing the header with the grid lost nothing.
Stored in uistate.conf, never in the hand-edited config, and range-guarded on
read: a stale file can hold anything, which is the lesson item 58 recorded.
SortOrder needed qRegisterMetaType despite carrying Q_ENUM. Q_ENUM gives the
enum a meta-object entry, not a metatype registered under the name invokeMethod
resolves, so the queued runQuery would have dropped its sort argument at runtime
and every query would have silently run newest-first. Nothing in the suite
exercises a real worker thread, so this was asserted directly rather than left
to a warning nobody would see. It is registered beside the type rather than in
MainWindow's constructor: a first attempt put it there and passed only because
the test that catches it never constructs a MainWindow.
The account dropdown's entries now carry their account's colour as a swatch,
which is what makes the accent bar on a card mean anything: a colour down a
card's edge says nothing until something maps it to a name. Raw colour here
rather than the blended line colour, since a swatch is a filled patch like a
chip rather than a thin line. Its test builds its own two-account config: reading
the environment's made it SKIP wherever no accounts are configured, which is a
test that asserts nothing while reporting success.
|
|
next_thread and prev_thread now walk with indexBelow/indexAbove, skipping
message rows, so they keep meaning thread-to-thread whatever is expanded.
Stepping message-to-message needs no code: QTreeView's own Up/Down walk VISIBLE
rows and already enter an expanded thread, and being the view's key handling
rather than a shortcut they stay inert when the message pane, a menu or an entry
bar has focus.
Item 60 turns out to have been fixed already, in 5487d58 on this branch, by
threadRowOf() walking up to the containing thread before doing the arithmetic.
The backlog entry was written against master, where that helper does not exist,
so it described a defect this branch had resolved a commit earlier. Verified by
writing both failing tests first and watching them pass: from the last reply of
an expanded thread, and from a thread root with its replies showing. They are
kept, because the property they assert is the one this change must not lose.
What the rewrite buys is that nothing is keyed on a row number any more, which
is the rule a deeper tree would break next.
Alt+Up/Down added alongside Ctrl+J/K. That required KeyMap::sequencesFor and a
move from setShortcut to setShortcuts, because the singular setter keeps only
the last binding and the second one was silently unreachable. Alt because
Shift+arrows is the built-in extend-selection that multi-row tagging depends on,
and because a bare arrow cannot be a window shortcut without breaking every text
field in the window, as Return already demonstrated. sequencesFor puts
sequenceFor's own choice first so the menus advertise an unchanged binding, and
sorts the tail, since QHash order is unspecified.
|
|
ThreadListView::paintEvent and its band arithmetic are deleted. The view existed
to paint a strip across five columns; with one column and one delegate painting
the whole card there is nothing to span, and the two faults that arithmetic kept
producing go with it: a deleted row cut in half, and every other row showing a
bare stripe.
What survives is the expander hit-test, because a delegate gets no click of its
own without an editor. It now asks CardDelegate for the rect rather than
recomputing it, so the drawn target and the clickable one cannot drift. The
siblingAtColumn(0) dance is gone: with one column, the index already is column 0.
Item 51 closes here rather than separately. A card is exactly viewport width, so
the view has no horizontal scroll range for a click to scroll into, and the test
asserts that directly.
Two rendering tests had to change how they measure, not merely which index they
name. The indent test asserted on visualRect, which now reports the SAME rect
for a thread and its reply by design, since setIndentation(0) leaves the indent
to CardLayout: it reads contentLeft off the layout instead. And the expander
test reported zero ink over a card the delegate paints 2183 pixels into, because
viewport()->render() returned a blank image, exactly as CLAUDE.md warns; it now
paints the delegate into an image directly and carries a guard proving the probe
can see ink before it reports finding none. Both were mutation-checked.
Two tests are deleted rather than ported. Both existed to prove the row-wide
strip spanned columns a delegate could not reach, which is a property of code
that no longer exists.
|
|
Five columns answered through Qt::DisplayRole; one column cannot, and a card
needs every field at once, so each gets its own role. Qt::DisplayRole keeps
answering the subject, which is what keyboard search and accessibility read.
Three things change shape rather than moving. DateRole hands over the QDateTime
itself, since the card decides how much of a date it has room for and a
pre-formatted string takes that decision away from the delegate. The subject
loses its "(3)" message-count suffix, which the reply count on line 2 now
states. And the two per-column tooltips become one card-wide tooltip, because
the marks no longer have columns of their own to hover.
The build is red at this commit: the view and the delegates still name the
deleted Column enumerators and are rewritten in the commits that follow.
|
|
Split from the delegate deliberately. A delegate needs a live painter and an
exposed view, which is what makes delegate tests fragile: viewport()->render()
returns a blank image in several ordinary situations, and a probe reporting no
ink is likelier broken than the code it tests. Every geometric claim about a
card is made here, where a test is a function call.
Three lines at a uniform height, so setUniformRowHeights(true) survives. Indent
caps at depth 4 with qMin rather than a branch, so depth 5 and depth 50 land in
the same place. The date is measured before the sender, so a long sender elides
instead of painting over it.
Two traps handled that a first pass gets wrong. QRect::right() is inclusive, so
the right edge is carried as an exclusive one and everything sized from it lands
where the padding constant says rather than a pixel short. And QFont::pointSizeF
returns -1 for a font set in pixels, which qt6ct does, so smallFont branches on
which unit the font actually carries instead of silently returning the card's
own size.
|
|
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.
|
|
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.
|
|
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.
|
|
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>
|
