This PR ports the wcswidth() function (seehttp://man7.org/linux/man-pages/man3/wcwidth.3.html andhttps://www.cl.cam.ac.uk/~mgk25/ucs/wcwidth.c) into the String component. This new method will be useful in the Console component to determine how many columns a character takes.

I kind of copied the Intl data import strategy.

carsonbot added Status: Needs Review Feature labels

Jan 1, 2020

fancyweb force-pushed thestring-wcswidth branch fromdd7eb72 toeba29c0Compare

January 1, 2020 13:58

fancyweb commented

Jan 1, 2020

View reviewed changes

src/Symfony/Component/String/AbstractUnicodeString.php OutdatedShow resolvedHide resolved

fancyweb commented

Jan 1, 2020

View reviewed changes

src/Symfony/Component/String/Tests/AbstractUnicodeTestCase.php OutdatedShow resolvedHide resolved

fancyweb force-pushed thestring-wcswidth branch fromeba29c0 to274edfcCompare

January 1, 2020 14:15

nicolas-grekas added this to thenext milestone

Jan 4, 2020

nicolas-grekas force-pushed thestring-wcswidth branch froma7fa164 to3c6e292Compare

January 13, 2020 14:00

nicolas-grekas reviewed

Jan 13, 2020

View reviewed changes

Copy link

Member

nicolas-grekas left a comment

Choose a reason for hiding this comment

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

Thanks, that looks good to me.
I pushed a second commit that makes the most important changes I think we should do.
Namely, we don't want to add a new method, but to improve the existing one.
That's why I madewcwidth() private, and use it inwidth().
Of course, I didn't finish the refacto so tests won't pass anymore :P

src/Symfony/Component/String/Data/WcswidthDataGenerator.php OutdatedShow resolvedHide resolved

src/Symfony/Component/String/Resources/bin/update-data.php OutdatedShow resolvedHide resolved

nicolas-grekas changed the title~~[String] Add the wcswdith() method~~[String] Made AbstractString::width() follow POSIX.1-2001

Jan 13, 2020

stof reviewed

Jan 13, 2020

View reviewed changes

src/Symfony/Component/String/Resources/bin/common.php OutdatedShow resolvedHide resolved

fancyweb force-pushed thestring-wcswidth branch 4 times, most recently from6ecc13e to495d959Compare

January 15, 2020 08:00

nicolas-grekas reviewed

Jan 15, 2020

View reviewed changes

src/Symfony/Component/String/AbstractString.php OutdatedShow resolvedHide resolved

Copy link

ContributorAuthor

fancyweb commentedJan 16, 2020

@nicolas-grekas Could we however somehow have a public accessible pure implementation of wcswidth in the String component? With moving the wcswidth() method logic somewhere else for example. Symfony wcswidth implementation could become the most robust tested PHP implementation.

Copy link

Member

nicolas-grekas commentedJan 20, 2020

Could we however somehow have a public accessible pure implementation of wcswidth in the String component?

Honestly, I'm not sure until someone gives us a real-world use case... width() looks more useful to me...

fancyweb force-pushed thestring-wcswidth branch 3 times, most recently frombd8e0b3 to25b3499Compare

January 20, 2020 17:37

nicolas-grekas approved these changes

Jan 23, 2020

View reviewed changes

Copy link

Member

nicolas-grekas left a comment

Choose a reason for hiding this comment

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

(with minor comments)

src/Symfony/Component/String/AbstractUnicodeString.php OutdatedShow resolvedHide resolved

src/Symfony/Component/String/CHANGELOG.md OutdatedShow resolvedHide resolved

carsonbot added Status: Reviewed and removed Status: Needs Review labels

Jan 23, 2020

fancyweb force-pushed thestring-wcswidth branch from25b3499 to4967e13Compare

January 23, 2020 09:35

[String] Made AbstractString::width() follow POSIX.1-2001

347d825

Co-authored-by: Nicolas Grekas <nicolas.grekas@gmail.com>

fancyweb force-pushed thestring-wcswidth branch from4967e13 to347d825Compare

January 23, 2020 10:38

fabpot approved these changes

Jan 30, 2020

View reviewed changes

Copy link

Member

fabpot commentedJan 30, 2020

Thank you@fancyweb.

fabpot added a commit that referenced this pull request

Jan 30, 2020

feature#35156[String] Made AbstractString::width() follow POSIX.1-2…

75fc3fa

…001 (fancyweb)This PR was merged into the 5.1-dev branch.Discussion----------[String] Made AbstractString::width() follow POSIX.1-2001| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -This PR ports the wcswidth() function (seehttp://man7.org/linux/man-pages/man3/wcwidth.3.html andhttps://www.cl.cam.ac.uk/~mgk25/ucs/wcwidth.c) into the String component. This new method will be useful in the Console component to determine how many columns a character takes.I kind of copied the Intl data import strategy.Commits-------347d825 [String] Made AbstractString::width() follow POSIX.1-2001

fabpot mentioned this pull request

Jan 30, 2020

[String] Made AbstractString::width() follow POSIX.1-2001symfony/symfony-docs#13013

Closed

fabpot merged commit347d825 intosymfony:master

Jan 30, 2020

fancyweb deleted the string-wcswidth branch

January 30, 2020 13:54

nicolas-grekas modified the milestones:next,5.1

May 4, 2020

fabpot mentioned this pull request

May 5, 2020