Replace clap with bpaf

[pacak]

So I did a thing and tried to replace clap's build API with bpaf's one.

Summary changes:

• Changed to use PathBuf instead of going though String.

  You can check the difference by running cargo run --release --example addr2line -- --exe $( echo "\xe9" ), older version would die complaining about non-utf8 symbols, new version would complain about missing file. This can be somewhat important on windows or with strange locales on unix.

• Collected options in a struct and made them strictly typed.

  Previous version used strings to refer to names and while everything seemed worked fine it is way too easy to make a typo while adding a new field defining it as "help" and asking for it later as "hepl". Similar with types. Nowpretty is bool, not something you need to ask for separately.

• I changed a bit how addresses are processed, output seems to match, but let me know - I can replicate the old behavior exactly.

• Version flag now works and takes version from Cargo.toml, current version is 0.18.0

• Clean release compilation time for the whole project went down from 14 to 10 seconds on my machine.

• Executable size went down from 26Mb to 14Mb

• --help looks a bit different and no longer green.

% cargo run --release --example addr2line -- --help
A fast addr2line Rust port

Usage: [-a] [-f] -e filename [--sup filename] [-p] [-C] [-s] [-i] [-L] <addrs>...

Available positional items:
    <addrs>  Addresses to use instead of reading from stdin.

Available options:
    -a, --addresses       Display the address before the function name, file and line number information.
    -f, --functions       Display function names as well as file and line number information.
    -e, --exe <filename>  Specify the name of the executable for which addresses should be translated.
        --sup <filename>  Path to supplementary object file.
    -p, --pretty-print    Make the output more human friendly: each location are printed on one line.
    -C, --demangle        Demangle function names.
                          Specifying a specific demangling style (like GNU addr2line) is not supported. (TODO)
    -s, --basenames       Display only the base of each file name.
    -i, --inlines         If the address belongs to a function that was inlined, the source information for
                          all enclosing scopes back to the first non-inlined function will also be printed.
    -L, --llvm            Display output in the same format as llvm-symbolizer.
    -h, --help            Prints help information
    -V, --version         Prints version information

PS: https://crates.io/crates/bpaf - something I made, this code uses combinatoric API, you can find a tutorial here: https://docs.rs/bpaf/0.5.3/bpaf/_derive_tutorial/index.html

I'm happy to answer any questions you might have.

[philipc]

Hi, thanks for the PR. The CI failure is probably unrelated to this PR.

Many of these changes certainly sound useful. However, I'm reluctant to switch this crate to use a dependency that appears to have few other users at this time. The example application is not an area where I want to be breaking new ground. Which of these changes would not be possible to achieve while still using clap? Is bpaf viable as a replacement for all users of clap?

[pacak]

However, I'm reluctant to switch this crate to use a dependency that appears to have few other users at this time.

Fair enough, I won't be objecting to closing this PR.

Which of these changes would not be possible to achieve while still using clap?

From developer point of you to get typo safety you'll have to switch to derive version. Big things you won't be able to achieve is compilation time and binary size.

Is bpaf viable as a replacement for all users of clap?

Getting there, in fact this PR served it's purpose for this as well - I'm looking at real world examples. Currently released version lacks some flexibility in filename parsing - this was fixed in unpublished version, with that in place it should have feature parity, at least as far as long as we are comparing base crates. Planning to release a new version it once I finish implementing dynamic shell completion and we'll see what kind of feedback I'll get from clap maintainers. So maybe wait a bit and I'll post a link to the discussion once I have it.

[pacak]

https://www.reddit.com/r/rust/comments/wvp9ao/yet_another_command_line_argument_parser_bpaf_052/ - discussion from the previous release and everything other than -fbar and colors can be achieved.

[pacak]

Released a new version, here's a discussion:
https://www.reddit.com/r/rust/comments/x3z5xn/yet_another_command_line_argument_parser_bpaf_055/

But as I said earlier - no objection to closing this PR, it served it's purpose for me.

[pacak]

Also added support for shell completion - because why not :)

[philipc]

Thanks, I am interested in this. I'll review it later.

[philipc]

From developer point of you to get typo safety you'll have to switch to derive version. Big things you won't be able to achieve is compilation time and binary size.

I think this is a good enough reason to change. Thanks!

[pacak]

Would you be interested in similar changes to other projects in gimli-rs organization? gimli seems to be using getopts to handle them doing more stringly typed things. Might take a week or two though.

[philipc]

Thanks, I'll merge this once I have CI working again.

Would you be interested in similar changes to other projects in gimli-rs organization? gimli seems to be using getopts to handle them doing more stringly typed things. Might take a week or two though.

Let's leave that for now. I think gimli is simple enough how it is. ddbug might benefit, but it's certainly not a priority.

[philipc]

Sorry for taking so long to merge this. I've changed my mind and I'm going to stay with clap for now. I've incorporated some of the improvements from this PR in #266. Thank you very much for working on these.

A couple of notes about this PR:

Executable size went down from 26Mb to 14Mb

This includes debuginfo, which isn't as relevant.

I changed a bit how addresses are processed, output seems to match

This isn't matching for the case where the address is invalid. I think it's better to leave it how it was.