Merge pull request #105 from reeflective/dev

Emacs backspace fix + buffered terminal output (one flush per frame)

Author: maxlandon

internal/completion/display.go

@@ -20,7 +20,7 @@ func Display(eng *Engine, maxRows int) {
 	// little more time. The engine itself is responsible for
 	// deleting those lists when it deems them useless.
 	if eng.Matches() == 0 || eng.skipDisplay {
-		fmt.Print(term.ClearLineAfter)
+		term.WriteString(term.ClearLineAfter)
 		return
 	}
 
@@ -38,7 +38,7 @@ func Display(eng *Engine, maxRows int) {
 	completions, eng.usedY = eng.cropCompletions(completions, maxRows)
 
 	if completions != "" {
-		fmt.Print(completions)
+		term.WriteString(completions)
 	}
 }
 

internal/core/api_windows.go

@@ -9,6 +9,8 @@ import (
 	"reflect"
 	"syscall"
 	"unsafe"
+
+	"github.com/reeflective/readline/internal/term"
 )
 
 var (
@@ -167,6 +169,10 @@ func setConsoleCursorPosition(c *_COORD) error {
 
 // GetCursorPos returns the current cursor position on Windows.
 func (k *Keys) GetCursorPos() (x, y int) {
+	// Flush any buffered frame output so the console cursor reflects everything
+	// printed so far before we read its position.
+	term.FlushBuffer()
+
 	t := new(_CONSOLE_SCREEN_BUFFER_INFO)
 	kernel.GetConsoleScreenBufferInfo(
 		stdout,

internal/core/keys_unix.go

@@ -28,6 +28,11 @@ func (k *Keys) GetCursorPos() (x, y int) {
 	var cursor []byte
 	var match [][]string
 
+	// Flush any buffered frame output first: the cursor position we are about
+	// to query is only correct once the prompt printed so far is actually on
+	// screen, not still sitting in the output buffer.
+	term.FlushBuffer()
+
 	// Echo the query and wait for the main key
 	// reading routine to send us the response back.
 	fmt.Print("\x1b[6n")

internal/core/line.go

@@ -311,7 +311,7 @@ func DisplayLine(l *Line, indent int) {
 
 	builtLine.WriteString(color.BgDefault)
 
-	fmt.Print(builtLine.String())
+	term.WriteString(builtLine.String())
 }
 
 // CoordinatesLine returns the number of real terminal lines on which the input line spans, considering

internal/display/engine.go

@@ -1,7 +1,6 @@
 package display
 
 import (
-	"fmt"
 	"regexp"
 
 	"github.com/reeflective/readline/inputrc"
@@ -82,8 +81,11 @@ func (e *Engine) PrintPrimaryPrompt() {
 
 // ClearHelpers clears the hint and completion sections below the line.
 func (e *Engine) ClearHelpers() {
+	term.BeginBuffer()
+	defer term.EndBuffer()
+
 	e.CursorBelowLine()
-	fmt.Print(term.ClearScreenBelow)
+	term.WriteString(term.ClearScreenBelow)
 
 	term.MoveCursorUp(1)
 	term.MoveCursorUp(e.lineRows)
@@ -102,6 +104,9 @@ func (e *Engine) ResetHelpers() {
 // hints, completions and some right prompts, the shell will put the
 // display at the start of the line immediately following the line.
 func (e *Engine) AcceptLine() {
+	term.BeginBuffer()
+	defer term.EndBuffer()
+
 	e.CursorToLineStart()
 
 	e.computeCoordinates(false)
@@ -110,14 +115,14 @@ func (e *Engine) AcceptLine() {
 	term.MoveCursorBackwards(term.GetWidth())
 	term.MoveCursorDown(e.lineRows)
 	term.MoveCursorForwards(e.lineCol)
-	fmt.Print(term.ClearScreenBelow)
+	term.WriteString(term.ClearScreenBelow)
 
 	// Reprint the right-side prompt if it's not a tooltip one.
 	e.prompt.RightPrint(e.lineCol, false)
 
 	// Go below this non-suggested line and clear everything.
 	term.MoveCursorBackwards(term.GetWidth())
-	fmt.Print(term.NewlineReturn)
+	term.WriteString(term.NewlineReturn)
 }
 
 // RefreshTransient goes back to the first line of the input buffer
@@ -127,14 +132,17 @@ func (e *Engine) RefreshTransient() {
 		return
 	}
 
+	term.BeginBuffer()
+	defer term.EndBuffer()
+
 	// Go to the beginning of the primary prompt.
 	e.CursorToLineStart()
 	term.MoveCursorUp(e.prompt.PrimaryUsed())
 
 	// And redisplay the transient/primary/line.
 	e.prompt.TransientPrint()
 	e.displayLine()
-	fmt.Print(term.NewlineReturn)
+	term.WriteString(term.NewlineReturn)
 }
 
 // CursorToLineStart moves the cursor just after the primary prompt.
@@ -153,7 +161,7 @@ func (e *Engine) CursorToLineStart() {
 func (e *Engine) CursorBelowLine() {
 	term.MoveCursorUp(e.cursorRow)
 	term.MoveCursorDown(e.lineRows)
-	fmt.Print(term.NewlineReturn)
+	term.WriteString(term.NewlineReturn)
 }
 
 func (e *Engine) computeCoordinates(suggested bool) {
@@ -236,8 +244,8 @@ func (e *Engine) displayLine() {
 
 	// Adjust the cursor if the line fits exactly in the terminal width.
 	if e.lineCol == 0 {
-		fmt.Print(term.NewlineReturn)
-		fmt.Print(term.ClearLineAfter)
+		term.WriteString(term.NewlineReturn)
+		term.WriteString(term.ClearLineAfter)
 	}
 }
 

internal/display/refresh.go

@@ -15,8 +15,13 @@ import (
 // Refresh recomputes and redisplays the entire readline interface, except
 // the first lines of the primary prompt when the latter is a multiline one.
 func (e *Engine) Refresh() {
+	// Buffer the whole frame and flush once, so the terminal never shows a
+	// partial repaint and we issue a single write instead of dozens.
+	term.BeginBuffer()
+	defer term.EndBuffer()
+
 	// 1. Preparation & Coordinates
-	fmt.Print(term.HideCursor)
+	term.WriteString(term.HideCursor)
 	// Go back to the first column, and if the primary prompt
 	// was not printed yet, back up to the line's beginning row.
 	term.MoveCursorBackwards(term.GetWidth())
@@ -63,10 +68,11 @@ func (e *Engine) Refresh() {
 	// CUD + clear + CUU to clean up artifacts from previous renders.
 	termHeight := term.GetLength()
 	atBottom := (e.startRows + e.lineRows) >= termHeight
+
 	if !atBottom {
 		term.MoveCursorDown(1)
 		term.MoveCursorBackwards(term.GetWidth())
-		fmt.Print(term.ClearScreenBelow)
+		term.WriteString(term.ClearScreenBelow)
 		term.MoveCursorUp(1)
 		term.MoveCursorForwards(e.lineCol)
 	}
@@ -83,7 +89,7 @@ func (e *Engine) Refresh() {
 	term.MoveCursorBackwards(term.GetWidth())
 	term.MoveCursorForwards(e.cursorCol)
 
-	fmt.Print(term.ShowCursor)
+	term.WriteString(term.ShowCursor)
 }
 
 // repaintPromptUpperLines reprints the upper lines of a multi-line prompt at
@@ -131,7 +137,7 @@ func (e *Engine) renderHelpers() {
 		return
 	}
 
-	fmt.Print(term.NewlineReturn)
+	term.WriteString(term.NewlineReturn)
 
 	// 3. Display Hints
 	ui.DisplayHint(e.hint)
@@ -190,13 +196,13 @@ func (e *Engine) ensureIndicatorSpace() {
 
 		e.startCols += indicatorWidth
 		// Print the indicator on the first line.
-		fmt.Print(indicator)
+		term.WriteString(indicator)
 	} else if e.line.Lines() > 0 && e.startCols < indicatorWidth {
 		// If the prompt is shorter than the indicator, pad with spaces
 		// to ensure the input text starts aligned with subsequent lines
 		// and isn't overwritten by the indicator.
 		padding := indicatorWidth - e.startCols
-		fmt.Printf("%*s", padding, "")
+		term.Printf("%*s", padding, "")
 
 		e.startCols = indicatorWidth
 	}
@@ -237,7 +243,7 @@ func (e *Engine) ensureInputSpace() {
 	term.MoveCursorDown(e.lineRows)
 
 	for range deficit {
-		fmt.Print(term.NewlineReturn)
+		term.WriteString(term.NewlineReturn)
 	}
 
 	e.startRows -= deficit
@@ -301,15 +307,15 @@ func (e *Engine) renderMultilineIndicators() {
 	pipe := ui.DefaultMultilineColumn
 
 	for i := 1; i <= e.line.Lines(); i++ {
-		fmt.Print("\n")
+		term.WriteString("\n")
 
 		switch {
 		case numbered:
-			fmt.Printf(color.FgBlackBright+"%d"+color.Reset+" ", i+1)
+			term.Printf(color.FgBlackBright+"%d"+color.Reset+" ", i+1)
 		case i == e.line.Lines():
 			e.prompt.SecondaryPrint()
 		default:
-			fmt.Print(pipe)
+			term.WriteString(pipe)
 		}
 
 		printedLines++

internal/keymap/cursor.go

@@ -1,8 +1,9 @@
 package keymap
 
 import (
-	"fmt"
 	"strings"
+
+	"github.com/reeflective/readline/internal/term"
 )
 
 // CursorStyle is the style of the cursor
@@ -58,14 +59,14 @@ func (m *Engine) PrintCursor(keymap Mode) {
 	modeSet := strings.TrimSpace(m.config.GetString(cursorOptname))
 
 	if _, valid := cursors[CursorStyle(modeSet)]; valid {
-		fmt.Print(cursors[CursorStyle(modeSet)])
+		term.WriteString(cursors[CursorStyle(modeSet)])
 		return
 	}
 
 	if defaultCur, valid := defaultCursors[keymap]; valid {
-		fmt.Print(cursors[defaultCur])
+		term.WriteString(cursors[defaultCur])
 		return
 	}
 
-	fmt.Print(cursors[cursor])
+	term.WriteString(cursors[cursor])
 }

internal/keymap/dispatch_test.go

@@ -0,0 +1,101 @@
+package keymap
+
+import (
+	"testing"
+
+	"github.com/reeflective/readline/inputrc"
+	"github.com/reeflective/readline/internal/core"
+	"github.com/reeflective/readline/internal/strutil"
+)
+
+// builtinBindMaps returns every keymap exactly as the dispatcher sees it after
+// a default engine is built (GNU defaults + this library's builtin overlays).
+func builtinBindMaps(t *testing.T) map[string]map[string]inputrc.Bind {
+	t.Helper()
+
+	_, cfg := NewEngine(&core.Keys{}, &core.Iterations{})
+
+	return cfg.Binds
+}
+
+// collisions returns, for a keymap, the ConvertMeta key bytes that more than one
+// sequence maps to with DIFFERING binds — inputs whose exact match would depend
+// on iteration order if matchBind did not impose a deterministic order.
+func collisions(binds map[string]inputrc.Bind) map[string][]inputrc.Bind {
+	bySeq := make(map[string][]inputrc.Bind)
+
+	for sequence, bind := range binds {
+		seq := strutil.ConvertMeta([]rune(sequence))
+		bySeq[seq] = append(bySeq[seq], bind)
+	}
+
+	out := make(map[string][]inputrc.Bind)
+
+	for seq, list := range bySeq {
+		differ := false
+
+		for _, b := range list[1:] {
+			if b != list[0] {
+				differ = true
+
+				break
+			}
+		}
+
+		if differ {
+			out[seq] = list
+		}
+	}
+
+	return out
+}
+
+// TestMatchBindSortIsLoadBearing documents why matchBind must impose a
+// deterministic order: the default keymaps DO contain sequences that collapse
+// (via ConvertMeta) to the same key bytes with different actions, e.g. ESC-prefixed
+// meta bindings overlapping a self-insert. With those collisions present, dropping
+// the ordering (as a naive optimization would) makes the exact `match` depend on
+// Go's randomized map iteration — i.e. nondeterministic keybind resolution. If
+// this ever reports zero collisions, the determinism requirement can be revisited.
+func TestMatchBindSortIsLoadBearing(t *testing.T) {
+	binds := builtinBindMaps(t)[string(Emacs)]
+
+	cols := collisions(binds)
+	if len(cols) == 0 {
+		t.Fatal("expected ConvertMeta collisions in the emacs keymap; if truly none, matchBind no longer needs to order its matches")
+	}
+
+	t.Logf("emacs keymap has %d colliding key sequences whose exact match is disambiguated only by matchBind's ordering", len(cols))
+}
+
+// TestMatchBindResolvesDeterministically guards the property the ordering buys:
+// for a colliding input, matchBind must return the SAME exact match on every
+// call despite map iteration randomness. Runs many times so a regression to
+// unordered iteration would be caught.
+func TestMatchBindResolvesDeterministically(t *testing.T) {
+	eng := &Engine{}
+	binds := builtinBindMaps(t)[string(Emacs)]
+
+	cols := collisions(binds)
+	if len(cols) == 0 {
+		t.Skip("no collisions to probe")
+	}
+
+	// Pick one colliding input deterministically (smallest key bytes).
+	var probe string
+
+	for seq := range cols {
+		if probe == "" || seq < probe {
+			probe = seq
+		}
+	}
+
+	want, _ := eng.matchBind([]byte(probe), binds)
+
+	for range 64 {
+		got, _ := eng.matchBind([]byte(probe), binds)
+		if got != want {
+			t.Fatalf("matchBind(%q) is nondeterministic: got %+v, first saw %+v", probe, got, want)
+		}
+	}
+}

internal/keymap/emacs.go

@@ -6,8 +6,12 @@ var unescape = inputrc.Unescape
 
 // emacsKeys are the default keymaps in Emacs mode.
 var emacsKeys = map[string]inputrc.Bind{
-	unescape(`\C-D`):     {Action: "end-of-file"},
-	unescape(`\C-h`):     {Action: "backward-kill-word"},
+	unescape(`\C-D`): {Action: "end-of-file"},
+	// NOTE: \C-h is deliberately NOT overridden here. Many terminals send ^H
+	// for Backspace, and the GNU default (inputrc DefaultBinds) binds \C-h to
+	// backward-delete-char. Binding it to backward-kill-word made Backspace
+	// delete a whole word on those terminals; leaving it unset restores the
+	// standard char-delete behaviour.
 	unescape(`\C-N`):     {Action: "down-line-or-history"},
 	unescape(`\C-P`):     {Action: "up-line-or-history"},
 	unescape(`\C-x\C-b`): {Action: "vi-match"},