Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Use grapheme_substr & pcre_match in safe_substr for #117. Ascii::columns fix.#118

Merged
miya0001 merged 3 commits into
masterwp-cli/php-cli-tools:masterfrom
issue_117wp-cli/php-cli-tools:issue_117Copy head branch name to clipboard
Aug 4, 2017
Merged

Use grapheme_substr & pcre_match in safe_substr for #117. Ascii::columns fix.#118
miya0001 merged 3 commits into
masterwp-cli/php-cli-tools:masterfrom
issue_117wp-cli/php-cli-tools:issue_117Copy head branch name to clipboard

Conversation

@gitlost

@gitlost gitlost commented Aug 3, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

Issue #117

Uses grapheme_substr() and preg_match() in safe_substr() (and grapheme_strlen() and preg_match_all() in safe_strlen()) to deal with combining characters correctly, in a manner similar to that in strwidth().

Refactors the East Asian Width stuff into a function _safe_substr_eaw() and adds can_use_pcre_x() helper to check for PCRE \X availability.

Removes use of iconv() as it varies widely over PHP versions and isn't easily tested and wasn't that useful anyway.

Incorporates the test data from @ShinichiNishikawa.

Fixes a gatepost error in Ascii::setWidths().

Also tries stty as well in Shell::columns() as relying on the env var COLUMNS as introduced by me in #113 turns out not to be that great as it's not normally exported. Sigh.

Edit: had to add an ICU check function to guard against the old 4.8.1.1 version on Travis (all PHP versions) flaking out - chose 54.1 as min which is Unicode 7.0 and fairly modern.

Also added "phpunit6-compat.php" from core (as introduced by @miya0001 in https://core.trac.wordpress.org/ticket/39822), modded to remove the getTickets(), and PHP 7.0 & 7.1 to the Travis build, and some php info and the --debug switch on phpunit.

@miya0001

miya0001 commented Aug 3, 2017

Copy link
Copy Markdown
Member

@gitlost
Oh, you are awesome! 😮
Is it ready to review?

@gitlost

gitlost commented Aug 3, 2017

Copy link
Copy Markdown
Contributor Author

Oh, you are awesome!

Ha, shucks!

Is it ready to review?

I think so, but then I thought so on the previous X versions of trying to fix this...

@danielbachhuber danielbachhuber added this to the 0.11.6 milestone Aug 3, 2017
@danielbachhuber

danielbachhuber commented Aug 3, 2017

Copy link
Copy Markdown
Member

@miya0001 @gitlost I'll tag a v0.11.6 when this lands.

@miya0001

miya0001 commented Aug 4, 2017

Copy link
Copy Markdown
Member

It seems working as expected! Great!

@miya0001 miya0001 merged commit d2a4e2e into master Aug 4, 2017
@miya0001 miya0001 mentioned this pull request Aug 4, 2017
Closed
@danielbachhuber danielbachhuber deleted the issue_117 branch August 4, 2017 10:54
@danielbachhuber

danielbachhuber commented Aug 4, 2017

Copy link
Copy Markdown
Member

Thanks @gitlost @miya0001 — shipped https://github.com/wp-cli/php-cli-tools/releases/tag/v0.11.6

@gitlost

gitlost commented Aug 4, 2017

Copy link
Copy Markdown
Contributor Author

Thanks very much @miya0001 for that testing and reviewing - I'm curious in your second example about what's pushing out the post_date column - do some dates have timezone info or something? Also, was wondering if you use the intl extension or not...

@gitlost gitlost mentioned this pull request Aug 4, 2017
Merged
@miya0001

miya0001 commented Aug 4, 2017 •
edited
Loading

Copy link
Copy Markdown
Member

@gitlost

I'm curious in your second example about what's pushing out the post_date column - do some dates have timezone info or something? Also, was wondering if you use the intl extension or not...

Ah, I tried to change the timezone, but post_date wasn't changed to current timezone.
Also the post_date wasn't changed in the dashboard too.

Following is an example of the UTC+9.

$ wp option get gmt_offset
9
$ wp post get 1761
+-----------------------+--------------------------------------+
| Field                 | Value                                |
+-----------------------+--------------------------------------+
| ID                    | 1761                                 |
| post_author           | 1                                    |
| post_date             | 2017-08-05 02:25:50                  |
| post_date_gmt         | 2017-08-04 17:25:50                  |
| post_content          | ああああ                             |
| post_title            | あいうけ                             |
| post_excerpt          |                                      |
| post_status           | publish                              |
| comment_status        | open                                 |
| ping_status           | open                                 |
| post_password         |                                      |
| post_name             | %e3%81%82%e3%81%84%e3%81%86%e3%81%91 |
| to_ping               |                                      |
| pinged                |                                      |
| post_modified         | 2017-08-05 02:25:50                  |
| post_modified_gmt     | 2017-08-04 17:25:50                  |
| post_content_filtered |                                      |
| post_parent           | 0                                    |
| guid                  | http://vccw.dev/?p=1761              |
| menu_order            | 0                                    |
| post_type             | post                                 |
| post_mime_type        |                                      |
| comment_count         | 0                                    |
+-----------------------+--------------------------------------+

Following is the example of the after changing the timezone to UTC+0.

$ wp option update gmt_offset 0
Success: Updated 'gmt_offset' option.
$ wp post get 1761
+-----------------------+--------------------------------------+
| Field                 | Value                                |
+-----------------------+--------------------------------------+
| ID                    | 1761                                 |
| post_author           | 1                                    |
| post_date             | 2017-08-05 02:25:50                  |
| post_date_gmt         | 2017-08-04 17:25:50                  |
| post_content          | ああああ                             |
| post_title            | あいうけ                             |
| post_excerpt          |                                      |
| post_status           | publish                              |
| comment_status        | open                                 |
| ping_status           | open                                 |
| post_password         |                                      |
| post_name             | %e3%81%82%e3%81%84%e3%81%86%e3%81%91 |
| to_ping               |                                      |
| pinged                |                                      |
| post_modified         | 2017-08-05 02:25:50                  |
| post_modified_gmt     | 2017-08-04 17:25:50                  |
| post_content_filtered |                                      |
| post_parent           | 0                                    |
| guid                  | http://vccw.dev/?p=1761              |
| menu_order            | 0                                    |
| post_type             | post                                 |
| post_mime_type        |                                      |
| comment_count         | 0                                    |
+-----------------------+--------------------------------------+

The value of the post_date seems to be saved as at the time of the timezone when post is saved statically.

It looks problem of the WordPress core.

@miya0001

miya0001 commented Aug 4, 2017

Copy link
Copy Markdown
Member

What do you mean "intl extension"? I guess I don't use it, the WordPress is default.

@gitlost

gitlost commented Aug 4, 2017

Copy link
Copy Markdown
Contributor Author

Ah sorry, I meant the PHP extension http://php.net/manual/en/book.intl.php - I'll take it as a no!

@gitlost gitlost mentioned this pull request Aug 9, 2017
Merged
@gitlost gitlost mentioned this pull request Aug 28, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@miya0001 miya0001 Awaiting requested review from miya0001

Assignees

No one assigned

Labels

bug

Projects

None yet

Milestone

0.11.6

Development

Successfully merging this pull request may close these issues.

3 participants

@gitlost @miya0001 @danielbachhuber
Morty Proxy This is a proxified and sanitized view of the page, visit original site.