| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
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.
|
|
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.
|
|
A composer is deliberately parentless, so that it appears in the task
switcher and stays usable while the main window is. Qt therefore does not
take it down with that window, and being a live top-level it kept the process
alive: the main window vanished, the composer stayed on screen with nothing
behind it, and closing it then raised the unsaved-edits dialog for a session
the user had already ended.
The quit path already ASKED about those edits and saved them. What it never
did was close the windows afterwards.
Closing rather than deleting: WA_DeleteOnClose is set on every composer, so
close() is what frees them, and it lets ComposeWindow::closeEvent() run its
own draft handling on the way out. Iterating a copy of the list, since
closing runs the `closed` handler and that mutates m_composers.
Placed last, after every route that turns back has returned: reaching it means
the application really is quitting.
The test opens TWO composers, so a fix that closed only the last one cannot
pass it. Mutation-checked by removing the loop.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q2koFevoSxTLhfexJTZWQd
|
|
The reply family is disabled on mail that arrived at an account with no
send_command, behind a ribbon in MessageView naming the account and the key
to add. save_message is deliberately never disabled: it is the escape hatch
for exactly that case.
The ribbon is a WIDGET in the pane's layout, never markup inside the web
view. Composing HTML from configuration into the one document that renders
input from strangers is the wrong direction, and the header row is already a
widget for the same reason.
Compose itself is disabled only when NO account can send, and that state is
not warned about at startup: an installation with no send_command anywhere
is a valid read-only installation.
Every reply resolves through messageScopeFor(), not threadFor(): a thread
row means the one message its card shows. Replying to a thread is
meaningless; a reply answers a message. The context is built from the
DATABASE rather than the model, the rule Restore already follows, because a
row whose state has not been re-queried carries stale values and a reply
built from one would carry the wrong recipients.
The mail root crosses from the worker as its own signal. There was no route
for it at all: mailRootOf() is file-static in notmuchworker.cpp, and item
124 records that composing a destination from database.path writes into the
Xapian tree under a split index. The test uses NotmuchFixture::splitIndex(),
the only layout where the two accessors disagree.
A thread row's path is RELATIVE to the mail root while a message row's is
absolute, so the account lookup matched nothing and the reply family was
dead on mail from an account that could send. Found by the positive guard
test rather than the negative one, which passed throughout for the wrong
reason.
The quit path checks the failed-save case FIRST. In the ordinary case
nothing is lost by saving; there, saving is what is already not working, so
the dialog says plainly that quitting loses that text rather than offering a
save that will fail again. Both dialogs name the composers, and the ordinary
one asks once whatever the count, because three modals in a row is worse
than a coarse answer. Its wording says drafts already saved stay in the
folder, so Discard cannot read as 'delete my three messages'.
The Save loop holds QPointers, not raw pointers. A deleteLater() posted
while a nested exec() runs IS processed by that nested loop, measured in a
standalone program: the guard nulls before the modal returns. Closing a
composer while the quit dialog is up therefore freed a window the loop then
called saveDraftNow() on, crashing at the exact moment the application
promised to preserve that text.
A compose request that matches nothing clears itself and says so. It was
cleared only on a match, so a message deleted between selection and Reply
left the request armed for the session: Reply did nothing, and the next
ordinary click on that message opened a composer nobody asked for while the
pane stayed blank.
Forward carries the original's attachments, which the context has always had
a field for and nothing ever filled, and seeds its HTML toggle from
[compose] send_html. Only Reply seeds that from the original.
save_message keeps its filename inside the chosen directory and no longer
overwrites a file already there. The check was correct and untested: the
test asserted through Attachment's helpers rather than through the function
production calls, so deleting the containment check outright left it green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TvwDptMWxjqhbCmjxwcSZ2
|
|
A separate top-level QMainWindow, one per draft, several open at once. A
modal dialog cannot consult another message while writing, which is most of
what replying is, and taking over the message pane fights the pane that
exists to show what is being replied to.
No geometry save and no restore, deliberately. Under a tiling compositor
saveGeometry stores normalGeometry while the compositor owns the tile, so
the restore is correct and looks broken; a whole session went into that
once.
Autosave is a debounce AND a dirty check: an unchanged message writes no
file and provokes no sync. The check is on a fingerprint of the
OutgoingMessage, NOT on the built bytes as the plan drafted. GMime is given
a fresh Date and Message-ID on every build, so two builds of an unchanged
message never compare equal; a check on the bytes would have read as
working while writing a file, and an mbsync upload, on every debounce.
Checking before the build also skips the blocking build for the no-change
case, which is the common one.
closeEvent writes the draft when the buffer is dirty. Without it the
debounce is a hole rather than a delay: typing a paragraph and pressing the
window manager's X inside the interval loses it silently, since
WA_DeleteOnClose destroys the window immediately afterwards. A failed save
there does NOT refuse the close, because a window that will not close
because it cannot save is worse than one that closes having raised the
banner, which is what the quit path reads.
One flag covers a send, countdown included. An earlier revision had two,
and the narrower "committed and running" one reads as the honest thing to
guard a live SMTP conversation with. It is not: a close during the
countdown destroys the parented SendDialog, committed() never fires, and
the user pressed Send, watched a countdown, and believes the mail went. The
narrow flag was also written in three places and read in none.
A failed draft write raises a persistent banner rather than a modal or a
fading status line. A modal mid-sentence is hostile while the user is
typing, but the warning must survive until it is dealt with, because the
quit path escalates exactly this state to a dialog on the way out. An
account with no drafts folder reports success rather than failure: nothing
was written and nothing failed, and a false there would make the quit path
offer a retry no retry can change.
A failed send saves the draft before reporting. send() builds from the
widgets without saving, so the revision on disk is whatever the last
debounce wrote: edit, send, fail, close, and the user gets the older text
back, having watched their correction be sent.
A failed sent copy after a successful send is a modal, and never a send
failure: the message went, and reporting otherwise makes someone send it
twice. It is the one failure here that silently diverges what the recipient
received from what the local archive shows, and nobody discovers a missing
sent copy by noticing a line that appeared for a few seconds.
The formatting toolbar applies its edits through a QTextCursor document
replacement inside one edit block, NOT setPlainText as the plan drafted.
Measured against a real widget: setPlainText destroys the document's undo
stack and resets the cursor to 0, so every toolbar press would throw away
everything the user could undo. The cursor route leaves undo available,
collapses to a single undo step, and emits textChanged once. The seeded
quote is cleared off the undo stack afterwards, since it is not an edit the
user made and one Ctrl+Z on a fresh composer must not wipe it.
The per-send connect carries Qt::SingleShotConnection. MessageSender is a
long-lived member, so a bare connect accumulates a permanent receiver per
send and the second result runs both lambdas, the first still holding the
first message's bytes: it files a sent copy of the wrong message and acts
on a dialog it already destroyed. Covered by a test that sends, fails,
corrects and sends again; without the flag it segfaults in QLabel::setText
on the destroyed dialog. Its companion disconnect takes the specific
connection handle rather than every finished receiver on this object, so a
later observer cannot be killed silently.
The attachment warning states sizes with a decimal and a stepped unit.
Integer MB division read as "'x' is 0 MB. Many mail servers refuse messages
above about 0 MB." for any attachment_warn_bytes below a megabyte, in both
halves of one sentence.
The autosave timer is created before buildUi(), which is load-bearing:
buildUi connects every field to markDirty and seeding then fills those
fields, so markDirty runs during construction. Created afterwards it is a
null dereference on the first seeded field, which is every composer.
Twenty-six cases in test_mainwindow, each mutation-checked.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Three rows in every state so nothing reflows and the window never jumps. The
bar changes MODE rather than place: determinate while the countdown drains,
because a countdown has measurable progress, and indeterminate once the
command starts, because a send does not. That is the pairing item 134's
widget was extracted to serve.
The delay is where cancelling is safe and it is the only place it is.
Nothing has reached a server during the countdown, so Undo means genuinely
nothing happened; killing send_command once it runs leaves an UNKNOWN send,
which is worse than either clean outcome. Undo therefore disables itself the
moment the command starts, and stays visible while disabled: a control that
vanishes re-lays out the popup mid-operation, and a greyed Undo says why
cancelling is no longer possible where an absent one looks like it was never
offered.
The test for this asserts the NEGATIVE property, that committed() never
fires after Undo, including after the original countdown would have elapsed.
Asserting only that undone() fired would pass against a design that ran the
command and threw the result away, which is the whole failure the delay
exists to prevent.
Removing the close BUTTON is not the same as closing the code path, and the
first draft did only the former while its comments claimed otherwise. Escape
still reached QDialog::reject(), and close() during the countdown hid the
window while leaving the timer running, so the send committed with nothing
on screen and the only cancel control destroyed: measured, committed=1 on a
dialog the user had dismissed. A never-shown dialog did the same, since
close() returns early without reaching done(). That is CLAUDE.md's done(int)
trap in the one place it costs mail rather than state.
Dismissal is REFUSED before commit rather than treated as an implicit Undo,
at the user's decision: a close that silently means cancel overloads one
gesture with two meanings, while a refusal leaves Undo as the only way out,
which is what the popup's single control already says. done(int) refuses
pre-commit and forces Accepted after, closeEvent covers the never-shown
route done() cannot see, and Undo passes through both. Task 12 needs no
special entry point, since it closes after the send finishes and that is
post-commit by definition.
A refusal must not read as a hang, so the label says how to leave. Making
the hint silent was a mutation that SURVIVED, because the text was written
in two places and neutering one was masked by the other; extracting it to
one function exposed a real defect behind the wrong green, in that the next
tick overwrote the hint 100ms later and the refusal was effectively silent
anyway. It is held for 1500ms now, with a test that it survives a tick and
still releases.
setStage is public and Task 12 passes values into it, so it refuses to wind
back to CountingDown after commit rather than trusting its caller with an
invariant this class documents as inviolable; the label read "Sending in
0..." and the bar returned to determinate. Both m_committed guards carry
tests: removing them left the suite green, so two deliberate safety
additions rested on reasoning alone.
Every route out is asserted, per the rule that a test using close() while
the user uses Cancel covers one route of three: close() shown, close()
never-shown, Escape bare and with Shift and Ctrl, reject() direct, and Undo,
which must still work or the popup is a trap.
The status label is sized to the longest string it can hold in the current
language rather than to its content: Italian 'Rimozione della bozza...' is
longer than 'Removing draft...', and a label sized to content resizes the
popup between stages.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FXF741wz4SY7j5dqvAxMU5
|
|
Handlers are empty for now; this commit is the registration, so the three
coverage tests guard every later task rather than being satisfied at the end.
Two corrections to the spec, both found in the code rather than assumed. It
calls for a new top-level Message menu and one already exists, so these join
it; two menus named Message would be a defect. And it says every action
needs a binding, which item 132 changed while this was being planned:
save_message ships with no chord, since it is the rarely-used escape hatch
and menu reachability is now the rule that must hold.
reply_no_quote shares reply's icon and is added to the no-duplicate-icons
exception list for the same reason the five thread actions are: it never
reaches the toolbar, and a menu entry always carries its text. That list is
renamed menuOnlySharedIconActions, after the property that earns the
exemption rather than the tier that first needed it.
Bindings are provisional. The user intends to rework them, and Ctrl+Alt+R
for reply_no_quote is an imperfect fit since that tier elsewhere means a
wider scope rather than a variant.
The six labels went through a mnemonic pass that nothing enforced before.
Four of them collided inside the Message menu on first writing, and the
whole class was invisible to a green suite: Qt does not error on a duplicate
mnemonic, it cycles the highlight instead of activating, so the key simply
stops working. Item 57 had already decided this rule by rejecting a label
that would have collided, but it lived in prose and in one test's comment,
which is precisely why it was broken again here.
noMenuHasTwoEntriesSharingAMnemonic() enforces it now, scoped per menu since
a mnemonic resolves among the open menu's entries, and keyed on
QKeySequence::mnemonic() rather than on parsing & by hand, because && is a
literal ampersand and only Qt answers which key it will dispatch. Three
pre-existing collisions are a named freeze list rather than a silent fix or
a narrowed test: Alt+R three ways and Alt+S twice in Message, Alt+O in View.
Renaming entries a user has had in their fingers since 0.1.0 belongs to the
shortcuts rework, and the freeze is written as exact groups so a new entry
joining any of them still fails.
Two of the test's own design choices came from mutation checks that failed
for the right reason while reporting the wrong thing. Reporting collisions
as pairs was order-dependent, so a new colliding entry re-keyed a frozen
pair and the fresh defect read as "a frozen collision no longer happens";
matching frozen entries by whole string broke the same way, since a growing
group stopped matching its frozen text. It reports whole groups and matches
on menu plus key.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FXF741wz4SY7j5dqvAxMU5
|
|
MarkdownFormat, task 8 of the compose-and-send plan. Three free functions
over (text, selection start, selection end) returning the new text and the
selection that follows it, so the grammar is tested without a widget.
Three gaps in the plan's draft, each now pinned by a test checked against the
mutation that breaks it:
- QString::lastIndexOf INCLUDES the position it is given, so quoting with the
cursor at the end of a line found that line's newline and quoted the
FOLLOWING one. The draft's fixtures never placed a cursor there.
- A backwards selection was normalised but never tested, so the swap was
unguarded; a right-to-left drag is an ordinary gesture and Qt reports the
anchor after the cursor. normalise() now swaps and clamps in one place.
- A blank line inside a quoted range produced "> " with trailing whitespace,
which editors and mail clients strip anyway. It is written bare.
Two further defects came out of review:
- quote()'s selectionStart was unasserted for any block not starting at line
zero. Hardcoding it to 0 passed all nineteen tests, because the one test
naming the property quoted the first line, where right and wrong coincide.
A wrong selection there means a second press quotes a line the user never
selected, and a following Bold bolds the wrong text.
- A selection splitting a surrogate pair split the character across the
inserted tokens, leaving invalid UTF-16. Not reachable from the toolbar,
where arrow keys and mouse hit-testing both move in whole clusters, but
reachable by any code computing a position arithmetically. normalise()
nudges off a low surrogate; a collapsed cursor moves back on both ends,
since widening would turn "insert an empty pair here" into "wrap the emoji".
The buttons stack rather than toggle: a second Bold press gives ****this****,
and a second Quote press nests. That is what the spec specifies, and the
preserved selection exists so a second press can apply a SECOND token. A
toggle was built during this task at the user's request and reverted on
finding it contradicts the spec at two sites; it is recorded as backlog item
135, where the unanswered question is what replaces bold-then-italic.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LoaLBowZ6w1JNx6SEhDP1L
|
|
ComposeContext, task 7 of the compose-and-send plan. Address parsing,
recipient derivation, the References chain, subject prefixing and account
resolution, as free functions over values so they test without a painter.
Recipient derivation was designed from the spec rather than transcribed: the
plan's draft omitted it and its tests could not compile, calling
QVERIFY(config.load(path)) against a void return.
Six defects found in review, each pinned by a test checked against the
mutation that breaks it:
- Message-ids reached GMime bare, and GMime writes an EMPTY header for a bare
addr-spec rather than complaining. In-Reply-To and References both shipped
blank, so every reply would have arrived as an orphan thread with nothing
wrong to see locally. MessageBuilder now brackets on write, in the one place
that composes those headers rather than in each caller.
- internet_address_to_string was called with FALSE for the encode flag, so a
display name carrying a raw newline rendered with the newline intact. That
is a header-injection primitive.
- A reply to the user's own message addressed the user. It now goes to that
message's original recipients, mirroring their To/Cc split, which is what
the Sent view and a follow-up on unanswered mail need.
- A From parsing to no mailbox left To empty, reachable from real mail
("From: Mailer Daemon"). MessageBuilder treats an empty recipient list as
success, so the message would have been handed to the send command with
nobody to deliver to and filed in Sent looking sent.
- The References header was split on whitespace alone, so a client's
non-conformant "<a@x>,<b@y>" became one token and the bracket strip produced
the fabricated id "a@x>,<b@y".
- Reply and forward prefixes were recognised in English only, doubling every
AW:, SV:, WG: and Re[2]: a mixed-locale mailbox receives. Single-letter
spellings are deliberately excluded: with R: recognised, "R: report on Q3"
reads as a prefix and a genuine first reply threads nowhere.
The mailbox-only guard in parseAddressHeader survived its first mutation
check, because removing it still yields no recipients: the invalid GObject
cast makes GMime's own assertion return NULL. That is undefined behaviour
papered over by an assertion G_DISABLE_CHECKS compiles out, so the test now
asserts on the emitted critical rather than on the count. Registering the log
handler on a NULL domain catches nothing; the criticals carry "GLib-GObject"
and "gmime".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LoaLBowZ6w1JNx6SEhDP1L
|
|
MessageSender runs the configured command with the message on stdin and
judges the result by its exit status alone. Nothing here waits on the
event loop, so a send does not block the GUI thread; a 1.6MB payload was
probed through a reading stub without deadlocking the pipe buffer.
The command is split and passed to QProcess as a program and an argument
list, never through a shell. A test asserts that by giving the command
shell metacharacters and checking that the marker file a shell would have
created does not exist, so the property fails a mutation rather than
resting on a comment.
Four corrections to the plan's draft. splitCommand handles double quotes
only, so a single-quoted argument splits wrongly and the header now says
so. A crashing command delivers finished(11, CrashExit) and would have
been reported as "exited with status 11", so a crash branch was added. A
command that exits without draining a large stdin emits WriteError before
finished(), which the draft handled correctly and by luck, untested. And
an empty send_command is checked after trimming.
Two contract gaps found in review, both about what this class promises
rather than what it does.
The exactly-once guarantee covers the EMIT, not what a caller receives: a
long-lived sender plus a connect() inside each send accumulates
receivers, and the second result then runs the first send's lambda too,
filing a sent copy of the wrong message. The header now scopes the
promise and requires Qt::SingleShotConnection. The plan's Task 11 call
site already had that flag, sixty-nine lines below the connect and
outside anything a reader would see, so the plan gained a note where
someone retyping it will read it.
And destruction mid-send killed the command with no report, announced
only by a Qt warning: a live SMTP conversation abandoned, possibly
partially delivered, while the user believes it was cancelled. The
destructor now closes stdin, waits a bounded five seconds, and only then
kills. It emits nothing either way, because the outcome after a kill is
genuinely unknown and reporting "not sent" for a message that may have
gone out is the mailsync.sh mistake pointing the other way. Claiming
m_reported before kill() is what makes that true, since kill() delivers
finished(CrashExit), which would otherwise emit exactly that untruth.
No timeout on the send itself: killing a slow but working send is worse
than waiting. Task 10 owns the popup, and deliberately offers no cancel
after commit, so this class promises none either.
Also refreshes the translations Task 5 left out. That gap was invisible
because test_translations builds its rows from the .ts file, so a string
that never entered it is never asserted on.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QP2g3b3kuLx6AYFCNEz6UR
|
|
DraftStore::write() renders a built message into <folder>/cur with the
given Maildir flags, writing through QSaveFile and unlinking the previous
revision only after the new file is in place.
Two orderings here are load-bearing and both are covered by a test that
was checked against the mutation that breaks it.
The unlink runs only after the write has succeeded, so a failed save
leaves the previous revision intact rather than losing both. Provoking
that failure needs care: the plan's version used an unwritable path where
mkpath() fails and the function returns before reaching either the write
or the unlink, so a mutation moving the unlink up survived it. The test
uses an existing but read-only cur/ instead, where the failure lands at
the write.
And the size comparison stays ahead of commit() in the condition, because
QSaveFile::commit() returns true after a short write and renames the
truncated bytes into place: measured, write 4096 of 65536 with commit
reporting true and the file left in the listing. What leaves the
directory empty is the short-circuit returning before commit() is
reached, after which ~QSaveFile() discards the uncommitted scratch file.
Reducing the condition to !file.commit() looks like a simplification and
writes a truncated draft into cur/, where notmuch would index it and
mbsync would upload it.
Resolving the mail root stays the caller's job, per item 124; this takes
an absolute folder path and composes nothing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QP2g3b3kuLx6AYFCNEz6UR
|
|
Two silent failures on the path that produces bytes for other people.
A directory passed the attachment guard, because QFileInfo reports a
directory as existing and readable, and opening one read-only is legal. GMime's
base64 encoder then looped on read() returning EISDIR without advancing:
measured at 2.1 million failed reads in twenty seconds and still going. Since
build() runs synchronously from autosave on the GUI thread, dragging a folder
into a composer froze the whole application with the draft unrecoverable.
isFile() also excludes device nodes and FIFOs, which block the same way.
An unparseable recipient was dropped rather than reported. The old code
skipped anything that failed to parse and then only wrote the header if what
survived was non-empty, so a message whose only recipient was mistyped was
built with no To: header at all and reported success. With msmtp -t taking its
recipients from the headers, that is a message handed to the send command with
nobody to deliver to, and a copy filed in Sent that looks sent and reached no
one. A recipient the user typed and this cannot understand now stops the send,
the way a missing attachment already does.
The directory test carries a timeout deliberately: a regression there hangs
the binary rather than failing it. Two details make that work and the first
draft had neither. It must not join the worker, since a thread stuck in the
defect never returns and the join reproduces the hang instead of reporting it,
verified by reverting the fix: with the join the binary had to be killed at
150s with no verdict, without it it reports a FAIL and exits in 15s. The
result is shared through a shared_ptr so the leaked thread cannot write into a
returned stack frame.
Also: the no-address error names the account, since it matters once several
exist; messageId is assigned once on the success path rather than set early
and cleared on each failure, which is an invariant the next early return would
forget; and the Bcc comment now records that keeping the header stores the
blind list in plaintext in the sent copy and any draft, which mbsync syncs to
the server. That is accepted knowingly, and saying so stops a later reader
"fixing" it and silently breaking blind delivery.
One correction to the review that prompted this. The claim that
internet_address_list_parse returns a zero-length list rather than NULL did not
reproduce: measured on GMime 3.2 with a standalone probe, every garbage input
tried returned NULL, and no input was found producing a non-null empty list.
The length check is kept as defensive code and is documented as such rather
than as observed behaviour, since no fixture reaches it and a mutation on it
survives the suite. The defect itself was real and is what the test kills.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015muoUo2GdxmBDSp5vjYcbE
|
|
One built message serves three consumers: the autosaved draft, the bytes on
the send command's stdin, and the sent copy. A draft is therefore
byte-identical to what would be sent.
Three GMime defaults are wrong for this application and each is corrected
explicitly, because all three fail only on accented text and this user
writes Italian:
GMime encodes as iso-8859-1 unless told otherwise, so the subject carries an
explicit utf-8 argument. g_mime_text_part_set_text() encodes with whatever
charset is set when it is CALLED, so setting the charset afterwards produces
a part labelled utf-8 carrying latin-1 bytes; the content stream is built
directly instead. And neither Date nor Message-ID is generated unless asked
for, and a message without a Message-ID cannot be threaded by anything that
receives it.
Attachments are checked at build time rather than at attach time: a file can
vanish in between, and a message missing the thing it was written to carry
must never reach the send command.
An account with no address fails the build rather than producing a message
with an empty From. Config::account() returns a default-constructed Account
for an unknown key rather than failing, so without that guard a bad key
would produce silently malformed mail.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015muoUo2GdxmBDSp5vjYcbE
|
|
toInt() and toLongLong() return 0 on failure rather than the default, so a
typo in autosave_interval_ms produced a zero-interval timer. That timer is
restarted on every keystroke, so it would fire on the next event-loop pass
and turn a 30 second debounce into a Maildir write per keystroke, each one
uploaded by mbsync: exactly the behaviour the debounce exists to prevent.
This file already had the right shape in five places, a checked parse that
reports the bad value and keeps the default. The [compose] keys were the
only numerics skipping it. The interval is also clamped, since nothing
assigns a meaning to a zero or negative autosave.
quote_position now warns on an unrecognised value, matching sync_on_exit,
language and date_format; the only silent fallbacks in this file are for
absent keys rather than malformed ones. And a missing `sent` folder is a
notice rather than a problem, because the spec blesses that configuration
and a modal on every launch for a permanently correct setup is how users
learn to dismiss dialogs unread.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015muoUo2GdxmBDSp5vjYcbE
|
|
DraftStore needs the same filename generation moveMessages() already has,
and duplicating it would duplicate a correctness property rather than a
convenience: the comment records that carrying mbsync's ,U= infix across a
folder boundary produced 'Maildir error: duplicate UID' on real mail.
A pure move with no behaviour change, committed on its own so a bisect can
tell it apart from the feature that needed it. The function gains its own
tests, including the UID-infix case that previously had none.
|
|
The header still credited CMARK_OPT_SAFE after the .cpp comment and the test
were corrected, which left the wrong mechanism named in the file
MessageBuilder's author will actually read.
Three test weaknesses, each measured rather than assumed. The accented-text
test survived a SYMMETRIC latin-1 mutation, since the round trip cancels for
codepoints under U+0100, so it now carries a character latin-1 cannot
represent. The tasklist test asserted on the bare word "checked", which
ordinary prose would satisfy, and now asserts the attribute. And the
extension registration is wrapped in a function-local static: cmark-gfm's
registry has no once-guard, and this project has a worker thread, so the
first call racing itself would tear the registry rather than crash cleanly.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015muoUo2GdxmBDSp5vjYcbE
|
|
An account's ability to send IS its send_command's presence. Not a separate
receive_only key: with one key there is nothing to keep in step and nothing
to contradict, and a receive-only account is expressed by omission, which is
how one real account here is meant to work.
Startup validation follows the startup_query pattern, and is deliberately
asymmetric. A default_account that cannot send is warned about, because the
user named an account and expects mail to come from it. An installation
where NO account can send is not: that is a valid read-only installation,
and warning about it would train the user to ignore warnings.
Every [compose] key reads through value(key, default) rather than testing
contains(), because send_delay_ms = 0 is a real setting meaning 'send at
once' that a zero-test would mistake for unset.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015muoUo2GdxmBDSp5vjYcbE
|
|
CMARK_OPT_SAFE has had no effect since cmark-gfm made safe mode the
default; the flag is retained for API compatibility and the real
protection is that CMARK_OPT_UNSAFE is never set. Measured against
0.29.0.gfm.13: rendering with OPT_DEFAULT alone, with OPT_SAFE, and with
OPT_UNSAFE shows the first two suppress a script element and a
javascript: link while the third leaks both.
The comment credited the flag, which would have sent the next reader to
the wrong place, and the test could not tell the two apart: it would
have passed just as well with the flag deleted. What it has to guard
against is OPT_UNSAFE being introduced, so it now also asserts that
unsafe links are stripped, which is a protection this gets for free and
previously asserted nothing about.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015muoUo2GdxmBDSp5vjYcbE
|
|
The composer's body is markdown and the text/html part is generated from it.
cmark-gfm rather than plain cmark for autolink: under CommonMark a bare URL
in a mail body is not a link, and in mail it is expected to be clickable.
Three extensions are enabled and tables are deliberately not, since they
render badly across mail clients whoever generates them. Raw HTML in the
input is suppressed with CMARK_OPT_SAFE: the body is the user's own text,
but a body that can inject markup into its own generated HTML part is a
sharp edge with no upside.
The build needs TWO lookups. Only the core library ships a pkg-config file;
libcmark-gfm-extensions has none and is located with find_library, the way
notmuch already is. All three extensions live in that second library, so
finding only the first produces a build that compiles and silently renders
plain CommonMark.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015muoUo2GdxmBDSp5vjYcbE
|
|
The status bar's sync bar was a bare QProgressBar configured inline in
MainWindow, and item 123's send popup needs the same thing again. It also
needs the half MainWindow does not use: the popup drains a determinate bar
through its cancellable countdown and switches THE SAME widget to
indeterminate when send_command starts and the duration stops being
knowable. Building that inline a second time is what this removes.
BusyIndicator carries both modes. setBusy() resets the range as well as the
visibility, so the switch out of the countdown cannot leave the bar drawing
its last fraction, and setProgress() treats a total of zero as busy rather
than passing it through: setRange(0, 0) IS the indeterminate range, so a
zero total would otherwise hand the caller an animating bar while it
believed it had drawn an empty one.
Only the bar is extracted, not the status label the backlog row mentions
beside it. m_statusLabel has 34 uses across MainWindow for transient
messages, selection counts and sync phases; it belongs to the window rather
than to the indicator, and the send popup owns its own phase text.
The hidden-on-construction test needs a shown parent, which cost a mutation
to find. Measured against a standalone Qt program: a parentless widget
reports isVisible() false and isHidden() true whether or not hide() was ever
called, so both obvious assertions passed against a constructor with the
hide() deleted. What differs is WA_WState_ExplicitShowHide, and the
behaviour it produces appears only once a parent is shown, which is how the
status bar holds this widget.
All five mutations checked and killed: the zero-total guard, the range reset
in setBusy(), the value clamp, the show() in setProgress() and the hide() in
the constructor.
|
