8000 [Console][ProgressBar] Invalid example of usage of setMessage() method and %message% placeholder · Issue #6544 · symfony/symfony-docs · GitHub
[go: up one dir, main page]
Skip to content

[Console][ProgressBar] Invalid example of usage of setMessage() method and %message% placeholder #6544

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
fre5h opened this issue May 8, 2016 · 7 comments
Closed

[Console][ProgressBar] Invalid example of usage of setMessage() method and %message% placeholder #6544

fre5h opened this issue May 8, 2016 · 7 comments
Labels
actionable Clear and specific issues ready for anyone to take them. bug Console hasPR A Pull Request has already been submitted for this issue.
Milestone
2.7

Comments

@fre5h
Copy link
Contributor
fre5h commented May 8, 2016 •
edited
Loading

In the documentation of the ProgressBar helper there is a list of built-in placeholders. As the documentations says, built-in placeholders are:

  • current: The current step;
  • max: The maximum number of steps (or 0 if no max is defined);
  • bar: The bar itself;
  • percent: The percentage of completion (not available if no max is defined);
  • elapsed: The time elapsed since the start of the progress bar;
  • remaining: The remaining time to complete the task (not available if no max is defined);
  • estimated: The estimated time to complete the task (not available if no max is defined);
  • memory: The current memory usage;
  • message: The current message attached to the progress bar.

All of these placeholders are supported except the message. To be sure check this method https://github.com/symfony/console/blob/3.0/Helper/ProgressBar.php#L512
Also If we check the built-in formats https://github.com/symfony/console/blob/3.0/Helper/ProgressBar.php#L569 there is no format which uses the %message% placeholder out of box.

So the problem is that the next code from the documentation does not do out of box what we expect. It does not print the message, because any of the default formats does not include the %message% placeholder.

$bar->setMessage('Task starts');
$bar->start();

$bar->setMessage('Task in progress...');
$bar->advance();

// ...

$bar->setMessage('Task is finished');
$bar->finish();

No message won't be displayed in the progress bar. Even if you select any of built-in formats and set it $progressBar->setFormat('built-in-format'); it will not print the message. Because no of the built-in formats: normal, normal_nomax, verbose, verbose_nomax, very_verbose, very_verbose_nomax, debug, debug_nomax does not use the %message% placeholder.

So the current version of documentation confuses a bit. For example when I found this nice feature with messages for progress bar, I wanted to use it in my project. But it didn't work, the method setMessage() does nothing and I spent a lot of time to investigate this issue. So now I know two ways how to display the message in the progress bar.

The first is to set a custom format definition and add the %message% placeholder there (This is not clear from the documentation):

$progressBar = new ProgressBar($output, 10000);
$progressBar->setFormatDefinition('custom', ' %current%/%max% [%bar%] %message% %percent:3s%% %elapsed:6s%/%estimated:-6s%');
$progressBar->setFormat('custom');
$progressBar->setMessage('Start');
// start the loop...

This way is preferred, because it requires only to update the some part of docs.

And the second way is to edit the build-in formats https://github.com/symfony/console/blob/3.0/Helper/ProgressBar.php#L569 to include the %message% placeholder to some existing formats or maybe add new versions of formats which include this placeholder. But this case needs to make changes in the source code.

This mistake in the documentation affects all version since the 2.5 when the ProgressBar was introduced.

So if you agree with me that this missed peace of documentation is the issue, then let me know, I can fix it and send the pull request.
@xabbuh
Copy link
Member
xabbuh commented May 9, 2016

@fre5h A PR would be warmly welcomed. And if you could also open an issue for the code that would be great too.

@fre5h
Copy link
Contributor Author
fre5h commented May 10, 2016

OK, I think it is better at first to send a fix to the code. If this fix be accepted, then there is no need to fix the documentation, because it will be correct.
Otherwise, if fix to the code be rejected, then I will create a fix to the docs.

@fre5h fre5h mentioned this issue May 11, 2016
Closed
@fre5h fre5h mentioned this issue Jun 11, 2016
Closed
@fabpot fabpot mentioned this issue Jun 12, 2016
Merged
@fabpot
Copy link
Member
fabpot commented Jun 12, 2016

This is indeed a doc issue, see symfony/symfony#19035.

fabpot added a commit to symfony/symfony that referenced this issue 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
@HeahDude
Copy link
Contributor

Can be closed?

@fre5h
Copy link
Contributor Author
fre5h commented Jun 13, 2016 •
edited
Loading

No, this is the issue. Documentation needs some small update to show that to use %message% placeholder user has to add custom format. Because there is no built-in format with supports %message% placeholder.

@HeahDude
Copy link
Contributor

Alright! Thanks for the update.

@xabbuh xabbuh added this to the 2.7 milestone Jun 18, 2016
@xabbuh xabbuh added the actionable Clear and specific issues ready for anyone to take them. label Jun 18, 2016
@Jean85 Jean85 mentioned this issue Dec 13, 2016
Merged
@Jean85
Copy link
Contributor
Jean85 commented Dec 13, 2016

I've created a PR to fix this: #7251

@xabbuh xabbuh added the hasPR A Pull Request has already been submitted for this issue. label Dec 13, 2016
xabbuh added a commit that referenced this issue Jan 24, 2017
@xabbuh
minor #7251 Fix invalid ProgressBar message examples (Jean85, javiere…
7f36089 
…guiluz)

This PR was merged into the 2.7 branch.

Discussion
----------

Fix invalid ProgressBar message examples

This fixes #6544.
I've removed the confusing bits at the top, where the message placeholder is mentioned, and changed the last part, making the example more meaningful.

Commits
-------

da7fd2d Reworded some explanations and expanded examples
89a2843 Fix explanation of custom placeholder
771f952 Fix invalid ProgressBar message examples
@xabbuh xabbuh closed this as completed in 771f952 Jan 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
actionable Clear and specific issues ready for anyone to take them. bug Console hasPR A Pull Request has already been submitted for this issue.
Projects
None yet
Milestone
2.7
Development

No branches or pull requests
5 participants
@fabpot @fre5h @xabbuh @Jean85 @HeahDude
0