Decouple builtins from arg-parsing engine; add per-engine modules for clap, bpaf, usage-rs by lu-zero · Pull Request #1309 · reubeno/brush

lu-zero · 2026-08-26T11:11:00Z

Summary

Decouples builtin argument parsing from the underlying engine. Each builtin declares its arguments as a plain struct in {builtin}.rs; per-engine modules provide instrumentation in {builtin}/{engine}.rs. A cargo feature selects which engine compiles — downstream enables exactly one.

Architecture

brush-builtins/src/
├── {builtin}.rs              ← logic only
├── {builtin}/clap.rs         ← #[derive(Parser)]              [parser-clap, default]
├── {builtin}/bpaf.rs         ← BpafArgs impl                  [parser-bpaf]
└── {builtin}/usage.rs        ← usage::Cli derive              [parser-usage]

arg_impl!(TheType) macro: declares gated sibling modules, re-exports selected type
brush_core::args::FromArgs: engine-neutral contract (words → T or ArgsError)
Exactly-one-engine compile_error! guard
Feature forwarding: brush-shell → brush-builtins → brush-core
Core types (CompleteOption, CompleteAction) instrumented via cfg_attr per parser feature

Benchmark (config-lint.sh, 30 runs)

implementation	engine	median	± σ
upstream main	clap native	71.7 ms	13.1
#1302 bpaf-port	bpaf native	62.6 ms	13.3
experiment/usage-parser	usage native	49.4 ms	9.5
this PR	clap via contract	74.7 ms	14.5
this PR	bpaf via contract	68.6 ms	16.2
this PR	usage via contract	51.9 ms	12.3

The contract layer adds no measurable overhead vs each native implementation.

Engine status

feature	crate	build	compat
`parser-clap` (default)	clap 4.6	✅	✅ 1854/0
`parser-bpaf`	bpaf 0.9.27	✅	needs runtime validation
`parser-usage`	usage-rs 6.1.1	✅	needs runtime validation

What's included

FromArgs trait in brush_core::args
All ~49 builtins converted to per-engine module layout
bpaf engine modules for all builtins
usage-rs engine modules for all builtins
cfg_attr-based type instrumentation for core types (completion.rs)
Feature forwarding chain: brush-shell → brush-builtins → brush-core
Exactly-one-engine compile guard
Compat suite green on default (clap): 1854 passed / 0 failed

Design notes

arg_impl! macro: expands to cfg-gated sibling mod declarations plus an internal imp namespace re-exporting whichever compiled. Factory registrations stay unchanged.
Blanket impl removal path: currently impl<T: clap::Parser> FromArgs for T covers unconverted builtins; removed once migration completes.
Transitional clap_content::<T> shim: renders help from clap metadata for converted builtins until brush grows its own engine-neutral help model.
Core type instrumentation: completion.rs uses cfg_attr(feature = "parser-X", derive(...)) — no standalone modules needed for type-level derives.
Dead-code expectations on parsed-but-unconsumed fields: a few struct fields are parsed for bash compatibility but not consumed by execute logic (e.g., fc -e, enable -p). These carry #[expect(dead_code)]; removing them would change parsing behavior.

Non-clap engine caveats

Help text rendering for bpaf/usage engines falls back to synopsis-based output rather than full detailed help (transitional)
Compat suite has only been run under the default clap engine
CI matrix job needed for three engine configurations

Supersedes #1302.

Assisted-by: ox-alpha (opencode)

Introduces brush_core::args with the contract 'words -> T or ArgsError': builtins will consume only this trait while argument-parsing engines provide implementations. ArgsError distinguishes usage failures from help/version requests. Includes a transitional blanket implementation (clap::Parser types satisfy FromArgs automatically) so existing builtins keep working unchanged during migration; it is removed once migration completes. Assisted-by: ox-alpha (opencode)

Replaces the clap::Parser supertrait on builtins::Command with the new brush_core::args::FromArgs contract, and neutralizes Command::new's error type to ArgsError (usage failure vs help request). Transitional scaffolding, all removable per-builtin as migration proceeds: - blanket impl: clap-derived types satisfy FromArgs unchanged - clap_content helper + one-line get_content impls preserve current help rendering from clap metadata - the four builtins overriding new() (echo/getopts/set/test) keep their '--' workaround via try_parse_known Behavior is unchanged; compat suite: 1854 passed / 0 failed. Assisted-by: ox-alpha (opencode)

echo becomes the reference for the converted shape: - echo.rs holds only the plain argument struct and execute logic; no engine types or derives - args/clap.rs (per-engine module) owns word binding via FromArgs and the mirror type carrying option/help metadata, preserving current output byte-for-byte including the '--' workaround - other engines add their own module (args/bpaf.rs, args/usage.rs) implementing the same FromArgs impls Compat suite: 1854 passed / 0 failed. Assisted-by: ox-alpha (opencode)

… types Per review feedback: instead of mirror structs + conversions, builtin argument structs carry the selected engine's instrumentation directly via cfg_attr, and the parser-clap/parser-bpaf/parser-usage feature trio (exactly-one guard included) selects which set compiles. Consequence: with clap selected, existing derives already satisfy the FromArgs contract through the blanket impl — no per-builtin binding code remains for simple builtins. Only genuine shell quirks (echo's '--' workaround) keep engine-specific overrides, now feature-gated. Help rendering stays on the transitional clap_content path, gated per engine until brush grows its own help model. Selecting an engine without bindings (e.g. parser-bpaf today) fails the build loudly, listing the builtins still lacking support. Assisted-by: ox-alpha (opencode)

Adopts the reviewed layout: each builtin keeps its logic in {builtin}.rs and its engine-specific instrumented structs (plus any impls) in sibling {builtin}/{clap,bpaf,usage}.rs files. A small arg_impl!(TheType) macro declares the engine-gated sibling modules and re-exports the selected engine's type, so builtin code and factory registrations stay engine-agnostic. With clap selected, blanket FromArgs covers plain derives; only shell quirks (echo's '--' handling) live as overrides inside the engine file. Other engines will provide their own struct instrumentation and quirk handling in their own files. Assisted-by: ox-alpha (opencode)

Completes the bulk migration to the reviewed layout: each builtin's logic lives in {builtin}.rs with only pure command behavior, while the engine-instrumented argument struct, trait impls, and any shell-quirk overrides live in {builtin}/clap.rs behind parser-clap. arg_impl! selects the module set and re-exports the type so factory registrations stay unchanged. Notable non-mechanical cases: - declare/export/set/umask: clap-flag helper structs generated by macros (minus_or_plus_flag_arg etc.) move into the engine module - bind/complete/unset: shared arg types referenced from logic gain explicit cross-module imports; fields exposed pub(super) - exec/popd/pushd: cross-builtin invocation switches to UFCS builtins::Command::execute - history/bind: unit tests follow their execute implementations SimpleCommand-style builtins (colon/true/false) have no arguments and remain single-file. Compat suite: 1854 passed / 0 failed. Workspace clippy/fmt clean. (interactive suspend test failure is pre-existing on main) Assisted-by: ox-alpha (opencode)

Infrastructure complete: args support module (BpafArgs trait, runner, splitter, help rendering), optional bpaf dep wired to parser-bpaf, generated modules for 45 structurally-matching builtins plus hand-written cd/pwd/getopts/set adapters. Remaining known issues tracked for next session: complete/bpaf needs FromStr impls + inherent parser impl placement; bind BindError unification; cd mode mapping; set named_option_section construction; assorted import pruning under the bpaf-only feature set. Assisted-by: ox-alpha (opencode)

Resolves the remaining engine-module issues: declaration-command impls transplanted for export/declare/builtin_, tri-state flag structs for the set/declare families via minus_or_plus_flag_bpaf, FromStr for BindKeyMap, per-file import repairs, and cfg-gating of bpaf-only helpers so the clap-default build stays clean. Both cargo configurations now compile: default (parser-clap) and parser-bpaf with all builtin features. Assisted-by: ox-alpha (opencode)

…t green Ports experiment/usage-parser's #[derive(usage::Cli)] instrumentation into per-engine {builtin}/usage.rs modules behind parser-usage: - args/usage_support.rs: UsageArgs trait + impl_usage_parse! macro + runner + splitter + help rendering (ported from that branch's core) - generated modules harvest struct instrumentation from the branch - declaration builtins carry DeclarationCommand impls Remaining known issues (~45 errors, all mechanical): missing helper-type imports (declare/set flag structs via minus_or_plus_flag_bpaf, bind types from clap sibling, complete CommonCompleteCommandArgs placement), duplicate builtins imports, exit String/i64 set_trailing_args leftover, read OpenFile::read trait import, unset shape alignment. Assisted-by: ox-alpha (opencode)

…ors left - ports experiment/usage-parser's 4-arg minus_or_plus_flag_arg macro (usage::Args derive) as usage_minus_or_plus_flag_arg!, wired into set/declare usage modules with accumulated -o/+o SetOption fixes - args/usage_support: correct 6.1.1 error/help rendering - builtin modules exposed pub(crate) so engine modules can reach shared helper types; import dedup across generated files Remaining 9 errors documented in /tmp capture: complete CommonArgs placement + duplicates, bind enum copies, command duplicate inherent, factory bounds following from those. Assisted-by: ox-alpha (opencode)

Assisted-by: ox-alpha (opencode)

Remaining errors are concentrated in three files: bind/usage.rs (BindKeyMap needs usage ValueEnum derive + BindError ownership), command/usage.rs (duplicate inherent 'command' fn), complete/usage.rs (CommonCompleteCommandArgs + CompGen/CompOpt definitions). Fix pattern is proven from unset: shared types move to the parent module pub(crate), engines import them. Assisted-by: ox-alpha (opencode)

- command.rs: cross-builtin .command() helper restored in each engine module; stale renamed method removed from parent - complete/usage.rs: CommonCompleteCommandArgs struct + CompGen/CompOpt definitions added with usage::Cli derives parser-usage still has ~15 architectural errors (ValueEnum impls needed for core types, derive-macro-vs-trait confusion in spec() calls) that require design decisions about type ownership across engines. Assisted-by: ox-alpha (opencode)

Completes the usage-rs engine port: - completion.rs types instrumented via cfg_attr per parser feature (no standalone modules needed — cfg_attr suffices for type-level derives) - brush-core gains parser-clap/bpaf/usage features with optional deps, forwarding from brush-builtins - clap-only code paths in core gated behind parser-clap - bind/usage.rs: BindKeyMap with usage::ValueEnum derive + variant attrs - command/usage.rs: fields pub(crate) for cross-builtin access; inherent helper renamed first_arg to avoid derive collision - complete/usage.rs: CommonCompleteCommandArgs + CompGen/CompOpt with usage::Cli derives, execute bodies transplanted from experiment branch All three configurations build with zero errors: cargo check -p brush-builtins # parser-clap cargo check -p brush-builtins --no-default-features --features parser-bpaf,$FEATS # parser-bpaf cargo check -p brush-builtins --no-default-features --features parser-usage,$FEATS # parser-usage Assisted-by: ox-alpha (opencode)

Assisted-by: ox-alpha (opencode)

github-actions · 2026-08-26T11:29:35Z

Public API changes for crate: brush-core

Added items

+pub mod brush_core::args
+pub struct brush_core::args::ArgsError
+pub brush_core::args::ArgsError::help_request: bool
+pub brush_core::args::ArgsError::message: alloc::string::String
+impl brush_core::args::ArgsError
+pub fn brush_core::args::ArgsError::from_clap_error(&clap_builder::Error) -> Self
+pub fn brush_core::args::ArgsError::help(impl core::convert::Into<alloc::string::String>) -> Self
+pub fn brush_core::args::ArgsError::new(impl core::convert::Into<alloc::string::String>) -> Self
+impl core::error::Error for brush_core::args::ArgsError
+impl core::fmt::Display for brush_core::args::ArgsError
+pub fn brush_core::args::ArgsError::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result
+pub trait brush_core::args::FromArgs: core::marker::Sized
+pub fn brush_core::args::FromArgs::from_args(&[alloc::string::String]) -> core::result::Result<Self, brush_core::args::ArgsError>
+impl<T: clap_builder::derive::Parser> brush_core::args::FromArgs for T
+pub fn T::from_args(&[alloc::string::String]) -> core::result::Result<Self, brush_core::args::ArgsError>
+pub fn brush_core::builtins::clap_content<T: clap_builder::derive::Parser>(&str, &brush_core::builtins::ContentType, &brush_core::builtins::ContentOptions) -> core::result::Result<alloc::string::String, brush_core::error::Error>
+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 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: brush_core::args::FromArgs
-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::args::ArgsError> where I: core::iter::traits::collect::IntoIterator<Item = alloc::string::String>

Performance Benchmark Report

Code Coverage Report: Only Changed Files listed

Package	Base Coverage	New Coverage	Difference
brush-builtins/src/alias/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/bind.rs	🟢 86.05%	🟢 86.15%	🟢 0.1%
brush-builtins/src/bind/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/break_/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/builtin_.rs	🟢 95.24%	🟢 94.44%	🔴 -0.8%
brush-builtins/src/builtin_/clap.rs	🔴 0%	🟠 56.25%	🟢 56.25%
brush-builtins/src/caller/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/cd/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/command/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/complete.rs	🟢 83.15%	🟢 89.82%	🟢 6.67%
brush-builtins/src/complete/clap.rs	🔴 0%	🟠 54.17%	🟢 54.17%
brush-builtins/src/continue_/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/declare.rs	🟢 90.35%	🟢 90.24%	🔴 -0.11%
brush-builtins/src/declare/clap.rs	🔴 0%	🟠 63.16%	🟢 63.16%
brush-builtins/src/dirs/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/dot/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/echo.rs	🟢 89.74%	🟢 86.67%	🔴 -3.07%
brush-builtins/src/echo/clap.rs	🔴 0%	🟢 100%	🟢 100%
brush-builtins/src/enable/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/eval/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/exec/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/exit/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/export.rs	🟢 93.48%	🟢 93.26%	🔴 -0.22%
brush-builtins/src/export/clap.rs	🔴 0%	🟠 56.25%	🟢 56.25%
brush-builtins/src/fc/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/getopts.rs	🟢 93.28%	🟢 93.01%	🔴 -0.27%
brush-builtins/src/getopts/clap.rs	🔴 0%	🟠 69.57%	🟢 69.57%
brush-builtins/src/hash/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/help/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/history/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/kill/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/let_/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/lib.rs	🟠 75%	🔴 39.13%	🔴 -35.87%
brush-builtins/src/mapfile/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/popd/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/printf.rs	🟢 93.58%	🟢 93.75%	🟢 0.17%
brush-builtins/src/printf/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/pushd/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/pwd/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/read.rs	🟢 91.38%	🟢 91.46%	🟢 0.08%
brush-builtins/src/read/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/return_/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/set.rs	🟢 80.18%	🟢 79.19%	🔴 -0.99%
brush-builtins/src/set/clap.rs	🔴 0%	🟢 77.78%	🟢 77.78%
brush-builtins/src/shift/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/shopt.rs	🟢 77.22%	🟢 76.71%	🔴 -0.51%
brush-builtins/src/shopt/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/test.rs	🟢 94.12%	🟢 92%	🔴 -2.12%
brush-builtins/src/test/clap.rs	🔴 0%	🟠 69.57%	🟢 69.57%
brush-builtins/src/times/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/trap/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/type_/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/ulimit.rs	🟠 69.33%	🟠 69.28%	🔴 -0.05%
brush-builtins/src/ulimit/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/unalias/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/unset/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-builtins/src/wait/clap.rs	🔴 0%	🔴 46.15%	🟢 46.15%
brush-core/src/args.rs	🔴 0%	🟢 98%	🟢 98%
brush-core/src/completion.rs	🟠 74.16%	🟠 70.78%	🔴 -3.38%
brush-shell/src/brushctl.rs	🔴 6.9%	🔴 6.38%	🔴 -0.52%
brush-test-harness/src/util.rs	🟠 52.24%	🟠 52.17%	🔴 -0.07%
Overall Coverage	🟢 76.38%	🟢 75.29%	🔴 -1.09%

Minimum allowed coverage is 70%, this run produced 75.29%
Maximum allowed coverage difference is -5%, this run produced -1.09%

Test Summary: bash-completion test suite

Outcome	Count	Percentage
✅ Pass	1583	75.06
❗️ Error	18	0.85
❌ Fail	154	7.30
⏩ Skip	339	16.07
❎ Expected Fail	13	0.62
✔️ Unexpected Pass	2	0.09
📊 Total	2109	100.00

…sions #[expect(...)] errors when the lint condition is unfulfilled, which varies across environments and feature resolutions. #[allow(...)] is the appropriate choice for transitional code. Also fixes clippy const-fn suggestions in declare.rs. Assisted-by: ox-alpha (opencode)

The compile_error! guard prevented cargo from building with --all-features (CI enables all features). Without the guard, the arg_impl! macro's cfg-gated module declarations naturally select only the engines whose features are active; with --all-features, multiple engines compile and the imp glob picks one deterministically. Assisted-by: ox-alpha (opencode)

The nix::sys::stat::Mode import was compiled out on Linux/Android but dead everywhere else, failing non-Linux builds (macOS, BSDs) run with -D warnings. Assisted-by: Maki:zai/glm-5.3-flash

The bpaf/clap/usage engine modules are parallel same-named type hierarchies, and builtin glue code binds against one selected engine, so enabling several together has no coherent interpretation yet (name collisions and monomorphic shared signatures). Enforce exactly-one via compile_error! guards so misuse fails fast with a clear message instead of dozens of downstream errors. This replaces the removed exactly-one-engine guard: wholesale --all-features builds are intentionally rejected now; workspace checks adapt in the following xtask change. Assisted-by: Maki:zai/glm-5.3-flash

check lint/build/unused-deps ran with a blanket --all-features, which now conflicts with the engine exclusivity enforced in brush-builtins (cargo-udeps was the first CI job to trip over it). Sweeps now run four passes that jointly cover the same ground: - one wholesale sweep of every crate except brush-core and brush-builtins (--workspace --exclude ... --all-features), then - one pass per parser engine over just those two crates with explicit, fully-qualified feature selections derived from cargo metadata. Feature lists are resolved dynamically so new features stay covered. Adds unit tests for metadata parsing, qualified feature selection, and sweep construction. Assisted-by: Maki:zai/glm-5.3-flash

Enables ergonomic '?' conversion into anyhow::Result contexts, which engine-agnostic builtin argument tests rely on under any parser engine. Assisted-by: Maki:zai/glm-5.3-flash

Replace the exactly-one-engine compile_error! guards with a fixed priority order when several engine features are enabled at once: parser-usage wins over parser-bpaf, which wins over parser-clap. arg_impl! now declares only the winning engine's sibling module, so a single unambiguous type namespace reaches both parent glue (via imp::) and cross-module re-exports (complete's CompGenCommand/CompOptCommand). Helpers tied to a specific engine (bpaf tri-state helpers, bpaf_support, OptionBoolExt) are now gated on engine *selection* rather than feature presence, keeping them alive exactly when used - including mixed setups like default+parser-bpaf that previously failed to compile. The history parse test now goes through the engine-agnostic FromArgs contract instead of clap's try_parse_from, so it runs under whichever engine is selected. All four feature combinations (default, each single engine, all combined) plus workspace --all-features --all-targets builds now pass; this also restores viability of blanket --all-features CI checks such as cargo-udeps. Assisted-by: Maki:zai/glm-5.3-flash

Parser engines now resolve by fixed priority, so workspace --all-features builds compile again; the per-engine sweep machinery added earlier is no longer needed and the simpler upstream checks (lint/build/unused-deps with --all-features) provide equivalent coverage. Assisted-by: Maki:zai/glm-5.3-flash

bpaf-only builds of the crate (a la carte consumption without builtin.set/builtin.declare) left tri_state_flag and minus_or_plus_flag orphaned, failing -D warnings. Gate their dead-code allowances on consumer presence so lint coverage stays strict in normal builds. Assisted-by: Maki:zai/glm-5.3-flash

Two defects made bpaf builds fail scripts that run fine under the clap/usage engines: - named_option_section kept a flag-less alternative first, so 'set' always parsed named options as Some(empty vec) and entered bash's list-all mode on every invocation, dumping the option tables. - bpaf implements no POSIX short-option clustering, so tokens like '-euo' failed to parse, silently aborting scripts at the ubiquitous 'set -euo pipefail' preamble with exit code 2. Clusters are now pre-split into individual switches (with value-taking 'o' handled at group boundaries), and the section parser gates purely on flag presence. Verified: probe scripts and both real-world benchmark scripts now pass identically under all three engines. Assisted-by: Maki:zai/glm-5.3-flash

Adds parser-clap/parser-bpaf/parser-usage forwarding features so a single source tree can produce binaries with any engine selected, enabling apples-to-apples engine comparisons and targeted testing. Assisted-by: Maki:zai/glm-5.3-flash

make_expectrl_output_readable previously panicked via unwrap() when a shell-under-test produced escaping that differs from expectrl's own (observed when running comparisons against non-default argument parsing engines). Keep the raw text (ANSI-stripped) so the comparison still runs and reports differences instead of aborting the whole run. Assisted-by: Maki:zai/glm-5.3-flash

Compat suite: 82 failures -> 0 (2310 cases; 456 known-fail excluded), matching the clap baseline exactly. Root causes were local port bugs, not usage-rs limitations: - declare/usage.rs: MakeIndexedArrayFlag was registered as 'i', a duplicate of MakeIntegerFlag, leaving 'a' unregistered; this broke '-ai' clusters and misbound '-i' to the array flag. - set/declare: opt into plus-cluster expansion via the new 'plus_options' arm of impl_usage_parse!; expansion now happens in UsageArgs::from_words where bash's boundary semantics (stop at plain word / '-' / '--') are honored, emitting long spellings (--+x) the parser matches. The dispatcher-level rename in builtins.rs was boundary-blind and leaked literal tokens into positionals. - set: bare '-o'/'+o' list-all via default_missing sentinel shared through BARE_OPTION/wants_list_all; route parsing through the trailing-args split so bare 'set --' no longer degrades into a full listing, with value_taking_short_options overridden so '-o NAME' keeps its operand on the options side of the boundary. - split_option_section: treat bare '-' as a boundary (kept verbatim in the trailing stream) and value-taking '--o NAME' longs as consuming their following token. - echo/test/getopts: double_dash = "preserve" so bash-literal '--' operands survive; shared getopts already skips a delivered '--'. - printf: hand-rolled leading-option scanner matching bash's rule (at most one leading '--' dropped, unknown leading options rejected with exit code 2); the generic argv parser stripped every '--'. - complete/compgen enums: usage(rename_all = "lowercase") so BashDefault/dirnames-style bash spellings parse and round-trip. - help topics: usage get_content falls back to concise spec summary when --help is swallowed as a positional instead of erroring (rc 99). Also drop a needless iter().copied().collect() flagged under the new lint surface. Assisted-by: Maki:zai/glm-5.3-flash

Compat suite: 3 printf failures plus 4 unrecorded ones -> 0 (2310 cases; 456 known-fail excluded), matching the clap and usage engine baselines. - printf: replace the generic split-then-parse route with a manual leading-option scanner mirroring bash's rule: only '-v' is accepted in the option zone, at most one *leading* '--' is dropped, unknown leading options are rejected, and an empty operand list is a usage error (exit code 2). The previous route kept the leading '--' in the trailing words, so it was mistaken for the format string. - command: drop one leading '--' from the captured operands when using the bpaf engine, matching clap's separator convention; bash resolves 'command -- cmd' to 'cmd' rather than executing '--'. - completion: teach CompleteOption::from_str the 'bashdefault', 'bash-default', and 'nospace' spellings so compgen/complete accept every bash option word on engines that parse options via FromStr. Assisted-by: Maki:zai/glm-5.3-flash

- echo/clap.rs: replace the strict dead_code expectation on the '-E' parity field with an allow; older toolchains in the supported range treat the derive-written field as live while newer ones consider it dead, so no single expectation holds across versions. - brush-core/brush-builtins manifests: declare bpaf as an ignored normal dependency for cargo-udeps. With all parser-engine features enabled at once, engine selection compiles only the priority winner's argument modules, so the losing engines' optional deps are legitimately unreferenced from udeps' point of view. Verified locally: cargo check/clippy --workspace --all-features --all-targets clean, and cargo +nightly udeps --workspace --all-targets --all-features exits 0. Assisted-by: Maki:zai/glm-5.3-flash

usage-rs (any published 6.x) declares rustc 1.91 as its MSRV while this repo's baseline stays 1.88.0, so a blanket --all-features check cannot resolve on the baseline job. Keep full-features validation on stable and give older toolchains an explicit scope covering the default build plus the remaining parser engines, mirroring how clippy already runs stable-only. Verified locally that all three substitute check invocations resolve. Assisted-by: Maki:zai/glm-5.3-flash

lu_zero added 17 commits August 25, 2026 18:54

wip(usage): bind/complete/unset type alignment; complete family pending

ada4dc3

Assisted-by: ox-alpha (opencode)

feat(shell): add parser-* feature forwarding; restore clap default

9b68502

Assisted-by: ox-alpha (opencode)

chore: remove build artifacts; update .gitignore

2bc3ebf

fix: clean orphaned impl blocks in command engine modules

6879b20

Assisted-by: ox-alpha (opencode)

lu_zero and others added 12 commits August 26, 2026 15:27

fix(builtins): remove unused platform-gated import in umask clap module

7815059

The nix::sys::stat::Mode import was compiled out on Linux/Android but dead everywhere else, failing non-Linux builds (macOS, BSDs) run with -D warnings. Assisted-by: Maki:zai/glm-5.3-flash

feat(core): implement std::error::Error for ArgsError

0eb7d38

Enables ergonomic '?' conversion into anyhow::Result contexts, which engine-agnostic builtin argument tests rely on under any parser engine. Assisted-by: Maki:zai/glm-5.3-flash

lu-zero added 4 commits August 26, 2026 23:21

lu-zero closed this Sep 6, 2026

Navigation Menu

Decouple builtins from arg-parsing engine; add per-engine modules for clap, bpaf, usage-rs - #1309

Decouple builtins from arg-parsing engine; add per-engine modules for clap, bpaf, usage-rs#1309
lu-zero wants to merge 33 commits into
reubeno:mainfrom
lu-zero:wip/bpaf-engine

lu-zero commented Aug 26, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 26, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

lu-zero commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Architecture

Benchmark (config-lint.sh, 30 runs)

Engine status

What's included

Design notes

Non-clap engine caveats

Uh oh!

github-actions Bot commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Public API changes for crate: brush-core

Added items

Changed items

Performance Benchmark Report

Code Coverage Report: Only Changed Files listed

Test Summary: bash-completion test suite

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

lu-zero commented Aug 26, 2026 •

edited

Loading

github-actions Bot commented Aug 26, 2026 •

edited

Loading