Skip to content
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

x/text: Preserve modifiers in AppendReverse() and ReverseString() #44

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

elliotwutingfeng
Copy link

@elliotwutingfeng elliotwutingfeng commented Jun 23, 2023

Issue

Current implementation of AppendReverse() and ReverseString() does not check for modifier runes, which causes modifiers to be applied to wrong characters after string is reversed.

Proposed change

AppendReverse() and ReverseString() to check for modifier runes. Unicode Character Categories M and Sk are considered as modifiers in this proposed change.

Questions

  • Is it safe to assume that categories M and Sk contain only modifiers?
  • Are there any unicode modifiers not covered by categories M and Sk?

Unresolved

  • Current proposed change does not handle grapheme clusters that use zero-width joiners sandwiched between non-modifier runes like 🏳️‍🌈 and ണ്‍. More information on grapheme clusters: https://unicode.org/reports/tr29
  • A third-party package that handles grapheme clusters: https://github.com/rivo/uniseg (It also has a ReverseString() function)

Fixes golang/go#50633

Current implementation does not check for modifier runes, which causes modifiers to be applied to wrong characters after string is reversed. Unicode Character Categories M and Sk are considered as modifiers.

Fixes golang/go#50633
@gopherbot
Copy link
Contributor

This PR (HEAD: 58b4a27) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/text/+/505775 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@elliotwutingfeng elliotwutingfeng marked this pull request as draft June 24, 2023 08:10
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.

x/text/unicode/bidi: ReverseString doesn't preserve modifiers
2 participants