Skip to content
This repository was archived by the owner on Mar 10, 2020. It is now read-only.

no error when using a unknow option #563

Closed
MichaelMure opened this issue Jun 8, 2017 · 3 comments
Closed

no error when using a unknow option #563

MichaelMure opened this issue Jun 8, 2017 · 3 comments

Comments

@MichaelMure
Copy link
Contributor

When using the go-ipfs command line, an unknow option will return an error:

$ ipfs name publish --blaah
Error: Unrecognized option 'blaah'

It would help the debugging if js-ipfs-api would do the same.

@vasco-santos
Copy link
Contributor

I think that we should try to standardize the CLI responses among js-ipfs and go-ipfs.

GO

1. Empty

$ ipfs
USAGE
 ...
SUBCOMMANDS
 ...

2. Unkown Command

$ ipfs randomcommand
Error: Unknown Command "randomcommand"

3. Unkown Option

$ ipfs id --random
Error: Unrecognized option 'random'

JS

1. Empty

$ jsipfs
jsipfs <command>

Commands:
...

2. Unkown Command

$ jsipfs randomcommand

Nothing happens here.

3. Unkown Option

$ ipfs id --random
{
  "id": ...,
  "publicKey": ...,
  "addresses": [...],
  "agentVersion": ...,
  "protocolVersion": ...
}

Approach

Since we are using yargs, I understand that we do not have the same flexibility as the provided by GO. However, in the js side the unknown command, as well as the unknown option should be handled. Unfortunately, yargs does not allow customizing certain aspects, such as the error message, and we cannot put the same as in GO.

Suggestion:

1. Empty

$ jsipfs
jsipfs <command>
USAGE:
...
Commands:
...

Usage should be added in this case.

2. Unkown Command

$ jsipfs randomcommand
Unknown argument: randomcommand

Error should be displayed.

3. Unkown Option

$ ipfs id --random
Unknown argument: random
...

What do you think @diasdavid @MichaelMure ?

@daviddias
Copy link
Contributor

@vasco-santos Thanks for the complete proposal. It is a good goal to have but very low priority compared to others and one that will require a good amount of time to fix given that as you analyzed, it would require changing the CLI library.

@vasco-santos
Copy link
Contributor

All right! Then, I will create a PR for this issue, only displaying the default yargs error when the option is not valid, which is, in fact, this issue. Afterward, I create a new issue with low priority for uniforming the CLI with GO, using another CLI library.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants