cr / hx870

Python tools for the Standard Horizon HX870
GNU General Public License v3.0
21 stars 5 forks source link

Confusing -d flag #4

Closed johannessen closed 5 years ago

johannessen commented 5 years ago

The new CLI offers a -d flag, and so does the config command. As a result, for that command the meaning of -d depends upon the position of that flag in the command line:

hx870 -d config test.dat  # means --debug
hx870 config -d test.dat  # means --dump

While the former triggers an error because config requires either --dump or --flash, this situation seems confusing to me.

Personally, I’d much prefer --read/-r and --write/-w over --dump and --flash. I do understand that “read” and “write” are in theory ambiguous with regards to the direction of the data transfer (i. e. “write” might mean writing to the HX870’s flash memory, or writing to the computer’s hard drive), but in practice, when working with pyhx870, I think about the radio, not the computer. (YMMV, obviously.)

Besides, I’m not sure “dump” is any clearer (i. e., will it dump data into the HX870 or into a local file?).

cr commented 5 years ago

Thanks for catching this! Those args are unfortunately positional (which in this case prevents ambiguity). I could live with this, but would propose removing the -d shortcut from the --debug option if it's really an issue.

The read/write terminology is a very old debate when it comes to external devices. I have seen both read/write and upload/download being used for either direction, so I strongly prefer staying away from those. Contrary, I have never seen dump being used for anything but reading data from an external device and writing it to a local file. Especially with the other direction being called flash there's not a single doubt on my mind to which is which.

johannessen commented 5 years ago

Right. I’m not sure either how big this issue is. I guess whether or not it warrants a change is in large part a matter of taste. I’m sure I’ll get used to flash and dump eventually. Feel free to close this as “won’t fix” if you like.

Should you decide to make a change, a possible alternative to removing -d might be to change --debug to --verbose. The shortcut -v is unused at the moment.

cr commented 5 years ago

This was fixed in #7 by simply removing the -d shortcut for --debug.

I have always hated that global commands are forced to be given before the subcommand, easily leading to such confusion, but unfortunately it's just the way Python's argparser works (or at least I haven't figured out how to work around).