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

nre returns "" instead of nil for missing matches #9243

Closed
flaviut opened this issue Oct 7, 2018 · 3 comments
Closed

nre returns "" instead of nil for missing matches #9243

flaviut opened this issue Oct 7, 2018 · 3 comments

Comments

@flaviut
Copy link
Contributor

flaviut commented Oct 7, 2018

let ex2 = "foo".find(re("(?<foo>foo)(?<bar>bar)?"))
doAssert ex2.captures["bar"] == nil

In this example, captures cannot return nil as nil is used to represent the empty string. The distinction between "no match" and "empty match" is significant and useful when it comes to regular expressions.

@flaviut flaviut changed the title nre returns "" instead of nil for missing matches` nre returns "" instead of nil for missing matches Oct 7, 2018
@timotheecour
Copy link
Member

other std libraries had the same issue after the not-nil string change.
what do you suggest for ease of migration?
#8638 would help assess impact of such change

@flaviut
Copy link
Contributor Author

flaviut commented Oct 8, 2018

A possible option for nre might be to have captures[] throw an exception when attempting to access an undefined capture, and provide some sort of API to detect if a capture exists (e.x. "foo" in captures or 4 in captures).

It'd still be necessary to figure out what the correct return type for captures.toSeq (and captures.toTable) would be, although there are still issues with nim-lang/RFCs#512 there. I'm leaning towards returning seq[Option[string]] there, but there might be some other way to do it.

@Araq
Copy link
Member

Araq commented Oct 9, 2018

Go for it.

@flaviut flaviut self-assigned this Oct 28, 2018
@Araq Araq closed this as completed in c0a47f7 Dec 12, 2018
Araq added a commit that referenced this issue Dec 12, 2018
Fix for #9243, nre returns "" instead of nil for missing matches
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants