gnirehtet icon indicating copy to clipboard operation
gnirehtet copied to clipboard

uses clap instead of custom cli arg parsing

Open kbknapp opened this issue 5 years ago • 2 comments

This commit uses clap to handle command line arugments instead of a custom rolled solution. The benefit that clap handles all validation as well as providing many "quality of life" or UX improvements such as hidden alias, suggestions on typo, auto-generated help, etc.

A previous branch of this project used structopt (which internally uses clap). structopt makes heavy use of Rust's Custom Derive feature to make the code less verbose and more ergonomic. clap is nearing it's v3 release, which pulls structopt into the mainline code base. This commit does not use the v3 structopt-like feature-set since that release is still in alpha. This meanas the code is more verbose (yet just as performant), and could be changed to the structopt-like features once clap v3 is officially released.

This commit also does not utilize some of clap's other features like the ability to generate Shell Completion scripts which could be added later if desired.

This commit is nearly a 1:1 translation from the original home rolled, to using clap. I say "nearly" because clap adds many features on top of what the home rolled solution provided "for free" as in with no additional configuration. Those features include:

  • Argument validation: --routes, --dns-servers, and --port all now have built in validation. If someone enters an IP that isn't valid, or a port that isn't valid an error is returned with a friendly message prior to advancing into gnirehtet code. --routes also checks the CIDR for validity as well.
  • Auto generated help message
  • Suggestions on typos (this feature could be turned off to reduce the binary size, see below)
  • Hidden Command aliases; The following hidden aliases have been added for commands:
    • rt which maps to run: I noticed gnirehtet rt used to be used for gnirehtet run
  • Long options for arguments (i.e. --port as well as -p)
  • Hidden Argument aliases; the following hidden aliases have been added for arguments (which provides help for common typos):
    • --dns-server -> --dns -> --dns-servers: All of these map to the same thing
    • --route -> --routes
  • Long and Short help messages: clap allows one to specify a short message for arguments and commands when -h is used, and display a longer more comprehensive message when --help is used.
  • Multiple ways to get help: clap provides -h|--help for the base gnirehtet command, as well as all it's subcommands, i.e. gnirehtet run --help. There is also a help subcommand gnirehtet help which can be used alone or to display other commands, i.e. gnirehtet help run. This helps because people assume various forms of help, and should be able to find the help messages no matter how they originally try it.

Binary Size

I see that binary size is a concern. This is the size after this commit:

❯ ls -l target/release/gnirehtet
.rwxrwxr-x 2.2M kevin 23 Feb 13:22 target/release/gnirehtet
❯ strip -g target/release/gnirehtet
❯ ls -l target/release/gnirehtet
.rwxrwxr-x 1.1M kevin 23 Feb 13:22 target/release/gnirehtet
❯ strip -s target/release/gnirehtet
❯ ls -l target/release/gnirehtet
.rwxrwxr-x 973k kevin 23 Feb 13:22 target/release/gnirehtet

So it does increase the binary size, but not dramatically. By further turning off clap's default cargo features, the size may be able to be reduced even more. However, I also am a firm believer that these slight increases are well worth the features they provide.

Relates to #243

kbknapp avatar Feb 23 '20 19:02 kbknapp

Oh, that's great, thank you! :+1:

I'll review as soon as I have time to work on gnirehtet. :wink:

rom1v avatar Feb 23 '20 19:02 rom1v

Hi,

Sorry for the delay.

I quickly looked at it today, but I have this issue:

$ ./gnirehtet uninstall
error: The argument 'port' wasn't found

This is weird, because "port" is not passed as argument to "uninstall" in clap.

Btw, I rebased on dev (I also replaced 24.24.24.24/8 (which is not valid) by 192.168.0.0/16 in the doc): pr271.

rom1v avatar Aug 08 '20 13:08 rom1v