diff options
| -rw-r--r-- | src/querycompleter.cpp | 67 | ||||
| -rw-r--r-- | src/querycompleter.h | 5 | ||||
| -rw-r--r-- | tests/test_querycompleter.cpp | 144 |
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" |