fix(tags): complete every tag in the list, not only the first

Reported by the user: in the Edit tags fields, the first tag suggested
existing tags and the second did not. Typing a comma, a space and a
letter offered nothing.

QLineEdit::setCompleter hands completion to the line edit, which
overwrites the completer's prefix with the widget's ENTIRE text on every
keystroke. These fields hold a comma-separated list, so once one reads
"unread, fl" that whole string is matched against the tag names, nothing
matches, and completion silently stops after the first tag. Confirmed
with a probe: the prefix really is "unread, fl" and the completion count
really is zero.

Attach with setWidget instead, which keeps the popup anchored without
ceding control of the prefix, and drive it from the token under the
cursor on every edit. Setting the prefix from a textEdited handler while
leaving setCompleter in place does NOT work, which was the first attempt:
the line edit sets it again afterwards.

Accepting a candidate needed the same treatment, and is the other half of
the fix. QCompleter's own insertion replaces the whole field, so taking
"flagged" from the popup would have discarded every tag already typed.
replaceCurrentToken() overwrites only the token under the cursor and
keeps the separator's spacing, so the result is "unread, flagged" rather
than "unread,flagged".

This is the same defect QueryCompleter hit in c98b179. Having now cost
two debugging rounds, it is written into CLAUDE.md as a Qt trap rather
than a property of either class, together with the reason a test using
setText() passes against it: setText does not drive a completer at all,
so the keys have to be typed.

Both new tests were confirmed to fail against setCompleter before the fix
was kept.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

3 files changed, 165 insertions, 1 deletions

diff --git a/CLAUDE.md b/CLAUDE.md
index ed8faca..694852f 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -94,6 +94,21 @@ via `MainWindow::uiStatePath()`. Never write window blobs into the hand-edited c
 Build the path from `QStandardPaths::GenericStateLocation`, not `StateLocation`: the
 latter appends both the organization and the application name, and both are `qtmaildir`.
 
+**`QLineEdit::setCompleter` is wrong for any field holding more than one
+value.** It hands completion to the line edit, which then overwrites the
+completer's `completionPrefix` with the widget's **entire text** on every
+keystroke. In a field holding a list, the first value completes and nothing
+after it ever does, because "unread, fl" is matched whole against the
+candidates. Setting the prefix from a `textEdited` handler does not help: the
+line edit sets it again afterwards. Use `setCompleter` only for a field whose
+whole contents are the thing being completed; otherwise attach with
+`QCompleter::setWidget` and drive `setCompletionPrefix` and `complete()`
+yourself, and replace the token under the cursor on `activated` rather than
+letting QCompleter overwrite the field. This has been hit twice, in
+`QueryCompleter` (01ba356) and in `TagDialog`; the trap belongs to Qt, not to
+either class. A test that uses `setText()` passes against the bug, since
+`setText` does not drive a completer at all: the keys must be typed.
+
 **Do not conclude a key binding is dead from `QTest::keyClick()`.** Whether a symbol needs
 Shift is a layout property, not a Qt one. `Ctrl++` is the shipped `zoom_in` default and is
 exactly what the `+` key emits on an Italian layout, while synthetic input never delivers
diff --git a/src/tagdialog.cpp b/src/tagdialog.cpp
index 94d2e05..75e6b03 100644
--- a/src/tagdialog.cpp
+++ b/src/tagdialog.cpp
@@ -18,6 +18,7 @@
 
 #include "tagdialog.h"
 
+#include <QAbstractItemView>
 #include <QCompleter>
 #include <QCoreApplication>
 #include <QDialogButtonBox>
@@ -97,6 +98,42 @@ QStringList splitTags(const QString &text)
     return tags;
 }
 
+/// The tag the cursor sits in, which is what completion should match against.
+///
+/// The field holds a comma-separated list, so the last comma before the cursor
+/// bounds the token. Leading space is dropped so "unread, fl" completes on "fl"
+/// rather than on " fl", which would match nothing.
+QString currentToken(const QLineEdit *edit)
+{
+    const QString text = edit->text();
+    const int cursor = qBound(0, edit->cursorPosition(), int(text.size()));
+
+    const int start = text.lastIndexOf(QLatin1Char(','), qMax(0, cursor - 1)) + 1;
+    return text.mid(start, cursor - start).trimmed();
+}
+
+/// Overwrites the token under the cursor with `value`, leaving the rest of the
+/// list alone, and puts the caret after what was inserted.
+void replaceCurrentToken(QLineEdit *edit, const QString &value)
+{
+    const QString text = edit->text();
+    const int cursor = qBound(0, edit->cursorPosition(), int(text.size()));
+
+    const int start = text.lastIndexOf(QLatin1Char(','), qMax(0, cursor - 1)) + 1;
+
+    // Keep the separator's spacing as the user typed it: replacing from `start`
+    // would eat the space after the comma and give "unread,flagged".
+    int tokenStart = start;
+    while (tokenStart < cursor && text.at(tokenStart).isSpace())
+        ++tokenStart;
+
+    QString updated = text;
+    updated.replace(tokenStart, cursor - tokenStart, value);
+
+    edit->setText(updated);
+    edit->setCursorPosition(tokenStart + value.size());
+}
+
 } // namespace
 
 TagDialog::TagDialog(const QStringList &knownTags,
@@ -129,7 +166,39 @@ TagDialog::TagDialog(const QStringList &knownTags,
         // Hierarchies are the reason this matters: typing "amazon" should find
         // "shopping/amazon".
         completer->setFilterMode(Qt::MatchContains);
-        edit->setCompleter(completer);
+
+        // setWidget, NOT QLineEdit::setCompleter. These fields hold a
+        // comma-separated LIST, and setCompleter makes the line edit drive
+        // completion, overwriting the prefix with the widget's ENTIRE text on
+        // every keystroke. Once the field reads "unread, fl" that whole string
+        // is matched against the tag names, nothing matches, and completion
+        // silently stops working after the first tag. Setting the prefix from a
+        // textEdited handler does not help: the line edit sets it again
+        // afterwards.
+        //
+        // setWidget keeps the popup anchored without ceding control of the
+        // prefix, which then becomes ours to drive per token. Exactly the fix
+        // QueryCompleter needed in 01ba356; the trap belongs to
+        // QLineEdit::setCompleter, not to either class.
+        completer->setWidget(edit);
+
+        connect(edit, &QLineEdit::textEdited, this, [edit, completer]() {
+            const QString token = currentToken(edit);
+            completer->setCompletionPrefix(token);
+            if (token.isEmpty() || completer->completionCount() == 0) {
+                completer->popup()->hide();
+                return;
+            }
+            completer->complete();
+        });
+
+        // Accepting a candidate has to replace the token under the cursor
+        // rather than the whole field, or taking "flagged" would discard every
+        // tag already typed.
+        connect(completer, QOverload<const QString &>::of(&QCompleter::activated),
+                this, [edit](const QString &value) {
+            replaceCurrentToken(edit, value);
+        });
     }
 
     form->addRow(tr("Add:"), m_addEdit);
diff --git a/tests/test_tagdialog.cpp b/tests/test_tagdialog.cpp
index f41850e..df0fece 100644
--- a/tests/test_tagdialog.cpp
+++ b/tests/test_tagdialog.cpp
@@ -19,6 +19,7 @@
 #include <QtTest>
 
 #include <QCheckBox>
+#include <QCompleter>
 #include <QLineEdit>
 #include <QListWidget>
 
@@ -42,6 +43,8 @@ private slots:
     void aPartialTagLeftAloneChangesNothing();
     void aPartialTagCheckedIsAddedEverywhere();
     void nothingTouchedYieldsNoChange();
+    void completionFollowsTheTagAfterAComma();
+    void acceptingACandidateKeepsTheOtherTags();
 };
 
 void TestTagDialog::validNamesAreAccepted()
@@ -239,5 +242,82 @@ void TestTagDialog::nothingTouchedYieldsNoChange()
     QVERIFY(dialog.tagsToRemove().isEmpty());
 }
 
+void TestTagDialog::completionFollowsTheTagAfterAComma()
+{
+    // Reported by the user: the first tag completes, the second does not.
+    //
+    // QLineEdit::setCompleter matches against the widget's ENTIRE text, so once
+    // the field reads "unread, fl" that whole string becomes the completion
+    // prefix and nothing matches. The completer has to be driven on the token
+    // under the cursor instead. This is the same defect QueryCompleter hit in
+    // 01ba356, in a second place.
+    //
+    // Typed rather than setText(): setText does not drive a completer at all,
+    // so a test using it passes against the broken code.
+    TagDialog dialog({ QStringLiteral("inbox"), QStringLiteral("unread"),
+                       QStringLiteral("flagged") }, {}, 1);
+    dialog.show();
+    QVERIFY(QTest::qWaitForWindowExposed(&dialog));
+
+    const QList<QLineEdit *> edits = dialog.findChildren<QLineEdit *>();
+    QVERIFY(edits.size() >= 2);
+    QLineEdit *addEdit = edits.at(0);
+    addEdit->setFocus();
+    QTRY_COMPARE(QApplication::focusWidget(), addEdit);
+
+    // findChild, not QLineEdit::completer(): the completer is attached with
+    // setWidget() rather than setCompleter(), for the reason the fix documents,
+    // so the line edit does not report one. It is parented to the edit, which
+    // is what makes it reachable here.
+    QCompleter *completer = addEdit->findChild<QCompleter *>();
+    QVERIFY(completer);
+
+    // First tag: this much always worked.
+    QTest::keyClicks(addEdit, QStringLiteral("un"));
+    QCOMPARE(completer->completionPrefix(), QStringLiteral("un"));
+    QVERIFY(completer->completionCount() > 0);
+
+    // Second tag, after a comma and a space. The prefix must be the new token,
+    // not the whole line.
+    QTest::keyClicks(addEdit, QStringLiteral("read, fl"));
+    QCOMPARE(addEdit->text(), QStringLiteral("unread, fl"));
+
+    QCOMPARE(completer->completionPrefix(), QStringLiteral("fl"));
+    QVERIFY2(completer->completionCount() > 0,
+             "no candidate for the tag after the comma: the completer is "
+             "matching against the whole line");
+}
+
+void TestTagDialog::acceptingACandidateKeepsTheOtherTags()
+{
+    // Driving the prefix per token is only half the fix. Accepting a candidate
+    // has to overwrite that token too: QCompleter's own insertion replaces the
+    // whole field, so taking "flagged" here would discard "unread" with it.
+    TagDialog dialog({ QStringLiteral("inbox"), QStringLiteral("unread"),
+                       QStringLiteral("flagged") }, {}, 1);
+    dialog.show();
+    QVERIFY(QTest::qWaitForWindowExposed(&dialog));
+
+    const QList<QLineEdit *> edits = dialog.findChildren<QLineEdit *>();
+    QVERIFY(edits.size() >= 2);
+    QLineEdit *addEdit = edits.at(0);
+    addEdit->setFocus();
+    QTRY_COMPARE(QApplication::focusWidget(), addEdit);
+
+    QCompleter *completer = addEdit->findChild<QCompleter *>();
+    QVERIFY(completer);
+
+    QTest::keyClicks(addEdit, QStringLiteral("unread, fl"));
+    QCOMPARE(completer->completionPrefix(), QStringLiteral("fl"));
+
+    // What clicking a row emits.
+    emit completer->activated(QStringLiteral("flagged"));
+
+    QCOMPARE(addEdit->text(), QStringLiteral("unread, flagged"));
+    // And the separator's spacing survives: replacing from the comma itself
+    // would have produced "unread,flagged".
+    QVERIFY(addEdit->text().contains(QStringLiteral(", ")));
+}
+
 QTEST_MAIN(TestTagDialog)
 #include "test_tagdialog.moc"