[Console] SymfonyStyle : EOL consistency #14741
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -100,7 +100,7 @@ public function block($messages, $type = null, $style = null, $prefix = ' ', $pa | ||
| */ | ||
| public function title($message) | ||
| { | ||
| $this->writeln(sprintf("\n<comment>%s</>\n<comment>%s</>\n", $message, str_repeat('=', strlen($message)))); | ||
| $this->writeln(sprintf('<comment>%s</>'.PHP_EOL.'<comment>%s</>'.PHP_EOL, $message, str_repeat('=', strlen($message)))); |
There was a problem hiding this comment.
Missing PHP_EOL at the beginning of the string.
|
@aitboudad : Actually, your comments gave me another idea. What do you think of this new suggestion ? The output is strictly equivalent, and relies on the real output instance in use. Do you think the |
| @@ -92,7 +92,8 @@ public function block($messages, $type = null, $style = null, $prefix = ' ', $pa | ||
| } | ||
| } | ||
|
|
||
| $this->writeln(implode("\n", $lines)."\n"); | ||
| $lines[] = ''; | ||
| $this->writeln($lines); |
There was a problem hiding this comment.
can we use array_push here instead $lines[] = ''; ?
$this->writeln(array_push($lines, ''));There was a problem hiding this comment.
@aitboudad : array_push returns the new number of elements in the array. So we will not be able to inline those statements using array_push.
There was a problem hiding this comment.
But this would be possible:
$this->writeln(array_merge($lines, (array) ''));
//or
$this->writeln(array_merge($lines, array('')));
fabpot
added a commit
that referenced
this pull request
Jun 8, 2015
…nagi) This PR was merged into the 2.7 branch. Discussion ---------- [Console] SymfonyStyle : fix & automate block gaps. | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - --- Depends on #14741 --- ## What it does - autoprepend appropriate blocks (like cautions, titles, sections, ...) by the correct number of blank lines considering history. - handle automatically most of the SymfonyStyle guide line breaks and gaps. Fix things such as unwanted double blank lines between titles and admonitions. - test outputs - fix an issue using questions with SymfonyStyle, which should not output extra blank lines when using with a non-interactive input. ## Description `SymfonyStyle` is great, but there are some issues, mostly when using blocks (text blocks, titles and admonitions): some extra blank lines might be generated. Plus, on the contrary, some line breaks or blank lines around blocks are missing, and the developer need to handle this himself by polluting his code with ugly `if` and `newLine()` statements. ### Before / After :  As you can see, it's still up to the developper to end his command by a blank line (unless using a block like `SymfonyStyle::success()`) in order to distinct different commands outputs more efficiently. Everything else is now handled properly, and automatically, according to the rules exposed in the symfony console style guide published some time ago by @javiereguiluz . Questions (not exposed in the above output) are considered as blocks, and follow, for instance, the same conditions than admonitions: 1 blank line before and after, no more (although, you'll still be able to output more blank lines yourself, using `newLine`). Commits ------- fc598ff [Console] SymfonyStyle : fix & automate block gaps. 260702e [Console] SymfonyStyle : Improve EOL consistency by relying on output instance
fabpot
added a commit
to symfony/console
that referenced
this pull request
Jun 8, 2015
…nagi) This PR was merged into the 2.7 branch. Discussion ---------- [Console] SymfonyStyle : fix & automate block gaps. | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - --- Depends on symfony/symfony#14741 --- ## What it does - autoprepend appropriate blocks (like cautions, titles, sections, ...) by the correct number of blank lines considering history. - handle automatically most of the SymfonyStyle guide line breaks and gaps. Fix things such as unwanted double blank lines between titles and admonitions. - test outputs - fix an issue using questions with SymfonyStyle, which should not output extra blank lines when using with a non-interactive input. ## Description `SymfonyStyle` is great, but there are some issues, mostly when using blocks (text blocks, titles and admonitions): some extra blank lines might be generated. Plus, on the contrary, some line breaks or blank lines around blocks are missing, and the developer need to handle this himself by polluting his code with ugly `if` and `newLine()` statements. ### Before / After :  As you can see, it's still up to the developper to end his command by a blank line (unless using a block like `SymfonyStyle::success()`) in order to distinct different commands outputs more efficiently. Everything else is now handled properly, and automatically, according to the rules exposed in the symfony console style guide published some time ago by @javiereguiluz . Questions (not exposed in the above output) are considered as blocks, and follow, for instance, the same conditions than admonitions: 1 blank line before and after, no more (although, you'll still be able to output more blank lines yourself, using `newLine`). Commits ------- fc598ff [Console] SymfonyStyle : fix & automate block gaps. 260702e [Console] SymfonyStyle : Improve EOL consistency by relying on output instance
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix EOL inconsistency - especially on Windows platforms where both
LFandCRLFwere mixed - by relying as much as possible on output implementation.Indeed,
PHP_EOLis used inOutputStyle::newLine()andStreamOutput, so there was no reason for usingLFinSymfonyStyle.However,
BufferedOutputusesLF. Then, EOL inconsistency might still be a problem if someone is usingSymfonyStyle/OutputStylewith aBufferedOutputinstance.