CLI: Use flags everywhere instead of positional arguments

[romac]

Summary

All CLI commands should use flags (--flag) options instead of positional arguments.

Problem Definition

At the moment, a lot of the CLI commands use a mix of positional arguments and flags, for example
the create channel CLI:

hermes create client ibc-1 ibc-0 --trusting-period 14days
                     ^^^^^ ^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^
                      positional          flag

When writing the commands it is now always clear what the positional arguments denote (eg. which is the source chain and destination chain), and one often has to resort to the command documentation (--help) to figure it out.

Even worse, the number of positional arguments and their denotation for the create channel command change depending on which flags are passed, eg.

hermes create channel ibc-0         -c ibc-1 --port-a transfer --port-b transfer --new-client-connection
                      ^^^^^         ^^^^^^^^ ^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^
                      positional    flag     flag              flag              flag

vs

hermes create channel  ibc-0       connection-0 --port-a transfer --port-b transfer
                       ^^^^^       ^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^
                    positional      positional        flag               flag

This is quite confusing, as expressed already by some operators: #1935

Proposal

I suggest using flags everywhere instead of positional arguments.

Positional arguments should be kept for the case where we want to pass a list of items to a command, which we don't have the need for currently.

Acceptance Criteria

All CLI commands accept only flags and no positional arguments.

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate milestone (priority) applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[ljoss17]

Hey @adizere and @romac .

I would like to use this issue in order to apply the following guidelines for the CLI command codes:

1. Explicitly give long and/or short flags. E.g:

    #[clap(short = 'a', long = "amount", required = true, help = "amount of stake")]
    amount: u64,

2. All flags which start with the character c would only have a long flag, to avoid conflicting with the global flag -c/—config
3. All flags referencing the height would have the short flag -H and all other flags starting with the character h would only have a long flag to avoid conflicting with the global flag -h/—help

Would you consider this a good idea ? Or am I maybe missing some additional guidelines ?

[romac]

I think we should get rid of short flags altogether so that we don't have to deal with conflicts and make everything consistent. What do you think?

[ljoss17]

Seeing the amount of fields with only long flags, for example all the chain_id, conn_id and client_id, I also think that removing all the short flags seems like a good idea.

[romac]

I would also suggest we drop the -id suffix in all flags to make the flags shorter and easier to type.

[plafer]

Or maybe global flags should be long form only, as there are less of them and are optional, and have a short form for the other flags? I find short flags to be quite nice when you are running a lot of commands; explicitly typing out the long form args over and over can get annoying and distracting. And hermes commands typically have many of them.

WDYT?

[ljoss17]

Generally I prefer having short flags, but after passing through the commands of this CLI I saw that there aren't only potential conflicts with global flags. There are many commands which would require finding a meaningful short flag which isn't the first character. The following are some examples of commands which would require special short flags:

• chain_a_id, chain_b_id, client_a_id, client_a_id for the command: create connection
• dst_chain_id, src_chain_id, dst_conn_id, dst_port_id, src_port_id for the command: tx raw chan-open-init

So I am a bit more in favor of having the default being all long flags, but reducing the length as @romac suggested so chain_id would be chain etc...
But if we are able to find a meaningful solution for all the conflicting cases, the short flags could be a nice addition.

[adizere]

One source of insights would be to check the gaiad CLI as well as the rly CLI, and see how they deal with some of the corner-cases we're identifying here. For example: Do they use long-only options? What do they do with short options in case both side "a" and side "b" are necessary?

[ljoss17]

From what I managed to observe the two CLIs seem to have the mix of position arguments and flags which the current Hermes CLI has. Most of the flags are long only but some of them have a short option. Some examples are the following:

For gaiad

Usage:
  gaiad tx ibc channel open-init [port-id] [counterparty-port-id] [connection-hops] [flags]

Flags:
  -a, --account-number uint      The account number of the signing account (offline mode only)
  -b, --broadcast-mode string    Transaction broadcasting mode (sync|async|block) (default "sync")
      --dry-run                  ignore the --gas flag and perform a simulation of a transaction, but don't broadcast it
      --fees string              Fees to pay along with transaction; eg: 10uatom
      --from string              Name or address of private key with which to sign
      --gas string               gas limit to set per-transaction; set to "auto" to calculate sufficient gas automatically (default 200000)
      --gas-adjustment float     adjustment factor to be multiplied against the estimate returned by the tx simulation; if the gas limit is set manually this flag is ignored  (default 1)
      --gas-prices string        Gas prices in decimal format to determine the transaction fee (e.g. 0.1uatom)
      --generate-only            Build an unsigned transaction and write it to STDOUT (when enabled, the local Keybase is not accessible)
  -h, --help                     help for open-init
      --ibc-version string       IBC application version (default "ics20-1")
      --keyring-backend string   Select keyring's backend (os|file|kwallet|pass|test|memory) (default "os")
      --keyring-dir string       The client Keyring directory; if omitted, the default 'home' directory will be used
      --ledger                   Use a connected Ledger device
      --memo string              Memo to send along with transaction
      --node string              <host>:<port> to tendermint rpc interface for this chain (default "tcp://localhost:26657")
      --offline                  Offline mode (does not allow any online functionality
      --ordered                  Pass flag for opening ordered channels (default true)
  -s, --sequence uint            The sequence number of the signing account (offline mode only)
      --sign-mode string         Choose sign mode (direct|amino-json), this is an advanced feature
      --timeout-height uint      Set a block timeout height to prevent the tx from being committed past a certain height
  -y, --yes                      Skip tx broadcasting prompt confirmation

Global Flags:
      --chain-id string     The network chain ID
      --home string         directory for config and data (default "/Users/ljoss/.gaia")
      --log_format string   The logging format (json|plain) (default "plain")
      --log_level string    The logging level (trace|debug|info|warn|error|fatal|panic) (default "info")
      --trace               print out full stack trace on errors

Usage:
  gaiad keys add <name> [flags]

Flags:
      --account uint32           Account number for HD derivation
      --algo string              Key signing algorithm to generate keys for (default "secp256k1")
      --coin-type uint32         coin type number for HD derivation (default 118)
      --dry-run                  Perform action, but don't add key to local keystore
      --hd-path string           Manual HD Path derivation (overrides BIP44 config)
  -h, --help                     help for add
      --index uint32             Address index number for HD derivation
  -i, --interactive              Interactively prompt user for BIP39 passphrase and mnemonic
      --ledger                   Store a local reference to a private key on a Ledger device
      --multisig strings         Construct and store a multisig public key (implies --pubkey)
      --multisig-threshold int   K out of N required signatures. For use in conjunction with --multisig (default 1)
      --no-backup                Don't print out seed phrase (if others are watching the terminal)
      --nosort                   Keys passed to --multisig are taken in the order they're supplied
      --pubkey string            Parse a public key in bech32 format and save it to disk
      --recover                  Provide seed phrase to recover existing key instead of creating

Global Flags:
      --home string              The application home directory (default "/Users/ljoss/.gaia")
      --keyring-backend string   Select keyring's backend (os|file|test) (default "os")
      --keyring-dir string       The client Keyring directory; if omitted, the default 'home' directory will be used
      --log_format string        The logging format (json|plain) (default "plain")
      --log_level string         The logging level (trace|debug|info|warn|error|fatal|panic) (default "info")
      --output string            Output format (text|json) (default "text")
      --trace                    print out full stack trace on errors

For rly

Usage:
  rly query channel [chain-id] [channel-id] [port-id] [flags]

Examples:
$ rly query channel ibc-0 ibczerochannel transfer
$ rly query channel ibc-2 ibctwochannel transfer --height 1205

Flags:
      --height int   Height of headers to fetch
  -h, --help         help for channel

Global Flags:
  -d, --debug               debug output
      --home string         set home directory (default "/Users/ljoss/.relayer")
      --log-format string   log output format (auto, logfmt, json, or console) (default "auto")

Usage:
  rly transact channel [path-name] [flags]

Aliases:
  channel, chan

Examples:
$ rly transact channel demo-path --src-port transfer --dst-port transfer --order unordered --version ics20-1
$ rly tx chan demo-path --timeout 5s --max-retries 10

Flags:
      --dst-port string    port on dst chain to use when generating path (default "transfer")
  -h, --help               help for channel
  -r, --max-retries uint   maximum retries after failed message send (default 3)
  -o, --order string       order of channel to create (ordered or unordered) (default "unordered")
      --override           option to not reuse existing client or channel
      --src-port string    port on src chain to use when generating path (default "transfer")
  -t, --timeout string     timeout between relayer runs (default "10s")
  -v, --version string     version of channel to create (default "ics20-1")

Global Flags:
  -d, --debug               debug output
      --home string         set home directory (default "/Users/ljoss/.relayer")
      --log-format string   log output format (auto, logfmt, json, or console) (default "auto")

To me it seems that both gaiad and rly both rely more on long flags, even if a short flag would be easy to set up. For example for the command gaiad tx ibc channel open-init, the flags --ledger, --node and --mnemonic don't have the short option -l, -m and -n even though they are the only flags starting with l, m and n.
For the rely the command rly transact channel uses long flags only for --dst-port and --src-port.

So in my opinion if we want to set flags everywhere for Hermes, I would be in favor of long flags only as a first step. And see afterwards if some strategic short flags could improve it.

What do you think ?

[adizere]

Great summary Luca!

So in my opinion if we want to set flags everywhere for Hermes, I would be in favor of long flags only as a first step. And see afterwards if some strategic short flags could improve it.

+1

[plafer]

Awesome, sounds good to me!