ssh/terminal: Use move-N sequences for >1 cursor moves

Before, we emitted N single-move sequences on a cursor move. For
example, "move 4 left" would emit "^[[D^[[D^[[D^[[D". With this change,
it would emit "^[[4D".

Using variable move sequences when possible reduces the amount of
rendering output that the terminal implementation produces. This can
have some low-level performance benefits, but also helps consumers
reason through the produced output.

Includes a test with a couple of cases.

Note: The old implementation used ^[[D instead of ^[D which is also
valid. This is true in several unrelated places, so this implementation
continues to use ^[[D for consistency.

Change-Id: If38eaaed8fb4075499fdda54c06681dc34c3ad70
GitHub-Last-Rev: 92ef253
GitHub-Pull-Request: #82
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/169077
Reviewed-by: Adam Langley <[email protected]>
Reviewed-by: Brad Fitzpatrick <[email protected]>

Author: shazow

Diff for: ssh/terminal/terminal.go

@@ -7,6 +7,7 @@ package terminal
 import (
 	"bytes"
 	"io"
+	"strconv"
 	"sync"
 	"unicode/utf8"
 )
@@ -271,34 +272,44 @@ func (t *Terminal) moveCursorToPos(pos int) {
 }
 
 func (t *Terminal) move(up, down, left, right int) {
-	movement := make([]rune, 3*(up+down+left+right))
-	m := movement
-	for i := 0; i < up; i++ {
-		m[0] = keyEscape
-		m[1] = '['
-		m[2] = 'A'
-		m = m[3:]
-	}
-	for i := 0; i < down; i++ {
-		m[0] = keyEscape
-		m[1] = '['
-		m[2] = 'B'
-		m = m[3:]
-	}
-	for i := 0; i < left; i++ {
-		m[0] = keyEscape
-		m[1] = '['
-		m[2] = 'D'
-		m = m[3:]
-	}
-	for i := 0; i < right; i++ {
-		m[0] = keyEscape
-		m[1] = '['
-		m[2] = 'C'
-		m = m[3:]
-	}
-
-	t.queue(movement)
+	m := []rune{}
+
+	// 1 unit up can be expressed as ^[[A or ^[A
+	// 5 units up can be expressed as ^[[5A
+
+	if up == 1 {
+		m = append(m, keyEscape, '[', 'A')
+	} else if up > 1 {
+		m = append(m, keyEscape, '[')
+		m = append(m, []rune(strconv.Itoa(up))...)
+		m = append(m, 'A')
+	}
+
+	if down == 1 {
+		m = append(m, keyEscape, '[', 'B')
+	} else if down > 1 {
+		m = append(m, keyEscape, '[')
+		m = append(m, []rune(strconv.Itoa(down))...)
+		m = append(m, 'B')
+	}
+
+	if right == 1 {
+		m = append(m, keyEscape, '[', 'C')
+	} else if right > 1 {
+		m = append(m, keyEscape, '[')
+		m = append(m, []rune(strconv.Itoa(right))...)
+		m = append(m, 'C')
+	}
+
+	if left == 1 {
+		m = append(m, keyEscape, '[', 'D')
+	} else if left > 1 {
+		m = append(m, keyEscape, '[')
+		m = append(m, []rune(strconv.Itoa(left))...)
+		m = append(m, 'D')
+	}
+
+	t.queue(m)
 }
 
 func (t *Terminal) clearLineToRight() {

Diff for: ssh/terminal/terminal_test.go

@@ -237,6 +237,49 @@ func TestKeyPresses(t *testing.T) {
 	}
 }
 
+var renderTests = []struct {
+	in       string
+	received string
+	err      error
+}{
+	{
+		// Cursor move after keyHome (left 4) then enter (right 4, newline)
+		in:       "abcd\x1b[H\r",
+		received: "> abcd\x1b[4D\x1b[4C\r\n",
+	},
+	{
+		// Write, home, prepend, enter. Prepends rewrites the line.
+		in: "cdef\x1b[Hab\r",
+		received: "> cdef" + // Initial input
+			"\x1b[4Da" + // Move cursor back, insert first char
+			"cdef" + // Copy over original string
+			"\x1b[4Dbcdef" + // Repeat for second char with copy
+			"\x1b[4D" + // Put cursor back in position to insert again
+			"\x1b[4C\r\n", // Put cursor at the end of the line and newline.
+	},
+}
+
+func TestRender(t *testing.T) {
+	for i, test := range renderTests {
+		for j := 1; j < len(test.in); j++ {
+			c := &MockTerminal{
+				toSend:       []byte(test.in),
+				bytesPerRead: j,
+			}
+			ss := NewTerminal(c, "> ")
+			_, err := ss.ReadLine()
+			if err != test.err {
+				t.Errorf("Error resulting from test %d (%d bytes per read) was '%v', expected '%v'", i, j, err, test.err)
+				break
+			}
+			if test.received != string(c.received) {
+				t.Errorf("Results rendered from test %d (%d bytes per read) was '%s', expected '%s'", i, j, c.received, test.received)
+				break
+			}
+		}
+	}
+}
+
 func TestPasswordNotSaved(t *testing.T) {
 	c := &MockTerminal{
 		toSend:       []byte("password\r\x1b[A\r"),