Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove user state from Megaparsec state #18

Closed
mrkkrp opened this issue Aug 13, 2015 · 17 comments
Closed

Remove user state from Megaparsec state #18

mrkkrp opened this issue Aug 13, 2015 · 17 comments

Comments

@mrkkrp
Copy link
Owner

mrkkrp commented Aug 13, 2015

Parsec itself is a monad transformer (ParsecT). This means that state monad can be used with it easily if user of the library needs to. However, Parsec, unlike other similar libraries, keep special user state field in its State record. As far as I can tell, in vast majority of cases user state is nothing but unit ().

Here is a comment found in original Parsec's source code, above definition of instance of MonadState:

I'm presuming the user might want a separate, non-backtracking state aside from the Parsec user state.

I'm seeing stateUser field as unnecessary complication. I'm proposing removal of this custom user state from Megaparsec. What do you think, @albertnetymk, @neongreen?

@neongreen
Copy link
Contributor

Can we get backtracking state by combining ParsecT and StateT? (I think Pandoc uses backtracking state.)

@mrkkrp
Copy link
Owner Author

mrkkrp commented Aug 13, 2015

I'm not sure. Ability to backtrack user state may be useful in some circumstances. What do you mean by "Pandoc uses this ability". Does Pandoc use Parsec's user state or some other parser in combination with StateT to achieve this effect?

@neongreen
Copy link
Contributor

Sorry, I realised it wasn't clear half a minute posting the comment and edited it.

I meant that Pandoc uses backtracking state.

@neongreen
Copy link
Contributor

I. e. Parsec user state.

@mrkkrp
Copy link
Owner Author

mrkkrp commented Aug 13, 2015

OK, I have to admit that I underestimated benefits of backtracking user state. In this case, we should continue to be maximalists in this Haskell-parsing world (because there are too many minimalists) and keep backtracking user state in Megaparsec as additional selling point.

@mrkkrp mrkkrp closed this as completed Aug 13, 2015
@neongreen
Copy link
Contributor

After reading this article I wonder now whether you could remove user state after all, since backtracking state could seemingly be achieved with StateT u (Parsec s ()) a.

@neongreen
Copy link
Contributor

Hm, no, then we'd have to use lift on all parsers.

@mrkkrp: how bad of an idea is it to create a MonadParsec class so that ParsecT would commute better with StateT, WriterT, etc? It looks like if there was a MonadParsec we could get backtracking/ordinary State, backtracking/ordinary Writer, etc easily (I actually needed WriterT [String] (Parser a) recently to add optional warnings to parsers).

@mrkkrp
Copy link
Owner Author

mrkkrp commented Sep 7, 2015

@neongreen, Interesting. We will return to this idea when obligatory part of our work is completed (i.e. finish Text.Megaparsec.Lexer and tests for it).

Which functions would you include into MonadParsec class?

@neongreen
Copy link
Contributor

@mrkkrp, it turns out that Edward Kmett already did it: Parsing, CharParsing. If only parsers didn't have both parsec and attoparsec as dependencies, we could even use it.

@mrkkrp
Copy link
Owner Author

mrkkrp commented Sep 7, 2015

@neongreen, Thanks for the links, I will return to this when everything else is finished. Indeed it looks like the right thing do to.

@abooij
Copy link
Contributor

abooij commented Sep 7, 2015

Before you reinvent the wheel and copy/paste this class, perhaps the author can be contacted to find a solution. It is strange that just to reasonably work with some parser in some typeclass, one would have to depend on all libraries that support that interface. So I think something will have to happen upstream.

@neongreen
Copy link
Contributor

@tulcod it's not “all libraries”, it's that it would be hard to convince authors of parsec and attoparsec to depend on parsers. Other libraries – like trifecta – depend on parsers (not the other way around).

@mrkkrp
Copy link
Owner Author

mrkkrp commented Sep 7, 2015

I don't really get why this thing depends on parsec and attoparsec.

it's that it would be hard to convince authors of parsec and attoparsec to depend on parsers

…and because of that parsers depends on them? This is ridiculous. I guess it would just as hard to convince author of parsers to drop these dependencies :-D

@neongreen
Copy link
Contributor

Well, the alternative is splitting it into parsers, parsers-parsec, parsers-attoparsec. I can't really blame Edward for wanting to minimise the amount of packages ne has to maintain.

I guess it would just as hard to convince author of parsers to drop these dependencies :-D

Most likely, yeah.

@mrkkrp
Copy link
Owner Author

mrkkrp commented Sep 7, 2015

@tulcod, I'm not against reinventing the wheel if it's easier then buying one and gives better results. For example Megaparsec have different naming conventions to parse various categories of characters, etc. Not sure we could reuse this “as is”.

@abooij
Copy link
Contributor

abooij commented Sep 7, 2015

@mrkkrp, well I definitely agree that we have to analyse carefully what this common interface would really express, since yes, many instances will have different naming and semantics. So most likely we will have to copy/paste and edit some stuff. But I'm just saying that before we do that, we should at least try to contact the author of parsers.

@neongreen
Copy link
Contributor

I created a separate issue for this so that it won't get forgotten.

tomjaguarpaw pushed a commit to tomjaguarpaw/megaparsec that referenced this issue Sep 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants