clikt icon indicating copy to clipboard operation
clikt copied to clipboard

Add an execute function that hides non-exceptional CliktError instances

Open pnq32 opened this issue 2 years ago • 0 comments

For the use case I am currently facing, CliktCommand.main does a little bit too much:

fun main(argv: List<String>) {
    try {
        parse(argv)
    } catch (e: ProgramResult) {
        exitProcessMpp(e.statusCode)
    } catch (e: PrintHelpMessage) {
        echo(e.command.getFormattedHelp())
        exitProcessMpp(if (e.error) 1 else 0)
    } catch (e: PrintCompletionMessage) {
        val s = if (e.forceUnixLineEndings) "\n" else currentContext.console.lineSeparator
        echo(e.message, lineSeparator = s)
        exitProcessMpp(0)
    } catch (e: PrintMessage) {
        echo(e.message)
        exitProcessMpp(if (e.error) 1 else 0)
    } catch (e: UsageError) {
        echo(e.helpMessage(), err = true)
        exitProcessMpp(e.statusCode)
    } catch (e: CliktError) {
        echo(e.message, err = true)
        exitProcessMpp(1)
    } catch (e: Abort) {
        echo(currentContext.localization.aborted(), err = true)
        exitProcessMpp(if (e.error) 1 else 0)
    }
}

Since it catches all relevant Exceptions and exits the process, it does not give clients the opportunity to customize the generated output in a fine-gradined manner or to perform additional logging. In this case, the documentation recommends to call CliktCommand.parse instead and to catch all the Exceptions manually.

In general, this is exactly the functionality I need. However, I think that in contrast to the main function, CliktCommand.parse does not do quite enough. By throwing Exceptions in non-exceptional cases, it forces my client code to replicate a significant portion of the code shown above. If I fail to catch PrintHelpMessage, for instance, the application will just not display a help message.

In my opinion, client code would look a look cleaner if it was possible to call a CliktCommand function that handles all non-exceptional cases (such as displaying the help message) internally and just throws/forwards an Exception if an actual error occurred.

Furthermore, I think that exec would be a better name for such a function than just parse. I was thinking of something like this:

fun CliktCommand.exec(argv: Array<String>): Int {
    try {
        parse(argv)
    } catch (e: ProgramResult) {
        return e.statusCode
    } catch (e: PrintHelpMessage) {
        println(e.command.getFormattedHelp())
        return if (e.error) 1 else 0
    } catch (e: PrintMessage) {
        println(e.message)
        return if (e.error) 1 else 0
    }

    return 0
}

Client code could then look like this, for instance:

try {
    exitProcess(Hello().exec(args))
} catch (e: CliktError) {
    // Handle command line parsing errors...
} catch (e: Abort) {
    // Handle abnormal internal conditions...
}

Personally, I would even consider classes like PrintHelpMessage an implementation detail that should not be exposed by the public API. What do you think, would such a change make sense from the perspective of the library?

pnq32 avatar Dec 29 '21 10:12 pnq32