Skip to content

WIP: port argument parsing from clap to bpaf - #1302

Draft
lu-zero wants to merge 7 commits into
reubeno:mainfrom
lu-zero:bpaf-port
Draft

lu-zero wants to merge 7 commits into
reubeno:mainfrom
lu-zero:bpaf-port

Conversation

@lu-zero

@lu-zero lu-zero commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Draft PR exploring a port of brush's argument parsing from clap to bpaf (0.9), linked to #520.

What this does

Replaces clap with bpaf in the runtime crates (brush-core, brush-builtins, brush-experimental-builtins, brush-shell, brush-test-harness), redesigning the builtin Command trait around bpaf parser combinators:

  • builtins::Command now requires fn parser() -> impl bpaf::Parser<Self> + 'static plus small metadata hooks (about(), synopsis()) that feed the help builtin. Detailed help is rendered by triggering bpaf's own --help handling.
  • A single bash-faithful splitter (builtins::split_option_section) decides where the option section ends for builtins with verbatim trailing operands (echo, test, set, fc, ...). Builtins declare their value-taking short options; the splitter honors --, first-operand termination, and attached values (-d:, -G=--foo).
  • Plus-style options (set +x, declare +a) no longer use the old clap-era +x → --+x encoding: +abc groups are expanded the way the shell tokenizes them and matched with bpaf literal("+x") parsers. As a bonus, literal --+o foo input — which bash rejects and the old encoding silently accepted as +o — is now rejected like bash.
  • clap::ValueEnum types become plain FromStr enums (completion actions/options, trace events, input backends).
  • The shell CLI is a combinatoric parser with bash-compatible -c/-- handling; script arguments are captured verbatim after the option section.
  • Option aliases use bpaf's chained-name support (long("rcfile").long("init-file"), short('l').short('L')): first spellings visible in help, rest hidden aliases.

Behavior notes

Several builtins now behave closer to bash than before:

  • echo -- -n z prints -n z; echo ---------------- echoes the dashes
  • kill -s TERM $$, kill -9 $$, kill -L work again under the harness
  • pwd -L -P / pwd -P -L: last flag wins
  • set -- clears positional parameters; test -- / [ -- ] are true
  • printf -- -5 and command -- ls honor the terminator

Validation

  • compat suite: 1854 passed, 0 failed, 456 known-failures, 29 skipped — identical to the pre-port baseline
  • all unit tests, completion tests (real /usr/share/bash-completion), integration tests pass
  • cargo fmt --check and cargo clippy --workspace --all-targets -D warnings clean
  • benchmarked against clap and the usage-based parser: see benchmark comment and benchmarks/three-way.py

Nice-to-have bpaf API changes

  1. Public access to rendered help text — detailed help currently has to be obtained by running run_inner(["--help"]) and unwrapping a ParseFailure. An OptionParser::render_help_string() (monochrome + colored variants) would remove the round-trip.
  2. Position-aware / sequential parsing primitive — switches scan all unconsumed items regardless of position ("before" = declaration order). Bash-style "stop option parsing at the first operand" (echo hi -n must keep -n literal) cannot be expressed combinatorially today; we pre-split argv ourselves. Something like a positional parser with allow_hyphen_values-plus-trailing_var_arg semantics would let us delete the splitter.
  3. Per-parser "last occurrence wins" for repeated flags — clap's overrides_with pattern (cd -P -L) can't be expressed; alternatives conflict and independent switches lose ordering.
  4. Man-page / markdown export — clap ecosystem had clap_mangen/clap_markdown. An official (or documented third-party) way to render bpaf parsers to roff/markdown would restore richer doc generation; xtask currently emits from monochrome help text.
  5. ParseFailure: std::error::Error (or an into_error()) so ?-style plumbing works in test code without manual matching.

(An earlier version of this list claimed aliases were unsupported; that was wrong — chained .long()/.short() provide hidden aliases, now used throughout.)

Follow-ups (not in this draft)

  • Port xtask's own CLI and any remaining dev-tool CLIs to bpaf
  • Colored help rendering (bpaf's colored renderer is crate-private)
  • Revisit ContentOptions::colorized once colorized rendering is reachable

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)
@github-actions

github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown

Public API changes for crate: brush-builtins

Removed items

-pub macro brush_builtins::minus_or_plus_flag_arg!

Public API changes for crate: brush-core

Removed items

-pub fn brush_core::builtins::parse_known<T: clap_builder::derive::Parser, S>(impl core::iter::traits::collect::IntoIterator<Item = S>) -> (T, core::option::Option<impl core::iter::traits::iterator::Iterator<Item = S>>) where S: core::convert::Into<std::ffi::os_str::OsString> + core::clone::Clone + core::cmp::PartialEq<&'static str>
-pub fn brush_core::builtins::try_parse_known<T: clap_builder::derive::Parser>(impl core::iter::traits::collect::IntoIterator<Item = alloc::string::String>) -> core::result::Result<(T, core::option::Option<impl core::iter::traits::iterator::Iterator<Item = alloc::string::String>>), clap_builder::Error>

Added items

+pub use brush_core::builtins::Parser
+pub struct brush_core::builtins::BuiltinArgParseError
+pub brush_core::builtins::BuiltinArgParseError::help_request: bool
+pub brush_core::builtins::BuiltinArgParseError::message: alloc::string::String
+impl core::error::Error for brush_core::builtins::BuiltinArgParseError
+impl core::fmt::Display for brush_core::builtins::BuiltinArgParseError
+pub fn brush_core::builtins::BuiltinArgParseError::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result
+pub fn brush_core::builtins::Command::about() -> &'static str
+pub fn brush_core::builtins::Command::parser() -> impl bpaf::Parser<Self> + 'static
+pub fn brush_core::builtins::Command::set_trailing_args(&mut self, alloc::vec::Vec<alloc::string::String>)
+pub fn brush_core::builtins::Command::synopsis() -> &'static str
+pub fn brush_core::builtins::Command::takes_trailing_args() -> bool
+pub fn brush_core::builtins::Command::value_taking_short_options() -> &'static str
+pub fn brush_core::builtins::split_option_section(&[alloc::string::String], &str, &[&str]) -> (alloc::vec::Vec<alloc::string::String>, alloc::vec::Vec<alloc::string::String>)
+impl brush_core::completion::CompleteAction
+pub fn brush_core::completion::CompleteAction::parse(&str) -> core::option::Option<Self>
+impl core::str::traits::FromStr for brush_core::completion::CompleteAction
+pub type brush_core::completion::CompleteAction::Err = alloc::string::String
+pub fn brush_core::completion::CompleteAction::from_str(&str) -> core::result::Result<Self, Self::Err>
+impl brush_core::completion::CompleteOption
+pub fn brush_core::completion::CompleteOption::parse(&str) -> core::option::Option<Self>
+impl core::str::traits::FromStr for brush_core::completion::CompleteOption
+pub type brush_core::completion::CompleteOption::Err = alloc::string::String
+pub fn brush_core::completion::CompleteOption::from_str(&str) -> core::result::Result<Self, Self::Err>

Changed items

-pub trait brush_core::builtins::Command: clap_builder::derive::Parser
+pub trait brush_core::builtins::Command: core::marker::Sized
-pub fn brush_core::builtins::Command::new<I>(I) -> core::result::Result<Self, clap_builder::Error> where I: core::iter::traits::collect::IntoIterator<Item = alloc::string::String>
+pub fn brush_core::builtins::Command::new<I>(I) -> core::result::Result<Self, brush_core::builtins::BuiltinArgParseError> where I: core::iter::traits::collect::IntoIterator<Item = alloc::string::String>

Public API changes for crate: brush-shell

Removed items

-pub brush_shell::args::CommandLineArgs::help: core::option::Option<bool>
-pub brush_shell::args::CommandLineArgs::version: core::option::Option<bool>

Added items

+impl core::str::traits::FromStr for brush_shell::args::InputBackendType
+pub type brush_shell::args::InputBackendType::Err = alloc::string::String
+pub fn brush_shell::args::InputBackendType::from_str(&str) -> core::result::Result<Self, Self::Err>
+pub brush_shell::args::CommandLineArgs::experimental_parser: bool
+pub brush_shell::args::CommandLineArgs::load_file: core::option::Option<std::path::PathBuf>
+pub fn brush_shell::args::CommandLineArgs::option_parser() -> bpaf::info::OptionParser<Self>
+pub fn brush_shell::args::CommandLineArgs::parser() -> impl bpaf::Parser<Self>
+pub fn brush_shell::args::CommandLineArgs::try_parse_from<S: core::convert::Into<alloc::string::String>>(impl core::iter::traits::collect::IntoIterator<Item = S>) -> core::result::Result<Self, bpaf::error::ParseFailure>
+impl brush_shell::events::TraceEvent
+pub const fn brush_shell::events::TraceEvent::names() -> &'static [&'static str]
+pub fn brush_shell::events::TraceEvent::parse(&str) -> core::option::Option<Self>
+impl core::str::traits::FromStr for brush_shell::events::TraceEvent
+pub type brush_shell::events::TraceEvent::Err = alloc::string::String
+pub fn brush_shell::events::TraceEvent::from_str(&str) -> core::result::Result<Self, Self::Err>

Changed items

-pub brush_shell::args::CommandLineArgs::login: bool
+pub brush_shell::args::CommandLineArgs::login: core::option::Option<bool>
-pub brush_shell::args::CommandLineArgs::verbose: bool
+pub brush_shell::args::CommandLineArgs::verbose: core::option::Option<bool>

Public API changes for crate: brush-test-harness

Removed items

-pub brush_test_harness::TestOptions::help: core::option::Option<bool>

Added items

+impl core::str::traits::FromStr for brush_test_harness::OutputFormat
+pub type brush_test_harness::OutputFormat::Err = alloc::string::String
+pub fn brush_test_harness::OutputFormat::from_str(&str) -> core::result::Result<Self, Self::Err>
+pub fn brush_test_harness::TestOptions::parse_from<S: core::convert::AsRef<str>>(impl core::iter::traits::collect::IntoIterator<Item = S>) -> Self
+pub fn brush_test_harness::TestOptions::parser() -> impl bpaf::Parser<Self>

Changed items

-pub brush_test_harness::TestOptions::color: clap_builder::util::color::ColorChoice
+pub brush_test_harness::TestOptions::color: core::option::Option<alloc::string::String>

Performance Benchmark Report

Code Coverage Report: Only Changed Files listed

Package Base Coverage New Coverage Difference
brush-builtins\src\bind.rs 🔴 25.17% 🔴 20.61% 🔴 -4.56%
brush-builtins\src\fc.rs 🔴 0% 🔴 36.65% 🟢 36.65%
brush-builtins\src\history.rs 🔴 7.87% 🔴 44.09% 🟢 36.22%
brush-builtins\src\mapfile.rs 🔴 0% 🔴 49.14% 🟢 49.14%
brush-builtins\src\printf.rs 🟠 57.8% 🔴 44.37% 🔴 -13.43%
brush-builtins\src\pwd.rs 🔴 0% 🟠 52.94% 🟢 52.94%
brush-builtins\src\read.rs 🔴 37.93% 🔴 48.02% 🟢 10.09%
brush-builtins\src\test.rs 🔴 0% 🔴 25.93% 🟢 25.93%
brush-core\src\builtins.rs 🔴 5.08% 🔴 32.99% 🟢 27.91%
brush-core\src\completion.rs 🔴 14.59% 🔴 13.83% 🔴 -0.76%
brush-shell\src\args.rs 🟠 62.5% 🟢 87.1% 🟢 24.6%
brush-shell\src\entry.rs 🔴 34.29% 🔴 0% 🔴 -34.29%
brush-test-harness\src\config.rs 🟠 70.73% 🟢 78.34% 🟢 7.61%
Overall Coverage 🟢 27.49% 🟢 28.43% 🟢 0.94%

Minimum allowed coverage is 20%, this run produced 28.43%
Maximum allowed coverage difference is -5%, this run produced 0.94%

Test Summary: bash-completion test suite

Outcome Count Percentage
✅ Pass 1571 74.49
❗️ Error 17 0.81
❌ Fail 167 7.92
⏩ Skip 339 16.07
❎ Expected Fail 13 0.62
✔️ Unexpected Pass 2 0.09
📊 Total 2109 100.00

lu_zero added 2 commits August 24, 2026 16:49
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)
@lu-zero

lu-zero commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

Benchmark: bash vs {clap, bpaf, usage} × {fat-LTO, thin-LTO}

Interleaved, pinned sampling (benchmarks/three-way.py, 15 samples, median ± MAD·1.4826), byte-for-byte output parity verified across all shells before timing. All builds from identical release-profile flags; thin-LTO via CARGO_PROFILE_RELEASE_LTO=thin.

Wall time (ms, median)

Workload bash clap-fat clap-thin bpaf-fat bpaf-thin usage-fat usage-thin
startup (-c exit) 1.72 1.73 1.74 1.70 1.76 1.72 1.71
interp-loop (60k) 139.5 273.5 275.3 285.0 286.3 220.2 221.7
wordops (string/array) 150.6 54.4 55.2 53.1 53.8 52.9 53.5
config-lint-500 (getopts-dense) 24.0 95.3 97.1 74.7 75.3 50.9 51.6
deploy-sim 15.9 21.2 21.9 21.0 21.1 19.8 19.6

Takeaways

Parser backends (config-lint, the parse-densest workload):

  • clap → bpaf: bpaf is 1.28× faster (95.3 → 74.7 ms)
  • clap → usage: usage is 1.87× faster (95.3 → 50.9 ms)
  • bpaf → usage: usage is 1.47× faster

The ranking (usage < bpaf < clap) is consistent and well outside the ±0.5% noise floor.

LTO: fat ≈ thin everywhere. Within each backend the two variants sit within 1–3% of each other on every workload, with fat slightly ahead more often than not. No evidence that LTO flavor is a meaningful lever here — and notably, an earlier apparent "+35% pure-interp regression" for bpaf did not reproduce under this harness (interp-loop: clap-fat 273 ms vs bpaf-fat 285 ms, ~+4%), indicating it was a measurement artifact of sequential test-vs-reference sampling on a multi-tenant machine rather than real codegen cost.

Other observations:

  • wordops: all brush builds are ~2.8× faster than bash
  • interp-loop: the usage fork is also faster outside of parsing (220 vs 273–286 ms) — worth investigating what else differs there
  • Startup: all builds within ±3% of bash's process spawn floor
  • Peak RSS: all brush builds within ~0.5 MiB of each other across workloads

Reproduce with:

python3 benchmarks/three-way.py -s bash=/usr/bin/bash \
  -s clap-fat=... -s bpaf-fat=... -s usage-fat=... [--core N] [--samples 15]

@pacak

pacak commented Aug 24, 2026

Copy link
Copy Markdown

Nice-to-have bpaf API changes

Public access to rendered help text — detailed help currently has to be obtained by running run_inner(["--help"]) and unwrapping a ParseFailure. A OptionParser::render_help_string() (monochrome + colored variants) would remove the round-trip.

Problem is OptionParser can contain many nested subparsers, say commands "foo" and "bar" and you can't really access it without running the parser. A helper like that can exist in theory, but to focus on a specific command it will be running the parser anyway.

0.10 should expose a bit more primitives so you can implement some bits, but even then dealing with global parsers will require building the whole context.

Position-aware / sequential parsing primitive — switches scan all unconsumed items regardless of position ("before" = declaration order). Bash-style "stop option parsing at the first operand" (echo hi -n must keep -n literal) cannot be expressed combinatorially today; we pre-split argv ourselves. Something like a positional parser with allow_hyphen_values-plus-trailing_var_arg semantics would let us delete the splitter.

0.10 adds leftovers(). If user passes a bunch of known named items followed by a positional item (and parser doesn't parse positionals) - leftovers() will collect that positional and everything else up to the end.

Per-parser "last occurrence wins" for repeated flags — clap's overrides_with pattern > (pwd -P -L, cd -L -P) can't be expressed; alternatives conflict and independent switches lose ordering. A .conflicts_with_self(LastWins) style modifier would help.

Hmm... Isn't it what .last() does?

https://docs.rs/bpaf/latest/bpaf/trait.Parser.html#method.last

NamedArg aliases & hide — .alias("log-enable") and calling .hide() on an unconsumed NamedArg aren't available; aliases currently require duplicating parsers inside a construct![a, b] alternative and hiding after conversion.

I don't think I understand this :)

Custom metavar on named arguments — metavar() exists on any-based parsers but not on NamedArg before .argument(); being able to set metavar/help/hide uniformly on both classes would reduce helper shims.

metavar is needed to indicate that something is an argument (an option-argument if you use Open group terminology). I'm not sure how ability to change that. Do you have any examples where this can be used?

Man-page / markdown export — clap ecosystem had clap_mangen/clap_markdown. An official (or documented third-party) way to render bpaf parsers to roff/markdown would restore richer doc generation; xtask currently emits from monochrome help text.

Hmm... https://docs.rs/bpaf/latest/bpaf/doc/index.html, requires docgen feature or something like that. Changed a bit in 0.10, but it should still be able to produce roff/markdown.

ParseFailure: std::error::Error (or an into_error()) so ?-style plumbing works in test code without manual matching.

Hmmm... Yeah, I can add that.

@lu-zero

lu-zero commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

NamedArg aliases & hide — .alias("log-enable") and calling .hide() on an unconsumed NamedArg aren't available; aliases currently require duplicating parsers inside a construct![a, b] alternative and hiding after conversion.

clap allows to have aliases, and bpaf apparently not in a as straightforward way, but I can be possibly wrong.

@pacak

pacak commented Aug 24, 2026

Copy link
Copy Markdown

clap allows to have aliases, and bpaf apparently not in a as straightforward way, but I can be possibly wrong.

Each named item can have two visible names (first short and first long) and as many hidden aliases as you want. short('n').short('N').long("name").long("also-name"). Getting them to show in help can be more tricky (but much easier in 0.10), I'm not offering an official way because it gets too wide and too ugly too fast.

@pacak

pacak commented Aug 24, 2026

Copy link
Copy Markdown

https://docs.rs/bpaf/latest/bpaf/fn.long.html

lu_zero added 2 commits August 24, 2026 18:35
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)
@lu-zero

lu-zero commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

Updated, thank you :)

lu_zero added 2 commits August 24, 2026 19:44
…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)
Render shell CLI parse failures through bpaf's print_message (respecting
NO_COLOR / terminal support) unless --disable-color was requested, and
finish the harness TestOptions env-var fallbacks (--bash-path via BASH_PATH
env with CLI precedence).

Assisted-by: ox-alpha (opencode)
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.

2 participants