-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[Console] Console output formatter improvement #45318
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
Changes from all commits
b6f9290
d8b9fd2
9c25f32
3d9eb48
b67a95e
c4a4138
9b799dd
e1470fd
7b5906d
cd0a65f
663330f
4616f5c
8e06868
7a1ec1c
d6de585
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ | |
| namespace Symfony\Component\Console\Formatter; | ||
|
|
||
| use Symfony\Component\Console\Exception\InvalidArgumentException; | ||
| use Symfony\Component\Console\Helper\OutputWrapperInterface; | ||
|
|
||
| /** | ||
| * Formatter class for console output. | ||
|
|
@@ -140,8 +141,8 @@ public function formatAndWrap(?string $message, int $width) | |
| { | ||
| $offset = 0; | ||
| $output = ''; | ||
| $openTagRegex = '[a-z](?:[^\\\\<>]*+ | \\\\.)*'; | ||
| $closeTagRegex = '[a-z][^<>]*+'; | ||
| $openTagRegex = OutputWrapperInterface::TAG_OPEN_REGEX_SEGMENT; | ||
| $closeTagRegex = OutputWrapperInterface::TAG_CLOSE_REGEX_SEGMENT; | ||
| $currentLineLength = 0; | ||
| preg_match_all("#<(($openTagRegex) | /($closeTagRegex)?)>#ix", $message, $matches, \PREG_OFFSET_CAPTURE); | ||
| foreach ($matches[0] as $i => $match) { | ||
|
|
@@ -157,7 +158,7 @@ public function formatAndWrap(?string $message, int $width) | |
| $offset = $pos + \strlen($text); | ||
|
|
||
| // opening tag? | ||
| if ($open = '/' != $text[1]) { | ||
| if ($open = ('/' !== $text[1])) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no need for the brackets
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added it because PHPStorm recommended this change, and I agree, it is more readable what is happening, even if you are totally right, the original version worked. If you want, I can 'undo' this, up to you. But now you know why I did it.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's revert as it's not needed. |
||
| $tag = $matches[1][$i][0]; | ||
| } else { | ||
| $tag = $matches[3][$i][0] ?? ''; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| <?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\Helper; | ||
|
|
||
| /** | ||
| * Simple output wrapper for "tagged outputs" instead of wordwrap(). This solution is based on a StackOverflow | ||
| * answer: https://stackoverflow.com/a/20434776/1476819 from user557597 (alias SLN). | ||
| * | ||
| * (?: | ||
| * # -- Words/Characters | ||
| * ( # (1 start) | ||
| * (?> # Atomic Group - Match words with valid breaks | ||
| * .{1,16} # 1-N characters | ||
| * # Followed by one of 4 prioritized, non-linebreak whitespace | ||
| * (?: # break types: | ||
| * (?<= [^\S\r\n] ) # 1. - Behind a non-linebreak whitespace | ||
| * [^\S\r\n]? # ( optionally accept an extra non-linebreak whitespace ) | ||
| * | (?= \r? \n ) # 2. - Ahead a linebreak | ||
| * | $ # 3. - EOS | ||
| * | [^\S\r\n] # 4. - Accept an extra non-linebreak whitespace | ||
| * ) | ||
| * ) # End atomic group | ||
| * | | ||
| * .{1,16} # No valid word breaks, just break on the N'th character | ||
| * ) # (1 end) | ||
| * (?: \r? \n )? # Optional linebreak after Words/Characters | ||
| * | | ||
| * # -- Or, Linebreak | ||
| * (?: \r? \n | $ ) # Stand alone linebreak or at EOS | ||
| * ) | ||
| * | ||
| * @author Krisztián Ferenczi <ferenczi.krisztian@gmail.com> | ||
| * | ||
| * @see https://stackoverflow.com/a/20434776/1476819 | ||
| */ | ||
| final class OutputWrapper implements OutputWrapperInterface | ||
| { | ||
| private const URL_PATTERN = 'https?://\S+'; | ||
|
|
||
| private bool $allowCutUrls = false; | ||
|
|
||
| public function isAllowCutUrls(): bool | ||
| { | ||
| return $this->allowCutUrls; | ||
| } | ||
|
|
||
| public function setAllowCutUrls(bool $allowCutUrls) | ||
| { | ||
| $this->allowCutUrls = $allowCutUrls; | ||
|
|
||
| return $this; | ||
| } | ||
|
|
||
| public function wrap(string $text, int $width, string $break = "\n"): string | ||
| { | ||
| if (!$width) { | ||
| return $text; | ||
| } | ||
|
|
||
| $tagPattern = sprintf('<(?:(?:%s)|/(?:%s)?)>', OutputWrapperInterface::TAG_OPEN_REGEX_SEGMENT, OutputWrapperInterface::TAG_CLOSE_REGEX_SEGMENT); | ||
| $limitPattern = "{1,$width}"; | ||
| $patternBlocks = [$tagPattern]; | ||
| if (!$this->allowCutUrls) { | ||
| $patternBlocks[] = self::URL_PATTERN; | ||
| } | ||
| $patternBlocks[] = '.'; | ||
| $blocks = implode('|', $patternBlocks); | ||
| $rowPattern = "(?:$blocks)$limitPattern"; | ||
| $pattern = sprintf('#(?:((?>(%1$s)((?<=[^\S\r\n])[^\S\r\n]?|(?=\r?\n)|$|[^\S\r\n]))|(%1$s))(?:\r?\n)?|(?:\r?\n|$))#imux', $rowPattern); | ||
| $output = rtrim(preg_replace($pattern, '\\1'.$break, $text), $break); | ||
|
|
||
| return str_replace(' '.$break, $break, $output); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| <?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\Helper; | ||
|
|
||
| interface OutputWrapperInterface | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's remove this interface. |
||
| { | ||
| public const TAG_OPEN_REGEX_SEGMENT = '[a-z](?:[^\\\\<>]*+ | \\\\.)*'; | ||
| public const TAG_CLOSE_REGEX_SEGMENT = '[a-z][^<>]*+'; | ||
|
|
||
| /** | ||
| * @param positive-int|0 $width | ||
| */ | ||
| public function wrap(string $text, int $width, string $break = "\n"): string; | ||
fchris82 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,8 @@ | |
| use Symfony\Component\Console\Exception\RuntimeException; | ||
| use Symfony\Component\Console\Formatter\OutputFormatter; | ||
| use Symfony\Component\Console\Helper\Helper; | ||
| use Symfony\Component\Console\Helper\OutputWrapper; | ||
| use Symfony\Component\Console\Helper\OutputWrapperInterface; | ||
| use Symfony\Component\Console\Helper\ProgressBar; | ||
| use Symfony\Component\Console\Helper\SymfonyQuestionHelper; | ||
| use Symfony\Component\Console\Helper\Table; | ||
|
|
@@ -44,18 +46,32 @@ class SymfonyStyle extends OutputStyle | |
| private ProgressBar $progressBar; | ||
| private int $lineLength; | ||
| private TrimmedBufferOutput $bufferedOutput; | ||
| private OutputWrapperInterface $outputWrapper; | ||
|
|
||
| public function __construct(InputInterface $input, OutputInterface $output) | ||
| public function __construct(InputInterface $input, OutputInterface $output, OutputWrapperInterface $outputWrapper = null) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's remove this argument.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the argument and the setters are removed, the property |
||
| { | ||
| $this->input = $input; | ||
| $this->bufferedOutput = new TrimmedBufferOutput(\DIRECTORY_SEPARATOR === '\\' ? 4 : 2, $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. | ||
| $width = (new Terminal())->getWidth() ?: self::MAX_LINE_LENGTH; | ||
| $this->lineLength = min($width - (int) (\DIRECTORY_SEPARATOR === '\\'), self::MAX_LINE_LENGTH); | ||
| $this->outputWrapper = $outputWrapper ?: new OutputWrapper(); | ||
|
|
||
| parent::__construct($this->output = $output); | ||
| } | ||
|
|
||
| public function getOutputWrapper(): OutputWrapperInterface | ||
| { | ||
| return $this->outputWrapper; | ||
| } | ||
|
|
||
| public function setOutputWrapper(OutputWrapperInterface $outputWrapper) | ||
| { | ||
| $this->outputWrapper = $outputWrapper; | ||
|
|
||
| return $this; | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can also remove these gettter/setter methods. |
||
|
|
||
| /** | ||
| * Formats a message as a block of text. | ||
| */ | ||
|
|
@@ -453,7 +469,7 @@ private function createBlock(iterable $messages, string $type = null, string $st | |
|
|
||
| if (null !== $type) { | ||
| $type = sprintf('[%s] ', $type); | ||
| $indentLength = \strlen($type); | ||
| $indentLength = Helper::width($type); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It doesn't look directly related to your change no? Asking this in case as it feels like there is a missing test for it and I don't know if your new tests covers it or not
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One of my new tests covers it: https://github.com/symfony/symfony/pull/45318/files#diff-6ef90609877201be87f303aa59301e30a147c322dffba8f96f3e27655a5bcf0fR11 |
||
| $lineIndentation = str_repeat(' ', $indentLength); | ||
| } | ||
|
|
||
|
|
@@ -463,12 +479,14 @@ private function createBlock(iterable $messages, string $type = null, string $st | |
| $message = OutputFormatter::escape($message); | ||
| } | ||
|
|
||
| $decorationLength = Helper::width($message) - Helper::width(Helper::removeDecoration($this->getFormatter(), $message)); | ||
| $messageLineLength = min($this->lineLength - $prefixLength - $indentLength + $decorationLength, $this->lineLength); | ||
| $messageLines = explode(\PHP_EOL, wordwrap($message, $messageLineLength, \PHP_EOL, true)); | ||
| foreach ($messageLines as $messageLine) { | ||
| $lines[] = $messageLine; | ||
| } | ||
| $lines = array_merge( | ||
| $lines, | ||
| explode(\PHP_EOL, $this->outputWrapper->wrap( | ||
| $message, | ||
| $this->lineLength - $prefixLength - $indentLength, | ||
| \PHP_EOL | ||
| )) | ||
| ); | ||
|
|
||
| if (\count($messages) > 1 && $key < \count($messages) - 1) { | ||
| $lines[] = ''; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| <?php | ||
|
|
||
| use Symfony\Component\Console\Input\InputInterface; | ||
| use Symfony\Component\Console\Output\OutputInterface; | ||
| use Symfony\Component\Console\Style\SymfonyStyle; | ||
|
|
||
| // ensure that nested tags have no effect on the color of the '//' prefix | ||
| return function (InputInterface $input, OutputInterface $output) { | ||
| $output->setDecorated(true); | ||
| $output = new SymfonyStyle($input, $output); | ||
| $output->block( | ||
| 'Árvíztűrőtükörfúrógép Lorem ipsum dolor sit <comment>amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur.</comment> Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum', | ||
| '★', // UTF-8 star! | ||
| null, | ||
| '<fg=default;bg=default> ║ </>', // UTF-8 double line! | ||
| false, | ||
| false | ||
| ); | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
|
|
||
| [39;49m // [39;49mLorem ipsum dolor sit [33mamet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore [39m | ||
| [39;49m // [39;49m[33mmagna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo [39m | ||
| [39;49m // [39;49m[33mconsequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla [39m | ||
| [39;49m // [39;49m[33mpariatur.[39m Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id | ||
| [39;49m // [39;49mest laborum | ||
| [39;49m // [39;49mÁrvíztűrőtükörfúrógép 🎼 Lorem ipsum dolor sit [33m💕 amet, consectetur adipisicing elit, sed do eiusmod tempor incididu[39m | ||
| [39;49m // [39;49m[33mlabore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex[39m | ||
| [39;49m // [39;49m[33mea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla [39m | ||
| [39;49m // [39;49m[33mpariatur.[39m Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est | ||
| [39;49m // [39;49mlaborum | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
|
|
||
| [39;49m ║ [39;49m[★] Árvíztűrőtükörfúrógép Lorem ipsum dolor sit [33mamet, consectetur adipisicing elit, sed do eiusmod tempor incididunt [39m | ||
| [39;49m ║ [39;49m[33m ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut [39m | ||
| [39;49m ║ [39;49m[33m aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu [39m | ||
| [39;49m ║ [39;49m[33m fugiat nulla pariatur.[39m Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit | ||
| [39;49m ║ [39;49m anim id est laborum | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| <?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\Helper; | ||
|
|
||
| use PHPUnit\Framework\TestCase; | ||
| use Symfony\Component\Console\Helper\OutputWrapper; | ||
|
|
||
| class OutputWrapperTest extends TestCase | ||
chalasr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| { | ||
| /** | ||
| * @dataProvider textProvider | ||
| */ | ||
| public function testBasicWrap(string $text, int $width, ?bool $allowCutUrls, string $expected) | ||
| { | ||
| $wrapper = new OutputWrapper(); | ||
| if (\is_bool($allowCutUrls)) { | ||
| $wrapper->setAllowCutUrls($allowCutUrls); | ||
| } | ||
| $result = $wrapper->wrap($text, $width); | ||
| $this->assertEquals($expected, $result); | ||
| } | ||
|
|
||
| public static function textProvider(): iterable | ||
| { | ||
| $baseTextWithUtf8AndUrl = 'Árvíztűrőtükörfúrógép https://github.com/symfony/symfony Lorem ipsum <comment>dolor</comment> sit amet, consectetur adipiscing elit. Praesent vestibulum nulla quis urna maximus porttitor. Donec ullamcorper risus at <error>libero ornare</error> efficitur.'; | ||
|
|
||
| yield 'Default URL cut' => [ | ||
| $baseTextWithUtf8AndUrl, | ||
| 20, | ||
| null, | ||
| <<<'EOS' | ||
| Árvíztűrőtükörfúrógé | ||
| p https://github.com/symfony/symfony Lorem ipsum | ||
| <comment>dolor</comment> sit amet, | ||
| consectetur | ||
| adipiscing elit. | ||
| Praesent vestibulum | ||
| nulla quis urna | ||
| maximus porttitor. | ||
| Donec ullamcorper | ||
| risus at <error>libero | ||
| ornare</error> efficitur. | ||
| EOS, | ||
| ]; | ||
|
|
||
| yield 'Allow URL cut' => [ | ||
| $baseTextWithUtf8AndUrl, | ||
| 20, | ||
| true, | ||
| <<<'EOS' | ||
| Árvíztűrőtükörfúrógé | ||
| p | ||
| https://github.com/s | ||
| ymfony/symfony Lorem | ||
| ipsum <comment>dolor</comment> sit | ||
| amet, consectetur | ||
| adipiscing elit. | ||
| Praesent vestibulum | ||
| nulla quis urna | ||
| maximus porttitor. | ||
| Donec ullamcorper | ||
| risus at <error>libero | ||
| ornare</error> efficitur. | ||
| EOS, | ||
| ]; | ||
| } | ||
| } | ||
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.
I'm not convinced we need to open for extensibility here. Fight me if you want to keep it :)
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.
I added this because the behavior you need can depend on your console, your language, and other things. I added a really basic solution here, but people may want different behavior, or they want to split the long URL-s on their way. Eg: originally I just wanted to solve the text wrapping but it turned out, URL wrapping is another issue.
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.
I agree with Nicolas, we should not make it extensible.