Title: Colors::length returns incorrect lengths for colorized strings · Issue #59 · wp-cli/php-cli-tools · GitHub
Open Graph Title: Colors::length returns incorrect lengths for colorized strings · Issue #59 · wp-cli/php-cli-tools
X Title: Colors::length returns incorrect lengths for colorized strings · Issue #59 · wp-cli/php-cli-tools
Description: When you pass a string to Colors::length that has already been colorized with Colors::colorized, the wrong result is given. This issue resulted in incorrectly formatted tables using \cli\Table::display(). To reproduce: Add the following ...
Open Graph Description: When you pass a string to Colors::length that has already been colorized with Colors::colorized, the wrong result is given. This issue resulted in incorrectly formatted tables using \cli\Table::dis...
X Description: When you pass a string to Colors::length that has already been colorized with Colors::colorized, the wrong result is given. This issue resulted in incorrectly formatted tables using \cli\Table::dis...
Opengraph URL: https://github.com/wp-cli/php-cli-tools/issues/59
X: @github
Domain: github.com
{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"Colors::length returns incorrect lengths for colorized strings","articleBody":"When you pass a string to `Colors::length` that has already been colorized with `Colors::colorized`, the wrong result is given. This issue resulted in incorrectly formatted tables using `\\cli\\Table::display()`.\n\nTo reproduce:\n1. Add the following WP CLI command:\n \n ``` php\n \u003c?php\n \n class ReducedTestCase extends WP_CLI_Command {\n function test() {\n $success = \\cli\\Colors::colorize( \"%Gx%n\" );\n \n // Display results\n $table = new \\cli\\Table();\n $table->setHeaders( array( 'Test Header' ) );\n $table->setRows( array( array( $success )) );\n $table->display();\n }\n }\n \n WP_CLI::add_command( 'failwhale', 'ReducedTestCase' );\n ```\n2. The output produced is:\n \n ``` bash\n vagrant@vvv:/srv/www/wordpress-develop/src$ wp failwhale test\n +--------------+\n | Test Header |\n +--------------+\n | x |\n +--------------+\n ```\n \n Note that the output _is colorized_ even though I cannot indicate that in the code above.\n3. The expected output is:\n \n ``` bash\n vagrant@vvv:/srv/www/wordpress-develop/src$ wp failwhale test\n +--------------+\n | Test Header |\n +--------------+\n | x |\n +--------------+\n ```\n\nThis problem is occurring because before the content is passed to the table methods, it is colorized. Colorizing the string turns it from:\n\n`%Gx%n` to something like `\\033[32;1mx\\033[0m`\n\nThe string length counts are thusly messed up. In fact, I'd argue that the string lengths, for any version of colorized values have always been messed up. The first value above would return a length of `5` and the second a value of `18` when really, the string length should be `1` as the only non-color value is `x`.\n\nI was able to implement a really sloppy proof of concept fix for the issue:\n\n``` php\nstatic public function length($string) {\n // Convert the string back to it's original state\n $conversions = array(\n '%y' => array('color' => 'yellow'),\n '%g' => array('color' => 'green'),\n '%b' => array('color' => 'blue'),\n '%r' => array('color' => 'red'),\n '%p' => array('color' => 'magenta'),\n '%m' => array('color' => 'magenta'),\n '%c' => array('color' => 'cyan'),\n '%w' => array('color' => 'grey'),\n '%k' => array('color' => 'black'),\n '%n' => array('color' => 'reset'),\n '%Y' => array('color' => 'yellow', 'style' => 'bright'),\n '%G' => array('color' => 'green', 'style' => 'bright'),\n '%B' => array('color' => 'blue', 'style' => 'bright'),\n '%R' => array('color' => 'red', 'style' => 'bright'),\n '%P' => array('color' => 'magenta', 'style' => 'bright'),\n '%M' => array('color' => 'magenta', 'style' => 'bright'),\n '%C' => array('color' => 'cyan', 'style' => 'bright'),\n '%W' => array('color' => 'grey', 'style' => 'bright'),\n '%K' => array('color' => 'black', 'style' => 'bright'),\n '%N' => array('color' => 'reset', 'style' => 'bright'),\n '%3' => array('background' => 'yellow'),\n '%2' => array('background' => 'green'),\n '%4' => array('background' => 'blue'),\n '%1' => array('background' => 'red'),\n '%5' => array('background' => 'magenta'),\n '%6' => array('background' => 'cyan'),\n '%7' => array('background' => 'grey'),\n '%0' => array('background' => 'black'),\n '%F' => array('style' => 'blink'),\n '%U' => array('style' => 'underline'),\n '%8' => array('style' => 'inverse'),\n '%9' => array('style' => 'bright'),\n '%_' => array('style' => 'bright')\n );\n\n foreach ($conversions as $key => $value) {\n $string = str_replace(self::color($value), '', self::colorize($string, true));\n }\n\n return safe_strlen(self::colorize($string, true));\n}\n```\n\nAs a proof of concept, this simply converts the colorized string to the original string without any color information. This gets the _true_ string length value. Furthermore, this gives the expected results in the table display. This is a sloppy fix and it produced noticeable performance issues.\n\nI think there are a lot of different approaches to fixing this issue. An approach that I think could work would be too:\n1. Whenever a new value is passed to `Colors::colorize`, store that value in memory and specifically track:\n - String without _any color information_\n - The original string value\n - The colorized value\n2. Change the `Colors::length` method to do a simple `strlen` call on the _string without any color information_.\n\nI think this method would be backwards compatible, and provide a nice performance boost by removing redundant color conversions.\n\nAdditionally, I think some unit testing should be added to show how broken these methods are.\n","author":{"url":"https://github.com/tollmanz","@type":"Person","name":"tollmanz"},"datePublished":"2014-08-03T07:11:37Z","url":"https://github.com/wp-cli/php-cli-tools/issues/59"}
| react-profiling | 0 |
| react-import-map | react,react-dom,react-dom/client,react-dom/profiling,react-is,react-reconciler,react/compiler-runtime,react/jsx-dev-runtime,react/jsx-runtime,scheduler@d5750093facf |
| route-pattern | /_view_fragments/issues/show/:user_id/:repository/:id/issue_layout(.:format) |
| route-controller | voltron_issues_fragments |
| route-action | issue_layout |
| fetch-nonce | v2:698c4f60-e4c9-f5eb-d125-c41864bded5c |
| current-catalog-service-hash | 81bb79d38c15960b92d99bca9288a9108c7a47b18f2423d0f6438c5b7bcd2114 |
| request-id | B3F8:2B0961:3C2F293:4EEF12C:6AFA1786 |
| html-safe-nonce | ad1965db80bd2deeed6cc22c4d035901d9398eea548d10e3601135d16fda660e |
| visitor-payload | eyJyZWZlcnJlciI6IiIsInJlcXVlc3RfaWQiOiJCM0Y4OjJCMDk2MTozQzJGMjkzOjRFRUYxMkM6NkFGQTE3ODYiLCJ2aXNpdG9yX2lkIjoiODI5MzQxMzIwMzE1ODAzNTI2IiwicmVnaW9uX2VkZ2UiOiJpYWQiLCJyZWdpb25fcmVuZGVyIjoiaWFkIn0= |
| visitor-hmac | 69825a3849a8df61e837e1ad1c1cccea25c4b1f8ee7bf8ad6611550f20265d06 |
| hovercard-subject-tag | issue:39365096 |
| github-keyboard-shortcuts | repository,issues,copilot |
| google-site-verification | Apib7-x98H0j5cPqHWwSMm6dNU4GmODRoqxLiDzdx9I |
| octolytics-url | https://collector.github.com/github/collect |
| analytics-location | / |
| fb:app_id | 1401488693436528 |
| apple-itunes-app | app-id=1477376905, app-argument=https://github.com/_view_fragments/issues/show/wp-cli/php-cli-tools/59/issue_layout |
| twitter:image | https://opengraph.githubassets.com/c5c95027b00f72024623f3d57f607f1ec40a554b66edbb3ee8d4c32b2146b7c0/wp-cli/php-cli-tools/issues/59 |
| twitter:card | summary_large_image |
| og:image | https://opengraph.githubassets.com/c5c95027b00f72024623f3d57f607f1ec40a554b66edbb3ee8d4c32b2146b7c0/wp-cli/php-cli-tools/issues/59 |
| og:image:alt | When you pass a string to Colors::length that has already been colorized with Colors::colorized, the wrong result is given. This issue resulted in incorrectly formatted tables using \cli\Table::dis... |
| og:image:width | 1200 |
| og:image:height | 600 |
| og:site_name | GitHub |
| og:type | object |
| og:author:username | tollmanz |
| hostname | github.com |
| expected-hostname | github.com |
| None | 612d3e6ffd693e7ea8d9fcf452370995872baef31d825b1fedef704cd3d24ac7 |
| turbo-cache-control | no-preview |
| go-import | github.com/wp-cli/php-cli-tools git https://github.com/wp-cli/php-cli-tools.git |
| octolytics-dimension-user_id | 1570774 |
| octolytics-dimension-user_login | wp-cli |
| octolytics-dimension-repository_id | 587642 |
| octolytics-dimension-repository_nwo | wp-cli/php-cli-tools |
| octolytics-dimension-repository_public | true |
| octolytics-dimension-repository_is_fork | false |
| octolytics-dimension-repository_network_root_id | 587642 |
| octolytics-dimension-repository_network_root_nwo | wp-cli/php-cli-tools |
| turbo-body-classes | logged-out env-production page-responsive |
| disable-turbo | false |
| browser-stats-url | https://api.github.com/_private/browser/stats |
| browser-errors-url | https://api.github.com/_private/browser/errors |
| release | 072e34035c13c31543fba9f025cbbbda4210dae4 |
| ui-target | full |
| theme-color | #1e2327 |
| color-scheme | light dark |
| react-app-name | issues |
Links:
Viewport: width=device-width