From 1745172912e033aebd39d0c18d4ee4c4ce3cc4e6 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 30 Oct 2026 20:36:02 +0000 Subject: [PATCH] feat(cli): Complete flags at every command depth in bash, zsh, and fish The generated shell scripts only knew the first two command levels, so flag names for nested commands (`alert issues list --`) completed nothing, zsh completed no paths, and fish leaked a group's subcommands into later words. Bash also added a space after a directory and inserted names with spaces unescaped. `__complete` now answers flag names: a partial starting with `-` gets the resolved command's visible flags and aliases, with their briefs, minus those already given unless they are variadic. The generator emits each command's visible flags for it. The three scripts keep their static command and subcommand lists and ask `__complete` for every later word, so all shells share one tested code path. Bash escapes suggestions with `printf %q` and sets `nospace` after a single `/` suggestion; zsh adds `/` suggestions with `-S ''` and unquotes words before passing them on; fish unescapes the current token. Fish descriptions no longer show backslashes before parentheses. --- packages/cli/AGENTS.md | 21 +- packages/cli/src/lib/complete.ts | 51 +- packages/cli/src/lib/completions.ts | 493 +++++------------- packages/cli/src/lib/introspect.ts | 34 +- packages/cli/test/lib/complete.test.ts | 43 ++ .../cli/test/lib/completions.property.test.ts | 198 +++++-- packages/cli/test/lib/completions.test.ts | 30 +- packages/cli/test/lib/introspect.test.ts | 37 +- 8 files changed, 467 insertions(+), 440 deletions(-) diff --git a/packages/cli/AGENTS.md b/packages/cli/AGENTS.md index e6cc7bb6c..b212ea208 100644 --- a/packages/cli/AGENTS.md +++ b/packages/cli/AGENTS.md @@ -421,15 +421,18 @@ parser, and an `entityCompleter` entry in `src/lib/complete.ts`. To add a kind that is not an entity, add its entry to `COMPLETERS` (`src/lib/complete.ts`), whose keys are the `CompletionTarget` type, a `*_TARGET` name for it in `completion-target.ts`, and a pre-tagged parser. -`pnpm run generate:completions` collects every command's kinds and choice -values, and every route group's subcommands, into the gitignored -`src/generated/completion-targets.json` that the `__complete` fast path -reads. Words that name a nested route group (`alert issues`) complete its -subcommands; the bash, zsh, and fish scripts embed only the first two -levels. Completers must stay light (synchronous `node:fs` reads, SQLite, -no network); ones that need a parser (`arg-parsing.ts`, the search grammar) -import it when called, so the fast path only loads it for the arguments -that need it. +`pnpm run generate:completions` collects every command's kinds, choice +values, and visible flags (with briefs and aliases), and every route group's +subcommands, into the gitignored `src/generated/completion-targets.json` +that the `__complete` fast path reads. Words that name a nested route group +(`alert issues`) complete its subcommands, and a partial starting with `-` +completes the command's flags not given yet. The bash, zsh, and fish +scripts (`src/lib/completions.ts`) embed only the command and subcommand +names and ask `__complete` for every later word, so every shell completes +every depth the same way. Completers must stay light (synchronous `node:fs` +reads, SQLite, no network); ones that need a parser (`arg-parsing.ts`, the +search grammar) import it when called, so the fast path only loads it for +the arguments that need it. ### Multiple Values in Arguments diff --git a/packages/cli/src/lib/complete.ts b/packages/cli/src/lib/complete.ts index 35a07bb43..7725bf358 100644 --- a/packages/cli/src/lib/complete.ts +++ b/packages/cli/src/lib/complete.ts @@ -137,7 +137,33 @@ type CompleterContext = { }; /** What the word being completed is, and the positionals typed before it. */ -type CompletionSlot = CompleterContext & { kind: CompletionTarget | null }; +type CompletionSlot = CompleterContext & { + kind: CompletionTarget | null; + /** The flags the word can name if it starts with `-`. */ + flagNames: Completion[]; +}; + +/** A flag's spelling without the value given with `=` (`--period=7d`). */ +const FLAG_VALUE_RE = /=.*$/s; + +/** + * The command's visible flags and their aliases, with their briefs, except + * those already in `words`. Variadic flags may be given again. + */ +function remainingFlagNames( + command: CommandCompletions[string], + words: readonly string[], +): Completion[] { + const given = new Set(words.map((word) => word.replace(FLAG_VALUE_RE, ""))); + return Object.entries(command.flagNames ?? {}).flatMap( + ([name, { brief, aliases = [], variadic }]) => { + const spellings = [name, ...aliases]; + return !variadic && spellings.some((spelling) => given.has(spelling)) + ? [] + : spellings.map((value) => ({ value, description: brief })); + }, + ); +} /** * Find the argument the word being completed fills. @@ -148,7 +174,9 @@ type CompletionSlot = CompleterContext & { kind: CompletionTarget | null }; * `--period=7d` is a single word. Other `-`-prefixed words are flags, and * like Stricli's scanner, a flag ends the one before it instead of becoming * its value: an `inferEmpty` flag such as `--time-series` then takes an - * empty value, and any other flag would be a usage error anyway. + * empty value, and any other flag would be a usage error anyway. For the + * same reason, a word starting with `-` names a flag even after one that + * takes a value. * * @returns The slot, or `undefined` when the words name no command with * completion metadata @@ -170,11 +198,13 @@ function findSlot(words: string[]): CompletionSlot | undefined { positionals.push(word); } } + const flagNames = remainingFlagNames(command, words.slice(length)); if (flag !== undefined) { return { kind: command.flags[flag] ?? null, positionals, choices: command.choices?.[flag], + flagNames, }; } // A variadic positional is a single parameter that every word fills. @@ -185,7 +215,12 @@ function findSlot(words: string[]): CompletionSlot | undefined { command.variadic && positionals.length > 0 ? undefined : command.choices?.[index]; - return { kind: command.positionals[index] ?? null, positionals, choices }; + return { + kind: command.positionals[index] ?? null, + positionals, + choices, + flagNames, + }; } return; } @@ -393,9 +428,10 @@ function completePeriods(partial: string): Completion[] { * Determine what completions to provide based on the command context. * * Finds the argument the partial fills (a positional or a flag value) and - * runs the completer for the kind its parser declares. Words that name a - * nested route group (`alert issues`) complete its subcommands instead; - * the shell scripts embed the first two levels themselves. + * runs the completer for the kind its parser declares. A partial starting + * with `-` completes the command's flag names instead. Words that name a + * nested route group (`alert issues`) complete its subcommands; the shell + * scripts embed the first two levels themselves. * * @param precedingWords - Words before the partial (determines context) * @param partial - The current partial word being completed @@ -410,6 +446,9 @@ export async function getCompletions( ? completeSubcommands(precedingWords.join(" "), partial) : []; } + if (partial.startsWith("-")) { + return slot.flagNames.filter(({ value }) => value.startsWith(partial)); + } return slot.kind ? await COMPLETERS[slot.kind](partial, slot) : []; } diff --git a/packages/cli/src/lib/completions.ts b/packages/cli/src/lib/completions.ts index 2cb92ab7e..7e9e85fee 100644 --- a/packages/cli/src/lib/completions.ts +++ b/packages/cli/src/lib/completions.ts @@ -5,18 +5,19 @@ * When commands are added or removed, completions update automatically. * * Uses a hybrid approach: - * - **Static**: command/subcommand names, flag names, enum flag values - * are embedded in the shell script for instant tab completion. - * - **Dynamic**: positional arg values (org slugs, project names, aliases) - * are completed by calling `sentry __complete` at runtime, which reads - * the SQLite cache with fuzzy matching. + * - **Static**: command and subcommand names are embedded in the shell + * script, so the first two words complete without starting the CLI. + * - **Dynamic**: everything after them (flag names, flag values, + * positionals, nested subcommands) comes from `sentry __complete` + * (`complete.ts`), so every shell completes every command depth the + * same way. */ import { existsSync, mkdirSync } from "node:fs"; import { writeFile } from "node:fs/promises"; import { dirname, join } from "node:path"; import { routes } from "../app.js"; -import { type FlagDef, isCommand, isRouteMap } from "./introspect.js"; +import { isRouteMap } from "./introspect.js"; import type { ShellType } from "./shell.js"; /** Where completions are installed */ @@ -27,27 +28,14 @@ export type CompletionLocation = { created: boolean; }; -/** Flag metadata for shell completion */ -type FlagEntry = { - /** Flag name without `--` prefix (e.g., "limit") */ - name: string; - /** Human-readable description */ - brief: string; - /** For enum flags, the valid values */ - enumValues?: string[]; -}; - -/** A command with its description and flags */ +/** A command or route group with its description */ type CommandEntry = { name: string; brief: string; - flags: FlagEntry[]; }; /** A command group (route map) containing subcommands */ -type CommandGroup = { - name: string; - brief: string; +type CommandGroup = CommandEntry & { subcommands: CommandEntry[]; }; @@ -60,72 +48,8 @@ export type CommandTree = { }; /** - * Extract flags from a Stricli command's parameters. - * - * Filters out hidden flags (like --log-level, --verbose) and returns - * metadata needed for completion. - */ -function extractFlagEntries( - flags: Record | undefined, -): FlagEntry[] { - if (!flags) { - return []; - } - - const entries: FlagEntry[] = []; - for (const [name, def] of Object.entries(flags)) { - if (def.hidden) { - continue; - } - const entry: FlagEntry = { name, brief: def.brief ?? "" }; - if (def.kind === "enum" && def.values) { - entry.enumValues = [...def.values]; - } - entries.push(entry); - } - return entries; -} - -/** - * Build a CommandEntry from a route target with its name. - * Extracts flags if the target is a command. - */ -function buildCommandEntry(name: string, target: unknown): CommandEntry { - const flags = isCommand(target) - ? extractFlagEntries(target.parameters.flags) - : []; - return { - name, - brief: (target as { brief: string }).brief, - flags, - }; -} - -/** - * Extract subcommands from a route map group. - */ -function extractSubcommands(routeMap: { - getAllEntries: () => readonly { - name: { original: string }; - target: unknown; - hidden: boolean; - }[]; -}): CommandEntry[] { - const subcommands: CommandEntry[] = []; - for (const sub of routeMap.getAllEntries()) { - if (!sub.hidden) { - subcommands.push(buildCommandEntry(sub.name.original, sub.target)); - } - } - return subcommands; -} - -/** - * Extract the command tree from the Stricli route map. - * - * Walks the route map recursively to build a structured command tree - * that can be used to generate completion scripts for any shell. - * Includes flag metadata for each command. + * Extract the first two levels of the Stricli route map: top-level + * commands and groups, and the visible subcommands of each group. */ export function extractCommandTree(): CommandTree { const groups: CommandGroup[] = []; @@ -142,10 +66,13 @@ export function extractCommandTree(): CommandTree { groups.push({ name, brief: entry.target.brief, - subcommands: extractSubcommands(entry.target), + subcommands: entry.target + .getAllEntries() + .filter((sub) => !sub.hidden) + .map((sub) => ({ name: sub.name.original, brief: sub.target.brief })), }); } else { - standalone.push(buildCommandEntry(name, entry.target)); + standalone.push({ name, brief: entry.target.brief }); } } @@ -157,85 +84,23 @@ export function extractCommandTree(): CommandTree { * * Uses an allowlist approach — replaces any character that is not a valid * shell identifier character (`[a-zA-Z0-9_]`) with an underscore. - * Input comes from Stricli route/flag names (alphanumeric + hyphen), - * but the whitelist guards against unexpected characters. + * Input comes from Stricli route names (alphanumeric + hyphen), but the + * allowlist guards against unexpected characters. */ function shellVarName(s: string): string { return s.replace(/[^a-zA-Z0-9_]/g, "_"); } -/** Collected flag and enum variable declarations for bash scripts. */ -type BashFlagVars = { - flagVarLines: string[]; - enumVarLines: string[]; - enumFlagNames: string[]; -}; - -/** - * Collect bash variable lines for a single command's flags and enum values. - * - * @param prefix - Variable name prefix (e.g., "issue_list" or "api") - * @param cmd - The command entry to process - * @param out - Accumulator for flag/enum variable lines - */ -function collectCommandFlagVars( - prefix: string, - cmd: CommandEntry, - out: BashFlagVars, -): void { - if (cmd.flags.length > 0) { - const flagNames = cmd.flags.map((f) => `--${f.name}`).join(" "); - out.flagVarLines.push(` local ${prefix}_flags="${flagNames}"`); - } - for (const flag of cmd.flags) { - if (flag.enumValues && flag.enumValues.length > 0) { - const varName = `${prefix}_${shellVarName(flag.name)}_values`; - out.enumVarLines.push( - ` local ${varName}="${flag.enumValues.join(" ")}"`, - ); - out.enumFlagNames.push(`--${flag.name}`); - } - } -} - -/** - * Build bash variable declarations for command flags and enum values. - * - * Extracted from generateBashCompletion to reduce complexity. - */ -function buildBashFlagVars(tree: CommandTree): BashFlagVars { - const out: BashFlagVars = { - flagVarLines: [], - enumVarLines: [], - enumFlagNames: [], - }; - - for (const g of tree.groups) { - for (const sub of g.subcommands) { - const prefix = `${shellVarName(g.name)}_${shellVarName(sub.name)}`; - collectCommandFlagVars(prefix, sub, out); - } - } - - for (const cmd of tree.standalone) { - collectCommandFlagVars(shellVarName(cmd.name), cmd, out); - } - - return out; -} - /** * Generate bash completion script. * - * Includes: - * - Static command/subcommand word lists - * - Static flag name lists per command - * - Static enum values per flag - * - Dynamic `sentry __complete` callback for positional values + * Completes the command and a group's subcommand from static word lists, + * and every later word with `sentry __complete`. Suggestions are escaped + * for the shell, and a single suggestion ending in `/` (a directory, + * `org/`) takes no trailing space so the next TAB continues it. */ export function generateBashCompletion(binaryName: string): string { - const tree = extractCommandTree(); - const { groups, standalone } = tree; + const { groups, standalone } = extractCommandTree(); const allTopLevel = [ ...groups.map((g) => g.name), @@ -249,34 +114,6 @@ export function generateBashCompletion(binaryName: string): string { }) .join("\n"); - const { flagVarLines, enumVarLines, enumFlagNames } = buildBashFlagVars(tree); - - const caseBranches = groups - .map( - (g) => - ` ${g.name})\n COMPREPLY=($(compgen -W "\${${shellVarName(g.name)}_commands}" -- "\${cur}"))\n ;;`, - ) - .join("\n"); - - const uniqueEnumFlags = [...new Set(enumFlagNames)]; - const enumFlagWords = uniqueEnumFlags.join(" "); - - // Build the enum flag test only if there are any enum flags - const enumBranch = enumFlagWords - ? `elif [[ " ${enumFlagWords} " == *" \${prev} "* ]]; then - # Enum value completion (static) - local val_var="$(__${binaryName}_varname "\${cmd}")_$(__${binaryName}_varname "\${subcmd}")_$(__${binaryName}_varname "\${prev#--}")_values" - if [[ -n "\${!val_var+x}" ]]; then - COMPREPLY=($(compgen -W "\${!val_var}" -- "\${cur}")) - else - # Fallback for standalone commands (subcmd is a flag, not a subcommand) - local standalone_val_var="$(__${binaryName}_varname "\${cmd}")_$(__${binaryName}_varname "\${prev#--}")_values" - if [[ -n "\${!standalone_val_var+x}" ]]; then - COMPREPLY=($(compgen -W "\${!standalone_val_var}" -- "\${cur}")) - fi - fi` - : ""; - return `# bash completion for ${binaryName} # Auto-generated from command definitions @@ -284,74 +121,59 @@ export function generateBashCompletion(binaryName: string): string { # Mirrors shellVarName() in completions.ts — replaces non-identifier chars. __${binaryName}_varname() { echo "\${1//[^a-zA-Z0-9_]/_}"; } +# Set variable $1 to word $2 without the backslash escapes readline keeps +# in it (\`docs\\ dir/\` → \`docs dir/\`). +__${binaryName}_dequote() { + local rest="$2" out="" + while [[ "\${rest}" == *'\\'* ]]; do + out+="\${rest%%'\\'*}" + rest="\${rest#*'\\'}" + out+="\${rest:0:1}" + rest="\${rest:1}" + done + printf -v "$1" '%s' "\${out}\${rest}" +} + _${binaryName}_completions() { local cur prev words cword _init_completion || return local commands="${allTopLevel.join(" ")}" ${subVars} -${flagVarLines.join("\n")} -${enumVarLines.join("\n")} - - case "\${COMP_CWORD}" in - 1) - COMPREPLY=($(compgen -W "\${commands}" -- "\${cur}")) - ;; - 2) - case "\${prev}" in -${caseBranches} - *) - # Standalone command: check for flags at position 2 - if [[ "\${cur}" == --* ]]; then - local standalone_var="$(__${binaryName}_varname "\${prev}")_flags" - if [[ -n "\${!standalone_var+x}" ]]; then - COMPREPLY=($(compgen -W "\${!standalone_var}" -- "\${cur}")) - fi - else - # Dynamic value completion for standalone commands - local IFS=$'\\n' - COMPREPLY=($(${binaryName} __complete "\${COMP_WORDS[@]:1}" 2>/dev/null)) - if [[ \${#COMPREPLY[@]} -gt 0 ]]; then - local i - for i in "\${!COMPREPLY[@]}"; do - COMPREPLY[$i]="\${COMPREPLY[$i]%%$'\\t'*}" - done - fi - fi - ;; - esac - ;; - *) - local cmd="\${COMP_WORDS[1]}" - local subcmd="\${COMP_WORDS[2]}" - - if [[ "\${cur}" == --* ]]; then - # Flag name completion (static) - local cmd_var="$(__${binaryName}_varname "\${cmd}")_$(__${binaryName}_varname "\${subcmd}")_flags" - if [[ -n "\${!cmd_var+x}" ]]; then - COMPREPLY=($(compgen -W "\${!cmd_var}" -- "\${cur}")) - else - # Try standalone command flags - local standalone_var="$(__${binaryName}_varname "\${cmd}")_flags" - if [[ -n "\${!standalone_var+x}" ]]; then - COMPREPLY=($(compgen -W "\${!standalone_var}" -- "\${cur}")) - fi - fi - ${enumBranch ? `${enumBranch}\n ` : ""}else - # Dynamic value completion (from cache, with fuzzy matching) - # The binary returns only relevant matches (prefix + fuzzy). - local IFS=$'\\n' - COMPREPLY=($(${binaryName} __complete "\${COMP_WORDS[@]:1}" 2>/dev/null)) - # Strip tab-separated descriptions (bash doesn't support them) - if [[ \${#COMPREPLY[@]} -gt 0 ]]; then - local i - for i in "\${!COMPREPLY[@]}"; do - COMPREPLY[$i]="\${COMPREPLY[$i]%%$'\\t'*}" - done - fi - fi - ;; - esac + + if [[ \${cword} -eq 1 ]]; then + COMPREPLY=($(compgen -W "\${commands}" -- "\${cur}")) + return + fi + local group_var="$(__${binaryName}_varname "\${words[1]}")_commands" + if [[ \${cword} -eq 2 && "\${cur}" != -* && -n "\${!group_var+x}" ]]; then + COMPREPLY=($(compgen -W "\${!group_var}" -- "\${cur}")) + return + fi + + # Flags, flag values, positionals, and nested subcommands come from the + # binary, one \`valuedescription\` per line. + local word partial line + local -a args=() + for word in "\${words[@]:1:cword-1}"; do + __${binaryName}_dequote word "\${word}" + args+=("\${word}") + done + __${binaryName}_dequote partial "\${cur}" + COMPREPLY=() + while IFS= read -r line; do + line="\${line%%$'\\t'*}" + # Escape spaces and shell characters, keeping a leading ~/ expandable + if [[ "\${line}" == "~/"?* ]]; then + printf -v line '~/%q' "\${line:2}" + else + printf -v line '%q' "\${line}" + fi + COMPREPLY+=("\${line}") + done < <(${binaryName} __complete "\${args[@]}" "\${partial}" 2>/dev/null) + if [[ \${#COMPREPLY[@]} -eq 1 && "\${COMPREPLY[0]}" == */ ]]; then + compopt -o nospace + fi } complete -F _${binaryName}_completions ${binaryName} @@ -361,8 +183,9 @@ complete -F _${binaryName}_completions ${binaryName} /** * Generate zsh completion script. * - * Includes static commands/subcommands, flag specs, and a dynamic - * completer that calls `sentry __complete` for positional values. + * Completes the command and a group's subcommand from static arrays, and + * every later word with `sentry __complete`, showing its descriptions. + * Suggestions ending in `/` (directories, `org/`) take no trailing space. */ export function generateZshCompletion(binaryName: string): string { const { groups, standalone } = extractCommandTree(); @@ -383,69 +206,44 @@ export function generateZshCompletion(binaryName: string): string { }) .join("\n\n"); - // Build case branches - const caseBranches = groups - .map( - (g) => - ` ${g.name})\n _describe -t commands '${g.name} command' ${shellVarName(g.name)}_commands\n ;;`, - ) - .join("\n"); - return `#compdef ${binaryName} # zsh completion for ${binaryName} # Auto-generated from command definitions _${binaryName}() { local -a commands - local curcontext="$curcontext" state line commands=( ${topLevelItems} ) ${subArrays} - _arguments -C \\ - '1: :->command' \\ - '2: :->subcommand' \\ - '*::arg:->args' - - case "$state" in - command) - _describe -t commands 'command' commands - ;; - subcommand) - case "$line[1]" in -${caseBranches} - *) - # Standalone command — delegate to dynamic completion - ;; - esac - ;| - args|subcommand) - # Dynamic completion for positional args (org slugs, project names) - # In the args state, $line[1] and $line[2] hold the parsed command - # and subcommand. Pass them to __complete for context detection. - local -a completions - local escaped_value - while IFS=$'\\t' read -r value desc; do - escaped_value="\${value//:/\\\\:}" - if [[ -n "$desc" ]]; then - completions+=("\${escaped_value}:\${desc}") - else - completions+=("\${escaped_value}") - fi - done < <( - if (( \${#words} == 0 )); then - ${binaryName} __complete "$line[1]" "$line[2]" "" 2>/dev/null - else - ${binaryName} __complete "$line[1]" "$line[2]" "\${words[@]}" 2>/dev/null - fi - ) - if (( \${#completions} )); then - _describe -t values 'value' completions - fi - ;; - esac + if (( CURRENT == 2 )); then + _describe -t commands 'command' commands + return + fi + local group="\${words[2]//[^a-zA-Z0-9_]/_}_commands" + if (( CURRENT == 3 )) && [[ $PREFIX != -* ]] && (( \${+parameters[$group]} )); then + _describe -t commands "$words[2] command" "$group" + return + fi + + # Flags, flag values, positionals, and nested subcommands come from the + # binary, one \`valuedescription\` per line. Unquote the words, since + # zsh keeps them as typed (\`docs\\ dir/\`). + local -a values slashed + local value desc ret=1 + while IFS=$'\\t' read -r value desc; do + value="\${value//:/\\\\:}" + if [[ $value == */ ]]; then + slashed+=("\${value}\${desc:+:$desc}") + else + values+=("\${value}\${desc:+:$desc}") + fi + done < <(${binaryName} __complete "\${(@Q)words[2,CURRENT-1]}" "\${(Q)PREFIX}" 2>/dev/null) + (( \${#values} )) && _describe -t values 'value' values && ret=0 + (( \${#slashed} )) && _describe -t values 'value' slashed -S '' && ret=0 + return ret } _${binaryName} @@ -455,23 +253,21 @@ _${binaryName} /** * Generate fish completion script. * - * Includes static commands/subcommands and flag completions, plus a - * dynamic completer for positional values. + * Completes the command and a group's subcommand from static completions, + * and every later token with `sentry __complete`, whose tab-separated + * descriptions fish shows as they are. */ export function generateFishCompletion(binaryName: string): string { const { groups, standalone } = extractCommandTree(); + const groupNames = groups.map((g) => g.name).join(" "); // Top-level command completions - const topLevelLines = [ - ...groups.map( - (g) => - `complete -c ${binaryName} -n "__fish_use_subcommand" -a "${g.name}" -d "${escapeDblQuote(g.brief)}"`, - ), - ...standalone.map( - (s) => - `complete -c ${binaryName} -n "__fish_use_subcommand" -a "${s.name}" -d "${escapeDblQuote(s.brief)}"`, - ), - ].join("\n"); + const topLevelLines = [...groups, ...standalone] + .map( + (c) => + `complete -c ${binaryName} -n "__fish_use_subcommand" -a "${c.name}" -d "${escapeDblQuote(c.brief)}"`, + ) + .join("\n"); // Subcommand completions const subLines = groups @@ -479,65 +275,45 @@ export function generateFishCompletion(binaryName: string): string { const cmdLines = g.subcommands .map( (s) => - `complete -c ${binaryName} -n "__fish_seen_subcommand_from ${g.name}" -a "${s.name}" -d "${escapeDblQuote(s.brief)}"`, + `complete -c ${binaryName} -n "__${binaryName}_group ${g.name}" -a "${s.name}" -d "${escapeDblQuote(s.brief)}"`, ) .join("\n"); - - // Flag completions per subcommand - const flagLines = g.subcommands - .flatMap((s) => - s.flags.map( - (f) => - `complete -c ${binaryName} -n "__fish_seen_subcommand_from ${g.name}; and __fish_seen_subcommand_from ${s.name}" -l "${f.name}" -d "${escapeDblQuote(f.brief)}"`, - ), - ) - .join("\n"); - - return `\n# ${g.name} subcommands\n${cmdLines}${flagLines ? `\n${flagLines}` : ""}`; - }) - .join("\n"); - - // Flag completions for standalone commands (e.g., `sentry api --method`) - const standaloneFlagLines = standalone - .filter((cmd) => cmd.flags.length > 0) - .map((cmd) => { - const flags = cmd.flags - .map( - (f) => - `complete -c ${binaryName} -n "__fish_seen_subcommand_from ${cmd.name}" -l "${f.name}" -d "${escapeDblQuote(f.brief)}"`, - ) - .join("\n"); - return `\n# ${cmd.name} flags\n${flags}`; + return `\n# ${g.name} subcommands\n${cmdLines}`; }) .join("\n"); return `# fish completion for ${binaryName} # Auto-generated from command definitions -# Disable file completion by default +# Disable file completion by default; \`__complete\` lists paths for the +# arguments that take them complete -c ${binaryName} -f -# Dynamic completion for positional values +# Succeed when the token being completed is the subcommand of one of the +# groups given as arguments +function __${binaryName}_group + set -l tokens (commandline -opc) + test (count $tokens) -eq 2 + and not string match -q -- '-*' (commandline -ct) + and contains -- $tokens[2] $argv +end + +# Flags, flag values, positionals, and nested subcommands function __${binaryName}_complete_dynamic - # commandline -opc: all tokens before cursor (excludes current partial) - # commandline -ct: the current token being completed (the partial) + # commandline -opc: the tokens before the cursor, unescaped + # commandline -ct: the token being completed, as typed set -l preceding (commandline -opc) - set -l current (commandline -ct) # Quote $current so an empty partial (TAB after space) is passed as "" # instead of being silently dropped by fish's empty-list expansion. - ${binaryName} __complete $preceding[2..] "$current" 2>/dev/null | while read -l line - echo $line - end + set -l current (commandline -ct | string unescape) + ${binaryName} __complete $preceding[2..] "$current" 2>/dev/null end # Top-level commands ${topLevelLines} ${subLines} -${standaloneFlagLines} -# Dynamic completions (org slugs, project names) -complete -c ${binaryName} -n "not __fish_use_subcommand" -a "" -k -complete -c ${binaryName} -a "(__${binaryName}_complete_dynamic)" +complete -c ${binaryName} -n "not __fish_use_subcommand; and not __${binaryName}_group ${groupNames}" -a "(__${binaryName}_complete_dynamic)" `; } @@ -547,18 +323,11 @@ function escapeSingleQuote(s: string): string { } /** - * Escape characters special in double-quoted shell strings. - * - * Handles bash (`\`, `$`, `"`) and fish (`\`, `$`, `"`, `(`, `)`) - * since parentheses trigger command substitution in fish double quotes. + * Escape the characters special in fish double-quoted strings (`\`, `$`, + * `"`). Fish keeps any other backslash, so parentheses stay as they are. */ function escapeDblQuote(s: string): string { - return s - .replace(/\\/g, "\\\\") - .replace(/\$/g, "\\$") - .replace(/"/g, '\\"') - .replace(/\(/g, "\\(") - .replace(/\)/g, "\\)"); + return s.replace(/\\/g, "\\\\").replace(/\$/g, "\\$").replace(/"/g, '\\"'); } /** diff --git a/packages/cli/src/lib/introspect.ts b/packages/cli/src/lib/introspect.ts index 822b16d1e..af3b65a72 100644 --- a/packages/cli/src/lib/introspect.ts +++ b/packages/cli/src/lib/introspect.ts @@ -426,6 +426,20 @@ type CommandCompletion = { * flag spelling (`"--format"`). Omitted when the command has none. */ choices?: Record; + /** + * Every visible flag by its `--name` spelling, booleans included, for + * completing flag names. Omitted when the command has none. + */ + flagNames?: Record; +}; + +/** What flag-name completion shows for one flag. */ +type FlagName = { + brief: string; + /** Its one-letter spellings (`-n`). */ + aliases?: string[]; + /** Whether it may be given more than once. */ + variadic?: true; }; /** {@link CommandCompletion} per command path (without the `sentry` prefix). */ @@ -481,6 +495,15 @@ function getCommandCompletion(command: Command): CommandCompletion { } } } + const flagNames: Record = {}; + for (const [name, def] of Object.entries(flags)) { + if (!def.hidden) { + flagNames[`--${name}`] = { + brief: def.brief ?? "", + ...(def.variadic ? { variadic: true } : {}), + }; + } + } for (const [alias, name] of Object.entries(aliases)) { const kind = flagKinds[`--${name}`]; if (kind !== undefined) { @@ -490,6 +513,10 @@ function getCommandCompletion(command: Command): CommandCompletion { if (values) { choices[`-${alias}`] = values; } + const flagName = flagNames[`--${name}`]; + if (flagName) { + flagName.aliases = [...(flagName.aliases ?? []), `-${alias}`].sort(); + } } return { positionals, @@ -498,12 +525,15 @@ function getCommandCompletion(command: Command): CommandCompletion { ...(Object.keys(choices).length > 0 ? { choices: sortedByKey(choices) } : {}), + ...(Object.keys(flagNames).length > 0 + ? { flagNames: sortedByKey(flagNames) } + : {}), }; } /** * Collect the completion metadata of every visible command that completes - * at least one positional or flag value. + * at least one positional or flag value, or has a visible flag. * * Paths are space-separated route segments without the binary name (e.g. * `"issue list"`); nested groups contribute their full path. Sorted for @@ -531,7 +561,7 @@ export function extractCommandCompletions( ...completion.positionals, ...Object.values(completion.flags), ]; - if (kinds.some((kind) => kind !== null)) { + if (completion.flagNames || kinds.some((kind) => kind !== null)) { result[segments.join(" ")] = completion; } } diff --git a/packages/cli/test/lib/complete.test.ts b/packages/cli/test/lib/complete.test.ts index acb2036fc..94f4efac9 100644 --- a/packages/cli/test/lib/complete.test.ts +++ b/packages/cli/test/lib/complete.test.ts @@ -851,6 +851,49 @@ describe("getCompletions: command paths", () => { }); }); +describe("getCompletions: flag names", () => { + test("a partial starting with - completes the command's flags with their briefs", async () => { + expect(await getCompletions(["issue", "list"], "--li")).toEqual([ + { value: "--limit", description: "Maximum number of issues to list" }, + ]); + const all = await values(["issue", "list"], "-"); + // Booleans and aliases too, but not hidden flags + expect(all).toEqual(expect.arrayContaining(["--json", "--limit", "-n"])); + expect(all).not.toContain("--log-level"); + expect(await values(["issue", "list"], "--")).not.toContain("-n"); + }); + + test("nested commands complete their own flags", async () => { + expect(await values(["alert", "issues", "list"], "--li")).toEqual([ + "--limit", + ]); + expect(await values(["dashboard", "widget", "add"], "--q")).toEqual([ + "--query", + ]); + }); + + test("flags already given are left out, by any spelling", async () => { + for (const given of [["--limit", "5"], ["-n", "5"], ["--limit=5"]]) { + const remaining = await values(["issue", "list", ...given], "-"); + expect(remaining).not.toContain("--limit"); + expect(remaining).not.toContain("-n"); + } + expect(await values(["issue", "list", "--json"], "--j")).toEqual([]); + }); + + test("variadic flags may be given again", async () => { + expect( + await values(["alert", "issues", "create", "--action", "{}"], "--act"), + ).toEqual(["--action"]); + }); + + test("a flag after one that takes a value is a flag, not its value", async () => { + expect(await values(["issue", "list", "--limit"], "--j")).toEqual([ + "--json", + ]); + }); +}); + describe("getCompletions: accounts", () => { function addAccount(host: string, email: string, userId: string): void { setAuthToken(`token-${userId}-${host}`, undefined, undefined, { diff --git a/packages/cli/test/lib/completions.property.test.ts b/packages/cli/test/lib/completions.property.test.ts index fbb5a7ec6..56b3c3d89 100644 --- a/packages/cli/test/lib/completions.property.test.ts +++ b/packages/cli/test/lib/completions.property.test.ts @@ -11,11 +11,13 @@ */ import { spawnSync } from "node:child_process"; -import { writeFileSync } from "node:fs"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { createRequire } from "node:module"; +import { tmpdir } from "node:os"; import { join } from "node:path"; import { proposeCompletions } from "@stricli/core"; import { constantFrom, assert as fcAssert, property } from "fast-check"; -import { describe, expect, test } from "vitest"; +import { afterAll, beforeAll, describe, expect, test } from "vitest"; import { app, routes } from "../../src/app.js"; import generatedTargets from "../../src/generated/completion-targets.json"; import { @@ -271,37 +273,11 @@ describe("proposeCompletions: Stricli integration", () => { }); }); -describe("property: flag extraction", () => { - const tree = extractCommandTree(); - - test("flags are non-empty arrays of objects with name and brief", () => { - for (const group of tree.groups) { - for (const sub of group.subcommands) { - for (const flag of sub.flags) { - expect(flag.name.length).toBeGreaterThan(0); - expect(typeof flag.brief).toBe("string"); - } - } - } - }); - - test("no duplicate flag names within a command", () => { - for (const group of tree.groups) { - for (const sub of group.subcommands) { - const names = sub.flags.map((f) => f.name); - expect(new Set(names).size).toBe(names.length); - } - } - }); - +describe("property: flag names", () => { test("hidden flags are excluded", () => { - for (const group of tree.groups) { - for (const sub of group.subcommands) { - // log-level and verbose are hidden — should not appear - const names = sub.flags.map((f) => f.name); - expect(names).not.toContain("log-level"); - expect(names).not.toContain("verbose"); - } + for (const command of Object.values(generatedTargets.commands)) { + // The global --log-level is hidden; the parser still accepts it + expect(command.flagNames).not.toHaveProperty("--log-level"); } }); }); @@ -323,6 +299,18 @@ describe("property: dynamic completion callback", () => { }); }); +/** + * Stands in for bash-completion's `_init_completion`, which is not + * installed everywhere: the words split as bash split them. + */ +const INIT_COMPLETION_STUB = `_init_completion() { + cur="\${COMP_WORDS[COMP_CWORD]}" + prev="\${COMP_WORDS[COMP_CWORD-1]}" + words=("\${COMP_WORDS[@]}") + cword=$COMP_CWORD + return 0 +}`; + describe("bash completion: real shell simulation", () => { test("top-level completion returns all known commands", async () => { const tree = extractCommandTree(); @@ -337,11 +325,7 @@ describe("bash completion: real shell simulation", () => { "-c", ` # Stub _init_completion (not available outside bash-completion package) -_init_completion() { - cur="\${COMP_WORDS[COMP_CWORD]}" - prev="\${COMP_WORDS[COMP_CWORD-1]}" - return 0 -} +${INIT_COMPLETION_STUB} source "${tmpScript}" COMP_WORDS=(sentry "") @@ -378,11 +362,7 @@ echo "\${COMPREPLY[*]}" [ "-c", ` -_init_completion() { - cur="\${COMP_WORDS[COMP_CWORD]}" - prev="\${COMP_WORDS[COMP_CWORD-1]}" - return 0 -} +${INIT_COMPLETION_STUB} source "${tmpScript}" COMP_WORDS=(sentry "${group.name}" "") @@ -402,6 +382,140 @@ echo "\${COMPREPLY[*]}" }); }); +// Each completion starts the CLI from source +describe( + "shell scripts: completion through __complete", + { timeout: 30_000 }, + () => { + const cliDir = join(import.meta.dirname, "../.."); + const hasFish = spawnSync("fish", ["--version"]).status === 0; + let root: string; + let env: NodeJS.ProcessEnv; + let workDir: string; + + beforeAll(() => { + root = mkdtempSync(join(tmpdir(), "sentry-shell-completion-")); + // A `sentry` on PATH that runs this checkout, from any directory + const binDir = join(root, "bin"); + mkdirSync(binDir); + const tsx = createRequire(import.meta.url).resolve("tsx/esm"); + writeFileSync( + join(binDir, "sentry"), + `#!/bin/sh\nexec "${process.execPath}" --import "${tsx}" --import "${join(cliDir, "script/require-shim.mjs")}" "${join(cliDir, "src/bin.ts")}" "$@"\n`, + { mode: 0o755 }, + ); + workDir = join(root, "work"); + mkdirSync(join(workDir, "dist"), { recursive: true }); + mkdirSync(join(workDir, "docs dir")); + writeFileSync( + join(root, "sentry.bash"), + generateBashCompletion("sentry"), + ); + writeFileSync( + join(root, "sentry.fish"), + generateFishCompletion("sentry"), + ); + env = { + ...process.env, + PATH: `${binDir}:${process.env.PATH}`, + SENTRY_CONFIG_DIR: join(root, "config"), + }; + }); + + afterAll(() => { + rmSync(root, { recursive: true, force: true }); + }); + + /** + * Run the bash completion function for the words after `sentry`, the + * last being completed, as bash would on TAB. + * + * @returns What COMPREPLY holds, and whether bash would add no space + */ + function bashComplete(...words: string[]) { + const result = spawnSync( + "bash", + [ + "-c", + `${INIT_COMPLETION_STUB} +compopt() { echo "compopt $*"; } +source "$0" +COMP_WORDS=(sentry "$@") +COMP_CWORD=$# +_sentry_completions +printf 'reply %s\\n' "\${COMPREPLY[@]}"`, + join(root, "sentry.bash"), + ...words, + ], + { cwd: workDir, env, encoding: "utf8" }, + ); + const lines = result.stdout.split("\n"); + return { + replies: lines + .filter((line) => line.startsWith("reply ") && line !== "reply ") + .map((line) => line.slice("reply ".length)), + nospace: lines.includes("compopt -o nospace"), + }; + } + + test("bash completes flag names at every depth", () => { + const issueList = bashComplete("issue", "list", "--"); + expect(issueList.replies).toContain("--limit"); + expect(issueList.replies).toContain("--json"); + expect(bashComplete("alert", "issues", "list", "--li").replies).toEqual([ + "--limit", + ]); + }); + + test("bash completes directories without a trailing space, escaped", () => { + expect(bashComplete("sourcemap", "upload", "d").replies).toEqual([ + "dist/", + "docs\\ dir/", + ]); + expect(bashComplete("sourcemap", "upload", "docs\\ d")).toEqual({ + replies: ["docs\\ dir/"], + nospace: true, + }); + }); + + test("bash completes enum flag values", () => { + expect(bashComplete("event", "send", "--level", "").replies).toEqual([ + "debug", + "info", + "warning", + "error", + "fatal", + ]); + }); + + test.skipIf(!hasFish)( + "fish completes flags with descriptions, values, and nested subcommands", + () => { + const fishComplete = (line: string) => + spawnSync( + "fish", + [ + "--no-config", + "-c", + 'source $argv[1]; complete -C "$argv[2]"', + join(root, "sentry.fish"), + line, + ], + { cwd: workDir, env, encoding: "utf8" }, + ).stdout.trim(); + expect(fishComplete("sentry alert issues list --li")).toBe( + "--limit\tMaximum number of issue alert rules to list", + ); + expect(fishComplete("sentry event send --level wa")).toBe("warning"); + expect(fishComplete("sentry sourcemap upload docs")).toBe("docs dir/"); + expect(fishComplete("sentry alert issues l")).toBe( + "list\tList issue alert rules", + ); + }, + ); + }, +); + describe("complete.ts: generated completion targets", () => { // Use the ESM import (routes) instead of require() which fails under // vitest because Node's CJS require can't resolve .js→.ts for transitive imports. diff --git a/packages/cli/test/lib/completions.test.ts b/packages/cli/test/lib/completions.test.ts index 4ea3e32b6..b41c3dd1a 100644 --- a/packages/cli/test/lib/completions.test.ts +++ b/packages/cli/test/lib/completions.test.ts @@ -52,7 +52,7 @@ describe("completions", () => { test("zsh script escapes colons in dynamic completion values", () => { const script = getCompletionScript("zsh"); - expect(script).toContain(`escaped_value="\${value//:/\\\\:}"`); + expect(script).toContain(`value="\${value//:/\\\\:}"`); }); test("fish script includes __complete callback", () => { @@ -60,27 +60,23 @@ describe("completions", () => { expect(script).toContain("__complete"); }); - test("bash script includes flag completion variables", () => { - const script = getCompletionScript("bash"); - // Should contain flag variables for at least some commands - expect(script).toContain("_flags="); + test("zsh script adds suggestions ending in a slash without a space", () => { + const script = getCompletionScript("zsh"); + expect(script).toContain("_describe -t values 'value' slashed -S ''"); }); - }); - describe("extractCommandTree", () => { - test("includes flags for subcommands", () => { - const tree = extractCommandTree(); - // At least one group should have subcommands with flags - const hasFlags = tree.groups.some((g) => - g.subcommands.some((s) => s.flags.length > 0), + test("fish descriptions keep their parentheses", () => { + const script = getCompletionScript("fish"); + expect(script).toContain( + '-a "init" -d "Initialize Sentry in your project (experimental)"', ); - expect(hasFlags).toBe(true); }); + }); - test("standalone commands include flags", () => { - const tree = extractCommandTree(); - const hasFlags = tree.standalone.some((s) => s.flags.length > 0); - expect(hasFlags).toBe(true); + describe("extractCommandTree", () => { + test("lists nested groups as subcommands of their group", () => { + const alert = extractCommandTree().groups.find((g) => g.name === "alert"); + expect(alert?.subcommands.map((s) => s.name)).toContain("issues"); }); }); diff --git a/packages/cli/test/lib/introspect.test.ts b/packages/cli/test/lib/introspect.test.ts index 9d9c416d6..0b1113c17 100644 --- a/packages/cli/test/lib/introspect.test.ts +++ b/packages/cli/test/lib/introspect.test.ts @@ -521,16 +521,43 @@ describe("extractCommandCompletions", () => { "-q": ISSUE_QUERY_TARGET, "-t": null, }, + flagNames: { + "--period": { brief: "", aliases: ["-t"] }, + "--query": { brief: "", aliases: ["-q"] }, + "--web": { brief: "", aliases: ["-w"] }, + }, }, search: { positionals: [], variadic: false, flags: { "--query": ISSUE_QUERY_TARGET }, + flagNames: { "--query": { brief: "" } }, }, }); }); - test("skips hidden commands, hidden groups, and commands without kinds", () => { + test("lists every visible flag's name, brief, aliases, and repeatability", () => { + const create = makeCommand({ + parameters: { + flags: { + json: { kind: "boolean", brief: "Output as JSON" }, + tag: { kind: "parsed", brief: "A tag", variadic: true }, + debug: { kind: "boolean", brief: "Internal", hidden: true }, + }, + aliases: { t: "tag", d: "debug" }, + }, + }); + + expect( + extractCommandCompletions(makeRouteMap([makeEntry("create", create)])) + .create?.flagNames, + ).toEqual({ + "--json": { brief: "Output as JSON" }, + "--tag": { brief: "A tag", aliases: ["-t"], variadic: true }, + }); + }); + + test("skips hidden commands, hidden groups, and commands with nothing to complete", () => { const annotated = commandWith(orgParser); const hiddenGroup = makeRouteMap([makeEntry("list", annotated)]); const topLevel = makeRouteMap([ @@ -538,7 +565,9 @@ describe("extractCommandCompletions", () => { makeEntry("internal", hiddenGroup, true), makeEntry( "plain", - makeCommand({ parameters: { flags: { period: { kind: "parsed" } } } }), + makeCommand({ + parameters: { flags: { period: { kind: "parsed", hidden: true } } }, + }), ), ]); @@ -580,6 +609,10 @@ describe("extractCommandCompletions", () => { "-f": ["json", "text"], "0": ["bash", "zsh"], }, + flagNames: { + "--format": { brief: "", aliases: ["-f"] }, + "--mode": { brief: "" }, + }, }, }); });