Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Autocompletion for multiple positional arguments when flags are present #146

Open
swalkinshaw opened this issue Jan 4, 2022 · 2 comments

Comments

@swalkinshaw
Copy link

I ran into troubles when implementing autocompletion for multiple positional arguments if flags are present.

Scenario:

  • command with multiple positional arguments (required or optional, doesn't matter)
  • command also accepts flags

In order to implement autocompletion for both position arguments, you need to check how many args have already been completed (via args.Completed). Here's an example from terraform (and the implementation).

And here's another example from my own CLI package.

These work fine when no flags are present, but it breaks down if there's flags because those flags will be included in args.Completed which means you can't naively just check the length of the array.

Example:

trellis deploy prod<tab>
# autocompletes to production

trellis deploy production exam<tab>
# autocompletes to example.com

trellis deploy --bran<tab>
# autocompletes to branch

trellis deploy --branch=foo prod<tab>
# does not autocomplete properly

Solution

I think I've been able to fix this by passing in the flag.FlagSet and parsing the args on every prediction. Then instead of checking the length of args.Completed, you use the length of parsed positional args. However, this adds a lot of complexity and seemed non-intuitive. Here's my solution so far: https://github.com/roots/trellis-cli/blob/dd2156bf2c923978ef3866c2b00517d903553cc8/trellis/complete.go#L38-L55

Since args and flags are handled separately in complete.Command, I'd kind of expect that the predict function just for args wouldn't include the flags. Or if it did (because that might be useful in case positional args depend on flags?), complete.Args would at least handle this automatically and provide both separately?

Idea

This would be a breaking change, but could this package move completed flags out of args.Completed and into args.FlagsCompleted? Or maybe Predict could be called like this for arguments:

c.Predict(args, flags)

Though that seems much more breaking. I'm probably missing other solutions as well though 😅

@posener
Copy link
Owner

posener commented Feb 13, 2022

Hi Scott,
Sorry for the late reply.
I see your point, and indeed this library does not provide a good solution for this.
I would recon that this is quite a corner case, since the chosen API for the CLI could be improved. I think it is not a common case where a program gets multiple positional arguments, and each of them has a different "type". Usually, one of these args would be given as a flag (In your case I guess that the environment could be given as a flag, and not as a positional argument).
Maybe I'm wrong, but I don't see an easy solution without breaking that API.
If you have any suggestions, please let me know!

@swalkinshaw
Copy link
Author

Maybe I'm wrong, but I don't see an easy solution without breaking that API.
If you have any suggestions, please let me know!

I was hoping you might since I couldn't see any other way either 😅

I've already solved it and the workaround isn't so bad so I'm fine regardless. Feel free to close this if you want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants