Skip to content

[Console] New styles on commands #14172

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

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 14 additions & 10 deletions src/Symfony/Bundle/FrameworkBundle/Command/ServerRunCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Console\Style\SymfonyStyle;
use Symfony\Component\Process\PhpExecutableFinder;
use Symfony\Component\Process\ProcessBuilder;

Expand Down Expand Up @@ -84,27 +85,30 @@ protected function configure()
protected function execute(InputInterface $input, OutputInterface $output)
{
$documentRoot = $input->getOption('docroot');
$outputStyle = new SymfonyStyle($input, $output);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we call this variable $outputStyle and in the rest of the commands we just call it $output. I'm not sure which is the best strategy to follow. @stof already expressed his concerns about the name of this variable in other pull request. In any case, I agree that it's hard to name this variable properly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I understand, the var name in this cmd is different because $output is used here https://github.com/94noni/symfony/blob/14138_commands_style/src/Symfony/Bundle/FrameworkBundle/Command/ServerRunCommand.php#L125

ok for waiting a proper name

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I said in #14132 (comment), I don't see why we should absolutly name this variable differently, since it implements OutputInterface, only wraps the standard $output and behaves the same.
IMO, we should name it the same as the standard output as soon as we can.

BTW, I don't think there is any issue using $output everywhere in this PR. The line you mentioned call the ProcessHelper:run() method which expects an OutputInterface instance.


if (null === $documentRoot) {
$documentRoot = $this->getContainer()->getParameter('kernel.root_dir').'/../web';
}

if (!is_dir($documentRoot)) {
$output->writeln(sprintf('<error>The given document root directory "%s" does not exist</error>', $documentRoot));
$outputStyle->error(sprintf('The given document root directory "%s" does not exist.', $documentRoot));

return 1;
}

$env = $this->getContainer()->getParameter('kernel.environment');

if ('prod' === $env) {
$output->writeln('<error>Running PHP built-in server in production environment is NOT recommended!</error>');
$outputStyle->error('Running PHP built-in server in production environment is NOT recommended!');
}

$output->writeln(sprintf("Server running on <info>http://%s</info>\n", $input->getArgument('address')));
$output->writeln('Quit the server with CONTROL-C.');
$outputStyle->text(array(
sprintf('Server running on <info>http://%s</info>.', $input->getArgument('address')),
'Quit the server with CONTROL-C.',
));

if (null === $builder = $this->createPhpProcessBuilder($input, $output, $env)) {
if (null === $builder = $this->createPhpProcessBuilder($input, $outputStyle, $env)) {
return 1;
}

Expand All @@ -121,17 +125,17 @@ protected function execute(InputInterface $input, OutputInterface $output)
->run($output, $process, null, null, OutputInterface::VERBOSITY_VERBOSE);

if (!$process->isSuccessful()) {
$output->writeln('<error>Built-in server terminated unexpectedly</error>');
$outputStyle->error('Built-in server terminated unexpectedly.');

if ($process->isOutputDisabled()) {
$output->writeln('<error>Run the command again with -v option for more details</error>');
$outputStyle->error('Run the command again with -v option for more details.');
}
}

return $process->getExitCode();
}

private function createPhpProcessBuilder(InputInterface $input, OutputInterface $output, $env)
private function createPhpProcessBuilder(InputInterface $input, OutputInterface $outputStyle, $env)
{
$router = $input->getOption('router') ?: $this
->getContainer()
Expand All @@ -140,7 +144,7 @@ private function createPhpProcessBuilder(InputInterface $input, OutputInterface
;

if (!file_exists($router)) {
$output->writeln(sprintf('<error>The given router script "%s" does not exist</error>', $router));
$outputStyle->error(sprintf('The given router script "%s" does not exist.', $router));

return;
}
Expand All @@ -149,7 +153,7 @@ private function createPhpProcessBuilder(InputInterface $input, OutputInterface
$finder = new PhpExecutableFinder();

if (false === $binary = $finder->find()) {
$output->writeln('<error>Unable to find PHP binary to run server</error>');
$outputStyle->error('Unable to find PHP binary to run server.');

return;
}
Expand Down
25 changes: 14 additions & 11 deletions src/Symfony/Bundle/FrameworkBundle/Command/ServerStartCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Console\Style\SymfonyStyle;
use Symfony\Component\Process\PhpExecutableFinder;
use Symfony\Component\Process\Process;

Expand Down Expand Up @@ -71,9 +72,11 @@ protected function configure()
*/
protected function execute(InputInterface $input, OutputInterface $output)
{
$output = new SymfonyStyle($input, $output);

if (!extension_loaded('pcntl')) {
$output->writeln('<error>This command needs the pcntl extension to run.</error>');
$output->writeln('You can either install it or use the <info>server:run</info> command instead to run the built-in web server.');
$output->error('This command needs the pcntl extension to run.');
$output->text('You can either install it or use the <comment>"server:run"</comment> command instead to run the built-in web server.');

return 1;
}
Expand All @@ -85,7 +88,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
}

if (!is_dir($documentRoot)) {
$output->writeln(sprintf('<error>The given document root directory "%s" does not exist</error>', $documentRoot));
$output->error(sprintf('The given document root directory "%s" does not exist.', $documentRoot));

return 1;
}
Expand All @@ -94,37 +97,37 @@ protected function execute(InputInterface $input, OutputInterface $output)
$address = $input->getArgument('address');

if (false === strpos($address, ':')) {
$output->writeln('The address has to be of the form <comment>bind-address:port</comment>.');
$output->text('The address has to be of the form <comment>bind-address:port</comment>.');

return 1;
}

if ($this->isOtherServerProcessRunning($address)) {
$output->writeln(sprintf('<error>A process is already listening on http://%s.</error>', $address));
$output->error(sprintf('A process is already listening on http://%s.', $address));

return 1;
}

if ('prod' === $env) {
$output->writeln('<error>Running PHP built-in server in production environment is NOT recommended!</error>');
$output->error('Running PHP built-in server in production environment is NOT recommended!');
}

$pid = pcntl_fork();

if ($pid < 0) {
$output->writeln('<error>Unable to start the server process</error>');
$output->error('Unable to start the server process.');

return 1;
}

if ($pid > 0) {
$output->writeln(sprintf('<info>Web server listening on http://%s</info>', $address));
$output->success(sprintf('Web server listening on http://%s.', $address));

return;
}

if (posix_setsid() < 0) {
$output->writeln('<error>Unable to set the child process as session leader</error>');
$output->error('Unable to set the child process as session leader.');

return 1;
}
Expand All @@ -139,7 +142,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
touch($lockFile);

if (!$process->isRunning()) {
$output->writeln('<error>Unable to start the server process</error>');
$output->error('Unable to start the server process.');
unlink($lockFile);

return 1;
Expand Down Expand Up @@ -198,7 +201,7 @@ private function createServerProcess(OutputInterface $output, $address, $documen

$finder = new PhpExecutableFinder();
if (false === $binary = $finder->find()) {
$output->writeln('<error>Unable to find PHP binary to start server</error>');
$output->error('Unable to find PHP binary to start server.');

return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Console\Style\SymfonyStyle;

/**
* Shows the status of a process that is running PHP's built-in web server in
Expand Down Expand Up @@ -49,10 +50,11 @@ protected function execute(InputInterface $input, OutputInterface $output)
unlink($this->getLockFile($address));
}

$output = new SymfonyStyle($input, $output);
if (file_exists($this->getLockFile($address))) {
$output->writeln(sprintf('<info>Web server still listening on http://%s</info>', $address));
$output->success(sprintf('Web server still listening on http://%s.', $address));
} else {
$output->writeln(sprintf('<error>No web server is listening on http://%s</error>', $address));
$output->error(sprintf('No web server is listening on http://%s.', $address));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Console\Style\SymfonyStyle;

/**
* Stops a background process running PHP's built-in web server.
Expand Down Expand Up @@ -54,14 +55,15 @@ protected function execute(InputInterface $input, OutputInterface $output)
{
$address = $input->getArgument('address');
$lockFile = $this->getLockFile($address);
$output = new SymfonyStyle($input, $output);

if (!file_exists($lockFile)) {
$output->writeln(sprintf('<error>No web server is listening on http://%s</error>', $address));
$output->error(sprintf('No web server is listening on http://%s.', $address));

return 1;
}

unlink($lockFile);
$output->writeln(sprintf('<info>Stopped the web server listening on http://%s</info>', $address));
$output->success(sprintf('Stopped the web server listening on http://%s.', $address));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Bundle\FrameworkBundle\Command;

use Symfony\Component\Console\Style\SymfonyStyle;
use Symfony\Component\Translation\Catalogue\MergeOperation;
use Symfony\Component\Console\Helper\Table;
use Symfony\Component\Console\Input\InputInterface;
Expand Down Expand Up @@ -81,8 +82,10 @@ protected function configure()
*/
protected function execute(InputInterface $input, OutputInterface $output)
{
$output = new SymfonyStyle($input, $output);

if (false !== strpos($input->getFirstArgument(), ':d')) {
$output->writeln('<comment>The use of "translation:debug" command is deprecated since version 2.7 and will be removed in 3.0. Use the "debug:translation" instead.</comment>');
$output->caution('The use of "translation:debug" command is deprecated since version 2.7 and will be removed in 3.0. Use the "debug:translation" instead.');
}

$locale = $input->getArgument('locale');
Expand All @@ -109,13 +112,13 @@ protected function execute(InputInterface $input, OutputInterface $output)

// No defined or extracted messages
if (empty($allMessages) || null !== $domain && empty($allMessages[$domain])) {
$outputMessage = sprintf('<info>No defined or extracted messages for locale "%s"</info>', $locale);
$outputMessage = sprintf('No defined or extracted messages for locale "%s"', $locale);

if (null !== $domain) {
$outputMessage .= sprintf(' <info>and domain "%s"</info>', $domain);
$outputMessage .= sprintf(' and domain "%s"', $domain);
}

$output->writeln($outputMessage);
$output->success($outputMessage);

return;
}
Expand All @@ -135,17 +138,13 @@ protected function execute(InputInterface $input, OutputInterface $output)
}
}

/** @var \Symfony\Component\Console\Helper\Table $table */
$table = new Table($output);

// Display header line
// Header line
$headers = array('State(s)', 'Domain', 'Id', sprintf('Message Preview (%s)', $locale));
foreach ($fallbackCatalogues as $fallbackCatalogue) {
$headers[] = sprintf('Fallback Message Preview (%s)', $fallbackCatalogue->getLocale());
}
$table->setHeaders($headers);

// Iterate all message ids and determine their state
$rows = array();
foreach ($allMessages as $domain => $messages) {
foreach (array_keys($messages) as $messageId) {
$value = $currentCatalogue->get($messageId, $domain);
Expand Down Expand Up @@ -177,17 +176,18 @@ protected function execute(InputInterface $input, OutputInterface $output)
$row[] = $this->sanitizeString($fallbackCatalogue->get($messageId, $domain));
}

$table->addRow($row);
$rows[] = $row;
}
}

$table->render();

$output->writeln('');
$output->writeln('<info>Legend:</info>');
$output->writeln(sprintf(' %s Missing message', $this->formatState(self::MESSAGE_MISSING)));
$output->writeln(sprintf(' %s Unused message', $this->formatState(self::MESSAGE_UNUSED)));
$output->writeln(sprintf(' %s Same as the fallback message', $this->formatState(self::MESSAGE_EQUALS_FALLBACK)));
$output->section('Debug Translation result');
$output->table($headers, $rows);
$output->section('Legend');
$output->listing(array(
sprintf('%s Missing message', $this->formatState(self::MESSAGE_MISSING)),
sprintf('%s Unused message', $this->formatState(self::MESSAGE_UNUSED)),
sprintf('%s Same as the fallback message', $this->formatState(self::MESSAGE_EQUALS_FALLBACK)),
));
}

private function formatState($state)
Expand Down
Loading