Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Fix #5770: Replace with $1 and so on works in Replace All mode, too #5771

Closed
wants to merge 2 commits into from

Conversation

marcelgerber
Copy link
Contributor

This fixes #5770, where regexp replace works incorrectly while in Replace All mode.

My question: Is there anything better than a do-while loop with an if to break (there must be anything better ;) )

@njx

@njx
Copy link

njx commented Oct 30, 2013

See my version in #5772. I slightly prefer maintaining the original while() and just living with the redundancy of doing the assignment twice. (I would prefer even more being able to just do while (matchResult = cursor.findNext()) but JSLint doesn't like that.)

@@ -468,7 +474,7 @@ define(function (require, exports, module) {
.reverse()
.forEach(function (checkedRow) {
var match = results[$(checkedRow).data("match")],
rw = typeof replaceWhat === "string" ? replaceWith : replaceWith.replace(/\$(\d)/g, replaceFunction);
rw = typeof replaceWhat === "string" ? replaceWith : replaceWith.replace(/\$(\d)/g, function (w, i) { return match.match[i]; });
Copy link

Choose a reason for hiding this comment

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

I don't like the shadowing of "match" here--I chose a different name for the outer variable for this reason.

@marcelgerber
Copy link
Contributor Author

Closed as #5772 (the better one) was merged.

@marcelgerber marcelgerber deleted the fix-5770 branch October 31, 2013 07:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regexp subexpression replace doesn't work in Replace All
2 participants