Unclear how "Out" and "Err" streams should be used

[mislav]

Consider the following program:

c := cobra.Command{...}
c.SetOut(os.Stdout)
c.SetErr(os.Stderr)

Expected behavior: Cobra prints help text, version information, and shell completion to Stdout; and error messages (e.g. "unknown command") and warning messages (e.g. using deprecated commands or flags) are printed to Stderr.

Actual behavior: Cobra prints some of its error messages and warning messages (e.g. deprecation notices) to Stdout.

I might have misunderstood the semantics of Cobra's "Out" stream. Based on the name alone, I thought it was for setting an equivalent of a "standard output" stream, but the method documentation says: SetOut sets the destination for usage messages. In Cobra, "usage messages" are help text that is rendered when the user has specified a wrong command name, used the wrong flag, or passed an invalid flag value. However, Cobra prints much more than just usage messages to the "Out" stream; see below.

Usage messages are always printed as a result of an error, and thus should be (as is my opinion) printed to an error stream. However, in Cobra that is only true until the developer explicitly invokes command.SetOut() in their app, after which usage messages and deprecation warnings are printed to the "Out" stream, which is the same stream where Cobra prints help text, version information, and completion scripts. Thus, Cobra blends the output of successful commands with the output of error messages in the same stream.

To resolve this issue, Cobra could change the logic of where usage messages and deprecation warnings are printed, but this could be backwards-incompatible for apps that already rely on this behavior. At minimum, if Cobra wanted to avoid making any functional changes, the developers could expand the documentation to clarify:

• It is not clear to me what the semantics of command.Print*() set of functions are and when should they be used. Should app developers use those functions in their command implementations to print to stdout? If so, why do they print to Stderr as default? If not, why are they exported functions?

• Is is not clear to me what the semantics of command.OutOrStderr() function are, which is currently the destination that command.Print*() are printing to. The documentation says "OutOrStderr returns output to stderr". However, if the "Out" writer was explicitly specified, then it will return that writer. With that in mind, when should app developers be using this method?

Ref. https://github.com/spf13/cobra/pull/822

P.S. Here is the breakdown of the current printing logic in the latest master. Notice that the current default destinations and their fallbacks are of mismatched natures:

functionality	cur. destination	fallback	ideal destination (according to OP)
--help	Out	Stdout	Out
--version	Out	Stdout	Out
completion <shell>	Out	Stdout	Out
__complete <args>	Out	Stdout	Out
UsageFunc	Out	Stderr	Err
deprecated command	Out	Stderr	Err
deprecated flag	Out	Stderr	Err
unknown help topic	Out	Stderr	Err
--help flag errors	Out	Stderr	Err
--version flag errors	Out	Stderr	Err
DebugFlags	Out	Stderr	Err
UsageFunc errors	Err	Stderr	Err
HelpFunc errors	Err	Stderr	Err
command Traverse errors	Err	Stderr	Err
command execute errors	Err	Stderr	Err

[github-actions[bot]]

The Cobra project currently lacks enough contributors to adequately respond to all issues. 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 issue is closed. You can:
• Make a comment to remove the stale label and show your support. The 60 days reset. - If an issue has lifecycle/rotten and is closed, comment and ask maintainers if they'd be interseted in reopening

[jpmcb]

This probably shouldn't have been closed. We recently removed the stalebot since it was creating alot of noise.

[tcarreira]

I'm interested as well. I support implementing non-breaking changes, and I believe it can be accomplished if the maintainers give their approval. In line with what @mislav already stated on the recently closed issue: https://github.com/spf13/cobra/pull/1709#issuecomment-1137358318

I'm writing this comment because there hasn't been any discussion here, for more than 1y. @jpmcb, what are your thoughts on this? Can someone implement PR with a non-breaking change with new functionality to address this issue?

[jpmcb]

If this can be done with a non-breaking change, then yes we should go with that implementation. The challenge with most things Cobra is that given its massive adoption and long standing APIs, these things stop being bug fixes and start becoming breaking changes.

You'd be surprised by the things people have built off of cobra's little nuances and I typically strongly discourage we break existing behaviors.

But I'm not sure how this can be implemented without sweeping breaking changes.

[tcarreira]

Hey @jpmcb There is a PR for fixing this, with no breaking changes. Is it OK, or do you think I can improve it?

Tests passed, but I didn't add new tests. I could try and add tests for the table @mislav wrote above Let me know how I can help

[ragaskar]

Just got bit by this problem, would love to see the PR merged (but our CLI is new so we don't have breaking changes concerns).