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

Autocomplete a command's args #106

Merged
merged 3 commits into from
Mar 14, 2016
Merged

Conversation

heewa
Copy link
Contributor

@heewa heewa commented Feb 18, 2016

This change handles autocompleting scenarios like program command <tab> where command has arguments with completion options or an action-fn. For ex, if git used kingpin, git checkout <tab> could complete branch names.

{
// After a command with arg options
Args: "--completion-bash three ",
ExpectedOptions: []string{"arg-2-opt-1", "arg-2-opt-2", "arg-3-opt-1", "arg-3-opt-2"},
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC, these hints shouldn't be available here, as (unlike flags) args are positional. At this point it should be completing for arg-1 only which has no hints and thus should not be completing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I'll have to spend a bit more time understanding the parsing logic to plumb down where in the process it is to the code that knows where to get completion options from. I'll get on that.

@alecthomas
Copy link
Owner

Hi @heewa,

I'd love to include this if the kinks can be worked out; it would make Kingpin's completion amazing.

Related, I don't think the completion code in master works with commands either (at least, not in the curl example code), so if you can get args working it would be great to get commands working too.

Thanks,
Alec

@heewa
Copy link
Contributor Author

heewa commented Feb 18, 2016

I'll take a look at the curl example. Also, I have a small change to hide hidden commands from completing. Would you like that in a separate PR, or should I attach it to this one?

@alecthomas
Copy link
Owner

Whichever you prefer.

@heewa
Copy link
Contributor Author

heewa commented Feb 25, 2016

@alecthomas Sorry it took so long, I had to work on other stuff for a bit. But I got the curl example (and any other program with default args, which were all broken) to work! 🎉

The implementation might seem a little weird - tacking on completion alternatives to a command. But the alternative was to re-implement something much like the regular parse logic in the completer function, since it's not clear to the completer what commands have been explicitly listed in the cmdline, and what were selected as defaults by the parser. And, even once it decided which ones were implicitly selected, deciding alternatives would be akin to going down the parse tree, since any level of sub-default-cmds could exist.

What do you think?

@alecthomas
Copy link
Owner

I think if it works, it's fine for now.

I think I'm going to reimplement the completion completely for v3, as the implementation is a bit all over the place at the moment.

alecthomas added a commit that referenced this pull request Mar 14, 2016
@alecthomas alecthomas merged commit 81eebc3 into alecthomas:master Mar 14, 2016
@alecthomas
Copy link
Owner

Thanks for implementing this!

@ches
Copy link

ches commented Aug 4, 2016

Thanks for this @heewa, this is really useful.

I've found a problem though, or at least an unfortunate inconsistency. Here's a diff with a failing test case that demonstrates:

diff --git a/app_test.go b/app_test.go
index 999af36..bdf8ae1 100644
--- a/app_test.go
+++ b/app_test.go
@@ -364,6 +364,12 @@ func TestBashCompletionOptions(t *testing.T) {
            ExpectedOptions: []string{"arg-2-opt-1", "arg-2-opt-2"},
        },
        {
+           // After a command with an arg that does have completions, they
+           // should be partially complete-able
+           Args:            "--completion-bash three arg1 arg-2-op",
+           ExpectedOptions: []string{"arg-2-opt-1", "arg-2-opt-2"},
+       },
+       {
            // After a command with an arg that does have completions, but a
            // flag is started, flag options should be completed
            Args:            "--completion-bash three arg1 --",

I would expect this to work, similar behavior for flag completion does work as this existing case shows:

        {
            Args:            "--completion-bash --flag-1 opt",
            ExpectedOptions: []string{"opt1", "opt2", "opt3"},
        },

But the test fails, it returns no completions. In practical terms, this means HintOptions and HintAction have differing behavior between flags and args. This lets you complete multiple values with the same prefix:

a := newTestApp()
a.Flag("flag", "").HintOptions("opt1", "opt2", "opt3").String()

This does not:

a := newTestApp()
c := a.Command("connect", "Connect to a host.")
c.Arg("host", "").HintOptions("host-one", "host-two").String()

You'll get host- completed on the command line and then you're stuck, you can't tab after typing "o". Commands with a common prefix can be completed as well.

I'll take a shot at fixing this, but I'm not familiar with the internals so @heewa if this code is still in your brain months later and you see a quick solution, let me know 😄

@heewa
Copy link
Contributor Author

heewa commented Aug 4, 2016

Oof, that's a hard one, @ches. It stems from the cmd completion logic being unable to tell that the last arg isn't finished. Because it gets the command line args as a slice of strings, it doesn't know that the user is still typing it (ie, that it isn't followed by a space).

So, it thinks the argument is already satisfied, and tries to complete whatever comes next, which in this case is nothing, because there aren't any hints for arg3. In fact, you can see this behavior by modifying the test like:

diff --git a/app_test.go b/app_test.go
index 999af36..6e42486 100644
--- a/app_test.go
+++ b/app_test.go
@@ -248,7 +248,7 @@ func TestBashCompletionOptions(t *testing.T) {
        three.Flag("flag-4", "").String()
        three.Arg("arg-1", "").String()
        three.Arg("arg-2", "").HintOptions("arg-2-opt-1", "arg-2-opt-2").String()
-       three.Arg("arg-3", "").String()
+       three.Arg("arg-3", "").HintOptions("arg-3-opt-1", "arg-3-opt-2").String()
        three.Arg("arg-4", "").HintAction(func() []string {
                return []string{"arg-4-opt-1", "arg-4-opt-2"}
        }).String()

and see that the test fails by outputting [arg-3-opt-1 arg-3-opt-2] instead of what you expected, [arg-2-opt-1 arg-2-opt-2].

If you wanted to fix this behavior, you'd have to get that information (that the last arg might not yet be "complete".. though it might be, cuz the hints are just hints) to the cmdMixin.CmdCompletion() fn in cmd.go.

Probably your best bet would be in the parse() fn in parse.go, but I'm not very hopeful that it's possible, considering that (at a very cursory glance) it looks like the tokenizer doesn't distinguish between a token that is proceeded by a space and one that isn't.

You might be able to kinda hack it by making the assumption that if the last thing in parsed command line is an arg, and that arg prefix-matches hints, then maybe it's not yet complete... but I wonder if that might raise other weird edge cases of its own. For ex, program completedArg <tab> would incorrectly complete completedArgWithMore if that was a hint for the first arg, even though the user typed a space before hitting tab.

Anyway, good luck, and let me know how it goes!

ches added a commit to go-kafka/connect that referenced this pull request Aug 4, 2016
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

Successfully merging this pull request may close these issues.

3 participants