RegisterFlagCompletionFunc writes what looks like debug info to screen

[ArnoSen]

Hi,

Great product you are making!

I have been using Cobra for a few years now and I wanted to try the flagcompletion.
I put this into a command for testing:

        createObjectCmd.RegisterFlagCompletionFunc(FlagOutput, func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
                return []string{"json", "table", "yaml"}, cobra.ShellCompDirectiveDefault
        }) 

When running the command and providing the flag and [tab][tab], I get:

mycli noun --output 
:0          Completion  directive   ended       json        table       with        yaml

I am using Cobra version 1.1.1.

Is there anything I am doing wrong here or how can I prevent the ':0 Completion directive ended' be printed on screen?

[Luap99]

Do you use fish? This sound like a dup of #1279

[ArnoSen]

Thanks for the hint!
I use bash but let me check if the cause is the same.

[ArnoSen]

I reviewed my code but there is no newline written.
I did find out that removing MyCmd.SetOutput(os.Stdout) suppressed the message :0 Completion directive ended being written to the console.

[Luap99]

Thanks for checking.
The completion logic has to write the debug output to stderr.

I looked at the code and we use cmd.OutOrStdout() and cmd.ErrOrStderr()
I guess the problem is that you write stderr to stdout.

cobra/custom_completions.go

Lines 156 to 163 in 08c51e5

	// 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 colon (:).
	// The completion script expects :<directive>
	fmt.Fprintf(finalCmd.OutOrStdout(), ":%d\n", directive)
	// Print some helpful info to stderr for the user to understand.
	// Output from stderr must be ignored by the completion script.
	fmt.Fprintf(finalCmd.ErrOrStderr(), "Completion ended with directive: %s\n", directive.string())

@marckhouzam Should this be hard coded to stdout and stderr?

[marckhouzam]

Interesting.

cmd.SetOutput() is deprecated and has now been split in cmd.SetOut() and cmd.SetErr(). So having both outputs on the same stream may have been an unintended situation for @ArnoSen.

But it does bring up the issue that someone may want to have stderr be directed to stdout, which would break completion.

@Luap99 Forcing the completion "debug" printouts to stderr would reduce testing options, I believe. For example. Helm has go tests which check that the "ShellDirectiveNoFileComp" string is part of the output of the __complete command. Helm could instead check the ":4" part of the output, but it would have to do modulo operations first to extract the right bit in the directive. This is not difficult, but I felt it was less readable, so looking for "ShellDirectiveNoFileComp" seemed like a good idea at the time.

Also, allowing to redirect "debug" output does give more testing options (like adding extra debug printouts to ValidArgsFunction code and checking that output in the go tests). But as I'm writing this, I'm realizing the code won't allow this right now, as we force CompDebug() and CompError() to os.Stderr:

cobra/custom_completions.go

Line 536 in 08c51e5

fmt.Fprintf(os.Stderr, msg)

Still, maybe we should only change the stream for the "debug" output if we see the stream is the same for out and err? Or we could just not print that line in that particular case.

What do you think?

[Luap99]

If you think that this is required for testing options we should keep it.
I feel like this a rather uncommon situation where someone redirects stderr to stdout. Not printing the line in this case sounds like the best solution to me.

[github-actions]

This issue is being marked as stale due to a long period of inactivity

[marckhouzam]

We need to fix this so let's keep it open.