Repository navigation
feat(cli): Declare a completion kind for every flag - #1759
Open
sentry-junior[bot] wants to merge 2 commits into
Open
sentry-junior[bot] wants to merge 2 commits into
sentry-junior[bot] wants to merge 2 commits into
Conversation
Every visible value-taking flag now says how shell completion handles its value, and test/lib/completion-coverage.test.ts fails, naming the command and flag, for any flag without a kind. There is no allow-list. - --fields completes the output schema's field paths, nested ones in dot notation, one comma-separated item at a time, leaving out the fields already listed (new choice-list kind, choiceListParser). - Shared parsers get kinds through PARSER_TARGETS: numberParser and sanitizeQuery complete nothing; parseCursorFlag, parseLogSort, parseSpanDepth, parseInstallationMethod, and parseReleaseChannel complete the values they accept. parseCursorFlag moves to arg-parsing.ts so completion-target.ts can import it without a cycle. - A command's own validating parser keeps its checks when wrapped: choiceParser(values, parse) completes values (sort, status, format, method, datasets), textParser(parse) completes nothing (ports, limits, thresholds). - Every parse: String flag uses a named parser: paths complete files or directories, --platform completes platforms, org list --account completes accounts, --scope/--features/--type complete their lists, --timezone completes tz names, and the rest are parseText.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
The viewer integration tests import `commands/local/server.ts`, which now reaches `lib/arg-parsing.ts` through the tagged flag parsers in `completion-target.ts`. That module imports the generated grammar, so a fresh CI checkout with only the API schema generated failed to resolve `org-project-target-grammar.js`. `ensure-generated.mjs` now takes the groups to check, and the viewer test asks for the schema and the grammar.
BYK
requested changes
Oct 30, 2026
BYK
left a comment
BYK
left a comment
Member
There was a problem hiding this comment.
Stopped at 41 files as it is apparent that you left many String parsers at parseText even though it is easy to see they are not random/free-form text. Let's do this properly.
| until: { | ||
| kind: "parsed", | ||
| parse: String, | ||
| parse: parseText, |
Member
There was a problem hiding this comment.
Based on the brief looks like this can be richer?
Comment on lines
+26
to
+32
| parse: parseText, | ||
| brief: "Native integration ID, when multiple installations match", | ||
| optional: true, | ||
| }, | ||
| app: { | ||
| kind: "parsed", | ||
| parse: String, | ||
| parse: parseText, |
Member
There was a problem hiding this comment.
Can these not be found through API calls etc?
| field: { | ||
| kind: "parsed", | ||
| parse: String, | ||
| parse: parseText, |
Member
There was a problem hiding this comment.
Looks like this actually has a strict format rather than free-form text?
| into: { | ||
| kind: "parsed", | ||
| parse: String, | ||
| parse: parseText, |
Member
There was a problem hiding this comment.
So rather than a random text, this actually needs to be an issue target?
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #1751, which did this for positionals. Now every visible value-taking flag declares how shell completion handles it, and the build fails when a new flag doesn't.
What changed
test/lib/completion-coverage.test.tsnow checks every visible, non-boolean flag on every visible command, not just positionals. A failure names the command and the flag. There is no allow-list.choice-listkind for comma-separated values.--fields id,ti<TAB>offersid,title, leaves out fields already chosen, and handles nested dot paths.buildFieldsFlagtags it once, and the generator writes each command's schema field names.PARSER_TARGETSnow covers the shared numeric, cursor, and limit parsers (text). It also covers fixed-value parsers such as sort, status, format, span depth, install method, HTTP method, and release channel. Their values come from the modules that define them; the lists are not duplicated.--platformcompletes from the platform list. Path flags complete as files or directories.--queryuses issue-query where the query is an issue search.parse: Stringis replaced with a named, tagged parser fromcompletion-target.ts. AddedtextParser(parse), andchoiceParsernow accepts an optionalparse.list-command.tstoarg-parsing.tsto avoid an import cycle.packages/cli/AGENTS.mdto cover flags.src/lib/completions.tsis untouched so it doesn't conflict with the parallel shell-script work.Visible non-boolean flags: 466. Without a kind: 447 before, 0 after.
Verification
pnpm run typecheck,pnpm run lint, andnode --test scripts/cli-lint-rules.test.mjsall pass.completions.test.ts.npx vitest run test/commands: all 3036 tests pass.Stringmakes the coverage test fail.Follow-ups
--environment,--release,--dist,--team, and bare-slug project flags. These are text for now.--fieldsflags belong to commands that have no output schema, so they fall back to text.--
View Session in Sentry