From a9771007b139aeb369c65293bb871ecd58bc7ac0 Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Wed, 14 Sep 2016 22:55:07 -0400 Subject: [PATCH] Pass string, not index from on_selection_changed. Simplify the CompletionWidget/Completer interface by changing on_selection_changed to pass the newly selected text rather than the index of the newly selected item. This moves the logic from Completer to CompletionWidget but simplifies the interaction between the two and makes testing easier. --- qutebrowser/completion/completer.py | 23 +++++++++------------- qutebrowser/completion/completionwidget.py | 8 ++++++-- tests/unit/completion/test_completer.py | 22 ++++++--------------- 3 files changed, 21 insertions(+), 32 deletions(-) diff --git a/qutebrowser/completion/completer.py b/qutebrowser/completion/completer.py index 8a84d3e41..2316db407 100644 --- a/qutebrowser/completion/completer.py +++ b/qutebrowser/completion/completer.py @@ -173,35 +173,30 @@ class Completer(QObject): "partitioned: {} '{}' {}".format(prefix, center, postfix)) return prefix, center, postfix - @pyqtSlot(QItemSelection) - def on_selection_changed(self, selected): + @pyqtSlot(str) + def on_selection_changed(self, text): """Change the completed part if a new item was selected. Called from the views selectionChanged method. Args: - selected: New selection. - _deselected: Previous selection. + text: Newly selected text. """ - indexes = selected.indexes() - if not indexes: - return - model = self._model() - data = model.data(indexes[0]) - if data is None: + if text is None: return before, center, after = self._partition() - log.completion.debug("Changing {} to '{}'".format(center, data)) + log.completion.debug("Changing {} to '{}'".format(center, text)) try: maxsplit = cmdutils.cmd_dict[before[0]].maxsplit except (KeyError, IndexError): maxsplit = None if maxsplit is None: - data = self._quote(data) + text = self._quote(text) + model = self._model() if model.count() == 1 and config.get('completion', 'quick-complete'): # If we only have one item, we want to apply it immediately # and go on to the next part. - self._change_completed_part(data, before, after, immediate=True) + self._change_completed_part(text, before, after, immediate=True) 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) @@ -209,7 +204,7 @@ class Completer(QObject): else: log.completion.debug("Will ignore next completion update.") self._ignore_change = True - self._change_completed_part(data, before, after) + self._change_completed_part(text, before, after) @pyqtSlot() def schedule_completion_update(self): diff --git a/qutebrowser/completion/completionwidget.py b/qutebrowser/completion/completionwidget.py index d61b32b9c..00e91c03c 100644 --- a/qutebrowser/completion/completionwidget.py +++ b/qutebrowser/completion/completionwidget.py @@ -104,7 +104,7 @@ class CompletionView(QTreeView): """ resize_completion = pyqtSignal() - selection_changed = pyqtSignal(QItemSelection) + selection_changed = pyqtSignal(str) def __init__(self, win_id, parent=None): super().__init__(parent) @@ -310,7 +310,11 @@ class CompletionView(QTreeView): if not self._active: return super().selectionChanged(selected, deselected) - self.selection_changed.emit(selected) + indexes = selected.indexes() + if not indexes: + return + data = self.model().data(indexes[0]) + self.selection_changed.emit(data) def resizeEvent(self, e): """Extend resizeEvent to adjust column size.""" diff --git a/tests/unit/completion/test_completer.py b/tests/unit/completion/test_completer.py index b84751b44..9601af57d 100644 --- a/tests/unit/completion/test_completer.py +++ b/tests/unit/completion/test_completer.py @@ -242,18 +242,13 @@ def test_on_selection_changed(before, newtxt, after, completer_obj, then we expect a space to be appended after the current word. """ model = unittest.mock.Mock() - model.data = unittest.mock.Mock(return_value=newtxt) - indexes = [unittest.mock.Mock()] - selection = unittest.mock.Mock() - selection.indexes = unittest.mock.Mock(return_value=indexes) completion_widget_stub.model.return_value = model def check(quick_complete, count, expected_txt, expected_pos): config_stub.data['completion']['quick-complete'] = quick_complete - model.count = unittest.mock.Mock(return_value=count) + model.count = lambda: count _set_cmd_prompt(status_command_stub, before) - completer_obj.on_selection_changed(selection) - model.data.assert_called_with(indexes[0]) + completer_obj.on_selection_changed(newtxt) assert status_command_stub.text() == expected_txt assert status_command_stub.cursorPosition() == expected_pos @@ -283,16 +278,11 @@ def test_quickcomplete_flicker(status_command_stub, completer_obj, quick-complete an entry after maxsplit. """ model = unittest.mock.Mock() - model.data = unittest.mock.Mock(return_value='http://example.com') - indexes = [unittest.mock.Mock()] - selection = unittest.mock.Mock() - selection.indexes = unittest.mock.Mock(return_value=indexes) - completion_widget_stub.model.return_value = model - - config_stub.data['completion']['quick-complete'] = True model.count = unittest.mock.Mock(return_value=1) - _set_cmd_prompt(status_command_stub, ':open |') - completer_obj.on_selection_changed(selection) + completion_widget_stub.model.return_value = model + config_stub.data['completion']['quick-complete'] = 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