[DX-1135] Fix arg names not converting to snake_case in error output#367
[DX-1135] Fix arg names not converting to snake_case in error output#367
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Adjusts oclif init-time command metadata so argument display names consistently render as UPPER_SNAKE_CASE in USAGE/ARGUMENTS output (including when shown as part of missing-arg errors), rather than leaking camelCase arg names.
Changes:
- Remove the
GENERATING_DOC=truegate so the init hook runs on every CLI invocation. - Patch
config.commands[*].args[*].namefrom camelCase → snake_case at init time. - Expand the hook’s docstring to document the affected output paths and rationale.
Comments suppressed due to low confidence (1)
src/hooks/init/patch-arg-names.ts:29
- This change makes the init hook mutate
config.commandson every invocation; it would be good to add a hooks-project test that builds a minimalConfigwith a command arg likekeyNameand asserts the hook converts it tokey_name. That would lock in the regression fix for missing-arg USAGE/ARGUMENTS rendering.
const hook: Hook<"init"> = async function ({ config }) {
for (const command of config.commands) {
for (const arg of Object.values(command.args)) {
arg.name = camelToSnake(arg.name);
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… guard from init hook
411abf2 to
b143860
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/hooks/init/patch-arg-names.ts:22
- This behavior change (patching
config.commandsarg names on every invocation) isn't covered by tests. Consider adding a unit/integration test that runs a command with a camelCase arg (e.g.auth keys createwithout args) and asserts the USAGE/ARGUMENTS sections renderKEY_NAME(with underscore) rather thanKEYNAME, so regressions in the oclif error/help path are caught.
const hook: Hook<"init"> = async function ({ config }) {
for (const command of config.commands) {
for (const arg of Object.values(command.args)) {
arg.name = camelToSnake(arg.name);
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
WalkthroughThis PR fixes a display bug where camelCase argument names (e.g., Changes
Review Notes
|
There was a problem hiding this comment.
Review Summary
One file changed: src/hooks/init/patch-arg-names.ts — removes the GENERATING_DOC=true guard so arg-name patching runs on every CLI invocation.
Analysis
The fix is correct and safe. Key points verified:
Idempotency holds. camelToSnake uses /([a-z])([A-Z])/g substitution followed by .toLowerCase(). Applied to an already-snake_case string (e.g., key_name) it produces no changes. The init hook, CustomHelp.formatCommand(), and any subsequent call all compose safely.
The CustomHelp.formatCommand() save/restore is now a no-op in production, but still correct. After the hook runs, config.commands holds snake_case names; formatCommand saves them, re-applies the idempotent transform, renders, and restores the same snake_case values. For tests that don't invoke the hook, the save/restore still serves its original purpose — so keeping it is the right call.
Hook registration is correct. patch-arg-names is the third entry under oclif.hooks.init in package.json, running after alias-command and version-flag — fine ordering for a pure metadata patch.
The root cause is accurately described. oclif's missing-arg error path instantiates the base Help class (not CustomHelp), reads directly from config.commands, and calls .toUpperCase() on the raw arg name — so patching the metadata at init time is the correct fix, not trying to override the error renderer.
No issues to flag
This is a small, well-scoped fix. The new comment block clearly explains the before/after and the relationship with CustomHelp.formatCommand(). Ready to merge.
camelCasearg names appearing in error output when required args are missing.pnpm cli auth keys createwithout explicit--helpflagBefore:
After:
KEY_NAMEproperly insnake_case.GENERATING_DOC=true, so it only ran during doc generation — not during normal CLI usage.Context
oclif's error handler creates a base
Helpinstance (not ourCustomHelp)to render USAGE/ARGUMENTS sections alongside missing-arg errors. Since it reads
from
config.commands, the init hook is the right place to patch arg namesfor this code path.
The existing
camelToSnakepatch inCustomHelp.formatCommand()is retainedas defense in depth —
camelToSnakeis idempotent so the double-patch is harmless.