wert007 / commit-analyzer

Gets the time somebody worked on something from `git log`
MIT License
1 stars 1 forks source link

Use `clap` instead of `getopts` to parse cli args #27

Closed wert007 closed 2 years ago

wert007 commented 2 years ago

clap allows using subcommands, which are helpfull for the input methods. This also makes the main method a lot shorter, since there is no need to add every single command line option per manually. This is handled by the derive feature of clap and the new Args struct.

commit message

I open this pull request to further discuss things and ensure feature equality, before merging it into main.

Pinging @kevinmatthes

kevinmatthes commented 2 years ago

Would you please post the link to the original repository of clap here? I would like to a look on the implemented traits since I experimented with getopts on how the getopts::Options and their getopts::Matches can be stored in structs to thin out the main.

The main problem was that getopts::Options does not implement neither Copy nor Clone, so storing the options in a struct is somehow difficult with the getopts crate. But with the getopts::Matches, everything is alright.

wert007 commented 2 years ago

https://github.com/clap-rs/clap

The nice thing here are two things. The first one is, that the cli args are defined in the rust code itself and are turned into the cli at compile time. This is way shorter then declaring every single option with a function call or something similar. The other being, that the input methods are turned into subcommands, which means, that no check is necessary, if it is specified, or if there are multiple specified.

kevinmatthes commented 2 years ago

By the way: I took a look at the slidesv3 project you referenced above. It might be implemented even "rustier" and more portable with a justfile instead of all these Windows scripts. The justfile is not considered a script at all but triggers the Makefile syntax highlighting of VS Code-like IDEs (as you seem to utilise as of your .gitignore).

Further readings:

wert007 commented 2 years ago

By the way: I took a look at the slidesv3 project you referenced above. It might be implemented even "rustier" and more portable with a justfile instead of all these Windows scripts. The justfile is not considered a script at all but triggers the Makefile syntax highlighting of VS Code-like IDEs (as you seem to utilise as of your .gitignore).

Further readings:

While this might be true, I have multiple reasons against that. For one this project is currently only developed on windows. It is also not really up to collaboration, so it does not really need to be portable at the time being. Also requiring just would add a not really necessary dependency, which I'm not really a fan of.

kevinmatthes commented 2 years ago

You forgot numerous docstrings which still carry Imperative / Present. Since you oversaw them multiple times, I would like to suggest to revert the Indicative / Simple Present. It does not add anything to the source files.