Skip to content

[Console] Added support for definition list and horizontal table #32742

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

Merged
merged 1 commit into from
Sep 8, 2019

Conversation

lyrixx
Copy link
Member

@lyrixx lyrixx commented Jul 25, 2019

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

I need that in a projet where I want to display some data horizontally.

Usage:

<?php

use Symfony\Component\Console\Helper\TableSeparator;
use Symfony\Component\Console\Input\ArrayInput;
use Symfony\Component\Console\Output\ConsoleOutput;
use Symfony\Component\Console\Style\SymfonyStyle;

require __DIR__.'/vendor/autoload.php';

$io = new SymfonyStyle(new ArrayInput([]), new ConsoleOutput());

$io->table(['a', 'b', 'c'], [[1, 2, 3], [4, 5, 6]]);

$io->table(['a', 'b', 'c'], [[1, 2, 3], [4, 5, 6]], true);

$io->definitionList(
    ['foo' => 'bar'],
    new TableSeparator(),
    'this is a title',
    new TableSeparator(),
    ['foo2' => 'bar2']
);

image

@lyrixx lyrixx force-pushed the console-table-horizontal branch 2 times, most recently from a31ec13 to 332f318 Compare July 25, 2019 15:27
@lyrixx lyrixx force-pushed the console-table-horizontal branch 3 times, most recently from dda6c0c to 034bef5 Compare July 25, 2019 22:19
@lyrixx lyrixx changed the title [Console] Added support for definition list [Console] Added support for definition list and horizontale table Jul 25, 2019
@nicolas-grekas nicolas-grekas added this to the next milestone Jul 26, 2019
@javiereguiluz
Copy link
Member

javiereguiluz commented Jul 26, 2019

I would change the proposed usage by the following:

$io->table(['a', 'b', 'c'], [[1, 2, 3], [4, 5, 6]]);

$io->horizontalTable(['a', 'b', 'c'], [[1, 2, 3], [4, 5, 6]]);

$io->horizontalTable(['id', 'title', 'foo'], [[123, 'My title', 'bar']]);

The boolean param of table() is replace by a new method called horizontalTable(). The definitionList() method is replaced by horizontalTable() too because is the same.

What if I have an associative array with the "definition list" information? do this:

$io->horizontalTable(array_keys($foo), [array_values($foo)]);

@javiereguiluz javiereguiluz changed the title [Console] Added support for definition list and horizontale table [Console] Added support for definition list and horizontal table Jul 26, 2019
@lyrixx
Copy link
Member Author

lyrixx commented Jul 26, 2019

The definitionList() method is replaced by horizontalTable() too because is the same.

Actually, it's way easier to write:

$io->definitonList(['title' =>  'value', 'title2' => 'value2', /** very long list */]);

than

$io->definitonList(['title', 'title2', /* very long list*/], [[1, 2, , /* very long list*/]]);

because you will be lost at some point. see this command for instance

What if I have an associative array with the "definition list" information? do this:

I did that as the first implementation. But it does not work well. See the test suite to understand what I want to achieve :)

@ro0NL
Copy link
Contributor

ro0NL commented Jul 28, 2019

What about

$output->definitionList(
    ['foo' => 'bar'],
    new TableSeparator(),
    'this is a title',
    new TableSeparator(),
    ['foo' => 'bar2']
);

@lyrixx
Copy link
Member Author

lyrixx commented Jul 28, 2019

It's already supported and tested. Did I miss something?

@ro0NL
Copy link
Contributor

ro0NL commented Jul 29, 2019

Did I miss something?

Yes, the variadic api :)

i see several advantages;

  • duplicate key support (i.e. repetitive sections with the same keys)
  • the expected argument types are clear, array (list), scalar (title) or TableSeparator. In favor of one magic structure

@lyrixx
Copy link
Member Author

lyrixx commented Jul 29, 2019

Oh, indeed I miss-read your example :)
This is a good idea, but it requires extra [``] :/

@ro0NL
Copy link
Contributor

ro0NL commented Jul 30, 2019

it's still less than the current no. of brackets used

$rows = [
['<info>Symfony</>'],
new TableSeparator(),
['Version', Kernel::VERSION],
['Long-Term Support', 4 === Kernel::MINOR_VERSION ? 'Yes' : 'No'],
['End of maintenance', Kernel::END_OF_MAINTENANCE.(self::isExpired(Kernel::END_OF_MAINTENANCE) ? ' <error>Expired</>' : '')],
['End of life', Kernel::END_OF_LIFE.(self::isExpired(Kernel::END_OF_LIFE) ? ' <error>Expired</>' : '')],
new TableSeparator(),
['<info>Kernel</>'],
new TableSeparator(),
['Type', \get_class($kernel)],
['Environment', $kernel->getEnvironment()],
['Debug', $kernel->isDebug() ? 'true' : 'false'],
['Charset', $kernel->getCharset()],
['Cache directory', self::formatPath($kernel->getCacheDir(), $kernel->getProjectDir()).' (<comment>'.self::formatFileSize($kernel->getCacheDir()).'</>)'],
['Log directory', self::formatPath($kernel->getLogDir(), $kernel->getProjectDir()).' (<comment>'.self::formatFileSize($kernel->getLogDir()).'</>)'],
new TableSeparator(),
['<info>PHP</>'],
new TableSeparator(),
['Version', PHP_VERSION],
['Architecture', (PHP_INT_SIZE * 8).' bits'],
['Intl locale', class_exists('Locale', false) && \Locale::getDefault() ? \Locale::getDefault() : 'n/a'],
['Timezone', date_default_timezone_get().' (<comment>'.(new \DateTime())->format(\DateTime::W3C).'</>)'],
['OPcache', \extension_loaded('Zend OPcache') && filter_var(ini_get('opcache.enable'), FILTER_VALIDATE_BOOLEAN) ? 'true' : 'false'],
['APCu', \extension_loaded('apcu') && filter_var(ini_get('apc.enabled'), FILTER_VALIDATE_BOOLEAN) ? 'true' : 'false'],
['Xdebug', \extension_loaded('xdebug') ? 'true' : 'false'],
];
:} (here we also see multiple Version keys btw)

in general i tend to avoid the mixed array structure. The 3rd advantage actually is it supports numeric/integer keys like any other, so less variance.

@lyrixx
Copy link
Member Author

lyrixx commented Jul 30, 2019

@ro0NL Actually, It will be the exact same number of brackets :)
Anyway, I will try to implement your proposal

@lyrixx lyrixx force-pushed the console-table-horizontal branch 3 times, most recently from 0f6106c to 09b244a Compare August 27, 2019 16:13
@lyrixx
Copy link
Member Author

lyrixx commented Aug 27, 2019

@ro0NL I finished this PR. I hope you like it :)

Copy link
Member

@chalasr chalasr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice :)

@lyrixx lyrixx force-pushed the console-table-horizontal branch 4 times, most recently from 08a1e18 to a5b71b5 Compare September 2, 2019 09:48
@lyrixx lyrixx force-pushed the console-table-horizontal branch from 1bf1af7 to 66028fe Compare September 5, 2019 09:05
@fabpot
Copy link
Member

fabpot commented Sep 8, 2019

Thank you @lyrixx.

fabpot added a commit that referenced this pull request Sep 8, 2019
…ntal table (lyrixx)

This PR was merged into the 4.4 branch.

Discussion
----------

[Console] Added support for definition list and horizontal table

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

I need that in a projet where I want to display some data horizontally.

Usage:
```php
<?php

use Symfony\Component\Console\Helper\TableSeparator;
use Symfony\Component\Console\Input\ArrayInput;
use Symfony\Component\Console\Output\ConsoleOutput;
use Symfony\Component\Console\Style\SymfonyStyle;

require __DIR__.'/vendor/autoload.php';

$io = new SymfonyStyle(new ArrayInput([]), new ConsoleOutput());

$io->table(['a', 'b', 'c'], [[1, 2, 3], [4, 5, 6]]);

$io->table(['a', 'b', 'c'], [[1, 2, 3], [4, 5, 6]], true);

$io->definitionList(
    ['foo' => 'bar'],
    new TableSeparator(),
    'this is a title',
    new TableSeparator(),
    ['foo2' => 'bar2']
);
```

![image](https://user-images.githubusercontent.com/408368/63788677-2df43580-c8f6-11e9-9ce6-b7abcecf7f24.png)

Commits
-------

66028fe [Console] Added support for definition list
@fabpot fabpot merged commit 66028fe into symfony:4.4 Sep 8, 2019
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.4 Oct 27, 2019
This was referenced Nov 12, 2019
@lyrixx lyrixx deleted the console-table-horizontal branch May 12, 2021 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants