Error handling #273

2020-01-03T12:06:44+01:00

lspitzner commented

2020-01-03 12:06:44 +01:00

(Migrated from github.com)

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.

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 commented

2020-01-03 18:59:59 +01:00

(Migrated from github.com)

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

lspitzner commented

2020-01-03 22:06:10 +01:00

(Migrated from github.com)

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".

eh, sure: Take Test.hs ~~~~.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 commented

2020-01-03 22:11:37 +01:00

(Migrated from github.com)

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

But the granularity is really the top-level module elements; if you have ~~~~.hs {-# 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 commented

2020-01-03 22:24:01 +01:00

(Migrated from github.com)

Thanks! Agreed this is much nicer behavior.

tfausak (Migrated from github.com) approved these changes 2020-01-09 17:56:57 +01:00

tfausak (Migrated from github.com) left a comment

Awesome!

lspitzner commented

2020-01-23 01:16:54 +01:00

(Migrated from github.com)

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.

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.

Sign in to join this conversation.

No reviewers

tfausak

No Milestone

No project

No Assignees

1 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: hexagoxel/brittany#273