-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Console] SymfonyStyle : fix & automate block gaps. #14623
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Console] SymfonyStyle : fix & automate block gaps. #14623
Conversation
Although the implementation might not be the most elegant solution, I'd like to have some opinions about such a "feature" (in fact, I consider this an improvement just as much as a bug fix) ? Fabbot issue will be fixed as soon as this PR is merged: #14620 Here is the code used in order to generate this output (+questions): protected function execute(InputInterface $input, OutputInterface $output)
{
$output = new SymfonyStyle($input, $output);
$output->caution(array('Lorem ipsum dolor sit amet', 'consectetur adipisicing elit'));
$output->note('Lorem ipsum dolor sit amet, ');
$output->title('First title');
$output->warning('Lorem ipsum dolor sit amet');
$output->title('Second title');
$output->section('Section One');
$output->listing(array(
'Lorem ipsum dolor sit amet, consectetur adipisicing elit',
'Excepteur sint occaecat cupidatat non proident.'
));
$output->write(array('<comment>Lorem ipsum dolor', 'ipsum', 'dolor</>'));
$output->write(' amet <info>consectetur</>', true);
$output->section('Section 2');
$output->writeln(array('Lorem ', 'ipsum'));
$output->write(array('Amet ', 'consectetur'));
$output->title('Third title');
$output->section('Section 1');
$output->write(array('Lorem ', 'ipsum ', 'dolor '));
$output->text(array(
'Duis aute irure dolor in reprehenderit in voluptate velit esse',
'cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat'
));
$output->success('Success');
$output->error('Success');
$output->table(array('Name', 'Method', 'Scheme', 'Host', 'Path'), array(
array('admin_post_new', 'ANY', 'ANY', 'ANY', '/admin/post/new'),
array('admin_post_show', 'GET', 'ANY', 'ANY', '/admin/post/{id}'),
));
$output->progressStart(100);
for ($i = 0; $i < 100; $i++) {
usleep(10000);
$output->progressAdvance();
}
$output->progressFinish();
$output->text('Duis aute irure dolor in reprehenderit in voluptate velit esse');
$output->note($output->askHidden('Hidden question'));
$output->note($output->choice('Choice question with default', array('choice1', 'choice2'), 'choice1'));
$output->text('Duis aute irure dolor in reprehenderit in voluptate velit esse');;
$output->section('Section 1');
$output->note($output->ask('Question with default', 'default'));
$output->write('<comment>Lorem ipsum dolor</>');
$output->writeln(' amet <info>consectetur</>');
$output->note($output->confirm('Confirmation with yes default', true) ? 'yes' : 'no');
$output->note('Duis aute irure dolor in reprehenderit in voluptate velit');
} Tests are also provided in order to ensure the output is what we expect it to be. Lastly, this PR also contains a bug fix about questions rendering with SymfonyStyle when using a non-interactive input. Before, extra blank lines were rendered. |
} | ||
//Has EOL. Split into multiples messages. | ||
$messages = array_map(function ($message) { | ||
return $message.PHP_EOL; //Re-add trailing EOL |
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.
This will add one more EOL than before the split, is it expected?
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.
I'm wondering if an EOL terminated $messages string can exist, which would mean an empty string here (before the concat)
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.
This will add one more EOL than before the split, is it expected?
Actually it is... But I can't remember the exact reason why. It has something to do with questions:
Before: current version, After: removing the extra EOL
However, you are right about an EOL terminated $messages string... Such code wouldn't perfectly render for instance:
$output->write('Lorem ipsum dolor'.PHP_EOL);
$output->note('NOTE');
$output->write('sit amet');
The note block will not be prepended by a blank line :/
How will this affect current commands that are adding the newlines already? Just asking, as we use this for Gush at the moment :) |
@sstok : If everything works as fine as I expect it to be (I need to test it on windows, soon), the only side-effect for your commands when upgrading to symfony 2.7 is that all the newlines you added manually in the aim of having proper gaps between text and block elements should be removed. However, note that some of the newlines you might have added will have no effect if kept: protected function execute(InputInterface $input, OutputInterface $output)
{
$output = new SymfonyStyle($input, $output);
$output->newLine(); //In order to have a first blank line at command start. Same effect if removed.
$output->section('Section 1');
$output->write('Lorem ipsum dolor');
$output->newLine(); //Should be removed
$output->text(array(
'Duis aute irure dolor in reprehenderit in voluptate velit esse',
'cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat'
));
$output->newLine(); //In order to have a blank line before the success block. Same effect if removed.
$output->success('Success');
} Before: This piece of code without this PR. The best would be to remove any manual newlines between text and blocks elements. If your are relying on the full command output in some of your tests, there is good chances those tests fail, as some extra lines are removed (like between an admonition and a title). I would be very pleased to receive your feedbacks if you try this code with Gush 😃 |
This PR has been rebased onto #14741, as it indirectly improves and allows to simplify the way we keep track of an "history buffer". $output->write('Lorem ipsum dolor'.PHP_EOL);
$output->note('NOTE');
$output->write('sit amet'); Will render as expected with this PR:
And everything works like a charm on Windows (if we forget about #14740). |
Autoprepend appropriate blocks by the correct number of blank lines. Handle most of the SymfonyStyle guide line breaks and gaps automatically and fix thing such as double blank lines between titles. Add output tests. Fix askQuestion method, which should not output anything when the input isn't interactive.
@fabpot : Do you think this PR is still applicable to the 2.7 branch ? |
👍 |
👍 |
Thank you @ogizanagi. |
…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
Depends on #14741
What it does
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
andnewLine()
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
).