diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-07 10:32:08 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-07 10:32:08 +0200 |
| commit | 6935bfc99ed0df5c79867f7015ba04041e03f908 (patch) | |
| tree | 38c407c5ce7a5ae55331c89603fa2b6e4776d147 | |
| parent | db4dbbf4bf4fd8aab1a9e20d87c266ce7836a946 (diff) | |
| download | qtmaildir-6935bfc99ed0df5c79867f7015ba04041e03f908.tar.gz qtmaildir-6935bfc99ed0df5c79867f7015ba04041e03f908.zip | |
fix(tags): the Remove field suggests only tags the selection has
Both TagDialog fields built their completer from knownTags, the whole
database's tag list, so removing a tag offered every tag in existence
rather than the handful the selected threads actually carry.
The candidates were already in the dialog: currentTags, used until now
only to render the checkbox list. The constructor now walks two
(field, vocabulary) pairs instead of two fields sharing one list, with
knownTags for Add and currentTags.keys() for Remove. On a multi-thread
selection that is the union, not the intersection, since removing a tag
two of three threads carry is a meaningful request.
The setWidget and per-token prefix machinery is untouched: these fields
hold a comma-separated list, and QLineEdit::setCompleter is the trap
this dialog already works around. Only the candidate list changed.
Completion stays a suggestion, never a whitelist, so a tag absent from
the candidates still applies.
Tests type keys rather than using setText, which does not drive a
completer at all. Verified load-bearing by mutation: reverting the
Remove vocabulary to knownTags fails the new test.
Closes item 48.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| -rw-r--r-- | docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md | 2 | ||||
| -rw-r--r-- | src/tagdialog.cpp | 19 | ||||
| -rw-r--r-- | tests/test_tagdialog.cpp | 62 |
3 files changed, 80 insertions, 3 deletions
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 f10a301..0091d27 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 @@ -95,7 +95,7 @@ taking that too literally. | 45 | Two Sync buttons, and only one of them works properly | correctness | S | **done** | | 46 | `uiStateSurvivesARestart` fails under the offscreen platform | testing | XS | **done** | | 47 | The query bar looks unfinished, and cannot be cleared by mouse | presentation | XS | **done** | -| 48 | Removing a tag suggests every tag, not the thread's own | workflow | XS | open | +| 48 | Removing a tag suggests every tag, not the thread's own | workflow | XS | **done** | Sizes are rough: XS under an hour, S a sitting, M a session. diff --git a/src/tagdialog.cpp b/src/tagdialog.cpp index 75e6b03..1fb3f18 100644 --- a/src/tagdialog.cpp +++ b/src/tagdialog.cpp @@ -160,8 +160,23 @@ TagDialog::TagDialog(const QStringList &knownTags, // Completion is a guard against typos, never a whitelist: a tag absent from // this list is exactly what the dialog exists to create, so the completer // suggests and does not constrain. - for (QLineEdit *edit : { m_addEdit, m_removeEdit }) { - auto *completer = new QCompleter(knownTags, edit); + // + // The two fields complete against different vocabularies. Add reaches the + // whole database, since naming a tag that does not exist yet is what it is + // for. Remove offers only what the selection actually carries: on a + // multi-thread selection that is the union with counts, not the + // intersection, because removing a tag two of three threads have is a + // meaningful thing to ask for. + QStringList removeCandidates = currentTags.keys(); + removeCandidates.sort(); + + const QList<QPair<QLineEdit *, QStringList>> fields = { + { m_addEdit, knownTags }, + { m_removeEdit, removeCandidates }, + }; + + for (const auto &[edit, candidates] : fields) { + auto *completer = new QCompleter(candidates, edit); completer->setCaseSensitivity(Qt::CaseInsensitive); // Hierarchies are the reason this matters: typing "amazon" should find // "shopping/amazon". diff --git a/tests/test_tagdialog.cpp b/tests/test_tagdialog.cpp index df0fece..5289599 100644 --- a/tests/test_tagdialog.cpp +++ b/tests/test_tagdialog.cpp @@ -45,6 +45,8 @@ private slots: void nothingTouchedYieldsNoChange(); void completionFollowsTheTagAfterAComma(); void acceptingACandidateKeepsTheOtherTags(); + void removeCompletesOnlyTheSelectionsOwnTags(); + void removeStillAcceptsATagItDoesNotSuggest(); }; void TestTagDialog::validNamesAreAccepted() @@ -319,5 +321,65 @@ void TestTagDialog::acceptingACandidateKeepsTheOtherTags() QVERIFY(addEdit->text().contains(QStringLiteral(", "))); } +void TestTagDialog::removeCompletesOnlyTheSelectionsOwnTags() +{ + // Reported by the user: removing a tag suggested every tag in the database. + // Only the tags the selection already carries can be removed, and those are + // already in the dialog as currentTags. + // + // Typed rather than setText(), which does not drive a completer at all. + TagDialog dialog({ QStringLiteral("inbox"), QStringLiteral("unread"), + QStringLiteral("flagged"), QStringLiteral("archive") }, + { { QStringLiteral("inbox"), 1 } }, 1); + dialog.show(); + QVERIFY(QTest::qWaitForWindowExposed(&dialog)); + + const QList<QLineEdit *> edits = dialog.findChildren<QLineEdit *>(); + QVERIFY(edits.size() >= 2); + QLineEdit *removeEdit = edits.at(1); + removeEdit->setFocus(); + QTRY_COMPARE(QApplication::focusWidget(), removeEdit); + + QCompleter *completer = removeEdit->findChild<QCompleter *>(); + QVERIFY(completer); + + // "fl" matches "flagged", which the database has and the selection does not. + QTest::keyClicks(removeEdit, QStringLiteral("fl")); + QCOMPARE(completer->completionPrefix(), QStringLiteral("fl")); + QCOMPARE(completer->completionCount(), 0); + + // A tag the selection does carry still completes. + removeEdit->clear(); + QTest::keyClicks(removeEdit, QStringLiteral("inb")); + QCOMPARE(completer->completionPrefix(), QStringLiteral("inb")); + QCOMPARE(completer->completionCount(), 1); + QCOMPARE(completer->currentCompletion(), QStringLiteral("inbox")); + + // Add is unchanged: it must still reach the whole vocabulary, since + // creating a tag is what that field is for. + QLineEdit *addEdit = edits.at(0); + addEdit->setFocus(); + QTRY_COMPARE(QApplication::focusWidget(), addEdit); + QCompleter *addCompleter = addEdit->findChild<QCompleter *>(); + QVERIFY(addCompleter); + QTest::keyClicks(addEdit, QStringLiteral("fl")); + QVERIFY(addCompleter->completionCount() > 0); +} + +void TestTagDialog::removeStillAcceptsATagItDoesNotSuggest() +{ + // Completion is a suggestion, never a whitelist. Narrowing the candidates + // must not start validating input against them. + TagDialog dialog({ QStringLiteral("inbox") }, + { { QStringLiteral("inbox"), 1 } }, 1); + + const QList<QLineEdit *> edits = dialog.findChildren<QLineEdit *>(); + QVERIFY(edits.size() >= 2); + edits.at(1)->setText(QStringLiteral("flagged")); + dialog.accept(); + + QVERIFY(dialog.tagsToRemove().contains(QStringLiteral("flagged"))); +} + QTEST_MAIN(TestTagDialog) #include "test_tagdialog.moc" |
