Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[Console] SymfonyStyle: Align multi-line/very-long-line blocks#18879
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Remove SymfonyStyle::comment() changes (out of scope)CS FixesAdd tests
chalasr commentedMay 25, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
My first intention was to propose this on 2.8, it's a mistake... |
@chalasr thanks for this PR. In my opinion this is a bug fix which should be merged in 2.7. The "new" behavior is the one we expected since the beginning. |
Thank you@chalasr. |
…ocks (chalasr)This PR was submitted for the master branch but it was merged into the 2.7 branch instead (closes#18879).Discussion----------[Console] SymfonyStyle: Align multi-line/very-long-line blocks| Q | A| ------------- | ---| Branch? | master| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#18564| License | MIT| Doc PR | n/aThis PR makes all lines aligned in multi-line blocks.Very-long-line block:```phpSymfonyStyle::warning('Lorem ipsum...');```Before:After:Multi-line block:```phpSymfonyStyle::success(['Lorem ipsum...', 'Lorem ipsum...', 'Lorem ipsum...']);```Before:After:Also@javiereguiluz pointed the case of `SymfonyStyle::comment()` in#18564, I needed to make it calling `SymfonyStyle::block()` with ` // ` as prefix to fit the first intention of this one.So if this one is merged I'll propose the changes for comments in a second PR (out of this scope).Commits-------963fe1d [Console] SymfonyStyle: Align multi-line/very-long-line blocks
@javiereguiluz Do you know if there is a reason for that the 2.7 branch doesn't have the |
@chalasr yes I know the reason well 😁 It was my mistake. At first there wasn't a |
Hmm ok so I'll keep it on 2.8 (stop me if I'm wrong). |
| @@ -57,7 +57,7 @@ public function inputCommandToOutputFilesProvider() | |||
| public function testLongWordsBlockWrapping() | |||
| { | |||
| $word = 'Lopadotemachoselachogaleokranioleipsanodrimhypotrimmatosilphioparaomelitokatakechymenokichlepikossyphophattoperisteralektryonoptekephalliokigklopeleiolagoiosiraiobaphetraganopterygon'; | |||
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.
As we discussed, this change should not have been necessary for the test to pass. So I suggest to see#18896
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.
Thank's for having made the change@ogizanagi
… to directly test output (ogizanagi)This PR was merged into the 2.7 branch.Discussion----------[Console] [SymfonyStyle] Replace long word wrapping test to directly test output| Q | A| ------------- | ---| Branch? | 2.7| Bug fix? | no| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#18879 (comment)| License | MIT| Doc PR | -This [line](https://github.com/symfony/symfony/pull/18879/files#diff-d3625f2548a3b329058ca5a0f5aa57feR60) should not have been changed in order to the test to pass. I assume the test was flawed at first, so I suggest to simply test the output as we did for other ones.Ping@chalasrCommits-------b78fff4 [Console] [SymfonyStyle] Replace long word wrapping test to directly test output
… to directly test output (ogizanagi)This PR was merged into the 2.7 branch.Discussion----------[Console] [SymfonyStyle] Replace long word wrapping test to directly test output| Q | A| ------------- | ---| Branch? | 2.7| Bug fix? | no| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |symfony/symfony#18879 (comment)| License | MIT| Doc PR | -This [line](https://github.com/symfony/symfony/pull/18879/files#diff-d3625f2548a3b329058ca5a0f5aa57feR60) should not have been changed in order to the test to pass. I assume the test was flawed at first, so I suggest to simply test the output as we did for other ones.Ping@chalasrCommits-------b78fff4 [Console] [SymfonyStyle] Replace long word wrapping test to directly test output
Uh oh!
There was an error while loading.Please reload this page.
This PR makes all lines aligned in multi-line blocks.
Very-long-line block:
Before:
After:
Multi-line block:
Before:
After:
Also@javiereguiluz pointed the case of
SymfonyStyle::comment()in#18564, I needed to make it callingSymfonyStyle::block()with//as prefix to fit the first intention of this one.So if this one is merged I'll propose the changes for comments in a second PR (out of this scope).