-
Notifications
You must be signed in to change notification settings - Fork 4
Update for PureScript 0.14 #9
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
Conversation
/node_modules/ | ||
/output/ | ||
/.psci* | ||
/.psc* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed this because of the generated .psc-ide-port
which slips through the .gitignore otherwise.
src/Data/Version/Haskell.purs
Outdated
bs <- option Nil (hyphen *> identifier `sepBy` hyphen) | ||
eof | ||
pure $ Version as bs | ||
pure $ Version (fromFoldable as) bs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-empty combinators like sepBy1
now return NonEmptyList
as of:
purescript-contrib/purescript-parsing#102
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think we should consider updating the definition of Version
here instead?
data Version = Version (NonEmptyList Int) (List String)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sepBy1
means that the parser already can't ever produce a version with no integer components anyway, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that’s probably the better idea. I forgot that the constructor isn’t exported
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The constructor in this module is exported, so it would be a breaking change (it's the other one whose constructor isn't exported). But this is kind of a breaking change anyway since we're effectively dropping support for pre 0.14.x versions, and I think this module is not used in many places either.
isAsciiAlpha :: Char -> Boolean | ||
isAsciiAlpha ch = between 'a' 'z' (toLower ch) | ||
isAsciiAlpha = | ||
between (codePointFromChar 'a') (codePointFromChar 'z') <<< toLowerSimple <<< codePointFromChar |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The unicode
package now uses code points for everything; toLower
now returns Array CodePoint
, but given that we're working with ascii here I thought toLowerSimple
would suffice (CodePoint -> CodePoint
).
@hdgarrood I've updated the |
Thanks! |
Thanks for merging! If you tag a release I can take care of updating this in package sets & Pursuit. |
Published v6.0.0! I always publish using |
This PR updates for PureScript 0.14 by taking the following actions:
unicode
andparsing
libraries (full comments in the code inline below).I've verified this builds and passes with
pulp build
andpulp test
.