Skip to content

Commit a29a60d

Browse files
fabpotfabpot
committed
bug symfony#10714 [Console]Improve formatter for double-width character (denkiryokuhatsuden)
This PR was squashed before being merged into the 2.3 branch (closes symfony#10714). Discussion ---------- [Console]Improve formatter for double-width character | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | yes for one that expecting current broken output | Deprecations? | no | Fixed tickets | | Tests pass? | yes | License | MIT | Doc PR | EDIT: fixed the table above not to remove irrelevant line As mb_strlen just returns "how many chars in the string", formatting with double-width character is bit broken. The test I add is skipped when mbstring extension is not loaded. I'm afraid if some of you cannot properly display japanese string. (表示するテキスト just means "Some text to display") Commits ------- a52f41d [Console]Improve formatter for double-width character
2 parents 1a64fef + a52f41d commit a29a60d

File tree

7 files changed

+144
-29
lines changed

7 files changed

+144
-29
lines changed

‎src/Symfony/Component/Console/Application.php

Lines changed: 66 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ public function setDispatcher(EventDispatcherInterface $dispatcher)
9999
* @param InputInterface $input An Input instance
100100
* @param OutputInterface $output An Output instance
101101
*
102-
* @return int 0 if everything went fine, or an error code
102+
* @return int 0 if everything went fine, or an error code
103103
*
104104
* @throws \Exception When doRun returns Exception
105105
*
@@ -159,7 +159,7 @@ public function run(InputInterface $input = null, OutputInterface $output = null
159159
* @param InputInterface $input An Input instance
160160
* @param OutputInterface $output An Output instance
161161
*
162-
* @return int 0 if everything went fine, or an error code
162+
* @return int 0 if everything went fine, or an error code
163163
*/
164164
public function doRun(InputInterface $input, OutputInterface $output)
165165
{
@@ -270,7 +270,7 @@ public function getHelp()
270270
/**
271271
* Sets whether to catch exceptions or not during commands execution.
272272
*
273-
* @param bool $boolean Whether to catch exceptions or not during commands execution
273+
* @param bool $boolean Whether to catch exceptions or not during commands execution
274274
*
275275
* @api
276276
*/
@@ -282,7 +282,7 @@ public function setCatchExceptions($boolean)
282282
/**
283283
* Sets whether to automatically exit after a command execution or not.
284284
*
285-
* @param bool $boolean Whether to automatically exit after a command execution or not
285+
* @param bool $boolean Whether to automatically exit after a command execution or not
286286
*
287287
* @api
288288
*/
@@ -449,7 +449,7 @@ public function get($name)
449449
*
450450
* @param string $name The command name or alias
451451
*
452-
* @return bool true if the command exists, false otherwise
452+
* @return bool true if the command exists, false otherwise
453453
*
454454
* @api
455455
*/
@@ -674,8 +674,8 @@ public static function getAbbreviations($names)
674674
/**
675675
* Returns a text representation of the Application.
676676
*
677-
* @param string $namespace An optional namespace name
678-
* @param bool $raw Whether to return raw command list
677+
* @param string $namespace An optional namespace name
678+
* @param bool $raw Whether to return raw command list
679679
*
680680
* @return string A string representing the Application
681681
*
@@ -691,8 +691,8 @@ public function asText($namespace = null, $raw = false)
691691
/**
692692
* Returns an XML representation of the Application.
693693
*
694-
* @param string $namespace An optional namespace name
695-
* @param bool $asDom Whether to return a DOM or an XML string
694+
* @param string $namespace An optional namespace name
695+
* @param bool $asDom Whether to return a DOM or an XML string
696696
*
697697
* @return string|\DOMDocument An XML string representing the Application
698698
*
@@ -708,26 +708,16 @@ public function asXml($namespace = null, $asDom = false)
708708
/**
709709
* Renders a caught exception.
710710
*
711-
* @param \Exception $e An exception instance
711+
* @param \Exception $e An exception instance
712712
* @param OutputInterface $output An OutputInterface instance
713713
*/
714714
public function renderException($e, $output)
715715
{
716-
$strlen = function ($string) {
717-
if (!function_exists('mb_strlen')) {
718-
return strlen($string);
719-
}
720-
721-
if (false === $encoding = mb_detect_encoding($string)) {
722-
return strlen($string);
723-
}
724-
725-
return mb_strlen($string, $encoding);
726-
};
727-
728716
do {
729717
$title = sprintf(' [%s] ', get_class($e));
730-
$len = $strlen($title);
718+
719+
$len = $this->stringWidth($title);
720+
731721
$width = $this->getTerminalWidth() ? $this->getTerminalWidth() - 1 : PHP_INT_MAX;
732722
// 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
733723
if (defined('HHVM_VERSION') && $width > 1 << 31) {
@@ -736,9 +726,9 @@ public function renderException($e, $output)
736726
$formatter = $output->getFormatter();
737727
$lines = array();
738728
foreach (preg_split('/\r?\n/', $e->getMessage()) as $line) {
739-
foreach (str_split($line, $width - 4) as $line) {
729+
foreach ($this->splitStringByWidth($line, $width - 4) as $line) {
740730
// pre-format lines to get the right string length
741-
$lineLength = $strlen(preg_replace('/\[[^m]*m/', '', $formatter->format($line))) + 4;
731+
$lineLength = $this->stringWidth(preg_replace('/\[[^m]*m/', '', $formatter->format($line))) + 4;
742732
$lines[] = array($line, $lineLength);
743733

744734
$len = max($lineLength, $len);
@@ -747,7 +737,7 @@ public function renderException($e, $output)
747737

748738
$messages = array('', '');
749739
$messages[] = $emptyLine = $formatter->format(sprintf('<error>%s</error>', str_repeat(' ', $len)));
750-
$messages[] = $formatter->format(sprintf('<error>%s%s</error>', $title, str_repeat(' ', max(0, $len - $strlen($title)))));
740+
$messages[] = $formatter->format(sprintf('<error>%s%s</error>', $title, str_repeat(' ', max(0, $len - $this->stringWidth($title)))));
751741
foreach ($lines as $line) {
752742
$messages[] = $formatter->format(sprintf('<error> %s %s</error>', $line[0], str_repeat(' ', $len - $line[1])));
753743
}
@@ -893,7 +883,7 @@ protected function configureIO(InputInterface $input, OutputInterface $output)
893883
* @param InputInterface $input An Input instance
894884
* @param OutputInterface $output An Output instance
895885
*
896-
* @return int 0 if everything went fine, or an error code
886+
* @return int 0 if everything went fine, or an error code
897887
*/
898888
protected function doRunCommand(Command $command, InputInterface $input, OutputInterface $output)
899889
{
@@ -1128,4 +1118,53 @@ private function findAlternatives($name, $collection, $abbrevs, $callback = null
11281118

11291119
return array_keys($alternatives);
11301120
}
1121+
1122+
private function stringWidth($string)
1123+
{
1124+
if (!function_exists('mb_strwidth')) {
1125+
return strlen($string);
1126+
}
1127+
1128+
if (false === $encoding = mb_detect_encoding($string)) {
1129+
return strlen($string);
1130+
}
1131+
1132+
return mb_strwidth($string, $encoding);
1133+
}
1134+
1135+
private function splitStringByWidth($string, $width)
1136+
{
1137+
// str_split is not suitable for multi-byte characters, we should use preg_split to get char array properly.
1138+
// additionally, array_slice() is not enough as some character has doubled width.
1139+
// we need a function to split string not by character count but by string width
1140+
1141+
if (!function_exists('mb_strwidth')) {
1142+
return str_split($string, $width);
1143+
}
1144+
1145+
if (false === $encoding = mb_detect_encoding($string)) {
1146+
return str_split($string, $width);
1147+
}
1148+
1149+
$utf8String = mb_convert_encoding($string, 'utf8', $encoding);
1150+
$lines = array();
1151+
$line = '';
1152+
foreach (preg_split('//u', $utf8String) as $char) {
1153+
// test if $char could be appended to current line
1154+
if (mb_strwidth($line.$char) <= $width) {
1155+
$line .= $char;
1156+
continue;
1157+
}
1158+
// if not, push current line to array and make new line
1159+
$lines[] = str_pad($line, $width);
1160+
$line = $char;
1161+
}
1162+
if (strlen($line)) {
1163+
$lines[] = count($lines) ? str_pad($line, $width) : $line;
1164+
}
1165+
1166+
mb_convert_variables($encoding, 'utf8', $lines);
1167+
1168+
return $lines;
1169+
}
11311170
}

‎src/Symfony/Component/Console/Helper/Helper.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,14 +49,14 @@ public function getHelperSet()
4949
*/
5050
protected function strlen($string)
5151
{
52-
if (!function_exists('mb_strlen')) {
52+
if (!function_exists('mb_strwidth')) {
5353
return strlen($string);
5454
}
5555

5656
if (false === $encoding = mb_detect_encoding($string)) {
5757
return strlen($string);
5858
}
5959

60-
return mb_strlen($string, $encoding);
60+
return mb_strwidth($string, $encoding);
6161
}
6262
}

‎src/Symfony/Component/Console/Tests/ApplicationTest.php

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,33 @@ public function testRenderException()
469469
$this->assertStringEqualsFile(self::$fixturesPath.'/application_renderexception4.txt', $tester->getDisplay(true), '->renderException() wraps messages when they are bigger than the terminal');
470470
}
471471

472+
public function testRenderExceptionWithDoubleWidthCharacters()
473+
{
474+
$application = $this->getMock('Symfony\Component\Console\Application', array('getTerminalWidth'));
475+
$application->setAutoExit(false);
476+
$application->expects($this->any())
477+
->method('getTerminalWidth')
478+
->will($this->returnValue(120));
479+
$application->register('foo')->setCode(function () {throw new \Exception('エラーメッセージ');});
480+
$tester = new ApplicationTester($application);
481+
482+
$tester->run(array('command' => 'foo'), array('decorated' => false));
483+
$this->assertStringEqualsFile(self::$fixturesPath.'/application_renderexception_doublewidth1.txt', $tester->getDisplay(true), '->renderException() renderes a pretty exceptions with previous exceptions');
484+
485+
$tester->run(array('command' => 'foo'), array('decorated' => true));
486+
$this->assertStringEqualsFile(self::$fixturesPath.'/application_renderexception_doublewidth1decorated.txt', $tester->getDisplay(true), '->renderException() renderes a pretty exceptions with previous exceptions');
487+
488+
$application = $this->getMock('Symfony\Component\Console\Application', array('getTerminalWidth'));
489+
$application->setAutoExit(false);
490+
$application->expects($this->any())
491+
->method('getTerminalWidth')
492+
->will($this->returnValue(32));
493+
$application->register('foo')->setCode(function () {throw new \Exception('コマンドの実行中にエラーが発生しました。');});
494+
$tester = new ApplicationTester($application);
495+
$tester->run(array('command' => 'foo'), array('decorated' => false));
496+
$this->assertStringEqualsFile(self::$fixturesPath.'/application_renderexception_doublewidth2.txt', $tester->getDisplay(true), '->renderException() wraps messages when they are bigger than the terminal');
497+
}
498+
472499
public function testRun()
473500
{
474501
$application = new Application();

‎src/Symfony/Component/Console/Tests/Fixtures/application_renderexception_doublewidth1.txt

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
2+
3+
4+
[Exception]
5+
エラーメッセージ
6+
7+
8+
9+
foo
10+
11+

‎src/Symfony/Component/Console/Tests/Fixtures/application_renderexception_doublewidth1decorated.txt

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
2+
3+
 
4+
 [Exception] 
5+
 エラーメッセージ 
6+
 
7+
8+
9+
foo
10+
11+

‎src/Symfony/Component/Console/Tests/Fixtures/application_renderexception_doublewidth2.txt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
2+
3+
4+
[Exception]
5+
コマンドの実行中にエラーが
6+
発生しました。
7+
8+
9+
10+
foo
11+
12+

‎src/Symfony/Component/Console/Tests/Helper/FormatterHelperTest.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,21 @@ public function testFormatBlockWithDiacriticLetters()
6969
);
7070
}
7171

72+
public function testFormatBlockWithDoubleWidthDiacriticLetters()
73+
{
74+
if (!extension_loaded('mbstring')) {
75+
$this->markTestSkipped('This test requires mbstring to work.');
76+
}
77+
$formatter = new FormatterHelper();
78+
$this->assertEquals(
79+
'<error> </error>'."\n" .
80+
'<error> 表示するテキスト </error>'."\n" .
81+
'<error> </error>',
82+
$formatter->formatBlock('表示するテキスト', 'error', true),
83+
'::formatBlock() formats a message in a block'
84+
);
85+
}
86+
7287
public function testFormatBlockLGEscaping()
7388
{
7489
$formatter = new FormatterHelper();

0 commit comments

Comments
 (0)