| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Item 80. A rule with eight From conditions left the list showing about
one and a half rows.
The list was added with stretch 1 and the form below it with none, which
looks decisive and is not: a stretch factor only distributes space above
each widget's minimum, and the form's minimum grew with every condition
row, so each row came straight out of the list. The builder asked for
120px with one row and 414px with eight.
A QSplitter now divides the list from the editor, so the balance is the
user's and is saved beside the column widths, and the condition rows sit
in a QScrollArea capped at 190px so the editor cannot grow without bound
however the splitter is set. The scroll area is what text mode hides;
hiding the builder inside it would leave an empty frame.
Three measures were tried in the test before one told the bug and the
fix apart, and two passed against broken code: the dialog's
minimumSizeHint does not track form rows and read 580 either way, and a
qMin against the scroll area's own hint read small whether or not the
cap was set, since an uncapped maximumHeight is QWIDGETSIZE_MAX. What
survives mutation is the editor pane's minimum inside the splitter, plus
the cap read directly, and both are asserted. A row's size hint is
invalid until the event loop runs, so the test calls processEvents after
selecting a rule or it measures the same height twice.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
The geometry was saved from closeEvent, and neither dialog button sends
one: Cancel calls reject(), Save calls accept(), and only the window
manager's X button produces a QCloseEvent. So the size and the column
widths were kept for the one route out of three that a user almost never
takes, and a resize followed by Cancel came back forgotten.
The save moves to a done(int) override, which both buttons funnel
through and which QWidget::close() also reaches.
The test that covered this passed against the bug because it asserted
with close(). It now drives all three routes rather than trusting one to
stand for the others, and shows the dialog before the close leg:
close() on a widget that was never visible returns early without
reaching done(), so that assertion would otherwise prove nothing.
Both traps recorded in CLAUDE.md, since neither is specific to this
dialog.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Item 75. saveGeometry() and the rule list header's saveState() go to
uistate.conf under keys of their own, written on closeEvent so a size
survives Cancel as well as Save. The 760x520 resize stays as the
first-run fallback.
The backlog's approach was wrong on one point and a test caught it. It
said to drop the resizeColumnToContents calls once a saved header state
exists, which fixes the restore and leaves the original defect standing:
with nothing saved, a width the user had just dragged was still
discarded by the next add or delete. Each column is instead auto-sized
once, on its first fill, after which the width belongs to the user
however it was set. Two flags, because the count column is filled later
by a reply from the worker.
The window stays a QDialog. Making it a top-level window needs the
unsaved-edit story that being modal currently sidesteps, and that is its
own decision rather than part of this item.
Both tests redirect XDG_STATE_HOME as well as XDG_CONFIG_HOME, so they
cannot write the real uistate.conf. The geometry is asserted on the
stored value rather than the reopened frame, per item 46: the offscreen
platform does not honour a resize.
Also corrects setFolders' doc comment, which still described the folder
list as coming from Config.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
|
Ticking "Edit as text" was a one-way trip: the only way back to the rows
was closing the dialog and reopening it. The checkbox was parented to the
builder widget and sat on the match row, and switching to text mode hides
that widget, so the toggle disappeared along with the rows it governs.
Move it to the query row, which is visible in both modes.
The existing tests all passed against this, because they drove the toggle
through setChecked and then asserted on the checked STATE. A hidden
checkbox reports its state perfectly well, so every one of those
assertions held while the widget was unreachable. The new test asks the
question that matters, whether the toggle would be on screen, and it uses
isVisibleTo since nothing is isVisible on a dialog that was never shown.
Worth recording how close the mutation check came to endorsing this too.
Reparenting the checkbox alone left it in the query row's layout, so it
stayed visible and the test still passed. Only restoring the full shipped
shape, parent and layout together, reproduced the fault and failed the
test. A mutation that does not reproduce the original bug proves nothing
about the test that is meant to catch it.
The spec's layout sketch carried the same error and is corrected, with
the reason, so the next reader does not reintroduce it.
|
|
|
|
Leaving text mode with a query the builder cannot represent has to
refuse, since there are no rows that mean that query. It announced this
with a QMessageBox, which made the branch untestable: a modal blocks the
test that reaches it, so the one path that can strand a user was the one
path shipping unverified.
Say it in the warning label the dialog already has instead. That also
suits the moment better, since it does not interrupt someone mid-edit to
tell them something the label can hold while they keep typing, and it
matches how the tag dialog reports a bad tag.
Returning to the rows now calls showWarnings(), because the refusal
writes into the same label the load warnings use and a stale complaint
would otherwise outlive the query that caused it.
The test drives the refusal and the recovery, and asserts the warning
appears and then clears. Verified by mutation: letting the checkbox clear
regardless fails it.
warningTextForTest uses isVisibleTo rather than isVisible. Every child of
a dialog that was never shown reports isVisible() false, so the seam
would have reported no warning whatever the label held, which is a probe
that cannot see the thing it checks.
|
|
|
|
Opening the tagging rules dialog and pressing Save destroyed the first
rule in the list, without any editing. The rule lost its query and its
tags, then vanished entirely on the next load, since a rule with an empty
query is dropped as malformed.
Reproduced against the released tag rather than the branch, in a
throwaway worktree at 9585674 with a two-rule fixture: constructing the
dialog and running its save path left one rule of two.
onSelectionChanged blocked signals for the note widget only, while
m_enabled::toggled two lines later reached applyEditsToCurrentRule, which
writes every field from widgets the loader has not filled yet. The
existing comment there shows the hazard was known for one widget and not
extended to the other.
The fix landed with the builder work: the reloading flag now covers the
whole load, and switchingRulesDoesNotLeakRowsBetweenThem is the
regression test, verified by mutation to fail without the guard.
The live rules file had one casualty, the account rule sitting first in
the list, with both its query and its tags empty while every sibling was
intact. Restored from the shell backup that the earlier migration kept
and verified through mailctl's own reader. The rule had stopped tagging,
but only one message had arrived meanwhile; that message is now tagged
and the account is complete again at 14969 of 14969.
|
|
Selecting a rule now parses its stored query and rebuilds the builder
rows from it, and a row edit compiles back onto the query line and into
the working copy.
Populating the form was already able to write the rule just loaded over
whichever rule is current: m_enabled's toggled runs
applyEditsToCurrentRule while m_query still holds the previous rule's
text, which emptied the first rule's query on open. The existing
m_reloading guard now covers the whole load rather than one signal
blocker on the note, which also covers the combo boxes rebuildRows
populates.
|
|
|
|
Edits land on a working copy and reach the file only on Save. The
dialog never opens a notmuch database of its own: it publishes the
queries it wants counted and MainWindow runs them through the worker,
because the worker owns the only handle.
Two departures from the drafted version, both of which lost edits.
QPlainTextEdit has no editingFinished, so the note reached the working
copy only for whichever row was current at Save; it is driven from
textChanged instead, with the selection handler blocking the signal so
loading a rule cannot write itself back over the one now current. And
reloadList()'s setCurrentItem emits currentItemChanged, so New and Copy
repopulated the form from m_working before the pending edit had been
flushed into it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
