From f5f11f7f4e6786ef2fa22258c5ed520fe661393e Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Fri, 27 Oct 2017 20:15:33 +0200 Subject: [PATCH 1/5] Remove `_ignore_change` --- qutebrowser/completion/completer.py | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/qutebrowser/completion/completer.py b/qutebrowser/completion/completer.py index 0840d6396..f0efd6751 100644 --- a/qutebrowser/completion/completer.py +++ b/qutebrowser/completion/completer.py @@ -43,7 +43,6 @@ class Completer(QObject): Attributes: _cmd: The statusbar Command object this completer belongs to. - _ignore_change: Whether to ignore the next completion update. _timer: The timer used to trigger the completion update. _last_cursor_pos: The old cursor position so we avoid double completion updates. @@ -54,7 +53,6 @@ class Completer(QObject): def __init__(self, cmd, parent=None): super().__init__(parent) self._cmd = cmd - self._ignore_change = False self._timer = QTimer() self._timer.setSingleShot(True) self._timer.setInterval(0) @@ -178,13 +176,14 @@ class Completer(QObject): text = self._quote(text) model = self._model() if model.count() == 1 and config.val.completion.quick: - # If we only have one item, we want to apply it immediately - # and go on to the next part. - self._change_completed_part(text, before, after, immediate=True) + # If we only have one item, we want to apply it immediately and go + # on to the next part, unless we are quick-completing the part + # after maxsplit, so that we don't keep offering completions + # (see issue #1519) if maxsplit is not None and maxsplit < len(before): - # If we are quick-completing the part after maxsplit, don't - # keep offering completions (see issue #1519) - self._ignore_change = True + self._change_completed_part(text, before, after) + else: + self._change_completed_part(text, before, after, immediate=True) else: self._change_completed_part(text, before, after) @@ -219,12 +218,6 @@ class Completer(QObject): @pyqtSlot() def _update_completion(self): """Check if completions are available and activate them.""" - if self._ignore_change: - log.completion.debug("Ignoring completion update because " - "ignore_change is True.") - self._ignore_change = False - return - completion = self.parent() if self._cmd.prefix() != ':': From 249164eb9b565eb54d37655737f2b0d9a00e7544 Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Fri, 27 Oct 2017 22:25:41 +0200 Subject: [PATCH 2/5] Fix `test_quickcomplete_flicker` The test needed to be fixed because of how the completer behaviour changed. Before: completer always scheduled a completion update on selection changed, but the updates themselves were ignored if not needed. Now: completer only schedules completion updates when actually needed, but never ignores a completion update. So, before it was correct to check whether `set_model` was called, now we must check if the completion was actually scheduled. This can be done by checking the parameters with which `_change_completed_part` is called, since a completion is scheduled only when `immediate=True` --- tests/unit/completion/test_completer.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tests/unit/completion/test_completer.py b/tests/unit/completion/test_completer.py index 8575cf02d..5f1581aa0 100644 --- a/tests/unit/completion/test_completer.py +++ b/tests/unit/completion/test_completer.py @@ -299,6 +299,12 @@ def test_quickcomplete_flicker(status_command_stub, completer_obj, config_stub.val.completion.quick = True _set_cmd_prompt(status_command_stub, ':open |') - completer_obj.on_selection_changed('http://example.com') - completer_obj.schedule_completion_update() - assert not completion_widget_stub.set_model.called + + url = 'http://example.com' + completer_obj._change_completed_part = unittest.mock.Mock() + completer_obj.on_selection_changed(url) + + # no immediate (default is false) + completer_obj._change_completed_part.assert_called_with(url, # text + ['open'], # before + []) # after From dcc53026a39edc9eebb3c416f9ca0974669eb813 Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Tue, 31 Oct 2017 23:14:07 +0100 Subject: [PATCH 3/5] Stay within 79 columns --- qutebrowser/completion/completer.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qutebrowser/completion/completer.py b/qutebrowser/completion/completer.py index f0efd6751..bc0e4991f 100644 --- a/qutebrowser/completion/completer.py +++ b/qutebrowser/completion/completer.py @@ -183,7 +183,8 @@ class Completer(QObject): if maxsplit is not None and maxsplit < len(before): self._change_completed_part(text, before, after) else: - self._change_completed_part(text, before, after, immediate=True) + self._change_completed_part(text, before, after, + immediate=True) else: self._change_completed_part(text, before, after) From 35597a7c0152013f3306d1b1a8c56e95067e7477 Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Tue, 31 Oct 2017 23:15:11 +0100 Subject: [PATCH 4/5] Change tests for trailing-space behaviour change --- tests/unit/completion/test_completer.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/unit/completion/test_completer.py b/tests/unit/completion/test_completer.py index 5f1581aa0..934872f2b 100644 --- a/tests/unit/completion/test_completer.py +++ b/tests/unit/completion/test_completer.py @@ -274,7 +274,11 @@ def test_on_selection_changed(before, newtxt, after, completer_obj, check(True, 2, after_txt, after_pos) # quick-completing a single item should move the cursor ahead by 1 and add - # a trailing space if at the end of the cmd string + # a trailing space if at the end of the cmd string, unless the command has + # maxsplit < len(before) (such as :open in these tests) + if after_txt.startswith(':open'): + return + after_pos += 1 if after_pos > len(after_txt): after_txt += ' ' From c28d68173647e8f068c9e12c3102ffc2cb86b624 Mon Sep 17 00:00:00 2001 From: Luca Benci Date: Thu, 2 Nov 2017 19:42:33 +0100 Subject: [PATCH 5/5] Change test to avoid calling private functions --- tests/unit/completion/test_completer.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/tests/unit/completion/test_completer.py b/tests/unit/completion/test_completer.py index 934872f2b..a32241621 100644 --- a/tests/unit/completion/test_completer.py +++ b/tests/unit/completion/test_completer.py @@ -303,12 +303,11 @@ def test_quickcomplete_flicker(status_command_stub, completer_obj, config_stub.val.completion.quick = True _set_cmd_prompt(status_command_stub, ':open |') + completer_obj.schedule_completion_update() + assert completion_widget_stub.set_model.called + completion_widget_stub.set_model.reset_mock() - url = 'http://example.com' - completer_obj._change_completed_part = unittest.mock.Mock() - completer_obj.on_selection_changed(url) - - # no immediate (default is false) - completer_obj._change_completed_part.assert_called_with(url, # text - ['open'], # before - []) # after + # selecting a completion should not re-set the model + completer_obj.on_selection_changed('http://example.com') + completer_obj.schedule_completion_update() + assert not completion_widget_stub.set_model.called