From c792ffda67c34fd3c5585dffcb8b68b547c1962e Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Tue, 7 Jun 2016 20:20:56 -0400 Subject: [PATCH 1/2] Remove Command.completion. Command completion types are now identified by ArgInfo, so just use that directly and cut out the middle-man. This shouldn't change any completion behavior. Adds a test for get_pos_arg_info to test_cmdutils. Modifies test_completer to test the use of get_pos_arg_info. Instead of using FakeCommand, real Command objects are used, to validate that the Completer works with the real Command interface. This also cleans out some test cases that were testing things already covered by other cases. --- qutebrowser/commands/command.py | 14 +++--- qutebrowser/completion/completer.py | 17 +++---- tests/unit/commands/test_cmdutils.py | 15 +++++++ tests/unit/completion/test_completer.py | 60 +++++++++++++++++-------- 4 files changed, 67 insertions(+), 39 deletions(-) diff --git a/qutebrowser/commands/command.py b/qutebrowser/commands/command.py index aa75e2748..2858a9f02 100644 --- a/qutebrowser/commands/command.py +++ b/qutebrowser/commands/command.py @@ -75,7 +75,6 @@ class Command: deprecated: False, or a string to describe why a command is deprecated. desc: The description of the command. handler: The handler function to call. - completion: Completions to use for arguments, as a list of strings. debug: Whether this is a debugging command (only shown with --debug). parser: The ArgumentParser to use to parse this command. flags_with_args: A list of flags which take an argument. @@ -148,13 +147,7 @@ class Command: self._qute_args = getattr(self.handler, 'qute_args', {}) self.handler.qute_args = None - args = self._inspect_func() - - self.completion = [] - for arg in args: - arg_completion = self.get_arg_info(arg).completion - if arg_completion is not None: - self.completion.append(arg_completion) + self._inspect_func() def _check_prerequisites(self, win_id): """Check if the command is permitted to run currently. @@ -208,6 +201,11 @@ class Command: """Get an ArgInfo tuple for the given inspect.Parameter.""" return self._qute_args.get(param.name, ArgInfo()) + def get_pos_arg_info(self, pos): + """Get an ArgInfo tuple for the given positional parameter.""" + name = self.pos_args[pos][0] + return self._qute_args.get(name, ArgInfo()) + def _inspect_special_param(self, param): """Check if the given parameter is a special one. diff --git a/qutebrowser/completion/completer.py b/qutebrowser/completion/completer.py index 71202d131..569e105be 100644 --- a/qutebrowser/completion/completer.py +++ b/qutebrowser/completion/completer.py @@ -204,25 +204,18 @@ class Completer(QObject): return sortfilter.CompletionFilterModel(source=model, parent=self) # delegate completion to command try: - completions = cmdutils.cmd_dict[parts[0]].completion + cmd = cmdutils.cmd_dict[parts[0]] except KeyError: # entering an unknown command return None - if completions is None: - # command without any available completions - return None - dbg_completions = [c.name for c in completions] try: idx = cursor_part - 1 - completion = completions[idx] + completion = cmd.get_pos_arg_info(idx).completion except IndexError: - # More arguments than completions - log.completion.debug("completions: {}".format( - ', '.join(dbg_completions))) + # user provided more positional arguments than the command takes + return None + if completion is None: return None - dbg_completions[idx] = '*' + dbg_completions[idx] + '*' - log.completion.debug("completions: {}".format( - ', '.join(dbg_completions))) model = self._get_completion_model(completion, parts, cursor_part) return model diff --git a/tests/unit/commands/test_cmdutils.py b/tests/unit/commands/test_cmdutils.py index b01411c3d..1ecdbaf3a 100644 --- a/tests/unit/commands/test_cmdutils.py +++ b/tests/unit/commands/test_cmdutils.py @@ -291,6 +291,21 @@ class TestRegister: else: assert cmd._get_call_args(win_id=0) == ([expected], {}) + def test_pos_arg_info(self): + @cmdutils.register() + @cmdutils.argument('foo', choices=('a', 'b')) + @cmdutils.argument('bar', choices=('x', 'y')) + @cmdutils.argument('opt') + def fun(foo, bar, opt=False): + """Blah.""" + pass + + cmd = cmdutils.cmd_dict['fun'] + assert cmd.get_pos_arg_info(0) == command.ArgInfo(choices=('a', 'b')) + assert cmd.get_pos_arg_info(1) == command.ArgInfo(choices=('x', 'y')) + with pytest.raises(IndexError): + cmd.get_pos_arg_info(2) + class TestArgument: diff --git a/tests/unit/completion/test_completer.py b/tests/unit/completion/test_completer.py index 29196ea3b..8e9273a90 100644 --- a/tests/unit/completion/test_completer.py +++ b/tests/unit/completion/test_completer.py @@ -27,6 +27,7 @@ from PyQt5.QtGui import QStandardItemModel from qutebrowser.completion import completer from qutebrowser.utils import usertypes +from qutebrowser.commands import command, cmdutils class FakeCompletionModel(QStandardItemModel): @@ -91,24 +92,49 @@ def instances(monkeypatch): @pytest.fixture(autouse=True) def cmdutils_patch(monkeypatch, stubs): """Patch the cmdutils module to provide fake commands.""" + @cmdutils.argument('section_', completion=usertypes.Completion.section) + @cmdutils.argument('option', completion=usertypes.Completion.option) + @cmdutils.argument('value', completion=usertypes.Completion.value) + def set_command(section_=None, option=None, value=None): + """docstring!""" + pass + + @cmdutils.argument('topic', completion=usertypes.Completion.helptopic) + def show_help(tab=False, bg=False, window=False, topic=None): + """docstring!""" + pass + + @cmdutils.argument('url', completion=usertypes.Completion.url) + @cmdutils.argument('count', count=True) + def openurl(url=None, implicit=False, bg=False, tab=False, window=False, + count=None): + """docstring!""" + pass + + @cmdutils.argument('win_id', win_id=True) + @cmdutils.argument('key', completion=usertypes.Completion.empty) + @cmdutils.argument('command', completion=usertypes.Completion.command) + def bind(key, win_id, command=None, *, mode='normal', force=False): + """docstring!""" + # pylint: disable=unused-variable + pass + + def tab_detach(): + """docstring!""" + pass + cmds = { - 'set': [usertypes.Completion.section, usertypes.Completion.option, - usertypes.Completion.value], - 'help': [usertypes.Completion.helptopic], - 'quickmark-load': [usertypes.Completion.quickmark_by_name], - 'bookmark-load': [usertypes.Completion.bookmark_by_url], - 'open': [usertypes.Completion.url], - 'buffer': [usertypes.Completion.tab], - 'session-load': [usertypes.Completion.sessions], - 'bind': [usertypes.Completion.empty, usertypes.Completion.command], - 'tab-detach': None, + 'set': set_command, + 'help': show_help, + 'open': openurl, + 'bind': bind, + 'tab-detach': tab_detach, } cmd_utils = stubs.FakeCmdUtils({ - name: stubs.FakeCommand(completion=compl) - for name, compl in cmds.items() + name: command.Command(name=name, handler=fn) + for name, fn in cmds.items() }) - monkeypatch.setattr('qutebrowser.completion.completer.cmdutils', - cmd_utils) + monkeypatch.setattr('qutebrowser.completion.completer.cmdutils', cmd_utils) def _set_cmd_prompt(cmd, txt): @@ -143,11 +169,8 @@ def _validate_cmd_prompt(cmd, txt): (':set general ignore-case |', usertypes.Completion.value), (':set general huh |', None), (':help |', usertypes.Completion.helptopic), - (':quickmark-load |', usertypes.Completion.quickmark_by_name), - (':bookmark-load |', usertypes.Completion.bookmark_by_url), + (':help |', usertypes.Completion.helptopic), (':open |', usertypes.Completion.url), - (':buffer |', usertypes.Completion.tab), - (':session-load |', usertypes.Completion.sessions), (':bind |', usertypes.Completion.empty), (':bind |', usertypes.Completion.command), (':bind foo|', usertypes.Completion.command), @@ -157,7 +180,6 @@ def _validate_cmd_prompt(cmd, txt): (':set gene|ral ignore-case', usertypes.Completion.section), (':|', usertypes.Completion.command), (': |', usertypes.Completion.command), - (':bookmark-load |', usertypes.Completion.bookmark_by_url), ('/|', None), (':open -t|', None), (':open --tab|', None), From 66358e5b0f496f6c9d0af81cc3650481936a792f Mon Sep 17 00:00:00 2001 From: Ryan Roden-Corrent Date: Sat, 6 Aug 2016 11:38:08 -0400 Subject: [PATCH 2/2] Remove usertypes.Completion.empty. Completion.empty existed to fill a slot in the old Command.completions interface if the first positional arg had no completions but the second did, as is the case for the `bind` command. Now that `Command.completions` is replaced by `Command.get_pos_arg_info`, this is no longer needed. --- qutebrowser/completion/models/instances.py | 11 +---------- qutebrowser/config/parsers/keyconf.py | 1 - qutebrowser/utils/usertypes.py | 3 +-- tests/unit/completion/test_completer.py | 5 ++--- 4 files changed, 4 insertions(+), 16 deletions(-) diff --git a/qutebrowser/completion/models/instances.py b/qutebrowser/completion/models/instances.py index c92225539..568607b79 100644 --- a/qutebrowser/completion/models/instances.py +++ b/qutebrowser/completion/models/instances.py @@ -27,8 +27,7 @@ Module attributes: import functools -from qutebrowser.completion.models import (miscmodels, urlmodel, configmodel, - base) +from qutebrowser.completion.models import miscmodels, urlmodel, configmodel from qutebrowser.utils import objreg, usertypes, log, debug from qutebrowser.config import configdata @@ -115,13 +114,6 @@ def init_session_completion(): _instances[usertypes.Completion.sessions] = model -def _init_empty_completion(): - """Initialize empty completion model.""" - log.completion.debug("Initializing empty completion.") - if usertypes.Completion.empty not in _instances: - _instances[usertypes.Completion.empty] = base.BaseCompletionModel() - - INITIALIZERS = { usertypes.Completion.command: _init_command_completion, usertypes.Completion.helptopic: _init_helptopic_completion, @@ -133,7 +125,6 @@ INITIALIZERS = { usertypes.Completion.quickmark_by_name: init_quickmark_completions, usertypes.Completion.bookmark_by_url: init_bookmark_completions, usertypes.Completion.sessions: init_session_completion, - usertypes.Completion.empty: _init_empty_completion, } diff --git a/qutebrowser/config/parsers/keyconf.py b/qutebrowser/config/parsers/keyconf.py index b2596fb62..e8e7cc6dc 100644 --- a/qutebrowser/config/parsers/keyconf.py +++ b/qutebrowser/config/parsers/keyconf.py @@ -152,7 +152,6 @@ class KeyConfigParser(QObject): @cmdutils.register(instance='key-config', maxsplit=1, no_cmd_split=True) @cmdutils.argument('win_id', win_id=True) - @cmdutils.argument('key', completion=usertypes.Completion.empty) @cmdutils.argument('command', completion=usertypes.Completion.command) def bind(self, key, win_id, command=None, *, mode='normal', force=False): """Bind a key to a command. diff --git a/qutebrowser/utils/usertypes.py b/qutebrowser/utils/usertypes.py index 65c3c31e2..1cd1f0385 100644 --- a/qutebrowser/utils/usertypes.py +++ b/qutebrowser/utils/usertypes.py @@ -238,8 +238,7 @@ KeyMode = enum('KeyMode', ['normal', 'hint', 'command', 'yesno', 'prompt', # Available command completions Completion = enum('Completion', ['command', 'section', 'option', 'value', 'helptopic', 'quickmark_by_name', - 'bookmark_by_url', 'url', 'tab', 'sessions', - 'empty']) + 'bookmark_by_url', 'url', 'tab', 'sessions']) # Exit statuses for errors. Needs to be an int for sys.exit. diff --git a/tests/unit/completion/test_completer.py b/tests/unit/completion/test_completer.py index 8e9273a90..f3567674e 100644 --- a/tests/unit/completion/test_completer.py +++ b/tests/unit/completion/test_completer.py @@ -112,7 +112,6 @@ def cmdutils_patch(monkeypatch, stubs): pass @cmdutils.argument('win_id', win_id=True) - @cmdutils.argument('key', completion=usertypes.Completion.empty) @cmdutils.argument('command', completion=usertypes.Completion.command) def bind(key, win_id, command=None, *, mode='normal', force=False): """docstring!""" @@ -171,10 +170,10 @@ def _validate_cmd_prompt(cmd, txt): (':help |', usertypes.Completion.helptopic), (':help |', usertypes.Completion.helptopic), (':open |', usertypes.Completion.url), - (':bind |', usertypes.Completion.empty), + (':bind |', None), (':bind |', usertypes.Completion.command), (':bind foo|', usertypes.Completion.command), - (':bind | foo', usertypes.Completion.empty), + (':bind | foo', None), (':set| general ', usertypes.Completion.command), (':|set general ', usertypes.Completion.command), (':set gene|ral ignore-case', usertypes.Completion.section),