Pass unknown syntactical constructs through instead of crashing #109

New Issue

2018-01-16T13:58:58+01:00

mrkgnao commented

2018-01-16 13:58:58 +01:00

(Migrated from github.com)

With a file with pattern synonyms in it, for instance, Brittany throws errors. I'm fine with it supporting a subset of full GHC Haskell syntax, but could there be a switch or something to pass unknown syntactic forms through unmodified (like is done for data declarations at present)?

👍 5

mrkgnao commented

2018-01-26 11:45:12 +01:00

(Migrated from github.com)

I wouldn't mind implementing this myself if someone points me to a source location or two.

eborden commented

2018-01-28 20:49:11 +01:00

(Migrated from github.com)

@mrkgnao brittany does this in a number of places, but you'll generally be looking in src/Language/Haskell/Brittany/Internal/Layouters/.

@mrkgnao `brittany` does this in a number of places, but you'll generally be looking in `src/Language/Haskell/Brittany/Internal/Layouters/.`

lspitzner commented

2018-02-03 23:10:33 +01:00

(Migrated from github.com)

A quick fix, just for this extension, might be to modify ppDecl to just ignore the PatSynBind node, along the lines of

ppDecl :: LHsDecl RdrName -> PPMLocal ()
ppDecl d@(L loc decl) = case decl of
  SigD sig -> -- trace (_sigHead sig) $
              withTransformedAnns d $ do
    -- runLayouter $ Old.layoutSig (L loc sig)
    briDoc <- briDocMToPPM $ layoutSig (L loc sig)
    layoutBriDoc briDoc
  ValD PatSynBind{} ->
    -- fall-back solution for -XPatternSynonyms
    briDocMToPPM (briDocByExactNoComment d) >>= layoutBriDoc
  ValD bind -> -- trace (_bindHead bind) $
               withTransformedAnns d $ do
    -- Old.layoutBind (L loc bind)
    briDoc <- briDocMToPPM $ do
      eitherNode <- layoutBind (L loc bind)
      case eitherNode of
        Left  ns -> docLines $ return <$> ns
        Right n  -> return n
    layoutBriDoc briDoc
  _ -> briDocMToPPM (briDocByExactNoComment d) >>= layoutBriDoc

this is in Language.Haskell.Brittany.Internal.

Regarding a more proper/general solution: It is true that the code currently assumes that there are "termination-worthy" errors, where we could just fall back on exactprint. We could wrap the whole ppDecl, run the writer-of-warnings/errors part of the transformer stack, check for termination-worthy errors, revert to exactprint and continue without aborting. This might turn both ErrorUnusedComment and ErrorUnknownNode into warnings effectively.

If we go that route, i'd consider throwing out the PPM transformer entirely (but not PPMLocal) because it does not make sense to invent a specific transformer stack if we use the type for one function only (and then have to unwrap/switch to a different stack again anyways.) I.e. let ppModule have a transformer-free type.

pull-requests for either solution welcome. I should probably do the PPM-related refactoring, as i started overusing MultiRWS - well the requirements changed over time, but whatever.

A quick fix, just for this extension, might be to modify `ppDecl` to just ignore the `PatSynBind` node, along the lines of ~~~~.hs ppDecl :: LHsDecl RdrName -> PPMLocal () ppDecl d@(L loc decl) = case decl of SigD sig -> -- trace (_sigHead sig) $ withTransformedAnns d $ do -- runLayouter $ Old.layoutSig (L loc sig) briDoc <- briDocMToPPM $ layoutSig (L loc sig) layoutBriDoc briDoc ValD PatSynBind{} -> -- fall-back solution for -XPatternSynonyms briDocMToPPM (briDocByExactNoComment d) >>= layoutBriDoc ValD bind -> -- trace (_bindHead bind) $ withTransformedAnns d $ do -- Old.layoutBind (L loc bind) briDoc <- briDocMToPPM $ do eitherNode <- layoutBind (L loc bind) case eitherNode of Left ns -> docLines $ return <$> ns Right n -> return n layoutBriDoc briDoc _ -> briDocMToPPM (briDocByExactNoComment d) >>= layoutBriDoc ~~~~ this is in `Language.Haskell.Brittany.Internal`. Regarding a more proper/general solution: It is true that the code currently assumes that there are "termination-worthy" errors, where we could just fall back on exactprint. We could wrap the whole `ppDecl`, run the writer-of-warnings/errors part of the transformer stack, check for termination-worthy errors, revert to exactprint and continue without aborting. This might turn both `ErrorUnusedComment` and `ErrorUnknownNode` into warnings effectively. If we go that route, i'd consider throwing out the `PPM` transformer entirely (but not `PPMLocal`) because it does not make sense to invent a specific transformer stack if we use the type for one function only (and then have to unwrap/switch to a different stack again anyways.) I.e. let `ppModule` have a transformer-free type. pull-requests for either solution welcome. I should probably do the PPM-related refactoring, as i started overusing MultiRWS - well the requirements changed over time, but whatever.

Sign in to join this conversation.

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#109