guardian / bucket-blocker

MIT License
0 stars 0 forks source link

Unexpected CLI argument DX #19

Open adamnfish opened 1 week ago

adamnfish commented 1 week ago

The use of a single dash (-) for multi-character flags (instead of --) is unusual! Additionally, dry run looks like it's being treated as a flag but looks a bit strange because it is inverted (defaulting to true, and false when provided). More detail below!

- / --

Generally, conventions around CLI programs are that single hyphens denote flags, or short versions of arguments.

For flags, you can provide one or more each with a -. Multiple flags can be combined after a single dash, so the following two commands are identical:

myprogram -abc
myprogram -a -b -c

Many programs also provide long versions of flags, which can be easier to understand. Here's an example of expanding the a, b and c flags for this hypothetical program.

myprogram --all --backtrack --crazy-mode

This also applies to arguments. A familiar example is that the AWS CLI accepts a profile parameter, which can be provided in short form (-p or long form --profile). This argument expects a value to be provided afterwards, both of the following are equivalent:

aws sts get-caller-identity -p myprofile
aws sts get-caller-identity --profile myprofile

In bucket-blocker, the help text uses a single dash for multi-character flags/arguments. For example, the exclusions flag is documented as the following:

bucket-blocker -exclusions string

But to me, this looks equivalent to passing multiple single character flags, the last of which takes an argument (which is itself a bit weird)!

bucket-blocker -e -x -c -l -u -s -i -o -n -s string

More canonical would be to use -- for the full name of this flag, and provide a short form behind a single hyphen to save typing, if desired.

bucket-blocker --exclusions string
bucket-blocker -x string

Flags defaulting to false

Generally, the presence of a flag enables that flag. This means they implictly default to "false" when absent, so if you want a flag to be an opt-out then it's typically inverted in its naming and meaning.

For example, if you want to have a dry run feature then you can add a --dry-run flag that enables a version of the program that does not perform side effects. However, if you want to have the program run in dry-run mode by default, then the flag will need to disable dry-run mode. AWS use --no-dry-run for this but another approach is to think about what it means for your program to run in real mode and name the flag the positive way for this, --commit for example!

Bucket blocker does this now, and I think this is why its help text is a bit confusing. It seems to imply that dry-run takes a parameter (since it defaults to true), but also looks like a flag that toggles something based on its presence or absence. Without reading the source code, I'd guess that including the -dry-run flag would in fact disable dry-run mode, and have it run in production mode.

marsavar commented 1 week ago

Re: single dash vs double dash, the Go standard library doesn't differentiate between single and double dash. They are treated as equivalent: https://pkg.go.dev/flag#hdr-Command_line_flag_syntax I know this because I had to look it up when the first PR in this repo was opened, as I also found it very confusing, and against the principle of least astonishment. I was expecting it to be the way you described. Afaik when CLIs tend to get more complex, people tend to use third party libraries, and they just seem to proliferate which doesn't make the choice very easy. Of course, If I had to pick, I'd go with Meta's recommendation ;)

Re: flags defaulting to false, I agree. If a boolean flag is absent, it should be treated as false.