Skip to content

Conversation

thomashoneyman
Copy link
Contributor

This PR updates for PureScript 0.14 by taking the following actions:

  1. Updated the bower link to match the registry url for this package
  2. Updated dependencies to their new major versions in bower
  3. Updated the Pulp dependency in package.json to v15, for compatibility with PureScript 0.14
  4. Made changes to the code to accommodate changes in the unicode and parsing libraries (full comments in the code inline below).

I've verified this builds and passes with pulp build and pulp test.

/node_modules/
/output/
/.psci*
/.psc*
Copy link
Contributor Author

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.

bs <- option Nil (hyphen *> identifier `sepBy` hyphen)
eof
pure $ Version as bs
pure $ Version (fromFoldable as) bs
Copy link
Contributor Author

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

Copy link
Owner

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)

Copy link
Owner

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?

Copy link
Contributor Author

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

Copy link
Owner

@hdgarrood hdgarrood Mar 2, 2021

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
Copy link
Contributor Author

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).

See:
purescript-contrib/purescript-unicode#15

@thomashoneyman
Copy link
Contributor Author

@hdgarrood I've updated the Version definition to use NonEmptyList. It doesn't look like Travis runs on pull requests anymore, but I've ensured tests complete with pulp test.

@hdgarrood
Copy link
Owner

Thanks!

@hdgarrood hdgarrood merged commit 7a89bf9 into hdgarrood:master Mar 2, 2021
@thomashoneyman
Copy link
Contributor Author

Thanks for merging! If you tag a release I can take care of updating this in package sets & Pursuit.

@hdgarrood
Copy link
Owner

Published v6.0.0! I always publish using pulp publish so the docs should be on Pursuit already. I would appreciate you taking care of package-sets though, thanks!

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