You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Standardize the notations used for the usage strings in our CLI commands.
Problem Definition
The usage strings of our CLI commands use different notations. It's not always clear what's an optional field and what's required, or what's a specific string vs a value to be filled in.
Once #1468 is done, and it's easier to see command usages all in one place, I'm sure this will become more noticeable.
Proposal
Evaluate all of our commands and update them to use standard notations. This issue might be better suited to be an epic with sub-issues for specific modules even subsets of commands. That decision can be made later, though. This should be done to all commands that we have control over. I.e. don't update the SDK commands (e.g. bank send) unless they're commands that we added to it (e.g. the x/quarantine commands). Any commands defined in this repo should be part of this.
Cobra has this in the documentation on the Use field:
[ ] identifies an optional argument. Arguments that are not enclosed in brackets are required.
... indicates that you can specify multiple values for the previous argument.
| indicates mutually exclusive information. You can use the argument to the left of the separator or the
argument to the right of the separator. You cannot use both arguments in a single use of the command.
{ } delimits a set of mutually exclusive arguments when one of the arguments is required. If the arguments are
optional, they are enclosed in brackets ([ ])
One addition I'd make is to use <> around field names to differentiate a field name and a possible string.
The {add|remove} part is correct since {} delimits a set of mutually exclusive arguments where one is required, and since it must be either exactly "add" or "remove", there shouldn't be any other delimiters in the message.
In that, I added some extra spacing around the options in the outside {} because I felt it made it easier to read and understand. Specifically, that 2nd option is easy to get lost in and not quite realize what's part of that 2nd option, and where the 3rd option begins. This is pretty subjective though. But the goal is to make the commands easier to learn about and understand.
I personally don't think the [flags] part should be included as I feel that the ability to provide flags is a given (i.e. It's understood that flags can be provided to CLI commands), and the flags don't have to go in a specific place in the command. I know the SDK includes it for most commands, but I feel we can leave it out.
Any flags that are booleans should have a default of false. In some cases, this might require renaming the flag and/or adjusting logic. If a boolean flag has a default of true then not providing the flag with a command is the same as providing it. The only way behavior changes is if you include =false on it, which isn't really documented anywhere, and isn't necessarily a standard CLI thing. An example of a flag that should be renamed or require logic changes would be an --optional flag with a default of true. In that case, the flag should be renamed to --required so it can have a default of false.
For Admin Use
Not duplicate issue
Appropriate labels applied
Appropriate contributors tagged
Contributor assigned/self-assigned
The text was updated successfully, but these errors were encountered:
Summary
Standardize the notations used for the usage strings in our CLI commands.
Problem Definition
The usage strings of our CLI commands use different notations. It's not always clear what's an optional field and what's required, or what's a specific string vs a value to be filled in.
Once #1468 is done, and it's easier to see command usages all in one place, I'm sure this will become more noticeable.
Proposal
Evaluate all of our commands and update them to use standard notations. This issue might be better suited to be an epic with sub-issues for specific modules even subsets of commands. That decision can be made later, though. This should be done to all commands that we have control over. I.e. don't update the SDK commands (e.g.
bank send
) unless they're commands that we added to it (e.g. thex/quarantine
commands). Any commands defined in this repo should be part of this.Cobra has this in the documentation on the
Use
field:One addition I'd make is to use
<>
around field names to differentiate a field name and a possible string.For example:
Currently, we have this:
Of those, only the flags are optional, so it'd be better shown as this:
Similarly, this:
Would be better as this:
The
{add|remove}
part is correct since{}
delimits a set of mutually exclusive arguments where one is required, and since it must be either exactly "add" or "remove", there shouldn't be any other delimiters in the message.This monster:
Would be better as this:
Note that the optional
[]
parts are all nested since, e.g. in order to provide the<description>
a<description name>
is needed before it.This one:
Would be better as:
In that, I added some extra spacing around the options in the outside
{}
because I felt it made it easier to read and understand. Specifically, that 2nd option is easy to get lost in and not quite realize what's part of that 2nd option, and where the 3rd option begins. This is pretty subjective though. But the goal is to make the commands easier to learn about and understand.I personally don't think the
[flags]
part should be included as I feel that the ability to provide flags is a given (i.e. It's understood that flags can be provided to CLI commands), and the flags don't have to go in a specific place in the command. I know the SDK includes it for most commands, but I feel we can leave it out.Any flags that are booleans should have a default of
false
. In some cases, this might require renaming the flag and/or adjusting logic. If a boolean flag has a default oftrue
then not providing the flag with a command is the same as providing it. The only way behavior changes is if you include=false
on it, which isn't really documented anywhere, and isn't necessarily a standard CLI thing. An example of a flag that should be renamed or require logic changes would be an--optional
flag with a default oftrue
. In that case, the flag should be renamed to--required
so it can have a default of false.For Admin Use
The text was updated successfully, but these errors were encountered: