| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
The fix landed in b1db3e8 and the row said "awaiting hand test". A sync
run from the application added 20 messages and they appeared without a
restart, which is the property the reopened read-only handle exists to
give, so the row is now done and the section moves to the closed file.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HFuRPtzFrSxCQjFk6tq7gD
|
|
Item 104 is fixed and its entry was wrong. It named mbsync's folder
patterns as the leading theory and concluded the cause was most likely
outside this repository; the reproduction put it at layer 3, in the
worker's own handle. The superseded theory is kept, since it would
produce a similar symptom and remains worth checking first in any future
report of this shape.
Two measurement errors from the diagnosis are recorded with it. A bare
`inbox` in a notmuch query is a free-text term rather than a tag term,
and the application generates `tag:inbox`; reading a message's tags
across every file matching a subject mixes several accounts' copies into
one answer. Each produced a confident wrong answer before it was caught.
Item 166 is new, found while setting up msmtp. The `post-new` hook's
sent-folder carve-out judges provenance by a file's path, but notmuch
deduplicates by Message-ID, so mail sent between two of the user's own
accounts is one message with a file in each. The carve-out matches the
sent copy and strips `inbox` from the message the recipient's inbox copy
also belongs to. Three options are laid out; the fix is a two-repo
change and the hook runs unattended on live mail, so it needs a decision
rather than a patch.
|
|
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.
|
|
Found while hand-testing items 163 and 164: four saves produced four
distinct ids, and one reopen-and-edit turned one into another.
Cause verified in the code rather than inferred. MessageBuilder::build()
calls g_mime_utils_generate_message_id() unconditionally and every
autosave calls build(); OutgoingMessage has no field to carry an id in,
and ComposeContext has none for the draft's own id either, since
inReplyTo and references are the ORIGINAL's when replying. So a stable
id needs a field threaded from forDraft() through both structs, not a
changed call site.
Filed as needing a DECISION rather than an implementation, because what
a draft's identity is is not obvious: a stable id reused at send makes
the draft and the sent message one message but means the server saw that
id before anything was sent; a stable id discarded at send keeps
revisions collapsed while drafting and threads under a fresh one; the
status quo never reuses an id for two different things, which is its one
real virtue.
Not urgent and explicitly not blocking item 163, whose fix restores
correct file replacement. This is the property that turned that fork
into two MESSAGES rather than one duplicated file, and the remaining
route to it is an interrupted save, since DraftStore::write() unlinks
the previous revision only after the new one is safely on disk.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UUQS6n3cmsFrsjCNmwNtf8
|
|
Its section moves to the closed file on this commit, with the outcome
recorded: MaildirName::resolveRenamed() wired into all three read sites,
the two deliberate refusals (ambiguous match, genuinely missing file)
and why each has a test, and the note that forDraft() must seed
draftPath from the resolved path or the fork simply arrives one step
later.
The stable-Message-ID question is recorded as left undecided rather than
quietly dropped: it is what turns a stale path into two server-side
messages rather than one replaced file, and a draft's id is not yet the
sent message's id, so it wants its own item.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UUQS6n3cmsFrsjCNmwNtf8
|
|
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 is done and its section moves to the closed file on this
commit, with the outcome recorded on it: moveMessages() re-resolves by
message id when the recorded path is gone, and the hold candidate was
investigated and rejected because aSyncHoldsTheWriteLock() guards
notmuch's lock while the window sits between mbsync's rename and that
sync's notmuch new.
Item 163 gains a second site, found while hand-testing 164 and worse
than the one it was filed for. openComposerFor() passes ref.filePath to
forDraft(); after mbsync renames the file the parse fails and the reopen
is refused BEFORE any composer exists, so composing again starts fresh
with no previous path to unlink. The old revision survives, each save
mints a new Message-ID, and both revisions reach the server. The unlink
machinery is entirely correct and never runs. Its heading and row now
name both sites, and the note that 162's fix would cover it is removed:
that fix re-resolves inside the worker, while these hold a stale path in
the UI.
Item 164 gains the reproducer's findings. Seven variants in throwaway
databases establish that index_file applies no tags, that the carve-out
strips inbox correctly in every filename shape and ordering tried, and
that the only reproduction is a pass applying inbox while tag:new is
already spent. The trigger is still not established, and the entry says
so: the live log shows the hook ran on the affected pass and logged
success, which the new match-count instrumentation will disambiguate on
the next occurrence.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UUQS6n3cmsFrsjCNmwNtf8
|
|
Item 164 diagnostics. A `notmuch tag` that matches nothing succeeds, so
the carve-out logged "applied over N folder(s)" whether it stripped
`inbox` from four messages or from none. A draft kept `inbox` on a pass
whose log claimed the carve-out had run, and that line could not tell
the two cases apart.
Count before tagging, since the tag is what makes the count zero, and
report it alongside the folder count. Nothing branches on the value: a
failed count yields `?` rather than failing the sync, because the tag
reports its own status separately.
This does not fix item 164, whose trigger is still unreproduced. It
makes the next occurrence self-explaining: `0 message(s)` means the
message was never in tag:new scope, and a non-zero count means the tag
ran over it and something re-added the tag afterwards.
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
|
|
The entry blamed a missing drafts helper. Neither half of that was true.
NOT_ARRIVALS in qtmaildirconf.py is ("sent", "drafts"), so sent_folders()
already returns both; the function name says sent and its contents do not,
which is what made the wrong reading plausible. Run against the real
config it returns every account's drafts folder, and notmuch count over
the carve-out query and the affected message id returns 1: the query the
hook builds MATCHES the draft. The folder list and the query are correct
and the fix is not there.
A second theory is also recorded as dead. An mbsync-style rename does not
re-apply new.tags: measured in a throwaway database, a file renamed to add
,U=4 and reindexed kept the tags it had.
What is established: the carve-out is scoped to tag:new, the draft carries
inbox, and tag:new is 0, so it was never in scope when the hook ran. The
installed hooks are symlinks into this repository, verified rather than
assumed, so the code read is the code that runs.
What is not established is which pass tagged the file. The likely shape is
an ordering one, since item 158 indexes a draft from the application
itself and a file already known to the database is not new on the next
pass, but that is a third hypothesis and the first two were both wrong.
The entry now calls for a reproducer driving the real sequence before any
code is written, since the hook tags real mail unattended every ten
minutes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UUQS6n3cmsFrsjCNmwNtf8
|
|
163: the message pane names a path that no longer exists and reports the
message unreadable. Same mechanism as 162, different site and different
fix. mbsync renames the file to add ,U=<uid>; by the time the pane fails,
notmuch is already CORRECT and the stale path is the MODEL's, cached when
the row was loaded. Measured: the index named the ,U=4 file while the
pane named the pre-U= one. 162's likely fix, refusing to write while a
sync runs, does not touch the read path. Points at recovering by
re-resolving the id, the way recoverStaleThread() already does, with a
bounded retry so a genuinely unparseable message still reports.
164: every newly synced draft carries inbox. Measured "draft inbox
unread" on a draft this application wrote. strip_inbox_from_sent() reads
qtmaildirconf.sent_folders() only, and qtmaildirconf.py has no drafts
equivalent, so the carve-out never covers a drafts folder. Item 158's
measurement was right and did not reach this: index_file assigns no tags,
but mbsync's upload and the next notmuch new re-tag the file.
164 also contradicts the shipped 0.27.0 changelog, which claims sent mail
and drafts both stay out of the inbox. The drafts half has never been
true, so correcting the entry is part of that item.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UUQS6n3cmsFrsjCNmwNtf8
|
|
Found by hand while deleting a draft: "Cannot move <file> to <folder>".
Neither Delete nor item 158 is at fault. mbsync uploads a saved draft and
RENAMES it to add its ,U=<uid> infix, and notmuch keeps the pre-U= name
until that sync's notmuch new runs, so moveMessages() calls QFile::rename
on a path that no longer exists.
Verified against the live Maildir rather than read: notmuch named a file
that was not on disk while a sync was running, and the same query was
clean afterwards with the file present under its new name. That is why it
reads as intermittent and why it heals itself.
Truthful and lossless, but the action silently does nothing and the
message blames a folder for a timing problem, which sent the user looking
at a configuration that was correct.
Records both candidate approaches and notes the likelier one: refuse the
move while a sync holds the lock, joining the held-edit machinery items
97 and 106 already built for exactly this shape, rather than re-resolving
the filename and racing the same window. Also notes that this is the ,U=
trap CLAUDE.md records for MaildirName::fresh(), seen from the other
side.
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
|
|
Both are composer feedback, and both causes are verified in the code
rather than copied from the note.
160: autosave works and is silent on success. The only feedback is the
failure banner, whose comment explicitly rejected the fading status line
that success actually wants. m_dirty and the draftSaved signal already
carry both states; nothing displays them.
161: the composer has no menu bar, and Save draft does not exist as an
action at all. saveDraftNow() is reachable only from the timer, Send and
closeEvent, so there is no way for the user to ask for a save. Notes that
the composer's actions stay out of KeyMap per item 148, that
everyActionIsReachableFromAMenu() walks the main window only, and that
"duplicate the other actions" needs the user to say which, since most
message actions are meaningless over a message being written.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UUQS6n3cmsFrsjCNmwNtf8
|
|
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UUQS6n3cmsFrsjCNmwNtf8
|
|
Signatures (item 152): one markdown file per signature under
~/.config/qtmaildir/signatures/, spliced into the composer buffer and
chosen from a switch on the editor bar. [compose] signature seeds a new
message, [account.<key>] signature overrides per account, and
[compose] signature_position picks end or above_quote.
Also carries three fixes found by hand-testing it: a saved draft is
indexed so it appears without a sync (item 158), the Drafts filter lists
messages rather than threads so a draft reply can be opened (item 159),
and a resumed draft no longer re-seeds its signature on a From: change.
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.
|
|
|
|
Part of item 152.
|
|
indexes it
Found by hand: autosave writes the draft to the Maildir drafts folder but
never indexes it, and the Drafts view is a notmuch path: query, so the
draft cannot be reopened until notmuch new runs. Approach reuses the
single-file index moveMessages already performs.
|
|
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
|
|
The test asserts that a delimiter inside the quoted original is not treated
as this message's signature, and it passes whether or not the code checks
for that. Two mutations were measured against it and both stayed green:
trimming the delimiter comparison so a quoted "> -- " matches, and making
the quoted text one of the known signatures so the match guard could not be
what refuses the removal.
Neither changes the output. blockEnd() stops the block at the quote, so the
quoted signature survives either way, and the behaviour is correct under
both. The comment says so, so the next reader does not spend the same
measurements discovering that the test cannot be sharpened.
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
|
|
Found incidentally while building item 152, by an agent that checked rather
than assumed: it ran test_mainwindow at the preceding commit in a throwaway
worktree and got the identical failure, so the signatures work is ruled out.
Records the assertion text, which is worth more than the flakiness history.
After the undo the message file is in neither cur nor new of the account
inbox, so the question narrows from "why does this race" to "where did the
file go", and the trash folder and the account root are the places to look
first. A move landing in the wrong folder is the mail-safety half of the
fork this entry already described, and it would present exactly this way.
The 70-second duration already recorded fits a QTRY_* waiting for a file
that is never going to appear.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEcn3u19xPqv6ggD15PG4c
|
|
The plan's test fixtures carried the maintainer's own first name as the
signature text, which reaches a committed test file. Task 1 caught and
corrected it in the code; this corrects the source so tasks 2, 3, 5 and 6
do not reintroduce it.
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
|
|
Eight tasks, TDD throughout, against
specs/2026-08-24-signatures-design.md.
Tasks 1 to 3 build the Signatures namespace: reading the directory, the
splice for both placements, and the match guard that keeps a "-- "
delimiter from authorising a deletion. Task 3 carries a mutation check on
that guard, since it is the one piece preventing data loss.
Task 4 adds the three config keys, task 5 the editor-bar switch and the
seeding, task 6 the From: follow that stops once the user chooses. Task 7
is documentation and the Italian translation; task 8 closes the backlog
item, and deliberately hands the work over for a hand test first rather
than marking it done on a green suite.
MessageBuilder is untouched by every task, which the plan states twice: the
signature lives in the composer's buffer and both MIME parts are already
derived from it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEcn3u19xPqv6ggD15PG4c
|
|
Finding a "-- " delimiter is not enough to authorise removing what
follows it. The block is replaced only when its text matches one of the
signatures on disk; otherwise the new signature is inserted and nothing
is removed.
"-- " can reach the buffer without the user ever choosing a signature,
most plausibly pasted in with quoted text from another client, and the
unguarded scan would have deleted everything after it silently. The
failure is now directional: a block that matches is replaced, and one
that does not produces a second signature, visible in the editor and one
undo away. A wrong guess adds text rather than losing it.
Two markers were considered for the same problem and refused, both
recorded with the reasons. A zero-width character ships in the sent
message, fingerprinting the client in outgoing mail, and has to survive
the draft round trip through GMime, quoted-printable and MimeParser,
which is the pipeline that normalises such characters away. A doubled
delimiter is not the RFC 3676 separator, so no receiving client would
recognise the signature, and it would not have caught the pasted-text
case that prompted it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEcn3u19xPqv6ggD15PG4c
|
|
One markdown file per signature under ~/.config/qtmaildir/signatures/,
spliced into the composer's buffer and switched from a control on the
editor bar.
MessageBuilder needs no change: it already builds text/plain from
markdownBody verbatim and text/html from MarkdownRenderer over the same
string, so one markdown signature in the buffer yields both forms
correctly. That is the "transparent to the user" requirement the note
asked for, and it is why a two-file text/HTML variant was dropped after
being chosen: it buys designed HTML signatures at the cost of the
signature no longer being visible while composing.
The switch stays stateless. seedBody() deliberately refuses to track
"my text" and "the quote" as separate pieces, and a toggle cannot duck
that question the way the quote did; it answers it by scanning for the
last "-- " block not followed by quoted lines, so nothing can desync
from the undo stack. That same scan is what lets signature_position
offer both end (the default) and above_quote over one implementation.
The per-account key does not reopen the note's constraint: an account
seeds the choice, the switch keeps every signature reachable, and the
automatic follow stops once the user picks one.
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.
|
|
Item 138 gave drafts a button and immediately surfaced that they cannot be
edited: DraftStore has a write() and no reader, and no action opens a
composer from an existing message, so a draft renders as ordinary mail and
can never be finished. Filed as 153, a defect rather than an enhancement.
The user's note splits it in two, "double-clicking a draft should open the
editor" and "there's no edit action anywhere". They are one item: the second
is the general case and the first is one route into it.
154 to 156 are the rest of that note's list, all v2 and all unspecified to
different degrees. 154 and 156 are deliberately separate: a read receipt is
the reader's client to honour, a delivery receipt the sending server's, and
whether the latter can be requested at all depends on the send_command.
152 gains the constraint the user added today: a signature is not tied to an
account and is switched from the editor bar, which rules out a plain
[account.*] key as the whole answer.
|
|
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.
|
|
The previous commit fixed the cursor within each quote_position branch and
the user still saw the old layout, because the branches were already right:
what was wrong was the DEFAULT. quote_position shipped as `above`, and the
layout asked for is exactly what `below` already produced, a blank line at
the top with the quote underneath.
So the default flips, along with the fallback for a malformed value and the
warning naming it. Nothing needs an Upgrading note: compose has not been
released, so no config in the wild sets this.
Focus goes to the body whenever To: is already filled, which a Reply and a
Forward always are. The form's first widget took it otherwise, so the user
had to click into the editor before typing. A New message keeps the default,
since an empty To: is genuinely the first thing to fill in.
The focus test asserts on the window's focusWidget() rather than on
QWidget::hasFocus(): an unshown window is never active, so hasFocus() reads
false whatever the code does and would fail against a correct fix. Both
directions are mutation-checked, since focusing unconditionally passes the
reply case while breaking the new-message one.
|
|
The three out-of-band bars in the message pane were plain labels on the
pane's own background, so they read as part of the page they were trying to
interrupt. They now carry a severity, as the user asked for:
- yellow for a warning that explains a limitation and offers nothing to do
about it (the receive-only ribbon)
- blue for one offering an action (remote content blocked, stale thread),
with the button moved right of a stretch, where the eye ends up after
reading the sentence
Each severity carries its own light and dark set rather than one tint
dimmed, and the theme is read off QPalette::Base, the same surface
HtmlBuilder reads, so a bar and the message under it cannot disagree about
which way round the theme is. Every ground states its own text colour: the
palette's may be near-white, which is unreadable on a pale tint.
The blocked row had to become a widget on the way. It was a bare
QHBoxLayout, which has nothing to paint a ground on, and once wrapped, the
six sites that hid its label and button individually had to hide the
wrapper instead or an empty painted strip would have been left behind.
The test asserts on the stylesheet string rather than on pixels, per
CLAUDE.md on rendering probes: an unshown widget offscreen renders nothing,
so a pixel test would pass whatever the code does. It compares the COLOURS
the sheets name rather than the sheets themselves, since each names its own
widget and would differ by that alone.
|
|
MessageView::clear() resets the blocked-content bar, the stale notice and
the attachment bar by hand, and forgot the receive-only ribbon. Only
setReceiveOnlyAccount() ever hid it, and that is reached from
updateComposeActions(), which runs on a SELECTION change.
So the ribbon survived every route to a blank pane that is not one:
clear_pane, clear_selection, a new query, and a multi-row selection. It sat
over a blank pane, or over another account's mail, naming an account that
was no longer on screen and contradicting the live Reply button beside it.
Worth recording because it cost a test: the first one written for this
selected receive-only mail and then mail from a sending account, which is
how the report reads. It passed against the unfixed code, because that
gesture is precisely the one path that was already covered. The test now
drives clear_pane and a new query as well.
Its own trap, in the test rather than the code: clear_pane leaves the row
CURRENT, so re-selecting it emits no change and the ribbon is never
re-raised. The test moves away and back instead.
|
|
quote_position names where the QUOTE goes, so the reply belongs on the
other side of it and the cursor has to follow the reply rather than the
buffer. seedBody() moved the cursor to Start under both positions, which
is correct only for Below: under Above, the shipped default, it landed on
the "On ... wrote:" attribution line, so every reply had to have room made
before it could be typed.
End under Above, Start under Below.
The existing theQuotePositionDecidesWhereTheQuoteLands passed throughout
the defect and still does, because the quote was in the right place all
along; only the cursor was not. The new test asserts the cursor's block is
blank, and that typing lands on the correct side of the quote, so a fix
that freed the cursor by inverting the position would not satisfy it.
Also re-measures item 136 in the backlog, which is not the intermittent
race it was filed as: undoMovesTheMessageBack fails 6 runs in 6 when named
alone and passes in the full suite, on a clean tree, so it depends on the
tests before it.
|
