Skip to content

[Console] Fix linewraps in OutputFormatter #51223

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

Merged
merged 1 commit into from
Aug 14, 2023
Merged

[Console] Fix linewraps in OutputFormatter #51223

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 10 additions & 1 deletion src/Symfony/Component/Console/Formatter/OutputFormatter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@

use Symfony\Component\Console\Exception\InvalidArgumentException;

use function Symfony\Component\String\b;

/**
* Formatter class for console output.
*
Expand Down Expand Up @@ -258,7 +260,7 @@ private function applyCurrentStyle(string $text, string $current, int $width, in
}

preg_match('~(\\n)$~', $text, $matches);
$text = $prefix.preg_replace('~([^\\n]{'.$width.'})\\ *~', "\$1\n", $text);
$text = $prefix.$this->addLineBreaks($text, $width);
$text = rtrim($text, "\n").($matches[1] ?? '');

if (!$currentLineLength && '' !== $current && "\n" !== substr($current, -1)) {
Expand All @@ -282,4 +284,11 @@ private function applyCurrentStyle(string $text, string $current, int $width, in

return implode("\n", $lines);
}

private function addLineBreaks(string $text, int $width): string
{
$encoding = mb_detect_encoding($text, null, true) ?: 'UTF-8';

return b($text)->toCodePointString($encoding)->wordwrap($width, "\n", true)->toByteString($encoding);
}
}
14 changes: 8 additions & 6 deletions src/Symfony/Component/Console/Tests/Formatter/OutputFormatterTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,21 +367,23 @@ public function testFormatAndWrap()
$formatter = new OutputFormatter(true);

$this->assertSame("fo\no\e[37;41mb\e[39;49m\n\e[37;41mar\e[39;49m\nba\nz", $formatter->formatAndWrap('foo<error>bar</error> baz', 2));
$this->assertSame("pr\ne \e[37;41m\e[39;49m\n\e[37;41mfo\e[39;49m\n\e[37;41mo \e[39;49m\n\e[37;41mba\e[39;49m\n\e[37;41mr \e[39;49m\n\e[37;41mba\e[39;49m\n\e[37;41mz\e[39;49m \npo\nst", $formatter->formatAndWrap('pre <error>foo bar baz</error> post', 2));
$this->assertSame("pr\ne \e[37;41m\e[39;49m\n\e[37;41mfo\e[39;49m\n\e[37;41mo\e[39;49m\n\e[37;41mba\e[39;49m\n\e[37;41mr\e[39;49m\n\e[37;41mba\e[39;49m\n\e[37;41mz\e[39;49m \npo\nst", $formatter->formatAndWrap('pre <error>foo bar baz</error> post', 2));
$this->assertSame("pre\e[37;41m\e[39;49m\n\e[37;41mfoo\e[39;49m\n\e[37;41mbar\e[39;49m\n\e[37;41mbaz\e[39;49m\npos\nt", $formatter->formatAndWrap('pre <error>foo bar baz</error> post', 3));
$this->assertSame("pre \e[37;41m\e[39;49m\n\e[37;41mfoo \e[39;49m\n\e[37;41mbar \e[39;49m\n\e[37;41mbaz\e[39;49m \npost", $formatter->formatAndWrap('pre <error>foo bar baz</error> post', 4));
$this->assertSame("pre \e[37;41mf\e[39;49m\n\e[37;41moo ba\e[39;49m\n\e[37;41mr baz\e[39;49m\npost", $formatter->formatAndWrap('pre <error>foo bar baz</error> post', 5));
$this->assertSame("pre \e[37;41m\e[39;49m\n\e[37;41mfoo\e[39;49m\n\e[37;41mbar\e[39;49m\n\e[37;41mbaz\e[39;49m \npost", $formatter->formatAndWrap('pre <error>foo bar baz</error> post', 4));
$this->assertSame("pre \e[37;41mf\e[39;49m\n\e[37;41moo\e[39;49m\n\e[37;41mbar\e[39;49m\n\e[37;41mbaz\e[39;49m p\nost", $formatter->formatAndWrap('pre <error>foo bar baz</error> post', 5));
$this->assertSame("Lore\nm \e[37;41mip\e[39;49m\n\e[37;41msum\e[39;49m \ndolo\nr \e[32msi\e[39m\n\e[32mt\e[39m am\net", $formatter->formatAndWrap('Lorem <error>ipsum</error> dolor <info>sit</info> amet', 4));
$this->assertSame("Lorem \e[37;41mip\e[39;49m\n\e[37;41msum\e[39;49m dolo\nr \e[32msit\e[39m am\net", $formatter->formatAndWrap('Lorem <error>ipsum</error> dolor <info>sit</info> amet', 8));
$this->assertSame("Lorem \e[37;41mipsum\e[39;49m dolor \e[32m\e[39m\n\e[32msit\e[39m, \e[37;41mamet\e[39;49m et \e[32mlauda\e[39m\n\e[32mntium\e[39m architecto", $formatter->formatAndWrap('Lorem <error>ipsum</error> dolor <info>sit</info>, <error>amet</error> et <info>laudantium</info> architecto', 18));

$formatter = new OutputFormatter();

$this->assertSame("fo\nob\nar\nba\nz", $formatter->formatAndWrap('foo<error>bar</error> baz', 2));
$this->assertSame("pr\ne \nfo\no \nba\nr \nba\nz \npo\nst", $formatter->formatAndWrap('pre <error>foo bar baz</error> post', 2));
$this->assertSame("pr\ne \nfo\no\nba\nr\nba\nz \npo\nst", $formatter->formatAndWrap('pre <error>foo bar baz</error> post', 2));
$this->assertSame("pre\nfoo\nbar\nbaz\npos\nt", $formatter->formatAndWrap('pre <error>foo bar baz</error> post', 3));
$this->assertSame("pre \nfoo \nbar \nbaz \npost", $formatter->formatAndWrap('pre <error>foo bar baz</error> post', 4));
$this->assertSame("pre f\noo ba\nr baz\npost", $formatter->formatAndWrap('pre <error>foo bar baz</error> post', 5));
$this->assertSame("pre \nfoo\nbar\nbaz \npost", $formatter->formatAndWrap('pre <error>foo bar baz</error> post', 4));
$this->assertSame("pre f\noo\nbar\nbaz p\nost", $formatter->formatAndWrap('pre <error>foo bar baz</error> post', 5));
$this->assertSame("Â rèälly\nlöng tîtlè\nthät cöüld\nnèêd\nmúltîplê\nlínès", $formatter->formatAndWrap('Â rèälly löng tîtlè thät cöüld nèêd múltîplê línès', 10));
$this->assertSame("Â rèälly\nlöng tîtlè\nthät cöüld\nnèêd\nmúltîplê\n línès", $formatter->formatAndWrap("Â rèälly löng tîtlè thät cöüld nèêd múltîplê\n línès", 10));
Copy link
Member

Choose a reason for hiding this comment

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

Are all these changes on purpose and behave as expected?

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, @nicolas-grekas (see comment) had the idea to use wordwrap from the string component for that. Wordwrap works a bit different with the line breaks than the regex before. now it breaks the line not in a word (or only if a word is longer than the full allowed length).
Example:
"Too long li\nne" => "Too long\nline"
IMHO this makes the output more readable.

$this->assertSame('', $formatter->formatAndWrap(null, 5));
}
}
Expand Down
75 changes: 39 additions & 36 deletions src/Symfony/Component/Console/Tests/Helper/TableTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,30 +118,30 @@ public static function renderProvider()
['ISBN', 'Title', 'Author'],
$books,
'compact',
<<<'TABLE'
ISBN Title Author
99921-58-10-7 Divine Comedy Dante Alighieri
9971-5-0210-0 A Tale of Two Cities Charles Dickens
960-425-059-0 The Lord of the Rings J. R. R. Tolkien
80-902734-1-6 And Then There Were None Agatha Christie

TABLE
implode("\n", [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is only because of the line "trim_trailing_whitespace = true" in .editorconfig.

'ISBN Title Author ',
'99921-58-10-7 Divine Comedy Dante Alighieri ',
'9971-5-0210-0 A Tale of Two Cities Charles Dickens ',
'960-425-059-0 The Lord of the Rings J. R. R. Tolkien ',
'80-902734-1-6 And Then There Were None Agatha Christie ',
'',
]),
],
[
['ISBN', 'Title', 'Author'],
$books,
'borderless',
<<<'TABLE'
=============== ========================== ==================
ISBN Title Author
=============== ========================== ==================
99921-58-10-7 Divine Comedy Dante Alighieri
9971-5-0210-0 A Tale of Two Cities Charles Dickens
960-425-059-0 The Lord of the Rings J. R. R. Tolkien
80-902734-1-6 And Then There Were None Agatha Christie
=============== ========================== ==================

TABLE
implode("\n", [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is only because of the line "trim_trailing_whitespace = true" in .editorconfig.

' =============== ========================== ================== ',
' ISBN Title Author ',
' =============== ========================== ================== ',
' 99921-58-10-7 Divine Comedy Dante Alighieri ',
' 9971-5-0210-0 A Tale of Two Cities Charles Dickens ',
' 960-425-059-0 The Lord of the Rings J. R. R. Tolkien ',
' 80-902734-1-6 And Then There Were None Agatha Christie ',
' =============== ========================== ================== ',
'',
]),
],
[
['ISBN', 'Title', 'Author'],
Expand Down Expand Up @@ -1378,12 +1378,14 @@ public function testColumnMaxWidths()

$expected =
<<<TABLE
+---------------+-------+------------+-----------------+
| Divine Comedy | A Tal | The Lord o | And Then There |
| | e of | f the Ring | Were None |
| | Two C | s | |
| | ities | | |
+---------------+-------+------------+-----------------+
+---------------+-------+----------+----------------+
| Divine Comedy | A | The Lord | And Then There |
| | Tale | of the | Were None |
| | of | Rings | |
| | Two | | |
| | Citie | | |
| | s | | |
+---------------+-------+----------+----------------+

TABLE;

Expand Down Expand Up @@ -1416,8 +1418,8 @@ public function testColumnMaxWidthsHeaders()
| Publication | Very long header with a lot of |
| | information |
+-------------+--------------------------------+
| 1954 | The Lord of the Rings, by J.R. |
| | R. Tolkien |
| 1954 | The Lord of the Rings, by |
| | J.R.R. Tolkien |
+-------------+--------------------------------+

TABLE;
Expand Down Expand Up @@ -1577,22 +1579,22 @@ public function testWithColspanAndMaxWith()
| Lorem ipsum dolor sit amet, consectetur adipi |
| scing elit, sed do eiusmod tempor |
+-----------------+-----------------+-----------------+
| Lorem ipsum dolor sit amet, consectetur adipi |
| scing elit, sed do eiusmod tempor |
| Lorem ipsum dolor sit amet, consectetur |
| adipiscing elit, sed do eiusmod tempor |
+-----------------+-----------------+-----------------+
| Lorem ipsum dolor sit amet, co | hello world |
| nsectetur | |
+-----------------+-----------------+-----------------+
| hello world | Lorem ipsum dolor sit amet, co |
| | nsectetur adipiscing elit |
+-----------------+-----------------+-----------------+
| hello | world | Lorem ipsum dol |
| | | or sit amet, co |
| | | nsectetur |
| hello | world | Lorem ipsum |
| | | dolor sit amet, |
| | | consectetur |
+-----------------+-----------------+-----------------+
| Symfony | Test | Lorem ipsum dol |
| | | or sit amet, co |
| | | nsectetur |
| | | or sit amet, |
| | | consectetur |
+-----------------+-----------------+-----------------+

TABLE;
Expand All @@ -1614,8 +1616,9 @@ public function testWithHyperlinkAndMaxWidth()
$expected =
<<<TABLE
+----------------------+
| \033]8;;Lorem\033\\Lorem ipsum dolor si\033]8;;\033\\ |
| \033]8;;Lorem\033\\t amet, consectetur \033]8;;\033\\ |
| \033]8;;Lorem\033\\Lorem ipsum dolor\033]8;;\033\\ |
| \033]8;;Lorem\033\\sit amet,\033]8;;\033\\ |
| \033]8;;Lorem\033\\consectetur\033]8;;\033\\ |
| \033]8;;Lorem\033\\adipiscing elit, sed\033]8;;\033\\ |
| \033]8;;Lorem\033\\do eiusmod tempor\033]8;;\033\\ |
+----------------------+
Expand Down