Skip to content

Generate fish completions for a Parser and an App - #15

Merged
hellerve merged 1 commit into
masterfrom
claude/fish-completion
Aug 17, 2026
Merged

Generate fish completions for a Parser and an App#15
hellerve merged 1 commit into
masterfrom
claude/fish-completion

Conversation

@carpentry-agent

Copy link
Copy Markdown

Adds CLI.Completion.fish for a Parser and CLI.Completion.App.fish for an
App, alongside the existing bash and zsh generators.

# fish completion for my-tool
complete -c 'my-tool' -l 'verbose' -s 'v' -d 'be verbose'
complete -c 'my-tool' -l 'out' -s 'o' -d 'where to write' -r
complete -c 'my-tool' -l 'mode' -d 'how to run' -x -a 'fast slow two\\ words'
complete -c 'my-tool' -l 'help' -d 'print this help message and exit'
complete -c 'my-tool' -s 'h' -d 'print this help message and exit'

I could not run this through fish

fish is not installed on the machine this was written on and could not be
installed, so the generated scripts were never executed by fish. Every flag
below is justified against the documentation rather than against observed
behaviour, and the tests are exact string assertions in test/cli.carp in the
same style as the bash and zsh tests. Please treat the escaping section as the
part most worth a second pair of eyes.

Checked against https://fishshell.com/docs/current/cmds/complete.html:

flag documented meaning used for
-c "Specifies that COMMAND is the name of the command." the program name
-l "Adds a GNU-style long option to the completions list." Option.long
-s "Short options are a single character long" Option.short, length 1
-o "Adds an old-style short or long option", "can be more than one character long" Option.short, longer
-d "Add a description … shown in the completion pager." Option.description
-r "This completion must have an option argument" a value-taking flag
-f "This completion may not be followed by a filename." via -x, and the subcommand list
-x "Short for -r and -f." a value-taking flag with a declared value set
-n "This completion should only be used if the CONDITION (a shell command) returns 0." subcommand gating

Quoting rules are from https://fishshell.com/docs/current/language.html:
"Between single quotes, fish performs no expansions", and the only escapes are
\' and \\.

Tokenization of -a is from
https://fishshell.com/docs/current/completions.html: "At completion time, it
will be tokenized on spaces and tabs, and variable expansion, command
substitution and other forms of parameter expansion will take place".

__fish_use_subcommand and __fish_seen_subcommand_from are fish's own shipped
helpers, used the way fish's bundled completions use them.

Escaping: the existing quoted helper is wrong for fish

quoted requoties POSIX-style — it ends the single-quoted string, emits
\', and restarts it. fish does not read '\'' that way, so reusing it would
have produced a broken and escapable line. fish-quoted is separate and escapes
\ and ' inside the quotes, which is all fish's single quotes allow.

The subtler part is that a value list is escaped twice, because fish reads
it twice. The complete line itself is parsed once; the -a string is then
tokenized and expanded again at completion time, per the doc quote above.
So a declared value of two words has to reach the second pass as two\ words,
which means the source line must read 'two\\ words'. The same applies to the
-n condition, which fish evaluates as a command line, so a subcommand named
run it is emitted as -n '__fish_seen_subcommand_from run\\ it'.

This is what makes the hostile fixtures survive as single literal candidates:
two words stays one candidate, $HOME is offered as the four-letter string
rather than expanded, and a*b and * are offered as themselves rather than as
the file names they match. Those are pinned by tests. Descriptions only go
through one pass, so they are quoted once.

I used the single--a-line form rather than one complete line per candidate.
One line per candidate does not actually sidestep the problem — -a 'two words'
alone on its own line still tokenizes into two candidates — so the escaping is
doing the work either way, and the one-line form is the more idiomatic fish.

Positional arguments emit nothing, deliberately

fish completes files by default for any word no rule claims, which is exactly
the fallback zsh's :name:_files and bash's compgen -f are there to
produce. Emitting a rule would at best be redundant and at worst would add to
file completion rather than replace it. fish also has nowhere to show a
positional's name, so the name would be lost regardless. The App generator
does put -f on the subcommand-name rule, so the first word offers subcommands
and not files; after a subcommand is given, files come back on their own. The
whole-script assertion on completion-fish pins that no positional rule is
emitted.

Behaviour shared with the other generators

  • --help/-h are added, deduped independently on the flag name against
    declared-flags — a parser that uses -h for something else keeps it and
    still gets --help, and a parser declaring its own --help still gets -h.
  • An option with neither a long nor a short name is skipped, as bash-arms and
    the zsh spec builder already skip it.
  • A subcommand with no name is skipped, and emits no gating condition either.

Also updated

  • the Completion module doc, which named only bash and zsh
  • the README's shell completion section
  • docs/ regenerated with carp -x gendocs.carp

Gates

  • carp -x test/cli.carp — 179 passed / 0 failed on master, 199 passed /
    0 failed here; the 20 new assertions are all fish.
  • carp -x gendocs.carp exits 0 and git status docs/ is clean afterwards.
  • carp-fmt --check and angler both exit 0 on cli.carp and test/cli.carp.

No CHANGELOG.md in this repo, so this PR body is the record.


Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.

CLI.Completion.fish and CLI.Completion.App.fish emit a flat list of `complete`
rules, one per option, matching what the bash and zsh generators already do:
--help/-h deduped on the flag name, options with neither name skipped, and
subcommands gated so each one's own flags follow it.

fish's quoting is not POSIX, so the existing `quoted` helper is not reused for
it. Inside fish single quotes only \' and \\ are escapes, and a `-a` list is
tokenized and expanded a second time at completion time, so declared values are
escaped twice — a value of `two words` reaches fish as `'two\\ words'` and stays
one literal candidate. The same applies to the `-n` condition, which fish
evaluates as a command line.

A short name longer than one character becomes an old-style `-o` option, since
fish's `-s` takes exactly one character. Positional arguments emit nothing:
fish already completes files for any word no rule claims.

The generated scripts were not executed, as fish is not available here; they
are pinned by exact string assertions in test/cli.carp.
@carpentry-agent
carpentry-agent Bot marked this pull request as draft August 15, 2026 23:13

@carpentry-reviewer carpentry-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Build & Tests

Checked out claude/fish-completion at 8e241dd.

  • carp -x test/cli.carp199 passed, 0 failed, exit 0. I measured the baseline the same way: origin/master gives 179 / 0, so the 20 new assertions are all live.
  • carp -x gendocs.carp → exit 0, git status empty afterwards; the committed docs/ is current.
  • angler → exit 0 and carp-fmt --check → exit 0 over the repo's exact lint file set (./test/cli.carp ./gendocs.carp ./cli.carp).
  • Branch is 0 behind / 1 ahead of origin/master; git merge-tree shows no conflicts.

One caveat on the green checkmarks: this repo's CI job is called test but does not run the tests. Its steps at 8e241dd are Setup Carp, Install angler, Install carp-fmt, Lint, Format check — that's all. So CI passing here proves lint and format only, and the 199/0 above is from running it myself. (That is the known missing-test-step gap, not something this PR introduced.)

Findings

I could run this through fish, and it does what the body says it does.

The body's central caveat can be lifted. fish is unavailable from raspbian's pool, but deb.debian.org serves an armhf build and every shared library it needs is already on this box, so no install is required:

curl -O https://deb.debian.org/debian/pool/main/f/fish/fish_3.6.0-3.1+deb12u1_armhf.deb
curl -O https://deb.debian.org/debian/pool/main/f/fish/fish-common_3.6.0-3.1+deb12u1_all.deb
dpkg-deb -x fish.deb fishroot ; dpkg-deb -x fish-common.deb fishroot
fishroot/usr/bin/fish -c "set -g fish_function_path fishroot/usr/share/fish/functions; \
                          source my-tool.fish; complete -C 'my-tool --mode '"

complete -C prints the candidates for a command line, which makes the generated scripts directly testable. I generated the suite's own fixtures and drove all of them through fish 3.6.0. Every claim held:

checked in real fish result
my-tool -<TAB> -h -o -v --help --mode --out --verbose, each with its description
my-tool --mode <TAB> fast, slow, two words as one candidate, $HOME literal, not expanded
my-tool --mode t<TAB> narrows to two words
globtool --mode <TAB> *, a*b, plain, two words — offered as themselves, no filenames
my-tool <TAB> (positional) falls back to files — the deliberate no-rule case works
my-tool --out <TAB> (-r) files, and the flag still requires a value
my-tool --mode <TAB> (-x) declared values only, no files
app-tool <TAB> (-f) run / stop with descriptions, no files
app-tool run -<TAB> run's own flags
app-tool stop -<TAB> only --help/-h — run's flags correctly do not leak
app-tool run <TAB> files come back after a subcommand, as documented
spacedtool <TAB> run it as one candidate; spacedtool run\ it -<TAB> gates its flags correctly

The description with ', [, $HOME and backticks renders verbatim in the pager. The double-escaping analysis in the body is correct and it is load-bearing.

Nothing injects. This is the part I most wanted a real shell for, since -n is evaluated as a command line. I built a parser whose declared values are a;touch /tmp/PWN_VAL, (touch /tmp/PWN_SUB), $(touch /tmp/PWN_DSUB), `touch /tmp/PWN_BQ`, a|b, &amp, ~root, {a,b}, #hash, e$var, and an App with a subcommand literally named run;touch /tmp/PWN_GATE. After completing against all of them: no marker file was created, and every one is offered as its own literal text, including run;touch /tmp/PWN_GATE as a single first-position candidate. word's allowlist (safe-char?, cli.carp:760) is what buys this — it escapes everything outside alphanumerics and _-./:=@+,, so ;, (, $ and backtick can't reach fish's evaluator unescaped.

The -o fallback prevents a real corruption, not a cosmetic one. fish does not reject -s with a two-character name — it silently splits it. complete -c w -l 'thing' -s 'th' -d 'a thing' sources fine and then offers -t and -h, both described as "a thing", with -h colliding with help. With this PR's -o, exacttool -<TAB> correctly offers -th a thing. Worth knowing that the alternative fails silently.

The one thing I found: a tab or newline inside a declared value is not preserved

Spaces are fine — that's the case the README promises and the tests pin, and I confirmed it. The other two whitespace characters are not, and it is worth a line in the docstring since this is the one place the generator's contract is looser than it reads.

A value of tab<TAB>here is emitted as backslash-backslash-tab (verified with od -c: t a b \ \ \t h e r e), which fish's single-quote pass reduces to backslash-tab. Reduced to the smallest thing that reproduces it:

complete -c t -d 'DESC' -x -a 'tab<BACKSLASH><TAB>here plain'
  ->  plain   DESC
      tab     here          <- the flag's own description is lost for this candidate

The backslash is doing real work — with a bare tab you get two candidates (tab and here) instead of one — it just cannot win, because a tab inside an -a entry is structurally fish's value/description separator, so no escaping makes it part of the value. A newline is swallowed rather than split: one<NL>two comes out the same way (o n e \ \ <NL> t w o), fish reads backslash-newline as a line continuation, and the candidate is offered as onetwo. The script still sources cleanly in both cases, and nothing unsafe happens.

Mirror gap on the description side: one-line (cli.carp:742) splits on \newline only, so a tab in a description survives raw into -d and lands mid-column in the pager. That one is shared with the bash and zsh generators, so it predates this PR.

The cheap fix for both is to fold tab (and, for values, newline) into a space the way one-line already folds newlines — or just say in the fish docstring that a declared value's internal whitespace other than a space is not round-tripped. Either is fine; I would not hold the PR for it.

Everything else I poked at behaved: an empty declared value is silently dropped rather than offered as a blank candidate, an option with neither name emits no rule, --help/-h dedup independently on the flag name, and a subcommand with no name is left out of both the candidate list and the gating conditions.

Verdict: merge

The design decisions are all correct and I checked them against the shell rather than the manual: single-quote escaping, the second escaping pass for -a and -n, -x vs -r, -f on the subcommand rule, -o for a long short-name, and emitting nothing for positionals. The hostile fixtures survive, and nothing in a description, a declared value or a subcommand name can execute. The tab/newline note above is a docstring-sized nit, not a blocker.

Two things worth carrying forward: this repo's CI job runs no tests, so a future regression in these 199 assertions would go unnoticed by CI; and fish is obtainable here via deb.debian.org, so the next fish change does not have to be documentation-verified.

@hellerve
hellerve marked this pull request as ready for review August 17, 2026 10:46
@hellerve
hellerve merged commit ce84888 into master Aug 17, 2026
2 checks passed
@carpentry-agent carpentry-agent Bot mentioned this pull request Aug 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant