Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upGitHub is where the world builds software
Millions of developers and companies build, ship, and maintain their software on GitHub — the largest and most advanced development platform in the world.
Built in Help and Version output goes to stderr not stdout. #399
Comments
|
That's an excellent point. |
|
Is there a drawback of directing the version and help screen to the standard error (stderr)
Summary |
|
@moh-hassan Would you say the 'fix' in #423 is unnecessary then, or does it still have merit as a way to help developers distinguish between explicit version/help errors and parser errors when implementing a custom help scenario? The Then, if a developer wants to treat the results of explicit help/--help/version/--version calls as actual output that should go to settings = new HelpWriter(Console.Out, Console.Error) |
|
I dont think it's reasonable that printing out version or help output when specifically requested is counted as an error. If help output is triggered by invalid parameters then absolutely. The GNU guidelines say the same. Azure DevOps pipelines count any write to stderr as a pipeline error, because why wouldnt it - it's specifically signalling an error! When I do an automated app installation I make it print the version output as a quick check for first run but using this library I have to redirect stderr to stdout, which means I can't possibly verify whether it worked or whether it legitimately errored. Printing information to a screen is only one of the uses for standard streams, there are other use-cases for applications other than running them manually in a terminal. It makes sense to respect the standards and behave like other applications - i.e. only using stderr for printing errors. |
|
Sure and agree with you @twaalewijn and @oliverholliday that the wrong thing is to print version information to stderr, BUT, I concentrate on the side effect of the change and the gain we can get.
and their resolution: "Wont Fix"
From POSIX specifications for the standard streams:
In other words, errors, diagnostic information, and anything that falls into diagnostic category goes into stderr and version can be considered as a diagnostic output (not true payload output).
for example: when you pipeline the output of command1 to command2
you don't want the version message going to stdout, because it's now unexpected output that should not be processed by command2, so version can be in stderr.
|
|
I don't believe that specifically requesting help output or a version string can be considered diagnostic output - in that case it is the payload. In all other cases I agree with you. Given that it is a bug is it not worth fixing it and then documenting how to return the behaviour to how it is now should anybody require it. |
|
@twaalewijn, |
|
@moh-hassan PR contextI'll start with some context and the assumption that the current behavior is considered a bug because that is how the PR is implemented right now. In the PR I've replaced the HelpWriter property's TextWriter type with a new class called HelpWriter. This HelpWriter class takes either one or two TextWriters to write help to. // All help written to Console.Error.
new HelpWriter(Console.Error);
// All help written to Console.Out.
new HelpWriter(Console.Out);
// help/version/--help/--version written to Console.Out and other errors to Console.Error.
new HelpWriter(Console.Out, Console.Error);Next the // HelpWriter.Default currently lazily returns a new HelpWriter(Console.Out, Console.Error).
private static readonly Lazy<Parser> DefaultParser = new Lazy<Parser>(
() => new Parser(new ParserSettings { HelpWriter = HelpWriter.Default }));So the new standard would be that help/version/--help/--version results are written to Console.Out and other parsing errors still get written to Console.Error. This is, as you rightfully pointed out, a breaking change for anyone using the default parser and expecting all Current PR behaviorA developer that that wants to disable all help/error text can still continue using the old way to do so: // Setting the HelpWriter to null still prevents any help from getting written
// so no change in this situation.
// This also means that developers who write their own completely custom help/error texts
// are probably unaffected since they would have had to disable auto help this way anyway.
Parser parser = new Parser(settings => settings.HelpWriter = null);A developer that was intercepting auto generated help will probably have assigned the HelpWriter on ParserSettings to their own TextWriter instance. StringWriter myStringWriter = new StringWriter();
// Developers assign their own TextWriter to manage auto written help themselves.
Parser parser = new Parser(settings => settings.HelpWriter = myStringWriter);These developers will find out that their code no longer compiles after updating the package. If they want to keep managing their help this way without any changes they can simply instantiate a HelpWriter with their current TextWriter. StringWriter myStringWriter = new StringWriter();
// Developer still wants all help to be written to their own own TextWriter.
Parser parser = new Parser(settings => settings.HelpWriter = new HelpWriter(myStringWriter));If they want to keep managing the help but differentiate between help/version output they can filter both streams by adding another TextWriter. StringWriter helpVersionStringWriter = new StringWriter();
StringWriter errorStringWriter = new StringWriter();
// Developer wants to do something with the auto generated help
// but filter between help/version verb/option output and parsing error output.
Parser parser = new Parser(
settings => settings.HelpWriter = new HelpWriter(helpVersionStringWriter, errorStringWriter));Breaking change workaroundNow, if it were to be decided that this issue is in fact not a bug but something that was done by design and changing it would cause a breaking change that cannot be accepted without bumping the major version number to a 3, I could change the PR with what I suggested in my earlier comment. This would stop the breaking change from occurring because the HelpWriter.Default that is used by the default Parser would still write everything to Console.Error: // HelpWriter.Default would lazily return new HelpWriter(Console.Error).
// No effective behavior changes for developers that only use the default parser.
private static readonly Lazy<Parser> DefaultParser = new Lazy<Parser>(
() => new Parser(new ParserSettings { HelpWriter = HelpWriter.Default }));As a result, myself and others like @oliverholliday who want to count output that was triggered by calling the help/version verbs or --help/--version options specifically as output of a command (and thus something that needs to be written to Console.Out), would still be able to easily do so by assigning a HelpWriter that does this to the ParserSettings: Parser parser = new Parser(
settings => settings.HelpWriter = new HelpWriter(Console.Out, Console.Error));I'd still prefer the behavior of how the PR is implemented now because personally I consider it a bug that specific calls to the help/version verbs and options were ever written to Console.Error. Having said that I also agree that potentially breaking scripts that were counting on everything being written to Console.Error without proper warning could be undesirable. So I could live with setting I think this covers all the use cases but if it did not and you have more questions please just ask. |
|
@twaalewijn I think starting with the default (directing version/help to stderror) is wisdom to start without breaking change. Just a suggestion:
The ParserSettings can be like:
So, developers can use the new feature like
You control the output stream based on the In this way the transition to the new feature is smooth and the new feature go side by side to the current behavior. |
|
@moh-hassan Before I implement it in the PR though could you clarify question I have about your example? If the HelpWriter property is marked as Obsolete, what would be the migration path for developers currently intercepting the written help text with their own TextWriter? A consumer of this pattern is the unit test project because it tests the written auto help this way. |
For the developers who are using custom Parser and configure
They will get a compilation Warning CS0672
So, they may start modifying their code to use the new HelpWriter as described in wiki The Migration path: The PR when merged,I expect, the old and new helpwriter are available so no breaking change occur in their code or in current unit test. The entry of auto help is this method:
Which you modified to other (overloading) method:
These two methods can go side by side and help can be generated using the old or new helpwriter class and current /new unit test pass. In a major release it will be only the new helpwriter. If you have a different opinion we can discuss it and agree together to the best migration approach. Also all developers are welcome to share their opinion in the quickest and safe migration path to use the new helpwriter. |
This is a bug in the Azure DevOps pipeline, because stderr is often used for other things besides errors. Many programs use stderr to print progress messages, with stdout being reserved for the output that might be passed down a pipe. So Azure DevOps is doing the wrong thing here. Though CommandLineParser's current behavior (printing help to stderr by default) is also wrong and needs to default to stdout in version 3. |
The built in --help and --version outputs are printed to stderr instead of stdout, you have to redirect them to use the commands properly.
test --versionreturns the version number, but on stderr not stdout.e.g.
test --version 2>nulreturns no output.test --version 2>&1prints the version to stdout as expected.