Conversation
added 16 commits
August 24, 2026 11:27
Replace clap with bpaf 0.9 across the runtime crates (brush-core,
brush-builtins, brush-experimental-builtins, brush-shell, and the test
harness), reworking the builtin Command trait around bpaf's parser
combinators.
Core changes:
- builtins::Command now requires a bpaf 'parser()' plus 'about()' /
'synopsis()' metadata used by the help builtin; detailed help is
rendered by triggering bpaf's --help handling
- add bash-faithful option/operand splitting (split_option_section) for
builtins that capture trailing operands verbatim, replacing the
per-builtin clap workarounds ('--' splitting, argv0 handling)
- plus-style options (+x, +o): expand '+abc' groups like the shell does
and match them with bpaf 'literal' parsers; drop the old '+x' ->
'--+x' encoding, so literal '--+o' input is now rejected like bash
- ValueEnum types become FromStr-based (completion actions/options,
trace events, input backends)
Shell CLI:
- rewrite CommandLineArgs as a combinatoric parser; bash-compatible
handling of '-c' with '--', script arguments captured verbatim after
the option section
Builtins: port all ~50 builtins; several now behave closer to bash
(echo preserves '--', kill -l no longer swallows signals, pwd -LP
last-wins, set -- clears positional parameters, printf/command honor
leading '--').
Dev tooling: xtask doc generation renders help via bpaf (clap_mangen/
clap_markdown/clap_complete dropped); completion scripts are generated
by running the brush binary's own bpaf completion support.
Assisted-by: ox-alpha (opencode)
Adds a self-contained benchmark driver comparing two brush builds against a bash oracle: - verifies byte-for-byte output parity across all three shells before timing anything - interleaves samples round-robin so machine drift affects every shell equally (sequential test-vs-reference runs proved drift-sensitive on multi-tenant machines) - reports median +- MAD wall time, pairwise speedups, and per-workload peak RSS (VmHWM) Workloads: process startup, pure interpreter loop, string/pattern/array ops, and the getopts-dense config-lint/deploy-sim scripts (copied from the usage-parser experiment). Assisted-by: ox-alpha (opencode)
The first listed shell acts as the oracle (bash); every other entry is a candidate build. Adds an all-pairs comparison section so parser-backend and LTO-variant matrices can be captured in one run. Assisted-by: ox-alpha (opencode)
bpaf supports multiple names per named item via repeated .long()/.short() calls: the first short and first long are visible in help, further ones act as hidden aliases. Replaces the construct![a, b] alternative pairs used for --init-file/--rcfile, --log-enable/--debug, --log-disable/--disable-event, and kill's -L/-l with single parsers. This removes duplicated parser definitions, keeps help output showing only the canonical spellings (matching the clap-era help), and lets kill accept -L again as a hidden alias of -l. Reported-by: reubeno (upstream author feedback) Assisted-by: ox-alpha (opencode)
Assisted-by: ox-alpha (opencode)
…ed deps The TestOptions port to bpaf lost several clap 'env = ...' fallbacks, which broke the WASI test job: BRUSH_PLATFORM_TAGS was no longer read, so the harness appended --input-backend=basic to a minimal-feature wasm build that does not contain that backend; brush rejected the unknown option and every integration case failed with exit 1. Restore all environment fallbacks using bpaf's native .env() support (BRUSH_VERBOSE, BASH_PATH, BRUSH_PATH, BRUSH_ARGS, BRUSH_LAUNCHER, BRUSH_PLATFORM_TAGS, BRUSH_TEST_CASES, BRUSH_TEST_PATH_VAR). For BRUSH_PLATFORM_TAGS, whose value is space-separated, use '.some()' plus a splitting parse rather than '.many()': many() succeeds with an empty vector, which would prevent the fallback from ever applying. Also remove dependencies left unused by the port (pretty_assertions in brush-shell after its tests were rewritten, clap-markdown in xtask). Assisted-by: ox-alpha (opencode)
Introduce an argument-description model (brush-core::argmodel) so that builtins declare their arguments as data and materialize themselves from parsed matches, with the actual parsing crate as a compile-time implementation detail: - CommandSpec/ArgSpec/PositionalSpec describe switches, value-taking options (with hidden aliases), and positionals; Matches exposes typed lookups plus verbatim trailing operands - ArgParserBackend trait with three implementations selected by cargo features: parser-bpaf (default), parser-usage, parser-clap - SpecCommand trait + spec_builtin() registration: declare() builds the spec, from_matches() builds the command; help content renders through the backend - shell-style option/operand splitting and '+x' group expansion stay in core, shared by every backend Converts echo to the new contract as the reference implementation and rewrites the custom-builtin example accordingly; remaining builtins continue using the legacy bpaf-bound trait until converted. The example now runs identically under all three backends. Assisted-by: ox-alpha (opencode)
…tract Every builtin now declares its argument surface as data (brush-core::argmodel) and materializes from parsed Matches; the legacy bpaf-bound Command/DeclarationCommand traits are no longer used by any registration (factory.rs is all spec_builtin, with special markers and declaration routing preserved). Notable conversions: - set: 19 +/- pairs via declare_plus_minus helpers; -o/+o tri-state and bare '--' detection preserved through an overridden new() - complete/compgen/compopt: shared arg surface flattened into each spec; flag-looking-value joining kept in overridden new() - kill: -l/-L as one spec arg with hidden short alias - history: -a/-n/-r/-w stay independent flags (matching old behavior); bare -X lifted out of the option section in overridden new() - test/[ and unimp: verbatim capture via overridden new() - pwd/cd: manual last-wins mode scan ported to overridden new() - export/declare/builtin_: uses_declarations() routes raw Vec<CommandArg> operands through set_declarations Core support added for the contract: uses_declarations()/set_declarations hooks on SpecCommand with declaration-aware execution routing, plus declare_plus_minus/read_plus_minus helpers replacing the bpaf-era macro. All suites green: compat 1854 passed / 0 failed (baseline parity), unit/integration tests pass, clippy -D warnings clean. Assisted-by: ox-alpha (opencode)
Introduce an argument-description model (brush-core::argmodel) so that builtins declare their arguments as data and materialize themselves from parsed matches, with the actual parsing crate as a compile-time implementation detail: - CommandSpec/ArgSpec/PositionalSpec describe switches, value-taking options (with hidden aliases), and positionals; Matches exposes typed lookups plus verbatim trailing operands - ArgParserBackend trait with three implementations selected by cargo features: parser-bpaf (default), parser-usage, parser-clap - SpecCommand trait + spec_builtin() registration: declare() builds the spec, from_matches() builds the command; help content renders through the backend - shell-style option/operand splitting and '+x' group expansion stay in core, shared by every backend Converts echo to the new contract as the reference implementation and rewrites the custom-builtin example accordingly; remaining builtins continue using the legacy bpaf-bound trait until converted. The example now runs identically under all three backends. Assisted-by: ox-alpha (opencode)
- backend selection becomes priority-based (usage > clap > bpaf) instead of compile_error, so workspace feature unification cannot break builds when several backends end up linked; a separate bpaf-linked feature keeps the bpaf backend compilable whenever the crate is in the graph (the shell CLI still uses bpaf directly for its own options) - brush-builtins/brush-shell forward parser-* features; brush-builtins no longer hard-depends on bpaf - pwd converts to SpecCommand (overridden new() keeps last-wins -L/-P) Assisted-by: ox-alpha (opencode)
bpaf_backend now compiles only when bpaf is the *selected* builtin backend (no parser-usage/parser-clap), so workspace feature unification that links bpaf for the shell CLI does not produce dead-code errors in usage-only builds. active() priority: usage > clap > bpaf. Assisted-by: ox-alpha (opencode)
usage backend: bind Event::Arg by positional id into positional slots; ParsedValues gains positional storage + accessors (positional_values/value_of_positional/push_positional_*). Converted builtins that read positionals via the named-arg getters (compgen word, mapfile array var, read var names, break/continue/caller/return, etc.) now use the positional accessors. Backend-parity tests extended to the usage backend. Assisted-by: ox-alpha (opencode)
The usage backend previously rebuilt and leaked the engine's Command<'static> graph on every builtin invocation, so scripts calling shift/getopts in loops leaked one allocation set per call. Graphs are now interned in a process-wide cache keyed by (spec address, invocation name), bounding total leakage to one graph per registered builtin — matching what usage's derive achieves with compile-time statics. Also adds backend-parity tests for the usage backend (single positional binding; graph reuse across parses). Assisted-by: ox-alpha (opencode)
The clap backend predates the positional-slot redesign and its tests were
stale, masking two bugs:
- Positional values were stored through the named-value API instead of
positional slots, so plain operands never reached callers.
- 'allow_hyphen_values' made flag-like leading words bind as operand
values where bash and every other backend reject them ('echo -x' must
error); trailing_var_arg alone keeps post-first-word words verbatim.
Tests are updated to the current pattern and now cover the same cases as
the bpaf and usage suites. Compat suite: 1854 passed / 0 failed.
Assisted-by: ox-alpha (opencode)
Assisted-by: ox-alpha (opencode)
Assisted-by: ox-alpha (opencode)
Contributor
Author
|
Closing per author — approach needs rework before review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft for design/data inspection. Contains and supersedes the stack in #1302 (bpaf port); this PR's diff = bpaf port + the contract work on top.
What this adds
Builtins declare their arguments as compile-time, backend-neutral data:
A build selects one engine via features (
parser-usage/parser-clap/parser-bpaf; priority in that order).ArgParserBackendtranslates the spec into engine structures, parses, maps results into slot-indexedParsedValues. Shell-specific semantics (declaration routing,+x/-xtoggles, verbatim trailing words) live once in core instead of per engine. All ~49 builtins converted.The manual
staticspecs above are interim. Hand-writing stringly-typed ids is worse DX than today's clap derive structs, and unacceptable for third-party builtin authors. The intended endgame is a small proc-macro crate:i.e. the ergonomic shape of
#[derive(Args)]/usage's#[derive(Cli)], but emitting backend-neutral data so engines stay swappable. Nothing in the contract layer changes for that to slot in — specs are already plain static data. Opening this draft now because the contract shape and the benchmark data below are what need review first; if the direction is wrong, the derive never gets written.Why this shape at all
usage-rs's
#[derive(Cli)]proved static spec → parser graph is the fast path. This keeps that shape but makes two things differ from a single-engine derive: spec source (hand-written today, derived later) and target engine (swappable). The runtime translation layer reconstructs what derive gets free (compile-time statics); an intern cache bounds it to one leaked graph per(builtin, name)— pointer-stability tested.Benchmark data
benchmarks/three-way.py(committed): pinned CPU, interleaved sampling across shells (machine drift hits every build equally), byte-for-byte output parity before timing, median of 12 samples. Workloads inbenchmarks/real-world/.7 shells, one machine, one run:
¹ upstream
main(clap derive builtins) · ² #1302 head (native bpaf impls) · ³ usage fork with#[derive(Cli)]builtinsReading:
main's own clap code by ~17% on config-lint (leaner spec-builtCommandvs derive path).Correctness
allow_hyphen_valuesacceptingecho -xwhere bash errors) — fixed.(spec, name); total leakage equals what derive would emit as statics.Open questions for review
Assisted-by: ox-alpha (opencode)