-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Console] RFC for Symfony 3.0: Show errors even when --quiet is present #15680
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Point 3 sounds the best option. |
@fabpot I think so too. I've discussed this a bit on IRC with Iltar and we thought it could also go into 2.8 if we turn the $type arg into a $flags arg that takes bit flags for either |
Changing the values is fine by me, if not, defining constants is useless. |
@Seldaek if I understand your proposal correctly, I think it's a bit contradictory: Problem: if I run a command with |
@javiereguiluz |
@wouterj yes, but I think these two things are different:
I think the first point makes sense. But I don't understand the second one. |
@javiereguiluz I think this is to be more flexible (e.g. allow errors that are not exceptions). Btw, point (3) of this issue (and your second point) is not only about writing messages in quiet mode, it's also about improving to conditionally write in certain modes. E.g: -if ($output->isVerbose()) {
- $this->writeln('Some verbose message');
-}
+$this->writeln('Some verbose message', default, OutputInterface::VERBOSITY_VERBOSE); |
@wouterj the new code would be: $output->writeln('Some verbose message', OutputInterface::OUTPUT_NORMAL, OutputInterface::VERBOSITY_VERY_VERBOSE); (which by the way is 114 chars long and the previous My concern is that the new behavior would allow commands to display any message even when running in |
@javiereguiluz the new code if we change $type to be a bitmask of options would be Right now, in --quiet mode AFAIK it's impossible to output anything at all. I'd just like to change that because it's usually not what people mean by quiet, they just don't want any output if things go well. If you get an error in a CI build or something and you used --quiet to keep things lean in the output, then it fails but you don't know what happened retroactively which sucks. It's been requested a few times by Composer users. If you really don't want any output ever, |
…erbosity can be passed in as well (Seldaek) This PR was merged into the 2.8 branch. Discussion ---------- Convert Output::write's type to an options arg where verbosity can be passed in as well | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #15680 | License | MIT | Doc PR | Commits ------- 749fba5 Convert Output::write's type to an options arg where verbosity can be passed in as well
…ixes #15680 (Seldaek) This PR was merged into the 2.8 branch. Discussion ---------- Make the exception output visible even in quiet mode, fixes #15680 | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #15680 | License | MIT | Doc PR | This builds upon #15772 and sets the exception output in Application::renderException to use VERBOSITY_QUIET so that exceptions are always visible. IMO it's a good change but I can see that it is potentially controversial. Commits ------- e9ee8f5 Make the exception output visible even in quiet mode, fixes #15680
* 2.8: (31 commits) [DomCrawler] Invalid uri created from forms if base tag present [VarDumper] Add caster for OuterIterator objects [Console] update param type phpdoc for StreamOutput [Console] fix typo in OutputInterface Use stderr by default when a specific output is not injected fixed bad merge [Debug] Fix case mismatch detection [HttpKernel] Add entry point to more easily create/configure the DI extension [DX] Added a logout link in the security panel of the web debug toolbar [HttpKernel] fix broken multiline <esi:remove> [DoctrineBridge] Fixed #14840 [FrameworkBundle] add a suggest for the serializer component fixed CS removed non-working tests [WIP] #15502 Make template shortcuts be usable without Templating component Redesigned the Symfony Profiler [Yaml] Fix the parsing of float keys Make the exception output visible even in quiet mode, fixes #15680 Convert Output::write's type to an options arg where verbosity can be passed in as well [Console] Ensure the console output is only detected as decorated when both stderr and stdout support colors ...
This PR was submitted for the 2.8 branch but it was merged into the 2.7 branch instead (closes #5672). Discussion ---------- Add constants to BC promise | Q | A | --- | --- | Doc fix? | no | New docs? | yes | Applies to | all | Fixed tickets | #5224 This does things a little bit different than proposed in #5224, based on [this comment by @fabpot](symfony/symfony#15680 (comment)) and the fact that one uses constants, to allow to change the value without breaking the code. /cc @webmozart @stof @weaverryan @xabbuh please review this & give your opinion on this. Commits ------- d25c28d Add constants to BC promise
* 2.8: (31 commits) [DomCrawler] Invalid uri created from forms if base tag present [VarDumper] Add caster for OuterIterator objects [Console] update param type phpdoc for StreamOutput [Console] fix typo in OutputInterface Use stderr by default when a specific output is not injected fixed bad merge [Debug] Fix case mismatch detection [HttpKernel] Add entry point to more easily create/configure the DI extension [DX] Added a logout link in the security panel of the web debug toolbar [HttpKernel] fix broken multiline <esi:remove> [DoctrineBridge] Fixed symfony#14840 [FrameworkBundle] add a suggest for the serializer component fixed CS removed non-working tests [WIP] symfony#15502 Make template shortcuts be usable without Templating component Redesigned the Symfony Profiler [Yaml] Fix the parsing of float keys Make the exception output visible even in quiet mode, fixes symfony#15680 Convert Output::write's type to an options arg where verbosity can be passed in as well [Console] Ensure the console output is only detected as decorated when both stderr and stdout support colors ...
… including errors (wouterj) This PR was merged into the 7.2 branch. Discussion ---------- [Console] Add silent verbosity suppressing all output, including errors | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | no | New feature? | yes | Deprecations? | yes | Issues | Fix #52777 | License | MIT <details> <summary>Original PR description</summary> Alternative to #53126 In Symfony 2.8, we decided to still show exceptions/errors when running a command with `--quiet` or `SHELL_VERBOSITY=-1` (#15680). Since that time, we've introduced the ConsoleLogger and 12-factor app logic. In todays landscape, it's more common to run commands that only output computer-readable text (e.g. JSON), which is ingested and displayed by services like Datadog and Kibana. Our decision from 2.8 breaks this, as the JSON is interrupted by human-readable exception output that users can't disable. At the same time, this information is duplicated as errors are always logged (and thus shown as JSON). I think we should revert the 2.8 decision, rather than adding a new "extremely quiet" verbosity mode. As far as I'm aware, this is also more consistent with normal unix commands which also don't output anything when passing `--quiet`. I don't think this warrants as a BC break, as the errors are human readable and we don't promise BC on human readable console output (afaik, we don't promise BC on any console output, but I think we should be careful when changing e.g. the JSON logging). </details> This updated PR adds a new `--silent` verbosity mode that suppresses all output, including exceptions caught by the Application. I went back and forth between simply passing `NullOutput` to the command in silent mode or not ignoring everything written to silent mode in `Output`. In the end, I've decided for the last to avoid bug reports from people silently expecting a `ConsoleOutputInterface` in their commands (e.g. for sections) without properly guarding it. The new `isSilent()` methods can be used by commands to e.g. write important messages to the logger instead of command output when running in silent mode. Commits ------- 57fe0bd [Console] Add silent verbosity mode suppressing all output, including errors
I'd like to discuss this (sort of BC breaking) change for 3.0. The problem is in most cases people use --quiet / -q when running a console app, they don't want any output but still expect output/details in case an error occurs.
I see a few options to handle this:
$verbosity = self::VERBOSITY_NORMAL
. That would allow any write call to contain information about which verbosity level it should be written to. I think that would be quite nice because it also enables dropping a lot of conditionals likeif ($output->isVeryVerbose()) { $output->writeln(...); }
, those would become$output->writeln('foo', OutputInterface::OUTPUT_NORMAL, OutputInterface::VERBOSITY_VERY_VERBOSE);
which reads a bit better, except for the constants being long as hell and the OUTPUT type sitting in the middle (not sure if that'd be too much of a BC break to add the new arg as second argument, I've never seen the output type being used in the wild, or maybe both could be cobbled together as a bitwise flag).Point 3 would be nice either way I think, and it would allow some messages to pass through the quiet mode, so if you do a write with VERBOSITY_QUIET it would always be sent, but VERBOSITY_NORMAL and above wouldn't be sent in quiet mode. Similarly we could change the exception writing to be written on the quiet verbosity.
If anyone has another idea or comments that'd be appreciated.
The text was updated successfully, but these errors were encountered: