cli: emoji welcome + screen copy catalog (golden)#385
Conversation
Two changes from Lukas's review of the real `tb` output. 1. Emoji are welcome. Reverses the "no emoji" rule — the 👋 greeting, 💚 sign-off, 🚀 sent, and the traffic-light/⚠ status glyphs are intentional brand warmth. The guard's emoji check is removed (it now enforces only hardcoded brand colour + the 'workspace' term); STYLE.md documents emoji as welcome. 2. testdata/screens.golden — a copy catalog. TestScreensGolden renders every home-view state (+ data list) through the real renderers, colour off, into one committed file, so the exact wording + spacing can be reviewed WITHOUT deploying — read the file, or the diff on any PR that changes copy. The test fails on drift; regenerate with TB_UPDATE_GOLDEN=1 go test -run TestScreensGolden. (It already surfaces one inconsistency: `data list` empty-state says `tracebloc data ingest` while the rest says `tb` — a separate fix.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@BugBot run |
Lukas: the home-only catalog wasn't enough — needs every possible string a user
could see. The catalog now has three parts, all in testdata/screens.golden:
A. COMMANDS — walks NewRootCmd and captures the `--help` of every command
(24 of them): all Short/Long descriptions + flag help, exactly as printed.
B. SCREENS — the stateful views rendered plain (13): home in all 7 states,
data list empty/populated/--all, client-create review, offboard summaries.
C. MESSAGES — a harvested, deduped, sorted index of every user-facing string
literal in internal/cli, submit, push, doctor, cluster (501 of them). This
catches error paths + the flows not rendered above (ingest validation,
login, resources) so nothing a user can see is missing.
1497 lines total. Read the file (or the PR diff) to review any copy without
deploying; the test fails on drift, regenerate with TB_UPDATE_GOLDEN=1.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@BugBot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 5ed4f88. Configure here.
Lukas: the wording was right but the layout wasn't — the catalog inserted blank lines the terminal doesn't show. Now each entry is `$ <command>` followed by the BYTE-EXACT output — line breaks, tabs, leading/trailing blanks all as printed: - Help is captured through the real `--help` flag path (SetArgs+Execute, exactly what the binary runs), not c.Help(). - Screens render through the real renderers (same code the binary runs). - No framing, no inserted whitespace: the only blank lines in the file are the outputs' own. `$ tracebloc --help` now starts its output on the very next line, and consecutive entries sit back-to-back like a real terminal session. The MESSAGE INDEX appendix stays (raw templates for search). Regenerate with TB_UPDATE_GOLDEN=1. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@BugBot run |
| r.SetArgs(append(append([]string{}, p...), "--help")) | ||
| _ = r.Execute() | ||
| block(strings.TrimSpace("tracebloc "+strings.Join(p, " "))+" --help", b.String()) | ||
| } |
There was a problem hiding this comment.
Golden help depends on env
Medium Severity
TestScreensGolden captures real --help output without clearing TRACEBLOC_CLIENT_NAME / TRACEBLOC_CLIENT_LOCATION. Those values are wired as flag defaults in client create, so a non-empty env makes pflag append an extra (default "…") and the committed catalog drifts.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 99d252c. Configure here.
Lukas: `tb data ingest` output wasn't findable. Two fixes:
- Add the ingest pre-flight `Review` screen (renderReview) to the SCREENS section.
- Replace the first-arg regex message-harvest with a go/ast walk that captures
EVERY string argument to a Printer method or error constructor — so labels that
aren't the first arg (Step("…","…") titles, MenuRow descriptions, Field values)
are now included. E.g. "Copy into your secure environment" (an ingest Step
label) was missing before; it's in the index now.
Honest limit: the assembled step-by-step RUN transcript of a live flow (ingest
steps + progress, login device flow) still isn't a single rendered screen — those
need the flow driven with a mocked cluster. But every STRING they print is now in
the MESSAGE INDEX, and the review screen is rendered. Can add driven flow
transcripts next if wanted.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@BugBot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
1 issue from previous review remains unresolved.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 567b26c. Configure here.
…iles) Per Lukas's approved structure: replace the monolith screens.golden with testdata/golden/, one file per command, ordered by how often a user hits it. Each file leads with what you see when you RUN the command (byte-exact, every state/path), and ends with its --help. This commit lands the generator + the flagship files: - 00-home.golden — tb / tracebloc, all 7 states + --help - 02-data-list.golden — empty / populated / --all + --help - zz-all-strings.golden — every user-facing string (AST index), completeness backstop Adding a command file is now one entry in the generator. Coming next: 01-ingest (driven run), 03-data-delete, 04-resources, 05-doctor, 06-delete, 07-login, the rest, and the installer folder in the client repo. (The catalog already caught drift: the root --help still says "your client" / "the cluster" instead of "secure environment" — a copy fix to do separately.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>


Summary
Follow-up to the merged style enforcement (#368), from Lukas's review of the real
tboutput. Two things:1. Emoji are welcome
Reverses the "no emoji" rule — the
👋greeting,💚sign-off,🚀sent, and the traffic-light /⚠status glyphs are intentional brand warmth. The guard's emoji check is removed (it now enforces only hardcoded brand colour + theworkspaceterm); STYLE.md documents emoji as welcome.2. A screen copy catalog —
internal/cli/testdata/screens.goldenThe recurring pain: you can't see the CLI's wording + spacing without deploying. Now
TestScreensGoldenrenders every home-view state (Online / running / starting / offline / no-env / not-signed-in) +data listthrough the real renderers (colour off), into one committed file. Read that file — or the diff on any PR that changes copy — instead of deploying. The test fails on drift; regenerate with:It already earns its keep: the catalog shows the
data listempty-state saystracebloc data ingestwhile everything else saystb— an inconsistency to fix separately.Test plan
TB_UPDATE_GOLDEN=1regenerates; the plaingo testverifies (drift guard).go build ./...,go test, gofmt, staticcheck, deadcode, file-budget, and the style guard all clean.🤖 Generated with Claude Code
Note
Low Risk
Test and documentation changes only; style guard is slightly relaxed (emoji) but user-facing runtime behaviour is unchanged except what copy goldens already document.
Overview
STYLE.md and
scripts/check-style.shnow treat emoji as intentional (👋, 💚, status glyphs): the CI style guard no longer bans traffic-light emoji and only enforces brand colour outsideinternal/uiplus theworkspace→ secure environment terminology rule.Golden copy review adds drift tests so CLI wording and layout can be reviewed without deploying.
TestCopyCatalogwrites per-command files undertestdata/golden/(home,data list, plus AST-harvestedzz-all-strings.golden).TestScreensGoldenpins a singletestdata/screens.goldentranscript: all--helpoutput, rendered screens (home states, data list, ingest/client/delete review), and a message index. Regenerate withTB_UPDATE_GOLDEN=1on the matching test run; CI fails on any byte drift.Reviewed by Cursor Bugbot for commit c24ff98. Bugbot is set up for automated code reviews on this repo. Configure here.