From 1b928dd8cd3333018dd9b4ee49e2eb090243ee5a Mon Sep 17 00:00:00 2001 From: zaidejjo Date: Sun, 18 Oct 2026 03:29:46 +0300 Subject: [PATCH] fix(tui): stop rows wrapping and shearing the layout The TUI wrapped rows whenever a repository supplied long paths, long branches or long subjects, and again whenever the theme changed. Every cause was structural, so each one is fixed where it starts instead of being papered over in View(): - Panel.View rendered Width(p.Width) while lipgloss draws the border outside Width, so every panel was two cells wider than the layout had reserved for it. Width is now the inner width, and the title and every body line are capped to it. - The header never adapted to the terminal: it measured ~170 cells on a 60-column terminal. It is now a density ladder (counts, then short labels, then drop the meta block, then padding-free tabs) that renders the first strip which fits. - The AI sidebar drew panelW+4 cells and sliced its input row by byte offset. It sizes itself from the inner width, keeps the tail of its input rune-wise, and clamps every row. - Dialogs were pinned at a stale huh form width, so the form burst out of its border. views/dialog.go now owns the budget (border, padding, form field frame) and every dialog clamps its body to it. - Rows went through per-view selection loops that sliced styled strings by byte or rune index. components.RowLine is now the single path: it charges the style frame, truncates with the shared width law, then draws the cursor. - components.CutLeft/TruncateLeft replace ansi.TruncateLeft, which cannot cut into a wide grapheme: asking it to drop one cell in front of a 2-cell character returns the string untouched, so a caller looping until the tail fits never terminates. - The modal compositor places overlays by cell column, not rune index, so a CJK or emoji background no longer shifts them. - Light theme: diff backgrounds are tokens instead of dark literals, and no style carries a hardcoded Width(80) any more. View() still clamps each line as a last line of defence, but the new tests assert that the raw header + content layout fits at eight widths, nine screens and both themes without it. --- pkg/tui/app.go | 273 +++++++++++++++++------------- pkg/tui/app_header_test.go | 59 ++++++- pkg/tui/components/panel.go | 31 ++-- pkg/tui/components/statusbar.go | 24 +-- pkg/tui/components/zlist.go | 145 ++++++++++++++-- pkg/tui/components/zlist_test.go | 215 ++++++++++++++++++++++- pkg/tui/frame_test.go | 282 +++++++++++++++++++++++++++++++ pkg/tui/overlay_test.go | 62 +++++++ pkg/tui/styles/colors.go | 9 + pkg/tui/styles/theme.go | 11 +- pkg/tui/styles/theme_test.go | 88 ++++++++++ pkg/tui/views/actions.go | 6 +- pkg/tui/views/ai_sidebar.go | 35 +++- pkg/tui/views/commit.go | 27 +-- pkg/tui/views/config.go | 43 +++-- pkg/tui/views/dialog.go | 91 ++++++++++ pkg/tui/views/log.go | 35 +--- pkg/tui/views/newviews_test.go | 18 -- pkg/tui/views/notifications.go | 11 +- pkg/tui/views/palette.go | 31 ++-- pkg/tui/views/pr_create.go | 10 +- pkg/tui/views/pr_merge.go | 3 +- pkg/tui/views/refs.go | 6 +- pkg/tui/views/status.go | 34 +--- 24 files changed, 1221 insertions(+), 328 deletions(-) create mode 100644 pkg/tui/frame_test.go create mode 100644 pkg/tui/overlay_test.go create mode 100644 pkg/tui/styles/theme_test.go create mode 100644 pkg/tui/views/dialog.go diff --git a/pkg/tui/app.go b/pkg/tui/app.go index ffea147..01e9456 100644 --- a/pkg/tui/app.go +++ b/pkg/tui/app.go @@ -13,6 +13,7 @@ import ( "github.com/charmbracelet/bubbles/viewport" tea "github.com/charmbracelet/bubbletea" "github.com/charmbracelet/lipgloss" + "github.com/charmbracelet/x/ansi" "github.com/muesli/termenv" "github.com/zaidejjo/zgit/pkg/core/config" gitpkg "github.com/zaidejjo/zgit/pkg/core/git" @@ -413,6 +414,23 @@ func (m *Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { // --- View --- // View implements tea.Model. +// clampFrame caps every rendered line to the terminal width. Last line +// of defence: one cell of overflow makes a real terminal wrap the row, +// which shears the whole layout sideways — this makes that impossible +// no matter what a view draws. +func (m *Model) clampFrame(s string) string { + if m.width <= 0 || s == "" { + return s + } + lines := strings.Split(s, "\n") + for i, l := range lines { + if lipgloss.Width(l) > m.width { + lines[i] = components.Truncate(l, m.width) + } + } + return strings.Join(lines, "\n") +} + func (m *Model) View() string { if !m.ready { return styles.LoadingStyle.Render("Loading...") @@ -442,35 +460,36 @@ func (m *Model) View() string { } fullLayout = fullLayout + "\n" + strings.Join(padding, "\n") } + fullLayout = m.clampFrame(fullLayout) // Floating modal overlays — centered on top of everything, no layout shift switch { case m.mode == modeCommit: overlay := m.commitDlg.View(m.width) if overlay != "" { - return overlayCenter(fullLayout, overlay, m.width, m.height) + return m.clampFrame(overlayCenter(fullLayout, overlay, m.width, m.height)) } case m.mode == modeConfig: overlay := m.configDlg.View(m.width) if overlay != "" { - return overlayCenter(fullLayout, overlay, m.width, m.height) + return m.clampFrame(overlayCenter(fullLayout, overlay, m.width, m.height)) } case m.mode == modePRCreate: overlay := m.prCreateDlg.View(m.width) if overlay != "" { - return overlayCenter(fullLayout, overlay, m.width, m.height) + return m.clampFrame(overlayCenter(fullLayout, overlay, m.width, m.height)) } case m.mode == modePRMerge: overlay := m.prMergeDlg.View(m.width) if overlay != "" { - return overlayCenter(fullLayout, overlay, m.width, m.height) + return m.clampFrame(overlayCenter(fullLayout, overlay, m.width, m.height)) } } if m.palette.Active() { overlay := m.palette.View(m.width) if overlay != "" { - return overlayCenter(fullLayout, overlay, m.width, m.height) + return m.clampFrame(overlayCenter(fullLayout, overlay, m.width, m.height)) } } @@ -480,74 +499,139 @@ func (m *Model) View() string { // renderHeader builds the Dale-style top tab bar: // [zgit] Status Log Branches │ 3 PRs 4 Issues │ @user main vX.Y.Z // The active view (focused panel or fullscreen) is highlighted. -func (m *Model) renderHeader() string { - tab := func(label string, active bool) string { - if active { - return styles.ActiveTabStyle.Render(label) - } - return styles.InactiveTabStyle.Render(label) - } - - var tabs []string - tabs = append(tabs, styles.HeaderLogoStyle.Render(" zgit ")) - panelActive := m.fullScreenView == shell.Panels - tabs = append(tabs, tab("Status", panelActive && m.focusedPanel == PanelStatus)) - tabs = append(tabs, tab("Log", panelActive && m.focusedPanel == PanelLog)) - tabs = append(tabs, tab("Branches", panelActive && m.focusedPanel == PanelBranch)) - compact := m.width > 0 && m.width < 110 - for _, t := range shell.Tabs { - label := t.Full() - if compact { - label = t.Short() - } - if !compact { - switch t.ID { - case shell.PRs: - if m.prs.Items != nil { - label = fmt.Sprintf("%s (%d)", label, len(m.prs.Items)) - } - case shell.Issues: - if m.issues.Items != nil { - label = fmt.Sprintf("%s (%d)", label, len(m.issues.Items)) - } - case shell.Dashboard: - if m.dash.Loaded { - label = fmt.Sprintf("%s (%d)", label, len(m.dash.Items)) - } - case shell.Actions: - if m.actions.Items != nil { - label = fmt.Sprintf("%s (%d)", label, len(m.actions.Items)) - } - case shell.Worktrees: - if m.worktrees.Loaded { - label = fmt.Sprintf("%s (%d)", label, len(m.worktrees.Items)) - } - case shell.Stash: - if m.stash.Loaded { - label = fmt.Sprintf("%s (%d)", label, len(m.stash.Items)) - } - } +// tabCount reports the badge count for a screen tab, or 0 when the screen +// has no data yet. +func (m *Model) tabCount(id shell.ScreenID) int { + switch id { + case shell.PRs: + return len(m.prs.Items) + case shell.Issues: + return len(m.issues.Items) + case shell.Dashboard: + if m.dash.Loaded { + return len(m.dash.Items) + } + case shell.Actions: + return len(m.actions.Items) + case shell.Worktrees: + if m.worktrees.Loaded { + return len(m.worktrees.Items) + } + case shell.Stash: + if m.stash.Loaded { + return len(m.stash.Items) } - if t.ID == shell.Notes && m.notes.Loaded && m.notes.UnreadCount() > 0 { - label = fmt.Sprintf("%s (%d)", label, m.notes.UnreadCount()) + case shell.Notes: + if m.notes.Loaded { + return m.notes.UnreadCount() } - tabs = append(tabs, tab(label, m.fullScreenView == t.ID)) } - left := lipgloss.JoinHorizontal(lipgloss.Center, tabs...) + return 0 +} + +// renderHeader builds the Dale-style top tab bar: +// [zgit] Status Log Branches │ 3 PRs 4 Issues │ @user main vX.Y.Z +// +// The strip is assembled by density, not by hope: on a narrow terminal +// labels shorten, then counts go, then the right-hand meta, and finally +// the tab padding — so the header always fits the terminal instead of +// wrapping the whole layout. +type headerDensity struct { + short bool // abbreviated labels + counts bool // "PRs (3)" badges + meta bool // user / branch / version on the right + tight bool // padding-free tabs (very narrow terminals) +} - var meta []string - if m.ghUser != "" { - meta = append(meta, styles.HeaderMetaStyle.Render("@"+m.ghUser)) +func (m *Model) renderHeader() string { + width := m.width + if width <= 0 { + width = 120 } + branch := m.currentBranch() - if branch != "" { - meta = append(meta, styles.HeaderBranchStyle.Render(branch)) + build := func(d headerDensity) (string, string) { + tab := func(label string, active bool) string { + st := styles.InactiveTabStyle + if active { + st = styles.ActiveTabStyle + } + if d.tight { + st = st.Padding(0) + } + return st.Render(label) + } + panel := func(long, short string, active bool) string { + if d.short { + return tab(short, active) + } + return tab(long, active) + } + panelActive := m.fullScreenView == shell.Panels + + logo := styles.HeaderLogoStyle.Render(" zgit ") + if d.tight { + logo = styles.HeaderLogoStyle.Padding(0).Render("zgit") + } + + tabs := []string{logo} + tabs = append(tabs, + panel("Status", "S", panelActive && m.focusedPanel == PanelStatus), + panel("Log", "L", panelActive && m.focusedPanel == PanelLog), + panel("Branches", "B", panelActive && m.focusedPanel == PanelBranch), + ) + for _, t := range shell.Tabs { + label := t.Full() + if d.short { + label = t.Short() + } + if n := m.tabCount(t.ID); n > 0 && d.counts { + label = fmt.Sprintf("%s (%d)", label, n) + } + tabs = append(tabs, tab(label, m.fullScreenView == t.ID)) + } + left := lipgloss.JoinHorizontal(lipgloss.Center, tabs...) + if !d.meta { + return left, "" + } + + var meta []string + if m.ghUser != "" { + meta = append(meta, styles.HeaderMetaStyle.Render("@"+m.ghUser)) + } + if branch != "" { + shown := branch + if d.short { + shown = components.Truncate(branch, 18) + } + meta = append(meta, styles.HeaderBranchStyle.Render(shown)) + } + meta = append(meta, styles.HeaderMetaStyle.Render("v"+AppVersion)) + right := lipgloss.JoinHorizontal(lipgloss.Center, meta...) + return left, right + } + + // Densities in order of decreasing detail; the first that fits wins. + densities := []headerDensity{ + {counts: true, meta: true}, + {short: true, counts: true, meta: true}, + {short: true, meta: true}, + {short: true}, + {short: true, tight: true}, + } + left, right := "", "" + for _, d := range densities { + l, r := build(d) + left, right = l, r + if lipgloss.Width(l)+lipgloss.Width(r) <= width { + break + } } - meta = append(meta, styles.HeaderMetaStyle.Render("v"+AppVersion)) - right := lipgloss.JoinHorizontal(lipgloss.Center, meta...) - // Pad the middle so meta hugs the right edge; truncate tabs on narrow terms. - gap := m.width - lipgloss.Width(left) - lipgloss.Width(right) + // Pad the gap so meta hugs the right edge. When even the leanest + // header is too wide (absurdly narrow terminal), the frame guard in + // View() clips it rather than letting the terminal wrap. + gap := width - lipgloss.Width(left) - lipgloss.Width(right) if gap < 1 { gap = 1 } @@ -2640,39 +2724,20 @@ func overlayCenter(base, modal string, termWidth, termHeight int) string { result := make([]string, len(baseLines)) for i, line := range baseLines { if i >= startY && i < startY+modalH && i < len(baseLines) { - // Strip ANSI from base line so slicing never cuts escape codes - plain := stripANSI(line) - plainRunes := []rune(plain) - - // Pad runes to termWidth so left/right slicing is safe (rune index = visual column for ASCII/narrow chars) - if len(plainRunes) < termWidth { - padded := make([]rune, termWidth) - for i := 0; i < termWidth; i++ { - if i < len(plainRunes) { - padded[i] = plainRunes[i] - } else { - padded[i] = ' ' - } - } - plainRunes = padded - } - modalLine := modalLines[i-startY] modalWidth := lipgloss.Width(modalLine) - // Left part (base content before modal) — rune-indexed to avoid multibyte slicing errors - left := string(plainRunes[:startX]) + // The base line is styled; take its visible text and cut it by + // *cells* (ansi.Truncate/CutLeft are grapheme- and wide-char + // aware, unlike rune indices which shift the overlay whenever + // the background contains CJK or emoji). + plain := ansi.Strip(line) + left := ansi.Truncate(plain, startX, "") + right := components.CutLeft(plain, startX+modalWidth) - // Right part (base content after modal) - rightEnd := startX + modalWidth - if rightEnd < len(plainRunes) { - right := string(plainRunes[rightEnd:]) - result[i] = left + modalLine + right - } else { - result[i] = left + modalLine - } + result[i] = left + modalLine + right - // Ensure result fills termWidth (modal may be narrower) + // Ensure the composited row still fills the terminal. if w := lipgloss.Width(result[i]); w < termWidth { result[i] += strings.Repeat(" ", termWidth-w) } @@ -2684,28 +2749,6 @@ func overlayCenter(base, modal string, termWidth, termHeight int) string { return strings.Join(result, "\n") } -// stripANSI removes ANSI escape sequences from a string. -// ANSI sequences start with \x1b (ESC) followed by '[', parameters, and a letter (a-z, A-Z). -func stripANSI(s string) string { - var out strings.Builder - inEscape := false - for _, r := range s { - if inEscape { - // ANSI escape sequence ends on any ASCII letter (a-z, A-Z), tilde, or @ - if (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || r == '~' || r == '@' { - inEscape = false - } - continue - } - if r == '\x1b' { - inEscape = true - continue - } - out.WriteRune(r) - } - return out.String() -} - // listenForMessages creates a command that polls the message channel. func listenForMessages(msgs <-chan teaMsg) tea.Cmd { return func() tea.Msg { diff --git a/pkg/tui/app_header_test.go b/pkg/tui/app_header_test.go index 334b126..17369c8 100644 --- a/pkg/tui/app_header_test.go +++ b/pkg/tui/app_header_test.go @@ -4,21 +4,32 @@ import ( "strings" "testing" + "github.com/charmbracelet/lipgloss" gitpkg "github.com/zaidejjo/zgit/pkg/core/git" "github.com/zaidejjo/zgit/pkg/core/models" "github.com/zaidejjo/zgit/pkg/tui/shell" + "github.com/zaidejjo/zgit/pkg/tui/styles" ) -func TestRenderHeader(t *testing.T) { +func newHeaderModel(t *testing.T, width int, user string) *Model { + t.Helper() gitExec, err := gitpkg.NewNativeExec("") if err != nil { t.Skip("git not available") } m := NewModel(gitExec) - m.width = 120 + m.ready = true + m.width = width m.status.Status = &models.Status{Branch: "dev"} - m.ghUser = "octo" + m.ghUser = user AppVersion = "9.9.9-test" + return m +} + +// TestRenderHeaderWide checks the full-detail strip on a terminal with +// room for everything. +func TestRenderHeaderWide(t *testing.T) { + m := newHeaderModel(t, 200, "octo") out := m.renderHeader() for _, want := range []string{"zgit", "Status", "Log", "Branches", "PRs", "Issues", "@octo", "dev", "v9.9.9-test"} { @@ -26,6 +37,9 @@ func TestRenderHeader(t *testing.T) { t.Errorf("header missing %q in %q", want, out) } } + if w := lipgloss.Width(out); w > 200 { + t.Errorf("header is %d cells wide, limit 200", w) + } // Fullscreen PR tab highlights. m.fullScreenView = shell.PRs @@ -35,6 +49,45 @@ func TestRenderHeader(t *testing.T) { } } +// TestRenderHeaderAdapts is the invariant that matters: whatever the +// terminal width, the header fits it. Detail is dropped instead of +// wrapping the layout. +func TestRenderHeaderAdapts(t *testing.T) { + for _, width := range []int{40, 60, 80, 100, 120, 149, 200} { + m := newHeaderModel(t, width, "octocat") + out := m.renderHeader() + if w := lipgloss.Width(out); w > width { + t.Errorf("width %d: header is %d cells (%q)", width, w, out) + } + if strings.Contains(out, "\n") { + t.Errorf("width %d: header must be a single line", width) + } + // The logo and the panel tabs are the last things to go. + if !strings.Contains(out, "zgit") { + t.Errorf("width %d: header lost the logo: %q", width, out) + } + } +} + +// TestRenderHeaderNarrowShrinks verifies the density ladder actually +// degrades rather than clipping mid-tab. +func TestRenderHeaderNarrowShrinks(t *testing.T) { + m := newHeaderModel(t, 200, "octocat") + wide := m.renderHeader() + + m.width = 60 + narrow := m.renderHeader() + + if lipgloss.Width(narrow) >= lipgloss.Width(wide) { + t.Fatalf("narrow header (%d) should be shorter than wide (%d)", lipgloss.Width(narrow), lipgloss.Width(wide)) + } + if w := lipgloss.Width(narrow); w > 60 { + t.Fatalf("narrow header is %d cells, limit 60: %q", w, narrow) + } + styles.SetDark() + styles.Refresh() +} + func TestConfirmTwice(t *testing.T) { m := &Model{} if m.confirmTwice("del:x") { diff --git a/pkg/tui/components/panel.go b/pkg/tui/components/panel.go index 5025ecc..747f589 100644 --- a/pkg/tui/components/panel.go +++ b/pkg/tui/components/panel.go @@ -52,29 +52,16 @@ func (p Panel) View() string { Foreground(borderColor). Bold(p.Focused). Render(titleStr) + titleLine = Truncate(titleLine, innerW) - // Body content — trim each line to inner width + // Body content — every line is hard-capped to the inner width so a + // long path/branch/message can never widen the panel (which would + // wrap the whole layout). lines := strings.Split(p.Content, "\n") - var bodyLines []string + bodyLines := make([]string, 0, len(lines)) for _, line := range lines { - trimmed := line - // Use visual width (lipgloss.Width) not byte length (len). - // len() overcounts because: - // - Multi-byte Unicode (● = 3 bytes, 1 visual col) - // - lipgloss ANSI codes (only present in TUI context) - // → byte check falsely triggers truncation on fitting lines. - // → byte-slicing breaks ANSI sequences and multi-byte chars. - // - // Visual width always ≤ innerW because views calculate content - // to fit. This check is a safety net — should never trigger. - if lipgloss.Width(trimmed) > innerW { - // Instead of byte-level truncation (corrupts ANSI/Unicode) - // or Style.Width (wraps lines with newlines), we leave - // overflow lines intact. This is vanishingly rare since - // views already bound content width. - } - bodyLines = append(bodyLines, trimmed) + bodyLines = append(bodyLines, Truncate(line, innerW)) } // If Height is set, pad or truncate content to exact height @@ -102,10 +89,12 @@ func (p Panel) View() string { content := titleLine + "\n" + strings.Join(bodyLines, "\n") - // Apply border + // Apply border. Width(innerW) makes the *content* innerW wide; the + // border is drawn outside it, so the panel occupies exactly p.Width + // columns — matching the width callers reserve for it. return lipgloss.NewStyle(). Border(p.BorderType). BorderForeground(borderColor). - Width(p.Width). + Width(innerW). Render(content) } diff --git a/pkg/tui/components/statusbar.go b/pkg/tui/components/statusbar.go index 3e43f72..7d83b32 100644 --- a/pkg/tui/components/statusbar.go +++ b/pkg/tui/components/statusbar.go @@ -3,6 +3,7 @@ package components import ( "fmt" + "github.com/charmbracelet/lipgloss" "github.com/zaidejjo/zgit/pkg/tui/styles" ) @@ -54,15 +55,10 @@ func (s StatusBar) View() string { } } - // Repository path (truncated on left if too long) + // Repository path (truncated on the left so the leaf stays visible) repoPath := "" if s.Data.RepoPath != "" { - repoShort := s.Data.RepoPath - maxPathLen := 40 - if len(repoShort) > maxPathLen { - // Truncate from left to fit - repoShort = "…" + repoShort[len(repoShort)-maxPathLen+1:] - } + repoShort := TruncateLeft(s.Data.RepoPath, 40) repoPath = styles.StatusBarInfoStyle.Render(" " + repoShort + " ") } @@ -92,10 +88,18 @@ func (s StatusBar) View() string { if s.Data.GhDetected { right += styles.StatusBarInfoStyle.Render(fmt.Sprintf(" %s/%s ", s.Data.GhOwner, s.Data.GhRepo)) } - right += styles.StatusBarInfoStyle.Render(" ?help qquit ") - right += styles.HintStyle.Render(" tab 1-4 /filter ^p ") + right += styles.StatusBarInfoStyle.Render(" ?help q:quit ") + + base := left + " " + middle + " " + right + hints := styles.HintStyle.Render(" tab 0-9 /:filter L:theme ") + full := base + if lipgloss.Width(base)+lipgloss.Width(hints) <= s.Width { + full = base + hints + } + // Absolute guard: the bar is one line, always. + full = Truncate(full, s.Width) return styles.StatusBarStyle. Width(s.Width). - Render(left + " " + middle + " " + right) + Render(full) } diff --git a/pkg/tui/components/zlist.go b/pkg/tui/components/zlist.go index 9412b49..ea4ac95 100644 --- a/pkg/tui/components/zlist.go +++ b/pkg/tui/components/zlist.go @@ -1,26 +1,143 @@ package components import ( - "regexp" "strings" + "github.com/charmbracelet/x/ansi" "github.com/zaidejjo/zgit/pkg/tui/styles" ) -// ansiEscape matches SGR/control sequences for width-safe reuse. -var ansiEscape = regexp.MustCompile("\x1b\\[[0-9;]*[a-zA-Z]") +// Truncate hard-caps a rendered line to width cells, ANSI- and +// wide-char aware. Final guardrail: no row may ever wrap the panel. +func Truncate(s string, width int) string { + if width <= 0 { + return "" + } + return ansi.Truncate(s, width, "…") +} -// CursorLine rebuilds a rendered line for the selected row: ANSI is -// stripped first (never slice styled strings by byte/rune — that -// decapitates escapes into visible "[38;2;…" garbage), the first -// visible rune becomes ❯, and the caller wraps it in the active style. +// CursorLine retargets the first visible grapheme of a rendered line to ❯ +// without touching the rest: ANSI- and grapheme-aware, so escapes and wide +// characters survive (never slice styled strings by byte/rune — that +// decapitates escapes into visible "[38;2;…" garbage). +// +// The whole first grapheme is replaced, never a single cell: cutting one +// cell out of a 2-cell character would leave it in place and *grow* the +// row by a cell, which wraps the panel. func CursorLine(line string) string { - plain := ansiEscape.ReplaceAllString(line, "") - runes := []rune(plain) - if len(runes) == 0 { + if ansi.StringWidth(line) <= 1 { + return "❯" + } + + // Walk to the first grapheme that occupies cells. Leading escape + // sequences carry none, but they set the colour of the text after + // them, so they are kept in front of the cursor. + var ( + state byte + offset int + leading strings.Builder + ) + cw := 0 + for offset < len(line) { + unit, w, n, newState := ansi.DecodeSequence(line[offset:], state, nil) + if n <= 0 { + break + } + state = newState + offset += n + if w == 0 { + leading.WriteString(unit) + continue + } + cw = w + break + } + if cw <= 0 { return "❯" } - return "❯" + string(runes[1:]) + if cw >= ansi.StringWidth(line) { + // The first grapheme is the whole line: the cursor alone fits. + return leading.String() + "❯" + } + return leading.String() + "❯" + line[offset:] +} + +// CutLeft drops the first cells cells of a rendered line and returns the +// rest, keeping escape sequences and never splitting a grapheme: a wide +// character straddling the cut is dropped whole, so the result can be one +// cell narrower than asked for, never wider. +// +// ansi.TruncateLeft cannot express this. It refuses to cut *into* a +// grapheme, so asking it to drop one cell in front of a 2-cell character +// hands the string straight back — a caller looping until the tail fits +// would never finish. +func CutLeft(s string, cells int) string { + if cells <= 0 { + return s + } + var ( + state byte + offset int + dropped int + leading strings.Builder + ) + for offset < len(s) && dropped < cells { + unit, w, n, newState := ansi.DecodeSequence(s[offset:], state, nil) + if n <= 0 { + break + } + state = newState + if w == 0 { + // Zero-width units cost no cells, so they ride along with the + // tail and keep its styling. + leading.WriteString(unit) + offset += n + continue + } + offset += n + dropped += w + } + if offset >= len(s) { + return "" + } + return leading.String() + s[offset:] +} + +// TruncateLeft keeps the tail of a line and prefixes an ellipsis, so the +// end of a long path (the part that identifies it) stays on screen. +func TruncateLeft(s string, width int) string { + if width <= 0 { + return "" + } + w := ansi.StringWidth(s) + if w <= width { + return s + } + // One cell of the budget goes to the ellipsis. + return "…" + CutLeft(s, w-width+1) +} + +// RowLine renders one list row as exactly width cells. It reserves the +// frame its style adds (padding/margin/border), hard-caps the text to +// what is left, then draws the cursor when selected. Every row goes +// through here, so a row can never grow wider than its panel and make +// the terminal wrap the whole layout. +func RowLine(line string, width int, selected bool) string { + st := styles.ListItemStyle + if selected { + st = styles.ListItemActiveStyle + } + inner := width - st.GetHorizontalFrameSize() + if inner < 1 { + inner = 1 + } + body := Truncate(line, inner) + if selected { + // A wide first grapheme can make the cursor swap return one cell + // more than it consumed, so cap again: exactly is the contract. + body = Truncate(CursorLine(body), inner) + } + return st.Render(body) } // ZList is the single filterable, scrollable list for every screen. @@ -182,11 +299,7 @@ func (l *ZList[T]) ViewRows(width int) string { if l.Render != nil { line = l.Render(item, width) } - if l.Offset+i == l.Cursor { - b.WriteString(styles.ListItemActiveStyle.Render(CursorLine(line))) - } else { - b.WriteString(styles.ListItemStyle.Render(line)) - } + b.WriteString(RowLine(line, width, l.Offset+i == l.Cursor)) b.WriteString("\n") } return b.String() diff --git a/pkg/tui/components/zlist_test.go b/pkg/tui/components/zlist_test.go index 59f30b0..cdbb14e 100644 --- a/pkg/tui/components/zlist_test.go +++ b/pkg/tui/components/zlist_test.go @@ -1,11 +1,20 @@ package components import ( + "regexp" "strings" "testing" "time" + + "github.com/charmbracelet/lipgloss" + "github.com/zaidejjo/zgit/pkg/tui/styles" ) +// stripANSI removes SGR sequences so tests can assert on visible text. +func stripANSI(s string) string { + return regexp.MustCompile("\x1b\\[[0-9;]*[a-zA-Z]").ReplaceAllString(s, "") +} + func testList() ZList[string] { l := ZList[string]{Height: 3} l.Items = []string{"dev", "feat/a", "feat/b", "main", "fix/c"} @@ -97,15 +106,6 @@ func TestZListSelectAndRows(t *testing.T) { } } -func TestCursorLine(t *testing.T) { - if got := CursorLine("\x1b[31m●\x1b[0m hi"); got != "❯ hi" { - t.Fatalf("= %q", got) - } - if CursorLine("") != "❯" { - t.Fatalf("empty") - } -} - func TestToastsPushExpire(t *testing.T) { tt := NewToasts(time.Hour) if !tt.Empty() { @@ -131,3 +131,200 @@ func TestToastsPushExpire(t *testing.T) { t.Fatalf("should be empty") } } + +func TestTruncateWidth(t *testing.T) { + styled := "\x1b[32m●\x1b[0m hello world, this is long" + got := Truncate(styled, 10) + if w := lipgloss.Width(got); w > 10 { + t.Fatalf("width %d > 10: %q", w, got) + } + // Escapes must be complete (no half-written "\x1b[3" left behind). + if dangling := regexp.MustCompile("\x1b\\[[0-9;]*$").FindString(got); dangling != "" { + t.Fatalf("truncation split an escape: %q", got) + } + if plain := stripANSI(got); plain != "● hello w…" { + t.Fatalf("plain = %q", plain) + } + if Truncate("", 10) != "" || Truncate("ab", 0) != "" { + t.Fatalf("edge cases") + } + // Wide chars count double. + wide := Truncate("日本語テストX", 6) + if lipgloss.Width(wide) > 6 { + t.Fatalf("wide overflow: %q", wide) + } +} + +func TestCursorLine(t *testing.T) { + styled := "\x1b[38;2;166;227;161m◆\x1b[0m 3fec75a msg" + got := CursorLine(styled) + if plain := stripANSI(got); plain != "❯ 3fec75a msg" { + t.Fatalf("plain = %q", plain) + } + // Styling is preserved (that is the point of the ANSI-aware swap). + if !strings.Contains(got, "\x1b[38;2;166;227;161m") { + t.Fatalf("color dropped: %q", got) + } + if CursorLine("") != "❯" || CursorLine("x") != "❯" { + t.Fatalf("short lines need a bare cursor") + } + if plain := stripANSI(CursorLine("◆ abc")); plain != "❯ abc" { + t.Fatalf("multibyte = %q", plain) + } +} + +func TestRowLineReservesStyleFrame(t *testing.T) { + // A row style that pads must not push the row past the panel: the + // padding is charged against the width, not added on top of it. + styles.ListItemActiveStyle = lipgloss.NewStyle().PaddingLeft(1) + defer styles.Refresh() + for _, selected := range []bool{false, true} { + got := RowLine(" ● some/long/path/file.go", 20, selected) + if w := lipgloss.Width(got); w > 20 { + t.Fatalf("selected=%v: row is %d cells, want <= 20 (%q)", selected, w, got) + } + } +} + +// TestCursorLineWideFirstGrapheme covers the case that used to overflow by +// one cell: a wide (2-cell) first grapheme. +func TestCursorLineWideFirstGrapheme(t *testing.T) { + got := CursorLine("日本語 abc") + if w := lipgloss.Width(got); w > lipgloss.Width("日本語 abc") { + t.Fatalf("cursor swap grew the line: %q (%d cells)", got, w) + } + if !strings.Contains(got, "abc") { + t.Fatalf("tail lost: %q", got) + } +} + +// TestRowLineNeverExceedsWidth is the hard contract for every rendered row, +// including wide characters and a styled first grapheme. +func TestRowLineNeverExceedsWidth(t *testing.T) { + cases := []struct { + line string + width int + selected bool + }{ + {" ● services/platform/" + strings.Repeat("deep/", 20) + "file.go", 40, true}, + {" ● services/platform/" + strings.Repeat("deep/", 20) + "file.go", 40, false}, + {"日本語のとても長いファイル名.go", 12, true}, + {strings.Repeat("🎉", 30), 20, true}, + {"", 10, true}, + {"x", 1, true}, + {"a", 0, false}, + } + for _, c := range cases { + got := RowLine(c.line, c.width, c.selected) + if c.width <= 0 { + continue + } + if w := lipgloss.Width(got); w > c.width { + t.Fatalf("RowLine(%q, %d, %v) = %d cells: %q", c.line, c.width, c.selected, w, got) + } + } +} + +func TestTruncateLeftKeepsTail(t *testing.T) { + if got := TruncateLeft("short", 10); got != "short" { + t.Fatalf("fits: %q", got) + } + if got := TruncateLeft("", 5); got != "" { + t.Fatalf("empty: %q", got) + } + got := TruncateLeft("/home/someone/Projects/Apps/zgit", 12) + if !strings.HasPrefix(got, "…") || !strings.HasSuffix(got, "zgit") { + t.Fatalf("tail lost: %q", got) + } + if w := lipgloss.Width(got); w > 12 { + t.Fatalf("width %d > 12: %q", w, got) + } + // Multi-byte runes must survive whole. A 2-cell first grapheme is the + // case ansi.TruncateLeft cannot cut into — asking it for one cell used + // to return the string untouched, so a "shave until it fits" loop spun + // forever. This test hanging is that bug. + wide := TruncateLeft("日本語のとても長いパス/ファイル", 8) + if w := lipgloss.Width(wide); w > 8 { + t.Fatalf("wide width %d: %q", w, wide) + } + if strings.ContainsRune(wide, '\uFFFD') { + t.Fatalf("split a rune: %q", wide) + } + // Every width from 1 to the full length must return in bounded time + // and stay inside the budget. + long := "日本語のとても長いパス/ファイル" + for w := 1; w <= lipgloss.Width(long); w++ { + got := TruncateLeft(long, w) + if cw := lipgloss.Width(got); cw > w { + t.Fatalf("width %d produced %d cells: %q", w, cw, got) + } + if strings.ContainsRune(got, '\uFFFD') { + t.Fatalf("width %d split a rune: %q", w, got) + } + } + // Zero-width budget is empty, not a lone ellipsis. + if got := TruncateLeft("abcdef", 0); got != "" { + t.Fatalf("zero width: %q", got) + } +} + +func TestCutLeftDropsWholeGraphemes(t *testing.T) { + if got := CutLeft("abcdef", 0); got != "abcdef" { + t.Fatalf("no cut: %q", got) + } + if got := CutLeft("abcdef", 2); got != "cdef" { + t.Fatalf("ascii cut: %q", got) + } + if got := CutLeft("abc", 99); got != "" { + t.Fatalf("over-cut: %q", got) + } + // A wide character straddling the cut is dropped whole: the result may + // be narrower than requested, never wider. + for cells := 1; cells <= 8; cells++ { + got := CutLeft("日本語abc", cells) + if w := lipgloss.Width(got); w > lipgloss.Width("日本語abc")-cells { + t.Fatalf("cut %d kept %d cells: %q", cells, w, got) + } + if strings.ContainsRune(got, '\uFFFD') { + t.Fatalf("cut %d split a rune: %q", cells, got) + } + } + // Styling that belongs to the kept text survives the cut. + styled := "\x1b[31mabcdef\x1b[0m" + got := CutLeft(styled, 2) + if plain := stripANSI(got); plain != "cdef" { + t.Fatalf("styled cut = %q", plain) + } + if !strings.Contains(got, "\x1b[31m") { + t.Fatalf("colour dropped: %q", got) + } +} + +// TestCursorLineKeepsLeadingEscapes pins the styled case: the escapes in +// front of the replaced grapheme must survive (the row keeps its colour), +// and the swap must never widen the line. +func TestCursorLineKeepsLeadingEscapes(t *testing.T) { + const esc = "\x1b[38;2;166;227;161m" + cases := []string{ + esc + "日本 abc", + esc + "◆ 3fec75a msg", + "日本語 abc", + } + for _, line := range cases { + got := CursorLine(line) + if w, max := lipgloss.Width(got), lipgloss.Width(line); w > max { + t.Fatalf("CursorLine(%q) = %d cells > %d: %q", line, w, max, got) + } + if strings.HasPrefix(line, esc) && !strings.Contains(got, esc) { + t.Fatalf("CursorLine(%q) dropped styling: %q", line, got) + } + if plain := stripANSI(got); !strings.HasPrefix(plain, "❯") { + t.Fatalf("CursorLine(%q) = %q, want cursor first", line, plain) + } + } + // A one-cell line is *entirely* the cursor, so the styling it carried + // is gone with the replaced text — the cursor lands in the row style. + if plain := stripANSI(CursorLine(esc + "x")); plain != "❯" { + t.Fatalf("one-cell line = %q, want a bare cursor", plain) + } +} diff --git a/pkg/tui/frame_test.go b/pkg/tui/frame_test.go new file mode 100644 index 0000000..729ffd4 --- /dev/null +++ b/pkg/tui/frame_test.go @@ -0,0 +1,282 @@ +package tui + +import ( + "strings" + "testing" + "time" + + "github.com/charmbracelet/bubbletea" + "github.com/charmbracelet/lipgloss" + gitpkg "github.com/zaidejjo/zgit/pkg/core/git" + "github.com/zaidejjo/zgit/pkg/core/models" + "github.com/zaidejjo/zgit/pkg/tui/components" + "github.com/zaidejjo/zgit/pkg/tui/shell" + "github.com/zaidejjo/zgit/pkg/tui/styles" + "github.com/zaidejjo/zgit/pkg/tui/views" +) + +// longPath is deliberately absurd: a deep monorepo path plus a long name +// is exactly what wrapped the UI on a big repository. +const longPath = "services/platform/internal/very/deep/nested/directory/structure/with_a_really_long_filename_that_never_ends.go" + +func frameTestModel(t *testing.T, width, height int) *Model { + t.Helper() + gitExec, err := gitpkg.NewNativeExec("") + if err != nil { + t.Skip("git not available") + } + m := NewModel(gitExec) + m.ready = true + + // Status: staged + unstaged + untracked, all with overlong paths. + m.status.UpdateStatus(&models.Status{ + Branch: "feature/extremely-long-branch-name-that-keeps-going-and-going", + Upstream: "origin/feature/extremely-long-branch-name-that-keeps-going-and-going", + Ahead: 12, + Behind: 7, + StagedCount: 2, + UnstagedCount: 1, + UntrackedCount: 1, + Files: []models.FileStatus{ + {Path: longPath, Staged: models.StatusModified}, + {Path: "docs/" + longPath, Staged: models.StatusAdded, Unstaged: models.StatusModified}, + {Path: "tmp/" + longPath, Staged: models.StatusUnmodified, Unstaged: models.StatusModified}, + {Path: "vendor/" + longPath, Staged: models.StatusUntracked, Unstaged: models.StatusUntracked}, + }, + }) + + // Log: lane-heavy graph + long subject + many ref badges. + commits := make([]*models.Commit, 0, 12) + for i := 0; i < 12; i++ { + commits = append(commits, &models.Commit{ + Hash: "abcdef0123456789", + Parents: []string{"p1", "p2"}, + Message: "fix(tui): a commit subject that is far too long for any panel width and will therefore need truncating " + strings.Repeat("x", 40), + Author: "octocat", + RefNames: "HEAD -> feature/extremely-long-branch-name, origin/feature/extremely-long-branch-name, tag:v9.9.9-rc1", + // Stagger so the graph renderer draws several lanes. + Timestamp: time.Now().Add(-time.Duration(i) * time.Minute), + }) + } + m.log.UpdateLog(commits) + + m.branches.UpdateBranches([]*models.Branch{ + {Name: "feature/extremely-long-branch-name-that-keeps-going", Upstream: "origin/feature/extremely-long-branch-name-that-keeps-going", IsHead: true, Ahead: 3, LatestMsg: strings.Repeat("long message ", 8)}, + {Name: "main", Upstream: "origin/main"}, + }) + + m.prs.UpdatePRs([]*models.PullRequestSummary{ + {Number: 1234, Title: strings.Repeat("a very long pull request title ", 5), Author: "octocat", BaseRef: "dev", HeadRef: "feature/extremely-long-branch-name", ReviewState: "CHANGES_REQUESTED", Labels: []string{"needs-review", "area:tui"}}, + }) + m.issues.UpdateIssues([]*models.Issue{ + {Number: 4321, Title: strings.Repeat("an equally long issue title ", 5), Author: "octocat", Labels: []models.Label{{Name: "bug"}, {Name: "priority:high"}}}, + }) + m.dash.UpdateEntries([]views.DashEntry{ + {Name: "zgit", Path: "/home/someone/Projects/Apps/zgit", Branch: "feature/extremely-long-branch-name", Dirty: 12, Ahead: 4, Behind: 2, Favorite: true, Current: true}, + {Name: strings.Repeat("very-long-repo-name-", 3), Path: "/home/someone/Projects/Apps/" + strings.Repeat("deep/", 6) + "repo", Err: "unreachable"}, + }) + m.actions.UpdateRuns([]*models.WorkflowRun{ + {ID: 1, WorkflowName: strings.Repeat("A very long workflow name ", 3), Branch: "feature/extremely-long-branch-name", HeadSHA: "abcdef0123456789", RunNumber: 9999, Status: "COMPLETED", Conclusion: "SUCCESS"}, + }) + m.notes.UpdateNotes([]*models.Notification{ + {ID: "1", Title: strings.Repeat("long notification title ", 4), Type: "PullRequest", Reason: "review_requested", RepoName: "someone/with-a-long-repository-name", Unread: true}, + }) + m.worktrees.UpdateWorktrees([]*models.Worktree{ + {Path: "/home/someone/Projects/Apps/zgit-worktrees/" + strings.Repeat("feature-branch-", 4), Branch: strings.Repeat("feature/very-long-", 3), Head: "abcdef0123456789"}, + }) + m.stash.UpdateStash([]*models.Stash{ + {Index: 0, Hash: "abcdef0123456789", Message: strings.Repeat("WIP on a very long stash message ", 3)}, + }) + m.refs.UpdateRefs( + []string{"v9.9.9-rc1-with-a-very-long-tag-name", strings.Repeat("release-candidate-", 3)}, + []*models.Remote{{Name: "origin-upstream-with-a-very-long-name", URL: "https://github.com/someone/with-a-really-long-repository-name.git"}}, + ) + + m.width = width + m.height = height + m.calcLayout() + return m +} + +// TestViewNeverWraps is the regression test for the reported defect: on a +// big repository (long paths, long subjects, many lanes) and after a +// theme switch, rows wrapped and sheared the layout. No rendered line may +// ever exceed the terminal width, in any screen, in either theme. +func TestViewNeverWraps(t *testing.T) { + // Breakpoints: compact, standard, wide, plus an odd tail width. + widths := []int{60, 80, 99, 100, 120, 149, 150, 200} + screens := []shell.ScreenID{shell.Panels, shell.PRs, shell.Issues, shell.Dashboard, shell.Actions, shell.Notes, shell.Worktrees, shell.Stash, shell.Refs} + + // One model, reused: NewModel reads config and probes git, so building + // it per size would dominate the test's runtime. + m := frameTestModel(t, 120, 40) + + for _, theme := range []string{"dark", "light"} { + if theme == "light" { + styles.SetLight() + } else { + styles.SetDark() + } + styles.Refresh() + + for _, width := range widths { + for _, height := range []int{24, 40} { + m.width = width + m.height = height + m.calcLayout() + for _, screen := range screens { + m.fullScreenView = screen + out := m.View() + if out == "" { + continue + } + for i, line := range strings.Split(out, "\n") { + if w := lipgloss.Width(line); w > width { + t.Fatalf("theme=%s screen=%v size=%dx%d line %d is %d cells wide (limit %d): %q", + theme, screen, width, height, i, w, width, line) + } + } + } + } + } + } + styles.SetDark() + styles.Refresh() +} + +// TestLayoutFitsWithoutTheSafetyNet renders the header and the content +// column *raw*, without clampFrame in the path. View() clips whatever +// slips through, which would hide a view that is structurally too wide +// (the actual reported defect: rows wrapping and shearing the layout). +func TestLayoutFitsWithoutTheSafetyNet(t *testing.T) { + widths := []int{60, 80, 99, 100, 120, 149, 150, 200} + screens := []shell.ScreenID{shell.Panels, shell.PRs, shell.Issues, shell.Dashboard, shell.Actions, shell.Notes, shell.Worktrees, shell.Stash, shell.Refs} + + m := frameTestModel(t, 120, 40) + + for _, theme := range []string{"dark", "light"} { + if theme == "light" { + styles.SetLight() + } else { + styles.SetDark() + } + styles.Refresh() + + for _, width := range widths { + m.width = width + m.height = 40 + m.calcLayout() + for _, screen := range screens { + m.fullScreenView = screen + raw := m.renderHeader() + "\n" + m.renderContent() + for i, line := range strings.Split(raw, "\n") { + if w := lipgloss.Width(line); w > width { + t.Errorf("theme=%s screen=%v width=%d: header/content line %d is %d cells: %q", + theme, screen, width, i, w, line) + } + } + } + } + } + styles.SetDark() + styles.Refresh() +} + +// TestViewNeverWrapsWithSidebar covers the third column: the AI sidebar +// reserves its own width, so it must not push content off-screen. +func TestViewNeverWrapsWithSidebar(t *testing.T) { + for _, width := range []int{100, 120, 160, 200} { + m := frameTestModel(t, width, 40) + m.aiData.Sidebar.State = views.SBSInput + m.aiData.Sidebar.Messages = []views.ChatMessage{ + {Role: "assistant", Content: strings.Repeat("A long assistant answer that would overflow the sidebar. ", 6)}, + {Role: "user", Content: strings.Repeat("long question ", 10)}, + } + m.calcLayout() + out := m.View() + for i, line := range strings.Split(out, "\n") { + if w := lipgloss.Width(line); w > width { + t.Fatalf("sidebar size=%d line %d is %d cells wide: %q", width, i, w, line) + } + } + } +} + +// TestPanelOccupiesExactWidth pins the container contract: a panel of +// width W renders exactly W columns (border included). Off-by-two here is +// what made every layout two cells too wide. +func TestPanelOccupiesExactWidth(t *testing.T) { + for _, width := range []int{12, 30, 80, 120} { + // Content deliberately too wide for the panel. + content := strings.Repeat("x", 400) + "\n" + strings.Repeat("日本語", 50) + p := components.NewPanel("Log", width, true) + p.Height = 8 + p.Content = content + for i, line := range strings.Split(p.View(), "\n") { + if w := lipgloss.Width(line); w != width { + t.Fatalf("panel width %d: line %d is %d cells: %q", width, i, w, line) + } + } + } +} + +// TestRowLineChargesForTheFrame proves RowLine can never exceed its width +// even when the row style adds padding. +func TestRowLineChargesForTheFrame(t *testing.T) { + styles.ListItemActiveStyle = lipgloss.NewStyle().PaddingLeft(2).PaddingRight(3) + defer styles.Refresh() + got := components.RowLine(strings.Repeat("wide ", 40), 30, true) + if w := lipgloss.Width(got); w != 30 { + t.Fatalf("row is %d cells, want exactly 30: %q", w, got) + } +} + +// TestDialogsFitTerminal measures each modal's raw output (before the +// View() frame guard) so a dialog that outgrows the terminal is a test +// failure rather than a clipped border in production. +func TestDialogsFitTerminal(t *testing.T) { + for _, width := range []int{40, 60, 80, 100, 120, 160, 220} { + m := frameTestModel(t, width, 40) + m.Update(tea.WindowSizeMsg{Width: width, Height: 40}) + + m.commitDlg.Open() + assertFits(t, "commit", width, m.commitDlg.View(width)) + m.commitDlg.Close() + + m.prCreateDlg.Open("feature/extremely-long-branch-name", "dev") + assertFits(t, "pr-create", width, m.prCreateDlg.View(width)) + m.prCreateDlg.Close() + + m.configDlg.OpenGitHubToken() + assertFits(t, "config", width, m.configDlg.View(width)) + m.configDlg.Close() + + m.prMergeDlg.Open(&models.PullRequestSummary{ + Number: 1234, + Title: strings.Repeat("an extremely long pull request title ", 4), + Author: "octocat", + HeadRef: "feature/extremely-long-branch-name-that-keeps-going", + BaseRef: "dev", + Mergeable: "CONFLICTING", + }) + assertFits(t, "pr-merge", width, m.prMergeDlg.View(width)) + m.prMergeDlg.Close() + + m.palette.Open() + assertFits(t, "palette", width, m.palette.View(width)) + } +} + +// assertFits fails when any line of a rendered block exceeds width cells. +func assertFits(t *testing.T, name string, width int, out string) { + t.Helper() + if out == "" { + return + } + for i, line := range strings.Split(out, "\n") { + if w := lipgloss.Width(line); w > width { + t.Errorf("%s at width %d: line %d is %d cells (limit %d): %q", name, width, i, w, width, line) + return + } + } +} diff --git a/pkg/tui/overlay_test.go b/pkg/tui/overlay_test.go new file mode 100644 index 0000000..0bd019f --- /dev/null +++ b/pkg/tui/overlay_test.go @@ -0,0 +1,62 @@ +package tui + +import ( + "strings" + "testing" + + "github.com/charmbracelet/lipgloss" + "github.com/charmbracelet/x/ansi" +) + +// TestOverlayCenterIsColumnAccurate pins the compositor: the modal must +// land on the same cell column whatever the background contains, and the +// composited rows must still fill the terminal exactly. Rune-index math +// used to shift the modal whenever the background had wide characters. +func TestOverlayCenterIsColumnAccurate(t *testing.T) { + const width = 40 + const rows = 7 + + modal := "╭──╮\n│ab│\n╰──╯" + modalW := 4 + wantCol := (width - modalW) / 2 + + bases := []string{ + "", + "ascii background", + "日本語のテキストです", + "🎉 emoji background", + "mixed 日本 and 🎉 and ascii", + } + + for _, base := range bases { + visible := ansi.Truncate(base, width, "") + bg := visible + strings.Repeat(" ", width-ansi.StringWidth(visible)) + lines := make([]string, rows) + for i := range lines { + lines[i] = bg + } + + out := overlayCenter(strings.Join(lines, "\n"), modal, width, rows) + got := strings.Split(out, "\n") + if len(got) != rows { + t.Fatalf("base %q: %d rows, want %d", base, len(got), rows) + } + + col, row := -1, -1 + for i, l := range got { + if w := lipgloss.Width(l); w != width { + t.Fatalf("base %q: row %d is %d cells, want %d: %q", base, i, w, width, l) + } + plain := ansi.Strip(l) + if idx := strings.Index(plain, "╭──╮"); idx >= 0 { + col, row = ansi.StringWidth(plain[:idx]), i + } + } + if row < 0 { + t.Fatalf("base %q: modal missing from composite:\n%s", base, out) + } + if col != wantCol { + t.Fatalf("base %q: modal starts at column %d, want %d", base, col, wantCol) + } + } +} diff --git a/pkg/tui/styles/colors.go b/pkg/tui/styles/colors.go index a350b43..c7fc22c 100644 --- a/pkg/tui/styles/colors.go +++ b/pkg/tui/styles/colors.go @@ -34,6 +34,11 @@ var ( ActivePanelBorderColor = Accent InactivePanelBorderColor = Overlay0 + // Diff line backgrounds. Tokens, not literals: a dark green would be + // unreadable the moment the light palette is active. + DiffAddBg = lipgloss.Color("#1a3a1a") + DiffDelBg = lipgloss.Color("#3a1a1a") + // Dark reports whether the dark palette is active. Dark = true ) @@ -60,6 +65,8 @@ func SetLight() { Info = Blue Accent = Mauve Selection = lipgloss.Color("#acb0be") + DiffAddBg = "#d7f0d7" + DiffDelBg = "#f7d7dc" ActivePanelBorderColor = Accent InactivePanelBorderColor = Overlay0 Dark = false @@ -87,6 +94,8 @@ func SetDark() { Info = Blue Accent = Mauve Selection = lipgloss.Color("#45475a") + DiffAddBg = "#1a3a1a" + DiffDelBg = "#3a1a1a" ActivePanelBorderColor = Accent InactivePanelBorderColor = Overlay0 Dark = true diff --git a/pkg/tui/styles/theme.go b/pkg/tui/styles/theme.go index dc75b62..37c932c 100644 --- a/pkg/tui/styles/theme.go +++ b/pkg/tui/styles/theme.go @@ -44,10 +44,10 @@ var ( // Refresh rebuilds every style from the current tokens. // Call after SetLight/SetDark. func Refresh() { - // App-level + // App-level. No fixed width: the frame is sized per render from the + // terminal, a hardcoded 80 only ever fit an 80-column terminal. AppStyle = lipgloss.NewStyle(). - Padding(0, 1). - Width(80) + Padding(0, 1) // Tabs ActiveTabStyle = lipgloss.NewStyle(). @@ -84,7 +84,6 @@ func Refresh() { Background(Surface). Foreground(Subtext). Padding(0, 1). - Width(80). Height(1) StatusBarBranchStyle = lipgloss.NewStyle(). Foreground(Teal). @@ -134,10 +133,10 @@ func Refresh() { // Diff DiffAddStyle = lipgloss.NewStyle(). Foreground(Green). - Background(lipgloss.Color("#1a3a1a")) + Background(DiffAddBg) DiffDelStyle = lipgloss.NewStyle(). Foreground(Red). - Background(lipgloss.Color("#3a1a1a")) + Background(DiffDelBg) DiffHeaderStyle = lipgloss.NewStyle(). Foreground(Blue). Bold(true) diff --git a/pkg/tui/styles/theme_test.go b/pkg/tui/styles/theme_test.go new file mode 100644 index 0000000..e8f7b3a --- /dev/null +++ b/pkg/tui/styles/theme_test.go @@ -0,0 +1,88 @@ +package styles + +import ( + "fmt" + "testing" +) + +// TestThemeSwitchRepaintsStyles pins the contract the TUI relies on: after +// SetLight/SetDark + Refresh, every prebuilt style points at the new +// tokens. Before Refresh existed these styles froze their colors at init +// and a theme change left the UI half-recolored. +func TestThemeSwitchRepaintsStyles(t *testing.T) { + SetDark() + Refresh() + darkFg := fmt.Sprintf("%v", StatusBarStyle.GetForeground()) + darkBg := fmt.Sprintf("%v", ListItemSelectedStyle.GetBackground()) + darkBase := Base + + SetLight() + Refresh() + lightFg := fmt.Sprintf("%v", StatusBarStyle.GetForeground()) + lightBg := fmt.Sprintf("%v", ListItemSelectedStyle.GetBackground()) + + if darkFg == lightFg { + t.Fatalf("status bar foreground did not follow the theme (%s)", darkFg) + } + if darkBg == lightBg { + t.Fatalf("selected row background did not follow the theme (%s)", darkBg) + } + if Base == darkBase { + t.Fatalf("Base token did not change") + } + if ActivePanelBorderColor != Accent { + t.Fatalf("active panel border should follow the accent token") + } + if InactivePanelBorderColor != Overlay0 { + t.Fatalf("inactive panel border should follow the muted token") + } + + SetDark() + Refresh() +} + +// TestThemeTokensStayLegible guards the light palette against the usual +// mistakes: a background token reused as a foreground, or a dark literal +// left behind in a style. +func TestThemeTokensStayLegible(t *testing.T) { + for _, theme := range []struct { + name string + set func() + dark bool + }{ + {"dark", SetDark, true}, + {"light", SetLight, false}, + } { + theme.set() + Refresh() + + if Dark != theme.dark { + t.Fatalf("%s: Dark flag = %v", theme.name, Dark) + } + if Success != Green || Warning != Yellow || Danger != Red || Info != Blue || Accent != Mauve { + t.Fatalf("%s: semantic aliases drifted from their tokens", theme.name) + } + if DiffAddBg == DiffDelBg { + t.Fatalf("%s: diff backgrounds collapsed", theme.name) + } + if fmt.Sprintf("%v", DiffAddStyle.GetBackground()) != fmt.Sprintf("%v", DiffAddBg) { + t.Fatalf("%s: diff add style not rebuilt from tokens", theme.name) + } + if fmt.Sprintf("%v", DiffDelStyle.GetBackground()) != fmt.Sprintf("%v", DiffDelBg) { + t.Fatalf("%s: diff delete style not rebuilt from tokens", theme.name) + } + if Selection == Text { + t.Fatalf("%s: selection equals text color", theme.name) + } + // No style may keep a fixed 80-column width: it only ever fits an + // 80-column terminal. + if w := AppStyle.GetWidth(); w != 0 { + t.Fatalf("%s: AppStyle has a fixed width of %d", theme.name, w) + } + if w := StatusBarStyle.GetWidth(); w != 0 { + t.Fatalf("%s: StatusBarStyle has a fixed width of %d", theme.name, w) + } + } + SetDark() + Refresh() +} diff --git a/pkg/tui/views/actions.go b/pkg/tui/views/actions.go index 4b8279f..9406199 100644 --- a/pkg/tui/views/actions.go +++ b/pkg/tui/views/actions.go @@ -162,11 +162,7 @@ func (m *ActionsModel) jobsView(width int) string { for i, j := range jobs { globalIdx := m.Offset + i line := fmt.Sprintf(" %s %s", jobMark(j), truncateStr(j.Name, width-6)) - if globalIdx == m.Cursor { - b.WriteString(styles.ListItemActiveStyle.Render(cursorLine(line))) - } else { - b.WriteString(styles.ListItemStyle.Render(line)) - } + b.WriteString(components.RowLine(line, width, globalIdx == m.Cursor)) b.WriteString("\n") } return b.String() diff --git a/pkg/tui/views/ai_sidebar.go b/pkg/tui/views/ai_sidebar.go index ecc3faf..07eae27 100644 --- a/pkg/tui/views/ai_sidebar.go +++ b/pkg/tui/views/ai_sidebar.go @@ -6,6 +6,7 @@ import ( "unicode/utf8" "github.com/charmbracelet/lipgloss" + "github.com/zaidejjo/zgit/pkg/tui/components" "github.com/zaidejjo/zgit/pkg/tui/styles" ) @@ -318,6 +319,14 @@ func (m *AISidebarModel) View() string { if contentH < 5 { contentH = 5 } + // The panel occupies panelW columns total: 2 for the border and 1 + // of padding on each side. Content is drawn inside the remaining + // innerW cells — getting this wrong is what makes a sidebar shove + // the whole layout off the right edge. + innerW := contentWidth(panelW) + if innerW < 6 { + innerW = 6 + } var b strings.Builder @@ -344,7 +353,7 @@ func (m *AISidebarModel) View() string { b.WriteString("\n") b.WriteString(lipgloss.NewStyle(). Foreground(styles.Overlay0). - Render(strings.Repeat("─", panelW-2))) + Render(strings.Repeat("─", innerW))) b.WriteString("\n") // Conversation area @@ -354,7 +363,7 @@ func (m *AISidebarModel) View() string { } // Show messages (most recent first, capped) - msgLines := m.renderMessages(panelW) + msgLines := m.renderMessages(innerW) msgLines = m.truncateLines(msgLines, availH) for _, line := range msgLines { @@ -417,7 +426,7 @@ func (m *AISidebarModel) View() string { // Separator b.WriteString(lipgloss.NewStyle(). Foreground(styles.Overlay0). - Render(strings.Repeat("─", panelW-2))) + Render(strings.Repeat("─", innerW))) b.WriteString("\n") // Input area @@ -427,23 +436,33 @@ func (m *AISidebarModel) View() string { cursorChar = "█" } inputRow += m.Input[:m.Cursor] + cursorChar + m.Input[m.Cursor:] - if len(inputRow) > panelW-4 { - inputRow = "…" + inputRow[len(inputRow)-panelW+8:] + // Keep the tail (where the caret is) — rune-aware so a multi-byte + // character is never cut in half. + if runes := []rune(inputRow); len(runes) > innerW { + inputRow = "…" + string(runes[len(runes)-innerW+1:]) } b.WriteString(inputRow) // Bottom hint b.WriteString("\n") b.WriteString(lipgloss.NewStyle(). - Foreground(styles.Selection). + Foreground(styles.Overlay0). Render(fmt.Sprintf(" %s — Esc to close", m.helpHint()))) + // Final guardrail: no sidebar row may exceed the inner width. + rows := strings.Split(b.String(), "\n") + for i, row := range rows { + rows[i] = components.Truncate(row, innerW) + } + + // Style.Width counts the padding; the border is added outside it, so + // this renders exactly panelW columns. return lipgloss.NewStyle(). Border(lipgloss.RoundedBorder()). BorderForeground(styles.Mauve). - Width(panelW). + Width(styleWidth(panelW)). Padding(0, 1). - Render(b.String()) + Render(strings.Join(rows, "\n")) } func (m *AISidebarModel) helpHint() string { diff --git a/pkg/tui/views/commit.go b/pkg/tui/views/commit.go index 2b165f3..28b8436 100644 --- a/pkg/tui/views/commit.go +++ b/pkg/tui/views/commit.go @@ -84,7 +84,8 @@ func (m *CommitModel) buildForm() { ), ).WithShowHelp(true) if m.width > 0 { - m.form = m.form.WithWidth(m.width) + _, formW := dialogWidths(m.width) + m.form = m.form.WithWidth(formW) } } @@ -139,9 +140,6 @@ func (m *CommitModel) Update(msg tea.Msg) (*CommitModel, tea.Cmd) { case tea.WindowSizeMsg: m.width = msg.Width m.height = msg.Height - if m.form != nil { - m.form = m.form.WithWidth(msg.Width - 4) - } return m, nil case tea.KeyMsg: @@ -176,21 +174,26 @@ func (m *CommitModel) View(width int) string { return "" } + // Size the form here rather than at construction/activation: the + // terminal may resize (or the dialog may open before the first + // WindowSizeMsg) and a stale width is what clipped the box. + content, formW := dialogWidths(width) + if m.form != nil && m.State == CommitForm { + m.form = m.form.WithWidth(formW) + } + var b strings.Builder b.WriteString(styles.DialogTitleStyle.Render(" Commit Changes")) b.WriteString("\n\n") - if m.State == CommitResult { + switch { + case m.State == CommitResult: b.WriteString(m.renderResult()) - return styles.DialogBoxStyle.Render(b.String()) - } - if m.State == CommitConfirming { + case m.State == CommitConfirming: b.WriteString(m.renderConfirm()) - return styles.DialogBoxStyle.Render(b.String()) - } - if m.form != nil { + case m.form != nil: b.WriteString(m.form.View()) } - return styles.DialogBoxStyle.Render(b.String()) + return styles.DialogBoxStyle.Render(clampBlock(b.String(), content)) } // renderConfirm shows the final review before executing. diff --git a/pkg/tui/views/config.go b/pkg/tui/views/config.go index 7918dfe..78d6f3b 100644 --- a/pkg/tui/views/config.go +++ b/pkg/tui/views/config.go @@ -514,33 +514,40 @@ func (m *ConfigModel) View(width int) string { } var b strings.Builder - dialogW := width - 8 - if dialogW < 30 { - dialogW = 30 + + // Total box width target; content inside is 4 cells narrower because + // lipgloss draws the border and padding outside Style.Width. + boxW := width - dialogMargin + if boxW > 76 { + boxW = 76 } - if dialogW > 72 { - dialogW = 72 + if boxW < 34 { + boxW = 34 } + contentW := contentWidth(boxW) title := m.dialogTitle() switch m.State { case ConfigInput, ConfigSelect: + if m.form != nil { + m.form = m.form.WithWidth(contentW - formFieldFrame) + } b.WriteString(lipgloss.NewStyle(). Border(lipgloss.RoundedBorder()). BorderForeground(styles.Mauve). - Width(dialogW). + Width(styleWidth(boxW)). Padding(0, 1). Background(styles.Surface). - Render(strings.Join([]string{ + Render(clampBlock(strings.Join([]string{ lipgloss.NewStyle().Bold(true).Foreground(styles.Mauve).Render(title), "", m.formView(), - }, "\n"))) + }, "\n"), contentW))) case ConfigLoading: - m.renderLoading(&b, dialogW, title) + m.renderLoading(&b, boxW, title) case ConfigResult: - m.renderResult(&b, dialogW, title) + m.renderResult(&b, boxW, title) } return lipgloss.NewStyle(). @@ -586,21 +593,21 @@ func (m *ConfigModel) dialogTitle() string { return prefix + " Config" } -func (m *ConfigModel) renderLoading(b *strings.Builder, dialogW int, title string) { +func (m *ConfigModel) renderLoading(b *strings.Builder, boxW int, title string) { b.WriteString(lipgloss.NewStyle(). Border(lipgloss.RoundedBorder()). BorderForeground(styles.Mauve). - Width(dialogW). + Width(styleWidth(boxW)). Padding(0, 1). Background(styles.Surface). - Render(strings.Join([]string{ + Render(clampBlock(strings.Join([]string{ lipgloss.NewStyle().Bold(true).Foreground(styles.Mauve).Render(title), "", styles.LoadingStyle.Render("Saving..."), - }, "\n"))) + }, "\n"), contentWidth(boxW)))) } -func (m *ConfigModel) renderResult(b *strings.Builder, dialogW int, title string) { +func (m *ConfigModel) renderResult(b *strings.Builder, boxW int, title string) { result := m.Result if m.Success { result = styles.StatusStagedStyle.Render("✓ " + m.Result) @@ -616,16 +623,16 @@ func (m *ConfigModel) renderResult(b *strings.Builder, dialogW int, title string b.WriteString(lipgloss.NewStyle(). Border(lipgloss.RoundedBorder()). BorderForeground(styles.Mauve). - Width(dialogW). + Width(styleWidth(boxW)). Padding(0, 1). Background(styles.Surface). - Render(strings.Join([]string{ + Render(clampBlock(strings.Join([]string{ lipgloss.NewStyle().Bold(true).Foreground(styles.Mauve).Render(title), "", result, "", lipgloss.NewStyle().Foreground(styles.Subtext).Italic(true).Render(hint), - }, "\n"))) + }, "\n"), contentWidth(boxW)))) } // FieldLabel returns a human-readable label for the config field. diff --git a/pkg/tui/views/dialog.go b/pkg/tui/views/dialog.go new file mode 100644 index 0000000..a35522f --- /dev/null +++ b/pkg/tui/views/dialog.go @@ -0,0 +1,91 @@ +package views + +import ( + "strings" + + "github.com/zaidejjo/zgit/pkg/tui/components" + "github.com/zaidejjo/zgit/pkg/tui/styles" +) + +// Modal sizing lives here so every dialog agrees on the budget: a box may +// never be wider than the terminal it is centred in. Getting this wrong is +// what clipped dialogs on narrow terminals and let a wide huh form burst +// out of its border. +// +// lipgloss geometry, for the boxes below (all of them are +// Border + Padding(0,1)): +// +// total width = Style.Width + 2 (the border is drawn outside Width, +// while padding is counted inside it) +// text area = Style.Width - 2 (one cell of padding each side) +const ( + // boxBorder is the horizontal border (one cell each side). + boxBorder = 2 + // boxPadding is the horizontal padding of a Padding(0,1) box. + boxPadding = 2 + + // dialogMaxContent caps content width so dialogs stay readable on + // ultrawide terminals instead of stretching edge to edge. + dialogMaxContent = 84 + // dialogMinContent keeps a dialog usable on a tiny terminal; below + // this the frame guard clips rather than reflowing to nonsense. + dialogMinContent = 24 + // dialogMargin keeps the box off the terminal edge. + dialogMargin = 4 + // formFieldFrame is what a focused huh field draws outside its text: + // a 1-cell left border plus 1 cell of padding. + formFieldFrame = 2 +) + +// styleWidth returns the value to pass to Style.Width so a bordered, +// padded box occupies exactly total columns. +func styleWidth(total int) int { + w := total - boxBorder + if w < 1 { + w = 1 + } + return w +} + +// contentWidth returns the text area available inside a bordered, padded +// box that occupies exactly total columns. +func contentWidth(total int) int { + w := total - boxBorder - boxPadding + if w < 1 { + w = 1 + } + return w +} + +// dialogWidths returns, for a dialog rendered with styles.DialogBoxStyle +// at the given terminal width, the widest content block that keeps the +// box inside the terminal (borders and padding included), and the width +// to hand a huh form placed inside that block. +func dialogWidths(termWidth int) (content int, form int) { + frame := styles.DialogBoxStyle.GetHorizontalFrameSize() + content = termWidth - frame - dialogMargin + if content > dialogMaxContent { + content = dialogMaxContent + } + if content < dialogMinContent { + content = dialogMinContent + } + form = content - formFieldFrame + if form < 10 { + form = 10 + } + return content, form +} + +// clampBlock hard-caps every line of a rendered block to width cells, so a +// dialog's content can never grow its border. +func clampBlock(s string, width int) string { + if width <= 0 { + return s + } + lines := strings.Split(s, "\n") + for i, l := range lines { + lines[i] = components.Truncate(l, width) + } + return strings.Join(lines, "\n") +} diff --git a/pkg/tui/views/log.go b/pkg/tui/views/log.go index a51f533..99826c0 100644 --- a/pkg/tui/views/log.go +++ b/pkg/tui/views/log.go @@ -112,24 +112,10 @@ func (m LogModel) View(width int) string { } msg := truncateStr(c.Message, msgMaxAdjusted) - // Build line: treeStr + " " + hash + badge + " " + msg + // Build line: treeStr + " " + hash + badge + " " + msg. + // RowLine caps it to the panel width (tree graphs grow lanes). line := fmt.Sprintf("%s %s%s %s", treeStr, hash, badgeStr, msg) - lineWidth := lipgloss.Width(line) - if lineWidth > width { - // Overflow — trim msg further - overflow := lineWidth - width - msg = truncateStr(c.Message, msgMaxAdjusted-overflow) - if msgMaxAdjusted-overflow < 0 { - msg = "" - } - line = fmt.Sprintf("%s %s%s %s", treeStr, hash, badgeStr, msg) - } - - if globalIdx == m.Cursor { - b.WriteString(styles.ListItemActiveStyle.Render(cursorLine(line))) - } else { - b.WriteString(styles.ListItemStyle.Render(line)) - } + b.WriteString(components.RowLine(line, width, globalIdx == m.Cursor)) b.WriteString("\n") } @@ -219,19 +205,12 @@ func formatTime(t time.Time) string { } } +// truncateStr shortens plain (unstyled) text to maxLen cells. Delegates to +// the shared ANSI/wide-char aware implementation so there is exactly one +// truncation rule in the TUI. func truncateStr(s string, maxLen int) string { if maxLen <= 0 { return "" } - runes := []rune(s) - if len(runes) <= maxLen { - return s - } - return string(runes[:maxLen-1]) + "…" -} - -// ansiEscape + cursorLine live in components (single canonical copy); -// this alias keeps existing views and tests compiling untouched. -func cursorLine(line string) string { - return components.CursorLine(line) + return components.Truncate(s, maxLen) } diff --git a/pkg/tui/views/newviews_test.go b/pkg/tui/views/newviews_test.go index a8e6fc9..de1ff77 100644 --- a/pkg/tui/views/newviews_test.go +++ b/pkg/tui/views/newviews_test.go @@ -89,24 +89,6 @@ func TestNotesUnreadFirst(t *testing.T) { } } -func TestCursorLineStripsANSI(t *testing.T) { - styled := "\x1b[38;2;166;227;161m◆\x1b[0m 3fec75a msg" - got := cursorLine(styled) - if got != "❯ 3fec75a msg" { - t.Fatalf("cursorLine = %q", got) - } - if strings.Contains(got, "[") && strings.Contains(got, "38;2") { - t.Fatalf("ANSI leaked: %q", got) - } - if cursorLine("") != "❯" { - t.Fatalf("empty should be bare cursor") - } - // Multibyte first rune survives. - if got := cursorLine("◆ abc"); got != "❯ abc" { - t.Fatalf("multibyte = %q", got) - } -} - func TestBranchSelfTrackingShort(t *testing.T) { m := NewBranchModel() m.UpdateBranches([]*models.Branch{ diff --git a/pkg/tui/views/notifications.go b/pkg/tui/views/notifications.go index b3cd2f1..f070b91 100644 --- a/pkg/tui/views/notifications.go +++ b/pkg/tui/views/notifications.go @@ -4,7 +4,6 @@ import ( "fmt" "strings" - "github.com/charmbracelet/lipgloss" "github.com/zaidejjo/zgit/pkg/core/models" "github.com/zaidejjo/zgit/pkg/tui/components" "github.com/zaidejjo/zgit/pkg/tui/styles" @@ -166,15 +165,7 @@ func (m *NotesModel) View(width int) string { line := fmt.Sprintf(" %s %s %s %s%s", dot, noteTypeMark(it.Type), truncateStr(it.Title, 44), styles.SubtitleStyle.Render(truncateStr(it.RepoName, 26)), styles.SubtitleStyle.Render(reason)) - if lipgloss.Width(line) > width { - line = fmt.Sprintf(" %s %s %s", dot, noteTypeMark(it.Type), truncateStr(it.Title, width-8)) - } - - if globalIdx == m.Cursor { - b.WriteString(styles.ListItemActiveStyle.Render(cursorLine(line))) - } else { - b.WriteString(styles.ListItemStyle.Render(line)) - } + b.WriteString(components.RowLine(line, width, globalIdx == m.Cursor)) b.WriteString("\n") } diff --git a/pkg/tui/views/palette.go b/pkg/tui/views/palette.go index e04d57c..79fddec 100644 --- a/pkg/tui/views/palette.go +++ b/pkg/tui/views/palette.go @@ -280,13 +280,16 @@ func (m PaletteModel) View(width int) string { return "" } - dialogW := width - 8 - if dialogW < 40 { - dialogW = 40 + // The palette occupies boxW columns: its text area is contentW, the + // padding is counted inside Style.Width and the border on top. + boxW := width - dialogMargin + if boxW > 80 { + boxW = 80 } - if dialogW > 80 { - dialogW = 80 + if boxW < 34 { + boxW = 34 } + contentW := contentWidth(boxW) var b strings.Builder @@ -325,12 +328,6 @@ func (m PaletteModel) View(width int) string { results = results[scrollOffset:endIdx] } - // Available width for content (dialog minus 4 for border + padding) - contentW := dialogW - 4 - if contentW < 30 { - contentW = 30 - } - for i, idx := range results { cmd := m.Commands[idx] globalIdx := scrollOffset + i @@ -398,14 +395,18 @@ func (m PaletteModel) View(width int) string { b.WriteString("\n") } - // Hint - b.WriteString(lipgloss.NewStyle().Foreground(styles.Overlay0).Render(" Type to filter ↑↓=navigate Enter=execute Esc=close")) + // Hint (shortened when the box is narrow, so it never has to be cut) + hint := " Type to filter ↑↓=navigate Enter=execute Esc=close" + if lipgloss.Width(hint) > contentW { + hint = " ↑↓=navigate Enter=execute Esc=close" + } + b.WriteString(lipgloss.NewStyle().Foreground(styles.Overlay0).Render(hint)) return lipgloss.NewStyle(). Border(lipgloss.RoundedBorder()). BorderForeground(styles.Mauve). - Width(dialogW). + Width(styleWidth(boxW)). Padding(0, 1). Background(styles.Surface). - Render(b.String()) + Render(clampBlock(b.String(), contentW)) } diff --git a/pkg/tui/views/pr_create.go b/pkg/tui/views/pr_create.go index 65764b9..4625452 100644 --- a/pkg/tui/views/pr_create.go +++ b/pkg/tui/views/pr_create.go @@ -192,9 +192,6 @@ func (m *PRCreateModel) Update(msg tea.Msg) (*PRCreateModel, tea.Cmd) { case tea.WindowSizeMsg: m.width = msg.Width m.height = msg.Height - if m.form != nil { - m.form = m.form.WithWidth(msg.Width - 4) - } return m, nil case tea.KeyMsg: @@ -245,6 +242,11 @@ func (m *PRCreateModel) View(width int) string { return "" } + content, formW := dialogWidths(width) + if m.form != nil && m.State == PRCreateForm { + m.form = m.form.WithWidth(formW) + } + var b strings.Builder b.WriteString(styles.DialogTitleStyle.Render(" Create Pull Request")) b.WriteString("\n\n") @@ -258,7 +260,7 @@ func (m *PRCreateModel) View(width int) string { case PRCreateResult: b.WriteString(m.renderResult()) } - return styles.DialogBoxStyle.Render(b.String()) + return styles.DialogBoxStyle.Render(clampBlock(b.String(), content)) } // renderConfirm shows the final review before creating. diff --git a/pkg/tui/views/pr_merge.go b/pkg/tui/views/pr_merge.go index a6c1749..80d730b 100644 --- a/pkg/tui/views/pr_merge.go +++ b/pkg/tui/views/pr_merge.go @@ -115,7 +115,8 @@ func (m *PRMergeModel) View(width int) string { return "" } - return styles.DialogBoxStyle.Render(m.renderContent(width)) + content, _ := dialogWidths(width) + return styles.DialogBoxStyle.Render(clampBlock(m.renderContent(width), content)) } func (m *PRMergeModel) renderContent(width int) string { diff --git a/pkg/tui/views/refs.go b/pkg/tui/views/refs.go index 209424e..426eddd 100644 --- a/pkg/tui/views/refs.go +++ b/pkg/tui/views/refs.go @@ -108,7 +108,9 @@ func (m *RefsModel) sectionTitle(name string, n int, focused bool) string { } func tagRow(tag string, width int) string { - return fmt.Sprintf(" %s %s", styles.SubtitleStyle.Render("◆"), truncateStr(tag, width-6)) + return components.Truncate( + fmt.Sprintf(" %s %s", styles.SubtitleStyle.Render("◆"), truncateStr(tag, width-6)), + width) } func remoteRow(r *models.Remote, width int) string { @@ -119,5 +121,5 @@ func remoteRow(r *models.Remote, width int) string { line = fmt.Sprintf(" %s %s", styles.StatusStagedStyle.Render("●"), truncateStr(r.Name, width-6)) } - return line + return components.Truncate(line, width) } diff --git a/pkg/tui/views/status.go b/pkg/tui/views/status.go index 2dbfd73..9e7e431 100644 --- a/pkg/tui/views/status.go +++ b/pkg/tui/views/status.go @@ -6,6 +6,7 @@ import ( "github.com/charmbracelet/lipgloss" "github.com/zaidejjo/zgit/pkg/core/models" + "github.com/zaidejjo/zgit/pkg/tui/components" "github.com/zaidejjo/zgit/pkg/tui/styles" ) @@ -96,12 +97,7 @@ func (m StatusModel) View(width int) string { for i, f := range staged { line := fmt.Sprintf(" %s %s", statusMark("staged"), formatFilePath(f.Path, width-6)) localIdx := stagedOffset + i - if localIdx == m.Cursor { - // Replace first space with ❯ prefix. First byte is always ASCII space (safe byte slice). - b.WriteString(styles.ListItemActiveStyle.Render(cursorLine(line))) - } else { - b.WriteString(styles.ListItemStyle.Render(line)) - } + b.WriteString(components.RowLine(line, width, localIdx == m.Cursor)) b.WriteString("\n") } } @@ -113,11 +109,7 @@ func (m StatusModel) View(width int) string { for i, f := range unstaged { line := fmt.Sprintf(" %s %s", statusMark("unstaged"), formatFilePath(f.Path, width-6)) localIdx := unstagedOffset + i - if localIdx == m.Cursor { - b.WriteString(styles.ListItemActiveStyle.Render(cursorLine(line))) - } else { - b.WriteString(styles.ListItemStyle.Render(line)) - } + b.WriteString(components.RowLine(line, width, localIdx == m.Cursor)) b.WriteString("\n") } } @@ -129,11 +121,7 @@ func (m StatusModel) View(width int) string { for i, f := range untracked { line := fmt.Sprintf(" %s %s", statusMark("untracked"), formatFilePath(f.Path, width-6)) localIdx := untrackedOffset + i - if localIdx == m.Cursor { - b.WriteString(styles.ListItemActiveStyle.Render(cursorLine(line))) - } else { - b.WriteString(styles.ListItemStyle.Render(line)) - } + b.WriteString(components.RowLine(line, width, localIdx == m.Cursor)) b.WriteString("\n") } } @@ -211,12 +199,8 @@ func formatFilePath(path string, maxLen int) string { if lipgloss.Width(path) <= maxLen { return path } - // Filename alone too wide — truncate rune-aware - runes := []rune(path) - if maxLen-1 > 0 { - return string(runes[:maxLen-1]) + "…" - } - return string(runes[:maxLen]) + // Filename alone too wide — truncate width-aware + return components.Truncate(path, maxLen) } filename := path[lastSlash+1:] @@ -273,9 +257,5 @@ func formatFilePath(path string, maxLen int) string { } // Priority 4: truncated filename — only when terminal is too narrow - runes := []rune(filename) - if maxLen-1 > 0 { - return string(runes[:maxLen-1]) + "…" - } - return string(runes[:maxLen]) + return components.Truncate(filename, maxLen) }