8000 Some Console Helpers are stateful and therefore not easily reusable · Issue #9788 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content
8000

Some Console Helpers are stateful and therefore not easily reusable #9788

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
Burgov opened this issue Dec 16, 2013 · 5 comments
Closed

Some Console Helpers are stateful and therefore not easily reusable #9788

Burgov opened this issue Dec 16, 2013 · 5 comments
Labels
Console

Comments

@Burgov
Copy link
Contributor
Burgov commented Dec 16, 2013

The most obvious problem with this appears when you want to render multiple tables in one command; there is no way to reset the table!

An easy fix would be to reset the $rows and $headers properties as well in the cleanup() method, but IMO a different design pattern should be applied altogether. I think usage like this should be best:

use Symfony\Component\Console\Helper\Table\Table;
use Symfony\Component\Console\Helper\Table\TableLayout;

/* ... */

$table = new Table();
$table->setHeaders(array('header 1', 'header 2'));
$table->addRow(array('row 1', 'row 1'));
$table->addRow(array('row 2', 'row 2'));

$tableLayout = new TableLayout();
$tableLayout->setPaddingChar('*');
$tableLayout->setHorizontalBorderChar('=');

$tableHelper = $this->getHelperSet()->getHelper('table');
$helperHelper->render($table, $tableLayout);

And for the predefined layouts perhaps something like

$tableLayout = TableLayout::create(TableLayout::LAYOUT_BORDERLESS);

The second argument to the render method would default to the line above.

The same should apply to the progressHelper although the problem is not so obvious there.
@Burgov
Copy link
Contributor Author
Burgov commented Dec 16, 2013

Actually just noticed that I could call setRows(array()) and make sure to call setHeaders() to reset it, which will help me for now, but issue still applies

@cordoval
Copy link
Contributor
cordoval 8000 commented Dec 16, 2013

there are many use cases for tables so i think it is better to keep it simple, if you need something else or beyond it should be a matter of copying the class and plugging a new custom table helper but it goes out of the general use case imo

@stof
Copy link
Member
stof commented Dec 16, 2013

Well, I agree with @Burgov that having stateful helpers can create all sort of weird issues.

@cordoval cordoval mentioned this issue Jan 6, 2014
Closed
@wouterj wouterj mentioned this issue Jan 6, 2014
Closed
@cordoval
Copy link
Contributor
cordoval commented Jan 7, 2014

I have solved it like this https://github.com/cordoval/gush/pull/62/files any comments, i can take a stab at this if @fabpot approves. I am 👍 for a combination between https://github.com/cordoval/gush/pull/62/files and @Burgov 's approach

@ghost
Copy link
ghost commented Jan 15, 2014

+1 for this issue, I experienced several problems due to the table helper holding onto old data. I have a command that prints about five tables for X iterations.

This was referenced Mar 1, 2014
Closed
Merged
fabpot added a commit that referenced this issue Mar 3, 2014
@fabpot
feature #10356 [Console] A better progress bar (fabpot)
554b28d 
This PR was merged into the 2.5-dev branch.

Discussion
----------

[Console] A better progress bar

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #9573, #9574, #10187, #9951, related to #9788
| License       | MIT
| Doc PR        | symfony/symfony-docs#3626

TODO:

 - [x] add some docs

See what this PR allows you to do easily:

![cwzpvk](https://f.cloud.github.com/assets/47313/2302190/30cd80e4-a170-11e3-8d88-80c4e4ca8b23.gif)

## New ProgressBar class

First, this PR deprecates `ProgressHelper` in favor of `ProgressBar`. The main difference is that the new `ProgressBar` class represents a single progress bar, which allows for more than one bar to be displayed at a time:

```php
use Symfony\Component\Console\Helper\ProgressBar;
use Symfony\Component\Console\Output\ConsoleOutput;

$output = new ConsoleOutput();

$bar1 = new ProgressBar($output, 10);
$bar2 = new ProgressBar($output, 20);
$bar2->setProgressCharacter('#');
$bar1->start();
print "\n";
$bar2->start();

for ($i = 1; $i <= 20; $i++) {
    // up one line
    $output->write("\033[1A");
    usleep(100000);
    if ($i <= 10) {
        $bar1->advance();
    }
    print "\n";
    $bar2->advance();
}
```

And here is how it looks like when run:

![progress-bars](https://f.cloud.github.com/assets/47313/2300612/4465889a-a0fd-11e3-8bc2-b1d2a0f5dc3d.gif)

## Format Placeholders

This pull request refactors the way placeholders in the progress bar are managed. It is now possible to add new placeholders or replace existing ones:

```php
// set a new placeholder
ProgressBar::setPlaceholderFormatterDefinition('remaining_steps', function (ProgressBar $bar) {
    return $bar->getMaxSteps() - $bar->getStep();
});

// change the behavior of an existing placeholder
ProgressBar::setPlaceholderFormatterDefinition('max', function (ProgressBar $bar) {
    return $bar->getMaxSteps() ?: '~';
});
```

Several new built-in placeholders have also been added:

 * `%remaining%`: Display the remaining time
 * `%estimated%`: Display the estimated time of the whole "task"
 * `%memory%`: Display the memory usage

## Formats

Formats can also be added (or built-in ones modified):

```php
ProgressBar::setFormatDefinition('simple', '%current%');

$bar->setFormat('simple');
// is equivalent to
$bar->setFormat('%current%');
```

Built-in formats are:

 * `quiet`
 * `normal`
 * `verbose`
 * `quiet_nomax`
 * `normal_nomax`
 * `verbose_nomax`

## Format Messages

You can also set arbitrary messages that depends on the progression in the progress bar:

```php
$bar = new ProgressBar($output, 10);
$bar->setFormat("%message% %current%/%max% [%bar%]");

$bar->setMessage('started');
$bar->start();

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

$bar->setMessage('finish');
$bar->finish();
```

You are not limited to a single message (`message` being just the default one):

```php
$bar = new ProgressBar($output, 10);
$bar->setFormat("%message% %current%/%max% [%bar%] %end%");

$bar->setMessage('started');
$bar->setMessage('', 'end');
$bar->start();

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

$bar->setMessage('finish');
$bar->setMessage('ended...', 'end');
$bar->finish();
```

## Multiline Formats

A progress bar can now span more than one line:

```php
$bar->setFormat("%current%/%max% [%bar%]\n%message%");
```

## Flexible Format Definitions

The hardcoded formatting for some placeholders (like `%percent%` or `%elapsed%`) have been removed in favor of a `sprintf`-like format:

```php
$bar->setFormat("%current%/%max% [%bar%] %percent:3s%");
```

Notice the `%percent:3s%` placeholder. Here, `%3s` is going to be used when rendering the placeholder.

## ANSI colors and Emojis

The new progress bar output can now contain ANSI colors and.or Emojis (see the small video at the top of this PR).

Commits
-------

0d1a58c [Console] made formats even more flexible
8c0022b [Console] fixed progress bar when using ANSI colors and Emojis
38f7a6f [Console] fixed PHP comptability
244d3b8 [Console] added a way to globally add a progress bar format or modify a built-in one
a9d47eb [Console] added a way to add a custom message on a progress bar
7a30e50 [Console] added support for multiline formats in ProgressBar
1aa7b8c [Console] added more default placeholder formatters for the progress bar
2a78a09 [Console] refactored the progress bar to allow placeholder to be extensible
4e76aa3 [Console] added ProgressBar (to replace the stateful ProgressHelper class)
@fabpot fabpot mentioned this issue Mar 3, 2014
Merged
fabpot added a commit that referenced this issue Mar 3, 2014
@fabpot
feature #10365 [Console] deprecated TableHelper in favor of Table (fa…
98c3fe7 
…bpot)

This PR was merged into the 2.5-dev branch.

Discussion
----------

[Console] deprecated TableHelper in favor of Table

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | #9788, #9680, #9325
| License       | MIT
| Doc PR        | symfony/symfony-docs#3627

This PR makes the Table helper stateless. It also adds a way to define global styles and to change/tweak existing styles easily.

The second commit adds the possibility to add a separator anywhere in the table output.

Commits
-------

21784ce [Console] make it possible to pass a style directly to Table::setStyle()
14caaec [Console] added the possibility to insert a table separator anywhere in a table output
39c495f [Console] deprecated TableHelper in favor of Table
@fabpot fabpot closed this as completed Mar 3, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Console
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@fabpot @cordoval @Burgov @stof
0