Skip to content

make match safe #14

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
merged 1 commit into from
Nov 4, 2014
Merged

make match safe #14

merged 1 commit into from
Nov 4, 2014

Conversation

davidchambers
Copy link
Contributor

String.prototype.match returns null if there is no match. For example:

> 'abc'.match(/x/)
null

This change is largely copied from #13.

"function _match(r, s, Just, Nothing) {\
\ var m = s.match(r);\
\ return m == null ? Nothing : Just(m);\
\}" :: forall a. Fn4 Regex String (a -> Maybe a) (Maybe a) (Maybe [String])
Copy link
Contributor

Choose a reason for hiding this comment

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

This type seems incorrect. Something like

forall r. Fn4 Regex String ([String] -> r) r r

makes more sense to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is one of the lines I copied without fully understanding. I'll update the type now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated. Could you explain ([String] -> r)?

Copy link
Contributor

Choose a reason for hiding this comment

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

The type r in our case is Maybe [String], so Just and Nothing have type [String] -> Maybe [String] and Maybe [String] respectively. But looking at the code, there is nothing which uses the fact that the result is of type Maybe [String], all we care about is that

  • Just(m) and Nothing have the same type.
  • Just takes an argument of type [String]

so we can generalize the result type from Maybe [String] to r for all r.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Terrific explanation! Thank you, @paf31.

@paf31
Copy link
Contributor

paf31 commented Nov 3, 2014

Looks good, thanks. I put one comment on the code itself, though.

garyb added a commit that referenced this pull request Nov 4, 2014
@garyb garyb merged commit 4e7f0d5 into purescript:master Nov 4, 2014
@garyb
Copy link
Member

garyb commented Nov 4, 2014

Thanks 👍

Before making a release for this I'll check that there's nothing else that needs making safe, as this should be a new v0.x.0 release given that it breaks the API yet again!

@davidchambers davidchambers deleted the match branch November 4, 2014 19:12
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