Skip to content

Improve list parser #12

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

Merged

Conversation

maksbotan
Copy link
Contributor

@maksbotan maksbotan commented Jun 4, 2020

Current parser for lists, like packages: in cabal.project.local,
fails to process some input, for example:

packages:
foo/foo.cabal
, bar/bar.cabal
, baz/baz.cabal

This commit simplifies the parser by using sepBy directly and by
handling indented fields more uniformly.

@Avi-D-coder
Copy link
Owner

See the test failure.

It also needs to parse newline delineated lists and mixed newline comma lists.
Cabal will except:

packages:
         ./
         ghcide

@maksbotan
Copy link
Contributor Author

Yeah, I'm already on it.

@maksbotan maksbotan force-pushed the maksbotan/fix-list-parser branch from c265bbf to 9af13b1 Compare June 4, 2020 21:05
@maksbotan
Copy link
Contributor Author

While reading Cabal sources I missed that it first parses fields into a tree and then parses its parts, that's why parseOptCommaList does not have to deal with indices.

This commit should work. I've also added a testcase.

Current parser for lists, like `packages:` in `cabal.project.local`,
fails to process some input, for example:

packages:
    foo/foo.cabal
  , bar/bar.cabal
  , baz/baz.cabal

This commit simplifies the parser by using `sepBy` directly and by
handling indented fields more uniformly.
@maksbotan maksbotan force-pushed the maksbotan/fix-list-parser branch from 9af13b1 to 7ae6642 Compare June 4, 2020 21:15
@Avi-D-coder Avi-D-coder merged commit 326f74f into Avi-D-coder:master Jun 4, 2020
@Avi-D-coder
Copy link
Owner

Avi-D-coder commented Jun 4, 2020

Thanks,
I'll publish new version in the next few days

@maksbotan
Copy link
Contributor Author

Thanks!
The tool is very useful, by the way :)

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

Successfully merging this pull request may close these issues.

2 participants