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

Relayer commands don't support help menu for subcommands #134

Closed
4 tasks
ebuchman opened this issue Jul 8, 2020 · 15 comments · Fixed by #1576
Closed
4 tasks

Relayer commands don't support help menu for subcommands #134

ebuchman opened this issue Jul 8, 2020 · 15 comments · Fixed by #1576
Assignees
Labels
I: CLI Internal: related to the relayer's CLI O: usability Objective: cause to improve the user experience (UX) and ease using the product
Milestone

Comments

@ebuchman
Copy link
Member

ebuchman commented Jul 8, 2020

Summary of Bug

When running relayer query --help I expect to see the list of subcommands but I just get:

$ relayer query --help
error: unrecognized command `--help`

relayer-cli 0.0.1
Anca Zamfir <anca@interchain.io>, Romain Ruetschi <romain@informal.systems>

FLAGS:
    -c, --config CONFIG       path to configuration file
    -h, --help                print help message
    -v, --verbose             be verbose

Version

v0.0.1

Steps to Reproduce


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@ebuchman ebuchman added this to the v0.0.2 milestone Jul 8, 2020
@andynog
Copy link
Contributor

andynog commented Jul 8, 2020

I've noticed this today when testing the query and mentioned on the group call. I'll take a look at that and ensure this works.

@andynog
Copy link
Contributor

andynog commented Jul 14, 2020

@ebuchman, I've done some investigation on this one. Haven't been able to solve it yet because was just trying to understand how abscissa works, and how it uses gumdrop for command line. I believe the way our code is currently implemented it should work (show usage for subcommands). Unfortunately the docs for abscissa and gumdrop are minimal on this topic. I'm more familiar with clap and structopt.

But found some issues that might point that help usage for subcommands might be a limitation

There are also a couple of issues open related to this

@ebuchman
Copy link
Member Author

ebuchman commented Jul 14, 2020

Ah, looks like abscissa is going in the direction of clap. Maybe we should wait for / help with that? I guess it would be good to know what this looks like with gumdrop (if I'm not mistaken just a bit more boilerplate).

@adizere
Copy link
Member

adizere commented Jul 15, 2020

Running this today (from targer/debug) works for me. The error message is gone.

adi@styx:ibc-rs $ target/debug/relayer --help
relayer-cli 0.0.1
Anca Zamfir <anca@interchain.io>, Romain Ruetschi <romain@informal.systems>

USAGE:
    relayer-cli <SUBCOMMAND>

SUBCOMMANDS:
    help       get usage information
    start      start the relayer
    listen     listen to IBC events
    config     manipulate the relayer configuration
    version    display version information
    query      query state from chain
    light      basic functionality for managing the lite clients

@andynog
Copy link
Contributor

andynog commented Jul 15, 2020

This works but I was talking about subcommands like relayer query should provide help context for the query subcommand. That doesn't work, currently it shows an error, it should show command usage

error: missing command name

relayer-cli 0.0.1
Anca Zamfir <anca@interchain.io>, Romain Ruetschi <romain@informal.systems>

FLAGS:
    -c, --config CONFIG       path to configuration file
    -h, --help                print help message
    -v, --verbose             be verbose

@andynog
Copy link
Contributor

andynog commented Jul 17, 2020

  • Added logic to display some help messages. So if the user type a subcommand followed by the help keyword, usage will show, e.g.
    • relayer query help
    • relayer query connection help

But I haven't figure out a way to show a more complete usage information with parameters for example. I was exploring tmkms tool and it seems the help usage information is the same as what I've implemented. I think that gumdrop builtin options to show help messages are not as robust as clap. I hope that abscissa implements it.

@adizere adizere added the I: CLI Internal: related to the relayer's CLI label Jul 21, 2020
@andynog
Copy link
Contributor

andynog commented Jul 23, 2020

got confirmation that indeed there's a bug in gumdrop v0.7 that doesn't show the parameters. I think this is the only solution for now until abscissa upgrades to gumdrop v0.8 or clap v3.

@ebuchman
Copy link
Member Author

Ach ok let's drop it from this milestone for now then and wait for abscissa to upgrade (maybe we can even help with that)

@ebuchman ebuchman removed this from the v0.0.2 milestone Jul 24, 2020
@andynog
Copy link
Contributor

andynog commented Jul 26, 2020

Sure, I think if we are using it as a core framework for our cli apps we should find a way to help.

@romac romac changed the title relayer commands don't help menu for subcommands relayer commands don't support help menu for subcommands Jul 27, 2020
@andynog
Copy link
Contributor

andynog commented Jul 31, 2020

Will try to experiment replacing gumdrop with clap v3 in an abscissa fork and see if that works.

@ancazamfir ancazamfir added this to the v0.0.4 milestone Aug 14, 2020
@ebuchman ebuchman modified the milestones: v0.0.4, v0.0.5 Aug 28, 2020
@ancazamfir
Copy link
Collaborator

Looks like using help before the command works:

$ rrly help query client consensus
relayer-cli 0.0.3
Informal Systems <hello@informal.systems>

USAGE:
    relayer-cli query client consensus <OPTIONS>

DESCRIPTION:
    query client consensus

POSITIONAL ARGUMENTS:
    chain_id                  identifier of the chain to query
    client_id                 identifier of the client to query
    consensus_epoch           epoch of the client's consensus state to query
    consensus_height          height of the client's consensus state to query

FLAGS:
    -h, --height HEIGHT       the chain height which this query should reflect
    -p, --proof PROOF         whether proof is required
`

@ancazamfir
Copy link
Collaborator

Will move this to a later milestone, doesn't look like we get a fix from abscissa by end of month. Will keep an eye on it.

@ancazamfir ancazamfir modified the milestones: v0.0.5, v0.0.7 Oct 16, 2020
@romac romac modified the milestones: v0.1.0, v0.1.2 Jan 6, 2021
@andynog
Copy link
Contributor

andynog commented Mar 4, 2021

I don't think this is an issue anymore. I think it works using the help keyword before the command. It's just usually not the default to ask for help (most CLIs use -h or --help parameter) but there's a way to do it. And now that we have a Hermes guide that makes this clear I think we can close this issue if you agree @ancazamfir

@ancazamfir
Copy link
Collaborator

ancazamfir commented Mar 4, 2021

I guess it's ok to close this issue. But the pain point is not gone. It's very annoying to do hermes help..., figure out the command and then skip back to remove help. Also, is this something we make clear in the guide? I haven't searched it all, but I see in some cases we just show the usage, without including the hermes help... command that gave that output.

@romac romac changed the title relayer commands don't support help menu for subcommands Relayer commands don't support help menu for subcommands Mar 24, 2021
@colin-axner
Copy link
Contributor

I support leaving this issue open. If I run hermes keys and see unrecognized command it can be quite confusing as a new user.

We had a similar issue to this one a few years ago with gaia where cli interaction could prove troublesome due to unsupported features of the library we were using. As an intern I picked up the issue and made a quick hack to fix the UX issue. The feature is still unimplemented upstream after 2 years of requesting a fix.

Leaving the issue open is useful as the painpoint is still there and perhaps in the future a new team member can come up with a reasonable hack in the meantime

@romac romac added the O: usability Objective: cause to improve the user experience (UX) and ease using the product label Nov 17, 2021
@adizere adizere modified the milestones: Backlog, v0.9.1 Dec 1, 2021
@adizere adizere assigned mzabaluev and unassigned andynog Dec 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: CLI Internal: related to the relayer's CLI O: usability Objective: cause to improve the user experience (UX) and ease using the product
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants