| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Item 171. A forward carried only the plain-text version of the original,
so formatting was lost; and an original with no plain-text part at all
(30 of 342 sampled inbox messages, ~9%) forwarded as an empty quote with
its content silently gone.
A forward now sends ONE part chosen by the Send-as-HTML toggle: the
original's markup when on, the text quote when off. Not a
multipart/alternative, at the user's decision: a forward's shape is
already decided by that toggle, and sending both hands the choice to the
recipient's client. The toggle is honoured even for an HTML-only
original, which then forwards as a text fallback.
HtmlSanitiser strips remote content from the forwarded markup, checked
by default with a per-forward opt-out. This is the security-critical
part: the markup leaves this process and is rendered by the recipient's
client, where none of MessageView's protections apply, so forwarding a
tracking pixel forwards the tracking. It is an ALLOW-LIST, unlike
HtmlBuilder::namespaceCids(), because a missed rewrite is a broken image
while a missed strip is a beacon reaching the recipient.
An HTML forward does not seed a text quote into the editor. The first
build did, then subtracted it when building the HTML part, so the user
could edit a quote whose edits were discarded; what the composer shows
must be what gets sent. The forwarded message appears in a read-only
pane beside the editor instead, a QSplitter at 60/40 with a toggle in
the Format menu. A plain forward is unchanged.
ComposeContextBuilder::quoteBody() renders htmlBody down to text when
there is no plain part, so the plain path never emits an empty quote.
Design in docs/superpowers/specs/2026-08-27-forward-html-design.md.
Two tests repaired for the splitter: the 60/40 assertion reads stretch
factors rather than pixels, since the offscreen platform gives the
splitter no width and reports 49/49 whatever the code asks; and
theComposerSplitsItsToolbarByScope looked for the body directly in the
composer's column.
Not yet hand-tested in this arrangement.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AtUzfNjMD8fiYfamDd3ywW
|
|
DraftStore::write() was called with "D", and it uses the flag string
verbatim, so every draft this application wrote landed as :2,D. With
maildir.synchronize_flags on, notmuch tags any message lacking the S
flag `unread`, and a draft the user authored is seen by definition.
The symptom heals itself: the next sync of that folder round-trips the
file, adds S, and the tag goes away. Only the newest draft in a folder
that has not synced since shows it, which is why it read as
intermittent and why measuring an older draft finds nothing wrong.
TestComposeWindow::aSavedDraftIsFlaggedSeen() asserts both flags on the
written filename, verified failing first against "D".
TestMainWindow::anAutosaveWritesADraftAndClearsTheDirtyFlag() asserted
endsWith(":2,D"), pinning the whole flag set where its own comment said
the point was the draft flag "not left bare", so it failed against the
corrected behaviour. It checks for D within the flag set now.
Also reconciles the backlog with the user's notes: records the
forwarded-HTML defect as item 171, closes item 169 (shipped last
session, its row still read open and its section was still in the open
file), and records this fix as item 172.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AtUzfNjMD8fiYfamDd3ywW
|
|
380px of dialog for three rows left most of itself empty. The height is
asked of the layout now, capped so a long list scrolls rather than growing
past the screen and floored so a single row does not collapse it.
A background role on the scroll viewport was tried in the same pass and
reverted: the dialog renders semi-transparent under the developer's
compositor, and painting a Base-coloured layer under the list made that
worse rather than better. The transparency is the desktop's own doing, which
is the trap this project has already recorded for window geometry.
Not covered by a test. The offscreen platform returns an identical frame for
a correct size and a broken one, so an assertion there would pass against
both; CLAUDE.md records that measurement. Confirmed by hand instead.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P88Q3MCSCSQxKDy7pmXh9F
|
|
Item 119, and item 146 which is the same request recorded again. The status
bar's count answers "is my work safe to quit on" and could not say what the
work was.
The label opens a read-only list on a click. A QLabel has no clicked signal,
so the press is taken by MainWindow's existing event filter rather than by
replacing the label with a flat QToolButton, which would have brought the
style's button metrics into a status bar the label already sits correctly
in. The pointing-hand cursor is the affordance, since a status-bar label has
room for nothing else.
The layout is the user's own: a message appears once with its actions
beneath it. PendingChangesDialog::rowsFor() does the grouping over a run of
rows sharing an id, which the snapshot has already ordered, so the actions
under one message keep the order they were made in.
Read-only, deliberately. Retrying or discarding a change from here would be
a new mutation path with its own undo question, and the count exists to be
understood rather than edited.
Three rules the tests pin, each of which is a way the list could disagree
with the count it was opened from:
- Grouping must not collapse: two actions on one message are two rows.
- A thread row stays thread-scoped and reports how many messages it covered.
- An id the index no longer holds still opens a run of its own, showing that
its subject is unknown rather than folding its actions under the message
above it. This is why the row carries startsMessage rather than inferring
it from a non-empty subject.
The queued call carrying QStringList, QList<bool> and QList<int> is covered
by a test that drives it across a real thread, since a container whose
metatype does not resolve is dropped at runtime and the slot runs with a
default. Both survive on Qt 6.11; the test is what says so, and what would
fail if that changed.
Italian ships with it: five new strings, lupdate clean, lrelease 522
finished and 0 unfinished.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P88Q3MCSCSQxKDy7pmXh9F
|
|
Item 119, second half of the data: the step that turns the snapshot's ids
into something worth showing.
resolvePendingSubjects() takes the rows' ids in order, each flagged as a
thread id or a message id, and answers positionally: one subject per input,
plus the thread's message total for a thread id and -1 for a message id.
Positional rather than set-based, and that is load-bearing. The caller has
already decided what its rows are and in what order, and one id can
legitimately appear on several rows: a message with two outstanding actions
is two rows carrying one id. A combined query returns a set, which loses
both the order and the duplicate, so the walk is one lookup per row instead.
The cost is bounded by what the user did by hand since the last sync, which
is not a query-sized number.
A missing id answers with an EMPTY subject rather than being dropped. The
dialog still shows that row, because the count the user clicked has to equal
the list they are shown, and dropping a row breaks that agreement in exactly
the case where the user is most likely to notice. An index that cannot be
opened answers the same way, one empty subject per row, so the list still
shows the changes with only the subjects missing.
The thread count is taken at snapshot time and says so: a held thread edit
applies when the sync ends, and a reply arriving in between makes the real
number larger. The row describes what the user is looking at, not what the
write will touch.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P88Q3MCSCSQxKDy7pmXh9F
|
|
Item 119, first half: the data the list behind the unsynced-changes count
is built from, with no dialog and no worker, so the rules it has to follow
are testable on their own.
pendingChangeSnapshot() gathers the three queues the count sums into
PendingChange rows. Two properties are the whole point.
Scope follows the ACTION, not the storage. A held thread edit stays one
thread row, because a `*_thread` action made it and reporting its messages
instead would claim the user acted on each one; a netted tag edit and a held
move are message rows. The queues already encode that distinction, so
nothing is expanded and nothing is escalated.
The rows are grouped by id, so a message with several outstanding actions
appears once with its actions beneath it, which is the layout the user
asked for. The sort is stable, so those actions keep the order they were
made in; QHash has none of its own, and without it the list would reshuffle
between openings.
A snapshot, taken once and frozen. Subjects are empty here and filled by the
resolve step to come.
m_pendingTagEdits gains the action name beside the direction it already
kept. The direction alone was enough to count with; a list has to say what
each change was, and only the action that made it knows. It is carried from
TagChange::description rather than derived from the tag, so there is no
second table of tag names to labels to drift from the first.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P88Q3MCSCSQxKDy7pmXh9F
|
|
Item 119's stated blocker, removed by finding out what it held: nothing.
pendingEditCount() summed four sources, three of which can name the messages
they hold and one of which was a bare int. That int counted confirmed
changes carrying no message ids, on the reasoning that an edit which cannot
be netted must still register rather than be lost. It was what made the
count impossible to open and list, since a dialog would have shown three
groups and then owed the user a remainder it could not describe.
The remainder is empty. NotmuchWorker::applyTags() is the only emitter of
tagsApplied(), and its first statement returns on an empty id list, which is
the exact condition the counter required. applyTagsToThreads() resolves
threads to message ids through a query and errors out when that comes back
empty, so it can only ever hand applyTags() a non-empty list.
Measured rather than read. A qFatal in the branch fired in 4 of 70
test_mainwindow cases, all four building a TagChange by hand and invoking
the slot directly with no worker involved; an assertion before the worker's
own emit never fired across the whole suite, worker-backed tests included.
The worker's guard stays and is pinned where it lives, by
applyTagsWithNoIdsDoesNothing() in test_notmuchworker. The MainWindow test
that asserted the deleted branch is replaced by one for the consequence: a
change reaching the indicator names its messages, and an edit with its
inverse nets back to nothing, which is the property a growing-only counter
could never have. Three tests that leaned on the counter to show the
indicator now carry message ids, as a real edit always does.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P88Q3MCSCSQxKDy7pmXh9F
|
|
Restore moved the file back to the inbox folder and left it invisible: the
message carried no `inbox` tag, so the Inbox view could not see it, and the
user reported restoring a message and losing it.
Delete strips `inbox` so a deleted message leaves that view, which makes
restoring it the other half of the same change. restoreResolvedMessages()
already meant to add the tag back, and the comment above the branch
describes exactly this failure, but the comparison deciding it read
`origin`, which four lines earlier had been reassigned from the bare folder
name to the finished tag. `deleted-from:Inbox` never equals `Inbox` however
an account spells its inbox, so the branch was dead and the tag never came
back.
The destination folder is taken from the move's own key instead, which is
what the surrounding code already builds and what the comment says is being
compared.
The existing test passed against this throughout. It asserted the file
moved, the origin tag came off and `deleted` came off, all of which were
true; nothing asserted the tag that decides whether the user can see the
message afterwards. It does now, and fails against the old comparison.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P88Q3MCSCSQxKDy7pmXh9F
|
|
Hand-testing item 169 found four defects, three of them visible on every
card.
The initials were taken from whatever the card's first line held, which is
the raw From header on a reply row and notmuch's comma-joined author
summary on a thread row. A naive space split therefore gave `T<` for
`tsujan <notifications@github.com>` and one letter each from two different
people for `Standreas, tsujan`, and a separator counted as a word, so
`INE - Expert IT Training` drew `I-`. Avatar::initialsFor() now normalises
first: the angle-addr and any quoting go, a comma takes the first entry
unless the name is quoted, a bare address is not a name, and a word has to
carry a letter or a digit. Avatar::fillFor() uses the same normalisation,
so an address in the name's place no longer reads as a person.
The two-tone fill built its gradient axis as a radius from the centre, so
the 0.5 colour stop landed on the squircle's edge and one hue filled almost
the whole face. The axis spans the diameter now.
The fade ran left to right, which put its hard stop at 60% of the card and
read as a slab rather than a wash. It runs right to left: opaque at the
card's right edge, where the only hard stop is the card's own boundary, and
gone before it reaches the accent bar that already states the account.
And the flat views hashed the user's own address on every row, so every
Sent and Drafts card shared one pattern. ThreadSummary::firstMessageRecipient
rides the recipient fold, which already parses the To header, and
SenderAddressRole prefers it, falling back to the sender when there is no
usable To.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P88Q3MCSCSQxKDy7pmXh9F
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
Item 68, which turned out to be three things once its premise was
measured. The note asked to extend a "passed" subject rule to "Fw:";
there was no subject rule, and the correlation it rested on did not
exist. What did exist was a gap nobody had reported.
Reply and forward now flag their source. The Maildir R and P flags,
which every other client sets and notmuch reads back as "replied" and
"passed", had never been written here: measured on the developer's
index, all 317 "replied" and all 6 "passed" came from other clients.
ComposeWindow emits sourceMessageAnswered after a successful send and
MainWindow routes it through sendMessageTagChange, message-scoped and
off the undo stack, for the reason auto mark-read is: the flag records
that the mail went, and the send cannot be undone.
ComposeContext carries sourceMessageId rather than reusing inReplyTo,
which is deliberately empty on a forward so the recipient's client does
not file it under the thread it left. Keying on it made the "passed"
half dead code that compiled and never fired. A resumed draft is
excluded: its kind records how the file was opened, not what the user is
doing, so flagging on it would set R from a guess.
A received forward gets its own mark. Derived from the subject at paint
time, storing nothing and reaching no server, because "passed" means "I
forwarded this" and setting it from a guess would assert something false
on 222 existing messages. subjectIsForwarded() shares forwardSubject()'s
prefix table so the two cannot disagree, strips a Re: chain first, and
takes extra locale spellings from [general] forward_prefixes, which
extends the built-in table rather than replacing it.
A mutation survived the first round and corrected a claim in the code:
QRegularExpression::escape already makes a punctuation prefix inert, so
the word guard is not about pattern validity. It stops a configured "-"
matching "-: x". The comment and test say that now.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LXCZFLXbAii5n5wtovpdhh
|
|
Item 168, found by the user while hand-testing 118: Delete could be
triggered on a message already in the trash. Not dangerous, which is how
it survived. moveMessages() finds the file already in the destination
and takes its early-return branch, so the message is reported as moved,
an unsynced change is counted, and nothing happened. Restore had the
mirror of the same problem, added unconditionally to both menus and so
offered on mail that was never deleted.
Each is now hidden where it has no meaning, which is the rule item 112
established for the unread entry. The question is about the PATH, never
the deleted tag: a message trashed by another client carries no such
tag, which is why the trash view is path-based, and asking the tag would
hide Delete on exactly the mail a trash view is full of.
Delete also removes unread now, at the user's request on the same
tangent. It travels inside the same sendMove() call rather than as a
second write, so one undo returns the folder and the tag together. This
rewrites the Maildir filename, because maildir.synchronize_flags is
true, and so reaches the server: the same mechanism the post-new hook
refuses to touch, and the difference is that the hook acts unattended on
arriving mail while this is an explicit gesture on a message in front of
the user.
A mutation survived the first round and found a real hole: comparing the
prefix without its trailing separator passed every test, because no
fixture had a folder whose name starts with the trash folder's. Under it
Delete silently vanished from mail in acct/trash-old, which is not the
trash. The fixture carries that row now and all three properties are
mutation-checked. The suite is 37 of 38, the failure being item 136 on
an unrelated path.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HFuRPtzFrSxCQjFk6tq7gD
|
|
Item 118, unblocked by 103. Message > Empty trash..., scoped to the
account selector, with no default shortcut.
purgeMessages() is a separate worker entry point from moveMessages()
rather than a flag on it, because the two look alike and only one can be
undone. It takes named ids, never a folder sweep, so the blast radius is
what the dialog enumerated and the user confirmed, and it deletes every
file of a message: notmuch deduplicates by Message-ID, so leaving one
behind leaves the message alive in the folder the user emptied.
It confirms, naming the count and the account, defaulting to Cancel.
That breaks CLAUDE.md's no-confirmation rule deliberately and the rule
now records it as its single exception, in the same paragraph: a purge
has no inverse to push onto the undo stack, so the protection the rule
provides has to come from somewhere, and the dialog is where.
Two defects found rather than reasoned. The count claimed messages whose
files were already gone, overstating an irreversible action; an absent
file is correctly not an error, but that is not the same as destroyed.
And the user's hand test found the list still showing mail that no
longer existed: a purge removes rows rather than changing them, so there
is no optimistic update to apply and nothing was connected to
messagesPurged at all. It re-runs the current query now.
Verified against the live index after the user emptied one real
account's trash: zero files on disk, zero in the index. The suite is 37
of 38, the failure being item 136 on an unrelated path. Ten new strings
translated, lrelease reports 0 unfinished.
Item 168 is filed from the same hand test, on Delete being offered on
mail already in the trash.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HFuRPtzFrSxCQjFk6tq7gD
|
|
Item 112, and 99 and 147 with it: the user's note is one design across
all three. A union is not a state. ThreadSummary::tags is notmuch's
union over the conversation, so a thread holding even one unread message
answered "unread" and the thread toggle always chose "mark read". There
was no input that reached "mark thread unread" on a mixed thread, which
is the thread a user wants it for.
The thread toggle becomes two absolute actions, mark_thread_read and
mark_thread_unread. Neither takes a default chord, at the user's choice:
Ctrl+Alt+U meant whichever direction the union picked, and since item
132 a shortcut is a chosen subset rather than a requirement. It is now
unbound.
The message-scoped toggle stays a toggle, because one message has a real
two-valued state, and its label now names the direction it will go. On a
selection with no single state the entry is hidden rather than labelled
wrongly, chosen over disabling it; the thread submenu is the route then,
and its entries are absolute.
selectionTagPresence() is the three-valued predicate that needed to
exist. everySelectedRowHasTag() delegates to it and keeps its two-valued
answer, which is all a direction needs; a label needs the third value.
The refresh is keyed on the model's dataChanged as well as on the
selection, so a write moves the label without reselecting and none of
the six optimistic-update call sites has to remember.
Three mutations fail: restoring the union predicate reports the user's
original symptom, showing the action on a mixed selection, and dropping
the dataChanged refresh. The suite is 37 of 38, the failure being item
136 on an unrelated path. Four new strings translated, lrelease reports
0 unfinished.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HFuRPtzFrSxCQjFk6tq7gD
|
|
The version alone cannot tell one build of an unreleased X.Y.Z from
another, and the user rebuilds and hand-tests unreleased builds daily.
They chose a counter over a git description: what they want to know is
that the binary is newer than the one they were running, not which
commit it came from.
QTMAILDIR_BUILD_NUMBER is a cmake option, ON by default, that runs
cmake/BuildNumber.cmake as a build step to increment a counter and write
buildnumber.h. It had to be a build step: configure_file runs once per
cmake run, so a counter interpolated into version.h.in would sit still
across exactly the rebuilds this exists to distinguish, which is why
version.h.in includes a second generated header rather than carrying the
number itself.
Two macros, and the split is load-bearing. QTMAILDIR_VERSION stays a
clean X.Y.Z and keeps the window title, applicationVersion and anything
that might ever compare versions; QTMAILDIR_VERSION_DISPLAY carries the
number and goes to the three surfaces the user picked, --version and
--help, the About dialog, and the placeholder pane. The window title was
offered and declined, since the number would then be in every
screenshot.
The counter lives in the build directory and is not tracked, so it
cannot conflict on a pull or leave the tree dirty; a fresh build
directory restarts at 1, which is honest, because it is a different
build tree. A release passes -DQTMAILDIR_BUILD_NUMBER=OFF and the header
is written empty. The SlackBuild in the my-slackbuilds repo needs that
flag and is a separate commit there.
Verified by running it, since none of this is reachable from a C++ test:
three consecutive builds reported build 2, 3 and 4, and a separate
Release configure with the option OFF reported a clean 0.27.0. Passing
the flag to a tree that does not have the option yet is an unused-cli
warning and exit 0, so the SlackBuild change is safe before 0.28.0
ships. The suite is 37 of 38, the one failure being item 136 on an
unrelated path.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HFuRPtzFrSxCQjFk6tq7gD
|
|
A read-only notmuch handle is a Xapian snapshot taken when it is opened,
so it never observes a write made by another process afterwards. The
worker opened one handle and kept it for the process lifetime, which made
the sync script's `notmuch new` invisible: every query after startup was
answered from the index as it stood when the application launched.
The symptom was mail arriving while the window was open and not appearing
until a restart. It was not confined to the post-sync refresh, which is
what made it hard to place: a query typed by hand also found nothing,
since it hits the same handle. Tag writes were unaffected throughout,
because applyTags opens its own read-write handle per call.
Reopen in openReadOnly() rather than at each call site: every read path
begins by asking for the handle. A reopen failure is deliberately not
fatal, since the existing handle is still usable and answering from a
slightly stale index beats refusing to answer.
The suite could not reproduce this before: the test helper builds a fresh
worker per query, so it opens a fresh handle every time. The new test
holds one worker across two queries and indexes between them from a
second process.
Item 104.
|
|
Item 163. mbsync renames an uploaded file to add its `,U=<uid>` infix,
and the model's `MessageRef::filePath` was captured when the query ran,
so a row loaded before that sync names a file that no longer exists.
MimeParser then honestly reports a message it cannot open.
MaildirName::resolveRenamed() answers the filesystem question: returns
the path unchanged when it still exists, otherwise looks in that one
directory for the file whose unique stem matches. mbsync preserves the
stem (`<stem>:2,D` becomes `<stem>,U=5:2,D`), which is what makes this
safe to do by filename at all. It never recurses, never crosses a folder
boundary, and refuses an ambiguous match rather than guessing, since
opening or moving the wrong message is worse than reporting none.
It lives in MaildirName because that namespace already owns the `,U=`
infix and is a pure-value unit testable without a widget. A file that
changed FOLDERS is a different question that only the message id can
answer, and NotmuchWorker::moveMessages() re-resolves that way already.
Three call sites, all of which held a stale path:
- The message pane, which reported "(unreadable message)" over a file
that was on disk and readable. Cosmetic and self-repairing.
- Reply and Forward, refused outright, so the user could not answer a
message that was sitting there.
- The draft reopen, and this is the half that costs data. The refusal
happens BEFORE any composer exists, so the user composes again into a
fresh window whose autosave has no previous path to unlink. The old
revision survives, each save mints a new Message-ID, and both files
reach the server. The unlink machinery was correct throughout and
never ran.
forDraft() seeds draftPath from the RESOLVED path, never the caller's:
seeding the stale one would let the reopen succeed and the unlink still
miss, which is the same fork arriving one step later.
Covered by five unit tests on the resolver, including the two that keep
it honest (a genuinely missing file yields nothing, and a neighbouring
message is never matched), and by an integration test that renames the
draft the way mbsync does and asserts the file COUNT, which is the shape
the fork actually takes. Both mutation-checked; the integration test
fails with the reported symptom when the resolution is removed.
The stable-Message-ID question is deliberately untouched: it is what
turns a stale path into two server-side messages rather than one
replaced file, and it wants its own item.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UUQS6n3cmsFrsjCNmwNtf8
|
|
Item 162. mbsync uploads a file and renames it to record the server UID
(`<name>,U=<uid>:2,<flags>`), and notmuch keeps the pre-`U=` name until
that sync's `notmuch new` runs. moveMessages() renamed a path that no
longer existed, reported "Cannot move <file> to <folder>", and skipped
the message: Delete silently did nothing while blaming the destination
folder for a timing problem.
Before renaming, check whether the recorded path still exists. If it
does not, reindex that one Maildir directory and re-read the message's
filenames by id, taking the one that is on disk.
Recovery is by MESSAGE ID rather than by scanning the folder, because
two files can carry the same id and scanning could move the wrong one.
reindexFolder() indexes a single directory and is deliberately not a
`notmuch new`, which would walk the whole Maildir and run the post-new
hook that tags real mail.
Bounded to one reindex and one retry, so a file that is genuinely gone
still reports rather than becoming a silent no-op. The second test pins
that half.
Holding the move while a sync runs was the other candidate and is not
the fix: sendMove() already refuses on the write lock (items 97 and
106), but aSyncHoldsTheWriteLock() tracks notmuch's lock, while this
window sits between mbsync's rename and that sync's `notmuch new`.
mbsync renames without touching that lock, so the damaging window is
open when there is nothing to observe. That refusal is left alone; it
does its own job.
The ordinary fixture layout cannot see this, since nothing renames a
file underneath the index. The test renames without reindexing, which
is exactly the window mbsync opens, and guards that the database still
names the old path so it cannot pass against a fixture that quietly
reindexed. Mutation-checked: disabling the recovery reproduces the
original error.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UUQS6n3cmsFrsjCNmwNtf8
|
|
File, Edit and Format, to the scope the user chose. Save draft (Ctrl+S)
is the only new action: saveDraftNow() was reachable from the autosave
timer, the send path and closeEvent, so there was no way for the user to
ask for a save. It routes through that same function, which is what
emits draftSaved for item 158's indexing, reports through item 160's
status bar and raises the failure banner; a second write path would have
to repeat all three.
The menus show the toolbar's own QAction objects rather than copies, as
item 140 required for the message pane's bar. Two needed hand-building.
The HTML toggle is a QToolButton and cannot go in a menu, so a checkable
twin mirrors it in both directions, since a menu entry that only follows
the button is half a control. The signature entry takes the switch's own
QMenu pointer, because that menu is rebuilt whenever the signatures
change and copied entries would go stale.
Edit's entries drive QPlainTextEdit and follow its own undoAvailable and
copyAvailable, so a greyed entry tells the truth about what pressing it
would do.
theMenuBarReachesEveryComposerAction() is item 132's reachability rule
applied to the composer: it walks the real menu bar and collects the
composer's actions with findChildren, so an action added to the toolbar
and forgotten in the menus fails without the test being touched. It
skips actions owning a submenu, since Qt emits no triggered for those.
The composer's actions stay out of KeyMap, per item 148: they are
parented to this window, so they are WindowShortcuts dispatched to the
active composer and the main window's namespace is untouched.
lrelease reports 496 finished, 0 unfinished.
Closes item 161.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UUQS6n3cmsFrsjCNmwNtf8
|
|
Autosave worked and said nothing on success. The only feedback was
m_banner, which is the failure channel and whose persistence is
load-bearing for the quit path, so success got its own channel rather
than sharing one.
The fix is a funnel, not a label. m_dirty had seven writers, four of
which clear it and only two of those are a save: the constructor clears
it because seeding is not an edit, and the send handler clears it
because the message is gone. A cue hung off saveDraftNow() would have
been silently wrong in both. setDirty() is the only writer now, and it
refreshes the status cue and setWindowModified() together so neither
display can drift from the flag.
The age line needs a tick of its own, since it moves with no edit to
drive it. Five seconds against a label that reads in tens of them.
Two defects found by probing rather than by reading. The %n plural
rendered as "2 minute(s) ago" for every English user, because Qt picks a
plural form only when a translation supplies the forms and there is no
English .ts; it uses %1 and "min" now, which Italian substitutes
identically. And the status mark was inside the translatable string,
where a translator could drop it; it is concatenated outside tr().
Presentation reworked after the user looked at it. The first version
reused item 151's yellow ribbon treatment, which reads as a misplaced
widget on a bare status label rather than as a warning, and put both
labels in the permanent widget area, which is the right-hand tray. They
are ordinary status text on the left now.
onlyTheSetterWritesTheDirtyFlag() asserts the funnel structurally, by
reading composewindow.cpp: the first test for the send path called
markClean() directly and a mutation restoring a direct assignment left
the whole suite green. Four mutations now fail. The suite still cannot
see the presentation, which is why that half needed a hand test.
lrelease reports 487 finished, 0 unfinished.
Closes item 160, and unblocks 161.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UUQS6n3cmsFrsjCNmwNtf8
|
|
The Drafts filter shipped threaded in item 138, reasoning that a draft
reply belongs with the conversation it answers. That reasoning cost the
feature: a thread row stands for its first matched message, which for a
draft reply is the message being replied to, so the draft itself had no
row of its own and double-clicking the conversation opened nothing.
Reversed with the user. Drafts now follows Sent; Trash deliberately does
not, since a deleted message still belongs to its conversation and
nothing there has to be reachable for editing.
The view mode was decided in three places that each compared against
"sent" and had to agree: builtinFilter(), the reader that reapplies the
mode, and the writer that skips storing what the generator implies.
generatorIsFlat() is now the one closed set they share, and
builtinFilter() sets flat from it rather than inside a branch so the set
cannot drift from the labels.
Setting only the branch would have looked correct. Its save/load pair
survives by accident, because the writer's skip knew only "sent" and so
would have stored the key for drafts. The gap is the reader's fallback,
for a file carrying no flat key at all: an older build, a migration or a
hand edit comes back threaded against a flat button, and the next save
persists the disagreement.
theDraftsFilterIsThreadedNotFlat is inverted rather than deleted, keeping
its history, and now also pins Trash as threaded. The round trip is
covered by extending aGeneratedEntryWritesNoRedundantKeys, which already
asserted that property for Sent. Mutation-checked: reverting
generatorIsFlat() to "sent" alone fails both.
Suite 37 of 38; undoMovesTheMessageBack is item 136, pre-existing and on
an unrelated path.
Closes item 159.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UUQS6n3cmsFrsjCNmwNtf8
|
|
Autosave writes the draft to the Maildir drafts folder and stops, while
the Drafts view is a notmuch path: query, so a freshly saved draft was
invisible until notmuch new ran. saveDraftNow() now emits draftSaved, and
MainWindow connects it to a new NotmuchWorker::indexDraftFile() that
indexes the one file the way moveMessages() does, with the previous
revision removed so a rewrite leaves no ghost.
The send path unlinks a draft that was indexed while being composed, so
draftRemoved -> removeIndexedFile() drops its entry too.
Measured: notmuch_database_index_file assigns NO tags (unlike notmuch
new, which adds draft inbox unread), so no tag-stripping is needed and the
draft cannot leak into a tag:inbox view.
Item 158.
|
|
A resumed draft kept m_signatureChosen false, so a From: change re-seeded
the signature and rewrote what the user had saved, inserting the new
account's where the saved block no longer matched a known file. The draft is
the user's deliberate prior state and must not follow a From: change, so the
draft branch marks it chosen.
|
|
A From: change re-seeds the signature from the newly selected account, and
stops doing so the moment the user picks one from the switch. Re-seeding
unconditionally is the one behaviour that can silently discard a deliberate
choice made a moment earlier; this is the shape send_html already uses.
seededSignatureName() reads the combo rather than the context, which
records where the composer opened and does not follow a change to it.
Part of item 152.
|
|
The guard compared the buffer block, with trailing blank lines trimmed,
against knownSignatures() values returned verbatim by text(), which carry
the trailing newline every editor writes. The two never compared equal, so
a signature read back from disk was always treated as unknown: switching
appended a second signature instead of replacing, and None removed nothing.
Normalise each known entry the same way the block scan does, once in
replace(), rather than per comparison.
Part of item 152.
|
|
A QToolButton with a checkable menu at the right end of the editor bar,
where item 142 put the controls of the editor. Not registered in KeyMap:
parented to the composer like the formatting actions, so its scope is this
window.
The signature is applied through a QTextCursor rather than setPlainText(),
which destroys the undo stack, and the seeded one is cleared from that
stack for the reason the seeded quote already is: one Ctrl+Z must not wipe
content the user never typed.
A resumed draft seeds nothing. Its body already carries the signature it
was written with, and seeding again would put a second one on a message
written once.
Part of item 152.
|
|
[compose] signature and signature_position, and a per-account signature
that OVERRIDES the former. The account seeds the choice rather than owning
it: the composer's switch keeps every signature reachable whichever account
is selected, which is what keeps the note's "not tied to an account"
constraint intact.
The fallback is deliberately NOT resolved here. An account with no key of
its own carries an empty string, so the composer can tell "says nothing"
from "says none" and fall through itself.
signature_position follows quote_position's shape exactly, reporting a
present-but-malformed value rather than accepting it silently.
Part of item 152.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEcn3u19xPqv6ggD15PG4c
|
|
Finding a "-- " delimiter is not authority to delete what follows it. The
block is replaced only when its text matches one of the signatures on disk,
and otherwise the new one is inserted with nothing removed, so a wrong guess
adds a visible duplicate rather than destroying the user's writing.
A quoted delimiter is never the signature either: the quoted original
carries the other party's, and it is not this message's to replace.
The block's lower boundary is quoteStart(), not the first quoted line. The
attribution introduces the quote and belongs with it, so scanning for '>'
alone swallowed "On Mon, someone wrote:" into the signature block: it then
matched no known signature, and had it matched, removal would have stranded
the attribution above the text it introduces. The boundary the insertion
uses and the boundary the removal uses have to be the same one.
The guard's test was mutation-checked by making the match unconditional,
which fails it.
Part of item 152.
|
|
Both placements over one implementation. above_quote inserts before the
attribution rather than before the first quoted line: the attribution
introduces the quote and belongs with it, and a signature between the two
would read as part of the quoted message.
A buffer with no quote makes above_quote identical to end, so a new
message needs no branch of its own.
Part of item 152.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEcn3u19xPqv6ggD15PG4c
|
|
One file per signature under a directory the caller names, the stem being
the name shown to the user. A name containing a path separator is refused:
it arrives from the config file, and it reaches a path that is read into a
message about to be sent.
Part of item 152.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEcn3u19xPqv6ggD15PG4c
|
|
Item 94. The query row is the six built-in filters (Unread, Inbox,
Important, Sent, Drafts, Trash), which compose with the account
dropdown, and every saved query lives in the More queries menu. Nothing
has to decide which of the user's queries get button space, which is the
question item 93 would otherwise have had to answer.
SavedQuery::pinned is gone from the struct, the reader, the writer, the
save dialog's checkbox and the pin/unpin context action.
The stored key is stripped rather than left ignored, at the user's
choice. That has one non-obvious requirement: `pinned` stays named in
loadSavedQueries' `known` list precisely so it is NOT collected as an
unknown field, since those are preserved and written straight back. A
mutation removing that name puts the key in the file for ever.
Confirmed with the user before starting that the built-in set covers
their use, since removing pinning removes the escape hatch this item was
blocked on.
Tests: four pinning tests replaced by two on the new rule, four more
converted from buttons to menu entries. migrationPinsEveryEntry and
aStoredGeneratedQueryIsUnpinnedNotDropped are rewritten around the
property that outlived the flag rather than deleted: an entry must be
KEPT, which is what both assertions were really guarding.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEcn3u19xPqv6ggD15PG4c
|
|
Item 157, the half item 153 did not close. A draft was editable by
double-click and by a Message-menu entry, neither of which is where the
user looks while reading one. populateMessageBar() swaps the reply pair
for edit_draft on a displayed draft.
Three things came out of hand-testing it, each invisible to the tests
written before them.
The bar keyed on currentIndex(), which a query leaves valid on a row of
the discarded result, so it kept the draft button after switching to the
inbox and the reply pair after switching to drafts. This is item 150's
trap one level up. It answers from m_currentMessageId/m_currentThreadId
now, which every blanking route clears, refilled from
showPlaceholderPane(), the one site all five of those routes share.
That exposed a defect predating the bar: updateComposeActions() ran only
from the two selection handlers, so Reply and Forward stayed enabled over
a blank pane. Invisible while they sat on the main toolbar among
always-on actions.
The bar is hidden over an empty pane, so it comes and goes with the
subject and the details button rather than hovering over the logo. That
in turn broke the showing half: setBarActions() runs before showThread()
fills m_items, so the first message opened after a blanking left the bar
hidden and the second showed it from stale items, one selection behind
for the life of the view. updateHeader() shows it, beside the details
button it rides with.
The test missed the last one by asserting before the render landed,
measuring the placeholder; it waits on showingPlaceholder() now.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEcn3u19xPqv6ggD15PG4c
|
|
Item 153. DraftStore had a write() and no reader, and nothing opened a
composer from an existing message, so a draft rendered like ordinary mail
and could never be finished or sent.
ComposeContextBuilder::forDraft() reads one back. A new Kind::Draft seeds
every field verbatim: the subject takes no Re:/Fwd: prefix, and the body
goes in exactly as it was left, with none of seedBody()'s quote framing. It
is reachable by double-click and by an edit_draft action in the Message
menu.
Three things the shape of this depends on.
A resumed draft must OWN its file. Maildir has no in-place edit, so an
autosave writes a new file and unlinks the old one; a composer that did not
know its own path would leave the original behind and one message would
become two. ComposeContext::draftPath carries it into m_draftPath, which the
autosave already knew how to replace.
MimeParser had no bcc, and nothing had ever needed one. MessageBuilder
writes Bcc into the draft file deliberately and explains why, so a resumed
draft that ignored it would drop every blind recipient from the message the
user then finishes and sends, reporting nothing.
edit_draft is gated on the file being inside a configured drafts folder,
matched on the PATH. A `draft` tag is not enough: notmuch surfaces the
Maildir D flag as one, and a message flagged by another client sits in the
inbox. Offered on ordinary mail, the composer would own a file it did not
write and the first autosave would delete a received message.
And a live defect found on the way, which is most of why this took as long
as it did. updateComposeActions() ran only from onSelectionChanged. Both
signals fire for an ordinary click, so nothing had noticed; but running a
query and setting the current index emits currentRowChanged ALONE, so the
enablement was computed against the previously selected row. Edit draft
stayed disabled on a draft selected that way, and the reply family had the
same blind spot with no test that could see it. Now connected to both.
Reading currentRowChanged is safe here for the reason CLAUDE.md gives: it
answers "which row is current", and no count is read.
WorkerBackedWindow::AccountSpec gains a drafts field, which the two new
tests need and which no fixture could express before.
|
|
Items 138 and 148.
The query row carried Unread, Inbox, Important, Sent and Trash, and no
Drafts, though the composer has been autosaving into each account's drafts
folder since compose shipped. Reaching them meant typing a query by hand.
Smaller than its size suggested: Account::draftsQuery() and
Config::allDraftsQuery() already existed for the placeholder pane's drafts
count, and builtinFilters() derives the row from kQueryGenerators, so the
work was the generator entry, two resolvedQuery branches, a label and an
icon.
It follows TRASH rather than Sent. Folder-matched like both, because `draft`
is a Maildir flag notmuch surfaces as a tag while the folder is what the
user means and what the composer actually writes into. But NOT flat: Sent is
flat so a thread cannot fold the user's own message back into the
conversation it answers, and a draft reply belongs with its conversation for
the same reason a trashed message does.
An account with no drafts folder shows no button, per item 103's rule. The
existing row test surfaced that by failing until its fixture configured one,
which is the rule working rather than a defect.
Ctrl+W closes the composer, which bound nothing at all: the only way out was
the title bar. The action is parented to the composer, so it is a
WindowShortcut dispatched to the active one only and the main window's
namespace is untouched, exactly like the formatting shortcuts. It calls
close() rather than doing anything of its own, since closeEvent() already
decides whether the draft is saved and a second route out that skipped it
would lose the message.
The Italian gains "Bozze"; lrelease reports 478 finished, 0 unfinished.
|
|
An Expanding vertical size policy stretched the button to the full height of
the header form beside it, while the icon and the label kept their natural
sizes. The result was a tall rectangle with the icon anchored near the top,
the label near the bottom, and a gap between them: two marks rather than one
control.
Fixed size, derived from the icon so the square still fits its contents if
toolbar_icon_size changes, and aligned vertically centre against the headers
so the whole button is centred rather than its contents. The icon grows to
the configured size plus 8, since at this scale it is the button's subject
and the word underneath is the caption.
Nothing in the layout or the actions could see this, which is why the test
now asserts the vertical policy and that width equals height. Both fail
against the previous code.
|
|
Items 142, 143, 144 and 145, to the layout the user described.
The composer had one addToolBar carrying three scopes at once: text
formatting, message composition, and the terminal action. It read as a menu
bar that is not one. There is now no window toolbar at all.
From: [.............] +--------+
To: [.........] [v Cc/Bcc] | Send |
Subject: [...........................] +
[B][I][</>][S][link]["] [Attach] [Send as HTML]
+---------------------------------------------+
| message text |
+---------------------------------------------+
[Remove] * report.pdf <- only when attached
Send is a large icon-above-text button beside the headers: it is the
terminal action and carries the weight to match. Formatting is a toolbar
widget in the central column directly above the text it formats, icon-only
with the words kept as tooltips, which is where a tooltip stops being
decoration. Attach and the HTML toggle ride the right end of that bar, past
a stretch, because neither formats text. Remove attachment sits with the
list it acts on and appears only once something is attached.
"Also send a formatted copy" becomes "Send as HTML": the old label described
a mechanism without naming it, leaving the reader to infer that "formatted"
meant HTML and that "copy" meant a MIME part rather than a second message.
Cc and Bcc hide behind a disclosure beside To:. revealCcBccIfUsed() only
ever shows, never hides, so nothing but the user's own click can make a
field holding an address invisible: a hidden recipient is a message going
somewhere the sender cannot see, which is worse than the clutter this
removes. The label is hidden with each field, since a QFormLayout holds the
two as separate items and hiding the line edit alone strands a "Cc:" over
empty space.
Two send-lock faults, one predicted and one not. The backlog warned that
setInputsEnabled() disabled the single toolbar wholesale, so the send-path
test was strengthened to name every control BEFORE the split; it then caught
Attach live during a countdown, where a file appended after MessageBuilder
has run is either dropped or added to bytes already sent, silently either
way. With every control named it failed again on format_bold: disabling a
QToolBar greys its buttons but leaves each QAction enabled, so Ctrl+B during
a send would have edited a message already being built, through a button
that looked unavailable. setInputsEnabled() now walks the bar's actions too.
The Italian translation is refreshed; lrelease reports 477 finished, 0
unfinished.
|
|
Three corrections from looking at the built bar.
Compose returns to the main toolbar. The split this was built to, "about a
message" against "about the list", does not survive contact: what matters is
what the action NEEDS. Reply and Forward are meaningless without a message on
display, while Compose needs none and is disabled only when no account can
send. So the pane's bar holds exactly the two actions that depend on what it
is showing, and Compose sits with the window-wide ones.
The bar moves below the subject and details rows, directly above the web
view. At the top of the pane it read as window chrome rather than as
belonging to the message. The transient notice bars stay above it: they
explain the message rather than offer an action on it.
Its icons were the style's own default, 16px, which is tiny beside a 32px
toolbar. They are now 7/8 of toolbar_icon_size, which is the 28 the user
asked for at their 32, derived rather than hardcoded so the relation holds
if that key changes. The test asserts the relation as well as the value,
since a bare 28 would stop meaning anything the moment the key moved.
m_headerLabel gains an object name so the placement test can find the row it
must sit below.
|
|
Items 139, 140 and 141, built together because the seam between them is
wasted work: 140 needs a container and 141 is that container.
The main toolbar had grown to mix two scopes. Sync, Archive, Delete, Mark
all read and Undo act on the list or the selection; Compose, Reply and
Forward are about one message. With everything in one row the distinction
was invisible, and Forward was on no toolbar at all, reachable only from the
Message menu, which is item 139.
Compose, Reply and Forward now sit on a bar above the message pane, and
LEAVE the main toolbar rather than gaining a second home: that is what makes
the toolbar's remaining contents mean one thing. Toggle HTML joins them at
the right end, separated by an expanding spacer, since changing how a
message is displayed is a different scope from acting on it. That layout was
the open design question item 141 recorded, and it was settled with the user
rather than guessed.
The actions are MainWindow's own QAction objects shown a second time, never
copies: a duplicate would carry its own enablement and drift from the menu
entry updateComposeActions() keeps in step. MessageView::setBarActions() is
the seam, so the pane still knows nothing about the window's action map.
Two things worth recording:
QToolBar has no addStretch(), so the separation is an expanding spacer
widget. A test asserting only on action ORDER passes with that spacer
deleted, measured, so it asserts on the spacer's size policy instead.
noTwoActionsShareAnIcon looked up the toolbar with an unnamed
findChild<QToolBar*>(). There are two toolbars now, so it is pinned to
main_toolbar: pointed at the pane's bar it would have asserted that a
thread action is absent from a bar that never holds any, and passed while
the rule it exists for went unchecked.
|
