refactor(output): remove rich terminal render path#1087
refactor(output): remove rich terminal render path#1087
Conversation
jackwener
left a comment
There was a problem hiding this comment.
A组主审结论:我这边看 #1087 作为 Step A 可以 green,目前没有 blocker。
我重点看了两件事:
presentation policy是否真的从 renderer heuristic 抽到了 command metadata- 这 4 条接入的命令是否确实属于 detail,而不是把 list 误标
结论:这条边界基本立住了。现在默认 table 路径已经不再靠 rows.length === 1 猜 key/value;只有 command 显式标 presentation: "detail" 时,table human-readable 输出才走 detail layout。twitter/profile、bluesky/profile、bilibili/me、v2ex/member 这 4 条我看下来都属于单实体资料查询,标成 detail 是对的。
我本地补跑了:
node node_modules/vitest/vitest.mjs run src/output.test.ts src/registry.test.ts src/commanderAdapter.test.ts src/serialization.test.tsnpm run typecheck
一个非 blocker 的边界提醒:serializeCommand() 现在把未显式设置的命令也序列化成 presentation: "list"。这对稳定 schema 没问题,但会丢掉“作者有没有显式审过 presentation”的信息。当前 PR 目标下我不把它升成 blocker;只是后面如果要做更系统的 output audit / migration,可能要决定是否保留 explicit-vs-default 的区分。
所以我的口径是:Step A 方向正确,可以继续往 sign-off 流程走;但不要把它当 output redesign 终态。
jackwener
left a comment
There was a problem hiding this comment.
A组主审按新 head 重看完了。旧的 5fc77cc 版本 verdict 已失效;我这次是按当前 head 7f82b6f / 之前代码 head 1490579 的方向来审。
结论:我最初看到一个真实问题是 README 把范围写成了 All built-in commands ... yaml (default),但实现实际只把 registry-backed commands、opencli list 和 opencli plugin list 切到默认 yaml,像 generate 这类 built-in 仍然默认 table。这个口径不一致会把行为变更范围说大。
这个问题我已经直接补掉,推了 doc-only commit 7f82b6f:README 现在改成和实现一致的范围描述。
按修正后的 latest head,我这边没有剩余 blocker。我的 review 口径是:
- 默认
yaml的主路径已经在 registry-backed commands /list/plugin list上收敛下来 defaultFormat不再偷偷改默认主路径,行为更直接- renderer 默认复杂度没有继续扩张,rich-text formats 退回显式 opt-in
我本地补跑了新代码 head 1490579 的验证:
node node_modules/vitest/vitest.mjs run src/output.test.ts src/commanderAdapter.test.ts src/registry.test.ts src/serialization.test.tsnpm run typecheck
7f82b6f 只是 README 对齐,不影响上述代码路径。当前 CI 已因这个 doc-only follow-up 重新起跑。
jackwener
left a comment
There was a problem hiding this comment.
我更正一下我前一条对 pivot 版的 green:@First-principles-0 提的 blocker 是对的。
问题不在 renderer,而在 policy flattening:pivot 版把 registry-backed command 的默认主路径直接改成 yaml,同时把已有 defaultFormat contract 一起抹掉了。这样会把一批本来就明确声明了 plain / json 语义的命令一起压平。
我已经直接在 PR 分支上补了最小修正,最新 head 现在是 cd00a46:
- 恢复
defaultFormat对显式声明命令的默认语义 - 仍然保留这次 PR 想要的收敛:没有显式
defaultFormat的 registry-backed commands 默认走yaml opencli list/opencli plugin list继续默认yaml- README 口径同步成和实现一致
也就是现在的 contract 变成:
- structured/list 主路径默认
yaml - 已明确声明
plain/json的命令继续保留自己的 command-level contract - rich-text formats 仍然是显式 opt-in
我本地补跑了:
node node_modules/vitest/vitest.mjs run src/output.test.ts src/commanderAdapter.test.ts src/registry.test.ts src/serialization.test.tsnpm run typecheck
按 latest head cd00a46,我这边现在重新回到 A组 main review = green。
Summary
yaml,json,plain,md,csvdefaultFormatcontracts such asplainandjsonopencli listandopencli plugin liston ayamldefault while still supporting non-rich text exportsWhy
Latest product direction is clear: terminal rich rendering can go, output contracts should stay simple, and existing text/structured paths should stay intact.
The resulting contract is:
yamlValidation
node node_modules/vitest/vitest.mjs run src/output.test.ts src/commanderAdapter.test.ts src/registry.test.ts src/serialization.test.ts src/cli.test.tsnpm run typecheck