Skip to content

PR 109 (double-width wrapping) and issue 106 (pre-colorization).#111

Merged
miya0001 merged 9 commits intowp-cli:masterfrom
gitlost:issue_106
Jul 25, 2017
Merged

PR 109 (double-width wrapping) and issue 106 (pre-colorization).#111
miya0001 merged 9 commits intowp-cli:masterfrom
gitlost:issue_106

Conversation

@gitlost
Copy link
Contributor

@gitlost gitlost commented Jul 25, 2017

PR #109 and issue #106

For PR #109 (incorrect wrapping of East Asian double-width chars in tables), adds an is_width arg to safe_substr() to make it interpret the length arg as a spacing width, and uses the East Asian Width regex (now refactored to load in a function) to adjust accordingly (a slow algorithm but couldn't think of anything better). Uses this in Ascii::row().

For issue #106 (related wp-cli/wp-cli#2458), adds pre_colorized arg to various functions, and adds Ascii::set/isPreColorized() and Table::set/isAsciiPreColorize() (the latter to be used via a corresponding ascii_pre_colorized arg to be added to WP_CLI/Formatter, see gitlost/wp-cli#15.)

Also centralizes decolorization into Colors::decolorize(), adding a keep arg to optionally keep either tokens or encodings.

Also makes sure Colors::colorize() only reads from cache after checking shouldColorize().

Also marks as unused the colored arg to Colors::cacheString(), and stops using the decolorized cache element in length() and width() as it never gets looked up if passed a colorized string (as they're indexed on the original string).

Also replicates the padding logic of safe_str_pad() in Colors::pad() and moves decolorization to Colors::width(), as it seems better that low-level funcs don't reference Colors.

Also makes length arg of safe_substr() cross PHP compat whether null or false.

Also adds an optional encoding arg to various functions to save some processing, and makes mb_detect_encoding() use strict.

@danielbachhuber danielbachhuber requested a review from miya0001 July 25, 2017 19:21
@danielbachhuber
Copy link
Member

Looks good on my end. Flagging a review from @miya0001 for a double-check

Copy link
Member

@miya0001 miya0001 left a comment

Choose a reason for hiding this comment

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

@gitlost

Great! This is the most beautiful table I have ever seen before!

@miya0001 miya0001 merged commit 2d2b582 into wp-cli:master Jul 25, 2017
@gitlost
Copy link
Contributor Author

gitlost commented Jul 25, 2017

Thanks @miya0001 - it's great to have a native kanji and kana user on board!

@danielbachhuber
Copy link
Member

Thanks for your work on this, @gitlost and @miya0001. I've tagged a v0.11.4 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants