ndmitchell / cmdargs

Haskell library for command line argument processing
Other
91 stars 12 forks source link

use errorWithoutStackTrace with GHC 8+ #42

Closed simonmichael closed 7 years ago

simonmichael commented 7 years ago

To avoid console output like:

$ hledger -f t.j print -V
hledger: Unknown flag: -V
CallStack (from HasCallStack):
  error, called at ./System/Console/CmdArgs/Explicit.hs:143:15 in cmdargs-0.10.14-8WOlFWoK1XlCebTZE2edgd:System.Console.CmdArgs.Explicit

I think cmdargs should use errorWithoutStackTrace with GHC 8 and above. Eg something like:

error' =
#if __GLASGOW_HASKELL__ < 800
  error
#else
  errorWithoutStackTrace
#endif
simonmichael commented 7 years ago

Or, indeed, avoid calling error at all.

ndmitchell commented 7 years ago

Probably a custom exception type, and then a throw would be my guess, which would then default to not having the stack trace. Good spot.

simonmichael commented 7 years ago

/me adds a bounty to this issue: codemill$25

ndmitchell commented 7 years ago

Please note that I am unable to accept bounties (the amount of paperwork from my day job would be too much hassle), so if it does get fixed to your satisfaction and you want to, please donate any money to a charity of your choice.

That said, a bounty does make it clear that it's an important issue, so it's helpful to see.

simonmichael commented 7 years ago

Thanks Neil. I'll reply here:

The problem is you are calling processValue, which is essentially process but with an error on the Left. > See https://hackage.haskell.org/package/cmdargs-0.10.16/docs/src/System-Console-CmdArgs-Explicit.html#processValue

I suspect you might get better results if you were to handle the error yourself doing:

case process m args of Left x -> do hPutStrLn stderr x; exitFailure Right x -> return x Should I add a processValueIO that does that? My guess is that processValue will never present exactly > the UI you want.

I now understand differences betwee process/processArgs/processValue better. You're right, it's an easy fix on my side: change processValue mode args to either errorWithoutStackTrace id $ process mode args. So that's good. But also, I think processValue should be calling errorWithoutStackTrace. I believe processArgs already does, which I realised after some head-scratching.

Re bounty - not even a simple personal donation ? Ok, will do! I too like the signalling effect of bounties.

ndmitchell commented 7 years ago

processArgs does the hPutStrLn/exitFailure combo, since it's in the IO monad. Unfortunately processValue isn't so it can't do that. Note that hPutStrLn gives you more control than error since it removes the *** Exception prefix as well.

simonmichael commented 7 years ago

Ok, but is it ever the right behaviour for cmdargs to show a stack trace to command line users ?

ndmitchell commented 7 years ago

No, it's not, I decided you are right. I've made 2 changes in the newly released 0.10.17:

Add processValueIO for more controlled error messages
#529, don't include the stack trace in processValue
simonmichael commented 7 years ago

Nice! $25 donated to EFF.