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

Regex, runtime error #378

Closed
gunpinyo opened this issue Aug 29, 2015 · 10 comments
Closed

Regex, runtime error #378

gunpinyo opened this issue Aug 29, 2015 · 10 comments

Comments

@gunpinyo
Copy link

In core package, module Regex

regex : String -> Regex

This function can produce runtime error if string contain invalid pattern.

Here is an example form elm-repl

b = Regex.regex("[a-z")
SyntaxError: Invalid regular expression: /[a-z/: Unterminated character class

It would be better if we change regex to be

regex : String -> Maybe Regex

it will return Nothing if string contains invalid pattern.

@eeue56
Copy link
Contributor

eeue56 commented Aug 29, 2015

I think it makes sense for regex to be regex : String -> Result String Regex here, so that the error message is captured.
If it's okay, I'll make a pull request for this so that it can be merged if it's approved.

@mgold
Copy link
Contributor

mgold commented Aug 29, 2015

Yeah, it's either that or keep the runtime error. Most regexes won't be generated dynamically (in signals or functions), so there's a case for keeping the runtime error since it will be easier to work with the absence of a Result or Maybe. (We seem to be getting useful error messages, so I support a Result in the API.)

@rtfeldman
Copy link
Member

In the spirit of these guidelines I've made a gist for a potentially interesting solution to this. Any discussion of that idea is welcome over on the gist itself, but let's not derail this issue with that stuff. 😄

@jvoigtlaender
Copy link
Contributor

Is this "the same" as https://github.com/elm-lang/core/issues/157? Should we close that other issue, then?

@eeue56
Copy link
Contributor

eeue56 commented Aug 30, 2015

I've created #380 that will change regex to return a Result instead, but I think it needs further discussion before merging. Having to use case..of everywhere doesn't quite fit right.

Perhaps leaving regex as it's own function and introducing safeRegex : String -> Result String Regex might be a better solution, with regex intended for static expressions and safeRegex for dynamic ones.

This would remove the need to introduce a new syntax as per rtfeldman's gist, though leave a runtime error possible.

@DemiMarie
Copy link

Regex literals might be a solution. That way invalid regexps are a compile time error

@mgold
Copy link
Contributor

mgold commented Dec 20, 2015

That's been brought up before, and I agree would be a good solution. But it involves a compiler change, and you'd have to validate JS regexes in Haskell.

@tmcw
Copy link

tmcw commented Jul 10, 2016

For my (admittedly rare) usecase, I'm creating regular expressions based on user input, in order to filter a list and extract results with matching groups - so unfortunately a literal syntax only would make that usecase more difficult - right now I'm porting from a JavaScript version which uses new RegExp(variable) in order to create regular expressions on the fly.

Returning a Maybe Regex would fit my usage perfectly, though I understand if that's not the most common way people interact with regular expressions.

@evancz evancz mentioned this issue Sep 22, 2016
@evancz
Copy link
Member

evancz commented Sep 22, 2016

Consolidated regex stuff in #722. Follow along there!

@evancz evancz closed this as completed Sep 22, 2016
@evancz
Copy link
Member

evancz commented Jul 8, 2017

The plan is to move regular expressions to elm-lang/regexp or something, and it will have something like this:

fromString : String -> Result String RegExp

As seen here: https://github.com/evancz/regexp/blob/master/src/RegExp.elm#L44

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

No branches or pull requests

8 participants