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

should help support --help? / help can't help help #5557

Closed
djdv opened this issue Oct 3, 2018 · 9 comments
Closed

should help support --help? / help can't help help #5557

djdv opened this issue Oct 3, 2018 · 9 comments
Labels
kind/bug A bug in existing code (including security flaws)

Comments

@djdv
Copy link
Contributor

djdv commented Oct 3, 2018

This is a strange one, but I'm reporting it anyway.
ipfs help --help

Error: Unknown Command "help"

This seems to be the only command that doesn't support the --help flag.
While it probably wouldn't do anything more than the bare help subcommand itself, I found the error odd.

@Stebalien
Copy link
Member

Ah... Looks like help was a leftover. It's not actually a command.

@Stebalien Stebalien added the kind/bug A bug in existing code (including security flaws) label Oct 5, 2018
@kjzz
Copy link
Contributor

kjzz commented Oct 5, 2018

So you mean that we should delete ipfs help?

@Stebalien
Copy link
Member

Unfortunately, we actually use it. I said "leftover" because it looks like it may be a leftover from before we had a commands library (not sure, it may not have been).

However, we should probably make it behave better. The solution here is probably to just remove the length check. That is, convert "help" to "--help", even if there are other flags. Thoughts?

@djdv
Copy link
Contributor Author

djdv commented Oct 5, 2018

Would it be better to formalize it as a command? With this kind of alias mapping:
ipfs help add -> ipfs add --help?

Edit: Stebalien was quicker 👏

@Stebalien
Copy link
Member

Maybe? I'm not sure it's worth the effort (that would probably require deep changes to the commands library).

@kjzz
Copy link
Contributor

kjzz commented Oct 5, 2018

Unfortunately, we actually use it. I said "leftover" because it looks like it may be a leftover from before we had a commands library (not sure, it may not have been).

Sorry @Stebalien , i have misunderstand your reply before.

Would it be better to formalize it as a command? With this kind of alias mapping:
ipfs help add -> ipfs add --help?

Agree,I think thatipfs <sub-command> --help maybe better,of course it will casue breanking?

If you decide to do it,I am willing to do this work.Thx

@djdv
Copy link
Contributor Author

djdv commented Oct 5, 2018

For now, we could probably use the same hack, but append to args.
i.e. something similar to

if helpcmd {
    args = append(args[:2], args[3:]...) //remove "help"
    args = append(args, "--help")

in phase: ipfs help add -> ipfs add -> ipfs add --help

Right now, we alias ipfs help to ipfs -h but ipfs --help works the same, so it shouldn't break that case either.
It seems to work with other flags too.
ipfs add -w --help is the same as ipfs add --help alone.

And keep it in mind as the cmdslib evolves. See if it's necessary to formalize it as a cmd command.

Not sure if stacking hacks on hacks is a good idea, but for this specific command it seems harmless and unlikely to ever require change.
I guess the only thing we'd need to do is create some kind of stub cmd that is there only to hold the documentation for help and propagate it in ipfs commands.

Thoughts?

@kjzz
Copy link
Contributor

kjzz commented Oct 6, 2018

Agree!@djdv , I am working for it right now.Thx a lot for very nice advices!!!

Stebalien added a commit that referenced this issue Nov 6, 2018
@Stebalien
Copy link
Member

Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A bug in existing code (including security flaws)
Projects
None yet
Development

No branches or pull requests

3 participants