Merge pull request #930 from stopthatcow/feature/925

Overhaul bash completion to mirror invoke logic

4 files changed, 138 insertions, 47 deletions

diff --git a/CHANGES.rst b/CHANGES.rst
index 6e0bb96..32d3baf 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -40,11 +40,11 @@ Unreleased
 -   Fix bug in test runner when calling ``sys.exit`` with ``None``. (`#739`_)
 -   Clarify documentation on command line options. (`#741`_, `#1003`_)
 -   Fix crash on Windows console. (`#744`_)
--   Fix bug that caused bash completion to give improper completions on chained commands when a required option/argument was being completed. (`#754`_, `#774`_, `#790`_, `#806`_)
+-   Fix bug that caused bash completion to give improper completions on chained commands. (`#754`_, `#774`_)
 -   Added support for dynamic bash completion from a user-supplied callback. (`#755`_)
 -   Added support for bash completions containing spaces. (`#773`_)
--   Fix option naming routine to match documentation. (`#793`_)
--   Use deterministic option name; can't rely on list sort. (`#793`_, `#794`_)
+-   Allow autocompletion function to determine whether or not to return completions that start with the incomplete argument. (`#790`_, `#806`_)
+-   Fix option naming routine to match documentation and be deterministic. (`#793`_, `#794`_)
 -   Fix path validation bug. (`#795`_, `#1020`_)
 -   Add test and documentation for ``Option`` naming: functionality. (`#799`_)
 -   Update doc to match arg name for ``path_type``. (`#801`_)
@@ -60,7 +60,9 @@ Unreleased
 -   When detecting a misconfigured locale, don't fail if the ``locale`` command fails. (`#880`_)
 -   Add ``case_sensitive=False`` as an option to ``Choice`` types. (`#887`_)
 -   Force stdout/stderr writable. This works around issues with badly patched standard streams like those from Jupyter. (`#918`_)
+-   Fix completion of subcommand options after last argument (`#919`_, `#930`_)
 -   ``_AtomicFile`` now uses the ``realpath`` of the original filename so that chaning the working directory does not affect it. (`#920`_)
+-   Fix incorrect completions when defaults are present (`#925`_, `#930`_)
 -   Add copy option attrs so that custom classes can be re-used. (`#926`_, `#994`_)
 -   "x" and "a" file modes now use stdout when file is ``"-"``. (`#929`_)
 -   Fix missing comma in ``__all__`` list. (`#935`_)
@@ -150,9 +152,12 @@ Unreleased
 .. _#887: https://github.com/pallets/click/pull/887
 .. _#889: https://github.com/pallets/click/pull/889
 .. _#918: https://github.com/pallets/click/pull/918
+.. _#919: https://github.com/pallets/click/issues/919
 .. _#920: https://github.com/pallets/click/pull/920
+.. _#925: https://github.com/pallets/click/issues/925
 .. _#926: https://github.com/pallets/click/issues/926
 .. _#929: https://github.com/pallets/click/pull/929
+.. _#930: https://github.com/pallets/click/pull/930
 .. _#935: https://github.com/pallets/click/pull/935
 .. _#949: https://github.com/pallets/click/issues/949
 .. _#954: https://github.com/pallets/click/pull/954
diff --git a/click/_bashcomplete.py b/click/_bashcomplete.py
index db42865..10202d3 100644
--- a/click/_bashcomplete.py
+++ b/click/_bashcomplete.py
@@ -73,18 +73,31 @@ def resolve_ctx(cli, prog_name, args):
     :return: the final context/command parsed
     """
     ctx = cli.make_context(prog_name, args, resilient_parsing=True)
-    args_remaining = ctx.protected_args + ctx.args
-    while ctx is not None and args_remaining:
+    args = ctx.protected_args + ctx.args
+    while args:
         if isinstance(ctx.command, MultiCommand):
-            cmd = ctx.command.get_command(ctx, args_remaining[0])
-            if cmd is None:
-                return None
-            ctx = cmd.make_context(
-                args_remaining[0], args_remaining[1:], parent=ctx, resilient_parsing=True)
-            args_remaining = ctx.protected_args + ctx.args
+            if not ctx.command.chain:
+                cmd_name, cmd, args = ctx.command.resolve_command(ctx, args)
+                if cmd is None:
+                    return ctx
+                ctx = cmd.make_context(cmd_name, args, parent=ctx,
+                                       resilient_parsing=True)
+                args = ctx.protected_args + ctx.args
+            else:
+                # Walk chained subcommand contexts saving the last one.
+                while args:
+                    cmd_name, cmd, args = ctx.command.resolve_command(ctx, args)
+                    if cmd is None:
+                        return ctx
+                    sub_ctx = cmd.make_context(cmd_name, args, parent=ctx,
+                                               allow_extra_args=True,
+                                               allow_interspersed_args=False,
+                                               resilient_parsing=True)
+                    args = sub_ctx.args
+                ctx = sub_ctx
+                args = sub_ctx.protected_args + sub_ctx.args
         else:
-            ctx = ctx.parent
-
+            break
     return ctx
 
 
@@ -216,12 +229,7 @@ def get_choices(cli, prog_name, args, incomplete):
     # completion for argument values from user supplied values
     for param in ctx.command.params:
         if is_incomplete_argument(ctx.params, param):
-            completions.extend(get_user_autocompletions(
-                ctx, all_args, incomplete, param))
-            # Stop looking for other completions only if this argument is required.
-            if param.required:
-                return completions
-            break
+            return get_user_autocompletions(ctx, all_args, incomplete, param)
 
     add_subcommand_completions(ctx, incomplete, completions)
     return completions
diff --git a/click/core.py b/click/core.py
index d81950a..c42e8f4 100644
--- a/click/core.py
+++ b/click/core.py
@@ -182,7 +182,8 @@ class Context(object):
                               add some safety mapping on the right.
     :param resilient_parsing: if this flag is enabled then Click will
                               parse without any interactivity or callback
-                              invocation.  This is useful for implementing
+                              invocation.  Default values will also be
+                              ignored.  This is useful for implementing
                               things such as completion support.
     :param allow_extra_args: if this is set to `True` then extra arguments
                              at the end will not raise an error and will be
@@ -312,7 +313,8 @@ class Context(object):
         self.token_normalize_func = token_normalize_func
 
         #: Indicates if resilient parsing is enabled.  In that case Click
-        #: will do its best to not cause any failures.
+        #: will do its best to not cause any failures and default values
+        #: will be ignored. Useful for completion.
         self.resilient_parsing = resilient_parsing
 
         # If there is no envvar prefix yet, but the parent has one and
@@ -1177,7 +1179,7 @@ class MultiCommand(Command):
         # an option we want to kick off parsing again for arguments to
         # resolve things like --help which now should go to the main
         # place.
-        if cmd is None:
+        if cmd is None and not ctx.resilient_parsing:
             if split_opt(cmd_name)[0]:
                 self.parse_args(ctx, ctx.args)
             ctx.fail('No such command "%s".' % original_cmd_name)
@@ -1432,7 +1434,7 @@ class Parameter(object):
     def full_process_value(self, ctx, value):
         value = self.process_value(ctx, value)
 
-        if value is None:
+        if value is None and not ctx.resilient_parsing:
             value = self.get_default(ctx)
 
         if self.required and self.value_is_missing(value):
diff --git a/tests/test_bashcomplete.py b/tests/test_bashcomplete.py
index 0390841..6818893 100644
--- a/tests/test_bashcomplete.py
+++ b/tests/test_bashcomplete.py
@@ -132,7 +132,8 @@ def test_long_chain():
 def test_chaining():
     @click.group('cli', chain=True)
     @click.option('--cli-opt')
-    def cli(cli_opt):
+    @click.argument('arg', type=click.Choice(['cliarg1', 'cliarg2']))
+    def cli(cli_opt, arg):
         pass
 
     @cli.command()
@@ -142,33 +143,35 @@ def test_chaining():
 
     @cli.command(help='bsub help')
     @click.option('--bsub-opt')
-    @click.argument('arg', type=click.Choice(['arg1', 'arg2']), required=True)
+    @click.argument('arg', type=click.Choice(['arg1', 'arg2']))
     def bsub(bsub_opt, arg):
         pass
 
     @cli.command()
     @click.option('--csub-opt')
-    @click.argument('arg', type=click.Choice(['carg1', 'carg2']), required=False)
+    @click.argument('arg', type=click.Choice(['carg1', 'carg2']), default='carg1')
     def csub(csub_opt, arg):
         pass
 
     assert choices_without_help(cli, [], '-') == ['--cli-opt']
-    assert choices_without_help(cli, [], '') == ['asub', 'bsub', 'csub']
-    assert choices_without_help(cli, ['asub'], '-') == ['--asub-opt']
-    assert choices_without_help(cli, ['asub'], '') == ['bsub', 'csub']
-    assert choices_without_help(cli, ['bsub'], '') == ['arg1', 'arg2']
-    assert choices_without_help(cli, ['asub', '--asub-opt'], '') == []
-    assert choices_without_help(cli, ['asub', '--asub-opt', '5', 'bsub'], '-') == ['--bsub-opt']
-    assert choices_without_help(cli, ['asub', 'bsub'], '-') == ['--bsub-opt']
-    assert choices_with_help(cli, ['asub'], 'b') == [('bsub', 'bsub help')]
-    assert choices_without_help(cli, ['asub', 'csub'], '-') == ['--csub-opt']
+    assert choices_without_help(cli, [], '') == ['cliarg1', 'cliarg2']
+    assert choices_without_help(cli, ['cliarg1', 'asub'], '-') == ['--asub-opt']
+    assert choices_without_help(cli, ['cliarg1', 'asub'], '') == ['bsub', 'csub']
+    assert choices_without_help(cli, ['cliarg1', 'bsub'], '') == ['arg1', 'arg2']
+    assert choices_without_help(cli, ['cliarg1', 'asub', '--asub-opt'], '') == []
+    assert choices_without_help(cli, ['cliarg1', 'asub', '--asub-opt', '5', 'bsub'], '-') == ['--bsub-opt']
+    assert choices_without_help(cli, ['cliarg1', 'asub', 'bsub'], '-') == ['--bsub-opt']
+    assert choices_without_help(cli, ['cliarg1', 'asub', 'csub'], '') == ['carg1', 'carg2']
+    assert choices_without_help(cli, ['cliarg1', 'bsub', 'arg1', 'csub'], '') == ['carg1', 'carg2']
+    assert choices_without_help(cli, ['cliarg1', 'asub', 'csub'], '-') == ['--csub-opt']
+    assert choices_with_help(cli, ['cliarg1', 'asub'], 'b') == [('bsub', 'bsub help')]
 
 
 def test_argument_choice():
     @click.command()
-    @click.argument('arg1', required=False, type=click.Choice(['arg11', 'arg12']))
-    @click.argument('arg2', required=False, type=click.Choice(['arg21', 'arg22']))
-    @click.argument('arg3', required=False, type=click.Choice(['arg', 'argument']))
+    @click.argument('arg1', required=True, type=click.Choice(['arg11', 'arg12']))
+    @click.argument('arg2', type=click.Choice(['arg21', 'arg22']), default='arg21')
+    @click.argument('arg3', type=click.Choice(['arg', 'argument']), default='arg')
     def cli():
         pass
 
@@ -182,7 +185,7 @@ def test_argument_choice():
 def test_option_choice():
     @click.command()
     @click.option('--opt1', type=click.Choice(['opt11', 'opt12']), help='opt1 help')
-    @click.option('--opt2', type=click.Choice(['opt21', 'opt22']))
+    @click.option('--opt2', type=click.Choice(['opt21', 'opt22']), default='opt21')
     @click.option('--opt3', type=click.Choice(['opt', 'option']))
     def cli():
         pass
@@ -218,6 +221,8 @@ def test_option_and_arg_choice():
     assert choices_without_help(cli, [''], '--opt1=') == ['opt11', 'opt12']
     assert choices_without_help(cli, [], '') == ['arg11', 'arg12']
     assert choices_without_help(cli, ['--opt2'], '') == ['opt21', 'opt22']
+    assert choices_without_help(cli, ['arg11'], '--opt') == ['--opt1', '--opt2']
+    assert choices_without_help(cli, [], '--opt') == ['--opt1', '--opt2']
 
 
 def test_boolean_flag_choice():
@@ -258,11 +263,37 @@ def test_multi_option_choice():
 
 def test_variadic_argument_choice():
     @click.command()
+    @click.option('--opt', type=click.Choice(['opt1', 'opt2']))
     @click.argument('src', nargs=-1, type=click.Choice(['src1', 'src2']))
     def cli(local_opt):
         pass
 
     assert choices_without_help(cli, ['src1', 'src2'], '') == ['src1', 'src2']
+    assert choices_without_help(cli, ['src1', 'src2'], '--o') == ['--opt']
+    assert choices_without_help(cli, ['src1', 'src2', '--opt'], '') == ['opt1', 'opt2']
+    assert choices_without_help(cli, ['src1', 'src2'], '') == ['src1', 'src2']
+
+
+def test_variadic_argument_complete():
+
+    def _complete(ctx, args, incomplete):
+        return ['abc', 'def', 'ghi', 'jkl', 'mno', 'pqr', 'stu', 'vwx', 'yz']
+
+    @click.group()
+    def entrypoint():
+        pass
+
+    @click.command()
+    @click.option('--opt', autocompletion=_complete)
+    @click.argument('arg', nargs=-1)
+    def subcommand(opt, arg):
+        pass
+
+    entrypoint.add_command(subcommand)
+
+    assert choices_without_help(entrypoint, ['subcommand', '--opt'], '') == _complete(0,0,0)
+    assert choices_without_help(entrypoint, ['subcommand', 'whatever', '--opt'], '') == _complete(0,0,0)
+    assert choices_without_help(entrypoint, ['subcommand', 'whatever', '--opt', 'abc'], '') == []
 
 
 def test_long_chain_choice():
@@ -273,7 +304,7 @@ def test_long_chain_choice():
     @cli.group()
     @click.option('--sub-opt', type=click.Choice(['subopt1', 'subopt2']))
     @click.argument('sub-arg', required=False, type=click.Choice(['subarg1', 'subarg2']))
-    def sub(sub_opt):
+    def sub(sub_opt, sub_arg):
         pass
 
     @sub.command(short_help='bsub help')
@@ -283,15 +314,60 @@ def test_long_chain_choice():
     def bsub(bsub_opt):
         pass
 
-    assert choices_with_help(cli, ['sub'], '') == [('subarg1', None), ('subarg2', None), ('bsub', 'bsub help')]
+    @sub.group('csub')
+    def csub():
+        pass
+
+    @csub.command()
+    def dsub():
+        pass
+
+    assert choices_with_help(cli, ['sub', 'subarg1'], '') == [('bsub', 'bsub help'), ('csub', '')]
+    assert choices_without_help(cli, ['sub'], '') == ['subarg1', 'subarg2']
     assert choices_without_help(cli, ['sub', '--sub-opt'], '') == ['subopt1', 'subopt2']
     assert choices_without_help(cli, ['sub', '--sub-opt', 'subopt1'], '') == \
-        ['subarg1', 'subarg2', 'bsub']
+           ['subarg1', 'subarg2']
+    assert choices_without_help(cli,
+                                ['sub', '--sub-opt', 'subopt1', 'subarg1', 'bsub'], '-') == ['--bsub-opt']
+    assert choices_without_help(cli,
+                                ['sub', '--sub-opt', 'subopt1', 'subarg1', 'bsub'], '') == ['bsubarg1', 'bsubarg2']
     assert choices_without_help(cli,
-        ['sub', '--sub-opt', 'subopt1', 'subarg1', 'bsub'], '-') == ['--bsub-opt']
+                                ['sub', '--sub-opt', 'subopt1', 'subarg1', 'bsub', '--bsub-opt'], '') == \
+           ['bsubopt1', 'bsubopt2']
     assert choices_without_help(cli,
-        ['sub', '--sub-opt', 'subopt1', 'subarg1', 'bsub', '--bsub-opt'], '') == \
-        ['bsubopt1', 'bsubopt2']
+                                ['sub', '--sub-opt', 'subopt1', 'subarg1', 'bsub', '--bsub-opt', 'bsubopt1', 'bsubarg1'],
+                                '') == ['bbsubarg1', 'bbsubarg2']
     assert choices_without_help(cli,
-        ['sub', '--sub-opt', 'subopt1', 'subarg1', 'bsub', '--bsub-opt', 'bsubopt1', 'bsubarg1'],
-                            '') == ['bbsubarg1', 'bbsubarg2']
+                                ['sub', '--sub-opt', 'subopt1', 'subarg1', 'csub'],
+                                '') == ['dsub']
+
+
+def test_chained_multi():
+    @click.group()
+    def cli():
+        pass
+
+    @cli.group()
+    def sub():
+        pass
+
+    @sub.group()
+    def bsub():
+        pass
+
+    @sub.group(chain=True)
+    def csub():
+        pass
+
+    @csub.command()
+    def dsub():
+        pass
+
+    @csub.command()
+    def esub():
+        pass
+
+    assert choices_without_help(cli, ['sub'], '') == ['bsub', 'csub']
+    assert choices_without_help(cli, ['sub'], 'c') == ['csub']
+    assert choices_without_help(cli, ['sub', 'csub'], '') == ['dsub', 'esub']
+    assert choices_without_help(cli, ['sub', 'csub', 'dsub'], '') == ['esub']