Skip to content

[Console] Preserving line breaks between sentences according to the exception message #24219

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
Sep 20, 2017
Merged

[Console] Preserving line breaks between sentences according to the exception message #24219

merged 1 commit into from
Sep 20, 2017

Conversation

yceruto
Copy link
Member

@yceruto yceruto commented Sep 15, 2017 •
edited
Loading

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

ro0NL
ro0NL approved these changes Sep 15, 2017
View reviewed changes
Copy link
Contributor

@ro0NL ro0NL left a comment

Choose a reason for hiding this comment

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

👍 for preserving lines. Think intention was pure for cosmetic reasons.

ro0NL
ro0NL reviewed Sep 15, 2017
View reviewed changes
src/Symfony/Component/Console/Application.php
@@ -1133,9 +1133,8 @@ private function splitStringByWidth($string, $width)
$lines[] = str_pad($line, $width);
$line = $char;
}
if ('' !== $line) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@yceruto i think this is trimming related in fact :)

throw new \Exception("\n\nline 1 with extra spaces        \nline 2\n\nline 4\n");

Before/After
image

image

So here's a better test ;-)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@yceruto yceruto force-pushed the console_error_linebreak branch 2 times, most recently from 63994c6 to dba55ef Compare September 15, 2017 22:42
@yceruto yceruto changed the title [Console] Render all line breaks according to the exception message [Console] Preserving line breaks between sentences according to the exception message Sep 15, 2017
@nicolas-grekas nicolas-grekas added this to the 2.7 milestone Sep 16, 2017
@yceruto yceruto force-pushed the console_error_linebreak branch from dba55ef to e2d4904 Compare September 19, 2017 20:31
@chalasr
Copy link
Member

chalasr commented Sep 20, 2017

Thanks for fixing this bug Yonel.

@chalasr chalasr merged commit e2d4904 into symfony:2.7 Sep 20, 2017
chalasr added a commit that referenced this pull request Sep 20, 2017
@chalasr
bug #24219 [Console] Preserving line breaks between sentences accordi…
a40c94d 
…ng to the exception message (yceruto)

This PR was merged into the 2.7 branch.

Discussion
----------

[Console] Preserving line breaks between sentences according to the exception message

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

Commits
-------

e2d4904 Render all line breaks according to the exception message
@yceruto yceruto deleted the console_error_linebreak branch September 20, 2017 13:09
@yceruto yceruto mentioned this pull request Sep 22, 2017
Merged
This was referenced Oct 5, 2017
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ro0NL ro0NL ro0NL approved these changes

@ogizanagi ogizanagi ogizanagi approved these changes

@chalasr chalasr chalasr approved these changes

Assignees
No one assigned
Labels
Bug Console Status: Needs Review
Projects
None yet
Milestone
2.7
Development

Successfully merging this pull request may close these issues.
6 participants
@yceruto @chalasr @ro0NL @ogizanagi @nicolas-grekas @carsonbot