Error handling

[lspitzner]

This turned out to be a rather simple change that might make brittany usage a good bit more pleasant if you use any yet-unsupported stuff (TemplateHaskell comes to mind).

Could need a review/a bit more manual testing, as I don't think the test suite would catch a problem here.

[eborden]

@lspitzner could you add a bit more color to this? What about this makes it more pleasant? Maybe some examples?

[lspitzner]

eh, sure: Take Test.hs

{-# LANGUAGE TemplateHaskell #-}
main = do
  print                      42

myTh = [t|Maybe
  Int|]

Previous behaviour

> command/stdin . stdout ! stderr

> brittany Test.hs
! ERROR: brittany pretty printer returned syntactically invalid result.
! ERROR: encountered unknown syntactical constructs:
! HsBracket{} at stdin:(5,8)-(6,7)

> brittany --output-on-errors Test.hs
! ERROR: brittany pretty printer returned syntactically invalid result.
! ERROR: encountered unknown syntactical constructs:
! HsBracket{} at stdin:(5,8)-(6,7)
. {-# LANGUAGE TemplateHaskell #-}
. main = do
.   print 42
. 
. myTh = {- BRITTANY ERROR UNHANDLED SYNTACTICAL CONSTRUCT -}

with this PR

> brittany Test.hs
! WARNING: encountered unknown syntactical constructs:
!   HsBracket{} at stdin:(5,8)-(6,7)
!   -> falling back on exactprint for this element of the module
. {-# LANGUAGE TemplateHaskell #-}
. main = do
.   print 42
. 
. myTh = [t|Maybe
.   Int|]

The behaviour is roughly "take each top-level module element, pass it through brittany, if you get errors, pass it through brittany --exactprint-only instead".

[lspitzner]

But the granularity is really the top-level module elements; if you have

{-# LANGUAGE TemplateHaskell #-}
func =      42
 where
  cantbeformatted = [t|Maybe
    Int|]

then it says "somewhere in the func is something that is not handled, fall back on exactprint for the whole func, and the whitespaces before "42" would remain. Making this work recursively would be much more work; for that we'd rather handle all syntax that is missing yet :p

[eborden]

Thanks! Agreed this is much nicer behavior.

[lspitzner]

Thanks for the feedback.

After testing with multiple inputs, I got sidetracked thinking about proper exit codes for warnings. But this PR certainly does not make this any worse (we just now a few more instances of exit-code-0 plus warnings on stderr). And I think that is the right choice anyway. Merging, finally.