Repository: charmbracelet/bubbletea
Pull request #326 (2022-06-03 02:54:59)

fix: handle batched key msgs
If tea can't keep up with reading from the input buffer, key messages might queue up and be retrieved as a batch of events. Emit them individually, so tea apps process them correctly.


Patch:
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index 4bebb3a..5d086b2 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -5,7 +5,7 @@ jobs:
   build:
     strategy:
       matrix:
-        go-version: [~1.13, ^1]
+        go-version: [~1.16, ^1]
         os: [ubuntu-latest, macos-latest, windows-latest]
     runs-on: ${{ matrix.os }}
     env:
@@ -25,6 +25,9 @@ jobs:
       - name: Build
         run: go build -v ./...
 
+      - name: Test
+        run: go test ./...
+
       - name: Build examples
         run: go build -v ./...
         working-directory: ./examples
diff --git a/key.go b/key.go
index e2415a9..c210376 100644
--- a/key.go
+++ b/key.go
@@ -492,35 +492,7 @@ func readInputs(input io.Reader) ([]Msg, error) {
 		return m, nil
 	}
 
-	// Is it a sequence, like an arrow key?
-	if k, ok := sequences[string(buf[:numBytes])]; ok {
-		return []Msg{
-			KeyMsg(k),
-		}, nil
-	}
-
-	// Some of these need special handling.
-	hex := fmt.Sprintf("%x", buf[:numBytes])
-	if k, ok := hexes[hex]; ok {
-		return []Msg{
-			KeyMsg(k),
-		}, nil
-	}
-
-	// Is the alt key pressed? If so, the buffer will be prefixed with an
-	// escape.
-	if numBytes > 1 && buf[0] == 0x1b {
-		// Now remove the initial escape sequence and re-process to get the
-		// character being pressed in combination with alt.
-		c, _ := utf8.DecodeRune(buf[1:])
-		if c == utf8.RuneError {
-			return nil, errors.New("could not decode rune after removing initial escape")
-		}
-		return []Msg{
-			KeyMsg(Key{Alt: true, Type: KeyRunes, Runes: []rune{c}}),
-		}, nil
-	}
-
+	var runeSets [][]rune
 	var runes []rune
 	b := buf[:numBytes]
 
@@ -532,38 +504,64 @@ func readInputs(input io.Reader) ([]Msg, error) {
 		if r == utf8.RuneError {
 			return nil, errors.New("could not decode rune")
 		}
+
+		if r == '\x1b' && len(runes) > 1 {
+			// a new key sequence has started
+			runeSets = append(runeSets, runes)
+			runes = []rune{}
+		}
+
 		runes = append(runes, r)
 		w = width
 	}
+	// add the final set of runes we decoded
+	runeSets = append(runeSets, runes)
 
-	if len(runes) == 0 {
+	if len(runeSets) == 0 {
 		return nil, errors.New("received 0 runes from input")
-	} else if len(runes) > 1 {
-		// We received multiple runes, so we know this isn't a control
-		// character, sequence, and so on.
-		return []Msg{
-			KeyMsg(Key{Type: KeyRunes, Runes: runes}),
-		}, nil
 	}
 
-	// Is the first rune a control character?
-	r := KeyType(runes[0])
-	if numBytes == 1 && r <= keyUS || r == keyDEL {
-		return []Msg{
-			KeyMsg(Key{Type: r}),
-		}, nil
-	}
+	var msgs []Msg
+	for _, runes := range runeSets {
+		// Is it a sequence, like an arrow key?
+		if k, ok := sequences[string(runes)]; ok {
+			msgs = append(msgs, KeyMsg(k))
+			continue
+		}
+
+		// Some of these need special handling.
+		hex := fmt.Sprintf("%x", runes)
+		if k, ok := hexes[hex]; ok {
+			msgs = append(msgs, KeyMsg(k))
+			continue
+		}
+
+		// Is the alt key pressed? If so, the buffer will be prefixed with an
+		// escape.
+		if len(runes) > 1 && runes[0] == 0x1b {
+			msgs = append(msgs, KeyMsg(Key{Alt: true, Type: KeyRunes, Runes: runes[1:]}))
+			continue
+		}
 
-	// If it's a space, override the type with KeySpace (but still include the
-	// rune).
-	if runes[0] == ' ' {
-		return []Msg{
-			KeyMsg(Key{Type: KeySpace, Runes: runes}),
-		}, nil
+		for _, v := range runes {
+			// Is the first rune a control character?
+			r := KeyType(v)
+			if r <= keyUS || r == keyDEL {
+				msgs = append(msgs, KeyMsg(Key{Type: r}))
+				continue
+			}
+
+			// If it's a space, override the type with KeySpace (but still include
+			// the rune).
+			if r == ' ' {
+				msgs = append(msgs, KeyMsg(Key{Type: KeySpace, Runes: []rune{v}}))
+				continue
+			}
+
+			// Welp, just regular, ol' runes.
+			msgs = append(msgs, KeyMsg(Key{Type: KeyRunes, Runes: []rune{v}}))
+		}
 	}
 
-	// Welp, it's just a regular, ol' single rune.
-	return []Msg{
-		KeyMsg(Key{Type: KeyRunes, Runes: runes}),
-	}, nil
+	return msgs, nil
 }
diff --git a/standard_renderer.go b/standard_renderer.go
index 12087bb..4e75403 100644
--- a/standard_renderer.go
+++ b/standard_renderer.go
@@ -348,7 +348,9 @@ func (r *standardRenderer) handleMessages(msg Msg) {
 	case repaintMsg:
 		// Force a repaint by clearing the render cache as we slide into a
 		// render.
+		r.mtx.Lock()
 		r.repaint()
+		r.mtx.Unlock()
 
 	case WindowSizeMsg:
 		r.mtx.Lock()
@@ -362,7 +364,7 @@ func (r *standardRenderer) handleMessages(msg Msg) {
 		// Force a repaint on the area where the scrollable stuff was in this
 		// update cycle
 		r.mtx.Lock()
-		r.lastRender = ""
+		r.repaint()
 		r.mtx.Unlock()
 
 	case syncScrollAreaMsg:
@@ -373,7 +375,7 @@ func (r *standardRenderer) handleMessages(msg Msg) {
 
 		// Force non-scrolling stuff to repaint in this update cycle
 		r.mtx.Lock()
-		r.lastRender = ""
+		r.repaint()
 		r.mtx.Unlock()
 
 	case scrollUpMsg:
diff --git a/tea.go b/tea.go
index 093e5f8..73be9bd 100644
--- a/tea.go
+++ b/tea.go
@@ -503,7 +503,9 @@ func (p *Program) StartReturningModel() (Model, error) {
 				continue
 
 			case WindowSizeMsg:
+				p.mtx.Lock()
 				p.renderer.repaint()
+				p.mtx.Unlock()
 
 			case enterAltScreenMsg:
 				p.EnterAltScreen()


Test patch:
diff --git a/key_test.go b/key_test.go
index 511ae75..b74f06b 100644
--- a/key_test.go
+++ b/key_test.go
@@ -48,29 +48,119 @@ func TestKeyTypeString(t *testing.T) {
 }
 
 func TestReadInput(t *testing.T) {
-	for out, in := range map[string][]byte{
-		"a":         {'a'},
-		"ctrl+a":    {byte(keySOH)},
-		"alt+a":     {0x1b, 'a'},
-		"abcd":      {'a', 'b', 'c', 'd'},
-		"up":        []byte("\x1b[A"),
-		"wheel up":  {'\x1b', '[', 'M', byte(32) + 0b0100_0000, byte(65), byte(49)},
-		"shift+tab": {'\x1b', '[', 'Z'},
+	type test struct {
+		in  []byte
+		out []Msg
+	}
+	for out, td := range map[string]test{
+		"a": {
+			[]byte{'a'},
+			[]Msg{
+				KeyMsg{
+					Type:  KeyRunes,
+					Runes: []rune{'a'},
+				},
+			},
+		},
+		" ": {
+			[]byte{' '},
+			[]Msg{
+				KeyMsg{
+					Type:  KeySpace,
+					Runes: []rune{' '},
+				},
+			},
+		},
+		"ctrl+a": {
+			[]byte{byte(keySOH)},
+			[]Msg{
+				KeyMsg{
+					Type: KeyCtrlA,
+				},
+			},
+		},
+		"alt+a": {
+			[]byte{byte(0x1b), 'a'},
+			[]Msg{
+				KeyMsg{
+					Type:  KeyRunes,
+					Alt:   true,
+					Runes: []rune{'a'},
+				},
+			},
+		},
+		"abcd": {
+			[]byte{'a', 'b', 'c', 'd'},
+			[]Msg{
+				KeyMsg{
+					Type:  KeyRunes,
+					Runes: []rune{'a'},
+				},
+				KeyMsg{
+					Type:  KeyRunes,
+					Runes: []rune{'b'},
+				},
+				KeyMsg{
+					Type:  KeyRunes,
+					Runes: []rune{'c'},
+				},
+				KeyMsg{
+					Type:  KeyRunes,
+					Runes: []rune{'d'},
+				},
+			},
+		},
+		"up": {
+			[]byte("\x1b[A"),
+			[]Msg{
+				KeyMsg{
+					Type: KeyUp,
+				},
+			},
+		},
+		"wheel up": {
+			[]byte{'\x1b', '[', 'M', byte(32) + 0b0100_0000, byte(65), byte(49)},
+			[]Msg{
+				MouseMsg{
+					Type: MouseWheelUp,
+				},
+			},
+		},
+		"shift+tab": {
+			[]byte{'\x1b', '[', 'Z'},
+			[]Msg{
+				KeyMsg{
+					Type: KeyShiftTab,
+				},
+			},
+		},
 	} {
 		t.Run(out, func(t *testing.T) {
-			msgs, err := readInputs(bytes.NewReader(in))
+			msgs, err := readInputs(bytes.NewReader(td.in))
 			if err != nil {
 				t.Fatalf("unexpected error: %v", err)
 			}
-			if len(msgs) == 0 {
-				t.Fatalf("unexpected empty message list")
+			if len(msgs) != len(td.out) {
+				t.Fatalf("unexpected message list length")
 			}
 
-			if m, ok := msgs[0].(KeyMsg); ok && m.String() != out {
-				t.Fatalf(`expected a keymsg %q, got %q`, out, m)
+			if len(msgs) == 1 {
+				if m, ok := msgs[0].(KeyMsg); ok && m.String() != out {
+					t.Fatalf(`expected a keymsg %q, got %q`, out, m)
+				}
 			}
-			if m, ok := msgs[0].(MouseMsg); ok && mouseEventTypes[m.Type] != out {
-				t.Fatalf(`expected a mousemsg %q, got %q`, out, mouseEventTypes[m.Type])
+
+			for i, v := range msgs {
+				if m, ok := v.(KeyMsg); ok &&
+					m.String() != td.out[i].(KeyMsg).String() {
+					t.Fatalf(`expected a keymsg %q, got %q`, td.out[i].(KeyMsg), m)
+				}
+				if m, ok := v.(MouseMsg); ok &&
+					(mouseEventTypes[m.Type] != out || m.Type != td.out[i].(MouseMsg).Type) {
+					t.Fatalf(`expected a mousemsg %q, got %q`,
+						out,
+						mouseEventTypes[td.out[i].(MouseMsg).Type])
+				}
 			}
 		})
 	}
diff --git a/options_test.go b/options_test.go
index 4b75994..1b77977 100644
--- a/options_test.go
+++ b/options_test.go
@@ -18,10 +18,10 @@ func TestOptions(t *testing.T) {
 		var b bytes.Buffer
 		p := NewProgram(nil, WithInput(&b))
 		if p.input != &b {
-			t.Errorf("expected input to custom, got %v", p.output)
+			t.Errorf("expected input to custom, got %v", p.input)
 		}
 		if p.startupOptions&withCustomInput == 0 {
-			t.Errorf("expected startup options to have custom input set, got %v", p.output)
+			t.Errorf("expected startup options to have custom input set, got %v", p.input)
 		}
 	})