Skip to content

Add unsafeRegex #74

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 3 commits into from
Dec 25, 2016
Merged

Add unsafeRegex #74

merged 3 commits into from
Dec 25, 2016

Conversation

no-longer-on-githu-b
Copy link
Contributor

Fixes #73.

@LiamGoodacre
Copy link
Member

👍 Except that usually we separate unsafe functions into sub modules like Data.String.Regex.Unsafe or Data.String.Regex.Partial.

@no-longer-on-githu-b
Copy link
Contributor Author

no-longer-on-githu-b commented Dec 22, 2016

Seems reasonable, requires another import though. The goal is mostly to reduce the amount of boilerplate needed for (poor man's) regex literals.

-- | Constructs a `Regex` from a pattern string and flags. Fails with
-- | an exception if the pattern contains a syntax error.
unsafeRegex :: String -> RegexFlags -> Regex
unsafeRegex s f = unsafePartial $ fromRight (regex s f)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: there's no need for the $ here.

@paf31
Copy link
Contributor

paf31 commented Dec 24, 2016

Yes, could we please move this into Data.String.Regex.Partial? Looks good otherwise.

@garyb
Copy link
Member

garyb commented Dec 24, 2016

Probably should be Data.String.Regex.Unsafe in this case, since there's no partial constraint?

@LiamGoodacre
Copy link
Member

Could possibly have a Partial version too/instead.

@garyb
Copy link
Member

garyb commented Dec 24, 2016

We already have a Partial version - that's the normal version 😄, this is a convenience to avoid that.

@garyb
Copy link
Member

garyb commented Dec 24, 2016

Well, kinda. Having a Partial verison would eliminate the need for fromRight, but that's not really much better.

@LiamGoodacre
Copy link
Member

@garyb you mean eliminate the need for unsafePartial, right? The fromRight is what would make this need a Partial constraint.

@garyb
Copy link
Member

garyb commented Dec 24, 2016

I meant if we had Partial => String -> RegexFlags -> Regex you'd still end up having to always unsafePartial for the case in the linked issue, so just providing a fully unsafe version is the only way to avoid the annoyance :)

@no-longer-on-githu-b
Copy link
Contributor Author

Yeah, it's the use of the function we want to make easier, not the definition.

@LiamGoodacre
Copy link
Member

@garyb oh you meant: eliminate the need for fromRight on use, not on definition

@no-longer-on-githu-b
Copy link
Contributor Author

I'll move the function to .Unsafe.

@paf31 paf31 merged commit 1108a60 into purescript:master Dec 25, 2016
@paf31
Copy link
Contributor

paf31 commented Dec 25, 2016

Great, 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.

4 participants