diff --git a/src/extensions/default/DarkTheme/main.less b/src/extensions/default/DarkTheme/main.less index f8c2a8fe38..5f705a5a10 100644 --- a/src/extensions/default/DarkTheme/main.less +++ b/src/extensions/default/DarkTheme/main.less @@ -93,7 +93,7 @@ color: @foreground !important; } -.CodeMirror-matchingtag { +.CodeMirror-matchingtag:not(.CodeMirror-searching) { /* Ensure visibility against gray inline editor background */ background-color: @matching-tags; --border-height: calc((var(--editor-line-height) *1em - 1em) / 2); diff --git a/src/extensions/default/LightTheme/main.less b/src/extensions/default/LightTheme/main.less index 04a970563f..19ce7d7c66 100644 --- a/src/extensions/default/LightTheme/main.less +++ b/src/extensions/default/LightTheme/main.less @@ -24,7 +24,7 @@ @matching-bracket: #cfead6; -.CodeMirror-matchingtag { +.CodeMirror-matchingtag:not(.CodeMirror-searching) { background: @matching-bracket; --border-height: calc((var(--editor-line-height) *1em - 1em) / 2); border-top: var(--border-height) solid @matching-bracket; diff --git a/src/search/FindBar.js b/src/search/FindBar.js index b0502c3013..f7fac40009 100644 --- a/src/search/FindBar.js +++ b/src/search/FindBar.js @@ -53,8 +53,9 @@ define(function (require, exports, module) { let intervalId = 0, lastQueriedText = "", lastTypedText = "", - lastTypedTextWasRegexp = false; - const MAX_HISTORY_RESULTS = 50; + lastTypedTextWasRegexp = false, + lastClosedQuery = null; + const MAX_HISTORY_RESULTS = 25; const PREF_MAX_HISTORY = "maxSearchHistory"; const INSTANT_SEARCH_INTERVAL_MS = 50; @@ -631,6 +632,12 @@ define(function (require, exports, module) { FindBar.prototype.close = function (suppressAnimation) { lastQueriedText = ""; if (this._modalBar) { + lastClosedQuery = { + query: this.$("#find-what").val() || "", + replaceText: this.getReplaceText(), + isCaseSensitive: this.$("#find-case-sensitive").is(".active"), + isRegexp: this.$("#find-regexp").is(".active") + }; this._addElementToSearchHistory($("#find-what").val(), $("#fif-filter-input").val()); // 1st arg = restore scroll pos; 2nd arg = no animation, since getting replaced immediately this._modalBar.close(true, !suppressAnimation); @@ -656,7 +663,9 @@ define(function (require, exports, module) { * @return {{query: string, caseSensitive: boolean, isRegexp: boolean}} */ FindBar.prototype.getQueryInfo = function (usePlatformLineEndings = true) { - let query = this.$("#find-what").val() || ""; + const $findWhat = this.$("#find-what"); + const findTextArea = $findWhat[0]; + let query = $findWhat.val() || ""; const lineEndings = FileUtils.sniffLineEndings(query); if (usePlatformLineEndings && lineEndings === FileUtils.LINE_ENDINGS_LF && brackets.platform === "win") { query = query.replace(/\n/g, "\r\n"); @@ -664,7 +673,8 @@ define(function (require, exports, module) { return { query: query, isCaseSensitive: this.$("#find-case-sensitive").is(".active"), - isRegexp: this.$("#find-regexp").is(".active") + isRegexp: this.$("#find-regexp").is(".active"), + isQueryTextSelected: findTextArea.selectionStart !== findTextArea.selectionEnd }; }; @@ -851,7 +861,7 @@ define(function (require, exports, module) { * @private * @static * @param {?FindBar} currentFindBar - The currently open Find Bar, if any. - * @param {?Editor} activeEditor - The active editor, if any. + * @param {?Editor} editor - The active editor, if any. * @return {{query: string, replaceText: string}} An object containing the query and replacement text * to prepopulate the Find Bar. */ @@ -872,11 +882,31 @@ define(function (require, exports, module) { return !bar.isClosed(); } ); - - if (openedFindBar) { + // this happens when the find in files bar is opened and we are trying to open single file search or + // vice versa. we need to detect the other findbar and determine what is the search term to use + + //debugger + const currentQueryInfo = openedFindBar && openedFindBar.getQueryInfo(); + if(!openedFindBar && selection){ + // when no findbar is open, the selected text always takes precedence in both single and multi file + query = selection; + } else if(openedFindBar && selection && currentQueryInfo && !currentQueryInfo.isRegexp && currentQueryInfo.isQueryTextSelected) { + // we are switching between single<>multi file search without the user editing the search text in between + // while there is an active selection, the selection takes precedence. + query = selection; + replaceText = openedFindBar.getReplaceText(); + } else if (openedFindBar) { + // there is no selection and we are switching between single<>multi file search, copy the + // current query from the open findbar as is query = openedFindBar.getQueryInfo().query; replaceText = openedFindBar.getReplaceText(); + } else if (lastClosedQuery) { + // these is no open find bar currently and there is no selection, but there is a last saved query, so + // load the last query. this happenes on all freash search cases apart from the very first time + query = lastClosedQuery.query; + replaceText = lastClosedQuery.replaceText; } else if (editor) { + // the very first query after app start, nothing to restore. query = (!lastTypedTextWasRegexp && selection) || lastQueriedText || lastTypedText; } } diff --git a/src/search/FindReplace.js b/src/search/FindReplace.js index c10d52fe7c..cc79b14b0e 100644 --- a/src/search/FindReplace.js +++ b/src/search/FindReplace.js @@ -400,6 +400,45 @@ define(function (require, exports, module) { } } + function _markCurrentPos(editor) { + var cm = editor._codeMirror; + const pos = editor.getCursorPos(false, "start"); + cm.operation(function () { + var state = getSearchState(cm); + clearCurrentMatchHighlight(cm, state); + + let curIndex = editor.indexFromPos(pos); + curIndex = curIndex >= 1 ? curIndex-- : 0; + let thisMatch = _getNextMatch(editor, false, editor.posFromIndex(curIndex)); + if (thisMatch) { + let previousMatch = _getNextMatch(editor, true, thisMatch.start); + if(previousMatch){ + const start = editor.indexFromPos(previousMatch.start), end = editor.indexFromPos(previousMatch.end); + if(curIndex >= start && curIndex <= end) { + thisMatch = previousMatch; + } + } + // Update match index indicators - only possible if we have resultSet saved (missing if FIND_MAX_FILE_SIZE threshold hit) + if (state.resultSet.length) { + _updateFindBarWithMatchInfo(state, + {from: thisMatch.start, to: thisMatch.end}, false); + // Update current-tickmark indicator - only if highlighting enabled (disabled if FIND_HIGHLIGHT_MAX threshold hit) + if (state.marked.length) { + ScrollTrackMarkers.markCurrent(state.matchIndex); // _updateFindBarWithMatchInfo() has updated this index + } + } + + // Only mark text with "current match" highlight if search bar still open + if (findBar && !findBar.isClosed()) { + // If highlighting disabled, apply both match AND current-match styles for correct appearance + var curentMatchClassName = state.marked.length ? "searching-current-match" : "CodeMirror-searching searching-current-match"; + state.markedCurrent = cm.markText(thisMatch.start, thisMatch.end, + { className: curentMatchClassName, startStyle: "searching-first", endStyle: "searching-last" }); + } + } + }); + } + /** * Selects the next match (or prev match, if searchBackwards==true) starting from either the current position * (if pos unspecified) or the given position (if pos specified explicitly). The starting position @@ -574,11 +613,13 @@ define(function (require, exports, module) { setQueryInfo(state, findBar.getQueryInfo(false)); updateResultSet(editor); - if (state.parsedQuery) { + if(initial){ + _markCurrentPos(editor); + } else if (state.parsedQuery && !initial) { // 3rd arg: prefer to avoid scrolling if result is anywhere within view, since in this case user // is in the middle of typing, not navigating explicitly; viewport jumping would be distracting. findNext(editor, false, true, state.searchStartPos); - } else if (!initial) { + } else { // Blank or invalid query: just jump back to initial pos editor._codeMirror.setCursor(state.searchStartPos); } diff --git a/src/styles/brackets_codemirror_override.less b/src/styles/brackets_codemirror_override.less index 4c6c84680b..915923ce78 100644 --- a/src/styles/brackets_codemirror_override.less +++ b/src/styles/brackets_codemirror_override.less @@ -271,7 +271,7 @@ div.CodeMirror-cursors { } } - .CodeMirror-matchingtag { + .CodeMirror-matchingtag:not(.CodeMirror-searching) { background: @matching-bracket; } } diff --git a/test/spec/FindInFiles-integ-test.js b/test/spec/FindInFiles-integ-test.js index 2228139b0b..7e7547dfbe 100644 --- a/test/spec/FindInFiles-integ-test.js +++ b/test/spec/FindInFiles-integ-test.js @@ -49,7 +49,8 @@ define(function (require, exports, module) { ProjectManager, SearchResultsView, testWindow, - $; + $, + __PR; beforeAll(async function () { await SpecRunnerUtils.createTempDirectory(); @@ -71,6 +72,7 @@ define(function (require, exports, module) { SearchResultsView = testWindow.brackets.test.SearchResultsView; $ = testWindow.$; PreferencesManager = testWindow.brackets.test.PreferencesManager; + __PR = testWindow.__PR; PreferencesManager.set("findInFiles.nodeSearch", false); PreferencesManager.set("findInFiles.instantSearch", false); PreferencesManager.set("maxSearchHistory", 5); @@ -89,6 +91,8 @@ define(function (require, exports, module) { $ = null; testWindow = null; PreferencesManager = null; + __PR = null; + await SpecRunnerUtils.closeTestWindow(); await SpecRunnerUtils.removeTempDirectory(); }, 30000); @@ -125,6 +129,10 @@ define(function (require, exports, module) { }); } + function getSearchField() { + return $("#find-what"); + } + async function closeSearchBar() { FindInFilesUI._closeFindBar(); await waitForSearchBarClose(); @@ -136,6 +144,12 @@ define(function (require, exports, module) { }, "indexing complete", 20000); var $searchField = $("#find-what"); FindInFiles._searchDone = false; + // if the search str is same as previous string, fif will not do any search as its already loaded + $searchField.val("another_str_to_trigger_fif").trigger("input"); + await awaitsFor(function () { + return FindInFiles._searchDone; + }, "Find in Files done"); + FindInFiles._searchDone = false; $searchField.val(searchString).trigger("input"); await awaitsFor(function () { return FindInFiles._searchDone; @@ -549,16 +563,44 @@ define(function (require, exports, module) { expect(Object.keys(FindInFiles.searchModel.results).length).not.toBe(0); await closeSearchBar(); - await openSearchBar(fileEntry); - // Search model shouldn't be cleared from merely reopening search bar + // Search model shouldn't be cleared after search bar closed expect(Object.keys(FindInFiles.searchModel.results).length).not.toBe(0); + await openSearchBar(fileEntry); + // opening searchbar will also trigger a search with last searched string + + await awaitsFor(function () { + return Object.keys(FindInFiles.searchModel.results).length; + }, "waiting for search results to be there"); + await closeSearchBar(); // Search model shouldn't be cleared after search bar closed without running a search expect(Object.keys(FindInFiles.searchModel.results).length).not.toBe(0); }); + + it("should switch between find replace and find in files with selection work as expected", async function () { + await closeSearchBar(); + const filePath = testPath + "/foo.js"; + await __PR.openFile(filePath); + __PR.setCursors(["7:5-7:13"]); // select text: function + CommandManager.execute(Commands.CMD_FIND); + + expect(getSearchField().val()).toEql("function"); + + CommandManager.execute(Commands.CMD_FIND_IN_FILES); + await awaitsFor(()=>{ + return $("#find-in-files-results .contracting-col").text() === "function"; + }, ()=>{ + return `Find in files search to be function but got${$("#find-in-files-results .contracting-col").text()}`; + }); + + __PR.setCursors(["5:15-5:22"]); // select text: require + CommandManager.execute(Commands.CMD_FIND); + expect(getSearchField().val()).toEql("require"); + await openSearchBar(); + }); }); }); }); diff --git a/test/spec/FindReplace-integ-test.js b/test/spec/FindReplace-integ-test.js index 01e5d079f0..d853ce0004 100644 --- a/test/spec/FindReplace-integ-test.js +++ b/test/spec/FindReplace-integ-test.js @@ -90,7 +90,7 @@ define(function (require, exports, module) { ]; - var testWindow, twCommandManager, twEditorManager, twFindInFiles, tw$; + var testWindow, twCommandManager, twEditorManager, twFindInFiles, tw$, __PR; var myDocument, myEditor; // Helper functions for testing cursor position / selection range @@ -305,18 +305,18 @@ define(function (require, exports, module) { twCommandManager.execute(Commands.CMD_FIND); // The previous search term "b" was pre-filled, so the editor was centered there already - expect(myEditor.centerOnCursor.calls.count()).toEql(1); + expect(myEditor.centerOnCursor.calls.count()).toEql(0); enterSearchText("foo"); expectHighlightedMatches(fooExpectedMatches); expectSelection(fooExpectedMatches[0]); expectMatchIndex(0, 4); - expect(myEditor.centerOnCursor.calls.count()).toEql(2); + expect(myEditor.centerOnCursor.calls.count()).toEql(1); twCommandManager.execute(Commands.CMD_FIND_NEXT); expectSelection(fooExpectedMatches[1]); expectMatchIndex(1, 4); - expect(myEditor.centerOnCursor.calls.count()).toEql(3); + expect(myEditor.centerOnCursor.calls.count()).toEql(2); twCommandManager.execute(Commands.CMD_FIND_NEXT); expectSelection(fooExpectedMatches[2]); expectMatchIndex(2, 4); @@ -329,7 +329,7 @@ define(function (require, exports, module) { twCommandManager.execute(Commands.CMD_FIND_NEXT); expectSelection(fooExpectedMatches[0]); expectMatchIndex(0, 4); - expect(myEditor.centerOnCursor.calls.count()).toEql(6); + expect(myEditor.centerOnCursor.calls.count()).toEql(5); }); it("should find all case-insensitive matches with mixed-case text", async function () { @@ -337,18 +337,18 @@ define(function (require, exports, module) { twCommandManager.execute(Commands.CMD_FIND); // The previous search term "foo" was pre-filled, so the editor was centered there already - expect(myEditor.centerOnCursor.calls.count()).toEql(1); + expect(myEditor.centerOnCursor.calls.count()).toEql(0); enterSearchText("Foo"); expectHighlightedMatches(fooExpectedMatches); expectSelection(fooExpectedMatches[0]); expectMatchIndex(0, 4); - expect(myEditor.centerOnCursor.calls.count()).toEql(2); + expect(myEditor.centerOnCursor.calls.count()).toEql(1); twCommandManager.execute(Commands.CMD_FIND_NEXT); expectSelection(fooExpectedMatches[1]); expectMatchIndex(1, 4); - expect(myEditor.centerOnCursor.calls.count()).toEql(3); + expect(myEditor.centerOnCursor.calls.count()).toEql(2); twCommandManager.execute(Commands.CMD_FIND_NEXT); expectSelection(fooExpectedMatches[2]); expectMatchIndex(2, 4); @@ -361,7 +361,7 @@ define(function (require, exports, module) { twCommandManager.execute(Commands.CMD_FIND_NEXT); expectSelection(fooExpectedMatches[0]); expectMatchIndex(0, 4); - expect(myEditor.centerOnCursor.calls.count()).toEql(6); + expect(myEditor.centerOnCursor.calls.count()).toEql(5); }); it("should find all case-sensitive matches with mixed-case text", async function () { @@ -454,7 +454,7 @@ define(function (require, exports, module) { twCommandManager.execute(Commands.CMD_FIND); // The previous search term "Foo" was pre-filled, so the editor was centered there already - expect(myEditor.centerOnCursor.calls.count()).toEql(1); + expect(myEditor.centerOnCursor.calls.count()).toEql(0); enterSearchText("foo"); pressEscape(); @@ -463,12 +463,12 @@ define(function (require, exports, module) { await waitsForSearchBarClose(); expectSelection({start: {line: LINE_FIRST_REQUIRE, ch: 8}, end: {line: LINE_FIRST_REQUIRE, ch: 11}}); - expect(myEditor.centerOnCursor.calls.count()).toEql(2); + expect(myEditor.centerOnCursor.calls.count()).toEql(1); // Simple linear Find Next twCommandManager.execute(Commands.CMD_FIND_NEXT); expectSelection({start: {line: LINE_FIRST_REQUIRE, ch: 31}, end: {line: LINE_FIRST_REQUIRE, ch: 34}}); - expect(myEditor.centerOnCursor.calls.count()).toEql(3); + expect(myEditor.centerOnCursor.calls.count()).toEql(2); twCommandManager.execute(Commands.CMD_FIND_NEXT); expectSelection({start: {line: 6, ch: 17}, end: {line: 6, ch: 20}}); twCommandManager.execute(Commands.CMD_FIND_NEXT); @@ -565,15 +565,44 @@ define(function (require, exports, module) { expectSearchBarOpen(); expect(getSearchField().val()).toEql("Foo"); expectHighlightedMatches(capitalFooSelections); + // not select anything by default at start + twCommandManager.execute(Commands.CMD_FIND_NEXT); expectSelection(capitalFooSelections[0]); expectMatchIndex(0, 3); - expect(myEditor.centerOnCursor.calls.count()).toEql(3); + expect(myEditor.centerOnCursor.calls.count()).toEql(2); twCommandManager.execute(Commands.CMD_FIND_NEXT); expectSelection(capitalFooSelections[1]); expectMatchIndex(1, 3); }); + it("should highlight the current position match properly", async function () { + myEditor.setCursorPos(0, 0); + + twCommandManager.execute(Commands.CMD_FIND); + + enterSearchText("Foo"); + pressEscape(); + + await waitsForSearchBarClose(); + + // Open search bar a second time + myEditor.setCursorPos(2, 8); // |Foo + twCommandManager.execute(Commands.CMD_FIND); + + expectSearchBarOpen(); + expect(getSearchField().val()).toEql("Foo"); + expectMatchIndex(0, 4); + + myEditor.setCursorPos(2, 9); // F|oo + twCommandManager.execute(Commands.CMD_FIND); + expectMatchIndex(0, 4); + + myEditor.setCursorPos(2, 11); // Foo| + twCommandManager.execute(Commands.CMD_FIND); + expectMatchIndex(0, 4); + }); + it("should open search bar on Find Next with no previous search", async function () { // Make sure we have no previous query twCommandManager.execute(Commands.CMD_FIND); @@ -806,6 +835,10 @@ define(function (require, exports, module) { myEditor.setCursorPos(LINE_FIRST_REQUIRE, 0); twCommandManager.execute(Commands.CMD_FIND); + // just executing find will load the search string from history(last search string) and it will not set + // selection so that the user can press escape and carry on from his cursor. the selection is set only + // on interacting with the find workflow + twCommandManager.execute(Commands.CMD_FIND_NEXT); expectSelection({start: {line: LINE_FIRST_REQUIRE, ch: CH_REQUIRE_START}, end: {line: LINE_FIRST_REQUIRE, ch: CH_REQUIRE_PAREN}}); twCommandManager.execute(Commands.CMD_FIND_NEXT);