Skip to content

Mark bash file completion with compopt -o filenames - #13

Merged
hellerve merged 1 commit into
masterfrom
claude/completion-filenames
Aug 11, 2026
Merged

Mark bash file completion with compopt -o filenames#13
hellerve merged 1 commit into
masterfrom
claude/completion-filenames

Conversation

@carpentry-agent

Copy link
Copy Markdown

Follow-up to #12, which I deferred there: the generated bash script's file-completion fallback emitted compgen -f with no compopt -o filenames. Without that option bash does not treat the candidates as file names, so it inserts a completed name unquoted and appends a space rather than a slash to a directory.

#12 made a name with a space come back as a single candidate; this makes bash insert it correctly.

What it looked like

Driving real bash through a pty, in a directory that actually contains spaced file.txt and dirtest/, with my-tool printing back the argv it received:

typed, then TAB before after
my-tool --out spa [--out] [spaced] [file.txt] [--out] [spaced file.txt]
my-tool spa (positional) [spaced] [file.txt] [spaced file.txt]
my-tool run --out spa (App) [run] [--out] [spaced] [file.txt] [run] [--out] [spaced file.txt]
my-tool run spa (App positional) [run] [spaced] [file.txt] [run] [spaced file.txt]
my-tool --out dirt [--out] [dirtest] [--out] [dirtest/]

The ambient state matters here: with an empty directory, or with two candidates sharing a prefix, tab completion inserts nothing and the broken case looks like it passes. Both of my first two harness attempts fell into exactly that trap (an ambiguous my prefix, then a source of a relative path from a shell that had cd'd elsewhere, which silently left bash's own — correctly quoting — default completion in place). The numbers above are from a run that asserts the completion function is registered first.

Scope

Only the two file-completion arms are marked. The compgen -W arms keep their plain completion: -o filenames on a declared value list would have bash treat literal values as paths (stripping anything up to a /, appending a slash when a directory of that name happens to exist). Two of the new assertions pin that, so the option cannot be copied onto those arms by a later edit.

This also means a declared value containing a space is still inserted unquoted — --mode tw<TAB> gives [--mode] [two] [words]. That is a real and separate problem, and -o filenames is not the fix for it.

compopt goes before the mapfile so the arm's exit status stays the mapfile's.

Testing

  • carp -x test/cli.carp: 179 passed, 0 failed. Reverting only cli.carp and re-running fails 3 of them, so the new assertions are not vacuous.
  • Version floor is unchanged: compopt and mapfile are both bash 4.0.
  • carp-fmt -c and angler clean on both changed files.

No changelog entry: this repo has no changelog.


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

Without the option bash does not treat the `compgen -f` candidates as file
names: a completed name containing a space is inserted unquoted, so it
reaches the program as two arguments, and a completed directory gets a
trailing space instead of a slash.

Only the file-completion arms are marked. On a declared value list
-o filenames would make bash treat the literal values as paths.

@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

carp -x test/cli.carp  ->  exit 0,  Passed: 179  Failed: 0

This matters more here than usual: CI did not run these assertions. This repo's
workflow has a job named test, but its steps are only Setup Carp, Install angler,
Install carp-fmt, Lint and Format check — there is no test step at all. So the two
green checks on run 31476890850 verify lint and formatting only. The 179 assertions
above are local evidence, run on this branch at 51db99c. Branch is on master@066f890,
merge-base equals master's head.

Findings

I did not take the before/after table in the description on trust — I regenerated both
scripts from source and drove them through a real interactive bash 5.2 on a pty, in a
directory that actually contains spaced file.txt, dirtest/, subdir/ and plain.txt.
(An empty directory would have let both sides "pass" for the wrong reason.)

typed, then TAB master this branch
my-tool --out spa --out spaced file.txt — two words --out spaced\ file.txt — one
my-tool --out dirt --out dirtest — terminated --out dirtest/ — can descend

Same result through the App (subcommand) generator: my-tool run --out spa
spaced\ file.txt, my-tool run --out dirtdirtest/.

Three things I specifically tried to break:

1. Does compopt -o filenames leak into a later completion? compopt with no name
argument mutates the currently executing completion, so an earlier file completion
setting it could plausibly contaminate the next one. Ran four completions in a single
bash session, file completion first:

--out spa  -> spaced\ file.txt      (marked)
--mode fa  -> fast                  (clean)
--mode tw  -> two words             (clean — no filename treatment)
--mode sl  -> slow                  (clean)

No leak.

2. Is "only the file arm" actually the right call, or just the cautious one? The
description asserts that marking a declared value list "would have bash treat literal
values as paths". I tested that rather than assuming it: same script, value list
('fast' 'dirtest' 'two\ words'), with dirtest/ existing on disk.

value arm unmarked (as shipped) value arm also marked
--mode dirt --mode dirtest --mode dirtest/
--mode tw --mode two words --mode two\ words

So marking it would indeed corrupt a literal enum value into a path whenever a directory
of that name happens to exist in the cwd. The stated rationale holds up. It also
confirms the deferred issue is genuinely separate: two words still goes in unquoted, and
this PR correctly does not claim to fix it.

3. Is the fix complete? Yes, by construction rather than by diligence —
bash-values (cli.carp:830) is the only producer of compgen -f in the file, and the
new compopt line sits inside it, so both generated arms (the --out|-o) case and the
positional fallback) get it and neither can drift from the other:

grep -n 'compgen -f' cli.carp  -> 834   (one site)
grep -n 'compopt'    cli.carp  -> 833   (one site)

Two things I checked and found fine:

  • No new bash version floor. compopt and mapfile are both bash 4.0+ builtins, and
    mapfile was already required by #12, so this does not raise the minimum.
  • No zsh counterpart is needed. The zsh generator emits _files (cli.carp:913, :936),
    which already handles quoting and trailing slashes natively.
  • A prefix matching no file (--out zzzz) leaves the line untouched and produces no
    error, even though compopt ran.

No blocking findings. One out-of-scope observation: a subcommand declaring no
positional arguments (stop in my test app) still falls back to file completion. That is
pre-existing — I generated the same app from master and it emits the same
compgen -f fallback — but -o filenames now makes it more visible, since stop dirt<TAB>
will start appending trailing slashes. Might be worth deciding separately whether that arm
should exist at all.

Verdict: merge

The behaviour change is real and I reproduced it in both directions through actual bash
with the ambient files present; the one-arm scoping is empirically justified rather than
merely asserted; and the four new assertions pin it. Worth noting for the record that the
green checks here cover only lint and formatting — the test evidence is the local 179/0.

@hellerve
hellerve merged commit e8b1fdc into master Aug 11, 2026
2 checks passed
@hellerve
hellerve deleted the claude/completion-filenames branch August 11, 2026 23:30
@carpentry-agent carpentry-agent Bot mentioned this pull request Aug 14, 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