Skip to content

feat(builtins): port ls argument surface from uutils via codegen#1560

Merged
chaliy merged 1 commit intomainfrom
feat/issue-1531-ls-codegen
May 6, 2026
Merged

feat(builtins): port ls argument surface from uutils via codegen#1560
chaliy merged 1 commit intomainfrom
feat/issue-1531-ls-codegen

Conversation

@chaliy
Copy link
Copy Markdown
Contributor

@chaliy chaliy commented May 6, 2026

Summary

Pull uu_ls's full clap argument definition into bashkit so scripts get the same flag set, abbreviations, and validation as GNU coreutils, while bashkit's existing rendering pipeline keeps emitting the bytes it always did. Flags accepted by uu_ls but not yet implemented return an explicit "not yet implemented" diagnostic (same pattern used by tac -b/-r/-s) instead of silently producing wrong output.

Why

Closes #1531. crates/bashkit/src/builtins/ls.rs previously parsed only 8 short flags (-l -a -h -1 -R -t -F -C) by hand and rejected everything else as invalid option. Scripts that called common GNU-ls flags (-Q, --quoting-style=, --group-directories-first, --reverse, etc.) hit a confusing parse-error path. With this PR, the clap surface matches uutils' and unimplemented flags get a deterministic "not yet implemented" message that names the offending flag.

How

Codegen-tool upgrades (bashkit-coreutils-port)

ls's structure broke three implicit assumptions in the existing tool:

  • Sibling-file mod options lookup. ls's options module lives in config.rs, not ls.rs. New find_mod_in_sibling_files() falls back to scanning every .rs in the util's src/ directory before bailing.
  • Nested submodules (options::format::COLUMNS, options::sort::TIME, options::indicator_style::CLASSIFY, …) — emit verbatim and resolve via a new conditional use options::*; glob in the generated preamble. Bare-name-const utils (mktemp, realpath, …) are unaffected because the glob is skipped when there's no mod options.
  • ShortcutValueParser (uucore-internal type) — substituted with clap::builder::PossibleValuesParser::new(...). Loses uutils' unambiguous-abbreviation behaviour in exchange for clap's exact-match semantics (documented divergence in the spec; out-of-scope per Port ls argument surface from uutils via codegen #1531's "Out of scope" section).

Generated preamble now imports the broader clap::builder surface (PossibleValue, PossibleValuesParser, NonEmptyStringValueParser). The change is benign for the previously-ported utils — they re-generate identically modulo the new imports (silenced by #![allow(unused_imports)]).

Cargo.toml: enable clap's env feature so Arg::env("TIME_STYLE") from uu_ls compiles.

ls.rs migration

Switches argument parsing to ls_command().try_get_matches_from(). The existing rendering loop is untouched. The 8 already-implemented flags are read off matches. Every other ID that clap saw a non-default value for hits a single not yet implemented branch:

ls: option(s) not yet implemented in bashkit: <ids>

-F/--classify carries an optional always|auto|never value — treated as on for always/auto/missing, off for never.

GNU help layout (Usage: first) re-applied via the same .help_template(...) chain cat.rs and tac.rs use.

Tests

  • crates/bashkit/tests/spec_cases/bash/ls.test.sh adds ls_quote_name_short_flag (-Q), ls_quoting_style_long_flag (--quoting-style=shell), ls_group_directories_first_flag, ls_composite_unsupported_then_supported (-lr), ls_unknown_flag_rejected, and ls_version. Each ### bash_diff-annotates the GNU divergence per AGENTS.md.
  • All 70 existing builtins::ls::* lib tests pass; test_ls_invalid_option updated for clap's "unexpected argument" wording.
  • bash_spec_tests integration suite passes.

Binary size delta

cargo build -p bashkit-cli --release:

size (bytes)
baseline (origin/main) 24,331,072
this PR 24,472,000
delta +140,928 (+138 KB)

Driven by clap's env feature + the 949-line generated ls_args.rs (full 80+ flag definition compiled into the binary). Reasonable for a fully-wired uutils argument surface and well below the cat/tac POC's "+9 KB declared-but-unused" baseline projection scaled to ls's flag count.

Out of scope (per #1531)

  • Implementing every uu_ls option (--context, --si, --block-size, full coloration). They stay parser-accepted with the explicit "not yet implemented" error.
  • Vendoring uucore::quoting-style for --quoting-style semantics — separate question, see the module-vendor mode issue (Add module-vendor mode to bashkit-coreutils-port #1533).
  • Full ShortcutValueParser semantics — see codegen substitution note above.

coreutils-args-drift.yml will pick up ls_args automatically via the existing pub mod ls_args; line — no workflow edit required.

Pre-PR

  • cargo fmt --check
  • cargo clippy -p bashkit --features http_client --all-targets -- -D warnings
  • cargo test -p bashkit --features http_client --lib — 2264 pass, 0 fail ✅
  • cargo test -p bashkit --features http_client --test spec_tests bash_spec_tests
  • TM-INF-022 static check (no_debug_fmt_in_builtin_source) passes — no {:?} in builtin source.

Generated by Claude Code

Pull uu_ls's full clap argument definition into bashkit so scripts get
the same flag set, abbreviations, and validation as GNU coreutils,
while bashkit's existing rendering pipeline keeps emitting the bytes
it always did. Flags accepted by uu_ls but not yet implemented return
an explicit "not yet implemented" diagnostic (same pattern used by
tac -b/-r/-s) instead of silently producing wrong output.

Codegen-tool upgrades needed for ls:

- Sibling-file mod-options lookup: ls's options module lives in
  config.rs, not ls.rs. find_mod_in_sibling_files() falls back to
  scanning every .rs in the util's src/ directory before bailing.
- Nested submodules in mod options (options::format::COLUMNS,
  options::sort::TIME, etc.) emit verbatim and resolve via the new
  conditional 'use options::*;' glob in the generated preamble.
- ShortcutValueParser::new(...) → clap::builder::PossibleValuesParser::new(...).
  uucore's unambiguous-abbreviation behaviour is dropped in favour of
  exact-match clap semantics; documented divergence in the spec.
- Generated preamble now imports the broader clap::builder surface
  (PossibleValue, PossibleValuesParser, NonEmptyStringValueParser).
- Cargo.toml: enable clap's 'env' feature for Arg::env("TIME_STYLE").

ls.rs now calls ls_command().try_get_matches_from() and reads the 8
short flags it already implemented (-l -a -h -1 -R -t -F -C). Any
other flag that clap parses triggers an early not-yet-implemented
exit. The existing rendering loop is untouched.

Spec tests in tests/spec_cases/bash/ls.test.sh cover -Q,
--quoting-style=, --group-directories-first, an unsupported short
flag inside a composite (-lr), the unknown-flag path, and ls --version.

Release-build size delta on bashkit-cli: 24,331,072 → 24,472,000 bytes
(+138 KB), driven by clap's env-feature + the 949-line generated file.

Closes #1531
@cloudflare-workers-and-pages
Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
bashkit 2c988a9 Commit Preview URL

Branch Preview URL
May 06 2026, 02:53 PM

@chaliy chaliy merged commit 80b202f into main May 6, 2026
34 checks passed
@chaliy chaliy deleted the feat/issue-1531-ls-codegen branch May 6, 2026 15:08
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.

Port ls argument surface from uutils via codegen

1 participant