From 90548c591d9f2662c2db303b4cb09feaab33a02d Mon Sep 17 00:00:00 2001 From: Marc Khouzam Date: Thu, 9 Jan 2020 20:28:46 -0500 Subject: [PATCH] feat(comp): Don't use error codes for completion To use error codes to indicate completion directive to the completion script had us use os.Exit() in the __complete command. This prevented go tests calling the __complete command from succeeding. Another option was to return an error containing an error code (like is done for helm plugins) instead of calling os.Exit(). However such an approach requires a change in how helm handles the returned error of a Cobra command; although we can do this for Helm, it would be an annoying requirement for other programs if we ever push this completion logic into Cobra. The chosen solution instead is to printout the directive at the end of the list of completions, and have the completion script extract it. Note that we print both the completions and directive to stdout. It would have been interesting to print the completions to stdout and the directive to stderr; however, it is very complicated for the completion script to extract both stdout and stderr to different variables, and even if possible, such code would not be portable to many shells. Printing both to stdout is much simpler. Signed-off-by: Marc Khouzam --- cmd/helm/root.go | 14 ++++++++++++- internal/completion/complete.go | 35 +++++++++++++++++++++------------ 2 files changed, 35 insertions(+), 14 deletions(-) diff --git a/cmd/helm/root.go b/cmd/helm/root.go index ab7b01c90..bf98e72fe 100644 --- a/cmd/helm/root.go +++ b/cmd/helm/root.go @@ -65,18 +65,30 @@ __helm_custom_func() __helm_debug "${FUNCNAME[0]}: calling ${requestComp}" # Use eval to handle any environment variables and such out=$(eval ${requestComp} 2>/dev/null) - directive=$? + + # Extract the directive int at the very end of the output following a : + directive=${out##*:} + # Remove the directive + out=${out%%:*} + if [ "${directive}" = "${out}" ]; then + # There is not directive specified + directive=0 + fi + __helm_debug "${FUNCNAME[0]}: the completion directive is: ${directive}" + __helm_debug "${FUNCNAME[0]}: the completions are: ${out[*]}" if [ $((${directive} & %[2]d)) -ne 0 ]; then __helm_debug "${FUNCNAME[0]}: received error, completion failed" else if [ $((${directive} & %[3]d)) -ne 0 ]; then if [[ $(type -t compopt) = "builtin" ]]; then + __helm_debug "${FUNCNAME[0]}: activating no space" compopt -o nospace fi fi if [ $((${directive} & %[4]d)) -ne 0 ]; then if [[ $(type -t compopt) = "builtin" ]]; then + __helm_debug "${FUNCNAME[0]}: activating no file completion" compopt +o default fi fi diff --git a/internal/completion/complete.go b/internal/completion/complete.go index c0fabda1b..d13c7f5bd 100644 --- a/internal/completion/complete.go +++ b/internal/completion/complete.go @@ -16,6 +16,7 @@ limitations under the License. package completion import ( + "errors" "fmt" "log" "os" @@ -124,13 +125,17 @@ func NewCompleteCmd(settings *cli.EnvSettings) *cobra.Command { Run: func(cmd *cobra.Command, args []string) { CompDebugln(fmt.Sprintf("%s was called with args %v", cmd.Name(), args)) - flag, trimmedArgs, toComplete := checkIfFlagCompletion(cmd.Root(), args[:len(args)-1], args[len(args)-1]) - + flag, trimmedArgs, toComplete, err := checkIfFlagCompletion(cmd.Root(), args[:len(args)-1], args[len(args)-1]) + if err != nil { + // Error while attempting to parse flags + CompErrorln(err.Error()) + return + } // Find the real command for which completion must be performed finalCmd, finalArgs, err := cmd.Root().Find(trimmedArgs) if err != nil { // Unable to find the real command. E.g., helm invalidCmd - os.Exit(int(BashCompDirectiveError)) + return } CompDebugln(fmt.Sprintf("Found final command '%s', with finalArgs %v", finalCmd.Name(), finalArgs)) @@ -167,10 +172,15 @@ func NewCompleteCmd(settings *cli.EnvSettings) *cobra.Command { fmt.Println(comp) } - // Print some helpful info to stderr for the user to see. + // As the last printout, print the completion directive for the + // completion script to parse. + // The directive integer must be that last character following a single : + // The completion script expects :directive + fmt.Printf("\n:%d\n", directive) + + // Print some helpful info to stderr for the user to understand. // Output from stderr should be ignored from the completion script. fmt.Fprintf(os.Stderr, "Completion ended with directive: %s\n", directive.string()) - os.Exit(int(directive)) }, } } @@ -179,7 +189,7 @@ func isFlag(arg string) bool { return len(arg) > 0 && arg[0] == '-' } -func checkIfFlagCompletion(rootCmd *cobra.Command, args []string, lastArg string) (*pflag.Flag, []string, string) { +func checkIfFlagCompletion(rootCmd *cobra.Command, args []string, lastArg string) (*pflag.Flag, []string, string, error) { var flagName string trimmedArgs := args flagWithEqual := false @@ -189,8 +199,7 @@ func checkIfFlagCompletion(rootCmd *cobra.Command, args []string, lastArg string lastArg = lastArg[index+1:] flagWithEqual = true } else { - CompErrorln("Unexpected completion request for flag") - os.Exit(int(BashCompDirectiveError)) + return nil, nil, "", errors.New("Unexpected completion request for flag") } } @@ -212,14 +221,14 @@ func checkIfFlagCompletion(rootCmd *cobra.Command, args []string, lastArg string if len(flagName) == 0 { // Not doing flag completion - return nil, trimmedArgs, lastArg + return nil, trimmedArgs, lastArg, nil } // Find the real command for which completion must be performed finalCmd, _, err := rootCmd.Find(trimmedArgs) if err != nil { // Unable to find the real command. E.g., helm invalidCmd - os.Exit(int(BashCompDirectiveError)) + return nil, nil, "", errors.New("Unable to find final command for completion") } CompDebugln(fmt.Sprintf("checkIfFlagCompletion: found final command '%s'", finalCmd.Name())) @@ -227,8 +236,8 @@ func checkIfFlagCompletion(rootCmd *cobra.Command, args []string, lastArg string flag := findFlag(finalCmd, flagName) if flag == nil { // Flag not supported by this command, nothing to complete - CompDebugln(fmt.Sprintf("Subcommand '%s' does not support flag '%s'", finalCmd.Name(), flagName)) - os.Exit(int(BashCompDirectiveNoFileComp)) + err = fmt.Errorf("Subcommand '%s' does not support flag '%s'", finalCmd.Name(), flagName) + return nil, nil, "", err } if !flagWithEqual { @@ -241,7 +250,7 @@ func checkIfFlagCompletion(rootCmd *cobra.Command, args []string, lastArg string } } - return flag, trimmedArgs, lastArg + return flag, trimmedArgs, lastArg, nil } func findFlag(cmd *cobra.Command, name string) *pflag.Flag {