8000 Updated the styles of the Twig commands by javiereguiluz · Pull Request #15989 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Updated the styles of the Twig commands #15989

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.

Sign up for GitHub

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

Updated the styles of the Twig commands #15989

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
384f0be
Updated the styles of the Twig commands
javiereguiluz Sep 29, 2015
adef8c4
Fixed some tests
javiereguiluz Sep 29, 2015
b60d937
Fixed tests for real
javiereguiluz Sep 29, 2015
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
15 changes: 7 additions & 8 deletions src/Symfony/Bridge/Twig/Command/DebugCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,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;

/**
* Lists twig functions, filters, globals and tests present in the current project.
Expand Down Expand Up @@ -82,10 +83,11 @@ protected function configure()

protected function execute(InputInterface $input, OutputInterface $output)
{
$output = new SymfonyStyle($input, $output);
$twig = $this->getTwigEnvironment();

if (null === $twig) {
$output->writeln('<error>The Twig environment needs to be set.</error>');
$output->error('The Twig environment needs to be set.');

return 1;
}
Expand Down Expand Up @@ -118,14 +120,11 @@ protected function execute(InputInterface $input, OutputInterface $output)
if (!$items) {
continue;
}
if ($index > 0) {
$output->writeln('');
}
$output->writeln('<info>'.ucfirst($type).'</info>');

$output->section(ucfirst($type));

ksort($items);
foreach ($items as $item) {
$output->writeln(' '.$item);
}
$output->listing($items);
}

return 0;
Expand Down
46 changes: 27 additions & 19 deletions src/Symfony/Bridge/Twig/Command/LintCommand.php
View file Edit file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,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\Finder\Finder;

/**
Expand Down Expand Up @@ -84,14 +85,17 @@ protected function configure()

protected function execute(InputInterface $input, OutputInterface $output)
{
$stdout = $output;
$output = new SymfonyStyle($input, $output);

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

$twig = $this->getTwigEnvironment();

if (null === $twig) {
$output->writeln('<error>The Twig environment needs to be set.</error>');
$output->error('The Twig environment needs to be set.');

return 1;
}
Expand All @@ -108,12 +112,12 @@ protected function execute(InputInterface $input, OutputInterface $output)
$template .= fread(STDIN, 1024);
}

return $this->display($input, $output, array($this->validate($twig, $template, uniqid('sf_'))));
return $this->display($input, $stdout, $output, array($this->validate($twig, $template, uniqid('sf_'))));
}

$filesInfo = $this->getFilesInfo($twig, $filenames);

return $this->display($input, $output, $filesInfo);
return $this->display($input, $stdout, $output, $filesInfo);
}

private function getFilesInfo(\Twig_Environment $twig, array $filenames)
Expand Down Expand Up @@ -157,32 +161,36 @@ private function validate(\Twig_Environment $twig, $template, $file)
return array('template' => $template, 'file' => $file, 'valid' => true);
}

private function display(InputInterface $input, OutputInterface $output, $files)
private function display(InputInterface $input, OutputInterface $stdout, $output, $files)
Copy link
Contributor

Choose a reason for hiding this comment

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

missing typehint for style

Copy link
Member

Choose a reason for hiding this comment

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

I would rather keep $output for the object received as $output in arguments and used a different name for the styling wrapper. It would be more readable IMO

{
switch ($input->getOption('format')) {
case 'txt':
return $this->displayTxt($output, $files);
return $this->displayTxt($stdout, $output, $files);
case 'json':
return $this->displayJson($output, $files);
return $this->displayJson($stdout, $files);
default:
throw new \InvalidArgumentException(sprintf('The format "%s" is not supported.', $input->getOption('format')));
}
}

private function displayTxt(OutputInterface $output, $filesInfo)
private function displayTxt(OutputInterface $stdout, $output, $filesInfo)
Copy link
Contributor

Choose a reason for hiding this comment

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

missing typehint

{
$errors = 0;

foreach ($filesInfo as $info) {
if ($info['valid'] && $output->isVerbose()) {
$output->writeln('<info>OK</info>'.($info['file'] ? sprintf(' in %s', $info['file']) : ''));
if ($info['valid'] && $stdout->isVerbose()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as #15991 (comment)
What do you think ?

$output->comment('<info>OK</info>'.($info['file'] ? sprintf(' in %s', $info['file']) : ''));
} elseif (!$info['valid']) {
++$errors;
$this->renderException($output, $info['template'], $info['exception'], $info['file']);
}
}

$output->writeln(sprintf('<comment>%d/%d valid files</comment>', count($filesInfo) - $errors, count($filesInfo)));
if ($errors === 0) {
$output->success(sprintf('All %d Twig files contain valid syntax.', count($filesInfo)));
} else {
$output->warning(sprintf('%d Twig files have valid syntax and %d contain errors.', count($filesInfo) - $errors, $errors));
}

return min($errors, 1);
}
Expand Down Expand Up @@ -211,20 +219,20 @@ private function renderException(OutputInterface $output, $template, \Twig_Error
$line = $exception->getTemplateLine();

if ($file) {
$output->writeln(sprintf('<error>KO</error> in %s (line %s)', $file, $line));
$output->text(sprintf('<error> ERROR </error> in %s (line %s)', $file, $line));
} else {
$output->writeln(sprintf('<error>KO</error> (line %s)', $line));
$output->text(sprintf('<error> ERROR </error> (line %s)', $line));
}

foreach ($this->getContext($template, $line) as $no => $code) {
$output->writeln(sprintf(
foreach ($this->getContext($template, $line) as $lineNumber => $code) {
$output->text(sprintf(
'%s %-6s %s',
$no == $line ? '<error>>></error>' : ' ',
$no,
$lineNumber === $line ? '<error> >> </error>' : ' ',
$lineNumber,
$code
));
if ($no == $line) {
$output->writeln(sprintf('<error>>> %s</error> ', $exception->getRawMessage()));
if ($lineNumber === $line) {
$output->text(sprintf('<error> >> %s</error> ', $exception->getRawMessage()));
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/Symfony/Bridge/Twig/Tests/Command/LintCommandTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public function testLintCorrectFile()
$ret = $tester->execute(array('filename' => array($filename)), array('verbosity' => OutputInterface::VERBOSITY_VERBOSE, 'decorated' => false));

$this->assertEquals(0, $ret, 'Returns 0 in case of success');
$this->assertRegExp('/^OK in /', $tester->getDisplay());
$this->assertRegExp('/^\/\/ OK in /', trim($tester->getDisplay()));
}

public function testLintIncorrectFile()
Expand All @@ -42,7 +42,7 @@ public function testLintIncorrectFile()
$ret = $tester->execute(array('filename' => array($filename)), array('decorated' => false));

$this->assertEquals(1, $ret, 'Returns 1 in case of error');
$this->assertRegExp('/^KO in /', $tester->getDisplay());
$this->assertRegExp('/^ERROR in /', trim($tester->getDisplay()));
}

/**
Expand All @@ -65,7 +65,7 @@ public function testLintFileCompileTimeException()
$ret = $tester->execute(array('filename' => array($filename)), array('decorated' => false));

$this->assertEquals(1, $ret, 'Returns 1 in case of error');
$this->assertRegExp('/^KO in /', $tester->getDisplay());
$this->assertRegExp('/^ERROR in /', trim($tester->getDisplay()));
}

/**
Expand Down
4 changes: 3 additions & 1 deletion src/Symfony/Bundle/TwigBundle/Command/DebugCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use Symfony\Bridge\Twig\Command\DebugCommand as BaseDebugCommand;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Console\Style\SymfonyStyle;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\DependencyInjection\ContainerAwareInterface;

Expand Down Expand Up @@ -57,8 +58,9 @@ protected function configure()

protected function execute(InputInterface $input, OutputInterface $output)
Copy link
Contributor

Choose a reason for hiding this comment

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

this class is missing an @deprecated phpdoc

{
$output = 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.

you should not replace the $output variable which will be passed to the parent IMO

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

parent::execute($input, $output);
Expand Down
0