xenomachina / kotlin-argparser

Easy to use and concise yet powerful and robust command line argument parsing for Kotlin
GNU Lesser General Public License v2.1
483 stars 33 forks source link

Include exception handling in force parse #64

Open exaV opened 6 years ago

exaV commented 6 years ago

I have lately been exploring kotlin for scripting purposes, using https://github.com/holgerbrandl/kscript. Kotlin-argparsers really shines in scripts because it's concise and still provides nice error messages.

On of the differences between regular Kotlin code and a KScript is that the script does not contain a main method. Instead it gets executed from top to bottom. Not having a main method suddenly makes the mainBody lambda look ugly, because I always have write something like this:

val parsedArgs = mainBody {
    ArgParser(args).parseInto(::Args)
}

The mainBody has to be there because otherwise we get ugly stacktraces instead of a simple error message like "missing option X". It would be much nicer if there was a method like parseInto() that would already contain the exception handling part so we could simply write:

val parsedArgs = ArgParser(args).parseIntoOrExit(::Args)

here is a full example

xenomachina commented 6 years ago

Thanks for reporting this. I've never used KScript, but ensuring that Kotlin-argparser works well with it sounds like a great idea.

For now, would it be possible to replace this:

val parsedArgs = mainBody {
    ArgParser(args).parseInto(::Args)
}

println(parsedArgs.message)

with this?

mainBody {
    val parsedArgs = ArgParser(args).parseInto(::Args)
    println(parsedArgs.message)
}

Making this less verbose would be nice, but part of the reason the "mainBody" is separate from "parseInto" is to make testing easier. mainBody calls exitProcess and also writes directly to System.out/System.err, and none of that is very easy to test. So the idea is that mainBody has factored out a bunch of difficult to test bits. The body you pass to mainBody can do nothing but call a "testableMain" method, and your tests can test that same method.

I should probably at least make this more clear in kotlin-argparser-example.

exaV commented 6 years ago

with this? mainBody { val parsedArgs = ArgParser(args).parseInto(::Args) println(parsedArgs.message) }

I reported this issue because I am not willing to put the whole script into a lambda in the first place for the same reasons I do not want to have a main method. The example I provided is somewhat minimal so maybe it did not seem significant there. However in any meaningful script most of the code would be wrapped into the mainBody lambda and thus have one level of indentation. Additional indentation makes the script harder to read, especially because usually there will be at least one or two more levels of indentation (say an if or a for).

I was already quite pleased when I realised I could use

val parsedArgs = mainBody {
    ArgParser(args).parseInto(::Args)
}

which lets me get around having additional indentation for the rest of the script. However having the mainBody lambda around parsing just seems unnecessarily verbose. Also my co-workers who are not familiar with the library asked my what the mainBody was for, which also hints at something, especially considering that they perfectly understood the rest of arg-parsing.

Coincidentally the maintainer of Kscript also opened an issue relating to this https://github.com/xenomachina/kotlin-argparser/issues/65 which indicated that I'm not the only one with this issue. I understand your reservations you have regarding testability, however as a user I would still much prefer not having to write mainBody to parse arguments.

exaV commented 6 years ago

Would it help testability if you declared the function I need as an extension function on Argparser in its own file? You could then test this file on its own.

I'm thinking of this:

fun <T> ArgParser.parse(constructor: (ArgParser) -> T) = mainBody {
    this.parseInto(constructor)
}

Of course the name could be different