summaryrefslogtreecommitdiffstats
path: root/src/tagrulesdialog.cpp
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-13 11:59:30 +0200
committerDanilo M. <danix@danix.xyz>2026-08-13 11:59:30 +0200
commite7cdd7fd3ae960572976cb4a053ecf192baa17c2 (patch)
treed191bd82d67b750a14e72fabdbacbd5eb60c5e58 /src/tagrulesdialog.cpp
parent2471356c99f506b6227c4f9a9399f051b85ef76c (diff)
downloadqtmaildir-e7cdd7fd3ae960572976cb4a053ecf192baa17c2.tar.gz
qtmaildir-e7cdd7fd3ae960572976cb4a053ecf192baa17c2.zip
fix(rules): keep the text-mode toggle reachable
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.
Diffstat (limited to 'src/tagrulesdialog.cpp')
-rw-r--r--src/tagrulesdialog.cpp29
1 files changed, 23 insertions, 6 deletions
diff --git a/src/tagrulesdialog.cpp b/src/tagrulesdialog.cpp
index c81450c..75a9cbe 100644
--- a/src/tagrulesdialog.cpp
+++ b/src/tagrulesdialog.cpp
@@ -131,14 +131,9 @@ TagRulesDialog::TagRulesDialog(QWidget *parent)
auto *matchGroup = new QButtonGroup(this);
matchGroup->addButton(m_matchAll);
matchGroup->addButton(m_matchAny);
- m_textMode = new QCheckBox(tr("Edit as &text"), m_builder);
- m_textMode->setToolTip(
- tr("Edit the notmuch query directly. A rule too complex to show as "
- "rows opens this way."));
matchRow->addWidget(m_matchAll);
matchRow->addWidget(m_matchAny);
matchRow->addStretch();
- matchRow->addWidget(m_textMode);
builderLayout->addLayout(matchRow);
m_rowsLayout = new QVBoxLayout;
@@ -153,7 +148,21 @@ TagRulesDialog::TagRulesDialog(QWidget *parent)
builderLayout->addWidget(m_addExclusion, 0, Qt::AlignLeft);
form->addRow(tr("Match"), m_builder);
- form->addRow(tr("Query"), m_query);
+
+ // The toggle sits with the QUERY line, not inside m_builder, because
+ // switching to text mode HIDES m_builder. A checkbox parented there
+ // vanishes with the rows it governs, leaving no way back except closing
+ // the dialog, which is exactly what shipped in the first draft of this
+ // builder. The query row is visible in both modes, so the toggle is
+ // always reachable.
+ auto *queryRow = new QHBoxLayout;
+ m_textMode = new QCheckBox(tr("Edit as &text"), this);
+ m_textMode->setToolTip(
+ tr("Edit the notmuch query directly. A rule too complex to show as "
+ "rows opens this way."));
+ queryRow->addWidget(m_query, 1);
+ queryRow->addWidget(m_textMode);
+ form->addRow(tr("Query"), queryRow);
// The query line shows what the rows compile to. Read-only in builder
// mode: it is what actually ships to the hook, and watching it change is
@@ -475,6 +484,14 @@ void TagRulesDialog::setTextModeForTest(bool on)
m_textMode->setChecked(on);
}
+bool TagRulesDialog::textModeToggleIsReachableForTest() const
+{
+ // isVisibleTo rather than isVisible: nothing is isVisible() on a dialog
+ // that was never shown, so that would report unreachable in both the
+ // working and the broken case.
+ return m_textMode->isVisibleTo(this);
+}
+
QString TagRulesDialog::warningTextForTest() const
{
// isVisible() is false for every child of a dialog that was never shown,