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

Parser configuration for convenience methods #561

Closed
remkop opened this issue Dec 1, 2018 · 7 comments
Closed

Parser configuration for convenience methods #561

remkop opened this issue Dec 1, 2018 · 7 comments

Comments

@remkop
Copy link
Owner

remkop commented Dec 1, 2018

The convenience methods CommandLine::run and CommandLine::call don’t allow for the parser to be configured.

Applications that need to parse with non-default parser settings can only use CommandLine::parseWithHandlers, which is more verbose and less readable.

See #556 for an example.

One idea is to introduce overloaded CommandLine::run and CommandLine::call methods that take a ParserSpec parameter used to config the parser.

@remkop
Copy link
Owner Author

remkop commented Dec 1, 2018

Perhaps it makes sense to do this work together with #516, which also modifies the API of the convenience methods.

@helpermethod
Copy link

This is a great idea!

@remkop
Copy link
Owner Author

remkop commented Dec 13, 2018

Thinking further, to future-proof this a little bit, it may be a good idea to use the builder pattern to let users create an execution configuration. An execution configuration is a set of attributes to customize the behaviour of the convenience methods. We know this configuration should include at least ParserSpec (this ticket), ColorScheme (#516), exit codes (#424), exception handling (#541), but we may get more requirements in the future. The builder pattern allows us to add features later without breaking backwards compatibility or adding many overloaded methods to the API surface of the CommandLine class.

User code could look something like this:

public static void main(String... args) {
    CommandLine.run(new MyRunnable(),
                    ExecutionSpec.builder()
                        .colorScheme(new ColorScheme().commands(bold, underline, blink))
                        .parserSpec(new ParserSpec().caseInsensitiveEnumValuesAllowed(true))
                        .exitCodeOnUsageHelp(0)
                        .exitCodeOnVersionHelp(0)
                        .exitCodeOnInvalidInput(1)
                        .exitCodeOnException(IOException.class, 2)
                        .exitCodeOnException(FileNotFoundException.class, 3)
                        .exitCodeOnOtherException(4)
                        .build(),
                    args);
}

New methods to add to CommandLine would be limited to the below. Future expansion can be taken care of by adding methods to the ExecutionSpecBuilder class:

public static <C extends Callable<T>, T> T call(C callable, ExecutionSpec spec, String... args);
public static <C extends Callable<T>, T> T call(Class<C> callableClass, IFactory factory, ExecutionSpec spec, String... args);
public static <R extends Runnable> void run(R runnable, ExecutionSpec spec, String... args);
public static <R extends Runnable> void run(Class<R> runnableClass, IFactory factory, ExecutionSpec spec, String... args);
public static Object invoke(String methodName, Class<?> cls, ExecutionSpec spec, String... args)

@remkop
Copy link
Owner Author

remkop commented Jan 8, 2019

An alternative idea: introduce a new Executable class.
Executable would have all the getters/setters of CommandLine, but should not have the parse, parseArgs, parseWithHandlers methods or the static CommandLine methods.
(Impl. note: both CommandLine and Executable can extend a package-protected superclass.)

CommandLine is for using picocli as a command line parser library, where Executable is for using picocli as a framework.
We can deprecate the current static convenience methods on CommandLine, except populateCommand and populateSpec, which are essentially parse methods.

Users can do all configuration on the CommandLine or Executable instance.
Instead of calling System.exit internally, return an exit code from a new execute method.

Executable can only be created with Runnable, Callable or Method (TBD).
If the user object implements IExitCodeGenerator the result of invoking its exitCode() method is returned from execute.
TBD: get exit code from return value of Callable<Integer> or Method returning Integer as convenience?
execute returns null if no exit code was specified.

TBD Alternatively, execute could return a more rich Status object, with a convenient exit method (calls System.exit iff an exit code was specified).

public static void main(String... args) {
	Executable executable = Executable.create(new MyRunnable())
			.setColorScheme(new ColorScheme().commands(bold, underline, blink))
			.setCaseInsensitiveEnumValuesAllowed(true)
			.setExitCodeForSuccess(0)
			.setExitCodeForException(ParameterException.class, 1) // invalid input
			.setExitCodeForException(IOException.class, 2) // custom application errors
			.setExitCodeForException(Exception.class, 3) // catch-all
			// any other configuration
			;
    Integer exitCode = executable.execute(args);
	if (exitCode != null) {
		System.exit(exitCode);
	}
}

Some methods that only make sense on the Executable can be defined on Executable instead of on CommandLine:

  • get/setColorScheme
  • setExitCodeForException
  • setExitCodeForSuccess
  • setExceptionHandler

Without configuration, the shortest invocation is almost as brief as the 2.x-3.x convenience methods:
Previous:

CommandLine.run(new MyRunnable(), args);

From picocli 4.0:

Executable.create(new MyRunnable()).execute(args);

@remkop
Copy link
Owner Author

remkop commented Feb 15, 2019

Jotting down some notes on the subject of exit codes for picocli 4.0:

Spring Boot

Spring Boot has some good ideas for handling exit codes:

  • it won't call System.exit itself, it just provides an API that returns an int. It is up to the application to call System.exit with that value.
  • it provides an interface ExitCodeGenerator. This solves the problem of deeply nested methods needing to return an exit code: there is no need to propagate this value through the call stack any more.
  • it provides a ExitCodeExceptionMapper interface that maps Exceptions to an exit code. Implementors can inspect details of the Exception object before deciding what exit code to return.

Current implementation (picocli 3.x)

  • Picocli currently has IResultHandler::andExit(int) and IExceptionHandler::andExit(int) methods that do call System.exit - deprecate this in picocli 4.0: these methods are hard to find, are not flexible enough to cover many use cases, and in general we should avoid calling System.exit in the library.

Goals

Documentation

When generating man page documentation (#299, #459), it would be nice to be able to include a section listing the EXIT STATUS values of the command (like man's man page). This would require picocli to know all exit codes and their description.

CommandLine cmd = new CommandLine(new App());

// "success" includes usage help request, version info request, and successful command execution
cmd.setExitCodeForSuccess(0, "Successful program execution.");

// static mapping: simple and self-documentating
cmd.setExitCodeForException(ParameterException.class, 1, "Invalid user input");
cmd.setExitCodeForException(MissingParameterException.class, 2, "Missing required user input parameter");
cmd.setExitCodeForException(IOException.class, 3, "Incorrect file name or disk full");

// Dynamic mapping: more powerful but not self-documenting.
// Only useful when used with `execute`, not with `parse`, `parseArgs` or `populateCommand`.
cmd.setExitCodeExceptionMapper(ex -> { 
        if ("Bad data".equals(ex.getMessage())) { return 4; }
        return 5;
    }
);

// ... so document dynamic exit codes with API like this?
cmd.setExitCode(4, "Application encountered bad data");
cmd.setExitCode(5, "Unknown exception");

Dynamic Exit Status section

If any exit code is defined, picocli could automatically insert help section keys SECTION_KEY_EXIT_STATUS_HEADING, SECTION_KEY_EXIT_STATUS before SECTION_KEY_FOOTER_HEADING, and put a IHelpSectionRenderer in the help section map that prints a list of the registered exit codes and their description.

Execution

Add a new execute method to CommandLine (or a subclass Executable?) that invokes the Runnable, Callable or Method and returns the exit code.
This makes the application responsible for calling System.exit:

public static void main(String[] args) {
    CommandLine cmd = new CommandLine(new App());
    //... configure

    int exitCode = cmd.execute(args);
	System.exit(exitCode);
}

This execute method should delegate to the IParseResultHandler2 and IExceptionHandler2 (but would ignore their return value).
Applications may customize runtime behaviour by supplying their own handlers, BUT custom handlers should really extend AbstractHandler so picocli can pass them the configured color scheme and OUT and ERR streams.

CommandLine cmd = new CommandLine(new App())
    .setColorScheme(myColorScheme);
    .setParseResultHandler(new RunLast());
    .setExceptionHandler(new DefaultExceptionHandler<List<Object>>()); // TODO too verbose
int exitCode = cmd.execute(args);

Exception handling

The execute method would delegate error handling to the IExceptionHandler2, but an exception handler implementation may rethrow an exception, or extract the cause exception and rethrow that. If any exit codes are configured, the execute method should catch all exceptions, print their stack trace, and then return the appropriate error code.

Implementation

public int execute() {
    int exitCode = 0;
    IExitCodeGenerator exitCodeGenerator = new IExitCodeGenerator() {
        public int getExitCode() { return getExitCodeForSuccess(); }
    };
	if (getCommandSpec().userObject() instanceof IExitCodeGenerator) {
		exitCodeGenerator = (IExitCodeGenerator) getCommandSpec().userObject();
	}
    IExitCodeExceptionMapper exceptionMapper = new IExitCodeExceptionMapper() {
        public int getExitCode(Throwable exception) {
            Integer registered = exceptionMap.get(exception.getClass());
            if (registered != null) { return registered; }
            return getExitCodeExceptionMapper().getExitCode(exception); // never null
        }
    };
	IExceptionHandler2<?> exceptionHandler = getExceptionHandler();
	if (exceptionHandler instanceof AbstractHandler) { ((AbstractHandler) exceptionHandler).useColorScheme(getColorScheme()); }

    ParseResult parseResult = null;
    try {
        parseResult = parseArgs(args);
        IParseResultHandler2<?> handler = getParseResultHandler();
        if (handler instanceof AbstractHandler) { ((AbstractHandler) handler).useColorScheme(getColorScheme()); }
        handler.handleParseResult(parseResult);
        exitCode = exitCodeGenerator.getExitCode();
    } catch (ParameterException ex) {
        try {
            exitCode = exceptionMapper.getExitCode(ex);
            exceptionHandler.handleParseException(ex, args);
        } catch (Exception ex) {
            ex.printStackTrace();
            exitCode = exitCode == 0 ? 1 : exitCode;
        }
    } catch (ExecutionException ex) {
        try {
            exitCode = exceptionMapper.getExitCode(ex.getCause());
            exceptionHandler.handleExecutionException(ex, parseResult);
        } catch (Exception ex) {
            ex.printStackTrace();
            exitCode = exitCode == 0 ? 1 : exitCode;
        }
    }
    return exitCode;
}

Miscellaneous

AbstractParseResultHandler needs an alternative for handleParseResult that takes a ColorScheme.

@remkop remkop modified the milestones: 4.0, 4.0-alpha-3 Apr 24, 2019
remkop added a commit that referenced this issue Apr 24, 2019
…te` and `tryExecute` methods: configurable convenience methods with improved exit code support.

* The new `execute` and `tryExecute` methods are similar to the `run`, `call` and `invoke` methods, but are not static, so they allow parser configuration.
* In addition, these methods, in combination with the new `IExitCodeGenerator` and `IExitCodeExceptionMapper` interfaces, offer clean exit code support.
* Finally, the `tryExecute` method rethrows any exception thrown from the Runnable, Callable or Method, while `execute` is guaranteed to never throw an exception.
* Many variants of the previous `run`, `call` and `invoke` convenience methods are now deprecated in favor of the new `execute` methods.
* Many methods on `AbstractHandler` are now deprecated.

Still TODO: tests and documentation.
@remkop
Copy link
Owner Author

remkop commented Apr 24, 2019

Initial version just landed in master.

@remkop
Copy link
Owner Author

remkop commented Apr 27, 2019

Todo: update javadoc to link these methods to each other:

  • execute
  • tryExecute
  • executeHelpRequest
  • get/setOut
  • get/setErr
  • get/setColorScheme
  • get/setExecutionStrategy
  • get/setParameterExceptionHandler
  • get/setExecutionExceptionHandler
  • get/setExitCodeExceptionMapper

remkop added a commit that referenced this issue Apr 27, 2019
remkop added a commit that referenced this issue Apr 30, 2019
remkop added a commit that referenced this issue May 1, 2019
…invoke() methods and associated classes and interfaces; update Javadoc
@remkop remkop closed this as completed in 1161b05 May 2, 2019
remkop added a commit that referenced this issue May 3, 2019
…er interface to simplify custom implementations; unwrap the `ExecutionException` in the `CommandLine.execute` method
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants