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: add S.replaceWith and S.replaceBy #686

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ jobs:
nvm exec $1 npm test
}
case $CIRCLE_NODE_INDEX in
0) test_with_version 8 ;;
1) test_with_version 10 ;;
2) npm install && npm test ;;
0) test_with_version 10 ;;
1) npm install && npm test ;;
2) test_with_version 14 ;;
esac
87 changes: 82 additions & 5 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -1731,12 +1731,12 @@
//. Curries the given ternary function.
//.
//. ```javascript
//. > const replaceString = S.curry3 ((what, replacement, string) =>
//. . string.replace (what, replacement)
//. . )
//. > const defineProperty = S.curry3 (Object.defineProperty)
//.
//. > const o = defineProperty ({}) ('x') ({value: 42, writable: false})
//.
//. > replaceString ('banana') ('orange') ('banana icecream')
//. 'orange icecream'
//. > o.x
//. 42
//. ```
function curry3(f) {
return function(x) {
Expand Down Expand Up @@ -4548,6 +4548,83 @@
impl: matchAll
};

//# replaceWith :: String -> RegExp -> String -> String
//.
//. Replaces occurrences of the given pattern with the given replacement
//. within the given string. Replaces all occurrences of the pattern if
//. its `g` flag is set; just the first occurrence otherwise.
//.
//. See also [`replaceBy`](#replaceBy).
//.
//. ```javascript
//. > S.replaceWith ('x') (/o/) ('foo')
//. 'fxo'
//.
//. > S.replaceWith ('x') (/o/g) ('foo')
//. 'fxx'
//. ```
function replaceWith(replacement) {
return function(pattern) {
return function(text) {
return text.replace (pattern, function() { return replacement; });
};
};
}
Comment on lines +4566 to +4572
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

_.replaceWith = {
consts: {},
types: [$.String, $.RegExp, $.String, $.String],
impl: replaceWith
};

//# replaceBy :: (Array (Maybe String) -> String) -> RegExp -> String -> String
//.
//. Replaces occurrences of the given pattern within the given string
//. in accordance with the given replacement function, which receives an
//. array of captured values. Replaces all occurrences of the pattern if
//. its `g` flag is set; just the first occurrence otherwise.
//.
//. See also [`replaceWith`](#replaceWith).
//.
//. ```javascript
//. > S.replaceBy (([$1]) => S.maybe ('') (S.toUpper) ($1)) (/(\w)/) ('foo')
//. 'Foo'
//.
//. > S.replaceBy (([$1]) => S.maybe ('') (S.toUpper) ($1)) (/(\w)/g) ('foo')
//. 'FOO'
//.
//. > S.replaceBy (S.show) (/(foo)(bar)?/) ('<>')
//. '<>'
//.
//. > S.replaceBy (S.show) (/(foo)(bar)?/) ('<foo>')
//. '<[Just ("foo"), Nothing]>'
//.
//. > S.replaceBy (S.show) (/(foo)(bar)?/) ('<foobar>')
//. '<[Just ("foo"), Just ("bar")]>'
//. ```
function replaceBy(substitute) {
Copy link
Member

@Avaq Avaq Aug 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me.

return function(pattern) {
return function(text) {
return text.replace (pattern, function() {
var groups = [];
var group, idx = 1;
// eslint-disable-next-line no-plusplus
while (typeof (group = arguments[idx++]) !== 'number') {
groups.push (group == null ? Nothing : Just (group));
}
return substitute (groups);
});
};
};
}
_.replaceBy = {
consts: {},
types: [$.Fn ($.Array ($.Maybe ($.String))) ($.String),
$.RegExp,
$.String,
$.String],
impl: replaceBy
};

//. ### String

//# toUpper :: String -> String
Expand Down
2 changes: 2 additions & 0 deletions test/.eslintrc.json
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
{
"root": true,
"extends": ["../node_modules/sanctuary-style/eslint-es6.json"],
"parserOptions": {"ecmaVersion": 2018},
"env": {"node": true},
"globals": {"suite": false, "test": false},
"rules": {
"comma-dangle": ["error", {"arrays": "always-multiline", "objects": "always-multiline", "functions": "never"}],
"max-len": ["off"]
}
}
19 changes: 19 additions & 0 deletions test/replaceBy.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
'use strict';

const S = require ('..');

const eq = require ('./internal/eq');


test ('replaceBy', () => {

eq (S.show (S.replaceBy)) ('replaceBy :: (Array (Maybe String) -> String) -> RegExp -> String -> String');

eq (S.replaceBy (([$1]) => S.maybe ('') (S.toUpper) ($1)) (/(\w)/) ('foo')) ('Foo');
eq (S.replaceBy (([$1]) => S.maybe ('') (S.toUpper) ($1)) (/(\w)/g) ('foo')) ('FOO');
eq (S.replaceBy (S.show) (/(foo)(bar)?/) ('<>')) ('<>');
eq (S.replaceBy (S.show) (/(foo)(bar)?/) ('<foo>')) ('<[Just ("foo"), Nothing]>');
eq (S.replaceBy (S.show) (/(foo)(bar)?/) ('<foobar>')) ('<[Just ("foo"), Just ("bar")]>');
eq (S.replaceBy (S.show) (/@(?<username>[-\w]+)/) ('@sanctuary-js')) ('[Just ("sanctuary-js")]');

});
21 changes: 21 additions & 0 deletions test/replaceWith.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
'use strict';

const S = require ('..');

const eq = require ('./internal/eq');


test ('replaceWith', () => {

eq (S.show (S.replaceWith)) ('replaceWith :: String -> RegExp -> String -> String');

eq (S.replaceWith ('x') (/o/) ('<bar>')) ('<bar>');
eq (S.replaceWith ('x') (/o/) ('<foo>')) ('<fxo>');
eq (S.replaceWith ('x') (/o/g) ('<bar>')) ('<bar>');
eq (S.replaceWith ('x') (/o/g) ('<foo>')) ('<fxx>');
eq (S.replaceWith ('$1') (/(o)/) ('<bar>')) ('<bar>');
eq (S.replaceWith ('$1') (/(o)/) ('<foo>')) ('<f$1o>');
eq (S.replaceWith ('$1') (/(o)/g) ('<bar>')) ('<bar>');
eq (S.replaceWith ('$1') (/(o)/g) ('<foo>')) ('<f$1$1>');

});