feature #19012 [Console] progress bar fix (TomasVotruba, fabpot)

This PR was merged into the 3.2-dev branch.

Discussion
----------

[Console] progress bar fix

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #13019
| License       | MIT
| Doc PR        | -

This is #16490 where I've simplified the code as much as possible and added a test for the bug we're trying to fix.

The main change is the renaming of the `TerminalDimensionsProvider` to just `Terminal`. The new class can probably be useful to add more about the terminal.

Commits
-------

2f81247 switched to use COLUMNS and LINES env vars to change terminal dimensions
bf7a5c5 fixed logic
a589635 deprecated some Console Application methods
8f206c8 fixed CS, simplified code
b030c24 [Console] ProgressBar - adjust to the window width (static)
This commit is contained in:
Fabien Potencier 2016-06-17 19:52:18 +02:00
commit afed2f888d
7 changed files with 288 additions and 137 deletions

View File

@ -65,13 +65,11 @@ class Application
private $definition;
private $helperSet;
private $dispatcher;
private $terminalDimensions;
private $terminal;
private $defaultCommand;
private $singleCommand;
/**
* Constructor.
*
* @param string $name The name of the application
* @param string $version The version of the application
*/
@ -79,6 +77,7 @@ class Application
{
$this->name = $name;
$this->version = $version;
$this->terminal = new Terminal();
$this->defaultCommand = 'list';
$this->helperSet = $this->getDefaultHelperSet();
$this->definition = $this->getDefaultInputDefinition();
@ -625,7 +624,7 @@ class Application
$len = $this->stringWidth($title);
$width = $this->getTerminalWidth() ? $this->getTerminalWidth() - 1 : PHP_INT_MAX;
$width = $this->terminal->getWidth() ? $this->terminal->getWidth() - 1 : PHP_INT_MAX;
// HHVM only accepts 32 bits integer in str_split, even when PHP_INT_MAX is a 64 bit integer: https://github.com/facebook/hhvm/issues/1327
if (defined('HHVM_VERSION') && $width > 1 << 31) {
$width = 1 << 31;
@ -689,60 +688,42 @@ class Application
* Tries to figure out the terminal width in which this application runs.
*
* @return int|null
*
* @deprecated since version 3.2, to be removed in 4.0. Create a Terminal instance instead.
*/
protected function getTerminalWidth()
{
$dimensions = $this->getTerminalDimensions();
@trigger_error(sprintf('%s is deprecated as of 3.2 and will be removed in 4.0. Create a Terminal instance instead.', __METHOD__, ArgumentResolverInterface::class), E_USER_DEPRECATED);
return $dimensions[0];
return $this->terminal->getWidth();
}
/**
* Tries to figure out the terminal height in which this application runs.
*
* @return int|null
*
* @deprecated since version 3.2, to be removed in 4.0. Create a Terminal instance instead.
*/
protected function getTerminalHeight()
{
$dimensions = $this->getTerminalDimensions();
@trigger_error(sprintf('%s is deprecated as of 3.2 and will be removed in 4.0. Create a Terminal instance instead.', __METHOD__, ArgumentResolverInterface::class), E_USER_DEPRECATED);
return $dimensions[1];
return $this->terminal->getHeight();
}
/**
* Tries to figure out the terminal dimensions based on the current environment.
*
* @return array Array containing width and height
*
* @deprecated since version 3.2, to be removed in 4.0. Create a Terminal instance instead.
*/
public function getTerminalDimensions()
{
if ($this->terminalDimensions) {
return $this->terminalDimensions;
}
@trigger_error(sprintf('%s is deprecated as of 3.2 and will be removed in 4.0. Create a Terminal instance instead.', __METHOD__, ArgumentResolverInterface::class), E_USER_DEPRECATED);
if ('\\' === DIRECTORY_SEPARATOR) {
// extract [w, H] from "wxh (WxH)"
if (preg_match('/^(\d+)x\d+ \(\d+x(\d+)\)$/', trim(getenv('ANSICON')), $matches)) {
return array((int) $matches[1], (int) $matches[2]);
}
// extract [w, h] from "wxh"
if (preg_match('/^(\d+)x(\d+)$/', $this->getConsoleMode(), $matches)) {
return array((int) $matches[1], (int) $matches[2]);
}
}
if ($sttyString = $this->getSttyColumns()) {
// extract [w, h] from "rows h; columns w;"
if (preg_match('/rows.(\d+);.columns.(\d+);/i', $sttyString, $matches)) {
return array((int) $matches[2], (int) $matches[1]);
}
// extract [w, h] from "; h rows; w columns"
if (preg_match('/;.(\d+).rows;.(\d+).columns/i', $sttyString, $matches)) {
return array((int) $matches[2], (int) $matches[1]);
}
}
return array(null, null);
return array($this->terminal->getWidth(), $this->terminal->getHeight());
}
/**
@ -754,10 +735,15 @@ class Application
* @param int $height The height
*
* @return Application The current application
*
* @deprecated since version 3.2, to be removed in 4.0. Set the COLUMNS and LINES env vars instead.
*/
public function setTerminalDimensions($width, $height)
{
$this->terminalDimensions = array($width, $height);
@trigger_error(sprintf('%s is deprecated as of 3.2 and will be removed in 4.0. Set the COLUMNS and LINES env vars instead.', __METHOD__, ArgumentResolverInterface::class), E_USER_DEPRECATED);
putenv('COLUMNS='.$width);
putenv('LINES='.$height);
return $this;
}
@ -927,54 +913,6 @@ class Application
));
}
/**
* Runs and parses stty -a if it's available, suppressing any error output.
*
* @return string
*/
private function getSttyColumns()
{
if (!function_exists('proc_open')) {
return;
}
$descriptorspec = array(1 => array('pipe', 'w'), 2 => array('pipe', 'w'));
$process = proc_open('stty -a | grep columns', $descriptorspec, $pipes, null, null, array('suppress_errors' => true));
if (is_resource($process)) {
$info = stream_get_contents($pipes[1]);
fclose($pipes[1]);
fclose($pipes[2]);
proc_close($process);
return $info;
}
}
/**
* Runs and parses mode CON if it's available, suppressing any error output.
*
* @return string <width>x<height> or null if it could not be parsed
*/
private function getConsoleMode()
{
if (!function_exists('proc_open')) {
return;
}
$descriptorspec = array(1 => array('pipe', 'w'), 2 => array('pipe', 'w'));
$process = proc_open('mode CON', $descriptorspec, $pipes, null, null, array('suppress_errors' => true));
if (is_resource($process)) {
$info = stream_get_contents($pipes[1]);
fclose($pipes[1]);
fclose($pipes[2]);
proc_close($process);
if (preg_match('/--------+\r?\n.+?(\d+)\r?\n.+?(\d+)\r?\n/', $info, $matches)) {
return $matches[2].'x'.$matches[1];
}
}
}
/**
* Returns abbreviated suggestions in string format.
*

View File

@ -14,6 +14,7 @@ namespace Symfony\Component\Console\Helper;
use Symfony\Component\Console\Output\ConsoleOutputInterface;
use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Console\Exception\LogicException;
use Symfony\Component\Console\Terminal;
/**
* The ProgressBar provides helpers to display progress output.
@ -44,13 +45,12 @@ class ProgressBar
private $formatLineCount;
private $messages = array();
private $overwrite = true;
private $terminal;
private static $formatters;
private static $formats;
/**
* Constructor.
*
* @param OutputInterface $output An OutputInterface instance
* @param int $max Maximum steps (0 if unknown)
*/
@ -62,6 +62,7 @@ class ProgressBar
$this->output = $output;
$this->setMaxSteps($max);
$this->terminal = new Terminal();
if (!$this->output->isDecorated()) {
// disable overwrite when output does not support ANSI codes.
@ -217,7 +218,7 @@ class ProgressBar
*/
public function setBarWidth($size)
{
$this->barWidth = (int) $size;
$this->barWidth = max(1, (int) $size);
}
/**
@ -412,21 +413,7 @@ class ProgressBar
$this->setRealFormat($this->internalFormat ?: $this->determineBestFormat());
}
$this->overwrite(preg_replace_callback("{%([a-z\-_]+)(?:\:([^%]+))?%}i", function ($matches) {
if ($formatter = $this::getPlaceholderFormatterDefinition($matches[1])) {
$text = call_user_func($formatter, $this, $this->output);
} elseif (isset($this->messages[$matches[1]])) {
$text = $this->messages[$matches[1]];
} else {
return $matches[0];
}
if (isset($matches[2])) {
$text = sprintf('%'.$matches[2], $text);
}
return $text;
}, $this->format));
$this->overwrite($this->buildLine());
}
/**
@ -592,4 +579,38 @@ class ProgressBar
'debug_nomax' => ' %current% [%bar%] %elapsed:6s% %memory:6s%',
);
}
/**
* @return string
*/
private function buildLine()
{
$regex = "{%([a-z\-_]+)(?:\:([^%]+))?%}i";
$callback = function ($matches) {
if ($formatter = $this::getPlaceholderFormatterDefinition($matches[1])) {
$text = call_user_func($formatter, $this, $this->output);
} elseif (isset($this->messages[$matches[1]])) {
$text = $this->messages[$matches[1]];
} else {
return $matches[0];
}
if (isset($matches[2])) {
$text = sprintf('%'.$matches[2], $text);
}
return $text;
};
$line = preg_replace_callback($regex, $callback, $this->format);
$lineLength = Helper::strlenWithoutDecoration($this->output->getFormatter(), $line);
$terminalWidth = $this->terminal->getWidth();
if ($lineLength <= $terminalWidth) {
return $line;
}
$this->setBarWidth($this->barWidth - $lineLength + $terminalWidth);
return preg_replace_callback($regex, $callback, $this->format);
}
}

View File

@ -11,7 +11,6 @@
namespace Symfony\Component\Console\Style;
use Symfony\Component\Console\Application;
use Symfony\Component\Console\Exception\RuntimeException;
use Symfony\Component\Console\Formatter\OutputFormatter;
use Symfony\Component\Console\Helper\Helper;
@ -25,6 +24,7 @@ use Symfony\Component\Console\Output\OutputInterface;
use Symfony\Component\Console\Question\ChoiceQuestion;
use Symfony\Component\Console\Question\ConfirmationQuestion;
use Symfony\Component\Console\Question\Question;
use Symfony\Component\Console\Terminal;
/**
* Output decorator helpers for the Symfony Style Guide.
@ -50,7 +50,8 @@ class SymfonyStyle extends OutputStyle
$this->input = $input;
$this->bufferedOutput = new BufferedOutput($output->getVerbosity(), false, clone $output->getFormatter());
// Windows cmd wraps lines as soon as the terminal width is reached, whether there are following chars or not.
$this->lineLength = min($this->getTerminalWidth() - (int) (DIRECTORY_SEPARATOR === '\\'), self::MAX_LINE_LENGTH);
$width = (new Terminal())->getWidth() ?: self::MAX_LINE_LENGTH;
$this->lineLength = min($width - (int) (DIRECTORY_SEPARATOR === '\\'), self::MAX_LINE_LENGTH);
parent::__construct($output);
}
@ -401,14 +402,6 @@ class SymfonyStyle extends OutputStyle
return $this->progressBar;
}
private function getTerminalWidth()
{
$application = new Application();
$dimensions = $application->getTerminalDimensions();
return $dimensions[0] ?: self::MAX_LINE_LENGTH;
}
private function autoPrependBlock()
{
$chars = substr(str_replace(PHP_EOL, "\n", $this->bufferedOutput->fetch()), -2);

View File

@ -0,0 +1,135 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Symfony\Component\Console;
class Terminal
{
private static $width;
private static $height;
/**
* Gets the terminal width.
*
* @return int|null
*/
public function getWidth()
{
if ($width = trim(getenv('COLUMNS'))) {
return (int) $width;
}
if (null === self::$width) {
self::initDimensions();
}
return self::$width;
}
/**
* Gets the terminal height.
*
* @return int|null
*/
public function getHeight()
{
if ($height = trim(getenv('LINES'))) {
return (int) $height;
}
if (null === self::$height) {
self::initDimensions();
}
return self::$height;
}
private static function initDimensions()
{
if ('\\' === DIRECTORY_SEPARATOR) {
if (preg_match('/^(\d+)x(\d+)(?: \((\d+)x(\d+)\))?$/', trim(getenv('ANSICON')), $matches)) {
// extract [w, H] from "wxh (WxH)"
// or [w, h] from "wxh"
self::$width = (int) $matches[1];
self::$height = isset($matches[4]) ? (int) $matches[4] : (int) $matches[2];
} elseif (null != $dimensions = self::getConsoleMode()) {
// extract [w, h] from "wxh"
self::$width = (int) $dimensions[0];
self::$height = (int) $dimensions[1];
}
} elseif ($sttyString = self::getSttyColumns()) {
if (preg_match('/rows.(\d+);.columns.(\d+);/i', $sttyString, $matches)) {
// extract [w, h] from "rows h; columns w;"
self::$width = (int) $matches[2];
self::$height = (int) $matches[1];
} elseif (preg_match('/;.(\d+).rows;.(\d+).columns/i', $sttyString, $matches)) {
// extract [w, h] from "; h rows; w columns"
self::$width = (int) $matches[2];
self::$height = (int) $matches[1];
}
}
}
/**
* Runs and parses mode CON if it's available, suppressing any error output.
*
* @return array|null An array composed of the width and the height or null if it could not be parsed
*/
private static function getConsoleMode()
{
if (!function_exists('proc_open')) {
return;
}
$descriptorspec = array(
1 => array('pipe', 'w'),
2 => array('pipe', 'w'),
);
$process = proc_open('mode CON', $descriptorspec, $pipes, null, null, array('suppress_errors' => true));
if (is_resource($process)) {
$info = stream_get_contents($pipes[1]);
fclose($pipes[1]);
fclose($pipes[2]);
proc_close($process);
if (preg_match('/--------+\r?\n.+?(\d+)\r?\n.+?(\d+)\r?\n/', $info, $matches)) {
return array((int) $matches[2], (int) $matches[1]);
}
}
}
/**
* Runs and parses stty -a if it's available, suppressing any error output.
*
* @return string
*/
private static function getSttyColumns()
{
if (!function_exists('proc_open')) {
return;
}
$descriptorspec = array(
1 => array('pipe', 'w'),
2 => array('pipe', 'w'),
);
$process = proc_open('stty -a | grep columns', $descriptorspec, $pipes, null, null, array('suppress_errors' => true));
if (is_resource($process)) {
$info = stream_get_contents($pipes[1]);
fclose($pipes[1]);
fclose($pipes[2]);
proc_close($process);
return $info;
}
}
}

View File

@ -476,11 +476,9 @@ class ApplicationTest extends \PHPUnit_Framework_TestCase
public function testSetCatchExceptions()
{
$application = $this->getMock('Symfony\Component\Console\Application', array('getTerminalWidth'));
$application = new Application();
$application->setAutoExit(false);
$application->expects($this->any())
->method('getTerminalWidth')
->will($this->returnValue(120));
putenv('COLUMNS=120');
$tester = new ApplicationTester($application);
$application->setCatchExceptions(true);
@ -514,11 +512,9 @@ class ApplicationTest extends \PHPUnit_Framework_TestCase
public function testRenderException()
{
$application = $this->getMock('Symfony\Component\Console\Application', array('getTerminalWidth'));
$application = new Application();
$application->setAutoExit(false);
$application->expects($this->any())
->method('getTerminalWidth')
->will($this->returnValue(120));
putenv('COLUMNS=120');
$tester = new ApplicationTester($application);
$tester->run(array('command' => 'foo'), array('decorated' => false, 'capture_stderr_separately' => true));
@ -546,24 +542,21 @@ class ApplicationTest extends \PHPUnit_Framework_TestCase
$tester->run(array('command' => 'foo3:bar'), array('decorated' => true, 'capture_stderr_separately' => true));
$this->assertStringEqualsFile(self::$fixturesPath.'/application_renderexception3decorated.txt', $tester->getErrorOutput(true), '->renderException() renders a pretty exceptions with previous exceptions');
$application = $this->getMock('Symfony\Component\Console\Application', array('getTerminalWidth'));
$application = new Application();
$application->setAutoExit(false);
$application->expects($this->any())
->method('getTerminalWidth')
->will($this->returnValue(32));
putenv('COLUMNS=32');
$tester = new ApplicationTester($application);
$tester->run(array('command' => 'foo'), array('decorated' => false, 'capture_stderr_separately' => true));
$this->assertStringEqualsFile(self::$fixturesPath.'/application_renderexception4.txt', $tester->getErrorOutput(true), '->renderException() wraps messages when they are bigger than the terminal');
putenv('COLUMNS=120');
}
public function testRenderExceptionWithDoubleWidthCharacters()
{
$application = $this->getMock('Symfony\Component\Console\Application', array('getTerminalWidth'));
$application = new Application();
$application->setAutoExit(false);
$application->expects($this->any())
->method('getTerminalWidth')
->will($this->returnValue(120));
putenv('COLUMNS=120');
$application->register('foo')->setCode(function () {
throw new \Exception('エラーメッセージ');
});
@ -575,17 +568,16 @@ class ApplicationTest extends \PHPUnit_Framework_TestCase
$tester->run(array('command' => 'foo'), array('decorated' => true, 'capture_stderr_separately' => true));
$this->assertStringEqualsFile(self::$fixturesPath.'/application_renderexception_doublewidth1decorated.txt', $tester->getErrorOutput(true), '->renderException() renders a pretty exceptions with previous exceptions');
$application = $this->getMock('Symfony\Component\Console\Application', array('getTerminalWidth'));
$application = new Application();
$application->setAutoExit(false);
$application->expects($this->any())
->method('getTerminalWidth')
->will($this->returnValue(32));
putenv('COLUMNS=32');
$application->register('foo')->setCode(function () {
throw new \Exception('コマンドの実行中にエラーが発生しました。');
});
$tester = new ApplicationTester($application);
$tester->run(array('command' => 'foo'), array('decorated' => false, 'capture_stderr_separately' => true));
$this->assertStringEqualsFile(self::$fixturesPath.'/application_renderexception_doublewidth2.txt', $tester->getErrorOutput(true), '->renderException() wraps messages when they are bigger than the terminal');
putenv('COLUMNS=120');
}
public function testRun()
@ -1023,6 +1015,9 @@ class ApplicationTest extends \PHPUnit_Framework_TestCase
$this->assertEquals('some test value', $extraValue);
}
/**
* @group legacy
*/
public function testTerminalDimensions()
{
$application = new Application();

View File

@ -517,6 +517,24 @@ class ProgressBarTest extends \PHPUnit_Framework_TestCase
);
}
public function testWithSmallScreen()
{
$output = $this->getOutputStream();
$bar = new ProgressBar($output);
putenv('COLUMNS=12');
$bar->start();
$bar->advance();
putenv('COLUMNS=120');
rewind($output->getStream());
$this->assertEquals(
$this->generateOutput(' 0 [>---]').
$this->generateOutput(' 1 [->--]'),
stream_get_contents($output->getStream())
);
}
public function testAddingPlaceholderFormatter()
{
ProgressBar::setPlaceholderFormatterDefinition('remaining_steps', function (ProgressBar $bar) {
@ -560,6 +578,8 @@ class ProgressBarTest extends \PHPUnit_Framework_TestCase
public function testAnsiColorsAndEmojis()
{
putenv('COLUMNS=156');
$bar = new ProgressBar($output = $this->getOutputStream(), 15);
ProgressBar::setPlaceholderFormatterDefinition('memory', function (ProgressBar $bar) {
static $i = 0;
@ -575,10 +595,6 @@ class ProgressBarTest extends \PHPUnit_Framework_TestCase
$bar->setMessage('Starting the demo... fingers crossed', 'title');
$bar->start();
$bar->setMessage('Looks good to me...', 'title');
$bar->advance(4);
$bar->setMessage('Thanks, bye', 'title');
$bar->finish();
rewind($output->getStream());
$this->assertEquals(
@ -586,12 +602,32 @@ class ProgressBarTest extends \PHPUnit_Framework_TestCase
" \033[44;37m Starting the demo... fingers crossed \033[0m\n".
' 0/15 '.$progress.str_repeat($empty, 26)." 0%\n".
" \xf0\x9f\x8f\x81 < 1 sec \033[44;37m 0 B \033[0m"
).
),
stream_get_contents($output->getStream())
);
ftruncate($output->getStream(), 0);
rewind($output->getStream());
$bar->setMessage('Looks good to me...', 'title');
$bar->advance(4);
rewind($output->getStream());
$this->assertEquals(
$this->generateOutput(
" \033[44;37m Looks good to me... \033[0m\n".
' 4/15 '.str_repeat($done, 7).$progress.str_repeat($empty, 19)." 26%\n".
" \xf0\x9f\x8f\x81 < 1 sec \033[41;37m 97 KiB \033[0m"
).
),
stream_get_contents($output->getStream())
);
ftruncate($output->getStream(), 0);
rewind($output->getStream());
$bar->setMessage('Thanks, bye', 'title');
$bar->finish();
rewind($output->getStream());
$this->assertEquals(
$this->generateOutput(
" \033[44;37m Thanks, bye \033[0m\n".
' 15/15 '.str_repeat($done, 28)." 100%\n".
@ -599,6 +635,7 @@ class ProgressBarTest extends \PHPUnit_Framework_TestCase
),
stream_get_contents($output->getStream())
);
putenv('COLUMNS=120');
}
public function testSetFormat()

View File

@ -0,0 +1,32 @@
<?php
/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/
namespace Symfony\Component\Console\Tests;
use Symfony\Component\Console\Terminal;
class TerminalTest extends \PHPUnit_Framework_TestCase
{
public function test()
{
putenv('COLUMNS=100');
putenv('LINES=50');
$terminal = new Terminal();
$this->assertSame(100, $terminal->getWidth());
$this->assertSame(50, $terminal->getHeight());
putenv('COLUMNS=120');
putenv('LINES=60');
$terminal = new Terminal();
$this->assertSame(120, $terminal->getWidth());
$this->assertSame(60, $terminal->getHeight());
}
}