[2.2][Console] Added more verbosity levels. See GH #6066 #6084

dlsniper · 2012-11-21T22:08:01Z

Bug fix: no
Feature addition: yes
Backwards compatibility break: yes, breaks the interface
Symfony2 tests pass: yes
Fixes the following tickets: #6066
Todo: documentation?
License of the code: MIT
Documentation PR: ~

See the request in #6066.

Seldaek · 2012-11-22T15:46:51Z

src/Symfony/Component/Console/Application.php

+
+            case $input->hasParameterOption(array('--verbose-debug', '-vvv')) :
+                $output->setVerbosity(OutputInterface::VERBOSITY_DEBUG);
+                break;


I have to say I don't quite see the advantage of this weird case evaluate me this vs the if/elseif that was there before. More lines and awkwardness for no benefit (that I can see).

I only do this type of switch(true) when it adds more readability to the code instead of having the whole bunch of elseif () lines followed by the long line again and so on as it allows for better separation when going quickly thru the code.

I agree with @Seldaek that ifs would be better.

Seldaek · 2012-11-22T15:47:29Z

Cool thanks, did you try out if -vv works though? I was wondering if that would blow up the console or not.

stof · 2012-11-22T18:42:32Z

This simply does not work. You cannot pass the option --verbose-debug as it is not in the input definition and so will explode when validating the input.

dlsniper · 2012-11-22T19:16:10Z

Hi, I haven't tried it yet unfortunately and my schedule has just been messed up by my work. I'll update this asap, most likely next Monday.

dlsniper · 2012-11-25T13:10:32Z

Thanks to @stof I've added the missing definition for the new input values and now everything should work properly.

dlsniper · 2012-12-08T18:34:38Z

Converted to ifs and squashed :)

cordoval · 2012-12-09T14:19:55Z

src/Symfony/Component/Console/Application.php

+            new InputOption('--help',           '-h',   InputOption::VALUE_NONE, 'Display this help message.'),
+            new InputOption('--quiet',          '-q',   InputOption::VALUE_NONE, 'Do not output any message.'),
+            new InputOption('--verbose',        '-v',   InputOption::VALUE_NONE, 'Increase verbosity of messages.'),
+            new InputOption('--verbose-very',   '-vv',  InputOption::VALUE_NONE, 'Increase verbosity of messages to display even more verbose messages.'),


there has to be a criteria dividing certain messages from others that would explain this better, rather than just saying much more

cordoval · 2012-12-09T14:23:06Z

maybe rather than say much more verbosity we should have all verbosity?

dlsniper · 2012-12-09T14:32:59Z

Like @Seldaek said, I'm not really good at naming this.

dlsniper · 2012-12-11T22:41:31Z

@fabpot is there something that prevents this from going into master? If it's naming, I'm sure someone else could provide a better naming for it. The build failed due to some rather strange condition but unrelated to the changes done.

Tobion · 2012-12-11T23:09:23Z

Why is it needed? Who draws / how to draw a line between verbose and very verbose?

dlsniper · 2012-12-11T23:15:55Z

Think of the way you do logging where you have multiple levels.
If you want to use this in the console for user output/debugging/whatnot then you need to flag this somehow.
Also I really don't think we need a ruling on what's the appropriate level of verbosity for everything, just like we don't have a guideline for marking which output is info and which is critical. Users have to decided for themselves when it's the case to use one or the other.

As for a quick example, not the brightest star in the Universe but it's very late here:
I want to display:

normal: Download in progress
verbose: Download in progress: x%
very verbose: Download in progress: x% speed y
verbose debug: Connecting to HOST Connected to HOST ... Download in progress: x% speed y

Tobion · 2012-12-11T23:24:00Z

Yeah I know the use case. But I wonder if it's really made use of by users. Because they usually don't want any extra information when everything is ok. Or they want to see all information that is available. Levels in between are probably not that common.
Looking at some verbosity of other well known command line interfaces, I figured there are some that only have a single -v, some that have multiple like -v, -vv as suggested here and some even have -v{int}. Then again one could also add a quite flag -q so nothing is printed except errors.
All in all, there is no standard way.

stof · 2012-12-12T00:09:35Z

@Tobion we already have the quiet flag...
And for the different levels, this idea came from composer. Currently, you can put the output in verbose mode, getting the normal output plus the changelog for updated versions (which is great) and the git output (a bit less useful). But some people are asking to get the detailed report about decisions made by the solver to understand why some packages have been selected (or why they are conflicting). This would require another verbosity level to keep composer usable.

fabpot · 2012-12-12T08:46:04Z

For the composer use case, I think a debug mode would be better:

-q: quiet, output nothing
-v: useful information
-d: debug more where a lot of not-so interesting stuff are displayed

Seldaek · 2012-12-12T09:05:35Z

I just think -vv/-vvv is a more common paradigm than --debug. People actually try it and get no result. Also it is just a verbosity change in the way that it gives you more verbose output. I think it's good to consolidate it all in one v/vv/vvv. 3 levels should be enough for most purposes really.

dlsniper · 2012-12-12T09:50:14Z

👍 to @Seldaek

fabpot · 2012-12-14T08:18:59Z

ok, let's find good names now. So this PR uses:

--verbose
--verbose-very
--verbose-debug

I'm not fan of these names. Here are some ideas:

--verbose
--more-verbose
--even-more-verbose

Seldaek · 2012-12-14T09:08:30Z

How about --verbose=1/2/3 (with --verbose being equivalent to --verbose=1) and -v/-vv/-vvv?

dlsniper · 2012-12-16T00:03:12Z

Any preferences?

dlsniper · 2013-01-02T11:19:47Z

@fabpot should I change the names to the one mentioned by you so that this can be merged?
Or, taking the idea from @Seldaek, should we have:

--verbose=normal
--verbose=high
--verbose=debug

dlsniper · 2013-02-07T16:29:33Z

ping @fabpot @Seldaek can we agree on naming this? I'll be using it in production here soon and I'd rather not mess around with names after our internal release.

extesy · 2013-02-26T01:33:17Z

@fabpot @dlsniper I would recommend having a digital verbosity level (same as what @Seldaek suggested) in addition to -vvv shortcuts. This scales much better in case if one more verbosity level is needed later. Also it seems to be a more common pattern around the internet, for example here or here, etc.

Seldaek · 2013-04-06T19:31:55Z

@fabpot can we have feedback from you? Is --verbose=1/2/3 (with --verbose being equivalent to --verbose=1) and -v/-vv/-vvv alright? I'd like to get this moved forward.

fabpot · 2013-04-07T15:13:09Z

@Seldaek Your proposal looks good to me.

Seldaek · 2013-04-07T15:21:44Z

OK cool, @dlsniper do you have some time to work on updating this?

This PR was merged into the 2.2 branch. Discussion ---------- [PropertyAccess] Add objectives to pluralMap | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony#7483 | License | MIT | Doc PR | Commits ------- 6ef92fd [PropertyAccess] Add objectives to pluralMap

dlsniper · 2013-04-08T08:01:22Z

Hey, I'll do this and the doc PR in a day or two tops.

This PR was merged into the 2.2 branch. Discussion ---------- [Security] Removed unused var | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | I guess | Fixed tickets | - Commits ------- bd26419 [Security] Removed unused var

Seldaek · 2013-04-11T09:23:54Z

@dlsniper new PR coming or why did you close?

dlsniper · 2013-04-11T09:45:46Z

@Seldaek yes, seems I've pushed the wrong button then but a new PR for this will come soon, working on it right now.

Seldaek reviewed Nov 22, 2012
View reviewed changes

cordoval reviewed Dec 9, 2012
View reviewed changes

[Security] Removed unused var

bd26419

franmomu and others added 2 commits April 8, 2013 00:02

[PropertyAccess] Add objectives to pluralMap

6ef92fd

fabpot and others added 2 commits April 8, 2013 16:57

Added more verbosity levels. See GH symfony#6066

fe88059

dlsniper closed this Apr 11, 2013

Uh oh!

[2.2][Console] Added more verbosity levels. See GH #6066 #6084

[2.2][Console] Added more verbosity levels. See GH #6066 #6084

Uh oh!

Conversation

dlsniper commented Nov 21, 2012

Uh oh!

Seldaek Nov 22, 2012

Choose a reason for hiding this comment

Uh oh!

dlsniper Nov 25, 2012

Choose a reason for hiding this comment

Uh oh!

fabpot Dec 3, 2012

Choose a reason for hiding this comment

Uh oh!

Seldaek commented Nov 22, 2012

Uh oh!

stof commented Nov 22, 2012

Uh oh!

dlsniper commented Nov 22, 2012

Uh oh!

dlsniper commented Nov 25, 2012

Uh oh!

dlsniper commented Dec 8, 2012

Uh oh!

cordoval Dec 9, 2012

Choose a reason for hiding this comment

Uh oh!

cordoval commented Dec 9, 2012

Uh oh!

dlsniper commented Dec 9, 2012

Uh oh!

dlsniper commented Dec 11, 2012

Uh oh!

Tobion commented Dec 11, 2012

Uh oh!

dlsniper commented Dec 11, 2012

Uh oh!

Tobion commented Dec 11, 2012

Uh oh!

stof commented Dec 12, 2012

Uh oh!

fabpot commented Dec 12, 2012

Uh oh!

Seldaek commented Dec 12, 2012

Uh oh!

dlsniper commented Dec 12, 2012

Uh oh!

fabpot commented Dec 14, 2012

Uh oh!

Seldaek commented Dec 14, 2012

Uh oh!

dlsniper commented Dec 16, 2012

Uh oh!

dlsniper commented Jan 2, 2013

Uh oh!

dlsniper commented Feb 7, 2013

Uh oh!

extesy commented Feb 26, 2013

Uh oh!

Seldaek commented Apr 6, 2013

Uh oh!

fabpot commented Apr 7, 2013

Uh oh!

Seldaek commented Apr 7, 2013

Uh oh!

dlsniper commented Apr 8, 2013

Uh oh!

Seldaek commented Apr 11, 2013

Uh oh!

dlsniper commented Apr 11, 2013

Uh oh!

Uh oh!