Allow using stderr for prompt output

[mortenscheel]

Sending the prompt output to stdout (which is the default behavior) can be problematic in situations where the output of the command needs to be redirected to a file, or piped to another command.

• If the output is redirected to a file, e.g. > data.json, the prompt is sent directly to the file and won't be displayed in the console. The command will keep running, waiting for input, but the user won't know what's going on.
• If the output is piped to another command, e.g. | grep "foo", the prompt might also be invisible, or if the receiving command expects structured data like csv or json, it might cause an error.

The fix is easy. Just send the prompt output to stderr in stead. That's what Symfony's QuestionHelper does by default. And according to this excellent post by Chris Fidao, it's actually a well established unix convention.

Like I mentioned in #107, I think the best solution would be to use stderr by default, but I realize that this could be considered a breaking change. So in stead I've just added a simple Prompt::useStderr() method that makes it easy to send all prompts to stderr.

I really wanted to add some tests that assert that actual output is sent to stdout, and prompts are send to stderr, but I honestly have no idea how to do that.

[taylorotwell]

I don't think we should have a method for this. The best practice should be the default.

[taylorotwell]

Waiting for review from @jessarcher

[jessarcher]

I don't think we should have a method for this. The best practice should be the default.

Agreed. Based on the linked post, I think I can agree that using STDERR makes sense for interactive prompts. I'm less certain about where output from functions like info should go. Am I right in assuming that Symfony still uses STDOUT for their equivalent methods, and by extension, so would Laravel's wrappers?

Any changes we make would need to consider the newline behaviour. We output a single blank line between prompts and other informational messages so we need to track the number of trailing newlines previously output so we know how many (if any) need to be output before the next prompt. If output is mixed between STDOUT and STDERR, we'd still need to track the newlines globally between them, but I could see this causing some layout problems if STDOUT was being redirected somewhere else.

[mortenscheel]

Am I right in assuming that Symfony still uses STDOUT for their equivalent methods, and by extension, so would Laravel's wrappers?

Symfony commands don't have convenience methods like info() or error(). You just receive an OutputInterface like this

protected function execute(InputInterface $input, OutputInterface $output): int
{
    $output->writeln('<info>This goes to STDOUT</info>');
    // If you want to output to STDERR, you have to do it manually
    $err = $output instanceof ConsoleOutputInterface ? $output->getErrorOutput() : $output;
    $err->writeln('<error>This goes to STDERR</error>');
    
    return self::SUCCESS;
}

Good point about info(). I personally think that prompt functions that can be considered "output formatter helpers" (info, note, error, table, etc.) should still use STDOUT, because they're used deliberately to produce output. Those that only produce temporary output (spin and table), as well as those that involve user interaction, should use STDERR. You could of course argue that error() messages should go to STDERR, but Artisan uses STDOUT, so it's probably best to stick with that.

If output is mixed between STDOUT and STDERR, we'd still need to track the newlines globally between them, but I could see this causing some layout problems if STDOUT was being redirected somewhere else.

We should be able to detect if STDOUT is being redirected by using this technique.

[jessarcher]

Good point about info(). I personally think that prompt functions that can be considered "output formatter helpers" (info, note, error, table, etc.) should still use STDOUT, because they're used deliberately to produce output. Those that only produce temporary output (spin and table), as well as those that involve user interaction, should use STDERR. You could of course argue that error() messages should go to STDERR, but Artisan uses STDOUT, so it's probably best to stick with that.

Makes sense, although it could be a pain if you want the command to output well-formed JSON that can be redirected to STDOUT, but still want to show some informational messages that don't get redirected.

We should be able to detect if STDOUT is being redirected by using this technique.

Currently, we only check whether STDIN is a TTY (or whether the --non-interactive option was passed to a Laravel command) to determine whether to prompt interactively.

What do you think about the following additional checks?

• If STDOUT is a TTY, then continue as usual.
• Otherwise, if STDERR is a TTY, use that for interactive output (or perhaps all output).
• If neither is a TTY, then disable prompt interactivity.

That should make things "just work" as expected for most scenarios.

[mortenscheel]

• If STDOUT is a TTY, then continue as usual.
• Otherwise, if STDERR is a TTY, use that for interactive output (or perhaps all output).
• If neither is a TTY, then disable prompt interactivity.

Excellent solution 👍 I think I'll have time to implement it this week.

[mortenscheel]

Looking forward to hearing your feedback.
Here's an easy way to test it:

php playground/index.php > out.txt

The prompt should work as always, and out.txt should only contain the final var_dump() with the selected values.

[jessarcher]

Hey @mortenscheel, that seems to work really well!

The only problem is that when Prompts is used with Laravel, we configure it to use Laravel's OutputStyle instance, instead of a ConsoleOutput instance from this repo. The ConsoleOutput class is only used when Prompts is used outside of Laravel.

See https://github.com/laravel/framework/blob/eb66928b4aaa3fa84bdd95db44d27fc7e0977563/src/Illuminate/Console/Concerns/ConfiguresPrompts.php#L27

This is so we can keep track of newlines output from both Prompts and Laravel to ensure the correct spacing between output from each source.

Rather than duplicate the logic in Laravel, can we move it within the Prompt class so it works regardless of the OutputInterface being used? Presumably, it will only be able to use STDERR if the output is an instance of ConsoleOutputInterface as it defines the getErrorOutput method.

[mortenscheel]

Rather than duplicate the logic in Laravel, can we move it within the Prompt class so it works regardless of the OutputInterface being used?

Sure. @jessarcher just to make sure I understand correctly, you're suggesting that the different prompts should call static::write() (where the counting happens before sending the message to an OutputInterface) in stead of static::output()->write()?

[mortenscheel]

@jessarcher please take a look at the implementation.
I've refactored the newline capturing to a new PromptWriter class, which handles the capturing regardless of which OutputInterface is being used.

I was a bit confused by the fact that Prompt sometimes rendered without "synchronizing" the number of captured newlines from ConsoleOutput. So when I tried using a single newLinesWritten for everything, the prompt started jumping around quite a bit. I just wanted to mention it, in case it isn't intentional. Let me know if you want an example.

[jessarcher]

Hi @mortenscheel,

I've just tested this out, and there's an issue with the newlines between Prompt output and Laravel output.

On the left is the current behaviour, and on the right is with this PR. Note the extra newlines between the Prompts output and the other output from Laravel. This is without redirecting any streams.

I'll need to do more testing to figure out what's going on, but I suspect it's to do with the two places where the $newLinesWritten is now monitored. In the PromptWriter class and in Laravel's OutputStyle class. Previously, it would only be monitored by whichever output was being used, ConsoleOutput or OutputStyle (but never both).

Additionally, STDERR doesn't seem to be used when STDOUT is redirected when Prompts are used in Laravel. Only when used standalone.

[jessarcher]

Laravel sends an instance of \Illuminate\Console\OutputStyle, which doesn't implement ConsoleOutputInterface, meaning we won't use STDERR with Laravel even when STDOUT is redirected elsewhere.

Laravel's OutputStyle class is an extension of Symfony's SymfonyStyle and OutputStyle classes, which are a wrapper around an underlying OutputInterface. It doesn't expose the getErrorOutput method but instead exposes a getErrorStyle method. Unfortunately, this returns a new instance of SymfonyStyle rather than Laravel's OutputStyle class (due to their use of self).

Even if we could get it to return a new instance of Laravel's OutputStyle, the newline tracking would break because Laravel will still be using its original OutputStyle instance rather than the new instance we'd be using in Prompts.

It's a complicated problem because we need to keep track of the newlines between Prompts's and Laravel's output. Both Laravel and Prompts need to know how many trailing newlines were in the previous output, regardless of where that previous output came from, so that they each know how many newlines to emit before any new output. If Laravel isn't aware of the trailing newlines that Prompts has written, it won't be able to output the correct amount of leading newlines before any new output, and vice versa. It gets more complicated with Symfony because they have a separate instance for STDERR. If Laravel and Prompts use different instances, they won't be aware of each other's trailing newlines.

The only solutions I can think of are:

1. Add logic to Laravel to make it use an STDERR output instance when appropriate (and share that instance with Prompts). This could have unintended consequences in user commands that expect to output on STDOUT regardless of any redirection, so it's probably a breaking change. Alternatively,
2. Add a method to Laravel's OutputStyle class (or create a new wrapper class) that allows us to write to STDERR, while still tracking trailing newlines regardless of the stream. It would potentially need to be smart enough to track newlines separately when streams are redirected to different places because, in that scenario, the trailing newlines from the previous output of another stream shouldn't be considered. Prompts could then choose to output to STDERR and both Laravel and Prompts would still know how many leading newlines they needed to output.

Either way it's a big change covering two code bases. I still see the value in doing this, but I'm concerned about the effort and risk involved.

[mortenscheel]

Thanks for the detailed explanation. I can't think of any other solutions, so you're welcome to close this if you want.
I learned a lot about console output, and it was nice to meet you guys 😄

[jessarcher]

Thanks, @mortenscheel! Nice to meet you as well 🙂

Happy to reconsider this at some point if we can figure out a good solution. Having one blank line between output looks great, but it certainly adds a lot of challenges 😅