Skip to content

Commit adc3ad9

Browse files
authored
Trie cleanup (#4)
* Remove ControlChar enum, just use ZeroWidth * Remove CombiningMark enum, just use ZeroWidth * Combine East Asian Full & Wide * No need to cast to property in the trie * Move benchmarks and compat into dedicated module
1 parent 9ea1c98 commit adc3ad9

15 files changed

Lines changed: 1225 additions & 1250 deletions

File tree

AGENTS.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,13 @@ dependencies and are hard to cleanup.)
1616
If you make changes to the trie generation in internal/gen, it can be invoked
1717
by running `go generate` from the top package directory.
1818

19-
## Pull Requests
19+
## Pull Requests and branches
2020

21-
For PRs, you can use the gh CLI tool to retrieve or post comments.
21+
For PRs (pull requests), you can use the gh CLI tool to retrieve details,
22+
or post comments. Then, compare the current branch with main. Reviewing a PR
23+
and reviewing a branch are about the same, but the PR may add context.
24+
25+
Look for bugs. Think like GitHub Copilot or Cursor BugBot.
2226

2327
## Comparisons to go-runewidth
2428

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
1-
package displaywidth
1+
package comparison
22

33
import (
44
"strings"
55
"testing"
66

7-
"github.com/clipperhouse/displaywidth/internal/testdata"
7+
"github.com/clipperhouse/displaywidth"
8+
"github.com/clipperhouse/displaywidth/comparison/testdata"
89
"github.com/mattn/go-runewidth"
910
)
1011

@@ -98,7 +99,7 @@ func BenchmarkStringDefault(b *testing.B) {
9899
for i := 0; i < b.N; i++ {
99100
for _, tc := range testCases {
100101
// Test with default settings (eastAsianWidth=false, strictEmojiNeutral=false)
101-
_ = String(tc.Input)
102+
_ = displaywidth.String(tc.Input)
102103
}
103104
}
104105

@@ -134,7 +135,7 @@ func BenchmarkStringDefault(b *testing.B) {
134135
}
135136

136137
func BenchmarkString_EAW(b *testing.B) {
137-
options := Options{
138+
options := displaywidth.Options{
138139
EastAsianWidth: true,
139140
StrictEmojiNeutral: false,
140141
}
@@ -192,7 +193,7 @@ func BenchmarkString_EAW(b *testing.B) {
192193

193194
// BenchmarkString_StrictEmoji benchmarks our package with strict emoji neutral
194195
func BenchmarkString_StrictEmoji(b *testing.B) {
195-
options := Options{
196+
options := displaywidth.Options{
196197
EastAsianWidth: false,
197198
StrictEmojiNeutral: true,
198199
}
@@ -262,7 +263,7 @@ func BenchmarkString_ASCII(b *testing.B) {
262263
b.ResetTimer()
263264
for i := 0; i < b.N; i++ {
264265
for _, s := range asciiStrings {
265-
_ = String(s)
266+
_ = displaywidth.String(s)
266267
}
267268
}
268269

@@ -310,7 +311,7 @@ func BenchmarkString_Unicode(b *testing.B) {
310311
b.ResetTimer()
311312
for i := 0; i < b.N; i++ {
312313
for _, s := range unicodeStrings {
313-
_ = String(s)
314+
_ = displaywidth.String(s)
314315
}
315316
}
316317

@@ -354,7 +355,7 @@ func BenchmarkStringWidth_Emoji(b *testing.B) {
354355
b.ResetTimer()
355356
for i := 0; i < b.N; i++ {
356357
for _, s := range emojiStrings {
357-
_ = String(s)
358+
_ = displaywidth.String(s)
358359
}
359360
}
360361

@@ -396,7 +397,7 @@ func BenchmarkString_Mixed(b *testing.B) {
396397
b.ResetTimer()
397398
for i := 0; i < b.N; i++ {
398399
for _, s := range mixedStrings {
399-
_ = String(s)
400+
_ = displaywidth.String(s)
400401
}
401402
}
402403

@@ -441,7 +442,7 @@ func BenchmarkString_ControlChars(b *testing.B) {
441442
b.ResetTimer()
442443
for i := 0; i < b.N; i++ {
443444
for _, s := range controlStrings {
444-
_ = String(s)
445+
_ = displaywidth.String(s)
445446
}
446447
}
447448

@@ -499,7 +500,7 @@ func BenchmarkRuneDefault(b *testing.B) {
499500
b.ResetTimer()
500501
for i := 0; i < b.N; i++ {
501502
for _, r := range testRunes {
502-
_ = Rune(r)
503+
_ = displaywidth.Rune(r)
503504
}
504505
}
505506

@@ -529,7 +530,7 @@ func BenchmarkRuneDefault(b *testing.B) {
529530

530531
// BenchmarkRuneWidth_EAW benchmarks rune width with East Asian Width option
531532
func BenchmarkRuneWidth_EAW(b *testing.B) {
532-
options := Options{
533+
options := displaywidth.Options{
533534
EastAsianWidth: true,
534535
StrictEmojiNeutral: false,
535536
}
@@ -591,7 +592,7 @@ func BenchmarkRuneWidth_ASCII(b *testing.B) {
591592
b.ResetTimer()
592593
for i := 0; i < b.N; i++ {
593594
for _, r := range asciiRunes {
594-
_ = Rune(r)
595+
_ = displaywidth.Rune(r)
595596
}
596597
}
597598

comparison/compatibility_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
package comparison
2+
3+
import (
4+
"testing"
5+
"unicode"
6+
7+
"github.com/clipperhouse/displaywidth"
8+
"github.com/mattn/go-runewidth"
9+
)
10+
11+
// Investigate differences between displaywidth and go-runewidth.
12+
// Not meant to be a real test, more of a tool, but good to
13+
// fail if something changes in the future.
14+
func TestCompatibility(t *testing.T) {
15+
t.Skip("skipping compatibility test")
16+
if unicode.Version < "15" {
17+
// We only care about Unicode 15 and above,
18+
// which I believe was Go version 1.21.
19+
return
20+
}
21+
22+
for r := rune(0); r <= unicode.MaxRune; r++ {
23+
w1 := displaywidth.Rune(r)
24+
w2 := runewidth.RuneWidth(r)
25+
if w1 != w2 {
26+
if unicode.Is(unicode.Mn, r) {
27+
// these are in the trie, known
28+
// we will return width 0,
29+
// go-runewidth may return width 1
30+
continue
31+
}
32+
if unicode.Is(unicode.Cf, r) {
33+
// these are in the trie, known
34+
// we will return width 0,
35+
// go-runewidth may return width 1
36+
continue
37+
}
38+
if unicode.Is(unicode.Mc, r) {
39+
// these are deliberately excluded from the trie, known
40+
// we will return width 1,
41+
// go-runewidth may return width 0
42+
continue
43+
}
44+
t.Errorf("%#x: runewidth is %d, displaywidth is %d, difference is %d", r, w2, w1, w2-w1)
45+
}
46+
}
47+
}

comparison/go.mod

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
module github.com/clipperhouse/displaywidth/comparison
2+
3+
go 1.18
4+
5+
require (
6+
github.com/clipperhouse/displaywidth v0.2.0
7+
github.com/mattn/go-runewidth v0.0.19
8+
)
9+
10+
require github.com/clipperhouse/uax29/v2 v2.2.0 // indirect

comparison/go.sum

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
github.com/clipperhouse/displaywidth v0.2.0 h1:rgsptyf+vdcUj2E5DADgM6nMMlcj5dOmpJqWeO4t/0Q=
2+
github.com/clipperhouse/displaywidth v0.2.0/go.mod h1:0pW8rEIr4/Pjji52WJsYnMpoj/8Cse3d703ZVdU6vBI=
3+
github.com/clipperhouse/uax29/v2 v2.2.0 h1:ChwIKnQN3kcZteTXMgb1wztSgaU+ZemkgWdohwgs8tY=
4+
github.com/clipperhouse/uax29/v2 v2.2.0/go.mod h1:EFJ2TJMRUaplDxHKj1qAEhCtQPW2tJSwu5BF98AuoVM=
5+
github.com/mattn/go-runewidth v0.0.19 h1:v++JhqYnZuu5jSKrk9RbgF5v4CGUjqRfBm05byFGLdw=
6+
github.com/mattn/go-runewidth v0.0.19/go.mod h1:XBkDxAl56ILZc9knddidhrOlY5R/pDhgLpndooCuJAs=

go.mod

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,4 @@ go 1.18
44

55
require (
66
github.com/clipperhouse/uax29/v2 v2.2.0
7-
github.com/mattn/go-runewidth v0.0.19
87
)

0 commit comments

Comments
 (0)