x/text/unicode/bidi: brackets not recognized

[pgundlach]

Go version

go version go1.24.0 darwin/arm64

Output of go env in your module/workspace:

AR='ar'
CC='cc'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='c++'
GCCGO='gccgo'
GO111MODULE=''
GOARCH='arm64'
GOARM64='v8.0'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/var/folders/md/l2nnr5490tq114003qtxfnk40000gn/T//gocache'
GOCACHEPROG=''
GODEBUG=''
GOENV='/Users/patrick/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/md/l2nnr5490tq114003qtxfnk40000gn/T/go-build3328391977=/tmp/go-build -gno-record-gcc-switches -fno-common'
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMOD='/Users/patrick/prog/go/segmentize/go.mod'
GOMODCACHE='/Users/patrick/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/patrick/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.24.0/libexec'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/patrick/Library/Application Support/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.24.0/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.24.0'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

package main

import (
	"fmt"
	"log"

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

func dothings() error {
	text := "ع a (b)"

	p := bidi.Paragraph{}
	_, err := p.SetString(text, bidi.DefaultDirection(bidi.RightToLeft))
	if err != nil {
		return err
	}

	order, err := p.Order()
	if err != nil {
		return err
	}

	for v := range order.NumRuns() {
		thisrun := order.Run(v)
		fmt.Println(thisrun.Direction())
		fmt.Printf("~~> thisrun.String() %#v\n", thisrun.String())
	}
	return nil
}

func main() {
	if err := dothings(); err != nil {
		log.Fatal(err)
	}
}

What did you see happen?

1
~~> thisrun.String() "ع "
0
~~> thisrun.String() "a (b"
1
~~> thisrun.String() ")"

What did you expect to see?

2
~~> thisrun.String() "ع "
1
~~> thisrun.String() "a (b)"

Brackets are not ordered correctly, but in the expected output it is correct.

A fix would be to change line 119 in bidi.go to p.pairValues = append(p.pairValues, props.reverseBracket(r))

The reason is that for bracket matching, the algorithm expects the same values for open brackets and for close brackets. Without the reverseBracket, the codes for open and close brackets differ and never match. The reverseBracket function changes the codes for closing brackets to opening brackets, for example ) = U+0029 to ( U+0028.

The tests in core_test.go don't fail, because the code from the default initialisation is bypassed with:

			switch {
			case !p.IsBracket():
				pairTypes = append(pairTypes, bpNone)
				pairValues = append(pairValues, 0)
			case p.IsOpeningBracket():
				pairTypes = append(pairTypes, bpOpen)
				pairValues = append(pairValues, r)
			default:
				pairTypes = append(pairTypes, bpClose)
				pairValues = append(pairValues, p.reverseBracket(r))
			}

which is IMO a bit more clear than the if .. elseif .. else part from the prepareInput which is called from the user API.

[pgundlach]

I can prepare a pull request.

A test could be

func TestBracket(t *testing.T) {
	str := `ع a (b)`
	p := Paragraph{}
	p.SetString(str, DefaultDirection(LeftToRight))
	order, err := p.Order()
	if err != nil {
		log.Fatal(err)
	}

	expectedRuns := []runInformation{
		{"ع", RightToLeft, 0, 0},
		{" a (b)", LeftToRight, 1, 6},
	}

	if nr, want := order.NumRuns(), len(expectedRuns); nr != want {
		t.Errorf("order.NumRuns() = %d; want %d", nr, want)
	}

	for i, want := range expectedRuns {
		r := order.Run(i)
		if got := r.String(); got != want.str {
			t.Errorf("Run(%d) = %q; want %q", i, got, want.str)
		}
		if s, e := r.Pos(); s != want.start || e != want.end {
			t.Errorf("Run(%d).start = %d, .end = %d; want start = %d, end = %d", i, s, e, want.start, want.end)
		}
		if d := r.Direction(); d != want.dir {
			t.Errorf("Run(%d).Direction = %d; want %d", i, d, want.dir)
		}
	}
}

[gabyhelp]

Related Issues

• x/text/unicode/bidi: Perhaps incorrect implementation of algorithm #71809
• x/text/unicode/bidi: `ReverseString` doesn't preserve modifiers #50633
• x/text/cases: incorrect case folding for Ꮭ #46101

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}