Skip to content

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

Merged
miya0001 merged 3 commits intomasterfrom
issue_117
Aug 4, 2017
Merged

Use grapheme_substr & pcre_match in safe_substr for #117. Ascii::columns fix.#118
miya0001 merged 3 commits intomasterfrom
issue_117

Conversation

@gitlost
Copy link
Contributor

@gitlost gitlost commented Aug 3, 2017 •
edited
Loading

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
Copy link
Member

miya0001 commented Aug 3, 2017

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

@gitlost
Copy link
Contributor Author

gitlost commented Aug 3, 2017

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
Copy link
Member

danielbachhuber commented Aug 3, 2017

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

@miya0001
Copy link
Member

miya0001 commented Aug 4, 2017

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
Copy link
Member

danielbachhuber commented Aug 4, 2017

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

@gitlost
Copy link
Contributor Author

gitlost commented Aug 4, 2017

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
Copy link
Member

miya0001 commented Aug 4, 2017 •
edited
Loading

@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
Copy link
Member

miya0001 commented Aug 4, 2017

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

@gitlost
Copy link
Contributor Author

gitlost commented Aug 4, 2017

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