Repository: spf13/cobra
Pull request #1621 (2022-03-08 19:48:28)

Allow user to add completion for powershell alias
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...

Luap99: 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. 
marckhouzam: 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
Luap99: 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.
jpmcb: Hmmmmm anyone out in the community have an easy way to test this on Windows? I'm without my windows box at the moment.
github-actions[bot]: 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.
marckhouzam: Still valid
CLAassistant: [![CLA assistant check](https://cla-assistant.io/pull/badge/not_signed)](https://cla-assistant.io/spf13/cobra?pullRequest=1621) <br/>Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our [Contributor License Agreement](https://cla-assistant.io/spf13/cobra?pullRequest=1621) before we can accept your contribution.<br/><hr/>**Marc Khouzam** seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please [add the email address used for this commit to your account](https://help.github.com/articles/why-are-my-commits-linked-to-the-wrong-user/#commits-are-not-linked-to-any-user).<br/><sub>You have signed the CLA already but the status is still pending? Let us [recheck](https://cla-assistant.io/check/spf13/cobra?pullRequest=1621) it.</sub>
marckhouzam: I fixed this PR which is still relevant.

Patch:
diff --git a/.github/workflows/labeler.yml b/.github/workflows/labeler.yml
index e8b9651..17f451f 100644
--- a/.github/workflows/labeler.yml
+++ b/.github/workflows/labeler.yml
@@ -2,8 +2,14 @@ name: "Pull Request Labeler"
 on:
 - pull_request_target
 
+permissions:
+  contents: read
+
 jobs:
   triage:
+    permissions:
+      contents: read  # for actions/labeler to determine modified files
+      pull-requests: write  # for actions/labeler to add labels to PRs
     runs-on: ubuntu-latest
     steps:
     - uses: actions/labeler@v4
diff --git a/.github/workflows/size-labeler.yml b/.github/workflows/size-labeler.yml
index f04024f..4c54d82 100644
--- a/.github/workflows/size-labeler.yml
+++ b/.github/workflows/size-labeler.yml
@@ -4,8 +4,13 @@ name: size-labeler
 
 on: [pull_request_target]
 
+permissions:
+  contents: read
+
 jobs:
   size-labeler:
+    permissions:
+      pull-requests: write  # for codelytv/pr-size-labeler to add labels & comment on PRs
     runs-on: ubuntu-latest
     name: Label the PR size
     steps:
diff --git a/.github/workflows/stale.yml b/.github/workflows/stale.yml
index 8f8c739..2db5123 100644
--- a/.github/workflows/stale.yml
+++ b/.github/workflows/stale.yml
@@ -4,13 +4,19 @@ on:
   schedule:
   - cron: "0 0 * * *"
 
+permissions:
+  contents: read
+
 jobs:
   stale:
 
+    permissions:
+      issues: write  # for actions/stale to close stale issues
+      pull-requests: write  # for actions/stale to close stale PRs
     runs-on: ubuntu-latest
 
     steps:
-    - uses: actions/stale@v5
+    - uses: actions/stale@v6
       with:
         repo-token: ${{ secrets.GITHUB_TOKEN }}
         stale-issue-message: "The Cobra project currently lacks enough contributors to adequately respond to all issues.
diff --git a/cobra.go b/cobra.go
index 1a95c3c..fe44bc8 100644
--- a/cobra.go
+++ b/cobra.go
@@ -40,6 +40,7 @@ var templateFuncs = template.FuncMap{
 }
 
 var initializers []func()
+var finalizers []func()
 
 const (
 	defaultPrefixMatching  = false
@@ -94,6 +95,12 @@ func OnInitialize(y ...func()) {
 	initializers = append(initializers, y...)
 }
 
+// OnFinalize sets the passed functions to be run when each command's
+// Execute method is terminated.
+func OnFinalize(y ...func()) {
+	finalizers = append(finalizers, y...)
+}
+
 // FIXME Gt is unused by cobra and should be removed in a version 2. It exists only for compatibility with users of cobra.
 
 // Gt takes two types and checks whether the first type is greater than the second. In case of types Arrays, Chans,
diff --git a/command.go b/command.go
index cdfd360..8ce5c3c 100644
--- a/command.go
+++ b/command.go
@@ -834,6 +834,8 @@ func (c *Command) execute(a []string) (err error) {
 
 	c.preRun()
 
+	defer c.postRun()
+
 	argWoFlags := c.Flags().Args()
 	if c.DisableFlagParsing {
 		argWoFlags = a
@@ -862,10 +864,10 @@ func (c *Command) execute(a []string) (err error) {
 		c.PreRun(c, argWoFlags)
 	}
 
-	if err := c.validateRequiredFlags(); err != nil {
+	if err := c.ValidateRequiredFlags(); err != nil {
 		return err
 	}
-	if err := c.validateFlagGroups(); err != nil {
+	if err := c.ValidateFlagGroups(); err != nil {
 		return err
 	}
 
@@ -904,6 +906,12 @@ func (c *Command) preRun() {
 	}
 }
 
+func (c *Command) postRun() {
+	for _, x := range finalizers {
+		x()
+	}
+}
+
 // ExecuteContext is the same as Execute(), but sets the ctx on the command.
 // Retrieve ctx by calling cmd.Context() inside your *Run lifecycle or ValidArgs
 // functions.
@@ -1019,7 +1027,8 @@ func (c *Command) ValidateArgs(args []string) error {
 	return c.Args(c, args)
 }
 
-func (c *Command) validateRequiredFlags() error {
+// ValidateRequiredFlags validates all required flags are present and returns an error otherwise
+func (c *Command) ValidateRequiredFlags() error {
 	if c.DisableFlagParsing {
 		return nil
 	}
@@ -1122,7 +1131,8 @@ Simply type ` + c.Name() + ` help [path to command] for full details.`,
 					c.Printf("Unknown help topic %#q\n", args)
 					CheckErr(c.Root().Usage())
 				} else {
-					cmd.InitDefaultHelpFlag() // make possible 'help' flag to be shown
+					cmd.InitDefaultHelpFlag()    // make possible 'help' flag to be shown
+					cmd.InitDefaultVersionFlag() // make possible 'version' flag to be shown
 					CheckErr(cmd.Help())
 				}
 			},
diff --git a/flag_groups.go b/flag_groups.go
index b09236f..9c377aa 100644
--- a/flag_groups.go
+++ b/flag_groups.go
@@ -59,9 +59,9 @@ func (c *Command) MarkFlagsMutuallyExclusive(flagNames ...string) {
 	}
 }
 
-// validateFlagGroups validates the mutuallyExclusive/requiredAsGroup logic and returns the
+// ValidateFlagGroups validates the mutuallyExclusive/requiredAsGroup logic and returns the
 // first error encountered.
-func (c *Command) validateFlagGroups() error {
+func (c *Command) ValidateFlagGroups() error {
 	if c.DisableFlagParsing {
 		return nil
 	}
diff --git a/powershell_completions.go b/powershell_completions.go
index 351ded6..004de42 100644
--- a/powershell_completions.go
+++ b/powershell_completions.go
@@ -22,9 +22,15 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"strings"
 )
 
 func genPowerShellComp(buf io.StringWriter, name string, includeDesc bool) {
+	// Variables should not contain a '-' or ':' character
+	nameForVar := name
+	nameForVar = strings.Replace(nameForVar, "-", "_", -1)
+	nameForVar = strings.Replace(nameForVar, ":", "_", -1)
+
 	compCmd := ShellCompRequestCmd
 	if !includeDesc {
 		compCmd = ShellCompNoDescRequestCmd
@@ -41,7 +47,7 @@ filter __%[1]s_escapeStringWithSpecialChars {
 `+"    $_ -replace '\\s|#|@|\\$|;|,|''|\\{|\\}|\\(|\\)|\"|`|\\||<|>|&','`$&'"+`
 }
 
-Register-ArgumentCompleter -CommandName '%[1]s' -ScriptBlock {
+[scriptblock]$__%[2]sCompleterBlock = {
     param(
             $WordToComplete,
             $CommandAst,
@@ -66,17 +72,17 @@ Register-ArgumentCompleter -CommandName '%[1]s' -ScriptBlock {
     }
     __%[1]s_debug "Truncated command: $Command"
 
-    $ShellCompDirectiveError=%[3]d
-    $ShellCompDirectiveNoSpace=%[4]d
-    $ShellCompDirectiveNoFileComp=%[5]d
-    $ShellCompDirectiveFilterFileExt=%[6]d
-    $ShellCompDirectiveFilterDirs=%[7]d
+    $ShellCompDirectiveError=%[4]d
+    $ShellCompDirectiveNoSpace=%[5]d
+    $ShellCompDirectiveNoFileComp=%[6]d
+    $ShellCompDirectiveFilterFileExt=%[7]d
+    $ShellCompDirectiveFilterDirs=%[8]d
 
     # Prepare the command to request completions for the program.
     # Split the command at the first space to separate the program and arguments.
     $Program,$Arguments = $Command.Split(" ",2)
 
-    $RequestComp="$Program %[2]s $Arguments"
+    $RequestComp="$Program %[3]s $Arguments"
     __%[1]s_debug "RequestComp: $RequestComp"
 
     # we cannot use $WordToComplete because it
@@ -106,7 +112,7 @@ Register-ArgumentCompleter -CommandName '%[1]s' -ScriptBlock {
 
     __%[1]s_debug "Calling $RequestComp"
     # First disable ActiveHelp which is not supported for Powershell
-    $env:%[8]s=0
+    $env:%[9]s=0
 
     #call the command store the output in $out and redirect stderr and stdout to null
     # $Out is an array contains each line per element
@@ -257,7 +263,9 @@ Register-ArgumentCompleter -CommandName '%[1]s' -ScriptBlock {
 
     }
 }
-`, name, compCmd,
+
+Register-ArgumentCompleter -CommandName '%[1]s' -ScriptBlock $__%[2]sCompleterBlock
+`, name, nameForVar, compCmd,
 		ShellCompDirectiveError, ShellCompDirectiveNoSpace, ShellCompDirectiveNoFileComp,
 		ShellCompDirectiveFilterFileExt, ShellCompDirectiveFilterDirs, activeHelpEnvVar(name)))
 }
diff --git a/projects_using_cobra.md b/projects_using_cobra.md
index b5b9dbf..6865f88 100644
--- a/projects_using_cobra.md
+++ b/projects_using_cobra.md
@@ -1,5 +1,6 @@
 ## Projects using Cobra
 
+- [Allero](https://github.com/allero-io/allero)
 - [Arduino CLI](https://github.com/arduino/arduino-cli)
 - [Bleve](https://blevesearch.com/)
 - [Cilium](https://cilium.io/)
diff --git a/shell_completions.md b/shell_completions.md
index a0957b4..553ee5d 100644
--- a/shell_completions.md
+++ b/shell_completions.md
@@ -535,6 +535,21 @@ search for a keyword in charts
 $ helm s[tab]
 search  show  status
 ```
+### Aliases
+
+You can also configure `powershell` aliases for your program and they will also support completions.
+
+```
+$ sal aliasname origcommand
+$ Register-ArgumentCompleter -CommandName 'aliasname' -ScriptBlock $__origcommandCompleterBlock
+
+# and now when you run `aliasname` completion will make
+# suggestions as it did for `origcommand`.
+
+$ aliasname <tab>
+completion     firstcommand   secondcommand
+```
+The name of the completer block variable is of the form `$__<programName>CompleterBlock` where every `-` and `:` in the program name have been replaced with `_`, to respect powershell naming syntax.
 
 ### Limitations
 


Test patch:
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 7865add..a55d39f 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -8,6 +8,9 @@ on:
 env:
   GO111MODULE: on
 
+permissions:
+  contents: read
+
 jobs:
 
 
@@ -30,6 +33,9 @@ jobs:
 
 
   golangci-lint:
+    permissions:
+      contents: read  # for actions/checkout to fetch code
+      pull-requests: read  # for golangci/golangci-lint-action to fetch pull requests
     runs-on: ubuntu-latest
     steps:
 
diff --git a/command_test.go b/command_test.go
index a4b7366..b3dd030 100644
--- a/command_test.go
+++ b/command_test.go
@@ -2343,3 +2343,90 @@ func TestSetContextPersistentPreRun(t *testing.T) {
 		t.Error(err)
 	}
 }
+
+const VersionFlag = "--version"
+const HelpFlag = "--help"
+
+func TestNoRootRunCommandExecutedWithVersionSet(t *testing.T) {
+	rootCmd := &Command{Use: "root", Version: "1.0.0", Long: "Long description"}
+	rootCmd.AddCommand(&Command{Use: "child", Run: emptyRun})
+
+	output, err := executeCommand(rootCmd)
+	if err != nil {
+		t.Errorf("Unexpected error: %v", err)
+	}
+
+	checkStringContains(t, output, rootCmd.Long)
+	checkStringContains(t, output, HelpFlag)
+	checkStringContains(t, output, VersionFlag)
+}
+
+func TestNoRootRunCommandExecutedWithoutVersionSet(t *testing.T) {
+	rootCmd := &Command{Use: "root", Long: "Long description"}
+	rootCmd.AddCommand(&Command{Use: "child", Run: emptyRun})
+
+	output, err := executeCommand(rootCmd)
+	if err != nil {
+		t.Errorf("Unexpected error: %v", err)
+	}
+
+	checkStringContains(t, output, rootCmd.Long)
+	checkStringContains(t, output, HelpFlag)
+	checkStringOmits(t, output, VersionFlag)
+}
+
+func TestHelpCommandExecutedWithVersionSet(t *testing.T) {
+	rootCmd := &Command{Use: "root", Version: "1.0.0", Long: "Long description", Run: emptyRun}
+	rootCmd.AddCommand(&Command{Use: "child", Run: emptyRun})
+
+	output, err := executeCommand(rootCmd, "help")
+	if err != nil {
+		t.Errorf("Unexpected error: %v", err)
+	}
+
+	checkStringContains(t, output, rootCmd.Long)
+	checkStringContains(t, output, HelpFlag)
+	checkStringContains(t, output, VersionFlag)
+}
+
+func TestHelpCommandExecutedWithoutVersionSet(t *testing.T) {
+	rootCmd := &Command{Use: "root", Long: "Long description", Run: emptyRun}
+	rootCmd.AddCommand(&Command{Use: "child", Run: emptyRun})
+
+	output, err := executeCommand(rootCmd, "help")
+	if err != nil {
+		t.Errorf("Unexpected error: %v", err)
+	}
+
+	checkStringContains(t, output, rootCmd.Long)
+	checkStringContains(t, output, HelpFlag)
+	checkStringOmits(t, output, VersionFlag)
+}
+
+func TestHelpflagCommandExecutedWithVersionSet(t *testing.T) {
+	rootCmd := &Command{Use: "root", Version: "1.0.0", Long: "Long description", Run: emptyRun}
+	rootCmd.AddCommand(&Command{Use: "child", Run: emptyRun})
+
+	output, err := executeCommand(rootCmd, HelpFlag)
+	if err != nil {
+		t.Errorf("Unexpected error: %v", err)
+	}
+
+	checkStringContains(t, output, rootCmd.Long)
+	checkStringContains(t, output, HelpFlag)
+	checkStringContains(t, output, VersionFlag)
+}
+
+func TestHelpflagCommandExecutedWithoutVersionSet(t *testing.T) {
+	rootCmd := &Command{Use: "root", Long: "Long description", Run: emptyRun}
+	rootCmd.AddCommand(&Command{Use: "child", Run: emptyRun})
+
+	output, err := executeCommand(rootCmd, HelpFlag)
+	if err != nil {
+		t.Errorf("Unexpected error: %v", err)
+	}
+
+	checkStringContains(t, output, rootCmd.Long)
+	checkStringContains(t, output, HelpFlag)
+	checkStringOmits(t, output, VersionFlag)
+}