From bda6ec714d0357c8e5f644fbd6d089d383751b98 Mon Sep 17 00:00:00 2001 From: Marcel Gerber Date: Sun, 10 Nov 2013 19:50:06 +0100 Subject: [PATCH 1/4] handle $& --- src/search/FindReplace.js | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/search/FindReplace.js b/src/search/FindReplace.js index 3b900de4850..fbdc9d7702c 100644 --- a/src/search/FindReplace.js +++ b/src/search/FindReplace.js @@ -118,15 +118,18 @@ define(function (require, exports, module) { } function parseDollars(replaceWith, match) { - replaceWith = replaceWith.replace(/(\$+)(\d{1,2})/g, function (whole, dollars, index) { + replaceWith = replaceWith.replace(/(\$+)(\d{1,2}|&)/g, function (whole, dollars, index) { var parsedIndex = parseInt(index, 10); - if (dollars.length % 2 === 1 && parsedIndex !== 0) { - return dollars.substr(1) + (match[parsedIndex] || ""); - } else { - return whole; + if (dollars.length % 2 === 1) { // check if dollar signs escape themselves (for example $$1, $$$$&) + if (index === "&") { // handle $& + return dollars.substr(1) + (match[0] || ""); + } else if (parsedIndex !== 0) { // handle $n or $nn, don't handle $0 or $00 + return dollars.substr(1) + (match[parsedIndex] || ""); + } } + return whole; }); - replaceWith = replaceWith.replace(/\$\$/g, "$"); + replaceWith = replaceWith.replace(/\$\$/g, "$"); // replace escaped dollar signs (for example $$) with single ones return replaceWith; } From e587d05bd5b38cc99dbdc0610f909aeaa464aa44 Mon Sep 17 00:00:00 2001 From: Marcel Gerber Date: Sun, 10 Nov 2013 20:05:19 +0100 Subject: [PATCH 2/4] Adding unit tests for "Replace $&" --- test/spec/FindReplace-test.js | 74 +++++++++++++++++++++++++++++++++-- 1 file changed, 71 insertions(+), 3 deletions(-) diff --git a/test/spec/FindReplace-test.js b/test/spec/FindReplace-test.js index 55800f06347..5958de7e4b5 100644 --- a/test/spec/FindReplace-test.js +++ b/test/spec/FindReplace-test.js @@ -942,6 +942,37 @@ define(function (require, exports, module) { expect(/Foo\$modules/i.test(myEditor.getSelectedText())).toBe(true); }); }); + + it("should find a regexp and replace it with $& (whole match)", function () { + runs(function () { + twCommandManager.execute(Commands.EDIT_REPLACE); + enterSearchText("/(modules)\\/(\\w+)/"); + pressEnter(); + }); + + waitsForSearchBarReopen(); + + runs(function () { + enterSearchText("_$&-$2$$&"); + pressEnter(); + }); + + waitsForSearchBarReopen(); + + runs(function () { + var expectedMatch = {start: {line: LINE_FIRST_REQUIRE, ch: 23}, end: {line: LINE_FIRST_REQUIRE, ch: 34}}; + + expectSelection(expectedMatch); + expect(/foo/i.test(myEditor.getSelectedText())).toBe(true); + + expect(tw$("#replace-yes").is(":visible")).toBe(true); + tw$("#replace-yes").click(); + tw$("#replace-stop").click(); + + myEditor.setSelection({line: LINE_FIRST_REQUIRE, ch: 23}, {line: LINE_FIRST_REQUIRE, ch: 41}); + expect(/_modules\/Foo-Foo\$&/i.test(myEditor.getSelectedText())).toBe(true); + }); + }); }); describe("Search -> Replace All", function () { @@ -1019,7 +1050,7 @@ define(function (require, exports, module) { }); }); - it("should find a regexp and replace it with $nn (n has two digits)", function () { + it("should find all regexps and replace them with $nn (n has two digits)", function () { runs(function () { twCommandManager.execute(Commands.EDIT_REPLACE); enterSearchText("/()()()()()()()()()()(modules)\\/()()()(\\w+)/"); @@ -1056,7 +1087,7 @@ define(function (require, exports, module) { }); }); - it("should find a regexp and replace it with $$n (not a subexpression, escaped dollar)", function () { + it("should find all regexps and replace them with $$n (not a subexpression, escaped dollar)", function () { runs(function () { twCommandManager.execute(Commands.EDIT_REPLACE); enterSearchText("/(modules)\\/(\\w+)/"); @@ -1093,7 +1124,7 @@ define(function (require, exports, module) { }); }); - it("should find a regexp and replace it with $$$n (correct subexpression)", function () { + it("should find all regexps and replace them with $$$n (correct subexpression)", function () { runs(function () { twCommandManager.execute(Commands.EDIT_REPLACE); enterSearchText("/(modules)\\/(\\w+)/"); @@ -1129,6 +1160,43 @@ define(function (require, exports, module) { expect(/Baz\$modules/i.test(myEditor.getSelectedText())).toBe(true); }); }); + + it("should find all regexps and replace them with $& (whole match)", function () { + runs(function () { + twCommandManager.execute(Commands.EDIT_REPLACE); + enterSearchText("/(modules)\\/(\\w+)/"); + pressEnter(); + }); + + waitsForSearchBarReopen(); + + runs(function () { + enterSearchText("_$&-$2$$&"); + pressEnter(); + }); + + waitsForSearchBarReopen(); + + runs(function () { + var expectedMatch = {start: {line: LINE_FIRST_REQUIRE, ch: 23}, end: {line: LINE_FIRST_REQUIRE, ch: 34}}; + + expectSelection(expectedMatch); + expect(/foo/i.test(myEditor.getSelectedText())).toBe(true); + + expect(tw$("#replace-all").is(":visible")).toBe(true); + tw$("#replace-all").click(); + tw$(".replace-checked").click(); + + myEditor.setSelection({line: LINE_FIRST_REQUIRE, ch: 23}, {line: LINE_FIRST_REQUIRE, ch: 41}); + expect(/_modules\/Foo-Foo\$&/i.test(myEditor.getSelectedText())).toBe(true); + + myEditor.setSelection({line: LINE_FIRST_REQUIRE + 1, ch: 23}, {line: LINE_FIRST_REQUIRE + 1, ch: 41}); + expect(/_modules\/Bar-Bar\$&/i.test(myEditor.getSelectedText())).toBe(true); + + myEditor.setSelection({line: LINE_FIRST_REQUIRE + 2, ch: 23}, {line: LINE_FIRST_REQUIRE + 2, ch: 41}); + expect(/_modules\/Baz-Baz\$&/i.test(myEditor.getSelectedText())).toBe(true); + }); + }); }); }); From 50a790d116419466adc2643fee5a17006ec9e628 Mon Sep 17 00:00:00 2001 From: Marcel Gerber Date: Tue, 28 Jan 2014 20:11:50 +0100 Subject: [PATCH 3/4] Add comment to describe why we need parseDollars() --- src/search/FindReplace.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/search/FindReplace.js b/src/search/FindReplace.js index 8e946b37b5e..269f0184f17 100644 --- a/src/search/FindReplace.js +++ b/src/search/FindReplace.js @@ -140,6 +140,8 @@ define(function (require, exports, module) { } } + // NOTE: we can't just use the ordinary replace() function here because the string has been + // extracted from the original text and so might be missing some context that the regexp matched. function parseDollars(replaceWith, match) { replaceWith = replaceWith.replace(/(\$+)(\d{1,2}|&)/g, function (whole, dollars, index) { var parsedIndex = parseInt(index, 10); From 4f1ac9f02f3a4866dfff9ce6c3965b2cac8e50b3 Mon Sep 17 00:00:00 2001 From: Marcel Gerber Date: Thu, 30 Jan 2014 21:33:34 +0100 Subject: [PATCH 4/4] Fix unit tests --- test/spec/FindReplace-test.js | 37 ++++++++--------------------------- 1 file changed, 8 insertions(+), 29 deletions(-) diff --git a/test/spec/FindReplace-test.js b/test/spec/FindReplace-test.js index f9d6b17eb14..6b579b96110 100644 --- a/test/spec/FindReplace-test.js +++ b/test/spec/FindReplace-test.js @@ -933,28 +933,17 @@ define(function (require, exports, module) { it("should find a regexp and replace it with $& (whole match)", function () { runs(function () { twCommandManager.execute(Commands.EDIT_REPLACE); - enterSearchText("/(modules)\\/(\\w+)/"); - pressEnter(); - }); - - waitsForSearchBarReopen(); - - runs(function () { - enterSearchText("_$&-$2$$&"); - pressEnter(); - }); - - waitsForSearchBarReopen(); + toggleRegexp(true); + enterSearchText("(modules)\\/(\\w+)"); + enterReplaceText("_$&-$2$$&"); - runs(function () { var expectedMatch = {start: {line: LINE_FIRST_REQUIRE, ch: 23}, end: {line: LINE_FIRST_REQUIRE, ch: 34}}; expectSelection(expectedMatch); expect(/foo/i.test(myEditor.getSelectedText())).toBe(true); - expect(tw$("#replace-yes").is(":visible")).toBe(true); + expect(tw$("#replace-yes").is(":enabled")).toBe(true); tw$("#replace-yes").click(); - tw$("#replace-stop").click(); myEditor.setSelection({line: LINE_FIRST_REQUIRE, ch: 23}, {line: LINE_FIRST_REQUIRE, ch: 41}); expect(/_modules\/Foo-Foo\$&/i.test(myEditor.getSelectedText())).toBe(true); @@ -1102,26 +1091,16 @@ define(function (require, exports, module) { it("should find all regexps and replace them with $& (whole match)", function () { runs(function () { twCommandManager.execute(Commands.EDIT_REPLACE); - enterSearchText("/(modules)\\/(\\w+)/"); - pressEnter(); - }); - - waitsForSearchBarReopen(); - - runs(function () { - enterSearchText("_$&-$2$$&"); - pressEnter(); - }); - - waitsForSearchBarReopen(); + toggleRegexp(true); + enterSearchText("(modules)\\/(\\w+)"); + enterReplaceText("_$&-$2$$&"); - runs(function () { var expectedMatch = {start: {line: LINE_FIRST_REQUIRE, ch: 23}, end: {line: LINE_FIRST_REQUIRE, ch: 34}}; expectSelection(expectedMatch); expect(/foo/i.test(myEditor.getSelectedText())).toBe(true); - expect(tw$("#replace-all").is(":visible")).toBe(true); + expect(tw$("#replace-all").is(":enabled")).toBe(true); tw$("#replace-all").click(); tw$(".replace-checked").click();