fix(overlay): unbreak the menu under atuin, wrapped commands, and max-height

README.md

@@ -309,7 +309,7 @@ style = "modern"       # "modern" or "classic"
 ghost-text = 1         # 0 = off, 1 = menu + ghost text, 2 = ghost text only
 hidden-files = false   # include dotfiles in suggestions
 max-suggestions = 100  # max suggestions ranked before display
-max-height = 15        # max visible rows in the menu
+max-height = 6         # max visible rows in the menu
 max-width = 0          # max menu width, 0 = auto
 nerd-fonts = true      # use nerd-font icons
 

integration/overlay.go

@@ -23,6 +23,9 @@ const (
 	defaultMaxItems = 6
 	// borderLines is the top and bottom border the box draws around its items.
 	borderLines = 2
+	// maxMenuItems is the largest ui.max-height accepted, mirroring the bound
+	// config validation enforces.
+	maxMenuItems = 50
 )
 
 // lastDrawnLines records the total height of the most recently drawn box, so a
@@ -31,40 +34,58 @@ const (
 // leave the surplus rows on screen.
 var lastDrawnLines atomic.Int32
 
+// lastDrawnInputRows records how many rows the prompt plus the typed text
+// wrapped onto when the box was last drawn. The box hangs off the cursor, so
+// when that count changes the whole box moves with it and the terminal keeps
+// whatever the new one no longer covers.
+var lastDrawnInputRows atomic.Int32
+
+// inputRows is how many rows past the first the prompt and typed text occupy.
+func inputRows(totalCol int) int {
+	w := termWidth()
+	if w <= 0 {
+		return 0
+	}
+	return totalCol / w
+}
+
 // menuItemRows is how many suggestion rows the overlay may show.
 //
-// ui.max-height is the height of the whole box, matching its name and the
-// comment `iris config init` writes, so the borders come out of that budget.
-// It is also clamped to the terminal: a box taller than the window can't be
-// scrolled back into view and would push the prompt off screen.
+// ui.max-height counts suggestions, not the lines the box occupies: it is the
+// number people compare against what they can see, and counting the border
+// into it meant max-height = 6 drew 4 rows.
+// It is still clamped to the terminal, since a box taller than the window
+// can't be scrolled back into view and would push the prompt off screen.
 func menuItemRows() int {
 	rows := config.Get().UI.MaxHeight
-	if rows < 3 || rows > 50 {
-		rows = defaultMaxItems + borderLines
+	if rows < 1 || rows > maxMenuItems {
+		rows = defaultMaxItems
 	}
 
 	if h := termHeight(); h > 0 {
-		// Leave a row for the prompt itself plus one of breathing room.
-		if avail := h - 2; rows > avail {
+		// Leave room for the border, the prompt itself, and one row of
+		// breathing room.
+		if avail := h - borderLines - 2; rows > avail {
 			rows = avail
 		}
 	}
 
-	rows -= borderLines
 	if rows < 1 {
 		rows = 1
 	}
 	return rows
 }
 
 // clearRows is the number of lines a clear must erase: whatever is on screen
-// now, which may be taller than what the next draw will produce.
+// now, which may be taller than what the next draw will produce, plus the rows
+// a box drawn against a wrapped input reached further down than the cursor
+// sits today.
 func clearRows() int {
 	n := int(lastDrawnLines.Load())
 	if want := menuItemRows() + borderLines; want > n {
 		n = want
 	}
-	return n
+	return n + int(lastDrawnInputRows.Load())
 }
 
 func ComputeCursorCol(data []byte) int {
@@ -185,6 +206,16 @@ type Overlay struct {
 	TypedQuery    string
 	UserNavigated bool
 	PromptLen     int
+	// CursorAtEnd gates the erase that follows the input onto a new wrapped
+	// row: it walks to the end-of-text column, which is only where the cursor
+	// really is when nothing has moved it left.
+	CursorAtEnd bool
+}
+
+func (o *Overlay) SetCursorAtEnd(v bool) {
+	o.mu.Lock()
+	defer o.mu.Unlock()
+	o.CursorAtEnd = v
 }
 
 func (o *Overlay) SetPromptLen(l int) {
@@ -197,7 +228,7 @@ func (o *Overlay) SetPromptLen(l int) {
 }
 
 func NewOverlay() *Overlay {
-	return &Overlay{Visible: false, Cursor: 0, StartIdx: 0}
+	return &Overlay{Visible: false, Cursor: 0, StartIdx: 0, CursorAtEnd: true}
 }
 
 func (o *Overlay) UpdateItems(items []spec.Suggestion) {
@@ -635,6 +666,26 @@ func (o *Overlay) draw() string {
 	}
 	logger.Debugf("Overlay draw: pLen=%d, typedLen=%d, totalCol=%d, cursorCol=%d, targetCol=%d, width=%d", o.PromptLen, typedLen, totalCol, cursorCol, targetCol, width)
 
+	// The box is placed relative to the cursor, so a change in how many rows the
+	// input wraps onto moves it. Whatever the previous box covered and this one
+	// will not has to be erased by hand; the shell only repaints the input.
+	rowsNow := inputRows(totalCol)
+	wrapShift := int(lastDrawnInputRows.Load()) - rowsNow
+	lastDrawnInputRows.Store(int32(rowsNow))
+
+	if wrapShift < 0 && o.CursorAtEnd {
+		// the input grew onto the row the old top border was on, and the shell
+		// redraw stops where the text stops. Start past the ghost text so the
+		// hint written just before this survives.
+		s.WriteString(ansi.SaveCursor)
+		s.WriteString("\r")
+		if from := cursorCol + o.LastGhostLen; from > 0 {
+			s.WriteString(ansi.CursorForward(from))
+		}
+		s.WriteString(ansi.EraseLineRight)
+		s.WriteString(ansi.RestoreCursor)
+	}
+
 	s.WriteString(ansi.SaveCursor)
 
 	windowSize := min(len(o.Items), menuItemRows())
@@ -843,6 +894,13 @@ func (o *Overlay) draw() string {
 
 	s.WriteString(titledEdge("╰", "╯", inner, footerInfo, border, inner-lipgloss.Width(footerInfo)-2))
 
+	// the previous box sat lower down; erase the rows this one leaves behind
+	for i := range wrapShift {
+		s.WriteString(ansi.RestoreCursor)
+		s.WriteString(ansi.CursorDown(totalLines + i + 1))
+		s.WriteString("\r" + ansi.EraseEntireLine)
+	}
+
 	s.WriteString(ansi.RestoreCursor)
 	s.WriteString(ansi.SetModeAutoWrap)
 	return s.String()
@@ -897,6 +955,8 @@ func (o *Overlay) HideMenu(query string) string {
 	}
 
 	clearLinesBelow(&s, clearRows())
+	// the box is gone, so nothing is hanging below a wrapped input any more
+	lastDrawnInputRows.Store(0)
 	s.WriteString(ansi.SetModeAutoWrap)
 	return s.String()
 }
@@ -927,6 +987,8 @@ func (o *Overlay) ClearAndDisable() string {
 	}
 
 	clearLinesBelow(&s, clearRows())
+	// the box is gone, so nothing is hanging below a wrapped input any more
+	lastDrawnInputRows.Store(0)
 	s.WriteString(ansi.SetModeAutoWrap)
 	return s.String()
 }

integration/overlay_test.go

@@ -161,15 +161,16 @@ func TestRenderMatchedTitle_CaseInsensitive(t *testing.T) {
 }
 
 func TestMenuItemRowsHonorsMaxHeight(t *testing.T) {
-	// ui.max-height was parsed and validated but never read: the box was fixed
-	// at 6 rows however it was configured, unlike ui.max-width.
+	// ui.max-height counts suggestion rows. It was read as the height of the
+	// whole box for a while, so a configured 6 drew 4 rows.
 	tests := []struct {
 		name      string
 		maxHeight int
 		want      int
 	}{
-		{"configured", 15, 15 - borderLines},
-		{"minimum", 3, 1},
+		{"configured", 15, 15},
+		{"borders do not eat rows", 6, 6},
+		{"minimum", 1, 1},
 		{"zero falls back", 0, defaultMaxItems},
 		{"out of range falls back", 999, defaultMaxItems},
 	}
@@ -209,7 +210,7 @@ func TestScrolloffIsSymmetric(t *testing.T) {
 	// The window kept a row of context above the highlight but none below, so
 	// paging down pinned it to the last visible row while paging up did not.
 	cfg := config.DefaultConfig()
-	cfg.UI.MaxHeight = 8 // 6 item rows
+	cfg.UI.MaxHeight = 6
 	config.Init(cfg)
 
 	items := make([]spec.Suggestion, 20)
@@ -239,3 +240,98 @@ func TestScrolloffIsSymmetric(t *testing.T) {
 		}
 	}
 }
+
+func TestInputRowsCountsWrappedRows(t *testing.T) {
+	// termWidth() falls back to 120 when stdout is not a TTY
+	tests := []struct {
+		totalCol int
+		want     int
+	}{
+		{0, 0},
+		{119, 0},
+		{120, 1},
+		{300, 2},
+	}
+
+	for _, tt := range tests {
+		if got := inputRows(tt.totalCol); got != tt.want {
+			t.Errorf("inputRows(%d) = %d; want %d", tt.totalCol, got, tt.want)
+		}
+	}
+}
+
+func wrappedOverlay(t *testing.T, promptLen int) (*Overlay, string) {
+	t.Helper()
+	cfg := config.DefaultConfig()
+	config.Init(cfg)
+
+	items := make([]spec.Suggestion, 10)
+	for i := range items {
+		items[i] = spec.Suggestion{Cmd: string(rune('a' + i))}
+	}
+
+	o := NewOverlay()
+	o.SetQueryAndItems("q", items)
+	o.SetPromptLen(promptLen)
+	return o, o.Render()
+}
+
+func TestDrawErasesRowsLeftBehindWhenInputStopsWrapping(t *testing.T) {
+	// The box hangs off the cursor. A command long enough to wrap pushes the
+	// cursor down, so the box sits lower; when the line shrinks back the cursor
+	// moves up and the rows the old box occupied were left on screen.
+	o, _ := wrappedOverlay(t, 250) // two wrapped rows
+
+	o.SetPromptLen(10) // back to a single row
+	out := o.Render()
+
+	totalLines := min(len(o.Items), menuItemRows()) + borderLines
+	for _, extra := range []int{1, 2} {
+		want := ansi.CursorDown(totalLines + extra)
+		if !strings.Contains(out, want) {
+			t.Errorf("redraw does not erase row %d below the box (missing %q)", extra, want)
+		}
+	}
+}
+
+func TestDrawErasesTheStaleTopBorderWhenInputGrows(t *testing.T) {
+	// The other direction: the input wraps onto the row the previous top border
+	// was on. The shell repaints only as far as the text reaches, so the tail of
+	// the old border survives to its right.
+	o, _ := wrappedOverlay(t, 10)
+
+	o.SetPromptLen(250)
+	out := o.Render()
+
+	if !strings.Contains(out, ansi.EraseLineRight) {
+		t.Errorf("redraw does not erase the stale border to the right of the wrapped input: %q", out)
+	}
+}
+
+func TestClearCoversABoxDrawnUnderWrappedInput(t *testing.T) {
+	o, _ := wrappedOverlay(t, 250)
+
+	rows := clearRows()
+	if want := min(len(o.Items), menuItemRows()) + borderLines + 2; rows != want {
+		t.Errorf("clearRows() = %d; want %d (the box plus the two rows it hangs below the cursor)", rows, want)
+	}
+
+	o.ClearAndDisable()
+	if got := lastDrawnInputRows.Load(); got != 0 {
+		t.Errorf("lastDrawnInputRows after teardown = %d; want 0", got)
+	}
+}
+
+func TestDrawLeavesTheLineAloneWhenTheCursorIsNotAtTheEnd(t *testing.T) {
+	// The erase walks to the end-of-text column, so with the cursor moved back
+	// into the middle of the command it would land mid-text and wipe it.
+	o, _ := wrappedOverlay(t, 10)
+
+	o.SetCursorAtEnd(false)
+	o.SetPromptLen(250)
+	out := o.Render()
+
+	if strings.Contains(out, ansi.EraseLineRight) {
+		t.Error("redraw erased to end of line while the cursor was mid-command")
+	}
+}

integration/shell/adapter.go

@@ -14,21 +14,25 @@ import (
 )
 
 // ReplaceLine builds the byte sequence that clears the shell's current input
-// line and types text in its place.
+// line and types text in its place. cursorFromEnd is how many characters the
+// cursor sits to the left of the end of the line.
 //
-// The clear is ctrl+e then ctrl+u, not ctrl+u alone. ctrl+u is only
-// kill-whole-line under zsh's stock emacs keymap; `bindkey '^U'
-// backward-kill-line` is a common preference, and it is also what bash does by
-// default (unix-line-discard). Those kill from the cursor backwards, so with
-// the cursor mid-line everything to its right survived and collided with the
-// text typed next -- selecting a suggestion left the tail of the old buffer
-// interleaved through the new one.
+// ctrl+u only clears the whole line from the end: it is kill-whole-line in
+// zsh's stock emacs keymap, but kills backwards under bash (unix-line-discard),
+// fish, zsh's vi keymap, and `bindkey '^U' backward-kill-line`. Left mid-line,
+// everything to the right of the cursor survived and collided with the text
+// typed next.
 //
-// Moving to end-of-line first makes all three bindings equivalent: there is
-// nothing to the right left to preserve.
-func ReplaceLine(text []byte) []byte {
-	out := make([]byte, 0, len(text)+2)
-	out = append(out, 0x05, 0x15) // ctrl+e (end-of-line), ctrl+u (kill)
+// The cursor therefore has to reach the end first, and it walks there with the
+// right arrow rather than ctrl+e. ctrl+e is a key people rebind -- binding it
+// to atuin-search is common, and iris sending it opened the atuin overlay every
+// time it rewrote the line.
+func ReplaceLine(text []byte, cursorFromEnd int) []byte {
+	out := make([]byte, 0, len(text)+3*max(cursorFromEnd, 0)+1)
+	for range cursorFromEnd {
+		out = append(out, 0x1b, '[', 'C') // right arrow (forward-char)
+	}
+	out = append(out, 0x15) // ctrl+u (kill)
 	return append(out, text...)
 }
 
@@ -37,7 +41,7 @@ type Adapter interface {
 	GetName() string
 	GetShellPath() string
 	GetEnv(fd int, pid int) []string
-	PrepareSelectSequence(selected string) []byte
+	PrepareSelectSequence(selected string, cursorFromEnd int) []byte
 	// ScanAliases returns a map of alias name to target command
 	ScanAliases() map[string]string
 }
@@ -64,8 +68,8 @@ func (b *BashAdapter) GetShellPath() string { return "bash" }
 func (b *BashAdapter) GetEnv(fd int, pid int) []string {
 	return append(os.Environ(), "IRIS_FD="+fmt.Sprint(fd), "IRIS_PID="+fmt.Sprint(pid))
 }
-func (b *BashAdapter) PrepareSelectSequence(selected string) []byte {
-	return ReplaceLine([]byte(selected))
+func (b *BashAdapter) PrepareSelectSequence(selected string, cursorFromEnd int) []byte {
+	return ReplaceLine([]byte(selected), cursorFromEnd)
 }
 func (b *BashAdapter) ScanAliases() map[string]string {
 	return ScanPosixAliases([]string{".bashrc", ".bash_profile", ".bash_aliases"})
@@ -79,8 +83,8 @@ func (z *ZshAdapter) GetShellPath() string { return "zsh" }
 func (z *ZshAdapter) GetEnv(fd int, pid int) []string {
 	return append(os.Environ(), "IRIS_FD="+fmt.Sprint(fd), "IRIS_PID="+fmt.Sprint(pid))
 }
-func (z *ZshAdapter) PrepareSelectSequence(selected string) []byte {
-	return ReplaceLine([]byte(selected))
+func (z *ZshAdapter) PrepareSelectSequence(selected string, cursorFromEnd int) []byte {
+	return ReplaceLine([]byte(selected), cursorFromEnd)
 }
 func (z *ZshAdapter) ScanAliases() map[string]string {
 	envSet := os.Getenv("ZDOTDIR") != ""
@@ -139,8 +143,8 @@ func (f *FishAdapter) GetShellPath() string { return "fish" }
 func (f *FishAdapter) GetEnv(fd int, pid int) []string {
 	return append(os.Environ(), "IRIS_FD="+fmt.Sprint(fd), "IRIS_PID="+fmt.Sprint(pid))
 }
-func (f *FishAdapter) PrepareSelectSequence(selected string) []byte {
-	return ReplaceLine([]byte(selected))
+func (f *FishAdapter) PrepareSelectSequence(selected string, cursorFromEnd int) []byte {
+	return ReplaceLine([]byte(selected), cursorFromEnd)
 }
 func (f *FishAdapter) ScanAliases() map[string]string {
 	// fish uses 'alias' command in config.fish or separate function files