Repository navigation
feat(cli): Declare a completion kind for every positional - #1751
Conversation
No command uses `parseOrgTarget`, `parseOrgProjectTarget`, or `parseIssueTarget` since #1564 moved every target and ID positional onto bound parsers, which carry the completion kind themselves. Remove them, and drop them and the already-removed `parseDashboardTarget` from the lint rules that banned them in commands. The lint rule now bans `withCompletionTarget` in commands instead, so a command cannot recreate an identity parser that only tags the kind. The introspection tests tag their own fixture parsers. Part of #1564 Co-Authored-By: BYK <burak.kaya@sentry.io>
Every visible positional now says how shell completion handles it, and test/lib/completion-coverage.test.ts fails, naming the command and placeholder, when a new one does not. New kinds in completion-target.ts, with pre-tagged parsers commands import instead of tagging String: parseFilePath, parseDirectoryPath, parseText, parseCommandPath, parseAccountSelector, and choiceParser(values). project create's name:platform completer becomes the project-spec kind instead of a special case in getCompletions, and parseLoginEmail moves to db/accounts.ts so PARSER_TARGETS can list it. The generator also writes choice values (including enum flag values, so zsh and fish get them through __complete) and the subcommands of every route group. __complete uses them to complete nested subcommands (alert issues <TAB>) and help's command path, and lists files and directories for path arguments. Co-Authored-By: Claude <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
…ompletion # Conflicts: # packages/cli/AGENTS.md # packages/cli/src/lib/completion-target.ts # packages/cli/test/lib/introspect.test.ts
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
init reads a word as its directory only when it looks like a path (`./`, `../`, `/`, `~/`); anything else is the target. Bare `name/` suggestions from the directory completer were therefore parsed as an org target. init now completes org/project targets, and directories only once the word looks like a path, keeping that prefix. Co-Authored-By: Claude <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 37dba0d. Configure here.
init accepts at most one target and one directory, in either order. The completer now offers only a directory after a target, only a target after a directory, and nothing once both are present, so Tab never completes into a command init rejects.
…ompletion # Conflicts: # packages/cli/src/commands/api.ts Co-Authored-By: BYK <burak.kaya@sentry.io>
BYK
left a comment
BYK
left a comment
There was a problem hiding this comment.
In general, this is AMAZING work well done!
| { | ||
| placeholder: "url", | ||
| parse: String, | ||
| parse: parseText, |
There was a problem hiding this comment.
Fair, parseText hid that it's a URL. f2ee33a adds parseUrl in completion-target.ts and uses it here.
It still returns the input unchanged. issue link/unlink already validate the URL further down (parseHttpUrl in issue-links.ts, issue-integrations.ts, issue-app-links.ts) with their own ValidationErrors. Rejecting the URL in the Stricli parser instead would change both the error and the exit code (an argument parse failure exits 252), so I left that out of this PR.
| export const TEXT_TARGET = "text"; | ||
|
|
||
| /** Every completion target kind, in a stable order. */ | ||
| export const COMPLETION_TARGETS = [ |
There was a problem hiding this comment.
Is this "poor man's enum" without using enum??
I was expecting a const map of "target name" to "target completer" or something. Then we can do Object.keys() or completers.keys() and also define the type based on that easily?
There was a problem hiding this comment.
Yep, fixed in f2ee33a. COMPLETERS in complete.ts is now the only definition, written as an object literal with satisfies Record<string, Completer>, and export type CompletionTarget = keyof typeof COMPLETERS.
- The 14
*_TARGETconstants and theCOMPLETION_TARGETSarray are gone.completion-target.tsre-exports the type. - Commands pass the kind as a string literal (
boundPositional("org/project", …)), and the type checks it. A kind with no completer is now a type error. - Each constant's doc comment moved onto its completer entry.
CompletionEntityKindis now plain"issue" | "dashboard". Deriving it fromCompletionTargetmade the type circular.
The regenerated completion-targets.json is byte-identical, so completion output doesn't change.
The kinds were a list of string constants plus an array that existed only
to derive the CompletionTarget type, kept in step with COMPLETERS by hand.
COMPLETERS is now the one definition: the type is its keys, so a parser
tagged with a kind that has no completer is a type error, and commands
name kinds as checked string literals ("org/project").
The issue link/unlink URL positional parses with a named parseUrl. It
returns its input like before; link and unlink keep validating the URL
with their own errors.
The generated completion-targets.json is byte-identical.
BYK
left a comment
BYK
left a comment
There was a problem hiding this comment.
Not a fan of the new string literals spread over the old const names so would be great if we can get that back. Rest looks good.
| * The completer of every kind a parser can declare. Its keys are the | ||
| * kinds: {@link CompletionTarget} is derived from them. | ||
| */ | ||
| const COMPLETERS = { |
There was a problem hiding this comment.
Should we also do Object.freeze() and as const at the end?
There was a problem hiding this comment.
Object.freeze() is in, as of 5384964: const COMPLETERS = Object.freeze({ … } satisfies Record<string, Completer>).
I left as const off because it would change nothing here. Object.freeze already types the result as Readonly<…>, so the properties are readonly. as const only narrows literal values, and every value in this map is a function, so there is nothing for it to narrow. keyof typeof COMPLETERS still gives the same 14-member union, and I confirmed tsc rejects "bogus" satisfies CompletionTarget.
Also in 5384964, per your review: the *_TARGET names are back at every call site. Each one is now export const ORG_TARGET = "org" satisfies CompletionTarget, so COMPLETERS stays the only list of kinds, and a name without a completer fails to compile. completion-targets.json is still byte-identical.
Commands, helpers, and tests name kinds with the ORG_TARGET-style constants again instead of string literals. Each constant is checked with `satisfies CompletionTarget`, so the keys of COMPLETERS stay the only list of kinds and a name with no completer is a type error. COMPLETERS is frozen, so no code can add or swap a completer at runtime.
…sh (#1760) Follows #1751. The generated completion scripts only knew two command levels. That caused four problems: - In bash, flag names for nested commands (`alert issues list --<TAB>`, `dashboard widget add --<TAB>`) completed nothing. - Zsh completed no directories. - Fish offered a group's subcommands in later words (`event send --level <TAB>` offered `list`) and showed `\(` in descriptions. - Bash added a space after `dist/` and inserted `docs dir/` unescaped. **What changed** - `__complete` now completes flag names. When the word being completed starts with `-`, it suggests the resolved command's visible flags and aliases, booleans included, each with its brief. - Flags already given are left out, whatever spelling was used (`--limit 5`, `-n 5`, `--limit=5`), unless they're variadic. - The generator writes each command's visible flags to a new `flagNames` field. Existing fields are unchanged. - All three scripts keep their static lists of commands and subcommands. For every later word they ask `__complete`, so flags, values, paths, and nested subcommands come from the same TypeScript code in every shell, at any depth. The per-command flag and enum tables are gone from the bash and fish scripts. - **Bash** escapes suggestions with `printf %q`, keeping a leading `~/`, and removes escapes from the words it passes on. It sets `compopt -o nospace` when the only suggestion ends in `/`. It does not use `-o default`. - **Zsh** unquotes words with `(Q)` and shows descriptions through `_describe`. It adds suggestions ending in `/` with `-S ''`, so no space follows them. - **Fish** runs `string unescape` on the current token. Each group's subcommands are offered only right after the group name. **Tradeoff** In bash and fish, flag names used to come from static tables. Each flag TAB now starts a `__complete` process, as positional values already did. On the npm bundle in this sandbox, one call took about 600 ms: `node -e 0` takes about 50 ms, and requiring `dist/index.cjs` alone takes about 420 ms. I didn't measure the SEA binary. Command and subcommand TABs stay instant because those lists are still static. If the delay bothers people, the fix is a lighter entry point for `__complete`, not more static tables. **Verification** - New unit tests cover flag-name completion: nested commands, aliases, leaving out flags already given, variadic flags, and a flag typed right after a flag that takes a value. - A bash harness test sources the generated script with an `_init_completion` stub. It runs the real CLI through a `sentry` wrapper on PATH for `issue list --`, `alert issues list --li`, `sourcemap upload d` and `docs\ d` (escaping and no trailing space), and `event send --level`. - A fish test runs `complete -C`. It's skipped when fish isn't installed. - I also ran real interactive shells in a pseudo-terminal: bash 5.2 with readline, zsh 5.9 with compinit, and fish 4.9.3. I checked the line after TAB. All three insert `docs\ dir/` and `dist/` with no trailing space, and `--limit ` and `warning ` with one. Zsh isn't in the automated tests. **Known gaps** - Bash still splits words at `:` and `=` (bash-completion's default), so `project create name:plat<TAB>` and `--flag=<TAB>` don't complete in bash. - Groups with a default command (`sentry issue --<TAB>`) don't complete that command's flags. <!-- junior-request-attribution:start --> via **BYK**. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:agent%3Ad521ba1434016143aa66a46a38b3a1e5 --> -- [View Session in Sentry](https://sentry.sentry.io/explore/conversations/agent%3Ad521ba1434016143aa66a46a38b3a1e5/?project=4510944073809921) <!-- junior-session-footer:end --> Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com>
Every visible positional now declares how shell completion handles it, and positionals that can complete now do. Before this, 53 of the 126 positionals had no kind.
sentry alert issues <TAB>,sentry sourcemap upload <TAB>,sentry help <TAB>, andsentry cli completion <TAB>all returned nothing.What changed
file,directory,org-project-or-directory,command-path,choice,account,project-spec, andtext(free text, no suggestions).COMPLETERSincomplete.tsis the only list of kinds.CompletionTargetis its keys, and it is frozen.*_TARGETconstants incompletion-target.tsare each checked withsatisfies CompletionTarget, so a kind with no completer is a type error.parseFilePath,parseDirectoryPath,parseText,parseUrl,parseCommandPath,parseAccountSelector, andchoiceParser(values).String, and the ref(cli): Remove the completion-only target parsers #1750 ban onwithCompletionTargetin commands stays.choiceParserandparseUrlreturn their input unchanged, so commands keep their own validation (issue link/unlinkstill validate the URL themselves).__complete. The generator now also writes choice values, including Striclienumflag values, so zsh and fish get them through__complete. It also writes every route group's subcommands.__completecompletes nested subcommands (alert issues <TAB>,dashboard widget <TAB>) andhelp's command path./, and dotfiles show only when the partial starts with..project createuses theproject-speckind instead of thecmdPathspecial case.auth switch,auth logout, andauth logincomplete stored accounts.parseLoginEmailmoved todb/accounts.tssoPARSER_TARGETScan list it.inittakes at most one target and one directory, in either order, and completion follows the arguments already typed. A word that doesn't look like a path completesorg/projecttargets, because init reads it as the target. A path-like word (./,../,/,~/) completes directories and keeps its prefix. After a target only directories are offered, and nothing is offered once both are given.cli completioncompletes bash/zsh/fish.cli defaultscompletes its registry keys, on the first word only.cli upgradecompletes nightly/stable/undo.test/lib/completion-coverage.test.tswalks the real route tree. It fails for any visible positional without a kind and names the command and placeholder. It has no allow-list.packages/cli/AGENTS.mdexplains which parser to use for each argument type and how to add a kind or an entity.Follow-ups
textpositionals that could complete:release deployenvironment,issue tagskey,debug-files findid,monitor runslug,build downloadid,apiendpoint (from the API schema),schemaquery, and local sessions. Local sessions live only in the daemon's memory, so there is no cheap synchronous read.event sendcompletes paths only, not its optional target.String-parsed flags, numeric parsers,--fieldsfrom output schemas, and--org/--project/--platform/--environment.-o nospace/filenamesin bash, so a directory doesn't get a trailing space).via BYK.
--
View Session in Sentry