From de8cb570144a6ba29c941143c6e22e15108a30cd Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Sun, 9 Aug 2026 17:25:08 +0300 Subject: [PATCH] fix(tui): stop rune-splitting truncation and the false full bar Three truncation sites sliced strings by byte index. A multi-byte rune at the cut produced invalid UTF-8 and rendered as mojibake. list.go's brokenReason, provstep.go's provLine, and progressbar.go's progressLabel now call ansi.Truncate, which cuts on rune and cell boundaries. cloudInitBar drew a full bar for the "error", "disabled" and "not-run" states, next to a red "failed" or dim status label. It now draws no bar for those three terminal non-success states, and keeps the bar for "waiting", "running" and "done". Removed a dead branch in cloudInitBar whose outer "done" check duplicated the switch's own fallthrough. --- internal/tui/cloudinit.go | 14 ++++++++---- internal/tui/cloudinit_test.go | 20 ++++++++++++++++ internal/tui/list.go | 6 ++--- internal/tui/progressbar.go | 7 +++--- internal/tui/provstep.go | 7 ++---- internal/tui/truncate_test.go | 42 ++++++++++++++++++++++++++++++++++ 6 files changed, 78 insertions(+), 18 deletions(-) create mode 100644 internal/tui/truncate_test.go diff --git a/internal/tui/cloudinit.go b/internal/tui/cloudinit.go index 15f7892..deccd96 100644 --- a/internal/tui/cloudinit.go +++ b/internal/tui/cloudinit.go @@ -142,13 +142,17 @@ func cloudInitFraction(status string) float64 { // hand-rolled track. ViewAs draws a fixed computed value, so none of the // component's animation machinery runs. The gradient and width handling // still come free. +// +// "error", "disabled" and "not-run" draw no bar. A bar reads as progress +// toward completion, and none of those three states is heading there; a full +// bar next to a red "failed" label previously said the opposite of the +// label. func cloudInitBar(p progress.Model, status string) string { - if status == "" || cloudInitDone(status) && status != "running" { - if status == "done" { - return p.ViewAs(1) - } + switch status { + case "waiting", "running", "done": + return p.ViewAs(cloudInitFraction(status)) } - return p.ViewAs(cloudInitFraction(status)) + return "" } // newCloudInitProgress is the bar used for cloud-init staging. diff --git a/internal/tui/cloudinit_test.go b/internal/tui/cloudinit_test.go index 343fd55..6f49c7e 100644 --- a/internal/tui/cloudinit_test.go +++ b/internal/tui/cloudinit_test.go @@ -2,6 +2,7 @@ package tui import ( "os/exec" + "strings" "testing" ) @@ -98,6 +99,25 @@ func TestDecodeCloudInitStatus(t *testing.T) { } } +// TestCloudInitBarSuppressedOnTerminalFailure pins the fix for a full bar +// rendering next to a red "failed" label. "error", "disabled" and "not-run" +// are terminal states that never reach 100% install; cloudInitBar must draw +// nothing for them. "running" still gets a bar, so the suppression is +// specific to the non-success terminal states, not to every status. +func TestCloudInitBarSuppressedOnTerminalFailure(t *testing.T) { + p := newCloudInitProgress() + + for _, status := range []string{"error", "disabled", "not-run"} { + if got := cloudInitBar(p, status); got != "" { + t.Errorf("cloudInitBar(%q) = %q, want no bar", status, got) + } + } + + if got := cloudInitBar(p, "running"); !strings.Contains(got, "█") { + t.Errorf("cloudInitBar(%q) = %q, want a bar", "running", got) + } +} + // TestOutputIgnoresStderrNoise exercises the mechanism checkCloudInit relies // on: Cmd.Output(), not Cmd.CombinedOutput(). A login banner or stray // warning an ssh session writes to stderr never reaches the decoder. An ssh diff --git a/internal/tui/list.go b/internal/tui/list.go index 58d6296..5f0a4ba 100644 --- a/internal/tui/list.go +++ b/internal/tui/list.go @@ -5,6 +5,7 @@ import ( tea "charm.land/bubbletea/v2" "charm.land/lipgloss/v2" + "github.com/charmbracelet/x/ansi" "github.com/novusedge/stoat/internal/core" ) @@ -242,10 +243,7 @@ func brokenReason(s string) string { s = s[:i] } const max = 60 - if len(s) > max { - s = s[:max-1] + "…" - } - return s + return ansi.Truncate(s, max, "…") } // viewList renders the list screen's body: everything below the banner. diff --git a/internal/tui/progressbar.go b/internal/tui/progressbar.go index 7e71d04..f665373 100644 --- a/internal/tui/progressbar.go +++ b/internal/tui/progressbar.go @@ -4,6 +4,8 @@ import ( "fmt" "time" + "github.com/charmbracelet/x/ansi" + "github.com/novusedge/stoat/internal/core" ) @@ -26,10 +28,7 @@ func progressLabel(p Progress, width int) string { } out := bar(p.Frac, width) + " " + dimStyle.Render(num) if l := p.Label; l != "" { - if len(l) > provLabelMax { - l = l[:provLabelMax-1] + "…" - } - out += dimStyle.Render(" · " + l) + out += dimStyle.Render(" · " + ansi.Truncate(l, provLabelMax, "…")) } return out } diff --git a/internal/tui/provstep.go b/internal/tui/provstep.go index 7612831..1192a3c 100644 --- a/internal/tui/provstep.go +++ b/internal/tui/provstep.go @@ -9,6 +9,7 @@ import ( "time" "charm.land/bubbles/v2/spinner" + "github.com/charmbracelet/x/ansi" "github.com/novusedge/stoat/internal/core" "github.com/novusedge/stoat/internal/sshx" @@ -128,11 +129,7 @@ func provLine(spin spinner.Model, name string, st provState, now time.Time) stri case st.hasProg: out += " " + progressLabel(st.prog, provBarWidth) case st.last != "": - l := st.last - if len(l) > provMaxLast { - l = l[:provMaxLast-1] + "…" - } - out += dimStyle.Render(" · " + l) + out += dimStyle.Render(" · " + ansi.Truncate(st.last, provMaxLast, "…")) } return out + dimStyle.Render(" · "+provElapsed(now.Sub(st.start))) } diff --git a/internal/tui/truncate_test.go b/internal/tui/truncate_test.go new file mode 100644 index 0000000..6bfb5b6 --- /dev/null +++ b/internal/tui/truncate_test.go @@ -0,0 +1,42 @@ +package tui + +import ( + "strings" + "testing" + "time" + "unicode/utf8" + + "charm.land/lipgloss/v2" + "github.com/charmbracelet/x/ansi" +) + +// longMultiByte is a run of 2-byte runes, so every rune boundary sits at an +// even byte offset. Every truncation budget in this package (max 60, +// provMaxLast 34, provLabelMax 18) cuts at an odd byte index (budget - 1), so +// a byte-index slice always lands mid-rune here. ansi.Truncate cuts on rune +// and cell boundaries instead. +var longMultiByte = strings.Repeat("é", 40) + +func TestTruncationKeepsValidUTF8(t *testing.T) { + outs := map[string]string{ + "brokenReason": brokenReason(longMultiByte), + "provLine": ansi.Strip(provLine(newSpinner(), "vm", provState{last: longMultiByte}, time.Now())), + "progressLabel": ansi.Strip(progressLabel(Progress{Frac: 0.5, Label: longMultiByte}, provBarWidth)), + } + for name, out := range outs { + if !utf8.ValidString(out) { + t.Errorf("%s: output is not valid UTF-8: %q", name, out) + } + } +} + +// TestBrokenReasonStaysWithinWidthBudget pins brokenReason's own budget, +// the one truncation site that returns the truncated string directly rather +// than wrapping it with other row content. +func TestBrokenReasonStaysWithinWidthBudget(t *testing.T) { + const budget = 60 + out := brokenReason(longMultiByte) + if w := lipgloss.Width(out); w > budget { + t.Errorf("brokenReason width = %d, want <= %d: %q", w, budget, out) + } +}