Callback for from
#46
Conversation
Added a callback for `from`, which takes file as an argument. Allows the user to tailor the search string according to the filename.
adamreisnz
left a comment
adamreisnz
left a comment
There was a problem hiding this comment.
Thanks for the PR! Just a few minor changes.
| //Make replacements | ||
| from.forEach((item, i) => { | ||
|
|
||
| if (typeof item === 'function') { |
There was a problem hiding this comment.
Can you add a comment above this block to indicate if a function is given it's called with the file name?
package.json
Outdated
| { | ||
| "name": "replace-in-file", | ||
| "version": "3.2.0", | ||
| "version": "3.3.0", |
There was a problem hiding this comment.
You don't need to bump the package version, I'll do that when I release a new version
Additional comments to explain the callback forms. Reversed minor version bump.
|
Changes made as requested... |
README.md
Outdated
| As the `to` parameter is passed to the native [String replace method](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replace), you can also specify a callback. This callback provides as an extra argument the name of the file in which the replacement is being performed. The following example uses a callback to convert matching strings to lowercase and appends the file extension to it: | ||
|
|
||
| ```js | ||
| var path = require('path'); |
There was a problem hiding this comment.
Sorry missed this, could you change var to const please, as the project is using ES6 throughout.
There was a problem hiding this comment.
Actually maybe remove the path reference altogether, as I'd like to keep the examples as simple as possible, without external dependencies.
README.md
Outdated
| from: /SomePattern[A-Za-z-]+/g, | ||
| to: (match) => match.toLowerCase(), | ||
| from: /SomePattern([A-Za-z-]+)/g, | ||
| to: (match, p1, offset, string, file) => `${match.toLowerCase()}${path.extname(file)}`, |
There was a problem hiding this comment.
Let's revert this example back to how it was to keep it simple, and instead create a second example for the file parameter usage.
There was a problem hiding this comment.
Ok, will fix these later in the day!
There was a problem hiding this comment.
Changes made as requested!
|
On the issue of |
|
You could use rest parameters to get around that, e.g. (match, ...rest) => {
const file = rest.pop();
} |
Reverted to original `to` callback example. Added a separate example example for using the `file` argument.
|
Just from an API point of view I had another idea. Instead of adding an extra argument {
from: (file) => [`SomeStringThatDependsOn${file}`, ...]
to: (file) => [(match) => `SomethingElseThatDependsOn${file}`, ...]
}Just an idea 😄 ! |
3 participants
Added a callback for
from, which takes file as an argument. Allows the user to tailor the search string according to the filename.You may want to rename the tests of fix the language in the documentation.
Also fixed the documentation for the
tocallback.