click-contrib / click-completion

Add or enhance bash, fish, zsh and powershell completion in Click
MIT License
288 stars 32 forks source link

Pass along cli args to the complete functions of params #12

Closed alanct-minted closed 6 years ago

alanct-minted commented 6 years ago

Thanks for the helpful library!

I wanted to see if you'd be open to an API change for custom complete() methods on params. This makes it more similar to how click handles bash autocomplete, and it unblocks some features for our team.


Passing the args along to the complete() function of a param provides for dynamic choices, similar to how the autocompletion() function documented by click describes:

https://github.com/pallets/click/blob/d37ff750f4ba4aa5af6104f5fcaa5d81dcc227d6/docs/bashcomplete.rst#what-it-completes

This is a breaking change for existing complete() functions.

cc @JoshDrake-minted

glehmann commented 6 years ago

I'm a bit reluctant to make an API change, because the library is beginning to be used in quite a few tools, and I'd prefer to not break anything. Could you elaborate on why you need the args to be passed to the completes method?

glehmann commented 6 years ago

BTW I was not aware of the changes made in click itself to add completion. We first work on fixing/enhancing click, but @mitsuhiko thought that the enhanced completion feature would be better in a click-contrib project.

Maintaining the same kind of completion feature in click and in click-contrib is a bit counter intuitive. Maybe @mitsuhiko, @stopthatcow or @davidism can give us more insight on where the completion feature should be?

alanct-minted commented 6 years ago

I wondered the impact of breaking API. If you want, we could inspect method signatures on complete() functions for an args keyword, and only pass if present to maintain backward compatibility.

Regarding the core click implementation-- honestly, I only saw some references in the code and found that bit of documentation in the repo, but I'm not sure it's published anywhere or that the API actually exists as documented. I didn't fiddle around enough to see since I'm already using click-completion, but I'm a little curious what the plans are there as well.

glehmann commented 6 years ago

Some inspection of the method signature would be nice!

But I still wonder what your use case is exactly — you've grabbed my curiosity :)

alanct-minted commented 6 years ago

Whoops, sorry! Missed that bit of the response. :)

We're making a CLI tool for managing our local docker/kubernetes containers in development. We have several "deployments" for various projects, and one of the commands we're providing in our CLI lets developers run configured project-specific commands inside the container.

So, say I have a deployment named ecommerce-backend, and it has a project-specific command loaddummyproducts, then the command in our CLI could look like:

$ ourtool run ecommerce-backend loaddummyproducts

So, the tough part is when we're trying to autocomplete here:

$ ourtool run ecommerce-backend <TAB>

At that point, we want to only show the project-specific commands as choices, based on the deployment argument before it (ecommerce-backend). We don't want to show project-specific commands from other deployments/projects. Having args in our complete() function makes this a breeze.

stopthatcow commented 6 years ago

Hi @alanct-minted Could you provide a snippet of code to illustrate what you are trying to accomplish? I don't see why what you are after is not supported by the current click completion system.

In the provided example is ecommerce-backend a subcommand of run or just an argument? From what I can understand it should probably be a subcommand. If it were a subcommand it could have its own completion function. Better yet if loaddummyproducts was be a subcommand of ecommerce-backend, then you wouldn't need a custom completion function at all!

Forgive me if I misunderstood your example.

glehmann commented 6 years ago

Thanks for the use case!

Wouldn't it be even easier if you had access to the current context? I guess you have to do some parsing by hand - not a problem if there if no option mixed to the arguments, but probably something to avoid if there are.

And in that specific use case, I think the API currently in click makes more sense, because the completion data is not linked to the data type only, but also to some of the other arguments. So the completion is probably better implemented at the command level rather than the type level.

In my opinion the implementation at the type level is the way to go most of the time, but we might want to keep the two API and make them work together for those more involved cases.

alanct-minted commented 6 years ago

@stopthatcow

Could you provide a snippet of code to illustrate what you are trying to accomplish?

Here's my current implementation (using args as passed in this PR). I simplified my example above for clarity, but the deployment argument is actually a <chart>.<deployment> argument. Eg. ourtool run ecommerce.backend loaddummyproducts or ourtool run ecommerce.frontend prettier:

class RunCommandParamType(click.ParamType):
    def complete(self, ctx, args, incomplete):
        try:
            deployment_arg = args[1]
        except IndexError:
            return []

        chart_name, deployment_name = deployment_arg.split('.')
        chart = Chart(chart_name, None)
        return [command for command, runnable
                in chart.config.get('runnables', {}).items()
                if runnable.get(deployment_name)]

Might be a little easier to read a test, which shows what the config object looks like:

    def test_run_choices(self, safe_load, builtin_open):
        param = RunCommandParamType()
        safe_load.return_value = {
            'runnables': {
                'dothing1': {
                    'foo': {
                        'command': 'ps',
                    },
                },
                'dothing2': {
                    'foo': {
                        'command': 'ps',
                    },
                    'buzz': {
                        'command': 'ps',
                    },
                },
                'dothing3': {
                    'buzz': {
                        'command': 'ps',
                    },
                },
            },
        }

        choices = param.complete(None, ['run', 'fizz.buzz'], None)
        self.assert_lists_equal(choices, ['dothing2', 'dothing3'])

So, while I can understand that making the deployment param a subcommand instead (and the runnable commands after them) would get me the default autocompletion, these are extremely dynamic arguments. Both parsing available charts and deployments from configs on the user's machine (they aren't the same for everyone), and parsing the argument to know which runnable commands are available. Since click and click-completion like to have all the commands defined at module instantiation time, we'd be looking at doing a lot of magic to build the subcommands, which would mean some costlier startup time and ultimately less understandable code in my opinion. Not totally opposed to the idea, though!

@glehmann

Wouldn't it be even easier if you had access to the current context?

Maybe! I tried poking around in context first, but didn't find what I was looking for. It seems ctx.params is empty during autocomplete. I didn't really want to reinvent the wheel on parsing, but I'd be game to do it. I was also concerned about interspersed options, though. Where are the currently completed args (or full command text) accessible in context?

So the completion is probably better implemented at the command level rather than the type level.

Ahh, I missed that detail, that click's implementation lived at the command level. I may try taking a stab at it from that angle. Is it bash-only? If click-completion's cross-shell goodness harmonized well with it, I'm totally fine with just doing completion on a command level.

Thanks so much to both for all the suggestions and different approaches!

stopthatcow commented 6 years ago

@alanct-minted there are always several ways to skin the proverbial cat! I believe that what you are trying to accomplish seems like a candidate for a supported pattern in core: the custom multi commands feature. This should allow you to dynamically generate commands lazily. You can get dynamic help this way as well and in general because commands are actually first class commands, autocompletion will work as well (get_command() will be invoked dynamically by core's completion). This approach seems like the most "click-ish" way to approach this feature however core's autocompletion function could be used to accomplish the same thing.

@glehmann I have been the author of some of the more recent click core completion PRs and was pleasantly surprised to find that this click-completion repo existed. Click's autocompletion has come a long way toward supporting dynamic completions in 7.0 (not yet released) Dynamic command completions (which would support this use case out of the box) are available via the autocompletion function parameter. Native Bash and ZSH completions (with help text) are both supported. In future I would be in favor of folding a lot of this repo's functionality into click core to better keep it in sync with changes there.

Konubinix commented 6 years ago

@alanct-minted We have a similar issues in several commands in my project. I general handle that this way: Provided I have a command CMD with arguments ARG1 and ARG2, with the completion of ARG2 that depends on the value of ARG1.

We know that:

  1. the ctx object is the parsing context. It is the same object during the parsing of all the parameters
  2. the callback of the parameters are always called and in the correct order

Then a. In ARG1, add a callback to store the value given to arg1 into ctx b. Add a type for ARG2 that uses the value stored in ctx.

Here is a minimal example to illustrate this.

import click
import click_completion

click_completion.init(complete_options="COMPLETE_ME")

def arg1_callback(ctx, attr, value):
    ctx.arg1_value = value

class Arg2Type(click.ParamType):
    def complete(self, ctx, incomplete):
        print(ctx.arg1_value)
        return [incomplete]

@click.command()
@click.argument("arg1", callback=arg1_callback)
@click.argument("arg2", type=Arg2Type())
def cmd(arg1, arg2):
    pass

if __name__ == "__main__":
    cmd(complete_var="COMPLETE_ME")

Now, call this with COMP_WORDS="test.py a " COMP_CWORD=3 COMPLETE_ME=complete python3 ~/test/test.py a to try it.

My two cents.

alanct-minted commented 6 years ago

@stopthatcow I'm still not fully convinced that the chart/deployment param is conceptually a command (with need for help text, it's own params, etc.), even though it's possible to treat it that way. It's a somewhat close match in our run command, but for all of our other commands, it's definitely just an argument, not a command. I do appreciate the multi-command option, though, as a potential workaround. It does still feel like a hack to me, but we may go that route if all else fails.

@Konubinix That's a nice workaround! Thanks for sharing it. This seems to be the cleanest without breaking anything conceptually. In fact Context has an obj attribute that I think it intended for arbitrary app data like this. It's somewhat akward to use it just for completion, but makes sense to me given the constraints. :+1:

glehmann commented 6 years ago

This seems to be the cleanest without breaking anything conceptually.

I agree, and it would work without any need to do some manual parsing of the args :+1:

For the future, we should think about something to access the already filled options and arguments values during the completion.

@stopthatcow I'll open a new issue on click to discuss about merging click-completion in click itself. We can continue the discussion there :)

alanct-minted commented 6 years ago

Sounds good! Since we can get where we're going with @Konubinix's workaround, I'll close this for now. We'll see if there's any reason to resurface once the click/click-completion destination is determined. Cool stuff, good luck with all that part!

Side note-- click-completion I think has the most robust shell-specific functionality for completion I've found in the python world. I even thought of using it as a library and importing its utility methods for some of our handrolled completion stuff. It would be a great contribution to click upstream if you end up merging in.