wp-cli / php-cli-tools

A collection of tools to help with PHP command line utilities
MIT License
673 stars 118 forks source link

Table::getDisplayLines() can output excessive lines #164

Open slaFFik opened 1 year ago

slaFFik commented 1 year ago

Bug Report

Describe the current, buggy behavior

I'm registering a table with this (simplified) code:

$table  = new \cli\Table();
$ascii  = new \cli\table\Ascii();
$widths = [
    3, // this is the longest default date format: [24-Sep-2023 20:50:51 UTC]
    3, // this is the longest default type: PHP Fatal error
    5, // trigger the overflow and last column max width by defining a very long string length
];

$ascii->setWidths( $widths );
$table->setRenderer( $ascii );

$table->setHeaders( [ 'one', 'two', 'three' ] );

$table->display();

Method Table::() returns this data:

array:4 [▶
  0 => "+----------------------------+-----------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+"
  1 => "| One                       | Two            | Three                                                                                                                                                                                                                                                             |"
  2 => "+----------------------------+-----------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+"
  3 => "+----------------------------+-----------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+"
]

As you can see, array values for 2 and 3 are the same - bottom border.

This is because I output just the table, without rows (I plan to use ->addRow() later). And because this code in this library:

foreach ($this->_rows as $row) {
    $row = $this->_renderer->row($row);
    $row = explode( PHP_EOL, $row );
    $out = array_merge( $out, $row );
}

if (isset($border)) {
    $out[] = $border;
}

Does not check whether rows is actually not empty.

Describe what you would expect as the correct outcome

If there is a header but no rows - we should display only 1 border.

Let us know what environment you are running this on

OS:     Darwin 22.6.0 Darwin Kernel Version 22.6.0: Fri Sep 15 13:41:28 PDT 2023; root:xnu-8796.141.3.700.8~1/RELEASE_ARM64_T6000 arm64
Shell:  /bin/zsh
PHP binary:     /opt/homebrew/Cellar/php/8.2.10/bin/php
PHP version:    8.2.10
php.ini used:   /opt/homebrew/etc/php/8.2/php.ini
MySQL binary:   
MySQL version:  
SQL modes:      
WP-CLI root dir:        phar://wp-cli.phar/vendor/wp-cli/wp-cli
WP-CLI vendor dir:      phar://wp-cli.phar/vendor
WP_CLI phar path:       /Users/slaffik/Projects/[redacted]
WP-CLI packages dir:    /Users/slaffik/.wp-cli/packages/
WP-CLI cache dir:       /Users/slaffik/.wp-cli/cache
WP-CLI global config:   
WP-CLI project config:  
WP-CLI version: 2.8.1

Provide a possible solution

I will create a PR.

danielbachhuber commented 1 year ago

If there is a header but no rows - we should display only 1 border.

To preserve backcompat, I think we should make this behavior opt-in with some new flag.