Move auto-open handling to CompletionView from Completer

qutebrowser/completion/completer.py

@@ -23,7 +23,7 @@ from PyQt5.QtCore import pyqtSlot, QObject, QTimer, QItemSelection
 
 from qutebrowser.config import config
 from qutebrowser.commands import cmdutils, runners
-from qutebrowser.utils import usertypes, log, objreg, utils
+from qutebrowser.utils import usertypes, log, utils
 from qutebrowser.completion.models import instances, sortfilter
 
 
@@ -40,8 +40,6 @@ class Completer(QObject):
         _last_cursor_pos: The old cursor position so we avoid double completion
                           updates.
         _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.
     """
 
     def __init__(self, cmd, win_id, parent=None):
@@ -58,64 +56,12 @@ class Completer(QObject):
         self._cursor_part = None
         self._last_cursor_pos = None
         self._last_text = None
-
-        objreg.get('config').changed.connect(self._on_auto_open_changed)
-        self._handle_signal_connections()
-        self._cmd.clear_completion_selection.connect(
-            self._handle_signal_connections)
+        self._cmd.update_completion.connect(self.schedule_completion_update)
+        self._cmd.textChanged.connect(self._on_text_edited)
 
     def __repr__(self):
         return utils.get_repr(self)
 
-    @config.change_filter('completion', 'auto-open')
-    def _on_auto_open_changed(self):
-        self._handle_signal_connections()
-
-    @pyqtSlot()
-    def _handle_signal_connections(self):
-        self._connect_signals(config.get('completion', 'auto-open'))
-
-    def _connect_signals(self, connect=True):
-        """Connect or disconnect the completion signals.
-
-        Args:
-            connect: Whether to connect (True) or disconnect (False) the
-                     signals.
-
-        Return:
-            True if the signals were connected (connect=True and aren't
-            connected yet) - otherwise False.
-        """
-        connections = [
-            (self._cmd.update_completion, self.schedule_completion_update),
-            (self._cmd.textChanged, self._on_text_edited),
-        ]
-
-        if connect and not self._signals_connected:
-            for sender, receiver in connections:
-                sender.connect(receiver)
-            self._signals_connected = True
-            return True
-        elif not connect:
-            for sender, receiver in connections:
-                try:
-                    sender.disconnect(receiver)
-                except TypeError:
-                    # Don't fail if not connected
-                    pass
-            self._signals_connected = False
-        return False
-
-    def _open_completion_if_needed(self):
-        """If auto-open is false, temporarily connect signals.
-
-        Also opens the completion.
-        """
-        if not config.get('completion', 'auto-open'):
-            connected = self._connect_signals(True)
-            if connected:
-                self._update_completion()
-
     def _model(self):
         """Convenience method to get the current completion model."""
         completion = self.parent()
@@ -244,7 +190,6 @@ class Completer(QObject):
             selected: New selection.
             _deselected: Previous selection.
         """
-        self._open_completion_if_needed()
         indexes = selected.indexes()
         if not indexes:
             return
@@ -335,10 +280,6 @@ class Completer(QObject):
 
         if self._model().count() == 0:
             completion.hide()
-            return
-
-        if completion.enabled:
-            completion.show()
 
     def _split(self, keep=False):
         """Get the text split up in parts.

qutebrowser/completion/completionwidget.py

@@ -30,7 +30,7 @@ from PyQt5.QtCore import (pyqtSlot, pyqtSignal, Qt, QItemSelectionModel,
 from qutebrowser.config import config, style
 from qutebrowser.completion import completiondelegate
 from qutebrowser.completion.models import base
-from qutebrowser.utils import objreg, utils, usertypes
+from qutebrowser.utils import utils, usertypes
 from qutebrowser.commands import cmdexc, cmdutils
 
 
@@ -42,7 +42,6 @@ class CompletionView(QTreeView):
     headers, and children show as flat list.
 
     Attributes:
-        enabled: Whether showing the CompletionView is enabled.
         _win_id: The ID of the window this CompletionView is associated with.
         _height: The height to use for the CompletionView.
         _height_perc: Either None or a percentage if height should be relative.
@@ -109,8 +108,6 @@ class CompletionView(QTreeView):
     def __init__(self, win_id, parent=None):
         super().__init__(parent)
         self._win_id = win_id
-        self.enabled = config.get('completion', 'show')
-        objreg.get('config').changed.connect(self.set_enabled)
         # FIXME handle new aliases.
         # objreg.get('config').changed.connect(self.init_command_completion)
 
@@ -190,11 +187,7 @@ class CompletionView(QTreeView):
         Args:
             which: 'next' or 'prev'
         """
-        # selmodel can be None if 'show' and 'auto-open' are set to False
-        # https://github.com/The-Compiler/qutebrowser/issues/1731
         selmodel = self.selectionModel()
-        if selmodel is None:
-            return
 
         idx = self._next_idx(which == 'prev')
         if not idx.isValid():
@@ -203,6 +196,9 @@ class CompletionView(QTreeView):
         selmodel.setCurrentIndex(
             idx, QItemSelectionModel.ClearAndSelect | QItemSelectionModel.Rows)
 
+        if config.get('completion', 'show'):
+            self.show()
+
     def set_model(self, model):
         """Switch completion to a new model.
 
@@ -211,6 +207,12 @@ class CompletionView(QTreeView):
         Args:
             model: The model to use.
         """
+        if (config.get('completion', 'auto-open') and
+            config.get('completion', 'show')):
+            self.show()
+        else:
+            self.hide()
+
         old_model = self.model()
         sel_model = self.selectionModel()
 
@@ -245,14 +247,10 @@ class CompletionView(QTreeView):
         if config.get('completion', 'shrink'):
             self.resize_completion.emit()
 
-    @config.change_filter('completion', 'show')
-    def set_enabled(self):
-        """Update self.enabled when the config changed."""
-        self.enabled = config.get('completion', 'show')
-
     @pyqtSlot()
     def on_clear_completion_selection(self):
         """Clear the selection model when an item is activated."""
+        self.hide()
         selmod = self.selectionModel()
         if selmod is not None:
             selmod.clearSelection()

tests/unit/completion/test_completionwidget.py

@@ -145,13 +145,3 @@ def test_completion_item_focus(tree, count, expected, completionview):
         completionview.completion_item_focus(direction)
     idx = completionview.selectionModel().currentIndex()
     assert filtermodel.data(idx) == expected
-
-
-def test_completion_item_focus_no_model(completionview):
-    """Test that next/prev won't crash with no model set.
-
-    This can happen if completion.show and completion.auto-open are False.
-    Regression test for issue #1722.
-    """
-    completionview.completion_item_focus('prev')
-    completionview.completion_item_focus('next')