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

Allow completionCommand to be set via showCompletionScript #1385

Merged
merged 2 commits into from Jul 30, 2019

Conversation

cameronhunter
Copy link
Contributor

Problem

When using showCompletionScript the variable completionCommand is null resulting in a bad installation instruction. For example:

yargs.showCompletionScript('/usr/local/bin/my-app');

###-begin-my-app-completions-###
#
# yargs command completion script
#
# Installation: /usr/local/bin/my-app null >> ~/.bashrc
#    or /usr/local/bin/my-app null >> ~/.bash_profile on OSX.
#
_yargs_completions()
{
    local cur_word args type_list

    cur_word="${COMP_WORDS[COMP_CWORD]}"
    args=("${COMP_WORDS[@]}")

    # ask yargs to generate completions.
    type_list=$(/usr/local/bin/my-app --get-yargs-completions "${args[@]}")

    COMPREPLY=( $(compgen -W "${type_list}" -- ${cur_word}) )

    # if no match was found, fall back to filename completion
    if [ ${#COMPREPLY[@]} -eq 0 ]; then
      COMPREPLY=()
    fi

    return 0
}
complete -o default -F _yargs_completions my-app
###-end-my-app-completions-###

Notice that null is printed for the command.

It's possible to set the completionCommand variable by calling yargs.completion() but this will also log the completion script resulting in two scripts being logged when used in conjunction with showCompletionScript:

yargs.completion().showCompletionScript('/usr/local/bin/my-app');

###-begin-my-app-completions-###
#
# yargs command completion script
#
# Installation: /usr/local/bin/my-app completion >> ~/.bashrc
#    or /usr/local/bin/my-app completion >> ~/.bash_profile on OSX.
#
_yargs_completions()
{
    local cur_word args type_list

    cur_word="${COMP_WORDS[COMP_CWORD]}"
    args=("${COMP_WORDS[@]}")

    # ask yargs to generate completions.
    type_list=$(/usr/local/bin/my-app --get-yargs-completions "${args[@]}")

    COMPREPLY=( $(compgen -W "${type_list}" -- ${cur_word}) )

    # if no match was found, fall back to filename completion
    if [ ${#COMPREPLY[@]} -eq 0 ]; then
      COMPREPLY=()
    fi

    return 0
}
complete -o default -F _yargs_completions my-app
###-end-my-app-completions-###

###-begin-my-app-completions-###
#
# yargs command completion script
#
# Installation: my-app completion >> ~/.bashrc
#    or my-app completion >> ~/.bash_profile on OSX.
#
_yargs_completions()
{
    local cur_word args type_list

    cur_word="${COMP_WORDS[COMP_CWORD]}"
    args=("${COMP_WORDS[@]}")

    # ask yargs to generate completions.
    type_list=$(my-app --get-yargs-completions "${args[@]}")

    COMPREPLY=( $(compgen -W "${type_list}" -- ${cur_word}) )

    # if no match was found, fall back to filename completion
    if [ ${#COMPREPLY[@]} -eq 0 ]; then
      COMPREPLY=()
    fi

    return 0
}
complete -o default -F _yargs_completions my-app
###-end-my-app-completions-###

In this case the first completion script output is correct but the second one is incorrect.

Fix

Add a second parameter to showCompletionScript that allows the developer to set the completion command variable.

yargs.showCompletionScript('/usr/local/bin/my-app', 'completion');

###-begin-my-app-completions-###
#
# yargs command completion script
#
# Installation: /usr/local/bin/my-app completion >> ~/.bashrc
#    or /usr/local/bin/my-app completion >> ~/.bash_profile on OSX.
#
_yargs_completions()
{
    local cur_word args type_list

    cur_word="${COMP_WORDS[COMP_CWORD]}"
    args=("${COMP_WORDS[@]}")

    # ask yargs to generate completions.
    type_list=$(/usr/local/bin/my-app --get-yargs-completions "${args[@]}")

    COMPREPLY=( $(compgen -W "${type_list}" -- ${cur_word}) )

    # if no match was found, fall back to filename completion
    if [ ${#COMPREPLY[@]} -eq 0 ]; then
      COMPREPLY=()
    fi

    return 0
}
complete -o default -F _yargs_completions my-app
###-end-my-app-completions-###

It will default to "completion" in the same way that completion() does.

Copy link
Member

@bcoe bcoe left a comment

Choose a reason for hiding this comment

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

so this bug cropped up if you manually called showCompletionScript to get yargs to output the completion script, rather than passing an argument to generate a completion command?

I'm curious, what's your use case?

@@ -907,7 +907,7 @@ function Yargs (processArgs, cwd, parentRequire) {
}

// register the completion command.
completionCommand = cmd || 'completion'
Copy link
Member

Choose a reason for hiding this comment

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

I'm slightly confused, wouldn't this have defaulted to completion? where was the null value coming from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'm not adding any custom completion options, I'm wanting to leverage yargs' built-in completions. Because of this I never call the completion() function, just showCompletionScript(). The completion command will always be null unless completion() is called (source 1, 2, 3).

test/completion.js Outdated Show resolved Hide resolved
@cameronhunter
Copy link
Contributor Author

Our use-case is that we want to set $0 and the completion command name. There are two APIs that could achieve this: showCompletionScript and completion.

showCompletionScript($0)

Currently showCompletionScript($0) supports setting $0 but not the completion command. It has a bug where it uses the variable completionCommand which will be null (source) since it can only be set via completion (source) resulting in the installation instructions showing null as the command completion:

###-begin-tvui-completions-###
#
# yargs command completion script
#
# Installation: tvui null >> ~/.bashrc
#    or tvui null >> ~/.bash_profile on OSX.
#

completion(cmd)

Currently completion allows us to set the completion command, but not $0. For us this results in the completion script not working because our yargs-powered command-line app isn't on the global path.

Can we use completion and showCompletionScript together to resolve this issue?

I gave this a go because I was hoping that I could set the completion command using completion('my-completion-command') and then set $0 when outputting with showCompletionScript('/path/to/my/app'). Unfortunately both of these functions output the completion script 😭

How I ended up where I ended up

In the end I added a second parameter to showCompletionScript to set the completion command name. I also fixed the null issue by defaulting the completion command to "completion" in the same manner as the completion function does.

@bcoe bcoe merged commit 5562853 into yargs:master Jul 30, 2019
@bcoe
Copy link
Member

bcoe commented Jul 30, 2019

@cameronhunter excited to have the contributions, thank you 👍 will work on getting a release out soon.

@bcoe
Copy link
Member

bcoe commented Jul 30, 2019

@cameronhunter please try npm i yargs@next on a few projects, and let me know if everything is working as expected 👍 thank you for the contributions.

@cameronhunter
Copy link
Contributor Author

Thanks for merging, though I think this PR isn't acting as expected 😓 – I'm seeing the script being logged twice.

I think it's due to me setting completionCommand in showCompletionScript and then this yargs.js#1106-1110 codepath.

I'm trying to piece together a test-case but I think the correct fix is not to set completionCommand. I'll follow-up with a PR.

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.

None yet

2 participants