[Console] [Helper] [Table] Columns styles #14044

MAXakaWIZARD · 2015-03-24T23:58:31Z

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets
License	MIT
Doc PR

This PR introduces ability to set styles for individual columns in table. For example, we can apply STR_PAD_LEFT for the last column (useful for money, file size etc).

Code:

use Symfony\Component\Finder\Finder;
use Symfony\Component\Console\Helper\Table;
use Symfony\Component\Console\Helper\TableStyle;

$table = new Table($output);
$table->setHeaders(['#', 'Path', 'Size']);

$style = new TableStyle();
$style->setPadType(STR_PAD_LEFT);

$table->setColumnStyle(2, $style);

$finder = new Finder();
$finder->files()->in("/path/to/dir")->name('*.php')
$counter = 0;
foreach ($finder as $file) {
    $counter++;
    $table->addRow([$counter, $file->getRealPath(), number_format($file->getSize(), 0, '.', ' ')]);
}

$table->render();

Output:

+---+---------------------+--------+
| # | Path                |   Size |
+---+---------------------+--------+
| 1 | autoload.php        |    183 |
| 2 | ApplicationTest.php | 47 794 |
| 3 | CommandTest.php     | 14 965 |
| 4 | ListCommandTest.php |  2 369 |
+---+---------------------+--------+

aitboudad · 2015-03-25T08:54:42Z

src/Symfony/Component/Console/Helper/Table.php

+    {
+        if (isset($this->columnStyles[$columnIndex])) {
+            return $this->columnStyles[$columnIndex];
+        } else {


you can remove else here

@aitboudad Nope, all columns should have current table style by default, if not assigned from outside. If I remove else, rendering will be broken.

@MAXakaWIZARD else is useless as you call return in if... Your code should look like:

if (isset($this->columnStyles[$columnIndex])) { return $this->columnStyles[$columnIndex]; } return $this->getStyle();

@stloyd got it

sstok · 2015-03-26T10:49:17Z

src/Symfony/Component/Console/Helper/Table.php

+     */
+    public function setColumnStyle($columnIndex, $name)
+    {
+        if (!is_numeric($columnIndex)) {


This should be is_int as 0.9 is not a valid column index but is valid for is_numeric

is_int will return false for string '23', for example

MAXakaWIZARD · 2015-03-26T19:59:52Z

@fabpot Any thoughts?

aitboudad · 2015-03-26T21:18:00Z

src/Symfony/Component/Console/Helper/Table.php

+     * Sets table style.
+     *
+     * @param int $columnIndex Column index
+     * @param TableStyle|string $name The style name or a TableStyle instance


you should apply cs fixer here to align doc block

@aitboudad done

MAXakaWIZARD · 2015-04-07T13:00:07Z

@fabpot Any chances to have this in 2.7?

fabpot · 2015-04-07T13:55:50Z

@MAXakaWIZARD Why do we provide a style for columns and not rows? Would it make sense as well?

MAXakaWIZARD · 2015-04-07T14:09:02Z

@fabpot Well, I described it in first comment. It is useful for numeric data, for example.

fabpot · 2015-10-05T14:25:12Z

👍 ping @symfony/deciders

weaverryan · 2015-10-05T15:13:00Z

👍 looks very nice. If we also wanted row styling, we could add that later.

aitboudad · 2015-10-05T15:24:49Z

src/Symfony/Component/Console/Helper/Table.php

+     */
+    public function setColumnStyle($columnIndex, $name)
+    {
+        $isIndexValid = false;


we should assume that $columnIndex is an int, IMO we can remove checking the columnIndex type

MAXakaWIZARD · 2015-10-05T15:40:40Z

@fabpot Should this PR target version 2.8?

fabpot · 2015-10-06T13:04:29Z

2.7 is fine. I will merge it automatically in the 2.8 branch anyway, no need to rebase on current 2.8.

fabpot · 2015-10-06T13:05:15Z

src/Symfony/Component/Console/Helper/Table.php

+
+        if (!$isIndexValid) {
+            throw new \InvalidArgumentException(sprintf('Invalid column index: "%s".', $columnIndex));
+        }


All the lines above can be removed.

MAXakaWIZARD · 2015-10-06T14:41:12Z

@fabpot @stof I've made changes to the code regarding your comments

fabpot · 2015-10-06T14:44:50Z

src/Symfony/Component/Console/Helper/Table.php

+
+    /**
+     * Gets the current style for specified column,
+     * if style was not set, returns global table style


I would prefer something along the lines of:

/** * Gets the current style for a column. * * If style was not set, it returns the global table style. */

fabpot · 2015-10-06T14:45:26Z

Apart from my small phpdoc comment, 👍

MAXakaWIZARD · 2015-10-06T14:48:59Z

@fabpot Done

fabpot · 2015-10-06T14:50:11Z

Can you keep the dots as in my original proposal?

fabpot · 2015-10-06T14:51:01Z

In fact, I still prefer my version than yours.

MAXakaWIZARD · 2015-10-06T14:55:35Z

@fabpot Sorry. Is it OK now?

fabpot · 2015-10-06T15:14:32Z

Looks good now. Can you rebase on current 2.7 to get rid of the merge commit? A simple git fetch origin && git rebase origin/2.7 from your branch should be enough. Thanks for your patience.

…olumns

MAXakaWIZARD · 2015-10-06T15:20:15Z

@fabpot Done. It's only one commit finally.

fabpot · 2015-10-06T15:26:09Z

Thank you @MAXakaWIZARD.

aitboudad reviewed Mar 25, 2015
View reviewed changes

aitboudad added the Console label Mar 25, 2015

MAXakaWIZARD force-pushed the pr-console-table-columns-styles branch from e88006a to 07be42a Compare March 25, 2015 09:42

sstok reviewed Mar 26, 2015
View reviewed changes

MAXakaWIZARD force-pushed the pr-console-table-columns-styles branch from 7c44129 to efba0a7 Compare March 26, 2015 19:59

MAXakaWIZARD force-pushed the pr-console-table-columns-styles branch from efba0a7 to ddd3620 Compare March 26, 2015 21:07

aitboudad reviewed Mar 26, 2015
View reviewed changes

MAXakaWIZARD force-pushed the pr-console-table-columns-styles branch from 93d0483 to 9489871 Compare March 26, 2015 21:27

aitboudad reviewed Oct 5, 2015
View reviewed changes

fabpot reviewed Oct 6, 2015
View reviewed changes

MAXakaWIZARD force-pushed the pr-console-table-columns-styles branch from 1a2a43d to 06f9c51 Compare October 6, 2015 14:48

MAXakaWIZARD force-pushed the pr-console-table-columns-styles branch 3 times, most recently from 50f0f20 to ad15c25 Compare October 6, 2015 14:55

[Console] [Helper] [Table] Add ability to set styles for individual c…

0623598

…olumns

MAXakaWIZARD force-pushed the pr-console-table-columns-styles branch from ad15c25 to 0623598 Compare October 6, 2015 15:18

fabpot closed this in 21e50d5 Oct 6, 2015

fabpot reopened this Oct 6, 2015

fabpot closed this Oct 6, 2015

fabpot mentioned this pull request Nov 16, 2015

Release v2.8.0-BETA1 #16550

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Console] [Helper] [Table] Columns styles #14044

[Console] [Helper] [Table] Columns styles #14044

MAXakaWIZARD commented Mar 24, 2015

aitboudad Mar 25, 2015

MAXakaWIZARD Mar 25, 2015

stloyd Mar 25, 2015

MAXakaWIZARD Mar 25, 2015

sstok Mar 26, 2015

MAXakaWIZARD Mar 26, 2015

MAXakaWIZARD commented Mar 26, 2015

aitboudad Mar 26, 2015

MAXakaWIZARD Mar 26, 2015

MAXakaWIZARD commented Apr 7, 2015

fabpot commented Apr 7, 2015

MAXakaWIZARD commented Apr 7, 2015

fabpot commented Oct 5, 2015

weaverryan commented Oct 5, 2015

aitboudad Oct 5, 2015

fabpot Oct 6, 2015

MAXakaWIZARD commented Oct 5, 2015

fabpot commented Oct 6, 2015

fabpot Oct 6, 2015

MAXakaWIZARD commented Oct 6, 2015

fabpot Oct 6, 2015

fabpot commented Oct 6, 2015

MAXakaWIZARD commented Oct 6, 2015

fabpot commented Oct 6, 2015

fabpot commented Oct 6, 2015

MAXakaWIZARD commented Oct 6, 2015

fabpot commented Oct 6, 2015

MAXakaWIZARD commented Oct 6, 2015

fabpot commented Oct 6, 2015

[Console] [Helper] [Table] Columns styles #14044

[Console] [Helper] [Table] Columns styles #14044

Conversation

MAXakaWIZARD commented Mar 24, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MAXakaWIZARD commented Mar 26, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MAXakaWIZARD commented Apr 7, 2015

fabpot commented Apr 7, 2015

MAXakaWIZARD commented Apr 7, 2015

fabpot commented Oct 5, 2015

weaverryan commented Oct 5, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MAXakaWIZARD commented Oct 5, 2015

fabpot commented Oct 6, 2015

Choose a reason for hiding this comment

MAXakaWIZARD commented Oct 6, 2015

Choose a reason for hiding this comment

fabpot commented Oct 6, 2015

MAXakaWIZARD commented Oct 6, 2015

fabpot commented Oct 6, 2015

fabpot commented Oct 6, 2015

MAXakaWIZARD commented Oct 6, 2015

fabpot commented Oct 6, 2015

MAXakaWIZARD commented Oct 6, 2015

fabpot commented Oct 6, 2015