From 5931ea62483e379c616bc60f7bb62c46b652b445 Mon Sep 17 00:00:00 2001 From: amrelnaggar Date: Wed, 19 Aug 2015 05:22:14 +0200 Subject: [PATCH 1/4] Add replace all button when doing find and replace --- src/htmlContent/findreplace-bar.html | 6 +++- src/nls/root/strings.js | 3 +- src/search/FindBar.js | 13 ++++++--- src/search/FindInFilesUI.js | 41 +++++++++++++++++++++++++--- src/search/FindReplace.js | 10 +++++-- src/search/SearchResultsView.js | 4 +-- src/styles/brackets.less | 7 +++-- 7 files changed, 67 insertions(+), 17 deletions(-) diff --git a/src/htmlContent/findreplace-bar.html b/src/htmlContent/findreplace-bar.html index 42311e46a3e..e2f8c2b4aac 100644 --- a/src/htmlContent/findreplace-bar.html +++ b/src/htmlContent/findreplace-bar.html @@ -17,7 +17,11 @@ {{^multifile}} --> + --> {{/replace}} diff --git a/src/nls/root/strings.js b/src/nls/root/strings.js index bb860d7ba3a..d42c684bc78 100644 --- a/src/nls/root/strings.js +++ b/src/nls/root/strings.js @@ -154,7 +154,8 @@ define({ "FIND_NO_RESULTS" : "No results", "FIND_QUERY_PLACEHOLDER" : "Find\u2026", "REPLACE_PLACEHOLDER" : "Replace with\u2026", - "BUTTON_REPLACE_ALL" : "Batch\u2026", + "BUTTON_REPLACE_ALL" : "Replace All\u2026", + "BUTTON_REPLACE_BATCH" : "Batch\u2026", "BUTTON_REPLACE_ALL_IN_FILES" : "Replace\u2026", "BUTTON_REPLACE" : "Replace", "BUTTON_NEXT" : "\u25B6", diff --git a/src/search/FindBar.js b/src/search/FindBar.js index 01fa0df3b97..08b5b5ebeaa 100644 --- a/src/search/FindBar.js +++ b/src/search/FindBar.js @@ -67,7 +67,8 @@ define(function (require, exports, module) { * Parameters are: * shiftKey - boolean, false for Find Next, true for Find Previous * - doReplace - when the user chooses to do a single replace. Use getReplaceText() to get the current replacement text. - * - doReplaceAll - when the user chooses to initiate a Replace All. Use getReplaceText() to get the current replacement text. + * - doReplaceBatch - when the user chooses to initiate a Replace All. Use getReplaceText() to get the current replacement text. + * - doReplaceAll - when the user chooses to perform a Replace All. Use getReplaceText() to get the current replacement text. *- close - when the find bar is closed * * @param {boolean=} options.multifile - true if this is a Find/Replace in Files (changes the behavior of Enter in @@ -239,7 +240,8 @@ define(function (require, exports, module) { var templateVars = _.clone(this._options); templateVars.Strings = Strings; - templateVars.replaceAllLabel = (templateVars.multifile ? Strings.BUTTON_REPLACE_ALL_IN_FILES : Strings.BUTTON_REPLACE_ALL); + templateVars.replaceBatchLabel = (templateVars.multifile ? Strings.BUTTON_REPLACE_ALL_IN_FILES : Strings.BUTTON_REPLACE_BATCH); + templateVars.replaceAllLabel = Strings.BUTTON_REPLACE_ALL; this._modalBar = new ModalBar(Mustache.render(_searchBarTemplate, templateVars), true); // 2nd arg = auto-close on Esc/blur @@ -313,7 +315,7 @@ define(function (require, exports, module) { self.trigger("doFind"); } } else { - self.trigger("doReplaceAll"); + self.trigger("doReplaceBatch"); } } else { // In the single file case, we just want to trigger a Find Next (or Find Previous @@ -341,6 +343,9 @@ define(function (require, exports, module) { .on("click", "#replace-yes", function (e) { self.trigger("doReplace"); }) + .on("click", "#replace-batch", function (e) { + self.trigger("doReplaceBatch"); + }) .on("click", "#replace-all", function (e) { self.trigger("doReplaceAll"); }) @@ -504,7 +509,7 @@ define(function (require, exports, module) { */ FindBar.prototype.enableReplace = function (enable) { if (this.isEnabled) { - this.$("#replace-yes, #replace-all").prop("disabled", !enable); + this.$("#replace-yes, #replace-batch, #replace-all").prop("disabled", !enable); } }; diff --git a/src/search/FindInFilesUI.js b/src/search/FindInFilesUI.js index f43c803bc01..f09fe580b61 100644 --- a/src/search/FindInFilesUI.js +++ b/src/search/FindInFilesUI.js @@ -108,6 +108,38 @@ define(function (require, exports, module) { }); } + /** + * Does a search in the given scope with the given filter. Replace the result list once the search is complete. + * @param {{query: string, caseSensitive: boolean, isRegexp: boolean}} queryInfo Query info object + * @param {?Entry} scope Project file/subfolder to search within; else searches whole project. + * @param {?string} filter A "compiled" filter as returned by FileFilters.compile(), or null for no filter + * @param {?string} replaceText If this is a replacement, the text to replace matches with. + * @param {?$.Promise} candidateFilesPromise If specified, a promise that should resolve with the same set of files that + * getCandidateFiles(scope) would return. + * @return {$.Promise} A promise that's resolved with the search results or rejected when the find competes. + */ + function searchAndReplaceResults(queryInfo, scope, filter, replaceText, candidateFilesPromise) { + return FindInFiles.doSearchInScope(queryInfo, scope, filter, replaceText, candidateFilesPromise) + .done(function (zeroFilesToken) { + // Done searching all files: replace all + if (FindInFiles.searchModel.hasResults()) { + _finishReplaceBatch(FindInFiles.searchModel); + + if (_findBar) { + _findBar.enable(true); + _findBar.focus(); + } + + } + + StatusBar.hideBusyIndicator(); + }) + .fail(function (err) { + console.log("replace all failed: ", err); + StatusBar.hideBusyIndicator(); + }); + } + /** * @private * Displays a non-modal embedded dialog above the code mirror editor that allows the user to do @@ -218,7 +250,7 @@ define(function (require, exports, module) { if (showReplace) { // We shouldn't get a "doReplace" in this case, since the Replace button // is hidden when we set options.multifile. - _findBar.on("doReplaceAll.FindInFiles", startReplace); + _findBar.on("doReplaceBatch.FindInFiles", startReplace); } var oldModalBarHeight = _findBar._modalBar.height(); @@ -257,7 +289,7 @@ define(function (require, exports, module) { * Finish a replace across files operation when the user clicks "Replace" on the results panel. * @param {SearchModel} model The model for the search associated with ths replace. */ - function _finishReplaceAll(model) { + function _finishReplaceBatch(model) { var replaceText = model.replaceText; if (replaceText === null) { return; @@ -412,8 +444,8 @@ define(function (require, exports, module) { var model = FindInFiles.searchModel; _resultsView = new SearchResultsView(model, "find-in-files-results", "find-in-files.results"); _resultsView - .on("replaceAll", function () { - _finishReplaceAll(model); + .on("replaceBatch", function () { + _finishReplaceBatch(model); }) .on("close", function () { FindInFiles.clearSearch(); @@ -451,6 +483,7 @@ define(function (require, exports, module) { // Public exports exports.searchAndShowResults = searchAndShowResults; + exports.searchAndReplaceResults = searchAndReplaceResults; // For unit testing exports._showFindBar = _showFindBar; diff --git a/src/search/FindReplace.js b/src/search/FindReplace.js index 178f4b5ac43..7b79fe084c5 100644 --- a/src/search/FindReplace.js +++ b/src/search/FindReplace.js @@ -680,7 +680,10 @@ define(function (require, exports, module) { state = getSearchState(cm), replaceText = findBar.getReplaceText(); - if (all) { + if(all===null){ + findBar.close(); + FindInFilesUI.searchAndReplaceResults(state.queryInfo, editor.document.file, null, replaceText); + } else if (all) { findBar.close(); // Delegate to Replace in Files. FindInFilesUI.searchAndShowResults(state.queryInfo, editor.document.file, null, replaceText); @@ -710,8 +713,11 @@ define(function (require, exports, module) { .on("doReplace.FindReplace", function (e) { doReplace(editor, false); }) - .on("doReplaceAll.FindReplace", function (e) { + .on("doReplaceBatch.FindReplace", function (e) { doReplace(editor, true); + }) + .on("doReplaceAll.FindReplace", function (e) { + doReplace(editor, null); }); } diff --git a/src/search/SearchResultsView.js b/src/search/SearchResultsView.js index e4ad4e126c9..9b5e7edfcdb 100644 --- a/src/search/SearchResultsView.js +++ b/src/search/SearchResultsView.js @@ -66,7 +66,7 @@ define(function (require, exports, module) { * @constructor * Handles the search results panel. * Dispatches the following events: - * replaceAll - when the "Replace" button is clicked. + * replaceBatch - when the "Replace" button is clicked. * close - when the panel is closed. * * @param {SearchModel} model The model that this view is showing. @@ -324,7 +324,7 @@ define(function (require, exports, module) { e.stopPropagation(); }) .on("click.searchResults", ".replace-checked", function (e) { - self.trigger("replaceAll"); + self.trigger("replaceBatch"); }); } }; diff --git a/src/styles/brackets.less b/src/styles/brackets.less index 6d5f092614f..8f64e53ff88 100644 --- a/src/styles/brackets.less +++ b/src/styles/brackets.less @@ -1568,18 +1568,19 @@ a, img { } // Make button pairs snug - #find-prev, #replace-yes, #find-case-sensitive { + #find-prev, #replace-yes, #replace-batch, #find-case-sensitive { border-right: none; margin-right: 0; } - #find-next, #replace-all, #find-regexp { + #find-next, #replace-batch, #replace-all, #find-regexp { border-top-left-radius: 0; border-bottom-left-radius: 0; margin-left: 0; } - #replace-all.solo { + #replace-batch.solo { border-left: none; margin-left: 0px; + border-right: 1px solid #b2b5b5; } // Make find field snug with options buttons From 80bf2a5ad23329c91563a75c96c26e3b623e3aab Mon Sep 17 00:00:00 2001 From: Amr El-Naggar Date: Mon, 28 Nov 2016 20:25:35 +0200 Subject: [PATCH 2/4] Update the id of batch replace button in tests --- test/spec/FindInFiles-test.js | 12 ++++++------ test/spec/FindReplace-test.js | 36 +++++++++++++++++------------------ 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/test/spec/FindInFiles-test.js b/test/spec/FindInFiles-test.js index ccfe912e6fc..702584535cf 100644 --- a/test/spec/FindInFiles-test.js +++ b/test/spec/FindInFiles-test.js @@ -1714,7 +1714,7 @@ define(function (require, exports, module) { if (fromKeyboard) { SpecRunnerUtils.simulateKeyEvent(KeyEvent.DOM_VK_RETURN, "keydown", $("#replace-with").get(0)); } else { - $("#replace-all").click(); + $("#replace-batch").click(); } }); } @@ -1738,7 +1738,7 @@ define(function (require, exports, module) { openSearchBar(null, true); runs(function () { expect($("#replace-yes").length).toBe(0); - expect($("#replace-all").length).toBe(1); + expect($("#replace-batch").length).toBe(1); }); }); @@ -1746,7 +1746,7 @@ define(function (require, exports, module) { openTestProjectCopy(defaultSourcePath); openSearchBar(null, true); runs(function () { - expect($("#replace-all").is(":disabled")).toBe(true); + expect($("#replace-batch").is(":disabled")).toBe(true); }); }); @@ -1755,7 +1755,7 @@ define(function (require, exports, module) { openSearchBar(null, true); runs(function () { $("#find-what").val("my query").trigger("input"); - expect($("#replace-all").is(":disabled")).toBe(false); + expect($("#replace-batch").is(":disabled")).toBe(false); }); }); @@ -1765,7 +1765,7 @@ define(function (require, exports, module) { runs(function () { $("#find-regexp").click(); $("#find-what").val("[invalid").trigger("input"); - expect($("#replace-all").is(":disabled")).toBe(true); + expect($("#replace-batch").is(":disabled")).toBe(true); }); }); @@ -1775,7 +1775,7 @@ define(function (require, exports, module) { runs(function () { $("#find-regexp").click(); $("#find-what").val("[valid]").trigger("input"); - expect($("#replace-all").is(":disabled")).toBe(false); + expect($("#replace-batch").is(":disabled")).toBe(false); }); }); diff --git a/test/spec/FindReplace-test.js b/test/spec/FindReplace-test.js index 7f112448c8c..982e041233a 100644 --- a/test/spec/FindReplace-test.js +++ b/test/spec/FindReplace-test.js @@ -1454,8 +1454,8 @@ define(function (require, exports, module) { expectSelection({start: {line: 1, ch: 17}, end: {line: 1, ch: 17 + searchText.length}}); expect(myEditor.getSelectedText()).toBe(searchText); - expect(tw$("#replace-all").is(":enabled")).toBe(true); - tw$("#replace-all").click(); + expect(tw$("#replace-batch").is(":enabled")).toBe(true); + tw$("#replace-batch").click(); }); waitsFor(function () { @@ -1493,8 +1493,8 @@ define(function (require, exports, module) { expectSelection({start: {line: 1, ch: 17}, end: {line: 1, ch: 17 + searchText.length}}); expect(myEditor.getSelectedText()).toBe(searchText); - expect(tw$("#replace-all").is(":enabled")).toBe(true); - tw$("#replace-all").click(); + expect(tw$("#replace-batch").is(":enabled")).toBe(true); + tw$("#replace-batch").click(); }); waitsFor(function () { @@ -1519,8 +1519,8 @@ define(function (require, exports, module) { expectSelection({start: {line: 1, ch: 17}, end: {line: 1, ch: 17 + searchText.length}}); expect(myEditor.getSelectedText()).toBe(searchText); - expect(tw$("#replace-all").is(":enabled")).toBe(true); - tw$("#replace-all").click(); + expect(tw$("#replace-batch").is(":enabled")).toBe(true); + tw$("#replace-batch").click(); }); waitsFor(function () { @@ -1572,8 +1572,8 @@ define(function (require, exports, module) { expectSelection(expectedMatch); expect(/foo/i.test(myEditor.getSelectedText())).toBe(true); - expect(tw$("#replace-all").is(":enabled")).toBe(true); - tw$("#replace-all").click(); + expect(tw$("#replace-batch").is(":enabled")).toBe(true); + tw$("#replace-batch").click(); }); waitsFor(function () { @@ -1612,8 +1612,8 @@ define(function (require, exports, module) { expectSelection(expectedMatch); expect(/foo/i.test(myEditor.getSelectedText())).toBe(true); - expect(tw$("#replace-all").is(":enabled")).toBe(true); - tw$("#replace-all").click(); + expect(tw$("#replace-batch").is(":enabled")).toBe(true); + tw$("#replace-batch").click(); }); waitsFor(function () { @@ -1652,8 +1652,8 @@ define(function (require, exports, module) { expectSelection(expectedMatch); expect(/foo/i.test(myEditor.getSelectedText())).toBe(true); - expect(tw$("#replace-all").is(":enabled")).toBe(true); - tw$("#replace-all").click(); + expect(tw$("#replace-batch").is(":enabled")).toBe(true); + tw$("#replace-batch").click(); }); waitsFor(function () { @@ -1692,8 +1692,8 @@ define(function (require, exports, module) { expectSelection(expectedMatch); expect(/foo/i.test(myEditor.getSelectedText())).toBe(true); - expect(tw$("#replace-all").is(":enabled")).toBe(true); - tw$("#replace-all").click(); + expect(tw$("#replace-batch").is(":enabled")).toBe(true); + tw$("#replace-batch").click(); }); waitsFor(function () { @@ -1732,8 +1732,8 @@ define(function (require, exports, module) { expectSelection(expectedMatch); expect(/foo/i.test(myEditor.getSelectedText())).toBe(true); - expect(tw$("#replace-all").is(":enabled")).toBe(true); - tw$("#replace-all").click(); + expect(tw$("#replace-batch").is(":enabled")).toBe(true); + tw$("#replace-batch").click(); }); waitsFor(function () { @@ -1772,8 +1772,8 @@ define(function (require, exports, module) { expectSelection(expectedMatch); expect(/foo/i.test(myEditor.getSelectedText())).toBe(true); - expect(tw$("#replace-all").is(":enabled")).toBe(true); - tw$("#replace-all").click(); + expect(tw$("#replace-batch").is(":enabled")).toBe(true); + tw$("#replace-batch").click(); }); waitsFor(function () { From ba8baf06fc4ee01d2b4f8008a34132fa560129ee Mon Sep 17 00:00:00 2001 From: Amr El-Naggar Date: Mon, 28 Nov 2016 20:29:29 +0200 Subject: [PATCH 3/4] Add test for replace all button --- test/spec/FindReplace-test.js | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/test/spec/FindReplace-test.js b/test/spec/FindReplace-test.js index 982e041233a..50a7669065c 100644 --- a/test/spec/FindReplace-test.js +++ b/test/spec/FindReplace-test.js @@ -1443,7 +1443,38 @@ define(function (require, exports, module) { twFindInFiles._replaceDone = false; }); - it("should find and replace all", function () { + it("should find and replace all using replace all button", function () { + var searchText = "require", + replaceText = "brackets.getModule"; + runs(function () { + twCommandManager.execute(Commands.CMD_REPLACE); + enterSearchText(searchText); + enterReplaceText(replaceText); + + expectSelection({start: {line: 1, ch: 17}, end: {line: 1, ch: 17 + searchText.length}}); + expect(myEditor.getSelectedText()).toBe(searchText); + + expect(tw$("#replace-all").is(":enabled")).toBe(true); + tw$("#replace-all").click(); + }); + + waitsFor(function () { + return twFindInFiles._replaceDone; + }, "replace finished"); + + runs(function () { + // Note: LINE_FIRST_REQUIRE and CH_REQUIRE_START refer to first call to "require", + // but not first instance of "require" in text + expectTextAtPositions(replaceText, [ + {line: 1, ch: 17}, + {line: LINE_FIRST_REQUIRE, ch: CH_REQUIRE_START}, + {line: LINE_FIRST_REQUIRE + 1, ch: CH_REQUIRE_START}, + {line: LINE_FIRST_REQUIRE + 2, ch: CH_REQUIRE_START} + ]); + }); + }); + + it("should find and replace all using batch replace button", function () { var searchText = "require", replaceText = "brackets.getModule"; runs(function () { From 0658a379442e06365c138f49001f42eb7b7ff09e Mon Sep 17 00:00:00 2001 From: Amr El-Naggar Date: Fri, 16 Dec 2016 13:52:54 +0200 Subject: [PATCH 4/4] Resolve conflicts --- src/search/FindBar.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/search/FindBar.js b/src/search/FindBar.js index 36b16f96f7a..317a0833b57 100644 --- a/src/search/FindBar.js +++ b/src/search/FindBar.js @@ -325,12 +325,8 @@ define(function (require, exports, module) { self.trigger("doFind"); } } else { -<<<<<<< HEAD - self.trigger("doReplaceBatch"); -======= HealthLogger.searchDone(HealthLogger.SEARCH_REPLACE_ALL); - self.trigger("doReplaceAll"); ->>>>>>> upstream/master + self.trigger("doReplaceBatch"); } } else { // In the single file case, we just want to trigger a Find Next (or Find Previous