From 894c97e374092bc42425c701c6696f81b1a6aa32 Mon Sep 17 00:00:00 2001 From: Tim Culverhouse Date: Mon, 1 Jul 2024 15:54:30 -0500 Subject: ui: make textinput grapheme aware The textinput widget operated on a slice of runes, and naively assumed a rune was a "character". When deleting or navigating the cursor through text which contains multi-codepoint characters (such as emoji), the cursor index could desync and cause panics. Use a slice of vaxis.Characters instead of runes to more accurately reflect the index state of the cursor with respect to characters. Fixes: https://todo.sr.ht/~rjarry/aerc/263 Reported-by: Bence Ferdinandy Signed-off-by: Tim Culverhouse Tested-by: Bence Ferdinandy Reviewed-by: Koni Marti Acked-by: Robin Jarry --- lib/ui/textinput.go | 55 ++++++++++++++++++++++++++++++------------------ lib/ui/textinput_test.go | 4 ++-- 2 files changed, 37 insertions(+), 22 deletions(-) (limited to 'lib') diff --git a/lib/ui/textinput.go b/lib/ui/textinput.go index 29f71509..6c4551b3 100644 --- a/lib/ui/textinput.go +++ b/lib/ui/textinput.go @@ -24,7 +24,7 @@ type TextInput struct { password bool prompt string scroll int - text []rune + text []vaxis.Character change []func(ti *TextInput) focusLost []func(ti *TextInput) tabcomplete func(s string) ([]string, string) @@ -42,10 +42,11 @@ type TextInput struct { // context they're given, and process keypresses to build a string from user // input. func NewTextInput(text string, ui *config.UIConfig) *TextInput { + chars := vaxis.Characters(text) return &TextInput{ cells: -1, - text: []rune(text), - index: len([]rune(text)), + text: chars, + index: len(chars), uiConfig: ui, } } @@ -72,22 +73,35 @@ func (ti *TextInput) TabComplete( } func (ti *TextInput) String() string { - return string(ti.text) + return charactersToString(ti.text) } func (ti *TextInput) StringLeft() string { - for ti.index > len(ti.text) { + if ti.index > len(ti.text) { ti.index = len(ti.text) } - return string(ti.text[:ti.index]) + left := ti.text[:ti.index] + return charactersToString(left) } func (ti *TextInput) StringRight() string { - return string(ti.text[ti.index:]) + if ti.index >= len(ti.text) { + return "" + } + right := ti.text[ti.index:] + return charactersToString(right) +} + +func charactersToString(chars []vaxis.Character) string { + buf := strings.Builder{} + for _, ch := range chars { + buf.WriteString(ch.Grapheme) + } + return buf.String() } func (ti *TextInput) Set(value string) *TextInput { - ti.text = []rune(value) + ti.text = vaxis.Characters(value) ti.index = len(ti.text) ti.scroll = 0 return ti @@ -112,12 +126,12 @@ func (ti *TextInput) Draw(ctx *Context) { sindex := ti.index - scroll if ti.password { x := ctx.Printf(0, 0, defaultStyle, "%s", ti.prompt) - cells := runewidth.StringWidth(string(text)) + cells := len(ti.text) ctx.Fill(x, 0, cells, 1, '*', defaultStyle) } else { - ctx.Printf(0, 0, defaultStyle, "%s%s", ti.prompt, string(text)) + ctx.Printf(0, 0, defaultStyle, "%s%s", ti.prompt, charactersToString(text)) } - cells := runewidth.StringWidth(string(text[:sindex]) + ti.prompt) + cells := runewidth.StringWidth(charactersToString(text[:sindex]) + ti.prompt) if ti.focus { ctx.SetCursor(cells, 0, vaxis.CursorDefault) ti.drawPopover(ctx) @@ -161,7 +175,7 @@ func (ti *TextInput) Focus(focus bool) { } ti.focus = focus if focus && ti.ctx != nil { - cells := runewidth.StringWidth(string(ti.text[:ti.index])) + cells := runewidth.StringWidth(charactersToString(ti.text[:ti.index])) ti.ctx.SetCursor(cells+1, 0, vaxis.CursorDefault) } else if !focus && ti.ctx != nil { ti.ctx.HideCursor() @@ -181,10 +195,10 @@ func (ti *TextInput) ensureScroll() { } } -func (ti *TextInput) insert(ch rune) { +func (ti *TextInput) insert(ch vaxis.Character) { left := ti.text[:ti.index] right := ti.text[ti.index:] - ti.text = append(left, append([]rune{ch}, right...)...) //nolint:gocritic // intentional append to different slice + ti.text = append(left, append([]vaxis.Character{ch}, right...)...) //nolint:gocritic // intentional append to different slice ti.index++ ti.ensureScroll() ti.Invalidate() @@ -197,16 +211,16 @@ func (ti *TextInput) deleteWord() { } separators := "/'\"" i := ti.index - 1 - for i >= 0 && ti.text[i] == ' ' { + for i >= 0 && ti.text[i].Grapheme == " " { i-- } - if i >= 0 && strings.ContainsRune(separators, ti.text[i]) { - for i >= 0 && strings.ContainsRune(separators, ti.text[i]) { + if i >= 0 && strings.Contains(separators, ti.text[i].Grapheme) { + for i >= 0 && strings.Contains(separators, ti.text[i].Grapheme) { i-- } } else { separators += " " - for i >= 0 && !strings.ContainsRune(separators, ti.text[i]) { + for i >= 0 && !strings.Contains(separators, ti.text[i].Grapheme) { i-- } } @@ -378,7 +392,8 @@ func (ti *TextInput) Event(event vaxis.Event) bool { case key.Matches(vaxis.KeyEsc): ti.Invalidate() case key.Text != "": - for _, ch := range key.Text { + chars := vaxis.Characters(key.Text) + for _, ch := range chars { ti.insert(ch) } } @@ -530,7 +545,7 @@ func (c *completions) needsStem(stem string) bool { func (c *completions) stem(stem string) { c.ti.Set(c.ti.prefix + stem + c.ti.StringRight()) - c.ti.index = runewidth.StringWidth(c.ti.prefix + stem) + c.ti.index = len(vaxis.Characters(c.ti.prefix + stem)) } func findStem(words []string) string { diff --git a/lib/ui/textinput_test.go b/lib/ui/textinput_test.go index 47a4c294..22bf43ae 100644 --- a/lib/ui/textinput_test.go +++ b/lib/ui/textinput_test.go @@ -64,8 +64,8 @@ func TestDeleteWord(t *testing.T) { t.Run(test.name, func(t *testing.T) { textinput := NewTextInput(test.text, nil) textinput.deleteWord() - if string(textinput.text) != test.expected { - t.Errorf("word was deleted incorrectly: got %s but expected %s", string(textinput.text), test.expected) + if charactersToString(textinput.text) != test.expected { + t.Errorf("word was deleted incorrectly: got %s but expected %s", charactersToString(textinput.text), test.expected) } }) } -- cgit