nitely / nim-regex

Pure Nim regex engine. Guarantees linear time matching
https://nitely.github.io/nim-regex/
MIT License
228 stars 21 forks source link

Remove all procs returning Option[T] #7

Closed nitely closed 6 years ago

nitely commented 6 years ago

Remove them all in favor of template blocks:

match("abc", re"abc"):
  assert matched.boundaries == 0 .. 2  # matched var gets injected

Can't support both cuz of https://github.com/nim-lang/Nim/issues/7503 . Also, I dislike Option[T]

dom96 commented 6 years ago

Sucks that you have to do this, but perhaps you could rename the procs returning an Option[T]?

nitely commented 6 years ago

I'd like to avoid having multiple APIs doing the same thing. I guess I could remove the optional start param in the hope that no one uses it and deprecate the APIs properly.

About the template based API: it seems optional parameters trigger the argument resolution on untyped templates, so I've to do overloading and I'm not sure I want that either nevermind, that does not work either. But python's re does not have a start param so maybe we can do without it, also I think toOpenArray could be used instead of it in the future if required.

dom96 commented 6 years ago

Please don't force users to use a template based API. You don't have to use Option[T], but only offering templates is inflexible. What if I want to pass the result of a match to one of my functions, even if the match didn't succeed?

nitely commented 6 years ago

What if I want to pass the result of a match to one of my functions, even if the match didn't succeed?

You would pass a RegexMatch default value:

var m: RegexMatch
match(...):
  m = matched
# pass m to some function
dom96 commented 6 years ago

Still, really not a big fan of that. Why not just use proc match(s: string, r: RegexPattern, m: var RegexMatch): bool?

nitely commented 6 years ago

I didn't think of it :sweat_smile:

var m: RegexMatch
if match("abc", re"abc", m):
   assert m.boundaries == 0 .. 2

# vs

match("abc", re"abc"):
  assert matched.boundaries == 0 .. 2
nitely commented 6 years ago

@dom96 yeah, I like your proposal better. I'm not happy with the implicit matched var. I'll keep this open for a while in case someone comes up with something else.

nitely commented 6 years ago

Done. https://github.com/nitely/nim-regex/commit/771a9e917da4657aca62372d6e5a2839b7b06d26