[FrameworkBundle] Make use of stderr for non reliable output #20632

chalasr · 2016-11-25T11:00:36Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	n/a
License	MIT
Doc PR	n/a

Built-in commands should make use of the proper outputs.
As a feature on master, considering that people may actually rely on stdout and the fact commands have been changed a lot since 2.7, I think it's not worth doing this change on lower branches.

Please see also #20586 which adds a SymfonyStyle::getErrorStyle() shortcut for easily switching to stderr.

ogizanagi · 2016-11-25T18:17:42Z

Isn't there a lot more than those 4 commands? What about LintCommand for instance? :)

(I guess keeping this in #20586 would be less painful to update as you target master too for this bug fix 😅)

chalasr · 2016-11-25T18:39:20Z

@ogizanagi I made the changes only where it makes sense in my opinion, which exclude commands where the output give no relevant information at all.

I just applied it on the TwigLintCommand which makes use of SymfonyStyle::error, because the output is intended to be redirected, but at the moment I don't see more commands to update.

I guess keeping this in #20586 would be less painful to update

Not that much, each one can be merged independently and, since the subject of #20586 was "Add getErrorIo AND use stderr in existing commands", I do think it should be done in two PRs :)

chalasr · 2016-11-25T18:42:54Z

Note that when exceptions are thrown (which is the case in lot of commands including lint ones), the output goes to stderr naturally.

ogizanagi · 2016-11-28T18:00:20Z

@chalasr : I may have found some other places where it'll be relevant. See #20674

chalasr · 2016-11-28T18:04:14Z

Oh, I could have did it here if you just said me.. but ok then

ogizanagi · 2016-11-28T18:05:57Z

I had to check relevant places myself so it was simpler to submit it 😅

ogizanagi · 2016-11-28T18:07:24Z

src/Symfony/Bridge/Twig/Command/LintCommand.php

@@ -88,7 +89,8 @@ protected function execute(InputInterface $input, OutputInterface $output)
        $io = new SymfonyStyle($input, $output);

        if (null === $twig = $this->getTwigEnvironment()) {
-            $io->error('The Twig environment needs to be set.');
+            $errorIo = $output instanceof ConsoleOutputInterface ? new SymfonyStyle($input, $output->getErrorOutput()) : $io;
+            $errorIo->error('The Twig environment needs to be set.');


I'd challenge this one in order to simply throw a \RuntimeException instead. WDYT?

BTW, I've reverted a similar change in my previous PR thinking it was the same, but actually it's another place: https://github.com/symfony/symfony/blob/master/src/Symfony/Bridge/Twig/Command/DebugCommand.php#L90

Sure 👍 but should it be done here?

As it slightly changes the output and is done mainly in the aim to output it to stderr more straightforwardly, I'd say yes.

The Twig Bridge DebugCommand update is still missing here, even in case the suggestion about throwing an exception instead is not considered. ^^

Both debug/lint commands have been updated :)

chalasr · 2016-11-28T18:15:35Z

Having two places to discuss/review/merge about the same thing looks not simpler to me, as you wish.

ogizanagi · 2016-11-28T18:17:00Z

Apply the patch then and I'll close mine.

chalasr · 2016-11-28T18:28:02Z

Done 2af000a

nicolas-grekas · 2017-01-06T13:54:10Z

👍

chalasr · 2017-01-06T22:09:44Z

Updated for using getErrorStyle() in the FrameworkBundle (as it requires the console to be ~3.3) and fixed the WebServerBundle commands.

xabbuh · 2017-01-07T08:49:46Z

src/Symfony/Bridge/Twig/Command/LintCommand.php

@@ -88,7 +88,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
        $io = new SymfonyStyle($input, $output);

        if (null === $twig = $this->getTwigEnvironment()) {
-            $io->error('The Twig environment needs to be set.');
+            $io->getErrorStyle()->error('The Twig environment needs to be set.');


This will fail when the bridge is used with older versions (before 3.3) of the Console component which is completely valid.

xabbuh · 2017-01-07T08:50:13Z

src/Symfony/Bundle/FrameworkBundle/Command/ConfigDebugCommand.php

@@ -62,11 +62,12 @@ protected function configure()
    protected function execute(InputInterface $input, OutputInterface $output)
    {
        $io = new SymfonyStyle($input, $output);
+        $errorIo = $io->getErrorStyle();


Reverted everywhere

@xabbuh The framework now has a conflict for symfony/console: <3.3 so it should be fine

xabbuh · 2017-01-07T08:50:52Z

src/Symfony/Bundle/SecurityBundle/Command/UserPasswordEncoderCommand.php

@@ -85,8 +85,9 @@ protected function configure()
    protected function execute(InputInterface $input, OutputInterface $output)
    {
        $io = new SymfonyStyle($input, $output);
+        $errorIo = $io->getErrorStyle();


and the same also applies to the SecurityBundle

Would it work (and make sense) to add a conflict with older versions of the Console?

chalasr · 2017-01-08T12:43:37Z

Build failure unrelated (see #21198)

chalasr · 2017-01-10T17:42:15Z

Rebased, tests are green. This is ready

chalasr · 2017-01-30T15:42:34Z

ping

xabbuh

👍

fabpot · 2017-02-19T22:39:24Z

Thank you @chalasr.

…output (chalasr, ogizanagi) This PR was merged into the 3.3-dev branch. Discussion ---------- [FrameworkBundle] Make use of stderr for non reliable output | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a Built-in commands should make use of the proper outputs. As a feature on master, considering that people may actually rely on stdout and the fact commands have been changed a lot since 2.7, I think it's not worth doing this change on lower branches. Please see also #20586 which adds a `SymfonyStyle::getErrorStyle()` shortcut for easily switching to stderr. Commits ------- 7b262d8 [FrameworkBundle] Use getErrorStyle() when relevant 9a3a568 Use stderr for some other commands 1ee48bf [FrameworkBundle] Make use of stderr for non reliable output

carsonbot added Status: Needs Review FrameworkBundle Bug labels Nov 25, 2016

chalasr mentioned this pull request Nov 25, 2016

[Console] Ease writing to stderr using SymfonyStyle #20586

Merged

chalasr force-pushed the framework/use_stderr_in_commands branch from 1eac2be to b7af8c5 Compare November 25, 2016 11:45

chalasr force-pushed the framework/use_stderr_in_commands branch from b7af8c5 to b2920e7 Compare November 25, 2016 18:28

chalasr force-pushed the framework/use_stderr_in_commands branch from b2920e7 to 2207831 Compare November 25, 2016 18:41

ogizanagi mentioned this pull request Nov 28, 2016

[FrameworkBundle][SecurityBundle] Use stderr for some other commands #20674

Closed

ogizanagi reviewed Nov 28, 2016

View reviewed changes

nicolas-grekas added this to the 3.x milestone Dec 6, 2016

nicolas-grekas added Feature and removed Bug labels Dec 6, 2016

chalasr force-pushed the framework/use_stderr_in_commands branch 6 times, most recently from 8ddb044 to 6eaaf28 Compare January 6, 2017 21:42

xabbuh reviewed Jan 7, 2017

View reviewed changes

chalasr force-pushed the framework/use_stderr_in_commands branch 5 times, most recently from bebd602 to b64fe2d Compare January 8, 2017 10:37

chalasr force-pushed the framework/use_stderr_in_commands branch from b64fe2d to 462a6f5 Compare January 10, 2017 17:16

chalasr force-pushed the framework/use_stderr_in_commands branch 5 times, most recently from 1b327f5 to 201bb4a Compare January 12, 2017 17:33

chalasr and others added 3 commits January 12, 2017 21:28

[FrameworkBundle] Make use of stderr for non reliable output

1ee48bf

Use stderr for some other commands

9a3a568

[FrameworkBundle] Use getErrorStyle() when relevant

7b262d8

chalasr force-pushed the framework/use_stderr_in_commands branch from 201bb4a to 7b262d8 Compare January 12, 2017 20:28

xabbuh approved these changes Jan 30, 2017

View reviewed changes

fabpot merged commit 7b262d8 into symfony:master Feb 19, 2017

chalasr deleted the framework/use_stderr_in_commands branch February 20, 2017 05:55

nicolas-grekas modified the milestones: 3.x, 3.3 Mar 24, 2017

fabpot mentioned this pull request May 1, 2017

Release v3.3.0-BETA1 #22603

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FrameworkBundle] Make use of stderr for non reliable output #20632

[FrameworkBundle] Make use of stderr for non reliable output #20632

chalasr commented Nov 25, 2016 •

edited

Loading

ogizanagi commented Nov 25, 2016

chalasr commented Nov 25, 2016

chalasr commented Nov 25, 2016

ogizanagi commented Nov 28, 2016

chalasr commented Nov 28, 2016

ogizanagi commented Nov 28, 2016 •

edited

Loading

ogizanagi Nov 28, 2016

ogizanagi Nov 28, 2016

chalasr Nov 28, 2016

ogizanagi Nov 28, 2016 •

edited

Loading

ogizanagi Jan 7, 2017 •

edited

Loading

chalasr Jan 8, 2017

chalasr commented Nov 28, 2016

ogizanagi commented Nov 28, 2016

chalasr commented Nov 28, 2016

nicolas-grekas commented Jan 6, 2017

chalasr commented Jan 6, 2017 •

edited

Loading

xabbuh Jan 7, 2017

chalasr Jan 7, 2017

xabbuh Jan 7, 2017

chalasr Jan 7, 2017

chalasr Jan 12, 2017 •

edited

Loading

xabbuh Jan 7, 2017

chalasr Jan 7, 2017

chalasr commented Jan 8, 2017

chalasr commented Jan 10, 2017 •

edited

Loading

chalasr commented Jan 30, 2017

xabbuh left a comment

fabpot commented Feb 19, 2017

[FrameworkBundle] Make use of stderr for non reliable output #20632

[FrameworkBundle] Make use of stderr for non reliable output #20632

Conversation

chalasr commented Nov 25, 2016 • edited Loading

ogizanagi commented Nov 25, 2016

chalasr commented Nov 25, 2016

chalasr commented Nov 25, 2016

ogizanagi commented Nov 28, 2016

chalasr commented Nov 28, 2016

ogizanagi commented Nov 28, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ogizanagi Nov 28, 2016 • edited Loading

Choose a reason for hiding this comment

ogizanagi Jan 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chalasr commented Nov 28, 2016

ogizanagi commented Nov 28, 2016

chalasr commented Nov 28, 2016

nicolas-grekas commented Jan 6, 2017

chalasr commented Jan 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chalasr Jan 12, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chalasr commented Jan 8, 2017

chalasr commented Jan 10, 2017 • edited Loading

chalasr commented Jan 30, 2017

xabbuh left a comment

Choose a reason for hiding this comment

fabpot commented Feb 19, 2017

chalasr commented Nov 25, 2016 •

edited

Loading

ogizanagi commented Nov 28, 2016 •

edited

Loading

ogizanagi Nov 28, 2016 •

edited

Loading

ogizanagi Jan 7, 2017 •

edited

Loading

chalasr commented Jan 6, 2017 •

edited

Loading

chalasr Jan 12, 2017 •

edited

Loading

chalasr commented Jan 10, 2017 •

edited

Loading