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

Powershell completion alias handling #2049

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bartoncasey
Copy link

Currently the Powershell completion generator registers the completion for the default command name. Any aliases of the command will not have completion enabled. (e.g. k -> kubectl)

This change will include code to register the completer for all current aliases of the command in addition to the command itself.

@CLAassistant
Copy link

CLAassistant commented Oct 18, 2023

CLA assistant check
All committers have signed the CLA.

@marckhouzam
Copy link
Collaborator

Thanks @bartoncasey.
There is #1977 which is trying to do this. I don't have much expertise with PowerShell, so maybe you can help figure out if #1977 solves things for you?

@bartoncasey
Copy link
Author

bartoncasey commented Oct 19, 2023

Oh, no. I should have done a better job of looking for existing PRs.

It looks like #1977 is assuming that you will always alias things using the absolute path of the executable:

Set-Alias -Name k -Value "C:\Program Files\Docker\Docker\resources\bin\kubectl.exe"

vs what I think is the more convential form of

Set-Alias -Name k -Value kubectl

Mine looks for aliases of the latter form.

@jpmcb jpmcb added area/shell-completion All shell completions triage/blocked Cannot move forward until some roadblock is lifted labels Oct 28, 2023
mavaddat added a commit to mavaddat/cobra that referenced this pull request Oct 29, 2023
Incorporate @bartoncasey insight about typical alias usage in spf13#2049
@marckhouzam
Copy link
Collaborator

Set-Alias -Name k -Value "C:\Program Files\Docker\Docker\resources\bin\kubectl.exe"

I've tested this PR and although it is working very nicely for normal aliases, I don't believe it works for the above form.

So now I'm going back and forth between this PR and #1977 hoping to find the sweet spot.

@marckhouzam
Copy link
Collaborator

marckhouzam commented Oct 31, 2023

After spending quite a bit of time trying things out, I think this solution is the simplest and I believe can be made to cover every case. I was thinking of these cases:

  1. any alias pointing to kubectl or kubectl.exe
  2. any alias pointing to /<path>/kubectl or /<path>/kubectl.exe

I think we can achieve this with a slight modification do have instead:

# Register the completer for the command and for all aliases of the command
'%[1]s', (Get-Alias -Definition %[1]s,*/%[1]s,%[1]s.exe,*/%[1]s.exe -ErrorAction Ignore).Name | ForEach-Object {
    if ($_) {
        Register-ArgumentCompleter -CommandName $_ -ScriptBlock ${__%[2]sCompleterBlock}
    }
}

This translates to something like
Get-Alias -Definition kubectl,*/kubectl,kubectl.exe,*/kubectl.exe
which covers the cases listed above.

@bartoncasey What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/shell-completion All shell completions triage/blocked Cannot move forward until some roadblock is lifted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants