[Feature]: Error output should be be written to stderr

[jennings]

Description

I use CSharpier as a jj fix tool, which means the current file is piped to CSharpier over stdin and the formatted result read from stdout.

If a file is malformed XML, CSharpier writes the following to stdout and CSharpier exits with code 0, causing jj to replace the entire file contents with this warning:

Warning <path> - Appeared to be invalid xml so was not formatted.

I'm happy to submit a PR to change this, I just want to make sure which of these cases would be welcome changes:

• (Required for my use case) Exit with a non-zero exit code. I would just pick code 1 unless you want a different value (I don't think the CLI has a convention for this yet?)
• (Optional but I strongly recommend) Print errors and warnings to stderr, not stdout.
• (Not required for my particular use case, but I noticed this while testing this changed behavior) Write the "Formatted N files in Xms." message to stderr as well.

[jennings]

Hm, I was going to fix this by making this change:

-fileIssueLogger.WriteWarning(codeFormattingResult.WarningMessage);
+fileIssueLogger.WriteError(codeFormattingResult.WarningMessage);
+Interlocked.Increment(ref commandLineFormatterResult.FailedCompilation);

But then I found the level checks in ConsoleLogger which writes errors and greater to stderr, and anything else to stdout.

My expectation was that CSharpier would use stderr and stdout in the Unix tradition:

• The data you're running the program for goes to stdout (in this case, the formatted file contents)
• All diagnostic information goes to stderr, regardless of "severity".

This works well because it means if you create a pipeline like echo "$SOME_CODE" | csharpier format --stdin-path Foobar.csproj | some_other_program, diagnostics always end up in the terminal and never get passed to some_other_program.

Would changing CSharpier to use stderr and stdout this way be welcome? Edit: I opened #1852 for this, if it's unwelcome it can just be closed, but I would be sad.

[belav]

For a reason that I can't currently remember, I decided that invalid xml should be a warning while invalid csharp is an error. I think it was because there are some file extensions like .config that are xml in some codebases but not in others.

I think changing that, at least for when piping content to std-in, would solve your issue without changing the behavior for anyone else. I'm worried logging warnings to stderr would be a breaking change.

I realized there is also a flag for how to deal with compilation errors on c# files. I don't know if it makes sense to make that work for xml as well - --compilation-errors-as-warnings.

Perhaps invalid xml should be treated the same as invalid c# for all cases, and that flag should apply to both. If a repo has a mix of .config files that are xml and non-xml it wouldn't be difficult to add the non-xml ones to an ignore file.

[jennings]

Anything that makes the stdin use case exit with a nonzero error code works for me, I agree.

I'm worried logging warnings to stderr would be a breaking change.

I suppose it's a breaking change when formatting in-place because somebody might have a script that does csharpier format ... > format-log.txt.

But when piping to stdin, it's probably okay to change because I hope nobody is writing scripts that expect stdout to contain either the formatted file or log messages.

Maybe it's okay to change now when piping to stdin, then potentially change for other use cases in a hypothetical 2.0? Or, --log-stderr that changes it now for all use cases, then make it default for 2.0?