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

add sepBy' and sepBy1' combinators #53

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

HuStmpHrrr
Copy link

this PR is majorly to introduce two combinators that have similar but slightly different behavior than original sepBy and sepBy1.

here is the illustration:

import Text.ParserCombinators.Parsec as Par
let cva1 = (char 'a' `sepBy` (spaces *> char ',' *> spaces)) :: Parser [Char]
let cva2 = (char 'a' `sepBy'` (spaces *> char ',' *> spaces)) :: Parser [Char] -- watch out, it's sepBy'
parse cva1 "" "a, a, a" -- fine
parse cva2 "" "a, a, a" -- fine
parse cva1 "" "a, a, a " -- not fine
parse cva2 "" "a, a, a " -- fine

here we defined cvas to be "comma separated letter 'a's". for cva1, it will accept such list of as without any trailing spaces, but if any trailing spaces exist, it will reject, while the one that uses sepBy' combinator accepts both cases.

the reason why sepBy doesn't accept the second case is, sep == spaces *> char ',' *> spaces, where sep has actually started to accept spaces already when it comes to the trailing spaces, and therefore it will expect more spaces, or comma, but not eof, so it rejects the input.

also another variation sepEndBy doesn't work because it requires the input terminates by the exact sep parser.

i've been looking around and couldn't find any existing solution to this incident so i ended up deciding to write one.

@hvr
Copy link
Member

hvr commented Aug 12, 2017

@mrkkrp
Copy link
Contributor

mrkkrp commented Aug 12, 2017

I don't think so. If I understand correctly, it's the same as sepBy with the exception that separator is tried with optionMaybe and if it fails we just return accumulator letting the sep parser get away with consuming part of input (spaces in the example).

I'd rather use the normal sepBy and pick trailing space (or whatever it happens to be) after it "manually". Just my opinion.

@mrkkrp
Copy link
Contributor

mrkkrp commented Aug 12, 2017

I actually honestly don't understand how cva2 is supposed to work. There is no try inside optionMaybe and so if spaces start to match, comma will be demanded and instead of returning Nothing (via the ... <|> pure Nothing construct) the whole thing will fail:

λ> let cva1 = (char 'a' `sepBy` (space *> char ',' *> space)) :: Parser [Char]
λ> let cva2 = (char 'a' `sepBy'` (space *> char ',' *> space)) :: Parser [Char]
λ> :set -XOverloadedStrings
λ> parseTest' cva1 "a, a, a"
"aaa"
λ> parseTest' cva2 "a, a, a"
"aaa"
λ> parseTest' cva1 "a, a, a "
1:9:
  |
1 | a, a, a 
  |         ^
unexpected end of input
expecting ',' or white space
λ> parseTest' cva2 "a, a, a "
1:9:
  |
1 | a, a, a 
  |         ^
unexpected end of input
expecting ',' or white space
λ>

Both cva1 and cva2 fail with trailing space for me. I tested with Megaparsec, maybe I forgot something about how Parsec works, but I think it'll fail with Parsec too. Can someone try it? I can try later today if desirable.

@mrkkrp
Copy link
Contributor

mrkkrp commented Aug 12, 2017

Here we go with Parsec:

The next big Haskell project is about to start!
λ> let cva1 = (char 'a' `sepBy` (spaces *> char ',' *> spaces)) :: Parser [Char]
λ> let cva2 = (char 'a' `sepBy'` (spaces *> char ',' *> spaces)) :: Parser [Char]
λ> parseTest cva1 "a, a, a"
"aaa"
λ> parseTest cva2 "a, a, a"
"aaa"
λ> parseTest cva1 "a, a, a "
parse error at (line 1, column 9):
unexpected end of input
expecting space or ","
λ> parseTest cva2 "a, a, a "
parse error at (line 1, column 9):
unexpected end of input
expecting space or ","
λ>

So I'm not sure what these new combinators solve.

int-index pushed a commit to int-index/parsec that referenced this pull request Sep 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants