From 4a9bbff4cb157538106b588bc84f39dd7fd38289 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Thu, 13 Aug 2026 11:46:16 +0200 Subject: feat(rules): a folder dropdown, so the path suffix is never typed --- src/mainwindow.cpp | 11 +++++++ src/tagrulesdialog.cpp | 78 +++++++++++++++++++++++++++++++++++++++++++++---- src/tagrulesdialog.h | 15 ++++++++++ tests/test_tagrules.cpp | 46 +++++++++++++++++++++++++++++ 4 files changed, 144 insertions(+), 6 deletions(-) diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 331354f..f622036 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -1313,6 +1313,17 @@ void MainWindow::showTagRulesDialog() auto *dialog = new TagRulesDialog(this); dialog->setAttribute(Qt::WA_DeleteOnClose); + + // The Folder row's dropdown. Config holds the account subdirectories and + // the dialog holds no Config, so they are handed over here. Account::maildir + // is the subtree name the Folder term compiles a path: against; the account + // key is a config section name and would match nothing. + QStringList folders; + for (const Account &account : m_config.accounts()) { + if (!account.maildir.isEmpty() && !folders.contains(account.maildir)) + folders.append(account.maildir); + } + dialog->setFolders(folders); m_tagRulesDialog = dialog; connect(dialog, &TagRulesDialog::countsRequested, this, [this, dialog]() { diff --git a/src/tagrulesdialog.cpp b/src/tagrulesdialog.cpp index 86566ba..c81450c 100644 --- a/src/tagrulesdialog.cpp +++ b/src/tagrulesdialog.cpp @@ -461,7 +461,7 @@ void TagRulesDialog::setRowValueForTest(int index, const QString &value) { if (index < 0 || index >= m_rows.size()) return; - m_rows.at(index).value->setText(value); + setRowValue(&m_rows[index], value); syncQueryLine(); } @@ -491,6 +491,44 @@ void TagRulesDialog::selectRuleForTest(int index) m_list->setCurrentItem(m_list->topLevelItem(index)); } +QString TagRulesDialog::rowValue(const Row &row) const +{ + const bool isFolder = + RuleTerm::Field(row.field->currentData().toInt()) == RuleTerm::Folder; + return (isFolder ? row.folder->currentText() : row.value->text()).trimmed(); +} + +void TagRulesDialog::setRowValue(Row *row, const QString &value) +{ + if (RuleTerm::Field(row->field->currentData().toInt()) == RuleTerm::Folder) + row->folder->setCurrentText(value); + else + row->value->setText(value); +} + +void TagRulesDialog::setFolders(const QStringList &folders) +{ + m_folders = folders; + + // Rows already exist by the time this is called: the constructor loads the + // first rule and builds its rows before the caller can hand the list over. + // Repopulating them here rather than only in addRow() is what stops the row + // on screen from opening with an empty dropdown. The current text is + // preserved across the refill, since the combo is editable and may hold a + // folder the config does not list. + const auto refill = [&folders](const QList &rows) { + for (const Row &row : rows) { + const QString had = row.folder->currentText(); + QSignalBlocker block(row.folder); + row.folder->clear(); + row.folder->addItems(folders); + row.folder->setCurrentText(had); + } + }; + refill(m_rows); + refill(m_exclusionRows); +} + TagRulesDialog::Row *TagRulesDialog::addRow(bool exclusion) { Row row; @@ -505,6 +543,13 @@ TagRulesDialog::Row *TagRulesDialog::addRow(bool exclusion) row.op = new QComboBox(row.container); row.value = new QLineEdit(row.container); + row.folder = new QComboBox(row.container); + // Editable so a folder present in the file but absent from the config + // still displays and still saves, rather than being silently blanked. + row.folder->setEditable(true); + row.folder->addItems(m_folders); + row.folder->setVisible(false); + auto *plus = new QPushButton(QStringLiteral("+"), row.container); auto *minus = new QPushButton(QStringLiteral("-"), row.container); plus->setFixedWidth(30); @@ -513,6 +558,7 @@ TagRulesDialog::Row *TagRulesDialog::addRow(bool exclusion) layout->addWidget(row.field); layout->addWidget(row.op); layout->addWidget(row.value, 1); + layout->addWidget(row.folder, 1); layout->addWidget(plus); layout->addWidget(minus); @@ -521,15 +567,28 @@ TagRulesDialog::Row *TagRulesDialog::addRow(bool exclusion) rows.append(row); target->addWidget(row.container); + // The three widgets by pointer, never the Row by value: the row lives in a + // QList that reallocates as rows are added, so a copy taken here would be + // compared against, or written through, after that list has moved. + QComboBox *field = row.field; + QLineEdit *value = row.value; + QComboBox *folder = row.folder; connect(row.field, &QComboBox::currentIndexChanged, this, - [this, exclusion](int) { + [this, exclusion, field, value, folder](int) { populateOperators(exclusion); + const bool isFolder = + RuleTerm::Field(field->currentData().toInt()) + == RuleTerm::Folder; + value->setVisible(!isFolder); + folder->setVisible(isFolder); syncQueryLine(); }); connect(row.op, &QComboBox::currentIndexChanged, this, [this](int) { syncQueryLine(); }); connect(row.value, &QLineEdit::textEdited, this, [this](const QString &) { syncQueryLine(); }); + connect(row.folder, &QComboBox::currentTextChanged, + this, [this](const QString &) { syncQueryLine(); }); connect(plus, &QPushButton::clicked, this, [this, exclusion] { addRow(exclusion); syncQueryLine(); @@ -562,7 +621,7 @@ void TagRulesDialog::removeRow(bool exclusion, int index) // empty query, which is reachable by clearing the value rather than by // deleting the last row out from under the user. if (!exclusion && rows.size() == 1) { - rows[0].value->clear(); + setRowValue(&rows[0], QString()); return; } @@ -622,7 +681,7 @@ RuleQuery TagRulesDialog::currentQueryFromRows() const query.join = m_matchAny->isChecked() ? RuleQuery::Any : RuleQuery::All; for (const Row &row : m_rows) { - const QString value = row.value->text().trimmed(); + const QString value = rowValue(row); if (value.isEmpty()) continue; query.terms.append({RuleTerm::Field(row.field->currentData().toInt()), @@ -630,7 +689,7 @@ RuleQuery TagRulesDialog::currentQueryFromRows() const value}); } for (const Row &row : m_exclusionRows) { - const QString value = row.value->text().trimmed(); + const QString value = rowValue(row); if (value.isEmpty()) continue; query.exclusions.append( @@ -712,11 +771,18 @@ void TagRulesDialog::applyTermToRow(Row *row, const RuleTerm &term) const QSignalBlocker blockField(row->field); const QSignalBlocker blockOp(row->op); const QSignalBlocker blockValue(row->value); + const QSignalBlocker blockFolder(row->folder); const int fieldIndex = row->field->findData(int(term.field)); if (fieldIndex >= 0) row->field->setCurrentIndex(fieldIndex); + // The field's own handler is blocked here, so the swap it would have done + // has to happen explicitly or a Folder row opens showing the line edit. + const bool isFolder = term.field == RuleTerm::Folder; + row->value->setVisible(!isFolder); + row->folder->setVisible(isFolder); + // The operator list depends on the field just set, so it must be rebuilt // before the operator can be found in it. populateOperators(false); @@ -726,7 +792,7 @@ void TagRulesDialog::applyTermToRow(Row *row, const RuleTerm &term) if (opIndex >= 0) row->op->setCurrentIndex(opIndex); - row->value->setText(term.value); + setRowValue(row, term.value); } void TagRulesDialog::syncQueryLine() diff --git a/src/tagrulesdialog.h b/src/tagrulesdialog.h index 948ebe2..c91dc0c 100644 --- a/src/tagrulesdialog.h +++ b/src/tagrulesdialog.h @@ -19,6 +19,7 @@ #pragma once #include +#include #include "rulequery.h" #include "tagrules.h" @@ -55,6 +56,11 @@ 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. + void setFolders(const QStringList &folders); + /// Test seams. The builder's state is otherwise reachable only through /// synthetic clicks on widgets whose geometry the offscreen platform does /// not guarantee. @@ -108,8 +114,15 @@ private: QComboBox *field = nullptr; QComboBox *op = nullptr; QLineEdit *value = nullptr; + /// Shown in place of `value` for a Folder row. Never visible together. + QComboBox *folder = nullptr; }; + /// Reads a row's value from whichever of its two widgets the field selects, + /// so the read and write paths cannot disagree about where it lives. + QString rowValue(const Row &row) const; + void setRowValue(Row *row, const QString &value); + Row *addRow(bool exclusion); void removeRow(bool exclusion, int index); void populateOperators(bool exclusion); @@ -127,6 +140,8 @@ private: /// left alone. RuleQuery m_loadedQuery; + QStringList m_folders; + QList m_rows; QList m_exclusionRows; diff --git a/tests/test_tagrules.cpp b/tests/test_tagrules.cpp index 2302111..8207b3c 100644 --- a/tests/test_tagrules.cpp +++ b/tests/test_tagrules.cpp @@ -47,6 +47,7 @@ private slots: void editingARowRewritesTheQuery(); void aTextModeRuleStaysTextWhenAnotherRuleIsVisited(); void leavingTextModeIsRefusedWhenTheQueryCannotBeShownAsRows(); + void aFolderRowUsesTheDropdownAndKeepsItsSuffix(); private: QString writeRules(const QString &json); @@ -544,5 +545,50 @@ void TestTagRules::leavingTextModeIsRefusedWhenTheQueryCannotBeShownAsRows() "a stale refusal must not outlive the query that caused it"); } +void TestTagRules::aFolderRowUsesTheDropdownAndKeepsItsSuffix() +{ + // A path: without its suffix matches nothing and notmuch says nothing + // about it, so the suffix must never depend on the user typing it. + QTemporaryDir configHome; + QVERIFY(configHome.isValid()); + qputenv("XDG_CONFIG_HOME", configHome.path().toUtf8()); + QVERIFY(QDir().mkpath(configHome.filePath(QStringLiteral("mailrules")))); + + const QString stored = configHome.filePath( + QStringLiteral("mailrules/rules.json")); + QFile out(stored); + QVERIFY(out.open(QIODevice::WriteOnly)); + out.write(R"({ + "version": 1, + "rules": [ + {"id": "account", "query": "path:\"account-one/**\"", + "add": ["account-one"], "stage": 10, "enabled": true} + ] + })"); + out.close(); + + TagRulesDialog dialog; + dialog.setFolders({QStringLiteral("account-one"), + QStringLiteral("account-two")}); + + // The stored rule round-trips: the row holds the bare name, and the + // query keeps the suffix. + QCOMPARE(dialog.rowCountForTest(), 1); + QCOMPARE(dialog.queryLineForTest(), + QStringLiteral("path:\"account-one/**\"")); + + dialog.setRowValueForTest(0, QStringLiteral("account-two")); + QCOMPARE(dialog.queryLineForTest(), + QStringLiteral("path:\"account-two/**\"")); + + dialog.saveForTest(); + + TagRules reloaded; + reloaded.load(stored); + QCOMPARE(reloaded.rules().size(), 1); + QCOMPARE(reloaded.rules().at(0).query, + QStringLiteral("path:\"account-two/**\"")); +} + QTEST_MAIN(TestTagRules) #include "test_tagrules.moc" -- cgit v1.2.3