regexp: add S.replaceWith and S.replaceBy

[davidchambers]

Competes with #685

This pull request adds the following two functions:

replaceWith ::                          String  -> RegExp -> String -> String
replaceBy   :: (Array (Maybe String) -> String) -> RegExp -> String -> String

Design decisions:

• Both functions take pattern :: RegExp, so there is no straightforward equivalent of s1.replace (s2, s3) for strings s1, s2, and s3. One could use S.replaceWith (s3) (S.regex ('') (S.regexEscape (s2))) (s1), but it would be simpler and clearer to use String#replace directly in such cases.

  Using pattern :: RegExp exclusively provides two benefits:

  • Simplicity. Supporting both RegExp and String would require splitting replaceWith into two functions (which would pose a naming challenge) or using Either RegExp String (which would not be ergonomic).

  • Clarity. It's unclear whether R.replace ('o') ('x') ('foo') should evaluate to 'fxo' or to 'fxx', whereas the presence of the g flag in /o/g indicates that all occurrences of o should be replaced, and the absence of the g flag in /o/ indicates that only the first occurrence of o should be replaced.

• Unlike replace, S.replaceWith does not support special replacement patterns. We are not obligated to expose every feature of the underlying API, and with S.replaceBy it is possible to reference values matched by capturing groups (the primary use case for special replacement patterns).

• Like replace', S.replaceBy ignores the “offset” and “string” arguments provided by String#replace (when given a function). These are rarely useful.

• Unlike replace', S.replaceBy ignores the “groups” argument provided by String#replace (when given a function) if the pattern contains any named capturing groups. I included a fix for this deficiency in purescript/purescript-strings#126.

• 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.

• Unlike replace', S.replaceBy acknowledges the existence of optional capturing groups:

  > S.replaceBy (S.show) (S.regex ('') ('(Pure)?(Script)')) ('JavaScript')
  'Java[Nothing, Just ("Script")]'

  Evaluating the equivalent PureScript expression results in an exception being thrown:

  > replace' (unsafeRegex "(Pure)?(Script)" noFlags) (const show) "JavaScript"
  ! TypeError: undefined is not an object (evaluating 's.length')

  I have proposed the use of Array (Maybe String) in place of Array String in purescript/purescript-strings#126.

I have opened sanctuary-js/sanctuary-site#88 to provide several examples of replaceWith and replaceBy in use.

@sanctuary-js/owners, what do you think of the proposed additions to the library?

[Bradcomp]

I think I like this one better than the other one. We lose the special replacement syntax from the other PR but I didn't even know that's a thing and I can't think of any case where I would want that over just using replaceBy. Removing that capability removes a gotcha, if they want that functionality String.replace isn't going anywhere.

I did like that with the other PR both functions began with the regexp, but with these names the parameter order you have makes perfect sense and reads well. I also like these names better than the other ones.

[davidchambers]

I did like that with the other PR both functions began with the regexp, but with these names the parameter order you have makes perfect sense and reads well.

It is convenient for S.replaceBy to take the replacement function first (and thus the pattern second) in case one wishes to define a variant for patterns without optional capturing groups:

//    replace__ :: RegExp -> (Array String -> String) -> String -> String
//
//    Variant of proposed function `S.replace_` (sanctuary-js/sanctuary#685)
//    for patterns without optional capturing groups.
const replace__ = S.flip (S.compose (S.flip (S.replace_)) (S.contramap (S.justs)));

//    replaceBy_ :: (Array String -> String) -> RegExp -> String -> String
//
//    Variant of proposed function `S.replaceBy` (sanctuary-js/sanctuary#686)
//    for patterns without optional capturing groups.
const replaceBy_ = S.compose (S.replaceBy) (S.contramap (S.justs));

I am aware of two other reasons to take the pattern second:

• replaceWith and replaceBy feel more closely related if their types are different → same → same → same rather than same → different → same → same; and
• replaceWith ('-') mirrors joinWith ('-').

[Avaq]

Should we consider B (replaceBy) (K) as the implementation of replaceWith?

[Avaq]

Have you considered naming it just replace? What moved you to replaceBy?

• I would confuse replaceWith and replaceBy all the time, and would have an easier time remembering how replace and replaceWith are different.
• It makes sense for me that replaceWith is a specialisation of replace.
• The name replace is closer to its name in Native JavaScript.
• It's shorter.

[davidchambers]

Excellent points. I completely agree. :)

In fact, I suggest we only provide this function (as S.replace). As you noted, one can easily ignore the array of captured values (using S.K, say). sanctuary-js/sanctuary-site#90 demonstrates this streamlined approach.

[Avaq]

I'm not sure how I feel about the exclusion of replaceWith altogether. I totally understand the reasoning, but since it's such a common use-case of replace, it might not be comfortable for users to have to make this little jump to use K with it every time. Then again, I would probably just define replaceWith in my own projects and be happy either way, so I don't feel particularly strongly about it.

[davidchambers]

I suggest we try replace for a few months to see how it feels. We can easily add replaceWith in the future.

[Avaq]

Sounds good to me.

[davidchambers]

Superseded by #693