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

regexp: simplify return types of S.match and S.matchAll #694

Merged
merged 1 commit into from
Aug 22, 2020

Conversation

davidchambers
Copy link
Member

match and matchAll are currently not enjoyable to use. I know why the return types are complex, but I still find accessing captured values annoying:

> S.join (S.chain (S.compose (S.head)
.                            (S.prop ('groups')))
.                 (S.match (/^(info|warn|error): (.*)$/i)
.                          ('info: Opening database connection...')))
Just ('info')

This pull request simplifies the match type, making captured values easier to access:

> S.join (S.chain (S.head)
.                 (S.match (/^(info|warn|error): (.*)$/i)
.                          ('info: Opening database connection...')))
Just ('info')

Before:

match :: NonGlobalRegExp -> String -> Maybe { match :: String, groups :: Array (Maybe String) }
matchAll :: GlobalRegExp -> String -> Array { match :: String, groups :: Array (Maybe String) }

After:

match :: NonGlobalRegExp -> String -> Maybe (Array (Maybe String))
matchAll :: GlobalRegExp -> String -> Array (Array (Maybe String))

Losing match :: String is not a problem in most cases, for the reason given in #686:

Unlike replace', S.replaceBy ignores the “match” argument provided by String#replace (when given a function). This simplifies the function's type, and saves one from using S.K (...) when only the captured groups are important (as is commonly the case). If one does require access to the matched substring as a whole, one can use /(...)/ to capture it. Admittedly, this approach is impractical if the RegExp object is defined in another module, but in such exceptional cases one can of course use String#replace directly.

Interestingly, the proposed type of match is the same as the function's type prior to #283. The incoherence @rjmk exposed in #253 does not apply, though, as the match is now omitted rather than prepended to the array of captured values.

@davidchambers davidchambers requested a review from a team August 16, 2020 16:42
@davidchambers davidchambers merged commit 70cb764 into master Aug 22, 2020
@davidchambers davidchambers deleted the davidchambers/match branch August 22, 2020 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants