From eb384fdda00fe32bb35a0e9cbaaa62cef9a27885 Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Mon, 5 Sep 2016 22:07:16 -0400 Subject: [PATCH 01/15] Better Completer.on_selection_changed testing. Preparation for refactoring. --- tests/unit/completion/test_completer.py | 86 ++++++++++++++++--------- 1 file changed, 56 insertions(+), 30 deletions(-) diff --git a/tests/unit/completion/test_completer.py b/tests/unit/completion/test_completer.py index b29f252e1..cb8b49d8a 100644 --- a/tests/unit/completion/test_completer.py +++ b/tests/unit/completion/test_completer.py @@ -147,17 +147,6 @@ def _set_cmd_prompt(cmd, txt): cmd.setCursorPosition(txt.index('|')) -def _validate_cmd_prompt(cmd, txt): - """Interpret fake command prompt text using | as the cursor placeholder. - - Args: - cmd: The command prompt object. - txt: The prompt text, using | as a placeholder for the cursor position. - """ - assert cmd.cursorPosition() == txt.index('|') - assert cmd.text() == txt.replace('|', '') - - @pytest.mark.parametrize('txt, expected', [ (':nope|', usertypes.Completion.command), (':nope |', None), @@ -208,35 +197,72 @@ def test_update_completion(txt, expected, status_command_stub, completer_obj, assert arg.srcmodel.kind == expected -@pytest.mark.parametrize('before, newtxt, quick_complete, count, after', [ - (':foo |', 'bar', False, 1, ':foo bar|'), - (':foo |', 'bar', True, 2, ':foo bar|'), - (':foo |', 'bar', True, 1, ':foo bar |'), - (':foo | bar', 'baz', False, 1, ':foo baz| bar'), - (':foo |', 'bar baz', True, 1, ":foo 'bar baz' |"), - (':foo |', '', True, 1, ":foo '' |"), - (':foo |', None, True, 1, ":foo |"), +@pytest.mark.parametrize('before, newtxt, after', [ + (':|', 'set', ':set|'), + (':| ', 'set', ':set|'), + (': |', 'set', ':set|'), + (':|set', 'set', ':set|'), + (':|set ', 'set', ':set|'), + (':|se', 'set', ':set|'), + (':s|e', 'set', ':set|'), + (':se|', 'set', ':set|'), + (':|se fonts', 'set', ':set| fonts'), + (':set |', 'fonts', ':set fonts|'), + (':set |', 'fonts', ':set fonts|'), + (':set --temp |', 'fonts', ':set --temp fonts|'), + (':set |fo', 'fonts', ':set fonts|'), + (':set f|o', 'fonts', ':set fonts|'), + (':set fo|', 'fonts', ':set fonts|'), + (':set fonts |', 'hints', ':set fonts hints|'), + (':set fonts |nt', 'hints', ':set fonts hints|'), + (':set fonts n|t', 'hints', ':set fonts hints|'), + (':set fonts nt|', 'hints', ':set fonts hints|'), + (':set | hints', 'fonts', ':set fonts| hints'), + (':set | hints', 'fonts', ':set fonts| hints'), + (':set |fo hints', 'fonts', ':set fonts| hints'), + (':set f|o hints', 'fonts', ':set fonts| hints'), + (':set fo| hints', 'fonts', ':set fonts| hints'), + (':set fonts hints |', 'Comic Sans', ":set fonts hints 'Comic Sans'|"), + (":set fonts hints 'Comic Sans'|", '12px Hack', + ":set fonts hints '12px Hack'|"), + (":set fonts hints 'Comic| Sans'", '12px Hack', + ":set fonts hints '12px Hack'|") ]) -def test_on_selection_changed(before, newtxt, count, quick_complete, after, - completer_obj, status_command_stub, - completion_widget_stub, config_stub): +def test_on_selection_changed(before, newtxt, after, completer_obj, + config_stub, status_command_stub, + completion_widget_stub): """Test that on_selection_changed modifies the cmd text properly. The | represents the current cursor position in the cmd prompt. If quick-complete is True and there is only 1 completion (count == 1), then we expect a space to be appended after the current word. """ - config_stub.data['completion']['quick-complete'] = quick_complete model = unittest.mock.Mock() model.data = unittest.mock.Mock(return_value=newtxt) - model.count = unittest.mock.Mock(return_value=count) indexes = [unittest.mock.Mock()] selection = unittest.mock.Mock() selection.indexes = unittest.mock.Mock(return_value=indexes) completion_widget_stub.model.return_value = model - _set_cmd_prompt(status_command_stub, before) - # schedule_completion_update is needed to pick up the cursor position - completer_obj.schedule_completion_update() - completer_obj.on_selection_changed(selection) - model.data.assert_called_with(indexes[0]) - _validate_cmd_prompt(status_command_stub, after) + + 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) + _set_cmd_prompt(status_command_stub, before) + # TODO: refactor so cursor_part is a local rather than class variable + completer_obj._update_cursor_part() + completer_obj.on_selection_changed(selection) + model.data.assert_called_with(indexes[0]) + assert status_command_stub.text() == expected_txt + assert status_command_stub.cursorPosition() == expected_pos + + after_pos = after.index('|') + after_txt = after.replace('|', '') + check(False, 1, after_txt, after_pos) + 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 + after_pos += 1 + if after_pos > len(after_txt): + after_txt += ' ' + check(True, 1, after_txt, after_pos) From 581d7659baa1ebe6f2f0fde6f6e847dcceef282a Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Fri, 9 Sep 2016 07:26:24 -0400 Subject: [PATCH 02/15] Clean up Completer.on_selection_changed. Remove the dependency on the class variables _empty_item_index and _cursor_part to make the code easier to follow. If _update_completion is refactored in a similar way these variables can be removed. --- qutebrowser/completion/completer.py | 52 +++++++++++++++---------- tests/unit/completion/test_completer.py | 3 +- 2 files changed, 32 insertions(+), 23 deletions(-) diff --git a/qutebrowser/completion/completer.py b/qutebrowser/completion/completer.py index 11d9abe0e..b9f7b711f 100644 --- a/qutebrowser/completion/completer.py +++ b/qutebrowser/completion/completer.py @@ -179,6 +179,27 @@ class Completer(QObject): else: return s + def _partition(self): + parts = [x for x in self._split(keep=True) if x] + pos = self._cmd.cursorPosition() - len(self._cmd.prefix()) + log.completion.debug('partitioning {} around position {}'.format(parts, + pos)) + for i, part in enumerate(parts): + pos -= len(part) + if pos <= 0: + if part[pos-1:pos+1].isspace(): + # cursor is in a space between two existing words + parts.insert(i, '') + prefix = [x.strip() for x in parts[:i]] + center = parts[i].strip() + # strip trailing whitepsace included as a separate token + postfix = [x.strip() for x in parts[i+1:] if not x.isspace()] + log.completion.debug( + 'partitioned: {} {} {}'.format(prefix, center, postfix)) + return prefix, center, postfix + log.completion.debug('empty partition result') + return [], '', [] + @pyqtSlot(QItemSelection) def on_selection_changed(self, selected): """Change the completed part if a new item was selected. @@ -355,29 +376,18 @@ class Completer(QObject): including a trailing space and we shouldn't continue completing the current item. """ - parts = self._split() - log.completion.debug("changing part {} to '{}'".format( - self._cursor_part, newtext)) - try: - parts[self._cursor_part] = newtext - except IndexError: - parts.append(newtext) - # We want to place the cursor directly after the part we just changed. - cursor_str = self._cmd.prefix() + ' '.join( - parts[:self._cursor_part + 1]) - if immediate: - # If we should complete immediately, we want to move the cursor by - # one more char, to get to the next field. - cursor_str += ' ' - text = self._cmd.prefix() + ' '.join(parts) - if immediate and self._cursor_part == len(parts) - 1: - # If we should complete immediately and we're completing the last - # part in the commandline, we automatically add a space. + before, center, after = self._partition() + log.completion.debug("changing {} to '{}'".format(center, newtext)) + text = self._cmd.prefix() + ' '.join([*before, newtext]) + pos = len(text) + (1 if immediate else 0) + if after: + text += ' ' + ' '.join(after) + elif immediate: + # pad with a space if quick-completing the last entry text += ' ' + log.completion.debug("setting text = '{}', pos = {}".format(text, pos)) self._cmd.setText(text) - log.completion.debug("Placing cursor after '{}'".format(cursor_str)) - log.modes.debug("Completion triggered, focusing {!r}".format(self)) - self._cmd.setCursorPosition(len(cursor_str)) + self._cmd.setCursorPosition(pos) self._cmd.setFocus() self._cmd.show_cmd.emit() diff --git a/tests/unit/completion/test_completer.py b/tests/unit/completion/test_completer.py index cb8b49d8a..5532e4400 100644 --- a/tests/unit/completion/test_completer.py +++ b/tests/unit/completion/test_completer.py @@ -204,6 +204,7 @@ def test_update_completion(txt, expected, status_command_stub, completer_obj, (':|set', 'set', ':set|'), (':|set ', 'set', ':set|'), (':|se', 'set', ':set|'), + (':|se ', 'set', ':set|'), (':s|e', 'set', ':set|'), (':se|', 'set', ':set|'), (':|se fonts', 'set', ':set| fonts'), @@ -248,8 +249,6 @@ def test_on_selection_changed(before, newtxt, after, completer_obj, config_stub.data['completion']['quick-complete'] = quick_complete model.count = unittest.mock.Mock(return_value=count) _set_cmd_prompt(status_command_stub, before) - # TODO: refactor so cursor_part is a local rather than class variable - completer_obj._update_cursor_part() completer_obj.on_selection_changed(selection) model.data.assert_called_with(indexes[0]) assert status_command_stub.text() == expected_txt From 127412d91c1221b1f6d7c764071aa5bb0d9c60a0 Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Sat, 10 Sep 2016 21:18:59 -0400 Subject: [PATCH 03/15] Simplify update_completion. Remove the class variables _cursor_part and _empty_item_index. Instead, split up the commandline around the cursor whenever that information is needed. Using locals instead of class variables makes the logic easier to follow and ends up requiring much less code. --- qutebrowser/completion/completer.py | 157 +++++------------------- tests/unit/completion/test_completer.py | 1 - 2 files changed, 30 insertions(+), 128 deletions(-) diff --git a/qutebrowser/completion/completer.py b/qutebrowser/completion/completer.py index b9f7b711f..5ed5267dd 100644 --- a/qutebrowser/completion/completer.py +++ b/qutebrowser/completion/completer.py @@ -36,7 +36,6 @@ class Completer(QObject): _ignore_change: Whether to ignore the next completion update. _win_id: The window ID this completer is in. _timer: The timer used to trigger the completion update. - _cursor_part: The cursor part index for the next completion update. _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. @@ -52,11 +51,9 @@ class Completer(QObject): self._timer.setSingleShot(True) self._timer.setInterval(0) self._timer.timeout.connect(self._update_completion) - self._cursor_part = None self._last_cursor_pos = None self._last_text = None 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) @@ -66,23 +63,22 @@ class Completer(QObject): completion = self.parent() return completion.model() - def _get_completion_model(self, completion, parts, cursor_part): + def _get_completion_model(self, completion, pos_args): """Get a completion model based on an enum member. Args: completion: A usertypes.Completion member. - parts: The parts currently in the commandline. - cursor_part: The part the cursor is in. + pos_args: The positional args entered before the cursor. Return: A completion model or None. """ if completion == usertypes.Completion.option: - section = parts[cursor_part - 1] + section = pos_args[0] model = instances.get(completion).get(section) elif completion == usertypes.Completion.value: - section = parts[cursor_part - 2] - option = parts[cursor_part - 1] + section = pos_args[0] + option = pos_args[1] try: model = instances.get(completion)[section][option] except KeyError: @@ -96,72 +92,46 @@ class Completer(QObject): else: return sortfilter.CompletionFilterModel(source=model, parent=self) - def _filter_cmdline_parts(self, parts, cursor_part): - """Filter a list of commandline parts to exclude flags. - - Args: - parts: A list of parts. - cursor_part: The index of the part the cursor is over. - - Return: - A (parts, cursor_part) tuple with the modified values. - """ - if parts == ['']: - # Empty commandline, i.e. only :. - return [''], 0 - filtered_parts = [] - for i, part in enumerate(parts): - if part == '--': - break - elif part.startswith('-'): - if cursor_part >= i: - cursor_part -= 1 - else: - filtered_parts.append(part) - return filtered_parts, cursor_part - - def _get_new_completion(self, parts, cursor_part): + def _get_new_completion(self, before_cursor, under_cursor): """Get a new completion. Args: - parts: The command chunks to get a completion for. - cursor_part: The part the cursor is over currently. + before_cursor: The command chunks before the cursor. + under_cursor: The command chunk under the cursor. Return: A completion model. """ + if '--' in before_cursor: + return None try: - if parts[cursor_part].startswith('-'): + if under_cursor.startswith('-'): # cursor on a flag - return + return None except IndexError: pass - log.completion.debug("Before filtering flags: parts {}, cursor_part " - "{}".format(parts, cursor_part)) - parts, cursor_part = self._filter_cmdline_parts(parts, cursor_part) - log.completion.debug("After filtering flags: parts {}, cursor_part " - "{}".format(parts, cursor_part)) - if not parts: - return None - if cursor_part == 0: + log.completion.debug("Before removing flags: {}".format(before_cursor)) + before_cursor = [x for x in before_cursor if not x.startswith('-')] + log.completion.debug("After removing flags: {}".format(before_cursor)) + if len(before_cursor) == 0: # '|' or 'set|' model = instances.get(usertypes.Completion.command) return sortfilter.CompletionFilterModel(source=model, parent=self) - # delegate completion to command try: - cmd = cmdutils.cmd_dict[parts[0]] + cmd = cmdutils.cmd_dict[before_cursor[0]] except KeyError: - # entering an unknown command + log.completion.debug("No completion for unknown command: {}" + .format(before_cursor[0])) return None + argpos = len(before_cursor) - 1 try: - idx = cursor_part - 1 - completion = cmd.get_pos_arg_info(idx).completion + completion = cmd.get_pos_arg_info(argpos).completion except IndexError: - # user provided more positional arguments than the command takes + log.completion.debug("No completion in position {}".format(argpos)) return None if completion is None: return None - model = self._get_completion_model(completion, parts, cursor_part) + model = self._get_completion_model(completion, before_cursor[1:]) return model def _quote(self, s): @@ -253,12 +223,10 @@ class Completer(QObject): @pyqtSlot() def _update_completion(self): """Check if completions are available and activate them.""" - self._update_cursor_part() - parts = self._split() + before_cursor, pattern, after_cursor = self._partition() - log.completion.debug( - "Updating completion - prefix {}, parts {}, cursor_part {}".format( - self._cmd.prefix(), parts, self._cursor_part)) + log.completion.debug("Updating completion: {} {} {}".format( + before_cursor, pattern, after_cursor)) if self._ignore_change: log.completion.debug("Ignoring completion update because " @@ -276,19 +244,11 @@ class Completer(QObject): completion.set_model(None) return - model = self._get_new_completion(parts, self._cursor_part) + model = self._get_new_completion(before_cursor, pattern) - try: - pattern = parts[self._cursor_part].strip() - except IndexError: - 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)) + log.completion.debug("Setting completion model to {} with pattern '{}'" + .format(model.srcmodel.__class__.__name__ if model else 'None', + pattern)) completion.set_model(model, pattern) @@ -318,55 +278,6 @@ class Completer(QObject): #log.completion.debug("Splitting '{}' -> {}".format(text, parts)) return parts - @pyqtSlot() - def _update_cursor_part(self): - """Get the part index of the commandline where the cursor is over.""" - cursor_pos = self._cmd.cursorPosition() - snippet = slice(cursor_pos - 1, cursor_pos + 1) - spaces = self._cmd.text()[snippet] == ' ' - cursor_pos -= len(self._cmd.prefix()) - parts = self._split(keep=True) - log.completion.vdebug( - "text: {}, parts: {}, cursor_pos after removing prefix '{}': " - "{}".format(self._cmd.text(), parts, self._cmd.prefix(), - cursor_pos)) - skip = 0 - for i, part in enumerate(parts): - log.completion.vdebug("Checking part {}: {!r}".format(i, parts[i])) - if not part: - skip += 1 - continue - if cursor_pos <= len(part): - # foo| bar - self._cursor_part = i - skip - if spaces: - self._empty_item_idx = i - skip - else: - self._empty_item_idx = None - log.completion.vdebug("cursor_pos {} <= len(part) {}, " - "setting cursor_part {} - {} (skip), " - "empty_item_idx {}".format( - cursor_pos, len(part), i, skip, - self._empty_item_idx)) - break - cursor_pos -= len(part) - log.completion.vdebug( - "Removing len({!r}) -> {} from cursor_pos -> {}".format( - part, len(part), cursor_pos)) - else: - if i == 0: - # Initial `:` press without any text. - self._cursor_part = 0 - else: - self._cursor_part = i - skip - if spaces: - self._empty_item_idx = i - skip - else: - self._empty_item_idx = None - log.completion.debug("cursor_part {}, spaces {}".format( - self._cursor_part, spaces)) - return - def _change_completed_part(self, newtext, immediate=False): """Change the part we're currently completing in the commandline. @@ -390,11 +301,3 @@ class Completer(QObject): self._cmd.setCursorPosition(pos) self._cmd.setFocus() self._cmd.show_cmd.emit() - - @pyqtSlot() - def _on_text_edited(self): - """Reset _empty_item_idx if text was edited.""" - self._empty_item_idx = None - # We also want to update the cursor part and emit _update_completion - # here, but that's already done for us by cursorPositionChanged - # anyways, so we don't need to do it twice. diff --git a/tests/unit/completion/test_completer.py b/tests/unit/completion/test_completer.py index 5532e4400..4326370e2 100644 --- a/tests/unit/completion/test_completer.py +++ b/tests/unit/completion/test_completer.py @@ -174,7 +174,6 @@ def _set_cmd_prompt(cmd, txt): (':open -t |', usertypes.Completion.url), (':open --tab |', usertypes.Completion.url), (':open | -t', usertypes.Completion.url), - (':--foo --bar |', None), (':tab-detach |', None), (':bind --mode=caret |', usertypes.Completion.command), pytest.mark.xfail(reason='issue #74')((':bind --mode caret |', From fcadde6aefe9562c04cf3f932ce18c3f244d1b2c Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Sun, 11 Sep 2016 07:56:42 -0400 Subject: [PATCH 04/15] Merge _split into _partition in Completer. After the refactoring, _split is only called by _partition so just make it part of the same method. This also removes the use of _empty_item_index, as it can be figured out on the fly. --- qutebrowser/completion/completer.py | 63 +++++++++++------------------ 1 file changed, 23 insertions(+), 40 deletions(-) diff --git a/qutebrowser/completion/completer.py b/qutebrowser/completion/completer.py index 5ed5267dd..810c7b34a 100644 --- a/qutebrowser/completion/completer.py +++ b/qutebrowser/completion/completer.py @@ -46,7 +46,6 @@ class Completer(QObject): self._win_id = win_id self._cmd = cmd self._ignore_change = False - self._empty_item_idx = None self._timer = QTimer() self._timer.setSingleShot(True) self._timer.setInterval(0) @@ -150,7 +149,18 @@ class Completer(QObject): return s def _partition(self): - parts = [x for x in self._split(keep=True) if x] + """Divide the commandline text into chunks around the cursor position. + + Return: + ([parts_before_cursor], 'part_under_cursor', [parts_after_cursor]) + """ + text = self._cmd.text()[len(self._cmd.prefix()):] + if not text or not text.strip(): + # Only ":", empty part under the cursor with nothing before/after + return [], '', [] + runner = runners.CommandRunner(self._win_id) + result = runner.parse(text, fallback=True, keep=True) + parts = [x for x in result.cmdline if x] pos = self._cmd.cursorPosition() - len(self._cmd.prefix()) log.completion.debug('partitioning {} around position {}'.format(parts, pos)) @@ -165,10 +175,8 @@ class Completer(QObject): # strip trailing whitepsace included as a separate token postfix = [x.strip() for x in parts[i+1:] if not x.isspace()] log.completion.debug( - 'partitioned: {} {} {}'.format(prefix, center, postfix)) + "partitioned: {} '{}' {}".format(prefix, center, postfix)) return prefix, center, postfix - log.completion.debug('empty partition result') - return [], '', [] @pyqtSlot(QItemSelection) def on_selection_changed(self, selected): @@ -187,21 +195,22 @@ class Completer(QObject): data = model.data(indexes[0]) if data is None: return - parts = self._split() + before, center, after = self._partition() + log.completion.debug("Changing {} to '{}'".format(center, data)) try: - needs_quoting = cmdutils.cmd_dict[parts[0]].maxsplit is None - except KeyError: + needs_quoting = cmdutils.cmd_dict[before[0]].maxsplit is None + except (KeyError, IndexError): needs_quoting = True if needs_quoting: data = self._quote(data) 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, immediate=True) + self._change_completed_part(data, before, after, immediate=True) else: log.completion.debug("Will ignore next completion update.") self._ignore_change = True - self._change_completed_part(data) + self._change_completed_part(data, before, after) @pyqtSlot() def schedule_completion_update(self): @@ -252,43 +261,17 @@ class Completer(QObject): completion.set_model(model, pattern) - def _split(self, keep=False): - """Get the text split up in parts. - - Args: - keep: Whether to keep special chars and whitespace. - aliases: Whether to resolve aliases. - """ - text = self._cmd.text()[len(self._cmd.prefix()):] - if not text: - # When only ":" is entered, we already have one imaginary part, - # which just is empty at the moment. - return [''] - if not text.strip(): - # Text is only whitespace so we treat this as a single element with - # the whitespace. - return [text] - runner = runners.CommandRunner(self._win_id) - result = runner.parse(text, fallback=True, keep=keep) - parts = result.cmdline - if self._empty_item_idx is not None: - log.completion.debug("Empty element queued at {}, " - "inserting.".format(self._empty_item_idx)) - parts.insert(self._empty_item_idx, '') - #log.completion.debug("Splitting '{}' -> {}".format(text, parts)) - return parts - - def _change_completed_part(self, newtext, immediate=False): + def _change_completed_part(self, newtext, before, after, immediate=False): """Change the part we're currently completing in the commandline. Args: - text: The text to set (string). + text: The text to set (string) for the token under the cursor. + before: Commandline tokens before the token under the cursor. + after: Commandline tokens after the token under the cursor. immediate: True if the text should be completed immediately including a trailing space and we shouldn't continue completing the current item. """ - before, center, after = self._partition() - log.completion.debug("changing {} to '{}'".format(center, newtext)) text = self._cmd.prefix() + ' '.join([*before, newtext]) pos = len(text) + (1 if immediate else 0) if after: From 808a645b4077595501071d82ca6797e66bb4c64f Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Sun, 11 Sep 2016 08:03:22 -0400 Subject: [PATCH 05/15] Fix quick-complete highlighting quirk. When the commandline reads ':open |', quick-completing the only offered completion will set the commandline to ':open some_url |'. Since `open` has `maxsplit=0`, everything after ':open' is (correctly) treated as one argument. This means completion is opened again with 'some url ' as the pattern (note trailing whitespace), which makes the comletion menu 'flicker' and stay open even though it was 'supposed' to quick compelte. This is fixed by ignoring the next completion request if we just completed something after maxsplit (because we don't expect any more completions after the last split). Resolves #1519. --- qutebrowser/completion/completer.py | 10 +++++-- tests/unit/completion/test_completer.py | 40 +++++++++++++++++++------ 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/qutebrowser/completion/completer.py b/qutebrowser/completion/completer.py index 810c7b34a..a3a6b5765 100644 --- a/qutebrowser/completion/completer.py +++ b/qutebrowser/completion/completer.py @@ -198,15 +198,19 @@ class Completer(QObject): before, center, after = self._partition() log.completion.debug("Changing {} to '{}'".format(center, data)) try: - needs_quoting = cmdutils.cmd_dict[before[0]].maxsplit is None + maxsplit = cmdutils.cmd_dict[before[0]].maxsplit except (KeyError, IndexError): - needs_quoting = True - if needs_quoting: + maxsplit = None + if maxsplit is None: data = self._quote(data) 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) + 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 else: log.completion.debug("Will ignore next completion update.") self._ignore_change = True diff --git a/tests/unit/completion/test_completer.py b/tests/unit/completion/test_completer.py index 4326370e2..16e9cea02 100644 --- a/tests/unit/completion/test_completer.py +++ b/tests/unit/completion/test_completer.py @@ -122,16 +122,12 @@ def cmdutils_patch(monkeypatch, stubs): """docstring.""" pass - cmds = { - 'set': set_command, - 'help': show_help, - 'open': openurl, - 'bind': bind, - 'tab-detach': tab_detach, - } cmd_utils = stubs.FakeCmdUtils({ - name: command.Command(name=name, handler=fn) - for name, fn in cmds.items() + 'set': command.Command(name='set', handler=set_command), + 'help': command.Command(name='help', handler=show_help), + 'open': command.Command(name='open', handler=openurl, maxsplit=0), + 'bind': command.Command(name='bind', handler=bind), + 'tab-detach': command.Command(name='tab-detach', handler=tab_detach), }) monkeypatch.setattr('qutebrowser.completion.completer.cmdutils', cmd_utils) @@ -264,3 +260,29 @@ def test_on_selection_changed(before, newtxt, after, completer_obj, if after_pos > len(after_txt): after_txt += ' ' check(True, 1, after_txt, after_pos) + +def test_quickcomplete_flicker(status_command_stub, completer_obj, + completion_widget_stub, config_stub): + """Validate fix for #1519: bookmark-load background highlighting quirk. + + For commands like bookmark-load and open with maxsplit=0, a commandline + that looks like ':open someurl |' is considered to be completing the first + arg with pattern 'someurl ' (note trailing whitespace). As this matches the + one completion available, it keeps the completionmenu open. + + This test validates that the completion model is not re-set after we + 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) + + completer_obj.schedule_completion_update() + assert not completion_widget_stub.set_model.called From d651cc75b0c6c2f3b9954c1ca5532c332b9c3f92 Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Sun, 11 Sep 2016 18:09:25 -0400 Subject: [PATCH 06/15] Fix flake8/pylint errors. --- qutebrowser/completion/completer.py | 2 +- tests/unit/completion/test_completer.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/qutebrowser/completion/completer.py b/qutebrowser/completion/completer.py index a3a6b5765..993c7cb83 100644 --- a/qutebrowser/completion/completer.py +++ b/qutebrowser/completion/completer.py @@ -276,7 +276,7 @@ class Completer(QObject): including a trailing space and we shouldn't continue completing the current item. """ - text = self._cmd.prefix() + ' '.join([*before, newtext]) + text = self._cmd.prefix() + ' '.join(before + [newtext]) pos = len(text) + (1 if immediate else 0) if after: text += ' ' + ' '.join(after) diff --git a/tests/unit/completion/test_completer.py b/tests/unit/completion/test_completer.py index 16e9cea02..8ed466fa0 100644 --- a/tests/unit/completion/test_completer.py +++ b/tests/unit/completion/test_completer.py @@ -261,6 +261,7 @@ def test_on_selection_changed(before, newtxt, after, completer_obj, after_txt += ' ' check(True, 1, after_txt, after_pos) + def test_quickcomplete_flicker(status_command_stub, completer_obj, completion_widget_stub, config_stub): """Validate fix for #1519: bookmark-load background highlighting quirk. @@ -271,7 +272,8 @@ def test_quickcomplete_flicker(status_command_stub, completer_obj, one completion available, it keeps the completionmenu open. This test validates that the completion model is not re-set after we - quick-complete an entry after maxsplit.""" + quick-complete an entry after maxsplit. + """ model = unittest.mock.Mock() model.data = unittest.mock.Mock(return_value='http://example.com') indexes = [unittest.mock.Mock()] From 52fdad8186fc3120b982b7c445541b7c90273530 Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Sun, 11 Sep 2016 18:19:16 -0400 Subject: [PATCH 07/15] Test on_selection_changed with maxsplit --- tests/unit/completion/test_completer.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/unit/completion/test_completer.py b/tests/unit/completion/test_completer.py index 8ed466fa0..233ba5307 100644 --- a/tests/unit/completion/test_completer.py +++ b/tests/unit/completion/test_completer.py @@ -222,7 +222,10 @@ def test_update_completion(txt, expected, status_command_stub, completer_obj, (":set fonts hints 'Comic Sans'|", '12px Hack', ":set fonts hints '12px Hack'|"), (":set fonts hints 'Comic| Sans'", '12px Hack', - ":set fonts hints '12px Hack'|") + ":set fonts hints '12px Hack'|"), + # open has maxsplit=0, so treat the last two tokens as one and don't quote + (':open foo bar|', 'baz', ':open baz|'), + (':open foo| bar', 'baz', ':open baz|'), ]) def test_on_selection_changed(before, newtxt, after, completer_obj, config_stub, status_command_stub, From 69a3df174d8e893cb60d83ef558822103da10080 Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Sun, 11 Sep 2016 18:19:25 -0400 Subject: [PATCH 08/15] Remove needless try/catch in on_selection_changed. --- qutebrowser/completion/completer.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/qutebrowser/completion/completer.py b/qutebrowser/completion/completer.py index 993c7cb83..9fdc816ed 100644 --- a/qutebrowser/completion/completer.py +++ b/qutebrowser/completion/completer.py @@ -101,14 +101,9 @@ class Completer(QObject): Return: A completion model. """ - if '--' in before_cursor: + if '--' in before_cursor or under_cursor.startswith('-'): + # cursor on a flag or after an explicit split (--) return None - try: - if under_cursor.startswith('-'): - # cursor on a flag - return None - except IndexError: - pass log.completion.debug("Before removing flags: {}".format(before_cursor)) before_cursor = [x for x in before_cursor if not x.startswith('-')] log.completion.debug("After removing flags: {}".format(before_cursor)) From b867b8795571f20e2f7ed7e9d262971cac79c2f0 Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Mon, 12 Sep 2016 12:35:57 -0400 Subject: [PATCH 09/15] Don't crash Completer on unknown command. The CommandRunner's fallback parsing behavior treated whitespace differently than the normal flow. When a user entered an unknown command, trailing whitespace would be stripped and the cmdline length would be less than the cursor position. This is fixed by making the fallback use the ShellLexer just as the 'normal' parsing does. --- qutebrowser/commands/runners.py | 13 +++---------- tests/unit/completion/test_completer.py | 1 + 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/qutebrowser/commands/runners.py b/qutebrowser/commands/runners.py index 970fbadbe..ea10520dc 100644 --- a/qutebrowser/commands/runners.py +++ b/qutebrowser/commands/runners.py @@ -174,15 +174,6 @@ class CommandRunner(QObject): count = None return (count, cmdstr) - def _parse_fallback(self, text, count, keep): - """Parse the given commandline without a valid command.""" - if keep: - cmdstr, sep, argstr = text.partition(' ') - cmdline = [cmdstr, sep] + argstr.split() - else: - cmdline = text.split() - return ParseResult(cmd=None, args=None, cmdline=cmdline, count=count) - def parse(self, text, *, fallback=False, keep=False): """Split the commandline text into command and arguments. @@ -210,7 +201,9 @@ class CommandRunner(QObject): if not fallback: raise cmdexc.NoSuchCommandError( '{}: no such command'.format(cmdstr)) - return self._parse_fallback(text, count, keep) + cmdline = split.split(text, keep=keep) + return ParseResult(cmd=None, args=None, cmdline=cmdline, + count=count) args = self._split_args(cmd, argstr, keep) if keep and args: diff --git a/tests/unit/completion/test_completer.py b/tests/unit/completion/test_completer.py index 233ba5307..6bc651d4f 100644 --- a/tests/unit/completion/test_completer.py +++ b/tests/unit/completion/test_completer.py @@ -176,6 +176,7 @@ def _set_cmd_prompt(cmd, txt): usertypes.Completion.command)), (':set -t -p |', usertypes.Completion.section), (':open -- |', None), + (':gibberish nonesense |', None), ]) def test_update_completion(txt, expected, status_command_stub, completer_obj, completion_widget_stub): From e65aba74fd39f6335634dedc310adbd090a669da Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Mon, 12 Sep 2016 17:20:36 -0400 Subject: [PATCH 10/15] Test pattern for Completion.update_completion. In the update_completion unit test, verify the `pattern` parameter as well as the `model`. --- tests/unit/completion/test_completer.py | 79 +++++++++++++------------ 1 file changed, 40 insertions(+), 39 deletions(-) diff --git a/tests/unit/completion/test_completer.py b/tests/unit/completion/test_completer.py index 6bc651d4f..473058a34 100644 --- a/tests/unit/completion/test_completer.py +++ b/tests/unit/completion/test_completer.py @@ -143,54 +143,55 @@ def _set_cmd_prompt(cmd, txt): cmd.setCursorPosition(txt.index('|')) -@pytest.mark.parametrize('txt, expected', [ - (':nope|', usertypes.Completion.command), - (':nope |', None), - (':set |', usertypes.Completion.section), - (':set gen|', usertypes.Completion.section), - (':set general |', usertypes.Completion.option), - (':set what |', None), - (':set general ignore-case |', usertypes.Completion.value), - (':set general huh |', None), - (':help |', usertypes.Completion.helptopic), - (':help |', usertypes.Completion.helptopic), - (':open |', usertypes.Completion.url), - (':bind |', None), - (':bind |', usertypes.Completion.command), - (':bind foo|', usertypes.Completion.command), - (':bind | foo', None), - (':set| general ', usertypes.Completion.command), - (':|set general ', usertypes.Completion.command), - (':set gene|ral ignore-case', usertypes.Completion.section), - (':|', usertypes.Completion.command), - (': |', usertypes.Completion.command), - ('/|', None), - (':open -t|', None), - (':open --tab|', None), - (':open -t |', usertypes.Completion.url), - (':open --tab |', usertypes.Completion.url), - (':open | -t', usertypes.Completion.url), - (':tab-detach |', None), - (':bind --mode=caret |', usertypes.Completion.command), +@pytest.mark.parametrize('txt, kind, pattern', [ + (':nope|', usertypes.Completion.command, 'nope'), + (':nope |', None, ''), + (':set |', usertypes.Completion.section, ''), + (':set gen|', usertypes.Completion.section, 'gen'), + (':set general |', usertypes.Completion.option, ''), + (':set what |', None, ''), + (':set general ignore-case |', usertypes.Completion.value, ''), + (':set general huh |', None, ''), + (':help |', usertypes.Completion.helptopic, ''), + (':help |', usertypes.Completion.helptopic, ''), + (':open |', usertypes.Completion.url, ''), + (':bind |', None, ''), + (':bind |', usertypes.Completion.command, ''), + (':bind foo|', usertypes.Completion.command, 'foo'), + (':bind | foo', None, ''), + (':set| general ', usertypes.Completion.command, 'set'), + (':|set general ', usertypes.Completion.command, 'set'), + (':set gene|ral ignore-case', usertypes.Completion.section, 'general'), + (':|', usertypes.Completion.command, ''), + (': |', usertypes.Completion.command, ''), + ('/|', None, ''), + (':open -t|', None, ''), + (':open --tab|', None, ''), + (':open -t |', usertypes.Completion.url, ''), + (':open --tab |', usertypes.Completion.url, ''), + (':open | -t', usertypes.Completion.url, ''), + (':tab-detach |', None, ''), + (':bind --mode=caret |', usertypes.Completion.command, ''), pytest.mark.xfail(reason='issue #74')((':bind --mode caret |', - usertypes.Completion.command)), - (':set -t -p |', usertypes.Completion.section), - (':open -- |', None), - (':gibberish nonesense |', None), + usertypes.Completion.command, '')), + (':set -t -p |', usertypes.Completion.section, ''), + (':open -- |', None, ''), + (':gibberish nonesense |', None, ''), ]) -def test_update_completion(txt, expected, status_command_stub, completer_obj, - completion_widget_stub): +def test_update_completion(txt, kind, pattern, status_command_stub, + completer_obj, completion_widget_stub): """Test setting the completion widget's model based on command text.""" # 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] + args = completion_widget_stub.set_model.call_args[0] # the outer model is just for sorting; srcmodel is the completion model - if expected is None: - assert arg == expected + if kind is None: + assert args[0] is None else: - assert arg.srcmodel.kind == expected + assert args[0].srcmodel.kind == kind + assert args[1] == pattern @pytest.mark.parametrize('before, newtxt, after', [ From e23d611b3750c2fcd295c4826dd794311de26278 Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Mon, 12 Sep 2016 21:30:10 -0400 Subject: [PATCH 11/15] Strip quotes from completion pattern. Given a commandline like: `:set general editor "gvim -f"|`, the pattern should be 'gvim -f' rather than '"gvim -f"'. --- qutebrowser/completion/completer.py | 1 + tests/unit/completion/test_completer.py | 7 +++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/qutebrowser/completion/completer.py b/qutebrowser/completion/completer.py index 9fdc816ed..e06187116 100644 --- a/qutebrowser/completion/completer.py +++ b/qutebrowser/completion/completer.py @@ -252,6 +252,7 @@ class Completer(QObject): completion.set_model(None) return + pattern = pattern.strip("'\"") model = self._get_new_completion(before_cursor, pattern) log.completion.debug("Setting completion model to {} with pattern '{}'" diff --git a/tests/unit/completion/test_completer.py b/tests/unit/completion/test_completer.py index 473058a34..b84751b44 100644 --- a/tests/unit/completion/test_completer.py +++ b/tests/unit/completion/test_completer.py @@ -82,7 +82,7 @@ def instances(monkeypatch): } instances[usertypes.Completion.value] = { 'general': { - 'ignore-case': FakeCompletionModel(usertypes.Completion.value), + 'editor': FakeCompletionModel(usertypes.Completion.value), } } monkeypatch.setattr('qutebrowser.completion.completer.instances', @@ -150,7 +150,10 @@ def _set_cmd_prompt(cmd, txt): (':set gen|', usertypes.Completion.section, 'gen'), (':set general |', usertypes.Completion.option, ''), (':set what |', None, ''), - (':set general ignore-case |', usertypes.Completion.value, ''), + (':set general editor |', usertypes.Completion.value, ''), + (':set general editor gv|', usertypes.Completion.value, 'gv'), + (':set general editor "gvim -f"|', usertypes.Completion.value, 'gvim -f'), + (':set general editor "gvim |', usertypes.Completion.value, 'gvim'), (':set general huh |', None, ''), (':help |', usertypes.Completion.helptopic, ''), (':help |', usertypes.Completion.helptopic, ''), From ac030955122a6ff9a9d275a622b97651338b880c Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Wed, 14 Sep 2016 22:35:05 -0400 Subject: [PATCH 12/15] Small if-statement style tweak in Completer. --- qutebrowser/completion/completer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qutebrowser/completion/completer.py b/qutebrowser/completion/completer.py index e06187116..8a84d3e41 100644 --- a/qutebrowser/completion/completer.py +++ b/qutebrowser/completion/completer.py @@ -107,7 +107,7 @@ class Completer(QObject): log.completion.debug("Before removing flags: {}".format(before_cursor)) before_cursor = [x for x in before_cursor if not x.startswith('-')] log.completion.debug("After removing flags: {}".format(before_cursor)) - if len(before_cursor) == 0: + if not before_cursor: # '|' or 'set|' model = instances.get(usertypes.Completion.command) return sortfilter.CompletionFilterModel(source=model, parent=self) From a9771007b139aeb369c65293bb871ecd58bc7ac0 Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Wed, 14 Sep 2016 22:55:07 -0400 Subject: [PATCH 13/15] 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 From a7eea6a0c1764ad38bfd94d8f035b9d55a2b29fd Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Thu, 15 Sep 2016 07:41:56 -0400 Subject: [PATCH 14/15] Rewrite test_on_next_prev_item after refactoring. Check the value of the signal emitted after each one of a series of next/prev_item calls. --- .../unit/completion/test_completionwidget.py | 111 ++++++++---------- 1 file changed, 51 insertions(+), 60 deletions(-) diff --git a/tests/unit/completion/test_completionwidget.py b/tests/unit/completion/test_completionwidget.py index 90be71b02..0f316a928 100644 --- a/tests/unit/completion/test_completionwidget.py +++ b/tests/unit/completion/test_completionwidget.py @@ -96,64 +96,56 @@ def test_maybe_resize_completion(completionview, config_stub, qtbot): completionview.maybe_resize_completion() -@pytest.mark.parametrize('which, tree, count, expected', [ - ('next', [['Aa']], 1, 'Aa'), - ('prev', [['Aa']], 1, 'Aa'), - ('next', [['Aa'], ['Ba']], 1, 'Aa'), - ('prev', [['Aa'], ['Ba']], 1, 'Ba'), - ('next', [['Aa'], ['Ba']], 2, 'Ba'), - ('prev', [['Aa'], ['Ba']], 2, 'Aa'), - ('next', [['Aa', 'Ab', 'Ac'], ['Ba', 'Bb'], ['Ca']], 3, 'Ac'), - ('next', [['Aa', 'Ab', 'Ac'], ['Ba', 'Bb'], ['Ca']], 4, 'Ba'), - ('next', [['Aa', 'Ab', 'Ac'], ['Ba', 'Bb'], ['Ca']], 6, 'Ca'), - ('next', [['Aa', 'Ab', 'Ac'], ['Ba', 'Bb'], ['Ca']], 7, 'Aa'), - ('prev', [['Aa', 'Ab', 'Ac'], ['Ba', 'Bb'], ['Ca']], 1, 'Ca'), - ('prev', [['Aa', 'Ab', 'Ac'], ['Ba', 'Bb'], ['Ca']], 2, 'Bb'), - ('prev', [['Aa', 'Ab', 'Ac'], ['Ba', 'Bb'], ['Ca']], 4, 'Ac'), - ('next', [[], ['Ba', 'Bb']], 1, 'Ba'), - ('prev', [[], ['Ba', 'Bb']], 1, 'Bb'), - ('next', [[], [], ['Ca', 'Cb']], 1, 'Ca'), - ('prev', [[], [], ['Ca', 'Cb']], 1, 'Cb'), - ('next', [['Aa'], []], 1, 'Aa'), - ('prev', [['Aa'], []], 1, 'Aa'), - ('next', [['Aa'], [], []], 1, 'Aa'), - ('prev', [['Aa'], [], []], 1, 'Aa'), - ('next', [['Aa'], [], ['Ca', 'Cb']], 2, 'Ca'), - ('prev', [['Aa'], [], ['Ca', 'Cb']], 1, 'Cb'), - ('next', [[]], 1, None), - ('prev', [[]], 1, None), - ('next-category', [['Aa']], 1, 'Aa'), - ('prev-category', [['Aa']], 1, 'Aa'), - ('next-category', [['Aa'], ['Ba']], 1, 'Aa'), - ('prev-category', [['Aa'], ['Ba']], 1, 'Ba'), - ('next-category', [['Aa'], ['Ba']], 2, 'Ba'), - ('prev-category', [['Aa'], ['Ba']], 2, 'Aa'), - ('next-category', [['Aa', 'Ab', 'Ac'], ['Ba', 'Bb'], ['Ca']], 2, 'Ba'), - ('prev-category', [['Aa', 'Ab', 'Ac'], ['Ba', 'Bb'], ['Ca']], 2, 'Ba'), - ('next-category', [['Aa', 'Ab', 'Ac'], ['Ba', 'Bb'], ['Ca']], 3, 'Ca'), - ('prev-category', [['Aa', 'Ab', 'Ac'], ['Ba', 'Bb'], ['Ca']], 3, 'Aa'), - ('next-category', [[], ['Ba', 'Bb']], 1, 'Ba'), - ('prev-category', [[], ['Ba', 'Bb']], 1, 'Ba'), - ('next-category', [[], [], ['Ca', 'Cb']], 1, 'Ca'), - ('prev-category', [[], [], ['Ca', 'Cb']], 1, 'Ca'), - ('next-category', [[], [], ['Ca', 'Cb']], 2, 'Ca'), - ('prev-category', [[], [], ['Ca', 'Cb']], 2, 'Ca'), - ('next-category', [['Aa'], [], []], 1, 'Aa'), - ('prev-category', [['Aa'], [], []], 1, 'Aa'), - ('next-category', [['Aa'], [], ['Ca', 'Cb']], 2, 'Ca'), - ('prev-category', [['Aa'], [], ['Ca', 'Cb']], 1, 'Ca'), - ('next-category', [[]], 1, None), - ('prev-category', [[]], 1, None), +@pytest.mark.parametrize('which, tree, expected', [ + ('next', [['Aa']], ['Aa', None, None]), + ('prev', [['Aa']], ['Aa', None, None]), + ('next', [['Aa'], ['Ba']], ['Aa', 'Ba', 'Aa']), + ('prev', [['Aa'], ['Ba']], ['Ba', 'Aa', 'Ba']), + ('next', [['Aa', 'Ab', 'Ac'], ['Ba', 'Bb'], ['Ca']], + ['Aa', 'Ab', 'Ac', 'Ba', 'Bb', 'Ca', 'Aa']), + ('prev', [['Aa', 'Ab', 'Ac'], ['Ba', 'Bb'], ['Ca']], + ['Ca', 'Bb', 'Ba', 'Ac', 'Ab', 'Aa', 'Ca']), + ('next', [[], ['Ba', 'Bb']], ['Ba', 'Bb', 'Ba']), + ('prev', [[], ['Ba', 'Bb']], ['Bb', 'Ba', 'Bb']), + ('next', [[], [], ['Ca', 'Cb']], ['Ca', 'Cb', 'Ca']), + ('prev', [[], [], ['Ca', 'Cb']], ['Cb', 'Ca', 'Cb']), + ('next', [['Aa'], []], ['Aa', None]), + ('prev', [['Aa'], []], ['Aa', None]), + ('next', [['Aa'], [], []], ['Aa', None]), + ('prev', [['Aa'], [], []], ['Aa', None]), + ('next', [['Aa'], [], ['Ca', 'Cb']], ['Aa', 'Ca', 'Cb', 'Aa']), + ('prev', [['Aa'], [], ['Ca', 'Cb']], ['Cb', 'Ca', 'Aa', 'Cb']), + ('next', [[]], [None, None]), + ('prev', [[]], [None, None]), + ('next-category', [['Aa']], ['Aa', None, None]), + ('prev-category', [['Aa']], ['Aa', None, None]), + ('next-category', [['Aa'], ['Ba']], ['Aa', 'Ba', 'Aa']), + ('prev-category', [['Aa'], ['Ba']], ['Ba', 'Aa', 'Ba']), + ('next-category', [['Aa', 'Ab', 'Ac'], ['Ba', 'Bb'], ['Ca']], + ['Aa', 'Ba', 'Ca', 'Aa']), + ('prev-category', [['Aa', 'Ab', 'Ac'], ['Ba', 'Bb'], ['Ca']], + ['Ca', 'Ba', 'Aa', 'Ca']), + ('next-category', [[], ['Ba', 'Bb']], ['Ba', None, None]), + ('prev-category', [[], ['Ba', 'Bb']], ['Ba', None, None]), + ('next-category', [[], [], ['Ca', 'Cb']], ['Ca', None, None]), + ('prev-category', [[], [], ['Ca', 'Cb']], ['Ca', None, None]), + ('next-category', [['Aa'], [], []], ['Aa', None, None]), + ('prev-category', [['Aa'], [], []], ['Aa', None, None]), + ('next-category', [['Aa'], [], ['Ca', 'Cb']], ['Aa', 'Ca', 'Aa']), + ('prev-category', [['Aa'], [], ['Ca', 'Cb']], ['Ca', 'Aa', 'Ca']), + ('next-category', [[]], [None, None]), + ('prev-category', [[]], [None, None]), ]) -def test_completion_item_focus(which, tree, count, expected, completionview, - qtbot): +def test_completion_item_focus(which, tree, expected, completionview, qtbot): """Test that on_next_prev_item moves the selection properly. Args: + which: the direction in which to move the selection. 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. + expected: expected argument from on_selection_changed for each + successive movement. None implies no signal should be + emitted. """ model = base.BaseCompletionModel() for catdata in tree: @@ -164,15 +156,14 @@ def test_completion_item_focus(which, tree, count, expected, completionview, filtermodel = sortfilter.CompletionFilterModel(model, parent=completionview) completionview.set_model(filtermodel) - if expected is None: - for _ in range(count): - completionview.completion_item_focus(which) - else: - with qtbot.waitSignal(completionview.selection_changed): - for _ in range(count): + for entry in expected: + if entry is None: + with qtbot.assertNotEmitted(completionview.selection_changed): completionview.completion_item_focus(which) - idx = completionview.selectionModel().currentIndex() - assert filtermodel.data(idx) == expected + else: + with qtbot.waitSignal(completionview.selection_changed) as sig: + completionview.completion_item_focus(which) + assert sig.args == [entry] @pytest.mark.parametrize('which', ['next', 'prev', 'next-category', From bb1b1c8ee81ac145d1f17620e4163f2ed5231f37 Mon Sep 17 00:00:00 2001 From: Florian Bruhin Date: Thu, 15 Sep 2016 16:45:48 +0200 Subject: [PATCH 15/15] Remove unused imports --- qutebrowser/completion/completer.py | 2 +- qutebrowser/completion/completionwidget.py | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/qutebrowser/completion/completer.py b/qutebrowser/completion/completer.py index 2316db407..ac2a84e6b 100644 --- a/qutebrowser/completion/completer.py +++ b/qutebrowser/completion/completer.py @@ -19,7 +19,7 @@ """Completer attached to a CompletionView.""" -from PyQt5.QtCore import pyqtSlot, QObject, QTimer, QItemSelection +from PyQt5.QtCore import pyqtSlot, QObject, QTimer from qutebrowser.config import config from qutebrowser.commands import cmdutils, runners diff --git a/qutebrowser/completion/completionwidget.py b/qutebrowser/completion/completionwidget.py index 00e91c03c..8eadcc599 100644 --- a/qutebrowser/completion/completionwidget.py +++ b/qutebrowser/completion/completionwidget.py @@ -24,8 +24,7 @@ subclasses to provide completions. """ from PyQt5.QtWidgets import QStyle, QTreeView, QSizePolicy -from PyQt5.QtCore import (pyqtSlot, pyqtSignal, Qt, QItemSelectionModel, - QItemSelection) +from PyQt5.QtCore import pyqtSlot, pyqtSignal, Qt, QItemSelectionModel from qutebrowser.config import config, style from qutebrowser.completion import completiondelegate