8000 [Console] Add default behavior for the %message% placeholder in the ProgressBar by fre5h · Pull Request #19031 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Console] Add default behavior for the %message% placeholder in the ProgressBar #19031

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
Closed
wants to merge 1 commit into from
Closed

[Console] Add default behavior for the %message% placeholder in the ProgressBar #19031

wants to merge 1 commit into from

Conversation

fre5h
Copy link
Contributor
@fre5h fre5h commented Jun 11, 2016
Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #18746 the same patch which was rejected because wrong branch (2.7) was selected
License MIT
Doc PR If this patch be accepted then current docs will be actual and no update needed. Otherwise documentation should be updated a bit because now it contains wrong example of default behavior, described in symfony/symfony-docs#6544

The description I totally copy pasted from the previous PR #18746

When I was reading the documentation for the ProgressBar helper http://symfony.com/doc/current/components/console/helpers/progressbar.html I found a nice feature - showing messages by using %message% placeholder. But when I tried the example from the docs, I found that it does not work as expected.

So this code will give the next output

protected function execute(InputInterface $input, OutputInterface $output)
{
    $bar = new ProgressBar($output, 4);

    $bar->setMessage('Starting the demo...');
    $bar->start();

    sleep(1);

    $bar->setMessage('First message...');
    $bar->advance();

    sleep(1);

    $bar->setMessage('Second message...');
    $bar->advance();

    sleep(1);

    $bar->setMessage('Third message...');
    $bar->advance();

    sleep(1);

    $bar->setMessage('Finish...');
    $bar->finish();

    $output->writeln('');
}

example1

So as you see no message is displayed. That is because all built-in formats do not have a %message% placeholder. You can check the master branch https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Console/Helper/ProgressBar.php#L569 to be sure. Same is in the older versions.

Or you can just quickly look at the initFormat 8000 s() which I copy-pasted here:

private static function initFormats()
{
    return array(
        'normal' => ' %current%/%max% [%bar%] %percent:3s%%',
        'normal_nomax' => ' %current% [%bar%]',

        'verbose' => ' %current%/%max% [%bar%] %percent:3s%% %elapsed:6s%',
        'verbose_nomax' => ' %current% [%bar%] %elapsed:6s%',

        'very_verbose' => ' %current%/%max% [%bar%] %percent:3s%% %elapsed:6s%/%estimated:-6s%',
        'very_verbose_nomax' => ' %current% [%bar%] %elapsed:6s%',

        'debug' => ' %current%/%max% [%bar%] %percent:3s%% %elapsed:6s%/%estimated:-6s% %memory:6s%',
        'debug_nomax' => ' %current% [%bar%] %elapsed:6s% %memory:6s%',
    );
}

For example ProgressIndicator has the %message% placeholder https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Console/Helper/ProgressIndicator.php#L314

So that's why out of box without customizing your format - messages won't be shown. For example in the current implementation of the ProgressBar if I want to display the message I have to add the next code:

$bar = new ProgressBar($output, 4);
// I just take the `normal` format and add %message% placeholder there
$bar->setFormatDefinition('normal_customized', ' %current%/%max% [%bar%] %message% %percent:3s%%');
$bar->setFormat('normal_customized');

// End with the code from my previous example...

And it works...

2

But as I described here symfony/symfony-docs#6544 It is not clear from the docs. After the reading of docs I thought that message should be displayed out of box without customizing the output.

After that I investigated the code of ProgressBar and found the cause.

So I see two ways how to solve it.

  1. Leave the code as it is now and fix the docs. But I think it is not best choice, because user still needs to add custom format every time when they need to display a simple message.
  2. Add the default behavior for the %message% placeholder which doesn't break the backwards compatibility.

So in this patch I propose a small tweak. I propose to add a placeholder formatter for %message% placeholder. It will check if there is a message with name message in the array of messages - then it will take it, otherwise - it will ignore it.
Also I propose to add a %message% to all built-in formats, e.g.
'normal' => ' %current%/%max% [%bar%]%message% %percent:3s%%',
But don't surround it with spaces, so in this case if user doesn't set the custom message, the message placeholder formatter will convert it to the empty string, so it behaves like in the current implementation.
If user adds a custom message $bar->setMessage('Starting'); the formatter will replace placeholder with the current text.
There is one additional tweak I have added there. As the %message% placeholder is set next to the [%bar%] without a space, if user adds the message without at least one space at the beginning, the progress bar will look a bit ugly
4/4 [============================]Finish... 100%
To prevent it I decided to add a space to the beginning of the message, if message doesn't start with a space. So in my implementation if user sets the message like this

$bar->setMessage('Finish'); // without a space at the beginning

The output will be next (with extra space)

 4/4 [============================] Finish 100%

If user manually adds some spaces at the beginning then no extra space will be added.

// user sets a space at the beginning, so no extra space added
$bar->setMessage(' Finish');
// outputs the:  4/4 [============================] Finish 100%

// user sets two spaces, so only two spaces are shown, no more extra space
$bar->setMessage('  Finish');
// outputs the:  4/4 [============================]  Finish 100%

// same with different number of spaces
$bar->setMessage('    Finish');
// outputs the:  4/4 [============================]    Finish 100%

So with the changes which I propose to the ProgressBar the next code will give the next output

protected function execute(InputInterface $input, OutputInterface $output)
{
    $bar = new ProgressBar($output, 4);

    $bar->setMessage('Starting the demo...');
    $bar->start();

    sleep(1);

    $bar->setMessage(' First message...');
    $bar->advance();

    sleep(1);

    $bar->setMessage('  Second message...');
    $bar->advance();

    sleep(1);

    $bar->setMessage('');
    $bar->advance();

    sleep(1);

    $bar->setMessage('Finish...');
    $bar->finish();

    $output->writeln('');
}

3

I didn't update existing tests, because this patch doesn't break them. But I added new tests to check the default behavior of the %message% placeholder.

That's all. Waiting for your feedback.

@fre5h
Copy link
Contributor Author
fre5h commented Jun 11, 2016

Only build for HHVM is failed. But that wasn't caused by my patch.

@fabpot fabpot mentioned this pull request Jun 12, 2016
Merged
@fabpot
Copy link
Member
fabpot commented Jun 12, 2016

Reading this change again and the corresponding code, I realize that this is a documentation issue instead.

See my alternative PR (#19035) for more explanations. I added a phpdoc there trying to explain how messages should be used.

https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Console/Tests/Helper/ProgressBarTest.php#L561 is a great example on how to use messages like intended initally.

@fabpot fabpot closed this Jun 12, 2016
fabpot added a commit that referenced this pull request Jun 13, 2016
@fabpot
minor #19035 [Console] added explanation of messages usage in a progr…
ad1ffc5 
…ess bar (fabpot)

This PR was merged into the 2.7 branch.

Discussion
----------

[Console] added explanation of messages usage in a progress bar

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

The intent of progress bar messages is currently wrongly documented. This PR updates the phpdoc to hopefully better describe the usage of such messages.

So, basically, messages are a way to add dynamic information in the progress bar; information that cannot be computed by the progress bar (like for all other placeholders).

Commits
-------

d92f3ea [Console] added explanation of messages usage in a progress bar
@fre5h fre5h deleted the feature-message-placeholder-in-progressbar branch January 24, 2020 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Console Feature Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fre5h @fabpot @carsonbot
0