fix(config): clarify init copy for TTY, preserve original for AI#448
fix(config): clarify init copy for TTY, preserve original for AI#448MaxHuang22 merged 1 commit intomainfrom
Conversation
The interactive `config init` flow showed a QR code and verification
link without indicating their relationship, leaving users unsure
which to act on first and whether the link was still needed after
scanning.
Split the message strings on TTY vs non-TTY:
- TTY: header above QR ("使用飞书 / Lark 扫码配置应用"), "或打开链接"
framing to mark the link as an alternative, and an active waiting
indicator.
- Non-TTY (AI / piped callers via --new): keep the original copy
verbatim so existing parsers and prompts are unaffected.
QR is still rendered in both branches.
Change-Id: I9b753f044ebefaedbb4b095cabf7beff4669eb2e
48d4295 to
60d7ab9
Compare
Greptile SummaryThis PR splits the Confidence Score: 5/5Safe to merge — all remaining findings are P2 style suggestions with no behavioral impact Non-TTY output is byte-for-byte identical to the original; TTY copy improvements are purely cosmetic. Struct, locale maps, and the exhaustiveness test are consistent. The only finding is a minor code duplication that does not affect correctness. cmd/config/init_interactive.go — minor QR-block duplication worth a follow-up refactor Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[runCreateAppFlow] --> B[Build verificationURL]
B --> C{f.IOStreams.IsTerminal?}
C -- TTY --> D[Print ScanQRCode header]
D --> E[Render QR code]
E --> F[Print ScanOrOpenLink]
F --> G[Print URL]
G --> H[Print WaitingForScan]
C -- non-TTY --> I[Render QR code]
I --> J[Print OpenLinkNonTTY]
J --> K[Print URL]
K --> L[Print WaitingForScanNonTTY]
H --> M[PollAppRegistration]
L --> M
Reviews (1): Last reviewed commit: "fix(config): clarify init copy for TTY, ..." | Re-trigger Greptile |
📝 WalkthroughWalkthroughThe pull request refactors the app creation flow to conditionally display different verification messages based on terminal interactivity. QR code and link prompts now branch into separate TTY and non-TTY message paths, replacing unified messaging with terminal-aware variants. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@60d7ab9642efe895c55203b18fb05fa85086a569🧩 Skill updatenpx skills add larksuite/cli#accurate-lotus -y -g |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
cmd/config/init_interactive.go (1)
184-195: Deduplicate QR rendering to reduce branch drift risk.
qrcode.New(...)/ToSmallString(...)is identical in both branches; rendering once keeps the TTY/non-TTY difference focused on copy only.Proposed cleanup
- if f.IOStreams.IsTerminal { - fmt.Fprintf(f.IOStreams.ErrOut, "%s", msg.ScanQRCode) - qr, qrErr := qrcode.New(verificationURL, qrcode.Medium) - if qrErr == nil { - fmt.Fprint(f.IOStreams.ErrOut, qr.ToSmallString(false)) - } + qr, qrErr := qrcode.New(verificationURL, qrcode.Medium) + if f.IOStreams.IsTerminal { + fmt.Fprintf(f.IOStreams.ErrOut, "%s", msg.ScanQRCode) + if qrErr == nil { + fmt.Fprint(f.IOStreams.ErrOut, qr.ToSmallString(false)) + } fmt.Fprintf(f.IOStreams.ErrOut, "%s", msg.ScanOrOpenLink) fmt.Fprintf(f.IOStreams.ErrOut, " %s\n\n", verificationURL) fmt.Fprintf(f.IOStreams.ErrOut, "%s\n", msg.WaitingForScan) } else { - qr, qrErr := qrcode.New(verificationURL, qrcode.Medium) if qrErr == nil { fmt.Fprint(f.IOStreams.ErrOut, qr.ToSmallString(false)) } fmt.Fprintf(f.IOStreams.ErrOut, "%s", msg.OpenLinkNonTTY)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/config/init_interactive.go` around lines 184 - 195, The QR generation and rendering (qrcode.New(verificationURL, qrcode.Medium) and qr.ToSmallString(false)) are duplicated in both branches; create the QR once before the TTY/non-TTY branch by calling qrcode.New(verificationURL, qrcode.Medium), check qrErr and print qr.ToSmallString(false) to f.IOStreams.ErrOut if nil, then keep the existing branch-specific output (the ScanOrOpenLink, verificationURL, WaitingForScan lines) only in their respective branches; remove the duplicate qrcode.New/qr.ToSmallString blocks so the TTY difference is limited to the copy text and not QR rendering.cmd/config/init_messages_test.go (1)
57-64: Add exact-string regression checks for non-TTY prompts.These assertions only catch empties; they won’t detect wording drift that could break AI consumers expecting stable non-TTY copy.
Proposed test hardening
+func TestInitMsg_NonTTYCopyStable(t *testing.T) { + cases := []struct { + name string + msg *initMsg + open string + waiting string + }{ + { + name: "zh", + msg: initMsgZh, + open: "\n打开以下链接配置应用:\n\n", + waiting: "等待配置应用...", + }, + { + name: "en", + msg: initMsgEn, + open: "\nOpen the link below to configure app:\n\n", + waiting: "Waiting for app configuration...", + }, + } + + for _, tc := range cases { + if tc.msg.OpenLinkNonTTY != tc.open { + t.Errorf("%s OpenLinkNonTTY changed: %q", tc.name, tc.msg.OpenLinkNonTTY) + } + if tc.msg.WaitingForScanNonTTY != tc.waiting { + t.Errorf("%s WaitingForScanNonTTY changed: %q", tc.name, tc.msg.WaitingForScanNonTTY) + } + } +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/config/init_messages_test.go` around lines 57 - 64, The test currently only checks that non-TTY prompt fields are non-empty, which won't catch wording drift; update the assertions in init_messages_test.go to assert exact string equality for the non-TTY prompts (specifically msg.OpenLinkNonTTY and msg.WaitingForScanNonTTY) against the expected literal texts used by AI/consumer contracts, replacing the emptiness checks with table-driven comparisons (or explicit t.Fatalf/t.Errorf checks) that fail if the strings differ; ensure you reference the same map keys ("OpenLinkNonTTY", "WaitingForScanNonTTY") and keep the expected strings as constants in the test so future diffs must intentionally update them.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cmd/config/init_interactive.go`:
- Around line 184-195: The QR generation and rendering
(qrcode.New(verificationURL, qrcode.Medium) and qr.ToSmallString(false)) are
duplicated in both branches; create the QR once before the TTY/non-TTY branch by
calling qrcode.New(verificationURL, qrcode.Medium), check qrErr and print
qr.ToSmallString(false) to f.IOStreams.ErrOut if nil, then keep the existing
branch-specific output (the ScanOrOpenLink, verificationURL, WaitingForScan
lines) only in their respective branches; remove the duplicate
qrcode.New/qr.ToSmallString blocks so the TTY difference is limited to the copy
text and not QR rendering.
In `@cmd/config/init_messages_test.go`:
- Around line 57-64: The test currently only checks that non-TTY prompt fields
are non-empty, which won't catch wording drift; update the assertions in
init_messages_test.go to assert exact string equality for the non-TTY prompts
(specifically msg.OpenLinkNonTTY and msg.WaitingForScanNonTTY) against the
expected literal texts used by AI/consumer contracts, replacing the emptiness
checks with table-driven comparisons (or explicit t.Fatalf/t.Errorf checks) that
fail if the strings differ; ensure you reference the same map keys
("OpenLinkNonTTY", "WaitingForScanNonTTY") and keep the expected strings as
constants in the test so future diffs must intentionally update them.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a443fe51-4ad4-47ef-993c-961f9bb051c5
📒 Files selected for processing (3)
cmd/config/init_interactive.gocmd/config/init_messages.gocmd/config/init_messages_test.go
…ksuite#448) The interactive `config init` flow showed a QR code and verification link without indicating their relationship, leaving users unsure which to act on first and whether the link was still needed after scanning. Split the message strings on TTY vs non-TTY: - TTY: header above QR ("使用飞书 / Lark 扫码配置应用"), "或打开链接" framing to mark the link as an alternative, and an active waiting indicator. - Non-TTY (AI / piped callers via --new): keep the original copy verbatim so existing parsers and prompts are unaffected. QR is still rendered in both branches. Change-Id: I9b753f044ebefaedbb4b095cabf7beff4669eb2e
Summary
config initflow showed both a QR code and a verification link without indicating which to use first. Users were also unsure whether the link still needed to be opened after scanning. This PR splits the copy by TTY:Scan the QR code with Feishu/Lark:), the link prompt becomesOr open the link below in your browser:, and the polling indicator changes toFetching configuration results...--newvia pipe): original copy preserved verbatim so existing AI parsers and prompts are unaffectedTest plan
go build ./cmd/config/...succeedsgo test ./cmd/config/ -run 'TestGetInitMsg|TestInitMsg'all green./lark-cli config init --newvia pipe) matches the pre-change copy exactlylark-cli config init --newin a real terminal and confirm the new TTY layout (QR header, "Or" prefix on link, clearer polling indicator)Summary by CodeRabbit