Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[FrameworkBundle] Show server:run logs by default#19174
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| $process->setTty(true); | ||
| } | ||
| $process->run(); | ||
| }catch (RuntimeException$e) { |
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.
couldn't runtime exceptions happen for other reasons than unsupported tty mode during$process->run();, causing weird cases of running the process twice ?
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.
let's be safe, fixed
| $process->disableOutput(); | ||
| try { | ||
| if (OutputInterface::VERBOSITY_NORMAL >$output->getVerbosity()) { | ||
| $process->disableOutput(); |
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.
do we need to have this inside the try/catch ?
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.
no but we need to have thisif/else logic so code is easier this way
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.
the try/cache can be inside theelse (and defining$callback = null; before theif)
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.
done
fabpot commentedJun 30, 2016
Thank you@nicolas-grekas. |
…olas-grekas)This PR was merged into the 3.2-dev branch.Discussion----------[FrameworkBundle] Show server:run logs by default| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -I propose to change the default for the `server:run` command and show `php -S` logs by default.I really miss them otherwise. The `-vvv` mode is not suited here, because it adds a useless ` ERR ` red prefix.I do this through a tty when available, so that the output remains colored.Ping@javiereguiluz@weaverryan since this is mostly a DX issue.Commits-------7cc6161 [FrameworkBundle] Show server:run logs by default
I propose to change the default for the
server:runcommand and showphp -Slogs by default.I really miss them otherwise. The
-vvvmode is not suited here, because it adds a uselessERRred prefix.I do this through a tty when available, so that the output remains colored.
Ping@javiereguiluz@weaverryan since this is mostly a DX issue.