Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Standardize CLI usage and examples #1529

Open
4 tasks
SpicyLemon opened this issue May 2, 2023 · 0 comments
Open
4 tasks

Standardize CLI usage and examples #1529

SpicyLemon opened this issue May 2, 2023 · 0 comments
Labels
CLI Command line interface feature
Milestone

Comments

@SpicyLemon
Copy link
Contributor

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. 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.

For example:
Currently, we have this:

write-scope [scope-id] [spec-id] [owners] [data-access] [value-owner-address] [flags]

Of those, only the flags are optional, so it'd be better shown as this:

write-scope <scope id> <spec id> <owners> <data access> <value owner address> [flags]

Similarly, this:

scope-data-access {add|remove} [scope id] [data access]

Would be better as this:

scope-data-access {add|remove} <scope id> <data access>

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:

write-scope-specification [specification-id] [owner-addresses] [responsible-parties] [contract-specification-ids] [description-name, optional] [description, optional] [website-url, optional] [icon-url, optional]

Would be better as this:

write-scope-specification <specification id> <owner addresses> <responsible parties> <contract specification ids> [<description name> [<description> [<website url> [<icon url>]]]]

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:

session {session_id|{scope_id|scope_uuid} [session_uuid|record_name]|record_id|"all"}

Would be better as:

session {<session id> | {<scope id>|<scope uuid>} [<session uuid>|<record name>] | <record id> | all}

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
@SpicyLemon SpicyLemon added the CLI Command line interface feature label May 2, 2023
@iramiller iramiller added this to the backlog milestone May 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI Command line interface feature
Projects
Development

No branches or pull requests

2 participants