Use ADTs for ParseError, PState and intermediate results

[felixSchl]

I have seen a major speed up of neodoc after this change. For an example, the lexer processing the git branch example in the repo ran at ~ 44ms before the change, and at around ~ 14 ms after the change.

Be aware that this will break the API though as all these types are exported.

[paf31]

Let's hide the constructor for this in the exports list, and provide a smart constructor. We might want to add additional information here later, without breaking the API.

[paf31]

Thanks for this! Some of my comments are not specific to this change, but now is a good time to fix some of these issues, since we'll need a major version bump anyway. Do you mind taking a look at these?

[felixSchl]

Thank you for the feedback, I will address all these issues. I have a question in regards to smart constructors though: I have a usecase where I need to match and construct the intermediate result since I am opening up ParserT manually and run code in the monad underneath: https://github.com/felixSchl/neodoc/blob/development/src/Language/Docopt/ArgParser/Parser.purs#L841. How could we achieve this if the constructors are hidden? Could we maybe export these types from an internal module? So I could import Text.Parsing.Internal on my own risk?

[paf31]

@felixSchl I think there are at least a couple of good options:

1. export lenses to access the fields of the data constructors
2. provide instances for standard type classes like MonadState, and functions to set flags like consumed

I think both would be good, but we don't need to do all of this at once.

[felixSchl]

@paf31 I have taken care of all the low hanging fruit. With the smart constructors and hiding the types, you'll have to have some patience with me. The WIP commit at the end (which I will remove from the history before we merge) reflects where I'm currently at.

It currently does not build:

  An export for ParserT requires the following to also be exported:

    PState
    Result

The only way I see around that is to add a smart constructor for ParserT also.

[paf31]

@felixSchl We should be able to export Result without its data constructor, and ParserT(..) too. I wouldn't export the PState constructor either, and instead just provide a function initialState :: forall s. s -> PState s.

This is looking good though, thanks!

[paf31]

I've ported this over to my updated branch. Thanks for the PR though!