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

Use megaparsec 6.0 #56

Merged
merged 1 commit into from
Aug 3, 2017
Merged

Use megaparsec 6.0 #56

merged 1 commit into from
Aug 3, 2017

Conversation

juanpaucar
Copy link
Contributor

@juanpaucar juanpaucar commented Jul 31, 2017

@juanpaucar juanpaucar changed the title [WIP] Use megaparsec 6 Use megaparsec 6 Jul 31, 2017
@juanpaucar juanpaucar changed the title Use megaparsec 6 Use megaparsec 6.0 Jul 31, 2017
@juanpaucar juanpaucar force-pushed the use_megaparsec_6 branch 2 times, most recently from 737ba2e to 07ebc72 Compare July 31, 2017 15:26
@juanpaucar juanpaucar self-assigned this Jul 31, 2017
Copy link

@sebashack sebashack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious about the comment I made... everything else looks good...

@@ -141,5 +142,5 @@ spec = describe "parse" $ do
it "doesn't allow more configuration options after a quoted value" $
parseConfig `shouldFailOn` "foo='bar'baz='buz'"

parseConfig :: String -> Either (ParseError Char Dec) [ParsedVariable]
parseConfig :: String -> Either (ParseError Char Void) [ParsedVariable]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@juanpaucar could you just tell me why the type Dec changed to Void?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because as mentioned in he first part of the documentation I defined a type alias for parser type Parser = Parsec Void String and ParseError must keep the same type. So, in this case it would be Void

Copy link
Member

@sestrella sestrella left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@juanpaucar looks good so far. I just left some comments.

dotenv.cabal Outdated
@@ -55,7 +55,7 @@ executable dotenv
, base-compat >= 0.4
, dotenv >= 0.3.1.0
, optparse-applicative >=0.11 && < 0.15
, megaparsec >= 5.0 && < 6.0
, megaparsec >= 5.0 && < 7.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@juanpaucar I think you need to increase the lower boundary as well.

dotenv.cabal Outdated
@@ -75,11 +75,14 @@ library

build-depends: base >=4.6 && <5.0
, base-compat >= 0.4
, megaparsec >= 5.0 && < 6.0
, megaparsec >= 5.0 && < 7.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@juanpaucar same here

Copy link
Member

@sestrella sestrella left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@juanpaucar nice job. LGTM

@juanpaucar juanpaucar merged commit 478a5c9 into master Aug 3, 2017
@juanpaucar juanpaucar deleted the use_megaparsec_6 branch August 3, 2017 15:01
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.

3 participants