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

Jump to bottom
Merged
merged 1 commit into from
Aug 14, 2023
Merged

[Console] Fix linewraps in OutputFormatter #51223

merged 1 commit into from
Aug 14, 2023

Conversation

maxbeckers
Copy link
Contributor

@maxbeckers maxbeckers commented Aug 2, 2023 •
edited
Loading

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #42034
License MIT
Doc PR n/a

Fix output for tables with linebreaks and special chars;

$table = new \Symfony\Component\Console\Helper\Table($output);
$table
    ->setColumnMaxWidth(1, 10)
    ->setHeaders(['ISBN', 'Title'])
    ->setRows([
        ['99921-58-10-7', 'A really long title that could need multiple lines'],
        new \Symfony\Component\Console\Helper\TableSeparator(),
        ['99921-58-10-7', 'Â rèälly löng tîtlè thät cöüld nèêd múltîplê línès']
    ])
;
$table->render();

Before

+---------------+------------+
| ISBN          | Title      |
+---------------+------------+
| 99921-58-10-7 | A really l |
|               | ong title  |
|               | that could |
|               | need multi |
|               | ple lines  |
+---------------+------------+
| 99921-58-10-7 | Â rèäll    |
|               | y löng t |
|               | tlè thä |
|               | t cöüld    |
|               | nèêd mú    |
|               | ltîplê l   |
|               | ínès       |
+---------------+------------+

After

+---------------+------------+
| ISBN          | Title      |
+---------------+------------+
| 99921-58-10-7 | A really   |
|               | long title |
|               | that could |
|               | need       |
|               | multiple   |
|               | lines      |
+---------------+------------+
| 99921-58-10-7 | Â rèälly   |
|               | löng tîtlè |
|               | thät cöüld |
|               | nèêd       |
|               | múltîplê   |
|               | línès      |
+---------------+------------+

@maxbeckers maxbeckers requested a review from chalasr as a code owner August 2, 2023 09:05
@carsonbot
Copy link

Hey!

Thanks for your PR. You are targeting branch "6.4" but it seems your PR description refers to branch "5.4,".
Could you update the PR description or change target branch? This helps core maintainers a lot.

Cheers!

Carsonbot

@carsonbot carsonbot added this to the 6.4 milestone Aug 2, 2023
@maxbeckers maxbeckers changed the base branch from 6.4 to 5.4 August 2, 2023 09:05
@maxbeckers maxbeckers force-pushed the patch-42034_special_chars_in_console_table branch 2 times, most recently from ec4b7af to dd95114 Compare August 2, 2023 09:09
@derrabus derrabus modified the milestones: 6.4, 5.4 Aug 2, 2023
@maxbeckers maxbeckers force-pushed the patch-42034_special_chars_in_console_table branch from dd95114 to c52f972 Compare August 2, 2023 09:34
@maxbeckers maxbeckers force-pushed the patch-42034_special_chars_in_console_table branch 3 times, most recently from e4d46f7 to 31dc48d Compare August 3, 2023 13:12
maxbeckers
maxbeckers commented Aug 3, 2023
View reviewed changes
src/Symfony/Component/Console/Tests/Helper/TableTest.php
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.

maxbeckers
maxbeckers commented Aug 3, 2023
View reviewed changes
src/Symfony/Component/Console/Tests/Helper/TableTest.php
=============== ========================== ==================

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.

@maxbeckers maxbeckers force-pushed the patch-42034_special_chars_in_console_table branch from 31dc48d to 4d44010 Compare August 7, 2023 05:59
@maxbeckers maxbeckers force-pushed the patch-42034_special_chars_in_console_table branch from 4d44010 to fcf86b3 Compare August 7, 2023 06:12
chalasr
chalasr reviewed Aug 7, 2023
View reviewed changes
src/Symfony/Component/Console/Tests/Formatter/OutputFormatterTest.php
$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.

@nicolas-grekas
Copy link
Member

Thank you @maxbeckers.

@nicolas-grekas nicolas-grekas merged commit 370d5c0 into symfony:5.4 Aug 14, 2023
This was referenced Aug 26, 2023
Merged
Merged
renovate bot added a commit to Lendable/composer-license-checker that referenced this pull request Aug 26, 2023
@renovate
Update Composer runtime dependencies to v6.3.4 (#179)
09ab4e9 
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [symfony/console](https://symfony.com)
([source](https://togithub.com/symfony/console)) | require | patch |
`6.3.2` -> `6.3.4` |
| [symfony/process](https://symfony.com)
([source](https://togithub.com/symfony/process)) | require | patch |
`6.3.2` -> `6.3.4` |

---

### Release Notes

<details>
<summary>symfony/console (symfony/console)</summary>

### [`v6.3.4`](https://togithub.com/symfony/console/releases/tag/v6.3.4)

[Compare
Source](https://togithub.com/symfony/console/compare/v6.3.2...v6.3.4)

**Changelog**
(symfony/console@v6.3.3...v6.3.4)

- bug
[symfony/symfony#51378](https://togithub.com/symfony/symfony/issues/51378)
\[Console] avoid multiple new line when message already ends with a new
line in section output
([@&#8203;joelwurtz](https://togithub.com/joelwurtz))
- bug
[symfony/symfony#51223](https://togithub.com/symfony/symfony/issues/51223)
\[Console] Fix linewraps in `OutputFormatter` (@&#8203;maxbeckers)
- bug
[symfony/symfony#51355](https://togithub.com/symfony/symfony/issues/51355)
\[Console] fix section output when multiples section with max height
([@&#8203;joelwurtz](https://togithub.com/joelwurtz))

</details>

<details>
<summary>symfony/process (symfony/process)</summary>

###
[`v6.3.4`](https://togithub.com/symfony/process/compare/v6.3.2...v6.3.4)

[Compare
Source](https://togithub.com/symfony/process/compare/v6.3.2...v6.3.4)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these
updates again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/Lendable/composer-license-checker).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi41Ni4wIiwidXBkYXRlZEluVmVyIjoiMzYuNTYuMCIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@OskarStark OskarStark OskarStark left review comments

@derrabus derrabus derrabus left review comments

@chalasr chalasr chalasr left review comments

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

Assignees
No one assigned
Labels
Bug Console Status: Reviewed
Projects
None yet
Milestone
5.4
Development

Successfully merging this pull request may close these issues.

Umlauts are incorrectly separated by the OutputFormatter when creating a table for the console
6 participants
@maxbeckers @carsonbot @nicolas-grekas @OskarStark @derrabus @chalasr