fix: find in files and search ux issues

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);

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;

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,15 +663,18 @@ 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");
         }
         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;
             }
         }

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);
         }

src/styles/brackets_codemirror_override.less

@@ -271,7 +271,7 @@ div.CodeMirror-cursors {
         }
     }
 
-    .CodeMirror-matchingtag {
+    .CodeMirror-matchingtag:not(.CodeMirror-searching) {
         background: @matching-bracket;
     }
 }

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();
+            });
         });
     });
 });