Better message, usage, option help and completion for CLIs

[ancazamfir]

@ljoss17 @mzabaluev

Summary

So not sure this is a feature request or a bug and if there is anything we can do about it.
With the new CLIs in master, the USAGE and help are strange and sometimes not correct.
For example:

$ hermes create channel -h
hermes-create-channel
Create a new channel between two chains using a pre-existing connection

USAGE:
    hermes create channel [OPTIONS] --a-chain <A_CHAIN_ID> --a-port <A_PORT_ID> --b-port <B_PORT_ID>

OPTIONS:
        --a-chain <A_CHAIN_ID>
            Identifier of the side `a` chain for the new channel

        --a-connection <A_CONNECTION_ID>
            Identifier of the connection on chain `a` to use in creating the new channel.

        --a-port <A_PORT_ID>
            Identifier of the side `a` port for the new channel

        --b-chain <B_CHAIN_ID>
            Identifier of the side `b` chain for the new channel

        --b-port <B_PORT_ID>
            Identifier of the side `b` port for the new channel

        --channel-version <VERSION>
            The version for the new channel

    -h, --help
            Print help information

        --new-client-connection
            Indicates that a new client and connection will be created underlying the new channel

        --order <ORDER>
            The channel ordering, valid options 'unordered' (default) and 'ordered' [default:
            ORDER_UNORDERED]

        --yes
            Skip new_client_conn confirmation

There are a few issues for this one:

• Create a new channel between two chains using a pre-existing connection is wrong, that is not the only option. Seems that a blank line (create.rs) in the description drops everything after when displayed with -h
• the USAGE is wrong. not sure how much control we have with abscissa and clap but --a-chain appears both in USAGE and in OPTIONS. The required flags should not be [OPTIONS]
  • Not sure about this in fact as we wouldn't have a way to describe the required flags. Maybe have a [REQUIRED] section
• the list of options in alphabetical order makes reading them hard, e.g. -h, --help appears in the middle of the option list. Is there a way to control the order in which they are listed?
• completion seems to work for the first few options and then it stops working.

Haven't checked all CLIs but I expect most of the above will show there.
Maybe most or all of these issues where there before but we should cleanup as much as possible before v1.0.

Problem Definition

Proposal

Acceptance Criteria

---

For Admin Use

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

[ljoss17]

There are a few issues for this one:

* `Create a new channel between two chains using a pre-existing connection` is wrong, that is not the only option. Seems that a blank line (`create.rs`) in the description drops everything after when displayed with -h

* the USAGE is wrong. not sure how much control we have with abscissa and clap but `--a-chain` appears both in `USAGE` and in `OPTIONS`. The required flags should not be `[OPTIONS]`
  
  * Not sure about this in fact as we wouldn't have a way to describe the required flags. Maybe have a [REQUIRED] section

* the list of options in alphabetical order makes reading them hard, e.g. `-h, --help` appears in the middle of the option list. Is there a way to control the order in which they are listed?

* completion seems to work for the first few options and then it stops working.

Haven't checked all CLIs but I expect most of the above will show there. Maybe most or all of these issues where there before but we should cleanup as much as possible before v1.0.

The description is the short version since the help was called with -h. By using the flag --help the long version will be displayed with more information. But I think the short help could be changed to be more clear.

Regarding the other issues you mentioned, I will look them up while working on the PR for the unit tests of the CLI command parsing.

[ljoss17]

After some additional experimentation, there is a potential solution for the ordering and the help headers, but it requires modifying every commands.

By adding AppSettings::NoAutoHelp and next_line_help = true from clap and then adding the following argument:

/// Query client consensus command
#[derive(Clone, Command, Debug, Parser, PartialEq)]
#[clap(global_setting(AppSettings::NoAutoHelp), next_line_help = true)]
pub struct QueryClientConsensusCmd {
    #[clap(
        short = 'h',
        long = "help",
        action = ArgAction::Help,
        hidden = true,
    )]
    h: bool,

It is possible to remove the -h, --help from the help message.

Once the -h, --help is removed, by adding a help_header = "REQUIRED" and help_header = "OPTIONS" to respectively the required and optional flags, it is possible to have the following help display:

Query the client consensus state

USAGE:
    hermes query client consensus [OPTIONS] --chain <CHAIN_ID> --client <CLIENT_ID>

REQUIRED:
        --chain <CHAIN_ID>
            Identifier of the chain to query

        --client <CLIENT_ID>
            Identifier of the client to query

OPTIONS:
        --consensus-height <CONSENSUS_HEIGHT>
            Height of the client's consensus state to query

        --height <HEIGHT>
            The chain height context to be used, applicable only to a specific height

        --heights-only
            Show only consensus heights

The advantages are that the help display message has the essential information and cleanly organised.
The disadvantage is that this requires modifying every command, so there is a higher risk that a command's help message won't be correctly formatted.

The disadvantage could be compensated with a unit test with the following format:

static HELP: &str = "hermes 
Query the client consensus state

USAGE:
    hermes [OPTIONS] --chain <CHAIN_ID> --client <CLIENT_ID>

REQUIRED:
        --chain <CHAIN_ID>
            Identifier of the chain to query

        --client <CLIENT_ID>
            Identifier of the client to query

OPTIONS:
        --consensus-height <CONSENSUS_HEIGHT>
            Height of the client's consensus state to query

        --height <HEIGHT>
            The chain height context to be used, applicable only to a specific height

        --heights-only
            Show only consensus heights
";

    #[test]
    fn test_help_output() {

        let mut cmd = QueryClientConsensusCmd::command();

        let mut buf = Vec::new();

        cmd.write_help(&mut buf).unwrap();
        assert_eq!(HELP, String::from_utf8(buf).unwrap());
    }

What do you think ?

[romac]

Thanks a lot for looking into this! While this would work, it seems like a lot of effort for fairly minimal gains. I think we'll just stick with -h and --help and their discrepancies.

What we can do to ensure users are more likely to see the full help message is direct them towards using the help subcommand (eg. hermes help create client) in the guide. What do you think?

[ljoss17]

I agree, the tradeoff between effort and gain was one of my concerns. I will stick to only adding the help_header to separate the required and optional flags in the help message.

I think it's a great idea to encourage the usage of the help subcommand.