New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

String.reverse doesn't handle unicode characters properly #625

Closed
eoftedal opened this Issue May 26, 2016 · 5 comments

Comments

Projects
None yet
2 participants
@eoftedal

eoftedal commented May 26, 2016

Examples from repl:

With combining unicode character (U+0300), the combining character becomes attached to the wrong character:

> String.reverse "màn"
"ǹam" : String 

With astral symbols (four byte UTF-8 character: U+1D306) it seems the surrogate pair is reversed:

> String.reverse "𝌆"
"��" : String
@eoftedal

This comment has been minimized.

Show comment
Hide comment
@eoftedal

eoftedal commented May 26, 2016

> String.length "𝌆"
2 : Int

https://github.com/elm-lang/core/pull/626

@eoftedal

This comment has been minimized.

Show comment
Hide comment

eoftedal commented May 26, 2016

@eoftedal

This comment has been minimized.

Show comment
Hide comment
@eoftedal

eoftedal commented May 26, 2016

Pull request for reversing: https://github.com/elm-lang/core/pull/627

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Sep 23, 2016

Member

Tracking in #725

Member

evancz commented Sep 23, 2016

Tracking in #725

@evancz evancz closed this Sep 23, 2016

@eoftedal

This comment has been minimized.

Show comment
Hide comment
@eoftedal

eoftedal Nov 4, 2016

@evancz In #725 you should also include the count issue from my second comment in this thread

eoftedal commented Nov 4, 2016

@evancz In #725 you should also include the count issue from my second comment in this thread

evancz added a commit that referenced this issue Mar 25, 2017

Fix #625, String.reverse works outside of BMP
Based on https://jsperf.com/utf16-string-reverse/1 it seems like the
new code is faster in Chrome and Safari, and the same in Opera.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment