diff options
| -rw-r--r-- | src/querycompleter.cpp | 61 | ||||
| -rw-r--r-- | src/querycompleter.h | 4 | ||||
| -rw-r--r-- | tests/test_querycompleter.cpp | 146 |
3 files changed, 208 insertions, 3 deletions
diff --git a/src/querycompleter.cpp b/src/querycompleter.cpp index b4968f5..a6a1aeb 100644 --- a/src/querycompleter.cpp +++ b/src/querycompleter.cpp @@ -367,12 +367,27 @@ QueryCompleter::QueryCompleter(QLineEdit *edit, const Config &config, connect(m_completer, QOverload<const QModelIndex &>::of(&QCompleter::activated), this, [this](const QModelIndex &index) { + // The mouse path. It must chain exactly like Tab does: the user's + // report was that clicking "tag:" offered no tags afterwards. acceptCompletion(index.data(Qt::DisplayRole).toString()); + continueCompletion(); }); - // 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. + // Two filters, because the two jobs need different vantage points. + // + // The line edit filter handles FocusIn, which by definition arrives while + // the popup is down and the edit is the delivery target, so watching the + // widget is both sufficient and correctly scoped. m_edit->installEventFilter(this); + + // The key filter must be application-wide. Showing the popup takes focus + // away from the line edit (focusWidget() becomes null) and the popup window + // grabs the keyboard, so keys pressed while it is up are delivered to the + // popup and a filter on the line edit never runs. That is precisely when + // Tab and Return need to be intercepted. Only an application filter sees + // those events. It is inert unless our own popup is visible. + if (QCoreApplication *app = QCoreApplication::instance()) + app->installEventFilter(this); } void QueryCompleter::triggerCompletion() @@ -401,6 +416,9 @@ bool QueryCompleter::eventFilter(QObject *watched, QEvent *event) return QObject::eventFilter(watched, event); } + // This filter is installed on the application, so it sees every key in the + // process. Claim nothing unless our own popup is on screen, otherwise the + // keyboard breaks everywhere else in the window. if (event->type() != QEvent::KeyPress || !popupVisible()) return QObject::eventFilter(watched, event); @@ -419,9 +437,13 @@ bool QueryCompleter::eventFilter(QObject *watched, QEvent *event) return QObject::eventFilter(watched, event); acceptCompletion(index.data(Qt::DisplayRole).toString()); + // Hide before reopening: accepting "tag:" moves the caret into value + // position, and the popup has to be rebuilt around the new context + // rather than left showing the prefix list. m_popup->hide(); + continueCompletion(); // Consume it. Tab would otherwise move focus to the next widget, and - // Return would run the half-typed query. + // Return would run the half-typed query or reach the thread list. return true; } case Qt::Key_Escape: @@ -474,6 +496,39 @@ void QueryCompleter::acceptCompletion(const QString &value) m_context = completionContext(m_edit->text(), m_edit->cursorPosition()); } +void QueryCompleter::continueCompletion() +{ + if (!m_edit || !m_completer) + return; + + // acceptCompletion() already recomputed the context from the caret it + // placed, so this reads the situation the accept created. + if (m_context.kind == CompletionContext::None) + return; + + // Reopen only for a value whose keyword actually offers candidates. + // Accepting a prefix ("tag:") lands here with an empty stem and the tag + // list waiting, which is the case worth reopening for. Accepting a value + // ("tag:unread") leaves a stem that already equals the only match, so + // reopening would show a one-entry popup that swallows the next Return. + if (m_context.kind != CompletionContext::Value) + return; + + const QStringList candidates = candidatesFor(m_context); + if (candidates.isEmpty()) + return; + + // A stem that is already a complete candidate needs nothing more. This is + // also what stops the reopen from recurring: the next accept always + // produces such a stem, so the chain terminates after one step. + if (candidates.contains(m_context.stem, Qt::CaseInsensitive)) + return; + + rebuildModel(m_context); + m_completer->setCompletionPrefix(m_context.stem); + m_completer->complete(); +} + void QueryCompleter::updateContext() { if (!m_edit) diff --git a/src/querycompleter.h b/src/querycompleter.h index 350b21c..6f658f2 100644 --- a/src/querycompleter.h +++ b/src/querycompleter.h @@ -124,6 +124,10 @@ private: /// otherwise. bool popupVisible() const; + /// Reopens the popup when an accepted completion leaves the caret somewhere + /// more can be offered, so taking "tag:" goes straight on to the tag list. + void continueCompletion(); + 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 5ae9312..c1d7530 100644 --- a/tests/test_querycompleter.cpp +++ b/tests/test_querycompleter.cpp @@ -63,6 +63,16 @@ private slots: void tabIsIgnoredWhileThePopupIsHidden(); void returnIsIgnoredWhileThePopupIsHidden(); void focusOpensThePopupOnlyWhenConfigured(); + + // Delivered to the widget the window system actually gives the key to, + // rather than straight to the line edit. While the popup is up that is the + // popup, which has grabbed the keyboard, and a filter on the line edit + // never runs. Sending to the edit hides exactly the bug the user reports. + void tabAcceptsWhenTheKeyGoesToTheGrabbingPopup(); + void returnAcceptsWhenTheKeyGoesToTheGrabbingPopup(); + void acceptingAPrefixReopensThePopupForValues(); + void acceptingAValueDoesNotReopenAnEmptyPopup(); + void keysFallThroughWhileThePopupIsHidden(); }; // Copied from tests/test_config.cpp rather than shared, so the two test files @@ -497,5 +507,141 @@ void TestQueryCompleter::focusOpensThePopupOnlyWhenConfigured() } } +// The widget the window system would hand the next key to. While the popup is +// up it has grabbed the keyboard, so that is the popup and NOT the line edit, +// which has by then lost focus entirely. Routing test keys through here is what +// makes these tests reproduce the user's experience instead of a synthetic one. +static QWidget *keyboardTarget(QLineEdit *edit) +{ + if (QWidget *popup = QApplication::activePopupWidget()) + return popup; + return edit; +} + +void TestQueryCompleter::tabAcceptsWhenTheKeyGoesToTheGrabbingPopup() +{ + Config config; + QLineEdit edit; + edit.show(); + QVERIFY(QTest::qWaitForWindowExposed(&edit)); + edit.setFocus(); + QueryCompleter completer(&edit, config); + + QTest::keyClicks(&edit, QStringLiteral("t")); + QVERIFY(findPopup() && findPopup()->isVisible()); + + QTest::keyClick(keyboardTarget(&edit), Qt::Key_Tab); + + QCOMPARE(edit.text(), QStringLiteral("tag:")); +} + +void TestQueryCompleter::returnAcceptsWhenTheKeyGoesToTheGrabbingPopup() +{ + // Return must be consumed too, or it reaches the thread list and opens a + // thread, which is what the user sees. + Config config; + QLineEdit edit; + edit.show(); + QVERIFY(QTest::qWaitForWindowExposed(&edit)); + edit.setFocus(); + QueryCompleter completer(&edit, config); + + bool ran = false; + connect(&edit, &QLineEdit::returnPressed, &edit, [&ran]() { ran = true; }); + + QTest::keyClicks(&edit, QStringLiteral("t")); + QVERIFY(findPopup() && findPopup()->isVisible()); + + QTest::keyClick(keyboardTarget(&edit), Qt::Key_Return); + + QCOMPARE(edit.text(), QStringLiteral("tag:")); + QVERIFY(!ran); +} + +void TestQueryCompleter::acceptingAPrefixReopensThePopupForValues() +{ + // The user's third complaint: after taking "tag:" the caret sits where a + // tag value goes, so the values must be offered without a second Ctrl+Space. + Config config; + QLineEdit edit; + edit.show(); + QVERIFY(QTest::qWaitForWindowExposed(&edit)); + edit.setFocus(); + QueryCompleter completer(&edit, config); + completer.setTags({ QStringLiteral("unread"), QStringLiteral("inbox") }); + + QTest::keyClicks(&edit, QStringLiteral("t")); + QVERIFY(findPopup() && findPopup()->isVisible()); + + QTest::keyClick(keyboardTarget(&edit), Qt::Key_Tab); + QCOMPARE(edit.text(), QStringLiteral("tag:")); + + 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("unread"), + QStringLiteral("inbox") })); + + // And the chain completes: typing into the reopened popup and accepting + // yields the finished term. + QTest::keyClicks(&edit, QStringLiteral("un")); + QVERIFY(findPopup() && findPopup()->isVisible()); + QTest::keyClick(keyboardTarget(&edit), Qt::Key_Tab); + QCOMPARE(edit.text(), QStringLiteral("tag:unread")); +} + +void TestQueryCompleter::acceptingAValueDoesNotReopenAnEmptyPopup() +{ + // "tag:unread" is complete. Reopening here would put an empty list under + // the caret and swallow the next Return. + Config config; + QLineEdit edit; + edit.show(); + QVERIFY(QTest::qWaitForWindowExposed(&edit)); + edit.setFocus(); + QueryCompleter completer(&edit, config); + completer.setTags({ QStringLiteral("unread") }); + + QTest::keyClicks(&edit, QStringLiteral("tag:un")); + QVERIFY(findPopup() && findPopup()->isVisible()); + + QTest::keyClick(keyboardTarget(&edit), Qt::Key_Tab); + + QCOMPARE(edit.text(), QStringLiteral("tag:unread")); + QVERIFY(!findPopup() || !findPopup()->isVisible()); +} + +void TestQueryCompleter::keysFallThroughWhileThePopupIsHidden() +{ + // The filter is application-wide, so proving it does nothing with the popup + // down is what keeps it from breaking the rest of the application. + Config config; + QLineEdit edit; + edit.show(); + QVERIFY(QTest::qWaitForWindowExposed(&edit)); + edit.setFocus(); + QueryCompleter completer(&edit, config); + + if (QListView *popup = findPopup()) + popup->hide(); + + QLineEdit other; + other.show(); + QVERIFY(QTest::qWaitForWindowExposed(&other)); + other.setFocus(); + + bool ran = false; + connect(&other, &QLineEdit::returnPressed, &other, [&ran]() { ran = true; }); + + QTest::keyClicks(&other, QStringLiteral("hello")); + QTest::keyClick(&other, Qt::Key_Return); + + QCOMPARE(other.text(), QStringLiteral("hello")); + QVERIFY(ran); +} + QTEST_MAIN(TestQueryCompleter) #include "test_querycompleter.moc" |