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

Jump to bottom

x/text/unicode/bidi: ReverseString doesn't preserve modifiers #50633

Open
hajimehoshi opened this issue Jan 15, 2022 · 3 comments · May be fixed by golang/text#44
Open

x/text/unicode/bidi: ReverseString doesn't preserve modifiers #50633

hajimehoshi opened this issue Jan 15, 2022 · 3 comments · May be fixed by golang/text#44
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@hajimehoshi
Copy link
Member

hajimehoshi commented Jan 15, 2022 •
edited

What version of Go are you using (go version)?

$ go version
go version go1.17.4 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/hajimehoshi/Library/Caches/go-build"
GOENV="/Users/hajimehoshi/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/hajimehoshi/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/hajimehoshi/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.17.4"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/hajimehoshi/ebiten/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/60/khbk2xqn1c5bml1byjn89dwc0000gn/T/go-build4087252806=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Run this with x/text 0.3.7

package main

import (
	"fmt"

	"golang.org/x/text/unicode/bidi"
)

func main() {
	str := "a\u0308u"
	fmt.Println(str)
	fmt.Println(bidi.ReverseString(str))
}

What did you expect to see?

äu
uä

The document of ReverseString says:

Modifiers will still follow the runes they modify.

So the modifiers should be reserved. Probably this is the same for AppendReverse.

By the way, in the ideal world, should all the graphemes be reserved (e.g., emoji flags)?

What did you see instead?

äu
üa

CC /@pgundlach
@gopherbot gopherbot added this to the Unreleased milestone Jan 15, 2022
@hajimehoshi
Copy link
Member Author

Modifiers will still follow the runes they modify.

Oops, I was misunderstanding this sentence, but is this expected that the position of modifiers are swapped with the modifying targets?

@pgundlach
Copy link
Contributor

I admit that this looks unexpected and strange. When I implemented the function I just followed the explanation in the doc string and thought that the implementation follows this.

I have no idea what the ReverseString was good for in the first place.

@hajimehoshi
Copy link
Member Author

I see. It was not clear to me what 'still' in the doc meant.

I have no idea what the ReverseString was good for in the first place.

This can be useful to reorder Arabic presentation forms characters in my case.

@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 16, 2022
elliotwutingfeng added a commit to elliotwutingfeng/text that referenced this issue Jun 23, 2023
@elliotwutingfeng
unicode: Preserve modifiers in AppendReverse() and ReverseString()
58b4a27 
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
@elliotwutingfeng elliotwutingfeng linked a pull request Jun 23, 2023 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unreleased
Development

Successfully merging a pull request may close this issue.

x/text: Preserve modifiers in AppendReverse() and ReverseString() elliotwutingfeng/text
4 participants
@hajimehoshi @pgundlach @gopherbot @seankhliao