Skip to content
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

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

Open
2 tasks done
slaFFik opened this issue Sep 28, 2023 · 1 comment · May be fixed by #165
Open
2 tasks done

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

slaFFik opened this issue Sep 28, 2023 · 1 comment · May be fixed by #165
Labels
scope:table

Comments

@slaFFik
Copy link
Contributor

slaFFik commented Sep 28, 2023 •
edited
Loading

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.
slaFFik added a commit to slaFFik/php-cli-tools that referenced this issue Sep 28, 2023
@slaFFik
Table::getDisplayLines() should not output excessive lines
4d4b774 
Fix wp-cli#164.
@slaFFik slaFFik linked a pull request Sep 28, 2023 that will close this issue
Open
@danielbachhuber
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
scope:table
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Table::getDisplayLines() should not output excessive lines when no rows slaFFik/php-cli-tools
2 participants
@danielbachhuber @slaFFik