Skip to content

Conversation

AvianY
Copy link
Contributor

@AvianY AvianY commented Apr 9, 2018

Various edge cases have been solved, such as completion for invalid subcommands etc.

@tyru
Copy link
Member

tyru commented Apr 9, 2018

hmm, would you please describe more details?
I want the examples of behaviors you fixed.

@AvianY
Copy link
Contributor Author

AvianY commented Apr 9, 2018

completion like:
volt migrate asdf <Tab> (note, asdf is not a subcommand of migrate)
or:
volt profile list <Tab> (list doesn't have a subcommand or argument)
or:
volt profile add asdf <Tab> (note, asdf is not a profile)
or:
volt get -u <Tab> (should autocomplete any plugin, because -u updates)
volt get -l <Tab> OR volt get -u -l <Tab> (should not autocomplete anything, -l means all)

@tyru
Copy link
Member

tyru commented Apr 9, 2018

volt migrate asdf <Tab> (note, asdf is not a subcommand of migrate)
or:
volt profile list <Tab> (list doesn't have a subcommand or argument)
or:
volt profile add asdf <Tab> (note, asdf is not a profile)
or:
volt get -u <Tab> (should autocomplete any plugin, because -u updates)

Yes, it seems good :)

-l means all

It's slightly different.
-l means "the repositories of current profile".
But I think the fixed behavior is better, because -l and arguments cannot be mixed (arguments are ignored. however, I think mixed -l and arguments should be error).


I found the case that completion does not work:

$ volt profile add def<Tab>

doesn't complete default.
Can you fix this?

@AvianY
Copy link
Contributor Author

AvianY commented Apr 9, 2018

yes, I fixed it, you just have to swap the first and second if statement in the large block and renama if/elif.
Here is the diff of the extra changes

diff --git a/_contrib/completion/bash b/_contrib/completion/bash
index 31cbb33..1b63951 100644
--- a/_contrib/completion/bash
+++ b/_contrib/completion/bash
@@ -38,6 +38,10 @@ _volt() {
        local second="${COMP_WORDS[2]}"
        local third="${COMP_WORDS[3]}"
+       elif [[ "${first}" == "profile" && "${second}" =~ ^(set|show|new|destroy|rename|add|rm)$ && "${last}" =~ ^(set|show|new|destroy|rename|add|rm)$ ]] ; then
+               local profiles=$(get_profiles)
+               COMPREPLY=( $(compgen -W "${profiles}" -- ${cur}) )
+
        if [[ "${first}" == "profile" && "${second}" =~ ^(add|rm)$ && "${third}" ]] ; then
                local profile=$third
                case "$second" in
@@ -46,10 +50,6 @@ _volt() {
                esac
                COMPREPLY=( $(compgen -W "${plugs}" -- ${cur}) )
-       elif [[ "${first}" == "profile" && "${second}" =~ ^(set|show|new|destroy|rename|add|rm)$ && "${last}" =~ ^(set|show|new|destroy|rename|add|rm)$ ]] ; then
-               local profiles=$(get_profiles)
-               COMPREPLY=( $(compgen -W "${profiles}" -- ${cur}) )
-
        elif [[ "${first}" == "profile" && "${last}" == "profile" ]] ; then
                COMPREPLY=( $(compgen -W "${PROFILE_CMDS}" -- ${cur}) )

I am not used to working with github yet, so I don't know how to work with forked repos and other things soefficiently

@tyru
Copy link
Member

tyru commented Apr 9, 2018

Okay, confirmed that the patch fixes volt profile add def<Tab> and does not break all above behaviors.

I am not used to working with github yet, so I don't know how to work with forked repos and other things soefficiently

You can commit the change, and git push again.
The change appears also in this page.

@AvianY
Copy link
Contributor Author

AvianY commented Apr 9, 2018

Thank you, it worked. I also removed the |new| part of the first if, because "new" does not receive an existing profile as argument.

@tyru tyru merged commit 5aa78e9 into vim-volt:devel Apr 9, 2018
@tyru
Copy link
Member

tyru commented Apr 9, 2018

Perfect! Merged it :)

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

Successfully merging this pull request may close these issues.

2 participants