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

Include exception handling in force parse #64

Open
exaV opened this issue Sep 6, 2018 · 3 comments
Open

Include exception handling in force parse #64

exaV opened this issue Sep 6, 2018 · 3 comments

Comments

@exaV
Copy link

exaV commented Sep 6, 2018

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

@exaV exaV changed the title Include exception in force parse Include exception handling in force parse Sep 6, 2018
@xenomachina
Copy link
Owner

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
Copy link
Author

exaV commented Sep 18, 2018

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 #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
Copy link
Author

exaV commented Sep 18, 2018

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

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

2 participants