fix: stderr/stdout handling when custom err/out is defined (SetErrFallback)

[tcarreira]

Fix: Allows capturing cmd output and errors
The current behavior of OutOrStderr() is inconsistent. This PR introduces a feature to achieve the desired behavior without causing any breaking changes.

---

If I understood correctly, the problem is caused by using OutOrStderr(), which uses out but falls back to err.
If you don't use the new SetOutFallback / SetErrFallback, the behavior remains the same as before (no breaking changes)
However, by replacing SetErr with SetErrFallback, we can achieve the desired behavior.

I'm not sure if SetOutFallback should be included. I added it for consistency, but it is not necessary in this context.
@mislav, please let me know what you think, as you were on the frontline with this one.

---

Closes: #1708

[CLAassistant]

All committers have signed the CLA.

[tcarreira]

Hi @jpmcb
Let me know if this PR solves the #1708 correctly ;)

[mislav]

I think this is a move in a correct direction, but I am not sure about the naming of the new APIs and the current behavior if both SetOut and SetOutFallback (for example) are used at the same time.

The crux of the original issue—as I see it—was that the outcomes of SetOut and SetErr were 1) poorly documented, and 2) surprising. If this PR was meant to address that, then the new APIs should have directly improved on both these issues. As it currently stands, I find the naming of the new APIs (e.g. SetErrFallback) a bit confusing, and their documentation is not any better than of the old APIs.

The names SetOutFallback and SetErrFallback indicate that they set a "fallback" of sorts, but that doesn't make sense to me. The purpose of these methods is to override output & error streams from os.Stdout & os.Stderr to custom writers defined by the user. So they are not setting a "fallback", they are directly defining the streams that should be written to. Since the names SetOut and SetErr are already taken (by now deprecated methods), why not name the new methods SetOutStream and SetErrStream? Then, their documentation could be more expressive and detail what kind of content will actually be written to these streams, and what is their default value.

Most of all, I find this documentation confusing:

SetOutFallback sets the destination for usage messages when SetOut() was not used.

This line implies that SetOutFallback is an API secondary to SetOut. However, since SetOut is now deprecated, SetOutFallback is now the primary API, and SetOut should not be used nor referenced from documentation of non-deprecated methods.

If both the new APIs and the old APIs are used at the same time (e.g. if SetOut is used along with SetOutFallback), the behavior of the new APIs should "win" over the old, deprecated ones. Right now it's the other way around.

[tcarreira]

Thanks for your input @mislav, I find all your comments very helpful.
I've made some changes, on multiple commits for easier review:

• Indeed, SetOutFallback and SetErrFallback were not the right ones, I didn't think of anything better. But SetOutStream and SetErrStream feel like perfect to me, thanks.

• I also prefer your suggestion about making the new API having priority over the old one. Still not a breaking change, until someone "opts in" on the new API.

• I added more documentation on SetOutStream and SetErrStream .

• Renamed internal variables c.outWritter and c.errWritter to c.legacyOutWriter and c.legacyErrWriter.

• For better clarity when using Print(), I deprecated it in favor of PrintErr or PrintOut flavors.

Please, let me know what you think and what I can improve.