When using in pipeline don't print info to stderr #1957
Conversation
When receiving data via stdIn and printing to stdout don't print info messages to stderr. Stderr output might be used to detect failure, info messages (like "Reformatting..."). The alternative is to use --quiet which also removes valid error messages.
| @@ -54,7 +54,7 @@ object CliOptions { | |||
| out = | |||
| guardPrintStream(parsed.quiet && !parsed.stdIn)(parsed.common.out), | |||
| info = guardPrintStream( | |||
| parsed.quiet || parsed.writeMode == WriteMode.List | |||
| parsed.stdIn || parsed.quiet || parsed.writeMode == WriteMode.List | |||
There was a problem hiding this comment.
is it important that one is reading from stdin? what if you are reading from a file but writing to stdout (via WriteMode.StdOut?)
There was a problem hiding this comment.
Good point, as far as I understand it if I consume from stdin the result will always be written to stdout which is problematic when writing info to stderr as it can break pipelines, but the same is true when consuming from a file and writing to stdout. I'll change this.
writing to stdout is either because the input is via stdin, or the output is set to stdout.
| @@ -54,7 +54,7 @@ object CliOptions { | |||
| out = | |||
| guardPrintStream(parsed.quiet && !parsed.stdIn)(parsed.common.out), | |||
| info = guardPrintStream( | |||
| parsed.quiet || parsed.writeMode == WriteMode.List | |||
| parsed.stdIn || parsed.writeMode == WriteMode.Stdout || parsed.quiet || parsed.writeMode == WriteMode.List | |||
There was a problem hiding this comment.
sorry to repeat myself... why do we need to check for stdIn? what if stdIn is used with WriteMode.Test, wouldn't it make sense to continue writing out info messages?
There was a problem hiding this comment.
I see what you mean now, sorry about not being clear here. As far as I understand the behaviour: If —stdin is passed the result will be written to stdout, without also passing —stdout. Given that the behaviour I would expect is to have info not being written to stderr in this case, as it causes the mentioned issue.
Tbh I was not aware of using test mode with —stdin, so would the recommendation be to use —stdin and —stdout to get the desired bahaviour for my case?
To be honest I’m getting the feeling this redirecting depending on the situation is becoming a little complex to follow, and maybe the clearer approach would be to offer a —no-info flag to explicitly disable info instead of trying to infer it from the various modes. What do you think?
There was a problem hiding this comment.
To be honest I’m getting the feeling this redirecting depending on the situation is becoming a little complex to follow, and maybe the clearer approach would be to offer a
—no-infoflag to explicitly disable info instead of trying to infer it from the various modes. What do you think?
I think it might be a good analog to the --no-stderr flag. If not, I'd just check for WriteMode.StdOut as sufficient, and remove stdIn check as redundant.
There was a problem hiding this comment.
--stdin does not set WriteMode.Stdout it is by default set to WriteMode.Override, even so --stdin causes the output to be written to stdout. To get consistent behaviour either --stdin would need to set WriteMode.Stdout, but this would make it order dependent in case someone also sets --test what the actual WriteMode ends up being.
I'm not quite sure what the "correct" behaviour would be as far as I can see it:
--stdinsetsWriteMode.Stdout--stdinrequires either the--testor--stdoutflags to be set- Keep as is with the default
--stdincausingWriteMode.Override, but introduce--no-infoflag
I'm not quite sure what makes most sense for this. I feel like --stdin with WriteMode.Override being set by default somehow seems wrong, but I'm not sure.
There was a problem hiding this comment.
if that's correct, then we are overthinking it, and cli opt parsing should change.
When receiving data via stdIn and printing to stdout don't print info messages to stderr. Stderr output might be used to detect failure, info messages (like "Reformatting..."). The alternative is to use --quiet which also removes valid error messages.
The use-case here is the integration into emacs format-all plugin, which is currently using
--quietto silence errors. Ideally it should report the errors on failure, but not show info output on every format.