Generate fish completions for a Parser and an App - #15
Conversation
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.
There was a problem hiding this comment.
Build & Tests
Checked out claude/fish-completion at 8e241dd.
carp -x test/cli.carp→ 199 passed, 0 failed, exit 0. I measured the baseline the same way:origin/mastergives 179 / 0, so the 20 new assertions are all live.carp -x gendocs.carp→ exit 0,git statusempty afterwards; the committeddocs/is current.angler→ exit 0 andcarp-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-treeshows 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, &, ~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.
Adds
CLI.Completion.fishfor aParserandCLI.Completion.App.fishfor anApp, alongside the existingbashandzshgenerators.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.carpin thesame 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:
-c-lOption.long-sOption.short, length 1-oOption.short, longer-dOption.description-r-f-x, and the subcommand list-x-nQuoting 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
-ais fromhttps://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_subcommandand__fish_seen_subcommand_fromare fish's own shippedhelpers, used the way fish's bundled completions use them.
Escaping: the existing
quotedhelper is wrong for fishquotedrequoties POSIX-style — it ends the single-quoted string, emits\', and restarts it. fish does not read'\''that way, so reusing it wouldhave produced a broken and escapable line.
fish-quotedis 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
completeline itself is parsed once; the-astring is thentokenized and expanded again at completion time, per the doc quote above.
So a declared value of
two wordshas to reach the second pass astwo\ words,which means the source line must read
'two\\ words'. The same applies to the-ncondition, which fish evaluates as a command line, so a subcommand namedrun itis emitted as-n '__fish_seen_subcommand_from run\\ it'.This is what makes the hostile fixtures survive as single literal candidates:
two wordsstays one candidate,$HOMEis offered as the four-letter stringrather than expanded, and
a*band*are offered as themselves rather than asthe 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 onecompleteline 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:_filesand bash'scompgen -fare there toproduce. 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
Appgeneratordoes put
-fon the subcommand-name rule, so the first word offers subcommandsand not files; after a subcommand is given, files come back on their own. The
whole-script assertion on
completion-fishpins that no positional rule isemitted.
Behaviour shared with the other generators
--help/-hare added, deduped independently on the flag name againstdeclared-flags— a parser that uses-hfor something else keeps it andstill gets
--help, and a parser declaring its own--helpstill gets-h.bash-armsandthe zsh spec builder already skip it.
Also updated
Completionmodule doc, which named only bash and zshdocs/regenerated withcarp -x gendocs.carpGates
carp -x test/cli.carp— 179 passed / 0 failed onmaster, 199 passed /0 failed here; the 20 new assertions are all fish.
carp -x gendocs.carpexits 0 andgit status docs/is clean afterwards.carp-fmt --checkandanglerboth exit 0 oncli.carpandtest/cli.carp.No
CHANGELOG.mdin this repo, so this PR body is the record.Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.