fix(completion): drive the popup instead of QLineEdit::setCompleter

QLineEdit::setCompleter hands completion to the line edit, which then
resets the completer's completionPrefix to the widget's entire text on
every keystroke. The prefix has to be the stem, so once the query grew
past its first token the whole-line prefix matched no candidate, the
popup stopped appearing, and the two reported symptoms followed: nothing
was there for Tab to accept, and Tab fell through to focus navigation.

Attach the completer with setWidget instead, which keeps the popup
anchored without ceding control of the prefix. complete() dereferences
widget() unconditionally, so leaving it unset segfaults rather than
degrading. Opening the popup then becomes ours to do on every edit.

Extend the existing event filter to route the keys the popup needs while
it is visible, and install it unconditionally now that it does more than
the completion_on_focus case. Enter accepts a completion only while the
popup is up, so returnPressed still runs the query when it is closed.

The existing tests called acceptCompletion() directly and so never
touched the widget, which is why neither bug was caught. Add four tests
that drive the real widget path plus one covering both settings of
completion_on_focus; the first two fail against the old code.

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

3 files changed, 210 insertions, 6 deletions

diff --git a/src/querycompleter.cpp b/src/querycompleter.cpp
index 1a6f6de..b4968f5 100644
--- a/src/querycompleter.cpp
+++ b/src/querycompleter.cpp
@@ -23,6 +23,7 @@
 #include <QCompleter>
 #include <QCoreApplication>
 #include <QFontMetrics>
+#include <QKeyEvent>
 #include <QLabel>
 #include <QLineEdit>
 #include <QListView>
@@ -350,9 +351,17 @@ QueryCompleter::QueryCompleter(QLineEdit *edit, const Config &config,
     // which silently drops the description column.
     m_popup->setItemDelegate(new CompletionDelegate(m_popup));
 
-    m_edit->setCompleter(m_completer);
-
-    connect(m_edit, &QLineEdit::textEdited, this, &QueryCompleter::updateContext);
+    // setWidget, NOT QLineEdit::setCompleter. setCompleter hands completion to
+    // the line edit, which then overwrites completionPrefix with the widget's
+    // ENTIRE text on every keystroke. The prefix must be the stem instead, so
+    // the whole-line prefix matches nothing and the popup stops appearing after
+    // the first token. setWidget still gives the completer the anchor it needs:
+    // complete() dereferences widget() unconditionally and crashes without one.
+    m_completer->setWidget(m_edit);
+
+    // With the line edit no longer driving completion, every edit has to open
+    // the popup explicitly.
+    connect(m_edit, &QLineEdit::textEdited, this, &QueryCompleter::triggerCompletion);
     connect(m_edit, &QLineEdit::cursorPositionChanged,
             this, [this]() { updateContext(); });
 
@@ -361,8 +370,9 @@ QueryCompleter::QueryCompleter(QLineEdit *edit, const Config &config,
         acceptCompletion(index.data(Qt::DisplayRole).toString());
     });
 
-    if (m_config.completionOnFocus())
-        m_edit->installEventFilter(this);
+    // Always filtered, not only for completion_on_focus: the popup is ours to
+    // drive now, so Tab, Enter, Escape and the arrows must be routed by hand.
+    m_edit->installEventFilter(this);
 }
 
 void QueryCompleter::triggerCompletion()
@@ -386,12 +396,57 @@ bool QueryCompleter::eventFilter(QObject *watched, QEvent *event)
     // Only the empty-bar case: once there is text, ordinary typing has
     // already driven completion.
     if (watched == m_edit && event->type() == QEvent::FocusIn
-        && m_edit->text().isEmpty()) {
+        && m_config.completionOnFocus() && m_edit->text().isEmpty()) {
         triggerCompletion();
+        return QObject::eventFilter(watched, event);
+    }
+
+    if (event->type() != QEvent::KeyPress || !popupVisible())
+        return QObject::eventFilter(watched, event);
+
+    auto *keyEvent = static_cast<QKeyEvent *>(event);
+    switch (keyEvent->key()) {
+    case Qt::Key_Tab:
+    case Qt::Key_Enter:
+    case Qt::Key_Return: {
+        // Accept whatever the popup highlights. A freshly opened popup has no
+        // current row, so fall back to the first entry: the user sees it at the
+        // top of the list and expects Tab to take it.
+        QModelIndex index = m_popup->currentIndex();
+        if (!index.isValid())
+            index = m_popup->model()->index(0, 0);
+        if (!index.isValid())
+            return QObject::eventFilter(watched, event);
+
+        acceptCompletion(index.data(Qt::DisplayRole).toString());
+        m_popup->hide();
+        // Consume it. Tab would otherwise move focus to the next widget, and
+        // Return would run the half-typed query.
+        return true;
     }
+    case Qt::Key_Escape:
+        m_popup->hide();
+        return true;
+    case Qt::Key_Up:
+    case Qt::Key_Down:
+    case Qt::Key_PageUp:
+    case Qt::Key_PageDown:
+        // Navigation belongs to the popup, which is not the focus widget while
+        // the user is typing in the bar.
+        QCoreApplication::sendEvent(m_popup, event);
+        return true;
+    default:
+        break;
+    }
+
     return QObject::eventFilter(watched, event);
 }
 
+bool QueryCompleter::popupVisible() const
+{
+    return m_popup && m_popup->isVisible();
+}
+
 void QueryCompleter::acceptCompletion(const QString &value)
 {
     if (!m_edit)
diff --git a/src/querycompleter.h b/src/querycompleter.h
index 61b2555..350b21c 100644
--- a/src/querycompleter.h
+++ b/src/querycompleter.h
@@ -119,6 +119,11 @@ protected:
     bool eventFilter(QObject *watched, QEvent *event) override;
 
 private:
+    /// Whether the completion popup is on screen. Every key the filter claims
+    /// is claimed only while it is, so the bar types and tabs normally
+    /// otherwise.
+    bool popupVisible() const;
+
     QList<CompletionEntry> entriesFor(const CompletionContext &context) const;
     void rebuildModel(const CompletionContext &context);
 
diff --git a/tests/test_querycompleter.cpp b/tests/test_querycompleter.cpp
index 56b5343..5ae9312 100644
--- a/tests/test_querycompleter.cpp
+++ b/tests/test_querycompleter.cpp
@@ -20,6 +20,7 @@
 #include <QTemporaryDir>
 
 #include <QLineEdit>
+#include <QListView>
 
 #include "config.h"
 #include "querycompleter.h"
@@ -54,6 +55,14 @@ private slots:
     void acceptReplacesOnlyTheValueAfterThePrefix();
     void acceptReplacesOnlyTheEditedRangeBound();
     void acceptReplacesTheWholeBoundWhenCompletingMidWord();
+
+    // The tests above call acceptCompletion() directly and so never touch the
+    // widget. These drive the path a user actually hits.
+    void typingOpensThePopupOnALaterToken();
+    void tabAcceptsTheHighlightedCompletion();
+    void tabIsIgnoredWhileThePopupIsHidden();
+    void returnIsIgnoredWhileThePopupIsHidden();
+    void focusOpensThePopupOnlyWhenConfigured();
 };
 
 // Copied from tests/test_config.cpp rather than shared, so the two test files
@@ -353,5 +362,140 @@ void TestQueryCompleter::acceptReplacesTheWholeBoundWhenCompletingMidWord()
              QStringLiteral("date:this_week..today"));
 }
 
+// The popup is owned by the QCompleter, which is not reachable from the line
+// edit now that setCompleter is deliberately not used. It is the only list
+// view these tests create, so find it that way.
+static QListView *findPopup()
+{
+    const auto widgets = QApplication::allWidgets();
+    for (QWidget *w : widgets) {
+        if (auto *view = qobject_cast<QListView *>(w))
+            return view;
+    }
+    return nullptr;
+}
+
+void TestQueryCompleter::typingOpensThePopupOnALaterToken()
+{
+    // The regression: QLineEdit::setCompleter reset the completion prefix to
+    // the widget's whole text on every keystroke, so nothing matched and the
+    // popup stopped appearing after the first token.
+    Config config;
+    QLineEdit edit;
+    edit.show();
+    QueryCompleter completer(&edit, config);
+
+    QTest::keyClicks(&edit, QStringLiteral("tag:unread date:last"));
+
+    QListView *popup = findPopup();
+    QVERIFY(popup);
+    QVERIFY(popup->isVisible());
+
+    QStringList offered;
+    for (int row = 0; row < popup->model()->rowCount(); ++row)
+        offered << popup->model()->index(row, 0).data().toString();
+    QCOMPARE(offered, QStringList({ QStringLiteral("last_week"),
+                                    QStringLiteral("last_month") }));
+}
+
+void TestQueryCompleter::tabAcceptsTheHighlightedCompletion()
+{
+    // The user's exact scenario. Tab used to fall through to focus navigation,
+    // leaving the query half-typed.
+    Config config;
+    QLineEdit edit;
+    edit.show();
+    QueryCompleter completer(&edit, config);
+
+    QTest::keyClicks(&edit, QStringLiteral("tag:unread date:last"));
+    QVERIFY(findPopup() && findPopup()->isVisible());
+
+    QKeyEvent tab(QEvent::KeyPress, Qt::Key_Tab, Qt::NoModifier,
+                  QStringLiteral("\t"));
+    QApplication::sendEvent(&edit, &tab);
+
+    // Consumed, so focus does not move to the next widget.
+    QVERIFY(tab.isAccepted());
+    // Only the token being completed is replaced, not the whole line.
+    QCOMPARE(edit.text(), QStringLiteral("tag:unread date:last_week"));
+    QVERIFY(!findPopup()->isVisible());
+}
+
+void TestQueryCompleter::tabIsIgnoredWhileThePopupIsHidden()
+{
+    // Every key must fall through when the popup is closed, or the query bar
+    // stops behaving like a line edit.
+    Config config;
+    QLineEdit edit;
+    edit.show();
+    QueryCompleter completer(&edit, config);
+
+    edit.setText(QStringLiteral("tag:unread"));
+    if (QListView *popup = findPopup())
+        popup->hide();
+
+    QKeyEvent tab(QEvent::KeyPress, Qt::Key_Tab, Qt::NoModifier,
+                  QStringLiteral("\t"));
+    tab.ignore();
+    QApplication::sendEvent(&edit, &tab);
+
+    QCOMPARE(edit.text(), QStringLiteral("tag:unread"));
+}
+
+void TestQueryCompleter::returnIsIgnoredWhileThePopupIsHidden()
+{
+    // Enter accepts a completion only while the popup is up. With it closed it
+    // must still reach returnPressed, which is what runs the query.
+    Config config;
+    QLineEdit edit;
+    edit.show();
+    QueryCompleter completer(&edit, config);
+
+    edit.setText(QStringLiteral("tag:unread"));
+    if (QListView *popup = findPopup())
+        popup->hide();
+
+    bool ran = false;
+    connect(&edit, &QLineEdit::returnPressed, &edit, [&ran]() { ran = true; });
+
+    QKeyEvent ret(QEvent::KeyPress, Qt::Key_Return, Qt::NoModifier);
+    QApplication::sendEvent(&edit, &ret);
+
+    QVERIFY(ran);
+    QCOMPARE(edit.text(), QStringLiteral("tag:unread"));
+}
+
+void TestQueryCompleter::focusOpensThePopupOnlyWhenConfigured()
+{
+    // The event filter is now installed unconditionally, so the
+    // completion_on_focus check moved inside it. Both settings still behave.
+    QTemporaryDir dir;
+
+    {
+        Config off;
+        off.load(writeIni(dir, QStringLiteral("[general]\n"
+                                              "completion_on_focus=false\n")));
+        QLineEdit edit;
+        edit.show();
+        QueryCompleter completer(&edit, off);
+        edit.setFocus();
+        QVERIFY(!findPopup() || !findPopup()->isVisible());
+    }
+
+    {
+        Config on;
+        on.load(writeIni(dir, QStringLiteral("[general]\n"
+                                             "completion_on_focus=true\n")));
+        QVERIFY(on.completionOnFocus());
+        QLineEdit edit;
+        edit.show();
+        QueryCompleter completer(&edit, on);
+        QFocusEvent focusIn(QEvent::FocusIn);
+        QApplication::sendEvent(&edit, &focusIn);
+        QVERIFY(findPopup());
+        QVERIFY(findPopup()->isVisible());
+    }
+}
+
 QTEST_MAIN(TestQueryCompleter)
 #include "test_querycompleter.moc"