-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Console] Added more verbosity levels #7626
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
Conversation
From the original proposal of @Seldaek @fabpot do we want to support multiple aliases? If so, I can do the patch as well. If the PR is ok, I'll create the doc PR for this to be mergeable. |
} | ||
|
||
if (true === $input->hasParameterOption('-vvv')) { | ||
$output->setVerbosity(OutputInterface::VERBOSITY_VERY_VERBOSE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be DEBUG
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've forgot to remove them from the commit as they are not used right now, see the comment above. Thanks for pointing them out.
@dlsniper Support for multiple aliases as you call them should be done in another PR. |
The PR is not mergeable yet IMO, it adds weird bugs. I'm investigating. |
Thanks for feedback, I'm travelling right now, I'll fix the remaining issues tonight. |
Sent a PR (dlsniper#2) with some fixes, but that's unfortunately not the end of it. Since --verbose now has an optional value, doing things like: I see a few ways of dealing with this:
I think I would prefer the first approach, because it's easy, and introduces a minor inconsistency for the verbose flag but that's hopefully ok. |
@Seldaek I've fixed the remaining bugs and I've used the first option. It's not really pretty but it works so it should be fine from my pov. Can you check if it works for you now? I've also changed the text to the one suggested by @fabpot If everything looks good, I'll do the doc PR asap. I'll start with the PR for multiple shortcuts/aliases if we want to have support for that as well, let me know about it. |
Answered inline in new commits regarding the code.
I don't think multiple aliases really makes sense in most cases. Might |
What the status of this PR? |
@fabpot sorry for not responding earlier. I don't have the time to make this feature work in time for 2.3. If someone else wants to take over it, please do so. Feel free to close this when a new PR appears. |
@dlsniper I'll try to take over because I really need this. |
Closing in favor of #7841 |
This PR was merged into the master branch. Discussion ---------- [Console] Add more verbosity levels | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #6066 | License | MIT | Doc PR | symfony/symfony-docs#2554 This adds new verbosity levels. Replaces #7626 /cc @dlsniper Commits ------- 02c2038 Fix tests 5327ec4 Add notes in UPGRADE and CHANGELOG 77c9791 [Console] Add tests and support for more types of inputs 54c1377 [Console] Handle new verbosity levels e639686 Revert invalid changes 8f4d376 Removed unused options 5bb4163 Fixed tests b62d35f Fix handling of --verbose=... and BC break 16cdb61 Added more verbosity levels
This adds new verbosity levels.