cobra icon indicating copy to clipboard operation
cobra copied to clipboard

Allow user to add completion for powershell alias

Open marckhouzam opened this issue 2 years ago • 8 comments

I ran across a possible problem with using powershell completion with aliases. I can't recall where it was though, but the situation is pretty simple.

IIUC, when a user has an alias in powershell, she needs to register that alias for completion using:

PS> Register-ArgumentCompleter -CommandName 'aliasname' -ScriptBlock <completionBlock>

The problem is that the <completionBlock> is currently not accessible in our powershell script.

To address this, this commit stores the completion logic into a scriptblock variable which can easily be accessed by the user to register aliases.

For example, if the user defines an alias for helm she will be able to register an alias like this:

PS> sal h helm
PS> Register-ArgumentCompleter -CommandName 'h' -ScriptBlock $__helmCompleterBlock

@Luap99 does this make sense to you? I may be misunderstanding something fundamental about aliases and powershell...

marckhouzam avatar Mar 08 '22 19:03 marckhouzam

How does this work with the other shells at the moment?

I do not use powershell any more but I think this will work. Back when I worked on this there weren't many projects with powershell completion so I am not sure if there is maybe some pseudo standard which can/should be followed.

Luap99 avatar Mar 17 '22 22:03 Luap99

This whole idea came out of a comment from the PR that added Powershell completion to kubectl: https://github.com/kubernetes/kubernetes/pull/103758#issuecomment-1007586166

There was a project that implemented a hard-coded script for powershell completion for kubectl which handled aliases in this way. This is the specific line that inspired me: https://github.com/mziyabo/PSKubectlCompletion/blob/f6f28153a616e14789a3a7fbcb493ff7f1f1ae85/PSKubectlCompletion.psm1#L20

marckhouzam avatar Mar 18 '22 01:03 marckhouzam

I personally do not like to pollute the global variable scope in the shell but the variable name seems reasonable unique so it should be fine. Since there is a legitimate use case for this and I cannot offer a better solution I am fine with this PR.

Luap99 avatar Mar 18 '22 09:03 Luap99

Hmmmmm anyone out in the community have an easy way to test this on Windows? I'm without my windows box at the moment.

jpmcb avatar Mar 29 '22 23:03 jpmcb

The Cobra project currently lacks enough contributors to adequately respond to all PRs. This bot triages issues and PRs according to the following rules:

  • After 60d of inactivity, lifecycle/stale is applied. - After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied and the PR is closed. You can:
  • Make a comment to remove the stale label and show your support. The 60 days reset. - If a PR has lifecycle/rotten and is closed, comment and ask maintainers if they'd be interseted in reopening.

github-actions[bot] avatar May 29 '22 00:05 github-actions[bot]

Still valid

marckhouzam avatar May 29 '22 01:05 marckhouzam

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Jul 21 '22 02:07 CLAassistant

I fixed this PR which is still relevant.

marckhouzam avatar Sep 18 '22 03:09 marckhouzam

Yeah, it is ready. I am able to test it on powershell on Mac, and @Luap99 is happy with the change. Will merge.

marckhouzam avatar Oct 03 '22 17:10 marckhouzam

Sorry. But why doesn't it work for me?

# PowerShell auto-completion
kubectl completion powershell | Out-String | Invoke-Expression
Set-Alias -Name k -Value kubectl
Register-ArgumentCompleter -CommandName k -ScriptBlock $__kubectlCompleterBlock

cloud-66 avatar Feb 17 '23 18:02 cloud-66

What version of kubectl are you using? I believe you need 1.26 or higher.

marckhouzam avatar Feb 18 '23 03:02 marckhouzam

What version of kubectl are you using? I believe you need 1.26 or higher.

Yeah. Sorry to bother you. Kubectl 1.26+ depends on cobra 1.6.0 with this pr. thank you it works .

cloud-66 avatar Feb 18 '23 15:02 cloud-66