Zebra: fix `unrecognized option help` on subcommands

Created on 28 May 2020  路  5Comments  路  Source: ZcashFoundation/zebra

Currently if you try to print the help text for a zebrad subcommand it returns an error indicating the argument is unrecognized and helpfully prints the expected arguments, which includes --help...

Screenshot from 2020-05-27 15-51-15

This is a bug.

C-bug E-help-wanted

Most helpful comment

Hi @oxarbitrage, thanks for helping out by looking into this. I think you're right that we should probably not spend any effort on fixing this short term. I'm personally hoping that abscissa switches to clap but either way it seems like the solution is to wait for them to make a decision and make the change.

pd: i think the config subcomamnd to generate a configuration file can be renamed to generate

This sounds fine to me, cc @ZcashFoundation/zebra-team ?

All 5 comments

this seems to be a worse issue than I thought...

Screenshot from 2020-05-27 15-55-17

more fun bugs, notice the log lines at the top of the file...

Screenshot from 2020-05-27 15-56-57

I was doing some research for this issue and i have to admit is pretty painful.

In your first image, the way to get help for a sub command should be:

./zebrad help config

In there you will see -o is the flag to specify an output file.

So, to execute this command with a specific output file the user will do:

./zebrad config -o myfile

For the config subcommand anything that is not -o string is an error as shown in your first 2 images. The flags displayed there(this is confusing and wrong) are not options inside the config subcommand but global flags that can be executed with any subcommand. For example, to be verbose in the config generation user will need to do:

./zebrad -v config -o myfile

Now, our command parsing tool is included in abscissa , specifically the tool is gumdrop v0.7. It is reported to have problems with subcommands and help: https://github.com/murarth/gumdrop/issues/29

This seems to be fixed in version 0.8 by commit: https://github.com/murarth/gumdrop/commit/f3144316af55c939d8cecaceaf67f13e5e958217

However we cant just upgrade, abscissa needs to update and they are considering to do so at: https://github.com/iqlusioninc/abscissa/issues/298 but as you can see they are also considering moving to clap.

Even with all this i think we still have some issue in our side as well that we could improve. I was comparing to https://github.com/iqlusioninc/tmkms that run the same abscissa and gumdrop versions and it is a bit better than us in some cases.

For example, in zebra:

$ ./zebrad version whatever
error: unexpected free argument `whatever`

zebrad 0.1.0
Zcash Foundation <[email protected]>

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

$

While in tmkms the error is a bit better as the version description is included:

$ ./target/release/tmkms version whatever
error: unexpected free argument `whatever`

tmkms 0.8.0-alpha1
Tony Arcieri <[email protected]>, Ismail Khoffi <[email protected]>
Tendermint Key Management System

DESCRIPTION:
    display version information

$

As mentioned we could improve some of the problems however i am not sure if it worth the effort, specially if abscissa changes to clap soon.

Hope it helps.

pd: i think the config subcomamnd to generate a configuration file can be renamed to generate.

Hi @oxarbitrage, thanks for helping out by looking into this. I think you're right that we should probably not spend any effort on fixing this short term. I'm personally hoping that abscissa switches to clap but either way it seems like the solution is to wait for them to make a decision and make the change.

pd: i think the config subcomamnd to generate a configuration file can be renamed to generate

This sounds fine to me, cc @ZcashFoundation/zebra-team ?

Merged #505, thanks for the PR!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

hdevalence picture hdevalence  路  4Comments

teor2345 picture teor2345  路  3Comments

dconnolly picture dconnolly  路  4Comments

yaahc picture yaahc  路  6Comments

teor2345 picture teor2345  路  5Comments