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

fix unrecognized option help on subcommands #400

Closed
yaahc opened this issue May 27, 2020 · 5 comments
Closed

fix unrecognized option help on subcommands #400

yaahc opened this issue May 27, 2020 · 5 comments
Labels
C-bug Category: This is a bug E-help-wanted Call for participation: Help is requested to fix this issue.

Comments

@yaahc
Copy link
Contributor

yaahc commented May 27, 2020

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.

@yaahc
Copy link
Contributor Author

yaahc commented May 27, 2020

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

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

@yaahc
Copy link
Contributor Author

yaahc commented May 27, 2020

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

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

@dconnolly dconnolly added C-bug Category: This is a bug E-med good first issue E-help-wanted Call for participation: Help is requested to fix this issue. and removed good first issue labels May 27, 2020
@oxarbitrage
Copy link
Contributor

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: murarth/gumdrop#29

This seems to be fixed in version 0.8 by commit: murarth/gumdrop@f314431

However we cant just upgrade, abscissa needs to update and they are considering to do so at: iqlusioninc/abscissa#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 <zebra@zfnd.org>

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 <tony@iqlusion.io>, Ismail Khoffi <Ismail.Khoffi@gmail.com>
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.

@yaahc
Copy link
Contributor Author

yaahc commented Jun 18, 2020

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 ?

@hdevalence
Copy link
Contributor

Merged #505, thanks for the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug E-help-wanted Call for participation: Help is requested to fix this issue.
Projects
None yet
Development

No branches or pull requests

4 participants