Replace tabs in tables with 4 spaces #181

swissspidy · 2025-03-03T14:49:39Z

While testing #179, I realized that tabs (\t) have always slightly messed up table formatting due to tab length.

By replacing them with spaces, the length calculation is correct again.

mrsdizzie · 2025-03-03T15:41:55Z

I tested this but I don't think it works properly because of the existing logic. Right now it only directly modifies $row to use $value if this condition is met :

php-cli-tools/lib/cli/table/Ascii.php

Line 144 in 77616d6

    
           if ( $col_width && ( $original_val_width > $col_width || strpos( $value, "\n" ) !== false ) ) {

But if it isn't, then it just uses the original $row data to calculate padding:

php-cli-tools/lib/cli/table/Ascii.php

Lines 168 to 172 in 77616d6

    
           $row = array_map(array($this, 'padColumn'), $row, array_keys($row)); 
        
           array_unshift($row, ''); // First border 
        
           array_push($row, ''); // Last border 
        
           $ret = join($this->_characters['border'], $row);

So it doesn't know about the tab replacement always.

I saw this in two ways when testing with the value apple\tbanana\tmango

When I first added it, output looked like this:

+---------+----------+--------------------+
| post_id | meta_key | meta_value         |
+---------+----------+--------------------+
| 1       | fruits   | apple              |
|         |          | banana             |
|         |          | mango              |
| 1       | foo      | bar                |
| 1       | fruitz   | apple    banana    |
|         |          |  mango             |
+---------+----------+--------------------+

Which seemed odd. Then when I added a longer string as another value, the table gets broken by the tab row again:

+---------+----------+------------------------------------------------------------------------+
| post_id | meta_key | meta_value                                                             |
+---------+----------+------------------------------------------------------------------------+
| 1       | fruits   | apple                                                                  |
|         |          | banana                                                                 |
|         |          | mango                                                                  |
| 1       | foo      | bar                                                                    |
| 1       | fruitz   | apple	banana	mango                                                     |
| 1       | fruitzz  | this is a really really really really really really really long string |
+---------+----------+------------------------------------------------------------------------+

Maybe a better place for this fix would be to modify $content here:

php-cli-tools/lib/cli/table/Ascii.php

Lines 200 to 202 in 77616d6

    
           private function padColumn($content, $column) { 
        
           	return $this->_characters['padding'] . Colors::pad( $content, $this->_widths[ $column ], $this->isPreColorized( $column ) ) . $this->_characters['padding']; 
        
           }

To make sure it always removes tabs from the content no matter what

swissspidy · 2025-03-03T15:59:28Z

Yes that makes more sense, thanks!

This adds the same functionality from both #179 and #181 to the tabular table output. In WP CLI behat tests, the 'table containing rows' check can only use tabular output so we need to fix it here in order for tests to work.

Replace tabs in tables with 4 spaces

0daf544

swissspidy added bug scope:table labels Mar 3, 2025

swissspidy requested a review from a team as a code owner March 3, 2025 14:49

swissspidy added this to the 0.12.2 milestone Mar 3, 2025

swissspidy added 2 commits March 3, 2025 16:57

Merge branch 'master' into replace-tabs

3234f3d

Move to padColumn

ab44ee3

Cast to string

bc28236

mrsdizzie approved these changes Mar 3, 2025

View reviewed changes

swissspidy merged commit 8063b4d into master Mar 3, 2025
17 checks passed

swissspidy deleted the replace-tabs branch March 3, 2025 16:22

mrsdizzie mentioned this pull request Mar 4, 2025

Support line breaks and tab replacement in tabular table values #182

Merged

swissspidy mentioned this pull request Mar 5, 2025

Possible regression in #182 #183

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Replace tabs in tables with 4 spaces #181

Replace tabs in tables with 4 spaces #181

Uh oh!

swissspidy commented Mar 3, 2025

Uh oh!

mrsdizzie commented Mar 3, 2025

Uh oh!

swissspidy commented Mar 3, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Replace tabs in tables with 4 spaces #181

Replace tabs in tables with 4 spaces #181

Uh oh!

Conversation

swissspidy commented Mar 3, 2025

Uh oh!

mrsdizzie commented Mar 3, 2025

Uh oh!

swissspidy commented Mar 3, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants