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

New module Text.Parsing.Parser.String.Basic #142

Merged
merged 1 commit into from
Jan 26, 2022
Merged

Conversation

jamesdbrock
Copy link
Member

@jamesdbrock jamesdbrock commented Jan 13, 2022

Description of the change

New module Text.Parsing.Parser.String.Basic

New functions

  • Parser.String.Basic.number
  • Parser.String.Basic.intDecimal

Moved the Parser.Token parsers digit, hexDigit, octDigit, upper, space, letter, alphaNum into the new module Parser.String.Basic.

Resolves the issues #73 #115 . Actually it doesn't resolve them directly because we still have the float token parser which was failing to parse correctly. We're keeping the float token parser which is documented to conform to “the grammar rules of the Haskell report,” but who knows if that's really true or not. Instead we've added a new number parser for parsing a Number, which I think is what most people want anyway.


Checklist:

  • Added the change to the changelog's "Unreleased" section with a link to this PR and your username
  • Linked any existing issues or proposals that this pull request should close
  • Updated or added relevant documentation in the README and/or documentation directory
  • Added a test for the contribution (if applicable)

@jamesdbrock jamesdbrock linked an issue Jan 13, 2022 that may be closed by this pull request
@jamesdbrock
Copy link
Member Author

The float parser was introduced in #27

@jamesdbrock jamesdbrock force-pushed the bugfix-token-float branch 2 times, most recently from 9c53c83 to 0c6de10 Compare January 14, 2022 00:50
@jamesdbrock jamesdbrock force-pushed the bugfix-token-float branch 4 times, most recently from 5b2f645 to eb81202 Compare January 23, 2022 05:50
New functions
- `Parser.String.Basic.number`
- `Parser.String.Basic.intDecimal`

Moved the `Parser.Token` parsers `digit`, `hexDigit`, `octDigit`, `upper`,
`space`, `letter`, `alphaNum` into the new module `Parser.String.Basic`.
@jamesdbrock jamesdbrock marked this pull request as ready for review January 23, 2022 06:06
@jamesdbrock jamesdbrock linked an issue Jan 23, 2022 that may be closed by this pull request
@jamesdbrock jamesdbrock changed the title TokenParser float bugfix New module Text.Parsing.Parser.String.Basic Jan 23, 2022
@@ -161,8 +161,6 @@ instance plusParserT :: Monad m => Plus (ParserT s m) where

instance alternativeParserT :: Monad m => Alternative (ParserT s m)

instance monadZeroParserT :: Monad m => MonadZero (ParserT s m)
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought we weren't removing these until v0.15.0 comes out?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh? Thanks @JordanMartinez good catch.

Copy link
Member Author

Choose a reason for hiding this comment

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

I can't find a reference for this, can you post a link please @JordanMartinez ?

Copy link
Contributor

Choose a reason for hiding this comment

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

See purescript/purescript-lists#176

We have a couple of MonadZero issues like that scattered throughout the core and contrib libs.

Copy link
Contributor

Choose a reason for hiding this comment

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

There’s a strict requirement that we don’t remove the warning in core libraries, and we’ve largely stuck to that in non-core libraries too. But there’s not a requirement that we keep these in place other than the core org (I’ve removed it elsewhere when making breaking changes 😅)

@thomashoneyman
Copy link
Contributor

We're keeping the float token parser which is documented to conform to “the grammar rules of the Haskell report,” but who knows if that's really true or not.

Personally speaking, I don't know why we even have the Haskell and Java parsers included with this library. They feel like they belong in some companion package, like purescript-parsing-language-haskell, if they're desired or used at all. It feels weird to make decisions in this library based on the grammar rules of the Haskell report. I'd be happy removing the language parsers altogether (do they even work?).

@thomashoneyman
Copy link
Contributor

@jamesdbrock I noticed this released as 8.2.0, but this looks like a breaking change because of functions changing modules. Was it supposed to be 9.0.0?

@jamesdbrock
Copy link
Member Author

@thomashoneyman

I'd be happy removing the language parsers altogether

Yeah I've been thinking that too.

but this looks like a breaking change because of functions changing modules

I'm re-exporting the functions from the old modules so there's no breaking change.

@jamesdbrock
Copy link
Member Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Float parser does not parse negative numbers Rounding errors when parsing floats
3 participants