From c792ffda67c34fd3c5585dffcb8b68b547c1962e Mon Sep 17 00:00:00 2001
From: Ryan Roden-Corrent <ryan@rcorre.net>
Date: Tue, 7 Jun 2016 20:20:56 -0400
Subject: [PATCH] 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 <c-x> |', usertypes.Completion.command),
     (':bind <c-x> 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),