Skip to content

Use published clap-sort crate instead of inlined module#409

Merged
jdx merged 2 commits intomainfrom
use-clap-sort-crate
Apr 12, 2026
Merged

Use published clap-sort crate instead of inlined module#409
jdx merged 2 commits intomainfrom
use-clap-sort-crate

Conversation

@jdx
Copy link
Copy Markdown
Owner

@jdx jdx commented Apr 12, 2026

Summary

`src/clap_sort.rs` has been duplicating the logic of the published `clap-sort` crate since the crate was released. This PR deletes the 461-line inlined copy and depends on `clap-sort = "1"` as a dev-dependency instead.

The two implementations are functionally equivalent for fnox's current CLI (the test passes both before and after), but the published crate has one improvement the inlined version didn't:

  • Positional args keep source order. The inlined version asserts positional args be alphabetical by id, which conflicts with clap's positional-parsing semantics (e.g. a `trailing_var_arg` must come last). The published crate documents this explicitly at `src/lib.rs:152`:

    ```
    // Note: We don't check if positional args are sorted - their order matters for parsing
    ```

    fnox's CLI happens not to have multi-positional commands today, so the inlined rule never fires, but it would break if one were added.

Changes

  • Delete `src/clap_sort.rs` (461 lines)
  • Drop `mod clap_sort;` from `src/lib.rs` and `src/main.rs`
  • `src/commands/mod.rs`: `test_cli_ordering` now calls `clap_sort::assert_sorted(&Cli::command())`
  • Add `clap-sort = "1"` to `[dev-dependencies]`

Test plan

  • `cargo test test_cli_ordering` — passes on both `lib` and `bin` targets
  • `cargo clippy --all-targets` — no new warnings (3 pre-existing `assert_eq!` warnings are unrelated)
  • `cargo fmt --check` — clean

🤖 Generated with Claude Code


Note

Low Risk
Low risk: replaces an internal test-only CLI ordering validator with the published clap-sort dev-dependency; runtime code paths are unchanged aside from test behavior.

Overview
Switches CLI ordering validation tests from an in-repo src/clap_sort.rs implementation to the published clap-sort crate (clap_sort::assert_sorted).

Removes the inlined 461-line module and its references from lib.rs/main.rs, and adds clap-sort = "1" as a dev-dependency (updating Cargo.lock accordingly).

Reviewed by Cursor Bugbot for commit 17f2491. Bugbot is set up for automated code reviews on this repo. Configure here.

The clap-sort crate on crates.io (https://github.com/jdx/clap-sort) is
the maintained upstream version of what src/clap_sort.rs duplicated.
It already handles positional args correctly (source order preserved
for parsing semantics; only short/long flags are sorted).

Delete the 461-line inlined copy and depend on `clap-sort = "1"` as a
dev-dependency instead. The `test_cli_ordering` test now calls
`clap_sort::assert_sorted(&Cli::command())`.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 12, 2026

Greptile Summary

Replaces the 461-line inlined src/clap_sort.rs module with the published clap-sort = "1" dev-dependency, updating the test_cli_ordering test to call clap_sort::assert_sorted. The core migration is clean and correct; all runtime code paths are unchanged.

The PR also includes changes to docs/public/schema.json that reorder ProviderConfig.oneOf entries but are not described in the PR description — worth confirming these are intentional.

Confidence Score: 5/5

Safe to merge — runtime code paths are entirely unchanged; only the test-only CLI-ordering validator is swapped for its published equivalent.

All remaining findings are P2. The schema.json reordering is functionally equivalent (JSON Schema oneOf is order-agnostic for validation), and the core clap-sort migration is correct, clean, and well-scoped.

docs/public/schema.json — worth confirming the provider ordering change is intentional and not incidental noise from another branch.

Important Files Changed

Filename Overview
src/clap_sort.rs 461-line inlined module deleted entirely; replaced by the published crate.
src/commands/mod.rs Test updated to call clap_sort::assert_sorted(&Cli::command()) — correct use of the published crate's API.
Cargo.toml Adds clap-sort = "1" under [dev-dependencies] as intended.
docs/public/schema.json ProviderConfig.oneOf entries reordered (pure positional shuffle, no content changes per provider) — not mentioned in the PR description.
src/lib.rs Removes mod clap_sort; declaration cleanly.
src/main.rs Removes mod clap_sort; declaration cleanly.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[test_cli_ordering] -->|before| B[src/clap_sort.rs
assert_command_order]
    A -->|after| C[clap-sort v1.0.3
clap_sort::assert_sorted]
    B --> D[461 lines removed]
    C --> E[Cargo.toml dev-dep]
    E --> F[Cargo.lock locked to 1.0.3]
Loading

Fix All in Claude Code

Reviews (2): Last reviewed commit: "[autofix.ci] apply automated fixes" | Re-trigger Greptile

@socket-security
Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedcargo/​clap-sort@​1.0.310010093100100

View full report

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request replaces the internal clap_sort module with the external clap-sort crate. The changes include adding the dependency to Cargo.toml and Cargo.lock, deleting the local implementation, and updating the CLI ordering test to use the external crate. A review comment suggests removing a redundant code comment in the test file.

Comment thread src/commands/mod.rs
// Validate that CLI commands and arguments are properly sorted
// according to clap_sort conventions
crate::clap_sort::assert_command_order(&Cli::command());
// using the published clap-sort crate.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The comment 'using the published clap-sort crate.' is redundant as the code change itself makes this clear. Please remove this comment to keep the code clean.

@jdx jdx merged commit 6556bf8 into main Apr 12, 2026
14 checks passed
@jdx jdx deleted the use-clap-sort-crate branch April 12, 2026 11:25
jdx pushed a commit that referenced this pull request Apr 21, 2026
### 🚀 Features

- Powershell integration by [@nbfritch](https://github.com/nbfritch) in
[#421](#421)

### 🐛 Bug Fixes

- **(Windows)** Nushell integration by
[@john-trieu-nguyen](https://github.com/john-trieu-nguyen) in
[#425](#425)
- **(Windows)** Command resolution for executables by
[@john-trieu-nguyen](https://github.com/john-trieu-nguyen) in
[#427](#427)

### 📚 Documentation

- add releases nav and aube lock by [@jdx](https://github.com/jdx) in
[#422](#422)
- include linux native packages in aube lock by
[@jdx](https://github.com/jdx) in
[#423](#423)

### 🔍 Other Changes

- Use published `clap-sort` crate instead of inlined module by
[@jdx](https://github.com/jdx) in
[#409](#409)
- add communique 1.0.1 by [@jdx](https://github.com/jdx) in
[#424](#424)

### 📦️ Dependency Updates

- lock file maintenance by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#381](#381)
- update taiki-e/upload-rust-binary-action digest to 10c1cf6 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#383](#383)
- update rust crate tokio to v1.51.1 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#384](#384)
- update rust crate indexmap to v2.14.0 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#385](#385)
- update rust crate rmcp to v1.4.0 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#389](#389)
- update rust crate strum to 0.28 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#395](#395)
- update rust crate toml_edit to 0.25 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#396](#396)
- update rust crate miniz_oxide to 0.9 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#390](#390)
- update rust crate ratatui to 0.30 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#392](#392)
- update actions/checkout action to v6 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#397](#397)
- update actions/deploy-pages action to v5 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#399](#399)
- update actions/configure-pages action to v6 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#398](#398)
- update actions/setup-node action to v6 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#400](#400)
- update actions/upload-pages-artifact action to v4 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#401](#401)
- update dependency node to v24 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#403](#403)
- update apple-actions/import-codesign-certs action to v6 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#402](#402)
- update nick-fields/retry action to v4 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#406](#406)
- update github artifact actions (major) by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#404](#404)
- update jdx/mise-action action to v4 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#405](#405)
- update rust crate which to v8 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#408](#408)
- update rust crate usage-lib to v3 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#407](#407)
- bump rustcrypto stack (aes-gcm, sha2, hkdf) together by
[@jdx](https://github.com/jdx) in
[#410](#410)
- update rust crate reqwest to 0.13 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#393](#393)
- update rust crate libloading to 0.9 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#388](#388)
- update rust crate keepass to 0.10 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#387](#387)
- update rust crate rand to 0.10 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#391](#391)
- lock file maintenance by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#411](#411)
- update rust crate google-cloud-secretmanager-v1 to v1.8.0 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#415](#415)
- update actions/upload-pages-artifact action to v5 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#418](#418)
- update rust crate rmcp to v1.5.0 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#416](#416)
- update rust crate clap to v4.6.1 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#413](#413)
- update rust crate tokio to v1.52.1 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#417](#417)
- update rust crate keepass to v0.10.6 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#414](#414)
- update taiki-e/upload-rust-binary-action digest to f0d45ae by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#419](#419)
- update rust crate aws-sdk-sts to v1.102.0 by
[@renovate[bot]](https://github.com/renovate[bot]) in
[#420](#420)

### New Contributors

- @john-trieu-nguyen made their first contribution in
[#427](#427)
- @nbfritch made their first contribution in
[#421](#421)
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