From 80db69904d2004922459ecfe22634deb7ae00505 Mon Sep 17 00:00:00 2001 From: Marshall Lochbaum Date: Mon, 8 Aug 2016 16:41:19 -0400 Subject: [PATCH 01/13] Move auto-open handling to CompletionView from Completer --- qutebrowser/completion/completer.py | 65 +------------------ qutebrowser/completion/completionwidget.py | 24 ++++--- .../unit/completion/test_completionwidget.py | 10 --- 3 files changed, 14 insertions(+), 85 deletions(-) diff --git a/qutebrowser/completion/completer.py b/qutebrowser/completion/completer.py index 569e105be..d6d33ed94 100644 --- a/qutebrowser/completion/completer.py +++ b/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. diff --git a/qutebrowser/completion/completionwidget.py b/qutebrowser/completion/completionwidget.py index 3c608951f..b6ee2cc65 100644 --- a/qutebrowser/completion/completionwidget.py +++ b/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() diff --git a/tests/unit/completion/test_completionwidget.py b/tests/unit/completion/test_completionwidget.py index f07bfc96b..3d7c082ea 100644 --- a/tests/unit/completion/test_completionwidget.py +++ b/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') From b2067ef1866fa8d389acc3ac818320e7d3bc0d48 Mon Sep 17 00:00:00 2001 From: Marshall Lochbaum Date: Mon, 8 Aug 2016 18:15:13 -0400 Subject: [PATCH 02/13] Remove unused _signals_connected --- qutebrowser/completion/completer.py | 1 - 1 file changed, 1 deletion(-) diff --git a/qutebrowser/completion/completer.py b/qutebrowser/completion/completer.py index d6d33ed94..08201443a 100644 --- a/qutebrowser/completion/completer.py +++ b/qutebrowser/completion/completer.py @@ -46,7 +46,6 @@ class Completer(QObject): 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() From d98c81c9d692ca3511dd210c1b99208ff5ab5fc8 Mon Sep 17 00:00:00 2001 From: Marshall Lochbaum Date: Tue, 9 Aug 2016 13:03:38 -0400 Subject: [PATCH 03/13] Correct widget behavior when there is no completion --- qutebrowser/completion/completer.py | 7 ++----- qutebrowser/completion/completionwidget.py | 20 ++++++++++++++++---- tests/unit/completion/test_completer.py | 8 ++++---- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/qutebrowser/completion/completer.py b/qutebrowser/completion/completer.py index 08201443a..1434d549e 100644 --- a/qutebrowser/completion/completer.py +++ b/qutebrowser/completion/completer.py @@ -252,16 +252,13 @@ 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) + completion.set_model(model) if model is None: log.completion.debug("No completion model for {}.".format(parts)) diff --git a/qutebrowser/completion/completionwidget.py b/qutebrowser/completion/completionwidget.py index b6ee2cc65..223e46355 100644 --- a/qutebrowser/completion/completionwidget.py +++ b/qutebrowser/completion/completionwidget.py @@ -47,6 +47,7 @@ class CompletionView(QTreeView): _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. @@ -112,6 +113,7 @@ class CompletionView(QTreeView): # 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) @@ -187,6 +189,8 @@ class CompletionView(QTreeView): Args: which: 'next' or 'prev' """ + if not self._active: + return selmodel = self.selectionModel() idx = self._next_idx(which == 'prev') @@ -207,22 +211,28 @@ class CompletionView(QTreeView): Args: model: The model to use. """ - if (config.get('completion', 'auto-open') and - config.get('completion', 'show')): - self.show() - else: + if model is None: + self._active = False self.hide() + return old_model = self.model() sel_model = self.selectionModel() self.setModel(model) + self._active = True if sel_model is not None: sel_model.deleteLater() if old_model is not None: old_model.deleteLater() + if (config.get('completion', 'auto-open') and + config.get('completion', 'show')): + self.show() + else: + self.hide() + for i in range(model.rowCount()): self.expand(model.index(i, 0)) @@ -258,6 +268,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) diff --git a/tests/unit/completion/test_completer.py b/tests/unit/completion/test_completer.py index f3567674e..5ce9cdada 100644 --- a/tests/unit/completion/test_completer.py +++ b/tests/unit/completion/test_completer.py @@ -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 From 8da8441623815c9681a8d0a5c0a0610cac524e2a Mon Sep 17 00:00:00 2001 From: Marshall Lochbaum Date: Mon, 8 Aug 2016 17:24:23 -0400 Subject: [PATCH 04/13] Merge auto-open option into show --- qutebrowser/completion/completionwidget.py | 5 ++--- qutebrowser/config/config.py | 3 +++ qutebrowser/config/configdata.py | 15 ++++++++------- tests/unit/completion/test_completer.py | 2 +- tests/unit/completion/test_completionwidget.py | 3 +-- 5 files changed, 15 insertions(+), 13 deletions(-) diff --git a/qutebrowser/completion/completionwidget.py b/qutebrowser/completion/completionwidget.py index 223e46355..865a892eb 100644 --- a/qutebrowser/completion/completionwidget.py +++ b/qutebrowser/completion/completionwidget.py @@ -200,7 +200,7 @@ class CompletionView(QTreeView): selmodel.setCurrentIndex( idx, QItemSelectionModel.ClearAndSelect | QItemSelectionModel.Rows) - if config.get('completion', 'show'): + if config.get('completion', 'show') == 'auto': self.show() def set_model(self, model): @@ -227,8 +227,7 @@ class CompletionView(QTreeView): if old_model is not None: old_model.deleteLater() - if (config.get('completion', 'auto-open') and - config.get('completion', 'show')): + if config.get('completion', 'show') == 'always': self.show() else: self.hide() diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py index a5521e275..5de647fb7 100644 --- a/qutebrowser/config/config.py +++ b/qutebrowser/config/config.py @@ -351,6 +351,7 @@ class ConfigManager(QObject): ('tabs', 'hide-always'), ('ui', 'display-statusbar-messages'), ('general', 'wrap-search'), + ('completion', 'auto-open'), ] CHANGED_OPTIONS = { ('content', 'cookies-accept'): @@ -363,6 +364,8 @@ class ConfigManager(QObject): _get_value_transformer({'false': 'none', 'true': 'debug'}), ('ui', 'keyhint-blacklist'): _get_value_transformer({'false': '*', 'true': ''}), + ('completion', 'show'): + _get_value_transformer({'false': 'never', 'true': 'always'}), } changed = pyqtSignal(str, str) diff --git a/qutebrowser/config/configdata.py b/qutebrowser/config/configdata.py index fd0d5925a..f3d6bc6e2 100644 --- a/qutebrowser/config/configdata.py +++ b/qutebrowser/config/configdata.py @@ -427,9 +427,14 @@ def data(readonly=False): )), ('completion', sect.KeyValue( - ('auto-open', - SettingValue(typ.Bool(), 'true'), - "Automatically open completion when typing."), + ('show', + SettingValue(typ.String( + 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( @@ -444,10 +449,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%'), diff --git a/tests/unit/completion/test_completer.py b/tests/unit/completion/test_completer.py index 5ce9cdada..93adf8a0c 100644 --- a/tests/unit/completion/test_completer.py +++ b/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) diff --git a/tests/unit/completion/test_completionwidget.py b/tests/unit/completion/test_completionwidget.py index 3d7c082ea..c9739661e 100644 --- a/tests/unit/completion/test_completionwidget.py +++ b/tests/unit/completion/test_completionwidget.py @@ -34,8 +34,7 @@ def completionview(qtbot, status_command_stub, config_stub, win_registry, """Create the CompletionView used for testing.""" config_stub.data = { 'completion': { - 'show': True, - 'auto-open': True, + 'show': 'always', 'scrollbar-width': 12, 'scrollbar-padding': 2, 'shrink': False, From c5ca102d93a801494abba8f2acecc5f807dfafdd Mon Sep 17 00:00:00 2001 From: Marshall Lochbaum Date: Sat, 20 Aug 2016 01:01:12 -0400 Subject: [PATCH 05/13] Fix completion flicker on quick-complete with show=auto --- qutebrowser/completion/completionwidget.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/qutebrowser/completion/completionwidget.py b/qutebrowser/completion/completionwidget.py index 65ba20a32..9d53df17a 100644 --- a/qutebrowser/completion/completionwidget.py +++ b/qutebrowser/completion/completionwidget.py @@ -238,7 +238,12 @@ class CompletionView(QTreeView): selmodel.setCurrentIndex( idx, QItemSelectionModel.ClearAndSelect | QItemSelectionModel.Rows) - if config.get('completion', 'show') == 'auto': + 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): From 64d53e4cf9bbfb1089a43bf0cf5580fa4aa68494 Mon Sep 17 00:00:00 2001 From: Marshall Lochbaum Date: Sat, 20 Aug 2016 01:01:38 -0400 Subject: [PATCH 06/13] Add unit test for completion show --- .../unit/completion/test_completionwidget.py | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/unit/completion/test_completionwidget.py b/tests/unit/completion/test_completionwidget.py index a5faa0df1..3ab0376ed 100644 --- a/tests/unit/completion/test_completionwidget.py +++ b/tests/unit/completion/test_completionwidget.py @@ -167,3 +167,36 @@ def test_completion_item_focus(which, tree, count, expected, completionview): completionview.completion_item_focus(which) idx = completionview.selectionModel().currentIndex() assert filtermodel.data(idx) == expected + + +@pytest.mark.parametrize('show', ['always', 'auto', 'never']) +@pytest.mark.parametrize('rows', [['Aa'], ['Aa', 'Bb']]) +@pytest.mark.parametrize('quick_complete', [True, False]) +def test_completion_item_focus(show, rows, quick_complete, config_stub, completionview): + """Test that on_next_prev_item moves the selection properly. + + Args: + tree: Each list represents a completion category, with each string + being an item under that category. + count: Number of times to go forward (or back if negative). + expected: item data that should be selected after going back/forward. + """ + 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') + completionview.completion_item_focus('next') + expected = show != 'never' and not (quick_complete and len(rows) == 1) + assert completionview.isVisible() == expected + completionview.set_model(None) + assert not completionview.isVisible() From 2db0ec8b6d178d660a9ee1c99cfc2bd7a7a82f9d Mon Sep 17 00:00:00 2001 From: Marshall Lochbaum Date: Sat, 20 Aug 2016 01:13:32 -0400 Subject: [PATCH 07/13] Fix test_completion_show name and docstring --- tests/unit/completion/test_completionwidget.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/unit/completion/test_completionwidget.py b/tests/unit/completion/test_completionwidget.py index 3ab0376ed..cfb3acb00 100644 --- a/tests/unit/completion/test_completionwidget.py +++ b/tests/unit/completion/test_completionwidget.py @@ -172,14 +172,14 @@ def test_completion_item_focus(which, tree, count, expected, completionview): @pytest.mark.parametrize('show', ['always', 'auto', 'never']) @pytest.mark.parametrize('rows', [['Aa'], ['Aa', 'Bb']]) @pytest.mark.parametrize('quick_complete', [True, False]) -def test_completion_item_focus(show, rows, quick_complete, config_stub, completionview): - """Test that on_next_prev_item moves the selection properly. +def test_completion_show(show, rows, quick_complete, completionview, + config_stub): + """Test that the completion widget is shown at appropriate times. Args: - tree: Each list represents a completion category, with each string - being an item under that category. - count: Number of times to go forward (or back if negative). - expected: item data that should be selected after going back/forward. + 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. """ config_stub.data['completion']['show'] = show config_stub.data['completion']['quick-complete'] = quick_complete From e4cec43cf4a88c98b2045764bb65c46f0e54ebf0 Mon Sep 17 00:00:00 2001 From: Marshall Lochbaum Date: Sat, 20 Aug 2016 01:27:39 -0400 Subject: [PATCH 08/13] Add quick-complete to completion widget tests --- tests/unit/completion/test_completionwidget.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/completion/test_completionwidget.py b/tests/unit/completion/test_completionwidget.py index cfb3acb00..deb7a55f7 100644 --- a/tests/unit/completion/test_completionwidget.py +++ b/tests/unit/completion/test_completionwidget.py @@ -38,6 +38,7 @@ def completionview(qtbot, status_command_stub, config_stub, win_registry, 'scrollbar-width': 12, 'scrollbar-padding': 2, 'shrink': False, + 'quick-complete': False, }, 'colors': { 'completion.fg': QColor(), From cbecd48871e9660f090754b25944e93639d31dca Mon Sep 17 00:00:00 2001 From: Marshall Lochbaum Date: Sun, 21 Aug 2016 22:53:00 -0400 Subject: [PATCH 09/13] Refactor Completer/CompletionView interface, removing set_pattern() --- qutebrowser/completion/completer.py | 20 ++++------- qutebrowser/completion/completionwidget.py | 35 ++++++++----------- .../unit/completion/test_completionwidget.py | 3 +- 3 files changed, 23 insertions(+), 35 deletions(-) diff --git a/qutebrowser/completion/completer.py b/qutebrowser/completion/completer.py index 1434d549e..11d9abe0e 100644 --- a/qutebrowser/completion/completer.py +++ b/qutebrowser/completion/completer.py @@ -257,25 +257,19 @@ class Completer(QObject): model = self._get_new_completion(parts, self._cursor_part) - if model != self._model(): - 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( - "New completion for {}: {}, with pattern '{}'".format( - parts, model.srcmodel.__class__.__name__, pattern)) + if model is None: + log.completion.debug("No completion model for {}.".format(parts)) + else: + log.completion.debug( + "New completion for {}: {}, with pattern '{}'".format( + parts, model.srcmodel.__class__.__name__, pattern)) - if self._model().count() == 0: - completion.hide() + completion.set_model(model, pattern) def _split(self, keep=False): """Get the text split up in parts. diff --git a/qutebrowser/completion/completionwidget.py b/qutebrowser/completion/completionwidget.py index 9d53df17a..5ca8e29de 100644 --- a/qutebrowser/completion/completionwidget.py +++ b/qutebrowser/completion/completionwidget.py @@ -246,13 +246,14 @@ class CompletionView(QTreeView): elif config.get('completion', 'show') == 'auto': self.show() - def set_model(self, model): + 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 @@ -260,17 +261,19 @@ class CompletionView(QTreeView): return old_model = self.model() - sel_model = self.selectionModel() + if model != old_model: + sel_model = self.selectionModel() - self.setModel(model) - self._active = True + self.setModel(model) + self._active = True - if sel_model is not None: - sel_model.deleteLater() - if old_model is not None: - old_model.deleteLater() + if sel_model is not None: + sel_model.deleteLater() + if old_model is not None: + old_model.deleteLater() - if config.get('completion', 'show') == 'always': + if (config.get('completion', 'show') == 'always' + and model.count() > 0): self.show() else: self.hide() @@ -278,21 +281,13 @@ class CompletionView(QTreeView): 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.""" diff --git a/tests/unit/completion/test_completionwidget.py b/tests/unit/completion/test_completionwidget.py index deb7a55f7..9e67a83db 100644 --- a/tests/unit/completion/test_completionwidget.py +++ b/tests/unit/completion/test_completionwidget.py @@ -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_pattern('foo') + completionview.set_model(model, 'foo') model.set_pattern.assert_called_with('foo') From fb9867b6a7a8c4bdd8a68611c30641ed4fd613b5 Mon Sep 17 00:00:00 2001 From: Marshall Lochbaum Date: Sun, 21 Aug 2016 22:57:16 -0400 Subject: [PATCH 10/13] Add tests for completion show with no rows --- tests/unit/completion/test_completionwidget.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/unit/completion/test_completionwidget.py b/tests/unit/completion/test_completionwidget.py index 9e67a83db..82c0e076a 100644 --- a/tests/unit/completion/test_completionwidget.py +++ b/tests/unit/completion/test_completionwidget.py @@ -170,7 +170,7 @@ def test_completion_item_focus(which, tree, count, expected, completionview): @pytest.mark.parametrize('show', ['always', 'auto', 'never']) -@pytest.mark.parametrize('rows', [['Aa'], ['Aa', 'Bb']]) +@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): @@ -194,9 +194,10 @@ def test_completion_show(show, rows, quick_complete, completionview, assert not completionview.isVisible() completionview.set_model(filtermodel) - assert completionview.isVisible() == (show == 'always') + assert completionview.isVisible() == (show == 'always' and len(rows) > 0) completionview.completion_item_focus('next') - expected = show != 'never' and not (quick_complete and len(rows) == 1) + expected = (show != 'never' and len(rows) > 0 and + not (quick_complete and len(rows) == 1)) assert completionview.isVisible() == expected completionview.set_model(None) assert not completionview.isVisible() From 994c6e35d962f49ac19a7ec342fad080d7dab3dc Mon Sep 17 00:00:00 2001 From: Marshall Lochbaum Date: Sun, 21 Aug 2016 23:05:56 -0400 Subject: [PATCH 11/13] Test completion show when calling next with no completion --- tests/unit/completion/test_completionwidget.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/completion/test_completionwidget.py b/tests/unit/completion/test_completionwidget.py index 82c0e076a..2f82799f2 100644 --- a/tests/unit/completion/test_completionwidget.py +++ b/tests/unit/completion/test_completionwidget.py @@ -200,4 +200,5 @@ def test_completion_show(show, rows, quick_complete, completionview, not (quick_complete and len(rows) == 1)) assert completionview.isVisible() == expected completionview.set_model(None) + completionview.completion_item_focus('next') assert not completionview.isVisible() From 2aef26c58b7d538b62f14d210f8efaeb1a1e3f54 Mon Sep 17 00:00:00 2001 From: Marshall Lochbaum Date: Sun, 21 Aug 2016 23:22:46 -0400 Subject: [PATCH 12/13] Linting --- qutebrowser/completion/completionwidget.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qutebrowser/completion/completionwidget.py b/qutebrowser/completion/completionwidget.py index 5ca8e29de..b01a8cb3e 100644 --- a/qutebrowser/completion/completionwidget.py +++ b/qutebrowser/completion/completionwidget.py @@ -272,8 +272,8 @@ class CompletionView(QTreeView): if old_model is not None: old_model.deleteLater() - if (config.get('completion', 'show') == 'always' - and model.count() > 0): + if (config.get('completion', 'show') == 'always' and + model.count() > 0): self.show() else: self.hide() From 44411e02c61904cc2da364022c25a0b49efd71be Mon Sep 17 00:00:00 2001 From: Marshall Lochbaum Date: Mon, 22 Aug 2016 13:02:13 -0400 Subject: [PATCH 13/13] Replace != with (is not) when testing whether a model is new --- qutebrowser/completion/completionwidget.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qutebrowser/completion/completionwidget.py b/qutebrowser/completion/completionwidget.py index b01a8cb3e..778140167 100644 --- a/qutebrowser/completion/completionwidget.py +++ b/qutebrowser/completion/completionwidget.py @@ -261,7 +261,7 @@ class CompletionView(QTreeView): return old_model = self.model() - if model != old_model: + if model is not old_model: sel_model = self.selectionModel() self.setModel(model)