Skip to content

Commit 360ec9e

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. It asks before every draw, not once at startup. The width is not a property of the process: a watch runs for minutes, and a window dragged narrower inside them leaves every later frame wrapped to a width the window no longer has. The terminal then wraps those lines itself — mid-word, ignoring the column alignment — and because the redraw counts the lines it emitted rather than the lines that appeared, it drifts further with every frame. Sampling once traded that away for an `ioctl` per second. Knowing the width is also what decides whether to redraw in place at all. Detecting a terminal and measuring it were two separate probes, so a terminal that answered the first and not the second got wrapped to the fallback constant — 120 columns of table in whatever window the reader actually had. They are now one question: no size, no wrapping, render as a log. 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` — 75 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. ✅ A resize is followed: a table drawn at 200 columns and redrawn after the window narrows to 94 keeps every line inside 94. This is the case that reaches a reader as a trail cut mid-word at the window's edge, since the terminal breaks anything the renderer lets past it. ✅ A probe that fails once leaves the last known width alone rather than snapping to the fallback, which on a narrow window is wider than the window. ✅ A terminal whose size cannot be read does not redraw in place, so wrapping never runs against a guessed width.
1 parent e62b671 commit 360ec9e

6 files changed

Lines changed: 287 additions & 26 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: 116 additions & 19 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,24 @@ 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, re-read before every draw. A watch runs for
210+
// minutes and a window can be resized inside them, so this is not a property
211+
// the process can sample once — see resize.
212+
width int
213+
214+
// size reports the terminal's width and whether it could be read. Held as a
215+
// field so a test can drive a resize without a terminal.
216+
size func() (int, bool)
217+
198218
wRequest int
199219
wChanges int
200220
wElapsed int
@@ -211,18 +231,71 @@ type renderer struct {
211231
}
212232

213233
func newRenderer() *renderer {
214-
info, err := os.Stdout.Stat()
215-
tty := err == nil && info.Mode()&os.ModeCharDevice != 0
234+
width, sized := terminalSize()
216235
return &renderer{
217-
inPlace: tty,
236+
// Redrawing in place requires knowing the width to wrap to. A terminal
237+
// that will not report its size is therefore treated as a log: emitting
238+
// a guessed width into a narrower window is what produces a physically
239+
// wrapped line, and the redraw counts the lines it emitted rather than
240+
// the lines that appeared, so one of those desyncs every frame after it.
241+
inPlace: sized,
242+
width: width,
243+
size: terminalSize,
218244
wRequest: len("REQUEST"),
219245
wChanges: len("CHANGES"),
220246
wElapsed: len("ELAPSED"),
221247
wStage: len("STAGE"),
222248
}
223249
}
224250

251+
// terminalSize is how wide the output is, in columns, and whether that came
252+
// from the terminal rather than from the fallback.
253+
//
254+
// Asking the terminal is what lets a wide window show a long trail in full,
255+
// rather than everyone being held to the narrowest window anyone might have.
256+
// Anything that is not a sized terminal — a pipe, a file, a CI log — falls back
257+
// to a fixed width, since there is no width to discover and a log wants a
258+
// stable one anyway.
259+
func terminalSize() (int, bool) {
260+
w, _, err := term.GetSize(int(os.Stdout.Fd()))
261+
if err != nil || w <= 0 {
262+
return defaultLineWidth, false
263+
}
264+
return w, true
265+
}
266+
267+
// lineWidth is the width to render to. It tolerates a renderer built without
268+
// one — a zero-value renderer in a test — rather than collapsing to nothing.
269+
func (r *renderer) lineWidth() int {
270+
if r.width <= 0 {
271+
return defaultLineWidth
272+
}
273+
return r.width
274+
}
275+
276+
// resize re-reads the terminal's width before a draw.
277+
//
278+
// The width is not a property of the process: a watch runs for minutes and the
279+
// window can be dragged narrower at any point in them. Sampling once at startup
280+
// means every frame after a resize is wrapped to a width the window no longer
281+
// has, and the terminal wraps those lines itself — mid-word, ignoring the
282+
// column alignment, and without telling the redraw, which then moves the cursor
283+
// back by fewer lines than actually appeared.
284+
//
285+
// Only the width is re-read. Whether output is a terminal at all cannot change
286+
// under a running process, and re-deciding it per frame would let a transient
287+
// probe failure switch rendering modes mid-run.
288+
func (r *renderer) resize() {
289+
if !r.inPlace || r.size == nil {
290+
return
291+
}
292+
if w, sized := r.size(); sized {
293+
r.width = w
294+
}
295+
}
296+
225297
func (r *renderer) draw(rows []*Row, status string) {
298+
r.resize()
226299
body := r.body(rows)
227300

228301
if !r.inPlace {
@@ -245,7 +318,7 @@ func (r *renderer) draw(rows []*Row, status string) {
245318
fmt.Printf("\033[K%s\n", line)
246319
}
247320
fmt.Printf("\033[K\n")
248-
fmt.Printf("\033[K ▸ %s\n", truncate(status, maxLineWidth-4))
321+
fmt.Printf("\033[K ▸ %s\n", truncate(status, r.lineWidth()-4))
249322
// Every draw emits the body, one blank line, and the status line; moving
250323
// back by exactly this many lines is what keeps the redraw from drifting.
251324
r.lastLines = len(body) + 2
@@ -264,7 +337,7 @@ func (r *renderer) body(rows []*Row) []string {
264337
rule(r.wRequest), rule(r.wChanges), rule(r.wElapsed), rule(r.wStage)))
265338

266339
for _, rw := range rows {
267-
lines = append(lines, r.rowLine(rw))
340+
lines = append(lines, r.rowLines(rw)...)
268341
lines = append(lines, r.noteLines(rw)...)
269342
}
270343
return lines
@@ -282,7 +355,7 @@ func (r *renderer) fit(rows []*Row) {
282355
r.wStage = max(r.wStage, utf8.RuneCountInString(rw.stage()))
283356
}
284357
if r.inPlace {
285-
r.wStage = min(r.wStage, max(len("STAGE"), maxLineWidth-r.prefixWidth()))
358+
r.wStage = min(r.wStage, r.stageWidth())
286359
}
287360
}
288361

@@ -291,7 +364,14 @@ func (r *renderer) prefixWidth() int {
291364
return 2 + r.wRequest + 2 + r.wChanges + 2 + r.wElapsed + 2
292365
}
293366

294-
func (r *renderer) rowLine(rw *Row) string {
367+
// rowLines renders one row: its columns, and the stage wrapped onto indented
368+
// continuation lines when the trail does not fit the width.
369+
//
370+
// Wrapping rather than cutting is what keeps a long trail readable — the end of
371+
// it is where the request actually is, so a cut there hides the interesting
372+
// part. Continuations align under the stage column so the wrapped text reads as
373+
// one field rather than as new rows.
374+
func (r *renderer) rowLines(rw *Row) []string {
295375
sqid := rw.SQID
296376
if sqid == "" {
297377
sqid = absent
@@ -302,14 +382,31 @@ func (r *renderer) rowLine(rw *Row) string {
302382
r.wRequest, sqid, pad(changes, visible, r.wChanges), r.wElapsed, rw.elapsed())
303383

304384
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())
385+
if !r.inPlace {
386+
// A log has no width to respect and is easier to read and grep on one
387+
// line, so it takes the trail whole.
388+
return []string{prefix + tail}
389+
}
390+
391+
indent := r.prefixWidth()
392+
segments := wrap(tail, r.stageWidth())
393+
if len(segments) == 0 {
394+
return []string{prefix + tail}
395+
}
396+
397+
lines := make([]string, 0, len(segments))
398+
lines = append(lines, prefix+segments[0])
399+
for _, segment := range segments[1:] {
400+
lines = append(lines, strings.Repeat(" ", indent)+" "+segment)
311401
}
312-
return prefix + tail
402+
return lines
403+
}
404+
405+
// stageWidth is the room a wrapped stage has. The two columns subtracted are
406+
// the indent a continuation line carries, so every line of a wrapped stage
407+
// fits the same budget as the first.
408+
func (r *renderer) stageWidth() int {
409+
return max(minStageWidth, r.lineWidth()-r.prefixWidth()-2)
313410
}
314411

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

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

0 commit comments

Comments
 (0)