diff --git a/internal/tui/app.go b/internal/tui/app.go index 33d30b1..5bcd208 100644 --- a/internal/tui/app.go +++ b/internal/tui/app.go @@ -448,17 +448,20 @@ func (m model) View() tea.View { s = lipgloss.JoinVertical(lipgloss.Center, banner(), "", body) } - vAlign := lipgloss.Center - if lipgloss.Height(s) > m.height { - // Content taller than the terminal: centering vertically would clip - // it top and bottom with no way to scroll back to what's lost. - // Anchor to the top instead so everything stays reachable. - vAlign = lipgloss.Top + // Anchored to the top, not centered: a centered screen re-centers + // vertically every time its height changes, e.g. a toast, a progress + // panel, or a search prompt appearing. Anchoring to the top means only + // content below the change moves, not the whole screen. + const topMargin = 1 + margin := topMargin + if lipgloss.Height(s)+margin > m.height { + margin = 0 } + s = strings.Repeat("\n", margin) + s // Overlays go on last, over the finished screen: they must not be part of // what Place centers. The modal sits under the toast, so a toast raised // while the picker is open is still readable. - return m.newView(m.renderToast(m.renderModal(lipgloss.Place(m.width, m.height, lipgloss.Center, vAlign, s)))) + return m.newView(m.renderToast(m.renderModal(lipgloss.Place(m.width, m.height, lipgloss.Center, lipgloss.Top, s)))) } // newView wraps a rendered frame in a tea.View with the alt-screen flag set, diff --git a/internal/tui/detail.go b/internal/tui/detail.go index 136cb23..bcdd001 100644 --- a/internal/tui/detail.go +++ b/internal/tui/detail.go @@ -310,12 +310,9 @@ func (m model) viewDetail() string { v := m.detail.vm if v.Name == "" { - parts := []string{pane("", dimStyle.Render("no vm selected"), m.width), ""} - if m.status != "" { - parts = append(parts, warnStyle.Render(m.status)) - } + parts := []string{pane("", dimStyle.Render("no vm selected"), m.width), "", warnStyle.Render(m.status)} parts = append(parts, dimStyle.Render("esc back")) - return lipgloss.JoinVertical(lipgloss.Center, parts...) + return column(appContentWidth, parts...) } if m.detail.pager != nil { @@ -333,7 +330,7 @@ func (m model) viewDetail() string { state = upStyle.Render("running") } - var facts fields + facts := fields{width: appContentWidth} facts.row("", "", dimStyle.Render(modeLabel(v.Mode))+dimStyle.Render(glyphSep)+state) facts.hint(modeHint(v.Mode)) facts.gap() @@ -462,13 +459,13 @@ func (m model) viewDetail() string { } } - factsBox := pane(v.Name, facts.String(), m.width) + factsBox := paneAt(v.Name, facts.String(), appContentWidth, m.width) parts := []string{factsBox} if m.detail.log != "" { // lipgloss re-applies the style on every line of a multi-line string, // so the log needs no per-line loop of its own. - parts = append(parts, "", pane("last provision", dimStyle.Render(m.detail.log), m.width)) + parts = append(parts, "", paneAt("last provision", dimStyle.Render(m.detail.log), appContentWidth, m.width)) } if panel := progressPanel(m); panel != "" { @@ -479,14 +476,12 @@ func (m model) viewDetail() string { for _, l := range provLinesExcept(m, v.Name) { parts = append(parts, l) } - if m.status != "" { - parts = append(parts, warnStyle.Render(m.status)) - } + // An always-present slot: an appearing status line replaces blank + // space instead of pushing the footer down. + parts = append(parts, warnStyle.Render(m.status)) parts = append(parts, renderFooter(detailHelp{ sshAvailable: v.State == core.StateRunning, consolePassword: consolePasswordAvailable(v), }, m.width, m.showHelp)) - // Center for the same reason as the list: the footer is wider than the - // pane, and a left join would pin the pane to the footer's left edge. - return lipgloss.JoinVertical(lipgloss.Center, parts...) + return column(appContentWidth, parts...) } diff --git a/internal/tui/detail_test.go b/internal/tui/detail_test.go index 6eab418..523a2a9 100644 --- a/internal/tui/detail_test.go +++ b/internal/tui/detail_test.go @@ -14,6 +14,15 @@ import ( "github.com/novusedge/stoat/internal/qemu" ) +// containsWrapped reports whether s contains needle once the pane's border +// and whitespace are stripped. The facts pane holds every value to a fixed +// column width now, so a path longer than that column wraps mid-token +// across two rendered lines instead of pushing the pane wider. +func containsWrapped(s, needle string) bool { + flat := strings.NewReplacer("\n", "", " ", "", "│", "").Replace(s) + return strings.Contains(flat, needle) +} + // TestTickGenerationOnlyReArmsCurrentChain proves the fix for the ticker // chain leak. A tickMsg carrying a stale generation must not re-arm; its // chain dies. One carrying the current generation must re-arm. Without the @@ -240,7 +249,7 @@ func TestDetailSurfacesVNCForAHeadlessVM(t *testing.T) { if !strings.Contains(out, "vnc") { t.Fatalf("headless VM's detail screen must show a vnc row:\n%s", out) } - if !strings.Contains(out, sock) { + if !containsWrapped(out, sock) { t.Fatalf("vnc row must show the actual socket path %q:\n%s", sock, out) } if strings.Contains(out, "qemu window only") { @@ -278,7 +287,7 @@ func TestDetailExplainsTheVNCFallbackOnAHeadlessHost(t *testing.T) { m.detail = newDetail(v) out := ansi.Strip(m.viewDetail()) - if !strings.Contains(out, sock) { + if !containsWrapped(out, sock) { t.Errorf("the install console is on the socket now; the detail screen must show it:\n%s", out) } if !strings.Contains(out, "no usable graphical session on this host") { diff --git a/internal/tui/edit.go b/internal/tui/edit.go index 6c6910f..f5ff6a9 100644 --- a/internal/tui/edit.go +++ b/internal/tui/edit.go @@ -9,7 +9,6 @@ import ( "charm.land/bubbles/v2/textinput" tea "charm.land/bubbletea/v2" - "charm.land/lipgloss/v2" "github.com/novusedge/stoat/internal/config" "github.com/novusedge/stoat/internal/core" @@ -522,12 +521,12 @@ func (m model) viewEdit() string { box := paneAt("edit "+e.vm.Name, body, editContentWidth, m.width) - parts := []string{box, ""} - if m.status != "" { - parts = append(parts, warnStyle.Render(m.status)) - } + // The status slot is always present, blank when there is nothing to + // show, so an appearing message replaces blank space instead of + // pushing the footer down. + parts := []string{box, "", warnStyle.Render(m.status)} parts = append(parts, renderFooter(editHelp{}, m.width, m.showHelp)) - return lipgloss.JoinVertical(lipgloss.Center, parts...) + return column(appContentWidth, parts...) } func editRecipesLabel(e editModel) string { diff --git a/internal/tui/form.go b/internal/tui/form.go index fba5cfd..67d6b05 100644 --- a/internal/tui/form.go +++ b/internal/tui/form.go @@ -11,7 +11,6 @@ import ( "charm.land/bubbles/v2/textinput" tea "charm.land/bubbletea/v2" - "charm.land/lipgloss/v2" "github.com/charmbracelet/x/ansi" "github.com/novusedge/stoat/internal/cloudinit" @@ -223,7 +222,10 @@ func byoOSNames() []string { // // 72 plus the pane frame is 78 cells, so the box fits an 80-column terminal // without paneAt having to clamp it. -const formContentWidth = 72 +// +// Equal to appContentWidth: the form and the list share one left edge and +// one width, so switching between screens does not shift the column. +const formContentWidth = appContentWidth type formModel struct { inputs []textinput.Model // name, ram, cpus, disk, share @@ -930,15 +932,12 @@ func (m model) viewForm() string { box := paneAt("new vm", body, formContentWidth, m.width) - // Center rather than Left: the footer is far wider than the box, so a - // left join pins the box to the footer's left edge and the pane reads as - // off-center once the whole rectangle is placed. - parts := []string{box, ""} - if m.status != "" { - parts = append(parts, warnStyle.Render(m.status)) - } + // The status slot is always present, blank when there is nothing to + // show, so an appearing message replaces blank space instead of + // pushing the footer down. + parts := []string{box, "", warnStyle.Render(m.status)} parts = append(parts, renderFooter(formHelp{}, m.width, m.showHelp)) - return lipgloss.JoinVertical(lipgloss.Center, parts...) + return column(appContentWidth, parts...) } // recipesLabel renders the recipes row's checkbox list, highlighting the diff --git a/internal/tui/geometry_test.go b/internal/tui/geometry_test.go index 78ee4a3..cb411e8 100644 --- a/internal/tui/geometry_test.go +++ b/internal/tui/geometry_test.go @@ -4,10 +4,13 @@ import ( "fmt" "strings" "testing" + "time" "charm.land/bubbles/v2/help" + "charm.land/bubbles/v2/list" tea "charm.land/bubbletea/v2" "charm.land/lipgloss/v2" + "github.com/charmbracelet/x/ansi" "github.com/novusedge/stoat/internal/core" ) @@ -74,6 +77,35 @@ func TestListWidthFitsARunningRow(t *testing.T) { } } +// TestColumnHoldsEveryLineWithinWidth pins column's left-align contract: a +// caller stacking narrower pieces gets back a block padded to width, not one +// that drifts wider or narrower line by line. +func TestColumnHoldsEveryLineWithinWidth(t *testing.T) { + out := column(appContentWidth, "short", "a bit longer than short", "") + for _, l := range strings.Split(out, "\n") { + if w := lipgloss.Width(l); w != appContentWidth { + t.Errorf("line %q is %d cells wide, want %d", l, w, appContentWidth) + } + } +} + +// TestListRowWidthInvariantForLongNames covers the bug hand-padded +// fmt.Sprintf rows had: a name longer than its column pushed every column +// after it to the right instead of being cut off. vmDelegate.Render must +// truncate the name instead, so the row's rendered width stays fixed +// regardless of how long a VM's name is. +func TestListRowWidthInvariantForLongNames(t *testing.T) { + v := core.VM{ + Name: strings.Repeat("n", 200), Mode: "cloud", State: core.StateRunning, + RAM: 8192, CPUs: 8, SSHPort: 65535, StartedAt: time.Now().Add(-999 * time.Hour), + } + l := list.New([]list.Item{vmItem{vm: v}}, vmDelegate{}, listWidth, 2) + if got := lipgloss.Width(ansi.Strip(l.View())); got > listWidth { + t.Errorf("row for a %d-char name rendered %d cells wide, want <= listWidth (%d)", + len(v.Name), got, listWidth) + } +} + // TestFooterNeverOverflows covers help.Model giving up on truncation. Once // its running total passes the width, it can no longer fit an ellipsis, so // it appends every remaining binding and returns a line WIDER than the diff --git a/internal/tui/list.go b/internal/tui/list.go index 5f0a4ba..1575cf9 100644 --- a/internal/tui/list.go +++ b/internal/tui/list.go @@ -4,7 +4,6 @@ import ( "strings" tea "charm.land/bubbletea/v2" - "charm.land/lipgloss/v2" "github.com/charmbracelet/x/ansi" "github.com/novusedge/stoat/internal/core" @@ -289,36 +288,24 @@ func (m model) viewList() string { } box = joinAccess(box, accessBox(cur, ci, m.ciProg, m.width), m.width) - // lipgloss.Place centers (or left-aligns) each LINE of a string - // independently, sized against that string's widest line. Handing it - // box+status+footer concatenated as-is would make every shorter line - // drift toward center on its own: the "justified" look this used to - // have. JoinVertical instead pads every line of every piece out to the - // widest piece's width first, so the result is one rectangle that moves - // as a whole once centered in the terminal. - // - // Center, not Left, join: the footer is much wider than the box, so a - // left join pins the box to the footer's left edge, leaving it visibly - // off-center under the centered banner. - parts := []string{box, ""} - // The search line sits between the pane and the status: while the input - // is open it IS the input, and once a filter is applied it reports what - // is being hidden. Without it a filtered list just looks like VMs went - // missing. - if search := listStatusLine(m.list); search != "" { - parts = append(parts, search, "") - } + // column holds every piece to appContentWidth and stacks them left- + // aligned. Without it, the box, the status line, and the footer would + // each center on their own width, drifting sideways from one another + // as content came and went. + // The search line and the status line are always-present slots, blank + // when there is nothing to show. A conditionally-appended line pushes + // everything below it down the moment it appears; a slot that is + // always there just fills in. + parts := []string{box, "", listStatusLine(m.list)} // In-flight provision runs sit above the status line: they are ongoing // state, not a one-off message, and a run started from here keeps going // while the user moves around the list. for _, l := range provLines(m) { parts = append(parts, l) } - if m.status != "" { - parts = append(parts, warnStyle.Render(m.status)) - } + parts = append(parts, warnStyle.Render(m.status)) v := m.current() sshAvailable := v != nil && v.State == core.StateRunning parts = append(parts, renderFooter(listHelp{sshAvailable: sshAvailable}, m.width, m.showHelp)) - return lipgloss.JoinVertical(lipgloss.Center, parts...) + return column(appContentWidth, parts...) } diff --git a/internal/tui/progressbar.go b/internal/tui/progressbar.go index f665373..4dce081 100644 --- a/internal/tui/progressbar.go +++ b/internal/tui/progressbar.go @@ -66,12 +66,11 @@ func renderProgressPanel(phase string, p Progress, hasProg bool, elapsed time.Du var f fields f.row("", "phase", dimStyle.Render(phase)) if hasProg { - f.row("", "", bar(p.Frac, detailBarWidth)) num := fmt.Sprintf("%3.0f%%", p.Frac*100) if p.Total > 0 { num = fmt.Sprintf("%d/%d", p.Done, p.Total) } - f.row("", "", dimStyle.Render(num)) + f.row("", "", bar(p.Frac, detailBarWidth)+" "+dimStyle.Render(num)) if p.Label != "" { f.row("", "package", p.Label) } @@ -79,5 +78,5 @@ func renderProgressPanel(phase string, p Progress, hasProg bool, elapsed time.Du f.row("", "", dimStyle.Render("waiting for output…")) } f.row("", "elapsed", provElapsed(elapsed)) - return pane("progress", f.String(), width) + return paneAt("progress", f.String(), appContentWidth, width) } diff --git a/internal/tui/provstep.go b/internal/tui/provstep.go index 1192a3c..19bff29 100644 --- a/internal/tui/provstep.go +++ b/internal/tui/provstep.go @@ -131,7 +131,11 @@ func provLine(spin spinner.Model, name string, st provState, now time.Time) stri case st.last != "": out += dimStyle.Render(" · " + ansi.Truncate(st.last, provMaxLast, "…")) } - return out + dimStyle.Render(" · "+provElapsed(now.Sub(st.start))) + out += dimStyle.Render(" · " + provElapsed(now.Sub(st.start))) + // A long VM name can still push the line past the pane and wrap what + // should be one row. Every field above already has its own cap; this + // is the backstop for the sum of them. + return ansi.Truncate(out, appContentWidth, "…") } // installLine renders one disk VM mid unattended install the same way diff --git a/internal/tui/theme.go b/internal/tui/theme.go index 5681fd4..05e7f24 100644 --- a/internal/tui/theme.go +++ b/internal/tui/theme.go @@ -115,6 +115,30 @@ func radio(label string, on bool) string { // one place. const rowGap = "\n\n" +// appContentWidth is the left edge every screen's stacked panes share. Before +// this, each pane centered on its own width, so switching screens, or a pane +// changing size as content appeared, shifted the whole block sideways. +const appContentWidth = 72 + +// column stacks parts left-aligned inside one block, at least width cells +// wide. Every part then starts at the same left edge, instead of each one +// centering on its own width. +// +// The block widens for a part wider than width rather than wrapping it: a +// lipgloss Style.Width() narrower than its content word-wraps that content, +// which mangles a bordered pane's box-drawing runs instead of just leaving +// it be. The list pane plus its side-by-side access box is routinely wider +// than appContentWidth, so this has to hold. +func column(width int, parts ...string) string { + for _, p := range parts { + if w := lipgloss.Width(p); w > width { + width = w + } + } + return lipgloss.NewStyle().Width(width).Align(lipgloss.Left). + Render(lipgloss.JoinVertical(lipgloss.Left, parts...)) +} + // paneAt draws a pane whose content is held at a fixed width, for screens // whose rows come and go, e.g. a download block, an error line, a // conditional disk row. pane() hugs its content, so without this the box diff --git a/internal/tui/vmlist.go b/internal/tui/vmlist.go index 701ef52..6926995 100644 --- a/internal/tui/vmlist.go +++ b/internal/tui/vmlist.go @@ -3,16 +3,37 @@ package tui import ( "fmt" "io" - "strings" "time" "charm.land/bubbles/v2/list" tea "charm.land/bubbletea/v2" "charm.land/lipgloss/v2" + "github.com/charmbracelet/x/ansi" "github.com/novusedge/stoat/internal/core" ) +// Row cells are fixed-width lipgloss styles, not fmt padding, so a value +// wider than its column truncates or wraps instead of shoving every column +// after it out of place. +const ( + nameCellWidth = 14 + modeCellWidth = 5 + ramValueWidth = 5 // digits only, right-aligned; "M" is appended after + cpuValueWidth = 2 // digits only, right-aligned; "c" is appended after + upCellWidth = 13 // "up " plus a duration up to "999h59m59s" + portCellWidth = 6 // ":" plus up to 5 digits +) + +var ( + nameCellStyle = lipgloss.NewStyle().Width(nameCellWidth) + modeCellStyle = lipgloss.NewStyle().Width(modeCellWidth) + ramCellStyle = lipgloss.NewStyle().Width(ramValueWidth).Align(lipgloss.Right) + cpuCellStyle = lipgloss.NewStyle().Width(cpuValueWidth).Align(lipgloss.Right) + upCellStyle = lipgloss.NewStyle().Width(upCellWidth) + portCellStyle = lipgloss.NewStyle().Width(portCellWidth).Align(lipgloss.Right) +) + // vmItem is one row of the VM list. A single type covers both good VMs and // broken ones (a directory whose vm.toml fails to parse), since the cursor // ranges over them as one sequence. Modelling them as two lists would mean @@ -59,22 +80,19 @@ func (d vmDelegate) Render(w io.Writer, m list.Model, index int, item list.Item) // even when the row isn't selected, while the text stays muted so a // whole line of red isn't shouting from the list. plain := fmt.Sprintf("%-14s broken: %s", it.vm.Name, brokenReason(it.vm.Error)) + glyph := cursor + errStyle.Render(glyphBroken) + " " // A long reason wraps inside the pane. paneAt wraps the whole - // rendered list as one blob and has no idea this line starts 4 - // columns in: the cursor, the glyph, and the space after it. Left - // to paneAt, the continuation lands flush against the pane's - // padding, under the cursor instead of under the text. Wrapping - // here first, with a hanging indent matching that prefix, makes - // every physical line already fit listWidth, so paneAt has - // nothing left to rewrap. - prefixWidth := lipgloss.Width(cursor) + lipgloss.Width(glyphBroken) + 1 // +1 for the space after the glyph - plain = strings.ReplaceAll(lipgloss.Wrap(plain, listWidth-prefixWidth, ""), "\n", "\n"+strings.Repeat(" ", prefixWidth)) + // rendered list as one blob and has no idea this line starts past + // the cursor and the glyph. Wrapping the reason to its own column + // here, then joining it beside the glyph, keeps every continuation + // line under the text instead of flush against the pane's padding. + reason := lipgloss.NewStyle().Width(listWidth - lipgloss.Width(glyph)).Render(plain) if selected { - plain = selStyle.Render(plain) + reason = selStyle.Render(reason) } else { - plain = downStyle.Render(plain) + reason = downStyle.Render(reason) } - fmt.Fprint(w, cursor+errStyle.Render(glyphBroken)+" "+plain) + fmt.Fprint(w, lipgloss.JoinHorizontal(lipgloss.Top, glyph, reason)) return } @@ -97,14 +115,20 @@ func (d vmDelegate) Render(w io.Writer, m list.Model, index int, item list.Item) if !v.StartedAt.IsZero() { up = "up " + time.Since(v.StartedAt).Truncate(time.Second).String() } - state = fmt.Sprintf("%s :%d", up, v.SSHPort) + // Their own fixed cells, so the port lands in the same column on + // every running row regardless of how long the uptime string is. + state = upCellStyle.Render(up) + " " + portCellStyle.Render(fmt.Sprintf(":%d", v.SSHPort)) } // The dot and the state stay OUTSIDE the selection wrap. A styled // substring ends in \x1b[0m, which resets the enclosing style too. // Wrapping a row that starts with a coloured dot would leave everything // after it unhighlighted, and a trailing dim "-" would render unbolded // inside an otherwise highlighted row. - label := fmt.Sprintf("%-14s %-5s %5dM %2dc ", v.Name, v.Mode, v.RAM, v.CPUs) + name := nameCellStyle.Render(ansi.Truncate(v.Name, nameCellWidth, "…")) + mode := modeCellStyle.Render(v.Mode) + ram := ramCellStyle.Render(fmt.Sprintf("%d", v.RAM)) + "M" + cpu := cpuCellStyle.Render(fmt.Sprintf("%d", v.CPUs)) + "c" + label := name + " " + mode + " " + ram + " " + cpu + " " if selected { label = selStyle.Render(label) } @@ -123,10 +147,13 @@ const ( // render and then wraps the port onto its own line the moment // something is actually up. A terminal narrower than this still clamps // (paneAt bounds to the window); that is unavoidable at that size. - listWidth = 60 - listVisibleRows = 6 - listMinRows = 2 - listRowsHeadroom = 14 // banner, pane frame, status, footer + listWidth = 60 + listVisibleRows = 6 + listMinRows = 2 + // banner, pane frame, search line, status line, footer. The search and + // status lines are always-present slots (see viewList), so they cost a + // line whether or not either has anything to show. + listRowsHeadroom = 16 ) // newVMList builds the list component with stoat's styling: the component's