| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
A text box could not carry a per-value context menu without parsing displayed
text back into structure, and the user did not want a text box. Each row now
holds its own value, its message index and its query, built from the parsed
message.
Every value label states Qt::PlainText. The QPlainTextEdit this replaced was
plain by design rather than by style: header values come from strangers, and a
QLabel guesses the format under AutoText.
|
|
chipRectAt() is the single source of a chip's geometry, used by paintEvent and
by the hit test, so the drawn chip and the clickable chip cannot drift.
The +N chip yields nothing: it stands for a list of tags rather than one, so
there is no single value a search could be built from.
|
|
One place for the query grammar behind every search surface, with no widget
involved so it is tested without a painter or a web engine.
extend() parenthesises both sides. The query bar may hold a hand-written
disjunction, and 'a or b AND c' binds as 'a or (b AND c)', which widens a
search meant to narrow it and reports nothing.
|
|
|
|
The same ~/.config/mailrules/rules.json mailctl reads, parsed here with
QJsonDocument and written atomically with QSaveFile. Fields this version
does not understand round-trip untouched, which is what keeps the format
neutral between the two tools.
Mutation-checked: removing the unknown-field write fails
unknownFieldsSurviveASave.
|
|
Items 70 and 69, the second folded into the first as item 70's own size note
predicted it should be.
The panes drew their state marks as font glyphs: U+1F4CE for an attachment and
U+2605 for a flagged thread, each with a fallback for a font that cannot render
it. Both fell back to "*", so on such a font a flagged thread and one carrying
an attachment were indistinguishable, which is a defect the fallback introduced
rather than prevented. What a mark looks like was also the desktop's decision
rather than this application's, and the panes are exactly where it should not
be: the user asked for the toolbar and menus to keep following their icon theme
while the panes stop.
Six marks now ship in assets/icons/marks/: flagged, attachment, passed, replied
and the two expander triangles. QIcon::fromTheme still resolves every toolbar
and menu icon and was not touched.
Licensing chose the shapes. The look came from a GPL3 icon theme, and this
project is GPLv2-only, which are incompatible: GPLv2's "no further
restrictions" clause bars shipping GPL3 assets in a v2-only work. The six were
drawn fresh in the same idiom instead, with no path data copied. The idiom is
generic: solid single-path silhouettes at 16x16 with no strokes.
They are compiled in as string literals rather than loaded from a .qrc.
src/CMakeLists.txt already records why resources belong to the executable: a
qrc in the static library registers itself from a global initialiser the linker
drops. The tests link the library, so a resource-based mark would be missing
exactly where it needs asserting. assets/icons/marks/ stays the editable
source.
One asset serves both palettes. Every payload paints with fill="currentColor",
which QSvgRenderer renders black rather than resolving, so Marks::pixmap
composites the wanted colour with CompositionMode_SourceIn. A mark then takes
the card's own pen colour and follows selection and the read/unread dimming
without a second variant to keep in step.
CardLayout reserves a rect per mark and CardDelegate paints into it. The marks
were glyphs inside the subject STRING, so their width came free from the text
metrics; as icons the geometry has to know they are there or the subject runs
underneath them. The expander pill had the same trap, its triangle being a
glyph in expanderLabel(), and now reserves that width explicitly.
Item 69's part: passed and replied were words in the tag strip and are marks
beside the subject now. The message pane's header carries the flagged and
attachment marks next to the subject, per the user's decision that the right
pane needs those two and only outside the message area.
A duplicate that no test caught is worth recording. Every geometry assertion
passed while a card showed passed as BOTH an arrow and a green tag chip: the
chip filter had no reason to know a mark had appeared. It was found by
rendering real cards to an image and looking at them. isDrawnAsAMark() is now
one list consulted by both PillTagsRole and MessageOwnTagsRole, since two
copies drifting apart is how a tag ends up drawn twice on one row and not at
all on another.
Fourteen tests: nine in test_marks, four in test_cardlayout, one in
test_threadlistmodel. Mutation-checked at four points, each failing a test: the
subject ignoring the marks, the flag not indenting the subject, the pill
forgetting the triangle's width, and the recolour composite removed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
The five real account colours are all mid-tone, because they were chosen as
CHIP fills with legible text on top, and three pixels of a mid-tone colour
barely registers beside a card's own background. The bar now lifts saturation
and lightness to a floor.
A floor, not a repaint: a colour already past it is returned untouched, so a
deliberately vivid choice is preserved and only the muted ones move. Hue is
never altered, because hue is the entire information the bar carries and a
shifted one would stop matching the account's chip and its dropdown swatch.
0.65 and 0.50 were chosen by rendering all five accounts as 3px bars against
both a dark and a light card background and looking at them. Higher pushed the
weakest of them, a green at S 0.33, toward a neon that no longer matched its own
chip; lower left it where it started.
accentLineColour had no test at all until now, which is how two earlier versions
of it shipped wrong: one blended toward Base until it WAS the background, the
other passed a chip colour through unchanged. test_carddelegate covers the lift,
the floor's idempotence, hue preservation across all five accounts, and the
fallback for a thread with no account tag.
|
|
Split from the delegate deliberately. A delegate needs a live painter and an
exposed view, which is what makes delegate tests fragile: viewport()->render()
returns a blank image in several ordinary situations, and a probe reporting no
ink is likelier broken than the code it tests. Every geometric claim about a
card is made here, where a test is a function call.
Three lines at a uniform height, so setUniformRowHeights(true) survives. Indent
caps at depth 4 with qMin rather than a branch, so depth 5 and depth 50 land in
the same place. The date is measured before the sender, so a long sender elides
instead of painting over it.
Two traps handled that a first pass gets wrong. QRect::right() is inclusive, so
the right edge is carried as an exclusive one and everything sized from it lands
where the padding constant says rather than a pixel short. And QFont::pointSizeF
returns -1 for a font set in pixels, which qt6ct does, so smallFont branches on
which unit the font actually carries instead of silently returning the card's
own size.
|
|
ctest sets no QT_QPA_PLATFORM, so the suite's verdict depended on how it
was invoked: green for anyone exporting `offscreen` by hand, red under
ctest in the same tree. That cost a wrong diagnosis before the cause was
found, an unrelated change blamed for a failure that reproduces on a
clean checkout.
The Wayland warning blames a missing transientParent, which is
misleading. Instrumenting the test showed the popup viewport arriving as
1278x0. The zero height is why the grab returned a null pixmap, but the
width is the worse half: this test sizes a line edit to 550px and exists
to prove a description survives a popup that size, so a working grab
would have measured a popup twice that wide and passed while proving
nothing. Offscreen gives 548x40, the geometry the test means.
Pinning the platform is therefore the correct fix rather than an opt-out.
It is set in add_qtmaildir_test so it covers every test, including the
two others that create widgets, and anything added later.
The test now also asserts its own geometry, since CMake only governs
ctest and the binary is often run directly. A bare !shot.isNull() said
nothing useful; it now reports "popup viewport has no height (1278x0)"
or names a popup too wide to be the case under test. Both guards were
verified by mutation.
|
|
An empty right pane said nothing, and multi-select made it a routine
sight. It now carries the wordmark, thread counts that run their query
when clicked, and a sync line that appears only when something needs
attention.
Rendered into the existing web view as a third document shape, so there
is one document path and one set of security rules. The brand palette is
a deliberate exception to deriving colours from the desktop theme, since
a logo is brand rather than chrome; the theme still picks which of the
two sets is used.
Counts refresh when the pane is about to show rather than in the
background: one goes stale the moment a tag is edited, and refreshing one
nobody is looking at is work for nothing. A generation counter discards a
superseded reply, and a late answer cannot repaint over an opened thread.
The helper lines are real links because JavaScript is off in this
profile. The handler is gated on the placeholder actually being
displayed, so the same URL inside a message body is dropped: a stranger's
mail must not drive the thread list, even to run a harmless query.
Three defects found while building, all silent:
- Every CSS percentage was invalid. QString::arg does not collapse "%%"
into "%", so the document carried "50%%" and the browser dropped each
declaration holding one, disabling the mask, the glow and both radial
gradients while still rendering something plausible. Substitution is by
named token now, which cannot collide with a percent sign.
- A geometry probe endorsed the layout while that was live, because it
measured only properties without percentages.
- The font test passed against a build with one face missing, since the
other satisfied both of its checks on its own.
The mockup's light values needed correcting against a real pane: the grid
vanished at a 2% luminance step on white, and the glow subtracts light
there rather than adding it, washing the pane. Strength only, not hue.
|
|
The user's cron runs mailsync.sh every ten minutes, so mail arrives and
tags change while the window sits idle, and nothing here noticed. The
script already holds an flock for the whole run, so that lock is the
signal: no status file is needed, and a kernel lock cannot go stale
because it dies with the process holding it.
The observation method is the part that matters, and two of the three
plausible ones are wrong. Both were probed on Linux 6.18 before any of
this was written:
- flock -n acquires in order to test, so polling every two seconds
would open a window every two seconds in which a starting
mailsync.sh is refused the lock and exits 75. It would cause the
very skips the script reports.
- fcntl(F_OFD_GETLK) never acquires and looks ideal, but reports
UNLOCKED against a lock held by flock(2): separate lock namespaces
in the kernel, which cannot see each other. A silent false negative.
- /proc/locks is a pure read. It observes flock(2) correctly, and
since it takes no lock at all it can never contend with the Xapian
write lock notmuch new holds during the same run. Confirmed: 200
reads left the lock table unchanged and this process holding
nothing.
SyncMonitor keeps the parsing separate from the polling so the parsing
is testable, and it reports Unknown rather than Idle where /proc/locks
cannot be read: "no sync is running" is the claim that would let the
window quit, so it must never be guessed. Verified against a real flock
end to end, not only against synthetic content.
It reports rather than refreshes. runCurrentQuery() clears the undo
stack, the selection and the message pane, which is right for a query
the user typed and hostile for one a cron timer fired: it would discard
undo history and close the thread being read up to six times an hour,
with no action from the user.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Five hardcoded tags were the only ones reachable from the UI: archive,
delete, spam, flag and toggle_unread. For an application whose purpose is
organising mail by tag, applying any other one meant leaving for a
terminal. Item 26 of the usability backlog, raised by the user asking how
to add a tag and finding they could not.
One dialog rather than separate add and remove actions, at the user's
choice: filing something under a new tag while dropping inbox is one
thought, not two. Type tags to add or remove, comma separated, or clear a
checkbox to drop a tag already on the selection without retyping its
name.
Both fields complete against the tag list MainWindow already holds for
the query completer. Completion is a guard against typing shoppping
beside shopping, never a whitelist: inventing a tag is the entire point,
so any valid name goes through whether or not it exists yet.
Tri-state checkboxes carry the multi-thread case, and are where the risk
is. A tag on some selected threads shows partially checked, and leaving
it alone changes nothing; the opposite reading would silently tag threads
the user never looked at. A tag already on every thread and left checked
is likewise not a change and is not sent as one.
Tag names are validated before anything is applied, through a free
function so the rules are testable on their own. Empty, a leading dash
(notmuch's CLI reads it as removal, making such a tag a trap), whitespace
and control characters are refused by name and reason. Nothing is applied
until the whole set passes, since the user cannot tell which half of a
partial change landed.
TagDialog is pure UI: handed the vocabulary and the current state,
returning two lists, contacting no worker. That is what lets its fifteen
tests run without a notmuch database. Integration is a single call to the
existing tagSelected(), so undo, the optimistic model update, the
combined multi-row query and the completer refresh for a brand-new tag
all come for free.
One test assumption was wrong and the code was right: a case asserted
that QStringLiteral("null\0byte") truncates at the null and reads as
empty. It does not, so the null is caught as a control character. The
test was corrected rather than the validator.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Prefix completion only so far: the token under the cursor, bounded by
whitespace or an opening parenthesis rather than by the start of the line.
|
|
Spelled out per row, tags ran to 500 pixels of largely repeated text and
took most of the thread list's width. The column is gone; tags render as
coloured chips split by what they actually mean.
An account tag says which mailbox a thread arrived in, and draws as a
chip in front of the subject. A functional tag says what state a thread
is in, and those fill one row under the message pane. One row keeps the
message area from shifting between threads with different tag counts, so
whatever does not fit collapses into a +N chip that names the rest in its
tooltip.
TagColors resolves a colour by exact tag first, then by top-level prefix,
so a single "shopping" entry covers shopping/amazon and shopping/nike
while shopping/amazon can still override its own. That matters at 96
tags. Built-in defaults cover the usual state tags, and anything left
unconfigured falls back to a hash of the name, stable so a chip does not
change colour as the list scrolls.
|
|
Clicking a thread left the pane blank. Two independent bugs, both from the
same false premise: that setHtml() navigates to the base URL it is given.
It does not. setHtml() navigates to a data: URL carrying the markup and
applies the base URL afterwards, purely as the document's origin. Verified
empirically on Qt 6.11.
Built on that wrong assumption were:
- MessagePage::acceptNavigationRequest compared the navigation's URL
against documentUrl() and rejected everything else, so the document load
was refused. It now accepts a typed main-frame navigation, which is one
we initiated ourselves.
- RequestInterceptor exempted exactly the qtmaildir: base URL and denied
everything else, so the data: document load was blocked too.
The interceptor fix is scoped to ResourceTypeMainFrame rather than allowing
the data: scheme outright. A blanket allow would have been a real hole: a
message body can write <img src="data:..."> or an iframe, and the existing
dataSchemeBlocked test in test_interceptor.cpp was right to fail when that
was tried. Sub-resource data: URLs remain denied.
Note this was never working. The drafted version had the same defect in a
different spelling (it compared url.scheme() rather than the whole URL, and
would have rejected the data: navigation just the same), and task 11 shipped
with no runtime test to catch it. test_messageview.cpp now pins all three
facts: the document loads, its text reaches the page, and a data: image
inside a hostile body stays blocked.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Wires the worker thread, thread list, message pane, sync process and undo
stack together, and replaces the placeholder main() with real startup:
custom URL schemes registered before QApplication, a libnotmuch ABI check,
and config loading.
Four fixes against the drafted version:
- onWorkerError() only set a status label. Its own comment elsewhere
claimed it reverted the optimistic update, and the spec requires that;
it did not, so a rejected write left the list showing a tag the database
never received. The pending change is now recorded and rolled back, and
a confirmed tagsApplied clears it so a later unrelated error cannot undo
a write that succeeded.
- runCurrentQuery() cleared the model but left the undo stack pointing at
rows that no longer exist. Undoing after a new query would have written
to the database while the visible list stayed put. The stack is cleared
with the model.
- m_currentMessages was assigned on every thread load and never read.
Removed.
- buildUi() connected sync output to m_syncLog and errors to m_statusLabel
before either existed. Both are constructed before the wiring now.
cidPrefix generation lives here, this being its only producer in the
application, and is pinned by tests: it must never contain '!' and must be
distinct per message, which are the invariants the cid: namespacing rests
on. A second test holds registeredActionNames() against
KeyMap::knownActions(), since those two hand-maintained lists drifting
either way silently breaks a user's key binding. Mutation-verified that
dropping an action fails the test by name.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Off-the-record profile, JavaScript off, deny-by-default interceptor, and a
page subclass that hands link clicks to the system browser so a message can
never navigate the pane.
Honours the obligation task 5 recorded: the interceptor trusts exactly one
qtmaildir: URL and fails closed otherwise, so setHtml() and setDocumentUrl()
must agree or the pane renders nothing. Rather than pairing those calls at
each site, every load goes through one setDocument() and the URL comes from
a single documentUrl() accessor. Verified against the real interceptor that
this URL is allowed while siblings, subpaths, remote and file: are not.
Three fixes against the drafted version:
- showError() called setHtml() with a base URL but never setDocumentUrl(),
so an error card would have rendered blank. Now impossible to repeat.
- clear() and showError() left the previous thread's inline parts in the
scheme handler and its cids in the interceptor. Both now empty the policy,
so no thread's parts outlive it.
- MessagePage trusted the whole qtmaildir: scheme for typed navigations,
which is the same blanket-trust mistake task 5 removed from the
interceptor. It now matches the exact document URL.
The parts-flattening is extracted into buildThreadCidMap() so it can be
tested without a live profile, and a cidPrefix containing '!' is sanitized
rather than trusted, since Q_ASSERT is compiled out in release and this map
decides which bytes a message can name. The sanitizer escapes '_' before
replacing '!', because a plain replace would map "m0!x" and "m0_x" onto one
key and merge two messages, which is the very collision the namespacing
exists to prevent. Mutation-verified: the naive replace fails the
distinctness test, and dropping the sanitizer trips the assert.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Runs the configured sync script through QProcess, merging stdout and
stderr into one log so a failing mbsync run has something to show. The
script is never run through a shell: the command is a config value, and
splitCommand keeps its arguments literal.
Two fixes against the drafted version:
- start() no longer calls waitForStarted(). It blocked the UI thread for
up to five seconds, which contradicts the spec's requirement that the UI
stay usable during sync, and it swallowed launch failures into a bare
false return. A missing script now surfaces asynchronously through
errorOccurred as finished(false, -1) with an explanatory log line, so
the spinner cannot hang with nothing to explain it.
- Removed a double-emit guard I had added on the assumption that QProcess
follows errorOccurred(FailedToStart) with finished(). Verified it does
not: FailedToStart is emitted instead of finished, never before it. The
guard was dead state and the comment justifying it was wrong.
Also corrects the sync interval throughout: the user's cron runs every 10
minutes, not hourly. The shorter interval strengthens the flock rationale
rather than weakening it, since collisions with a manual sync are that
much more likely.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
QAbstractTableModel over query results, appended in batches so a large
query paints its first screenful immediately. Tag changes apply locally
for optimistic UI; reverting a failed write means calling applyTagChange
again with added and removed swapped, which the round-trip test pins.
Two additions to the drafted version:
- A ThreadIdRole, so a view's QModelIndex maps back to the thread id the
worker speaks without every caller reaching around the model.
- data() checks its own row and column bounds. Qt will not hand out an
out-of-range index and invalidates persistent ones on reset, so this is
unreachable defence rather than a live path; the test says so instead of
pretending to cover it.
Verified by mutation that the empty-batch guard, the ThreadIdRole, and the
full-row dataChanged range each fail exactly one test when removed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Owns the only notmuch database handle. Queries run read-only and emit
threads in batches of 200 with a generation counter so the UI can discard
superseded results. Tag mutation closes the read-only handle, opens
read-write, applies, and closes, holding the process-wide write lock for
milliseconds rather than blocking a concurrent `notmuch new`.
Tested against a throwaway database built in a QTemporaryDir, superseding
the spec's original "no unit test" position: applyTags is the only code
here that writes to a notmuch index. The fixture never touches ~/Mail or
~/.notmuch-config.
Two fixes against the drafted implementation, both caught by mutating the
code and confirming exactly one test failed:
- loadThread conflated "no query given" with "query matched nothing in
this thread", so filtering a thread down to zero matches rendered every
message expanded. Tracked with an explicit haveMatchSet flag.
- applyTags now documents why a stale message id must skip rather than
abort: notmuch_database_find_message reports SUCCESS with a null message
for an unknown id, and the live ids alongside it still need tagging.
Note for fixture authors: notmuch synchronizes maildir flags with tags at
index time, so a file named `...:2,S` is indexed without the unread tag no
matter what [new] tags requests. Unread fixture messages go in new/.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
HtmlBuilder renders parsed messages (and whole threads, as one document,
so newsletter threads don't spawn one Chromium process per message) into
the HTML string the web view loads. Plain text is escaped and quote lines
marked; the cid: rewrite is namespaced per message ("<prefix>!<id>") so
two thread messages sharing a Content-ID don't collide.
Hardened namespaceCids beyond the initial sketch after attacking it:
handles unquoted cid: attribute values, background=/poster= (not just
src/href), and CSS url(cid:...) in both style="" attributes and <style>
blocks, all case-insensitively. Replaced the greedy [^"']+ capture with
per-quote-style alternation so two cid: refs on one line can't bleed into
each other.
CidSchemeHandler serves cid: requests from the thread's inline-parts map,
keyed by the same namespaced string, replaced wholesale per thread.
|
|
|
|
|
|
Accounts use [account.work] rather than [account/work]: QSettings' INI
backend treats "/" as its own hierarchical group separator, so a literal
slash in a section header parses as a nested group and trips
QSettings::FormatError, silently breaking childGroups() enumeration. A
dot carries no such meaning and keeps the format flat.
Saved-query order is alphabetical (QSettings::childKeys() sorts), not
file order; documented in code and tests rather than left to a false
assumption.
|
|
Maps key sequences to action name strings, with hardcoded vim-style
defaults and QSettings-based [keys] overrides. Unknown actions and
unparseable sequences are collected as warnings rather than treated
as fatal, so a typo in the config cannot silently misbind or crash.
Note: QKeySequence::fromString() on Qt 6.11 does not return an empty
sequence for unparseable input (e.g. "NotAKey++") -- it returns a
non-empty sequence whose toString() is empty. Detection uses that
instead of isEmpty().
|
|
|
