Merge branch 'mlochbaum-auto-open-fixes'

CHANGELOG.asciidoc

@@ -101,6 +101,8 @@ Changed
 - `tabs -> title-format` now treats an empty string as valid.
 - Bindings for `:`, `/` and `?` are now configured explicitly and not hardcoded
   anymore.
+- The `completion -> show` setting can now be set to `always`, `auto` or
+  `never`.
 
 Deprecated
 ~~~~~~~~~~
@@ -120,6 +122,8 @@ Removed
 - The `ui -> hide-mouse-cursor` setting since it was completely broken and
   nobody seemed to care.
 - The `hints -> opacity` setting - see the "Changed" section for details.
+- The `completion -> auto-open` setting got merged into `completion -> show` and
+  thus removed.
 
 Fixed
 ~~~~~

README.asciidoc

@@ -146,9 +146,9 @@ Contributors, sorted by the number of commits in descending order:
 * Antoni Boucher
 * Lamar Pavel
 * Jan Verbeek
+* Marshall Lochbaum
 * Bruno Oliveira
 * Alexander Cogneau
-* Marshall Lochbaum
 * Felix Van der Jeugt
 * Martin Tournoij
 * Raphael Pierzina

doc/help/settings.asciidoc

@@ -74,10 +74,9 @@
 [options="header",width="75%",cols="25%,75%"]
 |==============
 |Setting|Description
-|<<completion-auto-open,auto-open>>|Automatically open completion when typing.
+|<<completion-show,show>>|When to show the autocompletion window.
 |<<completion-download-path-suggestion,download-path-suggestion>>|What to display in the download filename input.
 |<<completion-timestamp-format,timestamp-format>>|How to format timestamps (e.g. for history)
-|<<completion-show,show>>|Whether to show the autocompletion window.
 |<<completion-height,height>>|The height of the completion, in px or as percentage of the window.
 |<<completion-cmd-history-max-items,cmd-history-max-items>>|How many commands to save in the command history.
 |<<completion-web-history-max-items,web-history-max-items>>|How many URLs to show in the web history.
@@ -803,16 +802,17 @@ This setting is only available with the QtWebKit backend.
 == completion
 Options related to completion and command history.
 
-[[completion-auto-open]]
+[[completion-show]]
-=== auto-open
+=== show
-Automatically open completion when typing.
+When to show the autocompletion window.
 
 Valid values:
 
- * +true+
+ * +always+: Whenever a completion is available.
- * +false+
+ * +auto+: Whenever a completion is requested.
+ * +never+: Never.
 
-Default: +pass:[true]+
+Default: +pass:[always]+
 
 [[completion-download-path-suggestion]]
 === download-path-suggestion
@@ -832,17 +832,6 @@ How to format timestamps (e.g. for history)
 
 Default: +pass:[%Y-%m-%d]+
 
-[[completion-show]]
-=== show
-Whether to show the autocompletion window.
-
-Valid values:
-
- * +true+
- * +false+
-
-Default: +pass:[true]+
-
 [[completion-height]]
 === height
 The height of the completion, in px or as percentage of the window.

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,15 +40,12 @@ 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):
         super().__init__(parent)
         self._win_id = win_id
         self._cmd = cmd
-        self._signals_connected = False
         self._ignore_change = False
         self._empty_item_idx = None
         self._timer = QTimer()
@@ -58,64 +55,12 @@ class Completer(QObject):
         self._cursor_part = None
         self._last_cursor_pos = None
         self._last_text = None
-
+        self._cmd.update_completion.connect(self.schedule_completion_update)
-        objreg.get('config').changed.connect(self._on_auto_open_changed)
+        self._cmd.textChanged.connect(self._on_text_edited)
-        self._handle_signal_connections()
-        self._cmd.clear_completion_selection.connect(
-            self._handle_signal_connections)
 
     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 +189,6 @@ class Completer(QObject):
             selected: New selection.
             _deselected: Previous selection.
         """
-        self._open_completion_if_needed()
         indexes = selected.indexes()
         if not indexes:
             return
@@ -308,37 +252,24 @@ class Completer(QObject):
             # anything (yet)
             # FIXME complete searches
             # https://github.com/The-Compiler/qutebrowser/issues/32
-            completion.hide()
+            completion.set_model(None)
             return
 
         model = self._get_new_completion(parts, self._cursor_part)
 
-        if model != self._model():
-            if model is None:
-                completion.hide()
-            else:
-                completion.set_model(model)
-
-        if model is None:
-            log.completion.debug("No completion model for {}.".format(parts))
-            return
-
         try:
             pattern = parts[self._cursor_part].strip()
         except IndexError:
             pattern = ''
-        completion.set_pattern(pattern)
 
-        log.completion.debug(
+        if model is None:
-            "New completion for {}: {}, with pattern '{}'".format(
+            log.completion.debug("No completion model for {}.".format(parts))
-                parts, model.srcmodel.__class__.__name__, pattern))
+        else:
+            log.completion.debug(
+                "New completion for {}: {}, with pattern '{}'".format(
+                    parts, model.srcmodel.__class__.__name__, pattern))
 
-        if self._model().count() == 0:
+        completion.set_model(model, pattern)
-            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,12 +42,12 @@ 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.
         _delegate: The item delegate used.
         _column_widths: A list of column widths, in percent.
+        _active: Whether a selection is active.
 
     Signals:
         resize_completion: Emitted when the completion should be resized.
@@ -109,12 +109,11 @@ 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)
 
         self._column_widths = base.BaseCompletionModel.COLUMN_WIDTHS
+        self._active = False
 
         self._delegate = completiondelegate.CompletionItemDelegate(self)
         self.setItemDelegate(self._delegate)
@@ -220,11 +219,9 @@ class CompletionView(QTreeView):
         Args:
             which: 'next', 'prev', 'next-category', or 'prev-category'.
         """
-        # selmodel can be None if 'show' and 'auto-open' are set to False
+        if not self._active:
-        # https://github.com/The-Compiler/qutebrowser/issues/1731
-        selmodel = self.selectionModel()
-        if selmodel is None:
             return
+        selmodel = self.selectionModel()
 
         if which == 'next':
             idx = self._next_idx(upwards=False)
@@ -243,56 +240,66 @@ class CompletionView(QTreeView):
         selmodel.setCurrentIndex(
             idx, QItemSelectionModel.ClearAndSelect | QItemSelectionModel.Rows)
 
-    def set_model(self, model):
+        count = self.model().count()
+        if count == 0:
+            self.hide()
+        elif count == 1 and config.get('completion', 'quick-complete'):
+            self.hide()
+        elif config.get('completion', 'show') == 'auto':
+            self.show()
+
+    def set_model(self, model, pattern=None):
         """Switch completion to a new model.
 
         Called from on_update_completion().
 
         Args:
             model: The model to use.
+            pattern: The filter pattern to set (what the user entered).
         """
+        if model is None:
+            self._active = False
+            self.hide()
+            return
+
         old_model = self.model()
-        sel_model = self.selectionModel()
+        if model is not old_model:
+            sel_model = self.selectionModel()
 
-        self.setModel(model)
+            self.setModel(model)
+            self._active = True
 
-        if sel_model is not None:
+            if sel_model is not None:
-            sel_model.deleteLater()
+                sel_model.deleteLater()
-        if old_model is not None:
+            if old_model is not None:
-            old_model.deleteLater()
+                old_model.deleteLater()
+
+        if (config.get('completion', 'show') == 'always' and
+                model.count() > 0):
+            self.show()
+        else:
+            self.hide()
 
         for i in range(model.rowCount()):
             self.expand(model.index(i, 0))
 
+        if pattern is not None:
+            model.set_pattern(pattern)
+
         self._column_widths = model.srcmodel.COLUMN_WIDTHS
         self._resize_columns()
         self.maybe_resize_completion()
 
-    def set_pattern(self, pattern):
-        """Set the completion pattern for the current model.
-
-        Called from on_update_completion().
-
-        Args:
-            pattern: The filter pattern to set (what the user entered).
-        """
-        self.model().set_pattern(pattern)
-        self.maybe_resize_completion()
-
     @pyqtSlot()
     def maybe_resize_completion(self):
         """Emit the resize_completion signal if the config says so."""
         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()
@@ -300,6 +307,8 @@ class CompletionView(QTreeView):
 
     def selectionChanged(self, selected, deselected):
         """Extend selectionChanged to call completers selection_changed."""
+        if not self._active:
+            return
         super().selectionChanged(selected, deselected)
         self.selection_changed.emit(selected)
 

qutebrowser/config/config.py

@@ -400,6 +400,7 @@ class ConfigManager(QObject):
         ('ui', 'hide-mouse-cursor'),
         ('general', 'wrap-search'),
         ('hints', 'opacity'),
+        ('completion', 'auto-open'),
     ]
     CHANGED_OPTIONS = {
         ('content', 'cookies-accept'):
@@ -418,6 +419,8 @@ class ConfigManager(QObject):
         ('colors', 'hints.fg'): _transform_hint_color,
         ('colors', 'hints.fg.match'): _transform_hint_color,
         ('fonts', 'hints'): _transform_hint_font,
+        ('completion', 'show'):
+            _get_value_transformer({'false': 'never', 'true': 'always'}),
     }
 
     changed = pyqtSignal(str, str)

qutebrowser/config/configdata.py

@@ -438,9 +438,14 @@ def data(readonly=False):
         )),
 
         ('completion', sect.KeyValue(
-            ('auto-open',
+            ('show',
-             SettingValue(typ.Bool(), 'true'),
+             SettingValue(typ.String(
-             "Automatically open completion when typing."),
+                 valid_values=typ.ValidValues(
+                     ('always', "Whenever a completion is available."),
+                     ('auto', "Whenever a completion is requested."),
+                     ('never', "Never.")
+                 )), 'always'),
+             "When to show the autocompletion window."),
 
             ('download-path-suggestion',
              SettingValue(
@@ -455,10 +460,6 @@ def data(readonly=False):
              SettingValue(typ.TimestampTemplate(none_ok=True), '%Y-%m-%d'),
              "How to format timestamps (e.g. for history)"),
 
-            ('show',
-             SettingValue(typ.Bool(), 'true'),
-             "Whether to show the autocompletion window."),
-
             ('height',
              SettingValue(typ.PercOrInt(minperc=0, maxperc=100, minint=1),
                           '50%'),

tests/unit/completion/test_completer.py

@@ -66,7 +66,7 @@ def completer_obj(qtbot, status_command_stub, config_stub, monkeypatch, stubs,
     """Create the completer used for testing."""
     monkeypatch.setattr('qutebrowser.completion.completer.QTimer',
         stubs.InstaTimer)
-    config_stub.data = {'completion': {'auto-open': False}}
+    config_stub.data = {'completion': {'show': 'auto'}}
     return completer.Completer(status_command_stub, 0, completion_widget_stub)
 
 
@@ -199,12 +199,12 @@ def test_update_completion(txt, expected, status_command_stub, completer_obj,
     # this test uses | as a placeholder for the current cursor position
     _set_cmd_prompt(status_command_stub, txt)
     completer_obj.schedule_completion_update()
+    assert completion_widget_stub.set_model.call_count == 1
+    arg = completion_widget_stub.set_model.call_args[0][0]
+    # the outer model is just for sorting; srcmodel is the completion model
     if expected is None:
-        assert not completion_widget_stub.set_model.called
+        assert arg == expected
     else:
-        assert completion_widget_stub.set_model.call_count == 1
-        arg = completion_widget_stub.set_model.call_args[0][0]
-        # the outer model is just for sorting; srcmodel is the completion model
         assert arg.srcmodel.kind == expected
 
 

tests/unit/completion/test_completionwidget.py

@@ -34,11 +34,11 @@ def completionview(qtbot, status_command_stub, config_stub, win_registry,
     """Create the CompletionView used for testing."""
     config_stub.data = {
         'completion': {
-            'show': True,
+            'show': 'always',
-            'auto-open': True,
             'scrollbar-width': 12,
             'scrollbar-padding': 2,
             'shrink': False,
+            'quick-complete': False,
         },
         'colors': {
             'completion.fg': QColor(),
@@ -83,8 +83,7 @@ def test_set_model(completionview):
 def test_set_pattern(completionview):
     model = sortfilter.CompletionFilterModel(base.BaseCompletionModel())
     model.set_pattern = unittest.mock.Mock()
-    completionview.set_model(model)
+    completionview.set_model(model, 'foo')
-    completionview.set_pattern('foo')
     model.set_pattern.assert_called_with('foo')
 
 
@@ -170,11 +169,36 @@ def test_completion_item_focus(which, tree, count, expected, completionview):
     assert filtermodel.data(idx) == expected
 
 
-def test_completion_item_focus_no_model(completionview):
+@pytest.mark.parametrize('show', ['always', 'auto', 'never'])
-    """Test that next/prev won't crash with no model set.
+@pytest.mark.parametrize('rows', [[], ['Aa'], ['Aa', 'Bb']])
+@pytest.mark.parametrize('quick_complete', [True, False])
+def test_completion_show(show, rows, quick_complete, completionview,
+                         config_stub):
+    """Test that the completion widget is shown at appropriate times.
 
-    This can happen if completion.show and completion.auto-open are False.
+    Args:
-    Regression test for issue #1722.
+        show: The completion show config setting.
+        rows: Each entry represents a completion category with only one item.
+        quick_complete: The completion quick-complete config setting.
     """
-    completionview.completion_item_focus('prev')
+    config_stub.data['completion']['show'] = show
+    config_stub.data['completion']['quick-complete'] = quick_complete
+
+    model = base.BaseCompletionModel()
+    for name in rows:
+        cat = QStandardItem()
+        model.appendRow(cat)
+        cat.appendRow(QStandardItem(name))
+    filtermodel = sortfilter.CompletionFilterModel(model,
+                                                   parent=completionview)
+
+    assert not completionview.isVisible()
+    completionview.set_model(filtermodel)
+    assert completionview.isVisible() == (show == 'always' and len(rows) > 0)
     completionview.completion_item_focus('next')
+    expected = (show != 'never' and len(rows) > 0 and
+                not (quick_complete and len(rows) == 1))
+    assert completionview.isVisible() == expected
+    completionview.set_model(None)
+    completionview.completion_item_focus('next')
+    assert not completionview.isVisible()