Skip to content

feat(shell-hook): validate --shell against the shells it accepts - #6685

Open
hunger wants to merge 1 commit into
prefix-dev:mainfrom
hunger:push-uryomknqqvrt
Open

feat(shell-hook): validate --shell against the shells it accepts#6685
hunger wants to merge 1 commit into
prefix-dev:mainfrom
hunger:push-uryomknqqvrt

Conversation

@hunger

@hunger hunger commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

--shell was a plain Option<ShellEnum> with the accepted names written out by hand in the doc-string, so the list went stale. rattler_shell gained the bash flavors sh, dash, ksh and busybox, plus brush, nu and powershell_ise, while --help and the CLI reference still named only the original seven. -s sh worked and nothing in pixi said so. A typo got a bare parse error with no alternatives, and completion had no candidates to offer.

Listing the names ShellEnum's FromStr accepts in a PossibleValuesParser lets clap reject an unknown shell with the alternatives, show the set in --help, and offer it in every generated completion script. The doc-string drops the hand-maintained list and states the default instead.

The list copies names rattler_shell cannot enumerate, so two tests hold it in step: every listed name must still parse, and an exhaustive match over ShellEnum plus a variant count catches a shell no listed name reaches. Neither notices a new alias for an existing variant, and the flavors behind ShellEnum::Bash cannot be guarded the same way because BashFlavor is #[non_exhaustive].

Refs #5495

AI Disclosure

  • This PR contains AI-generated content.
    • I have tested any AI-generated content in my PR.
    • I take responsibility for any AI-generated content in my PR.

Tools: Claude

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added sufficient tests to cover my changes.
  • I have verified that changes that would impact the JSON schema have been made in schema/model.py.

@hunger
hunger requested review from baszalmstra and ruben-arts July 27, 2026 17:08
`--shell` was a plain `Option<ShellEnum>` with the accepted names written out by
hand in the doc-string, so the list went stale. `rattler_shell` gained the bash
flavors `sh`, `dash`, `ksh` and `busybox`, plus `brush`, `nu` and
`powershell_ise`, while `--help` and the CLI reference still named only the
original seven. `-s sh` worked and nothing in pixi said so. A typo got a bare
parse error with no alternatives, and completion had no candidates to offer.

Listing the names `ShellEnum`'s `FromStr` accepts in a `PossibleValuesParser`
lets clap reject an unknown shell with the alternatives, show the set in
`--help`, and offer it in every generated completion script. The doc-string
drops the hand-maintained list and states the default instead.

The list copies names `rattler_shell` cannot enumerate, so two tests hold it in
step: every listed name must still parse, and an exhaustive match over
`ShellEnum` plus a variant count catches a shell no listed name reaches. Neither
notices a new alias for an existing variant, and the flavors behind
`ShellEnum::Bash` cannot be guarded the same way because `BashFlavor` is
`#[non_exhaustive]`.

Refs prefix-dev#5495
@hunger
hunger force-pushed the push-uryomknqqvrt branch from 453d59d to 6eb7785 Compare July 30, 2026 09:35
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