Move completion_item_{next,prev} to CompletionView.

These commands are more closely tied to the CompletionView than
Completer. This removes the need for an extra signal tying the
CompletionView to the Completer.

The call to _open_completion_if_needed was moved to
on_selection_changed, as this will already be called when a new item is
selected.

qutebrowser/completion/completer.py

@@ -42,15 +42,8 @@ class Completer(QObject):
         _last_text: The old command text so we avoid double completion updates.
         _signals_connected: Whether the signals are connected to update the
                             completion when the command widget requests that.
-
-    Signals:
-        next_prev_item: Emitted to select the next/previous item in the
-                        completion.
-                        arg0: True for the previous item, False for the next.
     """
 
-    next_prev_item = pyqtSignal(bool)
-
     def __init__(self, cmd, win_id, parent=None):
         super().__init__(parent)
         self._win_id = win_id
@@ -258,6 +251,7 @@ class Completer(QObject):
             selected: New selection.
             _deselected: Previous selection.
         """
+        self._open_completion_if_needed()
         indexes = selected.indexes()
         if not indexes:
             return
@@ -470,17 +464,3 @@ class Completer(QObject):
         # We also want to update the cursor part and emit _update_completion
         # here, but that's already done for us by cursorPositionChanged
         # anyways, so we don't need to do it twice.
-
-    @cmdutils.register(instance='completer', hide=True,
-                       modes=[usertypes.KeyMode.command], scope='window')
-    def completion_item_prev(self):
-        """Select the previous completion item."""
-        self._open_completion_if_needed()
-        self.next_prev_item.emit(True)
-
-    @cmdutils.register(instance='completer', hide=True,
-                       modes=[usertypes.KeyMode.command], scope='window')
-    def completion_item_next(self):
-        """Select the next completion item."""
-        self._open_completion_if_needed()
-        self.next_prev_item.emit(False)

qutebrowser/completion/completionwidget.py

@@ -181,8 +181,7 @@ class CompletionView(QTreeView):
                 # Item is a real item, not a category header -> success
                 return idx
 
-    @pyqtSlot(bool)
+    def _next_prev_item(self, prev):
-    def on_next_prev_item(self, prev):
         """Handle a tab press for the CompletionView.
 
         Select the previous/next item and write the new text to the
@@ -193,9 +192,6 @@ class CompletionView(QTreeView):
         Args:
             prev: True for prev item, False for next one.
         """
-        if not self.isVisible():
-            # No completion running at the moment, ignore keypress
-            return
         idx = self._next_idx(prev)
         qtutils.ensure_valid(idx)
         self.selectionModel().setCurrentIndex(
@@ -274,6 +270,18 @@ class CompletionView(QTreeView):
             scrollbar.setValue(scrollbar.minimum())
         super().showEvent(e)
 
+    @cmdutils.register(instance='completion', hide=True,
+                       modes=[usertypes.KeyMode.command], scope='window')
+    def completion_item_prev(self):
+        """Select the previous completion item."""
+        self._next_prev_item(True)
+
+    @cmdutils.register(instance='completion', hide=True,
+                       modes=[usertypes.KeyMode.command], scope='window')
+    def completion_item_next(self):
+        """Select the next completion item."""
+        self._next_prev_item(False)
+
     @cmdutils.register(instance='completion', hide=True,
                        modes=[usertypes.KeyMode.command], scope='window')
     def completion_item_del(self):

qutebrowser/mainwindow/mainwindow.py

@@ -160,11 +160,8 @@ class MainWindow(QWidget):
         self._completion = completionwidget.CompletionView(self.win_id, self)
         cmd = objreg.get('status-command', scope='window', window=self.win_id)
         completer_obj = completer.Completer(cmd, self.win_id, self._completion)
-        completer_obj.next_prev_item.connect(self._completion.on_next_prev_item)
         self._completion.selection_changed.connect(
             completer_obj.on_selection_changed)
-        objreg.register('completer', completer_obj, scope='window',
-                        window=self.win_id)
         objreg.register('completion', self._completion, scope='window',
                         window=self.win_id)
 

tests/unit/completion/test_completer.py

@@ -187,24 +187,6 @@ def test_update_completion(txt, expected, status_command_stub, completer_obj,
         assert arg.srcmodel.kind == expected
 
 
-def test_completion_item_prev(completer_obj, status_command_stub, config_stub,
-                              qtbot):
-    """Test that completion_item_prev emits next_prev_item."""
-    status_command_stub.setText(':')
-    with qtbot.waitSignal(completer_obj.next_prev_item) as blocker:
-        completer_obj.completion_item_prev()
-    assert blocker.args == [True]
-
-
-def test_completion_item_next(completer_obj, status_command_stub, config_stub,
-                              qtbot):
-    """Test that completion_item_next emits next_prev_item."""
-    status_command_stub.setText(':')
-    with qtbot.waitSignal(completer_obj.next_prev_item) as blocker:
-        completer_obj.completion_item_next()
-    assert blocker.args == [False]
-
-
 @pytest.mark.parametrize('before, newtxt, quick_complete, count, after', [
     (':foo |', 'bar', False, 1, ':foo bar|'),
     (':foo |', 'bar', True, 2, ':foo bar|'),

tests/unit/completion/test_completionwidget.py

@@ -120,9 +120,7 @@ def test_maybe_resize_completion(completionview, config_stub, qtbot):
     ([['Aa'], [], []], 1, 'Aa'),
     ([['Aa'], [], []], -1, 'Aa'),
 ])
-def test_on_next_prev_item(tree, count, expected, completionview,
+def test_completion_item_next_prev(tree, count, expected, completionview):
-                           config_stub, qtbot, monkeypatch,
-                           status_command_stub):
     """Test that on_next_prev_item moves the selection properly.
 
     Args:
@@ -140,10 +138,11 @@ def test_on_next_prev_item(tree, count, expected, completionview,
     filtermodel = sortfilter.CompletionFilterModel(model,
                                                    parent=completionview)
     completionview.set_model(filtermodel)
-    # actually calling show() will pop a window during the test, so just fool
+    if count < 0:
-    # the completionview into thinking it is visible instead
+        for _ in range(-count):
-    monkeypatch.setattr(completionview, 'isVisible', lambda: True)
+            completionview.completion_item_prev()
-    for _ in range(abs(count)):
+    else:
-        completionview.on_next_prev_item(count < 0)
+        for _ in range(count):
+            completionview.completion_item_next()
     idx = completionview.selectionModel().currentIndex()
     assert filtermodel.data(idx) == expected