aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-13 16:15:57 +0200
committerDanilo M. <danix@danix.xyz>2026-08-13 16:15:57 +0200
commit3b771d03f2df572de29af3156817de3cb7ef6bef (patch)
treeefd17e55320e472ee27160d935034afe31add6e4
parenta52e4e41c2bc8fc8eb23c4bc99f9b8724212bd23 (diff)
downloadqtmaildir-3b771d03f2df572de29af3156817de3cb7ef6bef.tar.gz
qtmaildir-3b771d03f2df572de29af3156817de3cb7ef6bef.zip
feat(rules): the rules window keeps its size and column widths
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>
-rw-r--r--CHANGELOG.md15
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md22
-rw-r--r--src/tagrulesdialog.cpp93
-rw-r--r--src/tagrulesdialog.h39
-rw-r--r--tests/test_tagrules.cpp99
5 files changed, 255 insertions, 13 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 03b5046..0637cd9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -19,11 +19,16 @@ point at which they are stable.
stays visible and is what gets saved, so a rule the builder cannot show
opens as text and still works. Opening a rule without editing it leaves the
stored query untouched.
-- A Folder condition picks from your configured accounts rather than being
- typed. A folder path with a typo matches nothing and notmuch reports no
- error, so the rule would simply never fire. The list stays editable, so a
- folder that is in the rules file but not in your config still opens and
- still saves.
+- A Folder condition picks from a list of every folder in your Maildir rather
+ than being typed, Drafts and Sent included, not only the top of each
+ account. A folder path with a typo matches nothing and notmuch reports no
+ error, so the rule would simply never fire. The list is read from the tree
+ on disk, so a folder that exists but has no mail in it yet is still offered.
+ It stays editable, so a folder in the rules file that is no longer on disk
+ still opens and still saves.
+- The tagging rules window remembers its size and the widths of the rule
+ list's columns. A column you widen also survives adding or deleting a rule,
+ which previously reset it.
### Fixed
diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
index 00a7f76..d55336d 100644
--- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
+++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
@@ -131,8 +131,8 @@ taking that too literally.
| 72 | No khard/khal integration | workflow | ? | open, unspecified; the user places it after send, so v2 at the earliest |
| 73 | This backlog is past four thousand lines | maintenance | S | open |
| 74 | "Searching..." keeps claiming a query is running while rows are already arriving | feedback | XS | open; cause measured 2026-08-11, the delay itself is the cold page cache and is not fixable here |
-| 75 | The tagging rules window forgets its size and its column widths | persistence | S | open; follows item 44 |
-| 76 | Every field in the rules dialog is free text, so a rule is easy to get wrong | workflow | M | open; design approved 2026-08-13, see `specs/2026-08-13-rule-builder-design.md` |
+| 75 | The tagging rules window forgets its size and its column widths | persistence | S | **done** 2026-08-13 on `rule-builder`, unreleased. The window-kind question is left open, see below |
+| 76 | Every field in the rules dialog is free text, so a rule is easy to get wrong | workflow | M | **done** 2026-08-13 on `rule-builder`, unreleased. See `specs/2026-08-13-rule-builder-design.md` |
| 77 | No way to see what a rule would collect, in the thread list | workflow | S | open; the dialog counts matches, it cannot show them |
| 78 | No way to build a rule from something visible in a message | workflow | M | open; wants 76 first, so the created rule lands in a form that can hold it |
| 79 | Opening the rules dialog and saving destroys the first rule | defect | XS | **fixed on `rule-builder`** 2026-08-13, unreleased. Shipped in 0.16.0; damaged one real rule, repaired by hand |
@@ -4764,6 +4764,24 @@ the test asserts on the saved value rather than on the resulting frame.
**Size: S.**
+**Done 2026-08-13** on `rule-builder`, unreleased. `saveGeometry()` and the
+list header's `saveState()` go to `tagrules/geometry` and `tagrules/header` in
+`uistate.conf`, written on `closeEvent` so a size survives Cancel as well as
+Save; `resize(760, 520)` stays as the first-run fallback.
+
+**The approach above 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 no saved
+state, a width the user had just dragged was still discarded by the next add or
+delete. The rule shipped instead is that each column is auto-sized ONCE, on its
+first fill, after which its 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 **popup or primary window** question was put to the user and deliberately
+not taken: it stays a `QDialog`. Reopening it needs the unsaved-edit story that
+being modal currently sidesteps, and that is its own decision rather than part
+of this item.
+
## 76. Every field in the rules dialog is free text, so a rule is easy to get wrong
**Observed.** A rule is written by typing into four line edits, and the user
diff --git a/src/tagrulesdialog.cpp b/src/tagrulesdialog.cpp
index 75a9cbe..61f2c21 100644
--- a/src/tagrulesdialog.cpp
+++ b/src/tagrulesdialog.cpp
@@ -20,8 +20,11 @@
#include <QButtonGroup>
#include <QCheckBox>
+#include <QCloseEvent>
#include <QComboBox>
#include <QDialogButtonBox>
+#include <QDir>
+#include <QFileInfo>
#include <QFormLayout>
#include <QHBoxLayout>
#include <QHeaderView>
@@ -31,10 +34,13 @@
#include <QPlainTextEdit>
#include <QPushButton>
#include <QRadioButton>
+#include <QSettings>
#include <QSpinBox>
#include <QTreeWidget>
#include <QVBoxLayout>
+#include "mainwindow.h"
+
namespace {
/// Columns of the rule list.
@@ -72,6 +78,9 @@ TagRulesDialog::TagRulesDialog(QWidget *parent)
: QDialog(parent)
{
setWindowTitle(tr("Tagging rules"));
+ // The fallback for a first run. restoreUiState() overwrites it when a
+ // size was saved, and is called at the end of this constructor because
+ // the header state cannot be restored before the columns exist.
resize(760, 520);
m_rules.load();
@@ -244,6 +253,72 @@ TagRulesDialog::TagRulesDialog(QWidget *parent)
reloadList();
showWarnings();
+
+ // Last, and after reloadList(): a header state cannot be applied before
+ // the columns it describes exist, and reloadList is what fills them.
+ restoreUiState();
+}
+
+/// Reads the window size and the rule list's header layout back.
+///
+/// The same file MainWindow uses, under keys of its own. Machine-written
+/// state, never the hand-edited config: a column width is not something
+/// anyone edits by hand, and mixing the two puts a blob in a file the user
+/// reads.
+void TagRulesDialog::restoreUiState()
+{
+ QSettings state(MainWindow::uiStatePath(), QSettings::IniFormat);
+
+ const QByteArray geometry =
+ state.value(QStringLiteral("tagrules/geometry")).toByteArray();
+ if (!geometry.isEmpty())
+ restoreGeometry(geometry);
+
+ const QByteArray header =
+ state.value(QStringLiteral("tagrules/header")).toByteArray();
+ if (!header.isEmpty()) {
+ m_list->header()->restoreState(header);
+ // Counts as the one auto-size each column gets, so the restore is not
+ // immediately overwritten. reloadList() runs before this in the
+ // constructor and has already sized the first two; the count column
+ // has not been filled yet, and would otherwise resize over the
+ // restored width as soon as the first counts arrived.
+ m_columnsSized = true;
+ m_countColumnSized = true;
+ }
+}
+
+void TagRulesDialog::saveUiState()
+{
+ QDir().mkpath(QFileInfo(MainWindow::uiStatePath()).absolutePath());
+ QSettings state(MainWindow::uiStatePath(), QSettings::IniFormat);
+ state.setValue(QStringLiteral("tagrules/geometry"), saveGeometry());
+ state.setValue(QStringLiteral("tagrules/header"),
+ m_list->header()->saveState());
+}
+
+/// Saves on close rather than on accept, so a size the user chose is kept
+/// whether they pressed Save or Cancel. The window's shape is not part of the
+/// edit being confirmed.
+void TagRulesDialog::closeEvent(QCloseEvent *event)
+{
+ saveUiState();
+ QDialog::closeEvent(event);
+}
+
+int TagRulesDialog::columnWidthForTest(int column) const
+{
+ return m_list->columnWidth(column);
+}
+
+void TagRulesDialog::setColumnWidthForTest(int column, int width)
+{
+ m_list->setColumnWidth(column, width);
+}
+
+void TagRulesDialog::reloadListForTest()
+{
+ reloadList();
}
void TagRulesDialog::showWarnings()
@@ -286,8 +361,15 @@ void TagRulesDialog::reloadList()
auto *item = new QTreeWidgetItem(m_list);
fillItem(item, rule);
}
- m_list->resizeColumnToContents(ColumnEnabled);
- m_list->resizeColumnToContents(ColumnStage);
+ // Auto-sized on the FIRST fill only. After that the widths belong to the
+ // user, whether they came from a restored header or from a drag in this
+ // session, and resizing on every repopulate threw both away on the next
+ // add or delete.
+ if (!m_columnsSized) {
+ m_list->resizeColumnToContents(ColumnEnabled);
+ m_list->resizeColumnToContents(ColumnStage);
+ m_columnsSized = true;
+ }
m_reloading = false;
if (!m_working.isEmpty())
@@ -439,7 +521,12 @@ void TagRulesDialog::setCounts(const QVector<int> &counts)
counts.at(i) < 0 ? tr("invalid")
: QString::number(counts.at(i)));
}
- m_list->resizeColumnToContents(ColumnCount);
+ // Once, like the columns in reloadList. The counts arrive after the first
+ // fill, so this column gets its own flag rather than sharing that one.
+ if (!m_countColumnSized) {
+ m_list->resizeColumnToContents(ColumnCount);
+ m_countColumnSized = true;
+ }
}
void TagRulesDialog::onSave()
diff --git a/src/tagrulesdialog.h b/src/tagrulesdialog.h
index 9559918..f86d210 100644
--- a/src/tagrulesdialog.h
+++ b/src/tagrulesdialog.h
@@ -56,9 +56,14 @@ public:
/// handle and notmuch permits one per process.
QStringList countQueries() const;
- /// Account subdirectory names, for the Folder row's dropdown. Supplied by
- /// the caller rather than read here: Config knows them, and this dialog
- /// deliberately holds no Config of its own.
+ /// Maildir folder paths, for the Folder row's dropdown, relative to the
+ /// database root. Supplied by the caller rather than read here: they come
+ /// from a scan of the tree under notmuch's database.path, which only
+ /// NotmuchWorker can answer for, and this dialog reaches neither it nor
+ /// Config.
+ ///
+ /// Arrives AFTER the dialog is on screen, since the scan crosses to the
+ /// worker on a queued call, so this refills the rows that already exist.
void setFolders(const QStringList &folders);
/// Test seams. The builder's state is otherwise reachable only through
@@ -88,6 +93,20 @@ public:
/// when the widget is not.
bool textModeToggleIsReachableForTest() const;
+ /// Width of one rule-list column. The geometry restore is asserted on the
+ /// saved and reread VALUE rather than on the resulting frame: item 46
+ /// records that the offscreen platform does not honour a window resize,
+ /// so a test comparing frames there passes or fails for reasons that have
+ /// nothing to do with the code.
+ int columnWidthForTest(int column) const;
+ void setColumnWidthForTest(int column, int width);
+
+ /// Repopulates the rule list, as adding or deleting a rule does. Exposed
+ /// because a restored column width has to survive one of these, not only
+ /// a close and reopen: `resizeColumnToContents` on every reload discarded
+ /// the width the user had dragged.
+ void reloadListForTest();
+
signals:
/// Asks the owner to run countQueries() through the worker.
void countsRequested();
@@ -104,8 +123,13 @@ private slots:
void applyEditsToCurrentRule();
void onSave();
+protected:
+ void closeEvent(QCloseEvent *event) override;
+
private:
void reloadList();
+ void restoreUiState();
+ void saveUiState();
void showWarnings();
int currentIndex() const;
@@ -160,6 +184,15 @@ private:
/// row's text, which applyEditsToCurrentRule() has no chance to flush.
bool m_reloading = false;
+ /// Each column is auto-sized to its contents ONCE, on its first fill.
+ /// After that its width belongs to the user, whether it came from a
+ /// restored header or from a drag, and resizeColumnToContents on every
+ /// repopulate threw both away on the next add or delete. Two flags rather
+ /// than one because the count column is filled later than the rest, by a
+ /// reply from the worker.
+ bool m_columnsSized = false;
+ bool m_countColumnSized = false;
+
QTreeWidget *m_list = nullptr;
QLineEdit *m_id = nullptr;
QLineEdit *m_add = nullptr;
diff --git a/tests/test_tagrules.cpp b/tests/test_tagrules.cpp
index cf7dadb..af30938 100644
--- a/tests/test_tagrules.cpp
+++ b/tests/test_tagrules.cpp
@@ -16,9 +16,11 @@
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*/
+#include <QSettings>
#include <QTemporaryDir>
#include <QtTest>
+#include "mainwindow.h"
#include "rulequery.h"
#include "tagrules.h"
#include "tagrulesdialog.h"
@@ -49,6 +51,8 @@ private slots:
void leavingTextModeIsRefusedWhenTheQueryCannotBeShownAsRows();
void aFolderRowUsesTheDropdownAndKeepsItsSuffix();
void theTextModeToggleSurvivesBeingSwitchedOn();
+ void theWindowSizeAndColumnWidthsSurviveAReopen();
+ void aReloadDoesNotDiscardARestoredColumnWidth();
private:
QString writeRules(const QString &json);
@@ -634,5 +638,100 @@ void TestTagRules::theTextModeToggleSurvivesBeingSwitchedOn()
QStringLiteral("from:vendor.example.org"));
}
+namespace {
+
+/// Writes a two-rule file under a throwaway XDG_CONFIG_HOME. Two rules rather
+/// than one because the column-width tests reload the list, and a list with a
+/// single row hides an off-by-one in the repopulate.
+void writeTwoRules(const QTemporaryDir &configHome)
+{
+ QVERIFY(QDir().mkpath(configHome.filePath(QStringLiteral("mailrules"))));
+ QFile out(configHome.filePath(QStringLiteral("mailrules/rules.json")));
+ QVERIFY(out.open(QIODevice::WriteOnly));
+ out.write(R"({
+ "version": 1,
+ "rules": [
+ {"id": "vendor", "query": "from:vendor.example.org",
+ "add": ["vendor"], "stage": 50, "enabled": true},
+ {"id": "lists", "query": "to:list.example.org",
+ "add": ["lists"], "stage": 60, "enabled": true}
+ ]
+ })");
+ out.close();
+}
+
+} // namespace
+
+void TestTagRules::theWindowSizeAndColumnWidthsSurviveAReopen()
+{
+ // The window opened at 760x520 whatever size it was left at, and the
+ // columns reset to their computed widths on every open.
+ //
+ // XDG_STATE_HOME is redirected as well as XDG_CONFIG_HOME: the state file
+ // is where this writes, and a test must not touch the user's real
+ // ~/.local/state/qtmaildir/uistate.conf.
+ QTemporaryDir configHome;
+ QTemporaryDir stateHome;
+ QVERIFY(configHome.isValid());
+ QVERIFY(stateHome.isValid());
+ const QByteArray previousState = qgetenv("XDG_STATE_HOME");
+ qputenv("XDG_CONFIG_HOME", configHome.path().toUtf8());
+ qputenv("XDG_STATE_HOME", stateHome.path().toUtf8());
+ writeTwoRules(configHome);
+
+ {
+ TagRulesDialog dialog;
+ dialog.resize(900, 640);
+ dialog.setColumnWidthForTest(0, 123);
+ // The save is on close, matching where MainWindow writes its own.
+ dialog.close();
+ }
+
+ // Asserted on the stored VALUE, not on the reopened frame. Item 46: the
+ // offscreen platform does not honour a resize, so a frame comparison here
+ // would report a failure the code did not cause.
+ QSettings state(MainWindow::uiStatePath(), QSettings::IniFormat);
+ QCOMPARE(state.value(QStringLiteral("tagrules/geometry")).toByteArray()
+ .isEmpty(), false);
+
+ {
+ TagRulesDialog reopened;
+ QCOMPARE(reopened.columnWidthForTest(0), 123);
+ }
+
+ if (previousState.isEmpty())
+ qunsetenv("XDG_STATE_HOME");
+ else
+ qputenv("XDG_STATE_HOME", previousState);
+}
+
+void TestTagRules::aReloadDoesNotDiscardARestoredColumnWidth()
+{
+ // The width did not survive a close, and it did not survive an ADD or a
+ // DELETE either: reloadList called resizeColumnToContents on every
+ // repopulate, so a restore was undone by the first thing the user did in
+ // the window. Restoring on open and reverting on the next click is worse
+ // than never restoring at all, because it looks like the setting is
+ // broken rather than absent.
+ QTemporaryDir configHome;
+ QTemporaryDir stateHome;
+ QVERIFY(configHome.isValid());
+ QVERIFY(stateHome.isValid());
+ const QByteArray previousState = qgetenv("XDG_STATE_HOME");
+ qputenv("XDG_CONFIG_HOME", configHome.path().toUtf8());
+ qputenv("XDG_STATE_HOME", stateHome.path().toUtf8());
+ writeTwoRules(configHome);
+
+ TagRulesDialog dialog;
+ dialog.setColumnWidthForTest(0, 137);
+ dialog.reloadListForTest();
+ QCOMPARE(dialog.columnWidthForTest(0), 137);
+
+ if (previousState.isEmpty())
+ qunsetenv("XDG_STATE_HOME");
+ else
+ qputenv("XDG_STATE_HOME", previousState);
+}
+
QTEST_MAIN(TestTagRules)
#include "test_tagrules.moc"