consider optional capturing groups in `match`

[davidchambers]

match currently has the following type:

match :: Regex -> String -> Maybe [String]

This is problematic, though, when one considers optional capturing groups:

> 'goodbye'.match(/(good)?bye/)
['goodbye', 'good']
> 'bye'.match(/(good)?bye/)
['bye', undefined]

Perhaps the function's type should be Regex -> String -> Maybe [Maybe String]. This would make the function cumbersome in the case of a pattern with no capturing groups: matching /hello/ would either give Nothing or Just([Just("hello")]).

This is an example of a dynamically typed language allowing a function to do too many things. Now we have the unenviable job of making sense of it all. :)

[paf31]

👍

[davidchambers]

@paf31, shall I open a pull request to change the return type from [String] to [Maybe String]? If so, are we happy to lose the index and input properties? In other words,

Just(["bye", undefined, index: 0, input: "bye"])

would become

Just([Just("bye"), Nothing])

[paf31]

Do we currently get the index and input properties?

[davidchambers]

Do we currently get the index and input properties?

Yes, we do. The current implementation is as follows:

foreign import _match
  """
  function _match(r, s, Just, Nothing) {
    var m = s.match(r);
    return m == null ? Nothing : Just(m);
  }
  """ :: forall r. Fn4 Regex String ([String] -> r) r r

We simply apply String.prototype.match and wrap the result in a Just if the result is not null. Here's another way to represent the results of the expressions in the issue description:

> 'goodbye'.match(/(good)?bye/)
['goodbye', 'good', index: 0, input: 'goodbye']
> 'bye'.match(/(good)?bye/)
['bye', undefined, index: 0, input: 'bye']

[paf31]

Oh, I see. That sucks :) Yes, I suppose for now we should just collect the Strings.

[davidchambers]

replace' does not acknowledge the possibility of unmatched optional capturing groups:

replace' :: Regex -> (String -> Array String -> String) -> String -> String

I believe we should use the following type instead:

replace' :: Regex -> (String -> Array (Maybe String) -> String) -> String -> String

We would need to update the implementation accordingly.

Would you like me to submit a pull request?