Skip to content

Conversation

@mo8it
Copy link
Contributor

@mo8it mo8it commented Aug 26, 2023

After porting to Clap in #1633, @pacak asked me on Mastodon to try bpaf.

Here is the result:

image

Compile time

Currently, bpaf has two dependencies less than Clap. The difference is not significant.

The benchmark:

$ hyperfine -- "rm -r target && cargo b -r"
Benchmark 1: rm -r target && cargo b -r
  Time (mean ± σ):     12.431 s ±  0.164 s    [User: 97.754 s, System: 6.167 s]
  Range (min … max):   12.085 s … 12.616 s    10 runs

bpaf has 8% additional compile time instead of the 15% of Clap. But remember, we are talking about one additional second instead of two of a compilation that is done only once!

Which to choose?

Personally, I like the output of bpaf more since it has colors, but if you don't like the colors and want only bold and underline highlightings, you can use the bpaf feature dull-color instead of bright-color.

The code quality is very similar to Clap. It doesn't have the boilerplate of argh.

I don't like that the Subcommand enum has to implement Clone. @pacak said that it is required to be able to run the parser multiple times, but I still think that it could be improved.

I will let you decide which one to merge :)

Personally, I will go with bpaf for my temporary fork for my Rust course. Just for the nice colors 🌈

@mo8it mo8it mentioned this pull request Aug 26, 2023
@pacak
Copy link

pacak commented Aug 26, 2023

Outside of the scope of porting specifically but there are a few things that can be improved:

  1. normalize data the app gets a bit more. Consider --solved and --unsolved flags in list. They are mutually exclusive and passing both doesn't really make much sense. From the app point of view it might make sense to represent this filter as something like this:

     #[derive(Debug, Clone, Bpaf)]
     #[bpaf(fallback(SolvedFilter::Everything))]
     enum SolvedFilter {
         /// Show solevd only
         Solved,
         /// Show unsolved only
         Unsolved,
         #[bpaf(skip)]
         Everything,
     }

    User will still get to pass either of --solved or --unsolved, but not both, the app will get a nice enum.
    Or --nocapture feels strange sitting at the top level.

  2. Generate and include command reference - something like here https://crates.io/crates/cargo-show-asm - basically help for all the commands accessible without having to run anything

  3. Generate and include shell completion at least with nix. And add instructions for users how to add them if they are not using nix. Mostly for conveniece, too lazy to type :)

I can help with some or all those items.

@epage
Copy link

epage commented Aug 26, 2023

imo bpaf isn't ready for introductory material (yet), see nicoburns/blessed-rs#90

I'm hoping we can work together to get it there.

@mo8it
Copy link
Contributor Author

mo8it commented Aug 26, 2023

After publishing this second PR, I started a vote on Mastodon about which version people prefer:
https://fosstodon.org/@mo8it/110955677826536002

Calp won after 18 votes. But the decision is left to the maintainer @shadows-withal

@manyinsects
Copy link
Member

I would also tend more towards using Clap, for the reasons discussed, but also I believe that it's easier to reason about, which improves the contributor experience.

@manyinsects
Copy link
Member

Closing as the clap PR is landed

@manyinsects manyinsects closed this Sep 4, 2023
@mo8it mo8it deleted the bpaf branch March 25, 2024 01:04
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.

4 participants