From f237a87ad0e568d305426ab6b3baf6b83702b4fc Mon Sep 17 00:00:00 2001
From: Ryan Roden-Corrent <ryan@rcorre.net>
Date: Sun, 25 Mar 2018 21:59:30 -0400
Subject: [PATCH 1/3] Completion for varargs.

When a command has positional varargs, keep offering the configured
completion for each successive argument.

Right now this only influences `config-cycle`.

Previously, `config-cycle <option> ` would offer a value completion for
only the first argument after the option. Now it will keep offering
value completion for each successive argument.

This will be useful for passing multiple tags to the new bookmark
commands that will be added for #882.
---
 qutebrowser/commands/command.py         |  5 +++++
 tests/unit/completion/test_completer.py | 12 ++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/qutebrowser/commands/command.py b/qutebrowser/commands/command.py
index 8dd830495..5c5ab1311 100644
--- a/qutebrowser/commands/command.py
+++ b/qutebrowser/commands/command.py
@@ -123,6 +123,7 @@ class Command:
         self.pos_args = []
         self.desc = None
         self.flags_with_args = []
+        self._has_vararg = False
 
         # This is checked by future @cmdutils.argument calls so they fail
         # (as they'd be silently ignored otherwise)
@@ -170,6 +171,8 @@ class Command:
 
     def get_pos_arg_info(self, pos):
         """Get an ArgInfo tuple for the given positional parameter."""
+        if pos >= len(self.pos_args) and self._has_vararg:
+            pos = len(self.pos_args) - 1
         name = self.pos_args[pos][0]
         return self._qute_args.get(name, ArgInfo())
 
@@ -233,6 +236,8 @@ class Command:
             log.commands.vdebug('Adding arg {} of type {} -> {}'.format(
                 param.name, typ, callsig))
             self.parser.add_argument(*args, **kwargs)
+            if param.kind == inspect.Parameter.VAR_POSITIONAL:
+                self._has_vararg = True
         return signature.parameters.values()
 
     def _param_to_argparse_kwargs(self, param, is_bool):
diff --git a/tests/unit/completion/test_completer.py b/tests/unit/completion/test_completer.py
index 51aa091b9..74313911f 100644
--- a/tests/unit/completion/test_completer.py
+++ b/tests/unit/completion/test_completer.py
@@ -129,12 +129,20 @@ def cmdutils_patch(monkeypatch, stubs, miscmodels_patch):
         """docstring."""
         pass
 
+    @cmdutils.argument('option', completion=miscmodels_patch.option)
+    @cmdutils.argument('values', completion=miscmodels_patch.value)
+    def config_cycle(option, *values):
+        """For testing varargs."""
+        pass
+
     cmd_utils = stubs.FakeCmdUtils({
         '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),
+        'config-cycle': command.Command(name='config-cycle',
+                                        handler=config_cycle),
     })
     monkeypatch.setattr(completer, 'cmdutils', cmd_utils)
 
@@ -191,6 +199,10 @@ def _set_cmd_prompt(cmd, txt):
     ('/:help|', None, '', []),
     ('::bind|', 'command', ':bind', []),
     (':-w open |', None, '', []),
+    # varargs
+    (':config-cycle option |', 'value', '', ['option']),
+    (':config-cycle option one |', 'value', '', ['option', 'one']),
+    (':config-cycle option one two |', 'value', '', ['option', 'one', 'two']),
 ])
 def test_update_completion(txt, kind, pattern, pos_args, status_command_stub,
                            completer_obj, completion_widget_stub, config_stub,

From 707fc1176d542e8c3b280c120157ca9f538bc98c Mon Sep 17 00:00:00 2001
From: Ryan Roden-Corrent <ryan@rcorre.net>
Date: Sat, 31 Mar 2018 11:40:51 -0400
Subject: [PATCH 2/3] Regen completion if args change.

Allow completion functions to react dynamically to args as the user
inputs them. This allows config-cycle to filter out values that were
already provided.

Args provided after the maxsplit do not cause the completion to regen.
For example, successive words typed after `:open` just set the filter
pattern and do not spuriously regenerate the completion model.
---
 qutebrowser/completion/completer.py          | 12 ++++-----
 qutebrowser/completion/models/configmodel.py | 21 ++++++++++------
 tests/unit/completion/test_completer.py      | 26 ++++++++++++++++++++
 3 files changed, 45 insertions(+), 14 deletions(-)

diff --git a/qutebrowser/completion/completer.py b/qutebrowser/completion/completer.py
index 8506f3aa7..4cbdc4724 100644
--- a/qutebrowser/completion/completer.py
+++ b/qutebrowser/completion/completer.py
@@ -49,7 +49,7 @@ class Completer(QObject):
         _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.
-        _last_completion_func: The completion function used for the last text.
+        _last_before_cursor: The prior value of before_cursor.
     """
 
     def __init__(self, *, cmd, win_id, parent=None):
@@ -62,7 +62,7 @@ class Completer(QObject):
         self._timer.timeout.connect(self._update_completion)
         self._last_cursor_pos = -1
         self._last_text = None
-        self._last_completion_func = None
+        self._last_before_cursor = None
         self._cmd.update_completion.connect(self.schedule_completion_update)
 
     def __repr__(self):
@@ -228,7 +228,7 @@ class Completer(QObject):
             # FIXME complete searches
             # https://github.com/qutebrowser/qutebrowser/issues/32
             completion.set_model(None)
-            self._last_completion_func = None
+            self._last_before_cursor = None
             return
 
         before_cursor, pattern, after_cursor = self._partition()
@@ -242,11 +242,11 @@ class Completer(QObject):
         if func is None:
             log.completion.debug('Clearing completion')
             completion.set_model(None)
-            self._last_completion_func = None
+            self._last_before_cursor = None
             return
 
-        if func != self._last_completion_func:
-            self._last_completion_func = func
+        if before_cursor != self._last_before_cursor:
+            self._last_before_cursor = before_cursor
             args = (x for x in before_cursor[1:] if not x.startswith('-'))
             with debug.log_time(log.completion, 'Starting {} completion'
                                 .format(func.__name__)):
diff --git a/qutebrowser/completion/models/configmodel.py b/qutebrowser/completion/models/configmodel.py
index b462442a0..6ee2f9566 100644
--- a/qutebrowser/completion/models/configmodel.py
+++ b/qutebrowser/completion/models/configmodel.py
@@ -47,12 +47,12 @@ def customized_option(*, info):
     return model
 
 
-def value(optname, *_values, info):
+def value(optname, *values, info):
     """A CompletionModel filled with setting values.
 
     Args:
         optname: The name of the config option this model shows.
-        _values: The values already provided on the command line.
+        values: The values already provided on the command line.
         info: A CompletionInfo instance.
     """
     model = completionmodel.CompletionModel(column_widths=(30, 70, 0))
@@ -64,13 +64,18 @@ def value(optname, *_values, info):
 
     opt = info.config.get_opt(optname)
     default = opt.typ.to_str(opt.default)
-    cur_cat = listcategory.ListCategory(
-        "Current/Default",
-        [(current, "Current value"), (default, "Default value")])
-    model.add_category(cur_cat)
+    cur_def = []
+    if current not in values:
+        cur_def.append((current, "Current value"))
+    if default not in values:
+        cur_def.append((default, "Default value"))
+    if cur_def:
+        cur_cat = listcategory.ListCategory("Current/Default", cur_def)
+        model.add_category(cur_cat)
 
-    vals = opt.typ.complete()
-    if vals is not None:
+    vals = opt.typ.complete() or []
+    vals = [x for x in vals if x[0] not in values]
+    if vals:
         model.add_category(listcategory.ListCategory("Completions", vals))
     return model
 
diff --git a/tests/unit/completion/test_completer.py b/tests/unit/completion/test_completer.py
index 74313911f..bdc0c1cf5 100644
--- a/tests/unit/completion/test_completer.py
+++ b/tests/unit/completion/test_completer.py
@@ -223,6 +223,32 @@ def test_update_completion(txt, kind, pattern, pos_args, status_command_stub,
         completion_widget_stub.set_pattern.assert_called_once_with(pattern)
 
 
+@pytest.mark.parametrize('txt1, txt2, regen', [
+    (':config-cycle |', ':config-cycle a|', False),
+    (':config-cycle abc|', ':config-cycle abc |', True),
+    (':config-cycle abc |', ':config-cycle abc d|', False),
+    (':config-cycle abc def|', ':config-cycle abc def |', True),
+    # open has maxsplit=0, so all args just set the pattern, not the model
+    (':open |', ':open a|', False),
+    (':open abc|', ':open abc |', False),
+    (':open abc |', ':open abc d|', False),
+    (':open abc def|', ':open abc def |', False),
+])
+def test_regen_completion(txt1, txt2, regen, status_command_stub,
+                          completer_obj, completion_widget_stub, config_stub,
+                          key_config_stub):
+    """Test that the completion function is only called as needed."""
+    # set the initial state
+    _set_cmd_prompt(status_command_stub, txt1)
+    completer_obj.schedule_completion_update()
+    completion_widget_stub.set_model.reset_mock()
+
+    # "move" the cursor and check if the completion function was called
+    _set_cmd_prompt(status_command_stub, txt2)
+    completer_obj.schedule_completion_update()
+    assert completion_widget_stub.set_model.called == regen
+
+
 @pytest.mark.parametrize('before, newtxt, after', [
     (':|', 'set', ':set|'),
     (':| ', 'set', ':set|'),

From bca07eebba029870057bae66d03f2c2c001b3d25 Mon Sep 17 00:00:00 2001
From: Ryan Roden-Corrent <ryan@rcorre.net>
Date: Sat, 31 Mar 2018 15:57:08 -0400
Subject: [PATCH 3/3] Unit-test configmodel arg filtering.

Test that values the user has already input are omitted from the
suggestions. This brings configmodel coverage back to 100%.
---
 tests/unit/completion/test_models.py | 38 ++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/tests/unit/completion/test_models.py b/tests/unit/completion/test_models.py
index efc30dd1c..5db390194 100644
--- a/tests/unit/completion/test_models.py
+++ b/tests/unit/completion/test_models.py
@@ -716,6 +716,44 @@ def test_setting_value_completion_invalid(info):
     assert configmodel.value(optname='foobarbaz', info=info) is None
 
 
+@pytest.mark.parametrize('args, expected', [
+    ([], {
+        "Current/Default": [
+            ('true', 'Current value', None),
+            ('true', 'Default value', None),
+        ],
+        "Completions": [
+            ('false', '', None),
+            ('true', '', None),
+        ],
+    }),
+    (['false'], {
+        "Current/Default": [
+            ('true', 'Current value', None),
+            ('true', 'Default value', None),
+        ],
+        "Completions": [
+            ('true', '', None),
+        ],
+    }),
+    (['true'], {
+        "Completions": [
+            ('false', '', None),
+        ],
+    }),
+    (['false', 'true'], {}),
+])
+def test_setting_value_cycle(qtmodeltester, config_stub, configdata_stub,
+                             info, args, expected):
+    opt = 'content.javascript.enabled'
+
+    model = configmodel.value(opt, *args, info=info)
+    model.set_pattern('')
+    qtmodeltester.data_display_may_return_none = True
+    qtmodeltester.check(model)
+    _check_completions(model, expected)
+
+
 def test_bind_completion(qtmodeltester, cmdutils_stub, config_stub,
                          key_config_stub, configdata_stub, info):
     """Test the results of keybinding command completion.