Skip to content

Commit 687e441

Browse files
committed
feat(client): wrap the stage at the terminal's width, never truncate
## Summary ### Why? The stage column was cut at a hard-coded 120 columns, and the cut fell at the end of the trail — which is exactly where the request currently is. Once the pipeline began reporting its finer stages, an ordinary trail outgrew the line and the run read like this, with the interesting part missing: ``` demo-queue/1 #147 22s accepted → started → validating → validated → batched → speculating → speculated → la… ``` The 120 was never a measurement. The renderer redraws in place by moving the cursor back over the lines it emitted, and a line that wraps physically occupies two rows, which desyncs every redraw after it — so the width had to be bounded somehow, and a constant was cheaper than asking. That trade was invisible while trails were short. ### What? The renderer now asks the terminal how wide it is, and wraps rather than cuts when a trail still will not fit. Asking first is what matters: on a wide window the whole trail simply fits on one line, and nobody is held to the narrowest window anyone might have. The fallback is the old constant, used whenever there is no size to discover — a pipe, a file, a CI log — where a stable width is what a log wants anyway. When a trail is longer than the line even so, it wraps onto continuation lines indented under the stage column, the way a wrapped error already does. This keeps the redraw honest rather than working around it: every line is one the renderer produced and counted, so the cursor arithmetic still holds, and nothing is ever cut. Piped output is left on a single unwrapped line, since a log is easier to read and grep that way and has no width to respect. ## Test Plan ✅ `bazel test //submitqueue/client:go_default_test` — 39 cases pass, including the pre-existing redraw-accounting ones that pin the property this all rests on: no emitted line exceeds the width. ✅ Seven new cases: a long trail wraps instead of truncating and every status survives it, the end of the trail is on the last line, continuations align under the stage column, no line exceeds the width at 80/100/120/200 columns, a 240-column terminal needs no wrapping at all, piped output stays on one line, and a window too narrow for the columns still wraps to a readable floor rather than one word per line. ✅ The zero-value renderer that tests construct directly falls back to the default width rather than collapsing, which is what keeps the moved tests working unchanged.
1 parent 94b7e6a commit 687e441

6 files changed

Lines changed: 194 additions & 21 deletions

File tree

MODULE.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ use_repo(
6666
"org_golang_google_protobuf",
6767
"org_golang_x_oauth2",
6868
"org_golang_x_sync",
69+
"org_golang_x_term",
6970
"org_uber_go_fx",
7071
"org_uber_go_mock",
7172
"org_uber_go_yarpc",

go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ require (
1414
go.uber.org/zap v1.27.1
1515
golang.org/x/oauth2 v0.34.0
1616
golang.org/x/sync v0.19.0
17+
golang.org/x/term v0.39.0
1718
google.golang.org/grpc v1.68.1
1819
google.golang.org/protobuf v1.36.10
1920
gopkg.in/yaml.v3 v3.0.1

go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,8 @@ golang.org/x/sys v0.0.0-20211019181941-9d821ace8654/go.mod h1:oPkhp1MJrh7nUepCBc
249249
golang.org/x/sys v0.40.0 h1:DBZZqJ2Rkml6QMQsZywtnjnnGvHza6BTfYFWY9kjEWQ=
250250
golang.org/x/sys v0.40.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks=
251251
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
252+
golang.org/x/term v0.39.0 h1:RclSuaJf32jOqZz74CkPA9qFuVTX7vhLlpfj/IGWlqY=
253+
golang.org/x/term v0.39.0/go.mod h1:yxzUCTP/U+FzoxfdKmLaA0RV1WgE0VY7hXBwKtY/4ww=
252254
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
253255
golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk=
254256
golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=

submitqueue/client/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ go_library(
1919
"@org_golang_google_grpc//:go_default_library",
2020
"@org_golang_google_grpc//credentials:go_default_library",
2121
"@org_golang_google_grpc//credentials/insecure:go_default_library",
22+
"@org_golang_x_term//:go_default_library",
2223
],
2324
)
2425

submitqueue/client/view.go

Lines changed: 81 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,23 +24,28 @@ import (
2424

2525
pb "github.com/uber/submitqueue/api/submitqueue/gateway/protopb"
2626
"github.com/uber/submitqueue/submitqueue/entity"
27+
"golang.org/x/term"
2728
)
2829

2930
const (
3031
// pollInterval bounds how often the watcher re-reads every request's history.
3132
pollInterval = 2 * time.Second
3233

33-
// maxLineWidth caps a redrawn line. A line that wraps occupies two physical
34-
// rows, which permanently desyncs the cursor arithmetic the in-place redraw
35-
// depends on; capping is cheaper than asking the terminal how wide it is.
36-
maxLineWidth = 120
34+
// defaultLineWidth is the width assumed when the terminal will not say how
35+
// wide it is — piped output, or a terminal that answers no size at all.
36+
defaultLineWidth = 120
3737

3838
// absent is what a cell shows before there is anything to put in it.
3939
absent = "—"
4040

4141
// minNoteWidth keeps a wrapped error readable even when the columns before
4242
// it have eaten most of the line.
4343
minNoteWidth = 40
44+
45+
// minStageWidth is the narrowest the stage column is allowed to wrap to. A
46+
// window narrow enough to force this is already unreadable; the floor keeps
47+
// the wrap from degenerating into one word per line.
48+
minStageWidth = 24
4449
)
4550

4651
// terminalStatuses are the states a land request settles on. They are keyed off
@@ -192,9 +197,20 @@ func summarize(rows []*Row) error {
192197
//
193198
// Column widths only ever grow, so a value that turns out to be wider than the
194199
// header does not make the table jitter as rows fill in.
200+
//
201+
// No line the renderer emits may exceed the terminal's width. A line that wraps
202+
// occupies two physical rows, and the in-place redraw moves the cursor back by
203+
// the number of lines it *emitted* — so one wrapped line desyncs every redraw
204+
// after it. Everything wide is therefore wrapped deliberately, into lines the
205+
// renderer counts itself.
195206
type renderer struct {
196207
inPlace bool
197208

209+
// width is the terminal's width, or defaultLineWidth when it will not say.
210+
// Read once: a window resized mid-run is rare next to the cost of asking on
211+
// every redraw, and the redraw is already bounded by what it last emitted.
212+
width int
213+
198214
wRequest int
199215
wChanges int
200216
wElapsed int
@@ -215,13 +231,38 @@ func newRenderer() *renderer {
215231
tty := err == nil && info.Mode()&os.ModeCharDevice != 0
216232
return &renderer{
217233
inPlace: tty,
234+
width: terminalWidth(),
218235
wRequest: len("REQUEST"),
219236
wChanges: len("CHANGES"),
220237
wElapsed: len("ELAPSED"),
221238
wStage: len("STAGE"),
222239
}
223240
}
224241

242+
// terminalWidth is how wide the output is, in columns.
243+
//
244+
// Asking the terminal is what lets a wide window show a long trail in full,
245+
// rather than everyone being held to the narrowest window anyone might have.
246+
// Anything that is not a sized terminal — a pipe, a file, a CI log — falls back
247+
// to a fixed width, since there is no width to discover and a log wants a
248+
// stable one anyway.
249+
func terminalWidth() int {
250+
w, _, err := term.GetSize(int(os.Stdout.Fd()))
251+
if err != nil || w <= 0 {
252+
return defaultLineWidth
253+
}
254+
return w
255+
}
256+
257+
// lineWidth is the width to render to. It tolerates a renderer built without
258+
// one — a zero-value renderer in a test — rather than collapsing to nothing.
259+
func (r *renderer) lineWidth() int {
260+
if r.width <= 0 {
261+
return defaultLineWidth
262+
}
263+
return r.width
264+
}
265+
225266
func (r *renderer) draw(rows []*Row, status string) {
226267
body := r.body(rows)
227268

@@ -245,7 +286,7 @@ func (r *renderer) draw(rows []*Row, status string) {
245286
fmt.Printf("\033[K%s\n", line)
246287
}
247288
fmt.Printf("\033[K\n")
248-
fmt.Printf("\033[K ▸ %s\n", truncate(status, maxLineWidth-4))
289+
fmt.Printf("\033[K ▸ %s\n", truncate(status, r.lineWidth()-4))
249290
// Every draw emits the body, one blank line, and the status line; moving
250291
// back by exactly this many lines is what keeps the redraw from drifting.
251292
r.lastLines = len(body) + 2
@@ -264,7 +305,7 @@ func (r *renderer) body(rows []*Row) []string {
264305
rule(r.wRequest), rule(r.wChanges), rule(r.wElapsed), rule(r.wStage)))
265306

266307
for _, rw := range rows {
267-
lines = append(lines, r.rowLine(rw))
308+
lines = append(lines, r.rowLines(rw)...)
268309
lines = append(lines, r.noteLines(rw)...)
269310
}
270311
return lines
@@ -282,7 +323,7 @@ func (r *renderer) fit(rows []*Row) {
282323
r.wStage = max(r.wStage, utf8.RuneCountInString(rw.stage()))
283324
}
284325
if r.inPlace {
285-
r.wStage = min(r.wStage, max(len("STAGE"), maxLineWidth-r.prefixWidth()))
326+
r.wStage = min(r.wStage, r.stageWidth())
286327
}
287328
}
288329

@@ -291,7 +332,14 @@ func (r *renderer) prefixWidth() int {
291332
return 2 + r.wRequest + 2 + r.wChanges + 2 + r.wElapsed + 2
292333
}
293334

294-
func (r *renderer) rowLine(rw *Row) string {
335+
// rowLines renders one row: its columns, and the stage wrapped onto indented
336+
// continuation lines when the trail does not fit the width.
337+
//
338+
// Wrapping rather than cutting is what keeps a long trail readable — the end of
339+
// it is where the request actually is, so a cut there hides the interesting
340+
// part. Continuations align under the stage column so the wrapped text reads as
341+
// one field rather than as new rows.
342+
func (r *renderer) rowLines(rw *Row) []string {
295343
sqid := rw.SQID
296344
if sqid == "" {
297345
sqid = absent
@@ -302,14 +350,31 @@ func (r *renderer) rowLine(rw *Row) string {
302350
r.wRequest, sqid, pad(changes, visible, r.wChanges), r.wElapsed, rw.elapsed())
303351

304352
tail := rw.stage()
305-
if r.inPlace {
306-
// Only the tail can overflow, and unlike the changes cell it never holds
307-
// escape sequences, so it is the one part safe to cut. The budget comes
308-
// from the column widths rather than the rendered prefix, which counts a
309-
// hyperlink's escape bytes that take up no space on screen.
310-
tail = truncate(tail, maxLineWidth-r.prefixWidth())
353+
if !r.inPlace {
354+
// A log has no width to respect and is easier to read and grep on one
355+
// line, so it takes the trail whole.
356+
return []string{prefix + tail}
357+
}
358+
359+
indent := r.prefixWidth()
360+
segments := wrap(tail, r.stageWidth())
361+
if len(segments) == 0 {
362+
return []string{prefix + tail}
311363
}
312-
return prefix + tail
364+
365+
lines := make([]string, 0, len(segments))
366+
lines = append(lines, prefix+segments[0])
367+
for _, segment := range segments[1:] {
368+
lines = append(lines, strings.Repeat(" ", indent)+" "+segment)
369+
}
370+
return lines
371+
}
372+
373+
// stageWidth is the room a wrapped stage has. The two columns subtracted are
374+
// the indent a continuation line carries, so every line of a wrapped stage
375+
// fits the same budget as the first.
376+
func (r *renderer) stageWidth() int {
377+
return max(minStageWidth, r.lineWidth()-r.prefixWidth()-2)
313378
}
314379

315380
// noteLines renders a request's error under its row, wrapped and indented to
@@ -324,7 +389,7 @@ func (r *renderer) noteLines(rw *Row) []string {
324389
indent := r.prefixWidth()
325390
// A piped run spends most of the line on URLs, so the wrap width is floored
326391
// rather than allowed to collapse to nothing.
327-
width := max(minNoteWidth, maxLineWidth-indent-2)
392+
width := max(minNoteWidth, r.lineWidth()-indent-2)
328393

329394
wrapped := wrap(rw.Note, width)
330395
lines := make([]string, 0, len(wrapped))

submitqueue/client/view_test.go

Lines changed: 108 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ func TestDrawStaysWithinLineWidth(t *testing.T) {
294294
out := captureStdout(t, func() { r.draw(rows, strings.Repeat("status ", 40)) })
295295
for _, line := range strings.Split(out, "\n") {
296296
line = strings.ReplaceAll(line, "\033[K", "")
297-
assert.LessOrEqual(t, len([]rune(line)), maxLineWidth, "line too wide: %q", line)
297+
assert.LessOrEqual(t, len([]rune(line)), r.lineWidth(), "line too wide: %q", line)
298298
}
299299
}
300300

@@ -554,7 +554,7 @@ func TestNoteLinesRenderErrorInFull(t *testing.T) {
554554

555555
var text strings.Builder
556556
for i, line := range lines {
557-
assert.LessOrEqual(t, len([]rune(line)), maxLineWidth, "a wrapped note still has to fit the line")
557+
assert.LessOrEqual(t, len([]rune(line)), r.lineWidth(), "a wrapped note still has to fit the line")
558558
trimmed := strings.TrimLeft(line, " ")
559559
if i == 0 {
560560
assert.True(t, strings.HasPrefix(trimmed, "↳ "), "the first line is marked")
@@ -567,8 +567,8 @@ func TestNoteLinesRenderErrorInFull(t *testing.T) {
567567
"the tail of the error is what says what went wrong; it must survive")
568568

569569
// The row itself keeps only the trail, so the columns stay aligned.
570-
assert.NotContains(t, r.rowLine(failed), "speculator failed")
571-
assert.NotContains(t, r.rowLine(failed), "…")
570+
assert.NotContains(t, strings.Join(r.rowLines(failed), "\n"), "speculator failed")
571+
assert.NotContains(t, strings.Join(r.rowLines(failed), "\n"), "…")
572572
}
573573

574574
// TestNoteLinesIndentToStageColumn keeps a wrapped error visually attached to
@@ -600,7 +600,7 @@ func TestRowLineAlignment(t *testing.T) {
600600
r.fit(rows)
601601

602602
for _, rw := range rows {
603-
shown := []rune(visible(r.rowLine(rw)))
603+
shown := []rune(visible(r.rowLines(rw)[0]))
604604
require.GreaterOrEqual(t, len(shown), r.prefixWidth())
605605
assert.Equal(t, rw.stage(), string(shown[r.prefixWidth():]),
606606
"the stage should start at column %d and be rendered whole", r.prefixWidth())
@@ -656,3 +656,106 @@ func head(s string, n int) string {
656656
}
657657
return s[:n]
658658
}
659+
660+
// longTrail is the shape that prompted wrapping: every status the pipeline now
661+
// publishes, which no longer fits a default-width line.
662+
var longTrail = []string{
663+
"accepted", "started", "validating", "validated", "batched",
664+
"speculating", "speculated", "building", "built", "landing", "landed",
665+
}
666+
667+
func TestRowLinesWrapsRatherThanTruncates(t *testing.T) {
668+
r := newRenderer()
669+
r.inPlace = true
670+
rw := &Row{SQID: "demo-queue/1", Submitted: time.Now(), Trail: longTrail}
671+
r.fit([]*Row{rw})
672+
673+
lines := r.rowLines(rw)
674+
require.Greater(t, len(lines), 1, "a trail this long has to wrap")
675+
676+
joined := strings.Join(lines, " ")
677+
assert.NotContains(t, joined, "…", "nothing is cut, so there is no ellipsis")
678+
for _, status := range longTrail {
679+
assert.Contains(t, joined, status, "every status survives the wrap")
680+
}
681+
assert.Contains(t, lines[len(lines)-1], "landed",
682+
"the end of the trail is where the request is; it must be the part that shows")
683+
}
684+
685+
func TestRowLinesContinuationsAlignUnderTheStage(t *testing.T) {
686+
r := newRenderer()
687+
r.inPlace = true
688+
rw := &Row{SQID: "demo-queue/1", Submitted: time.Now(), Trail: longTrail}
689+
r.fit([]*Row{rw})
690+
691+
lines := r.rowLines(rw)
692+
require.Greater(t, len(lines), 1)
693+
694+
for _, line := range lines[1:] {
695+
leading := len(line) - len(strings.TrimLeft(line, " "))
696+
assert.GreaterOrEqual(t, leading, r.prefixWidth(),
697+
"a continuation sits under the stage column, not under the request id")
698+
}
699+
}
700+
701+
func TestRowLinesFitTheWidth(t *testing.T) {
702+
// The redraw moves the cursor back by the number of lines it emitted, so a
703+
// line wide enough to wrap physically would desync every redraw after it.
704+
widths := []int{80, 100, 120, 200}
705+
for _, width := range widths {
706+
t.Run(fmt.Sprintf("width %d", width), func(t *testing.T) {
707+
r := newRenderer()
708+
r.inPlace = true
709+
r.width = width
710+
rw := &Row{SQID: "demo-queue/1", Submitted: time.Now(), Trail: longTrail}
711+
r.fit([]*Row{rw})
712+
713+
for _, line := range r.rowLines(rw) {
714+
assert.LessOrEqual(t, len([]rune(visible(line))), width, "line too wide: %q", line)
715+
}
716+
})
717+
}
718+
}
719+
720+
func TestRowLinesUseTheWholeWidthBeforeWrapping(t *testing.T) {
721+
// The point of asking the terminal how wide it is: a window with room for
722+
// the whole trail should show it on one line.
723+
r := newRenderer()
724+
r.inPlace = true
725+
r.width = 240
726+
rw := &Row{SQID: "demo-queue/1", Submitted: time.Now(), Trail: longTrail}
727+
r.fit([]*Row{rw})
728+
729+
lines := r.rowLines(rw)
730+
assert.Len(t, lines, 1, "a wide terminal needs no wrapping")
731+
assert.Contains(t, lines[0], "landed")
732+
}
733+
734+
func TestRowLinesPipedStayOnOneLine(t *testing.T) {
735+
// A log has no width to respect and is easier to read and grep unwrapped.
736+
r := newRenderer()
737+
r.inPlace = false
738+
rw := &Row{SQID: "demo-queue/1", Submitted: time.Now(), Trail: longTrail}
739+
r.fit([]*Row{rw})
740+
741+
lines := r.rowLines(rw)
742+
require.Len(t, lines, 1)
743+
assert.Contains(t, lines[0], "landed")
744+
assert.NotContains(t, lines[0], "…")
745+
}
746+
747+
func TestStageWidthHasAFloor(t *testing.T) {
748+
r := newRenderer()
749+
r.inPlace = true
750+
r.width = 20
751+
r.wRequest, r.wChanges, r.wElapsed = 40, 40, 40
752+
753+
assert.Equal(t, minStageWidth, r.stageWidth(),
754+
"a window too narrow to hold the columns still wraps to something readable")
755+
}
756+
757+
func TestLineWidthFallsBackWhenUnset(t *testing.T) {
758+
// Tests build renderers directly; a zero width must not collapse the table.
759+
r := &renderer{inPlace: true}
760+
assert.Equal(t, defaultLineWidth, r.lineWidth())
761+
}

0 commit comments

Comments
 (0)