search

markhibberd / pirate

Non-horrible command line parsing.
https://github.com/markhibberd/pirate
BSD 3-Clause "New" or "Revised" License
42 stars 10 forks source link

Ensure correct exit codes when running #43

Closed charleso closed 9 years ago

charleso commented 9 years ago

My bad, I told @FrontierOtr that we shouldn't be doing this, but as it stands this function is possible to use and will make it impossible to return the correct exit code.

etorreborre commented 9 years ago

I'm on the fence with that one. On one hand it is good to have the error codes. On the other hand, having them in runOrFail rather than in PirateMain means that runOrFail is really, really unsafe to call anywhere else than in main which is not enforced.

charleso commented 9 years ago

I'm on the fence with that one. On one hand it is good to have the error codes. On the other hand, having them in runOrFail rather than in PirateMain means that runOrFail is really, really unsafe to call anywhere else than in main which is not enforced.

That was my first reaction too. It's either this or IO[Option[A]] or something. Right now the errors are being swallowed.

markhibberd commented 9 years ago

@charleso @etorreborre I am not sure I am following this correctly - but having written it, the only purpose of Runner is to run a program to the end, it not setting exit codes is a really really bad bug from my perspective. It is pretty much it's one and only job. Forcing use of PirateMain/PirateMainIO isn't feasable, Runner is meant to be the top level, those are just conveniences. I am a little concerned about the non termination related code in here, but I don't see how you can not be terminating.

etorreborre commented 9 years ago

I think it's ok but maybe it is worth mentioning in the comments somewhere that the program will exit after that and not just have a side-effect on the console.

markhibberd commented 9 years ago

@etorreborre @charleso I don't particularly mind how it is done, but it does have to call sys.exit at some point. My preference would be for this to return to being about 5 lines of code rather than 50, and I would like there to be pure versions that have the type IO[ExitStatus], but I really want there to a single call that just does the right thing. Just look through all of our code, the number of programs that don't set error codes is horrifying.

charleso commented 9 years ago

@markhibberd @etorreborre Let me know if that looks better.

markhibberd commented 9 years ago

@markhibberd https://github.com/markhibberd @etorreborre https://github.com/etorreborre Let me know if that looks better.

LGTM :100:

etorreborre commented 9 years ago

That's great! Thanks @charleso.