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

Show help when missing a required parameter #22

Open
thombergs opened this issue Oct 8, 2013 · 4 comments
Open

Show help when missing a required parameter #22

thombergs opened this issue Oct 8, 2013 · 4 comments

Comments

@thombergs
Copy link

When I call "command --help" when missing a required parameter specified with required=true, all I see is a stacktrace like this:

Exception in thread "main" io.airlift.command.ParseOptionMissingException:
 Required option '-rev' is missing

When specifying the help parameter the help should be displayed no matter if required parameters are missing or not.

I used the code shown on the readme page:

public static void main(String... args) {
  MyCommand command = SingleCommand.singleCommand(MyCommand.class).parse(args);

  // cancel further processing if help is displayed
  if (command.help.showHelpIfRequested()) {
    return;
  }

  command.run();
}

The exception is thrown in the parse() method naturally. Because the Help can only be triggered AFTER the command object is created, I cannot show the help if required parameters are missing. Is there an existing solution to this?

Regards,
Tom

@thombergs thombergs reopened this Oct 8, 2013
@electrum
Copy link
Member

electrum commented Oct 9, 2013

This is an interesting issue. The library was originally designed to be agnostic about the help system. Before the previous release, it was simply another command. This is an important usability problem, so we'll probably need more special casing for help.

thombergs added a commit to thombergs/airline that referenced this issue Oct 9, 2013
thombergs added a commit to thombergs/airline that referenced this issue Oct 9, 2013
@jontodd
Copy link

jontodd commented Dec 30, 2013

+1 for fixing this issue. If you want to continue to be agnostic to the help system the the Errors object pattern might be a better approach than throwing. Especially when dealing with input from a user which may actually have multiple things wrong with it, this pattern will allow for the reporting of each problem at once rather than failing once at a time.

You could still keep the current semantic you have today with:

MyCommand command = SingleCommand.singleCommand(MyCommand.class).parseFailFast(args);

But then you could also support a model like this:

ParseResult parseResult = new ParseResult();
MyCommand command = SingleCommand.singleCommand(MyCommand.class).parse(args, parseResult);

if (parseResult.hasErrors()) {
   command.helpOption.showHelpWithErrors(parseResult.errors());
}

Thoughts?

jontodd added a commit to jontodd/airline that referenced this issue Dec 30, 2013
This is a proposed fix for: airlift#22

Tests still need to be fixed.
jontodd added a commit to jontodd/airline that referenced this issue Dec 30, 2013
This is a proposed fix for: airlift#22

This approach introduces a ParseResult object which allows for the collection of multiple parse errors from the user input
and then provides this back to the caller to decide what to do. E.G. send the user the to the help dialog.

This keeps the existing API and fits with the desire to keep the parsing logic agnostic to the details of the HelpOption.

The existing pattern still fails fast with an exception:

```java
MyCommand command = SingleCommand.singleCommand(MyCommand.class).parse(args);
```

This fix introduces the following pattern from the caller's perspective:

```java
ParseResult parseResult = new ParseResult();
MyCommand command = SingleCommand.singleCommand(MyCommand.class).parse(args, parseResult);

if (parseResult.hasErrors()) {
     command.helpOption.showHelpWithErrors(parseResult.errors());
}
```
jontodd added a commit to jontodd/airline that referenced this issue Dec 30, 2013
This is a proposed fix for: airlift#22

This approach introduces a ParseResult object which allows for the collection of multiple parse errors from the user input
and then provides this back to the caller to decide what to do. E.G. send the user the to the help dialog.

This keeps the existing API and fits with the desire to keep the parsing logic agnostic to the details of the HelpOption.

The existing pattern still fails fast with an exception:

```java
MyCommand command = SingleCommand.singleCommand(MyCommand.class).parse(args);
```

This fix introduces the following pattern from the caller's perspective:

```java
ParseResult parseResult = new ParseResult();
MyCommand command = SingleCommand.singleCommand(MyCommand.class).parse(args, parseResult);
command.helpOption.runOrShowHelp(parseResult, command);
```

The call to runOrShowHelp will:
* Run the command if there are no parse errors and help has not been requsted
* Will display help if it is requested regardless of whether there are parse errors
* Will display parse errors and help when there are parse errors
jontodd added a commit to jontodd/airline that referenced this issue Jan 26, 2014
This is a proposed fix for: airlift#22

This approach introduces a ParseResult object which allows for the collection of multiple parse errors from the user input
and then provides this back to the caller to decide what to do. E.G. send the user the to the help dialog.

This keeps the existing API and fits with the desire to keep the parsing logic agnostic to the details of the HelpOption.

The existing pattern still fails fast with an exception:

```java
MyCommand command = SingleCommand.singleCommand(MyCommand.class).parse(args);
```

This fix introduces the following pattern from the caller's perspective:

```java
ParseResult parseResult = new ParseResult();
MyCommand command = SingleCommand.singleCommand(MyCommand.class).parse(args, parseResult);
command.helpOption.runOrShowHelp(parseResult, command);
```

The call to runOrShowHelp will:
* Run the command if there are no parse errors and help has not been requsted
* Will display help if it is requested regardless of whether there are parse errors
* Will display parse errors and help when there are parse errors
@taotaotw
Copy link

Seems this fix never goes into the master branch here: https://github.com/airlift/airline Is there any reason?

@hristo-vrigazov
Copy link

Any updates on this one?

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

No branches or pull requests

5 participants