Merge pull request #2062 from adobe/pflynn/quickopen-highlighting

Fix Quick Open so discontiguous matches are highlighted in all modes. Fix bug where stringMatch() returns ranges missing the first char if the 1st char in unmatched while the 2nd char IS matched.

Author: peterflynn

src/search/QuickOpen.js

@@ -451,11 +451,12 @@ define(function (require, exports, module) {
     * for sorting. It also has a stringRanges array, each entry with
     * "text", "matched" and "segment". If you string the "text" properties together, you will
     * get the original str. Using the matched property, you can highlight
-    * the string matches. The segment property tells you the most specific segment
-    * covered by the range, though there may be more than one segment convered.
+    * the string matches. The segment property tells you the most specific (rightmost)
+    * segment covered by the range, though there may be more than one segment covered.
+    * Segments are currently determined by "/"s.
     *
-    * Use basicMatchSort() to sort the filtered results taking this ranking
-    * label of the SearchResult is set to 'str'.
+    * Use basicMatchSort() to sort the filtered results taking this ranking into account.
+    * The label of the SearchResult is set to 'str'.
     * @param {!string} str
     * @param {!string} query
     * @return {?SearchResult}
@@ -493,7 +494,7 @@ define(function (require, exports, module) {
         }
 
         // No query? Short circuit the normal work done and just
-        // return a simple match with a range that covers the whole string.
+        // return a single range that covers the whole string.
         if (!query) {
             result = new SearchResult(str);
             result.matchGoodness = 0;
@@ -569,7 +570,7 @@ define(function (require, exports, module) {
             addToStringRanges(Math.abs(sequentialMatches), sequentialMatches > 0);
         }
 
-        if (strCounter > 0) {
+        if (strCounter >= 0) {
             stringRanges.unshift({
                 text: str.substring(0, strCounter + 1),
                 matched: false,
@@ -644,7 +645,7 @@ define(function (require, exports, module) {
      * within each tier.
      */
     function basicMatchSort(searchResults) {
-        multiFieldSort(searchResults, { matchGoodness: 0, label: 2 });
+        multiFieldSort(searchResults, { matchGoodness: 0, label: 1 });
     }
 
 
@@ -713,55 +714,66 @@ define(function (require, exports, module) {
     }
 
 
-    function defaultResultsFormatter(item, query) {
-        query = query.slice(query.indexOf("@") + 1, query.length);
-
-        // Escape both query and item so the replace works properly below
-        query = StringUtils.htmlEscape(query);
+    /**
+     * Formats item's label as properly escaped HTML text, highlighting sections that match 'query'.
+     * If item is a SearchResult generated by stringMatch(), uses its metadata about which string ranges
+     * matched; else formats the label with no highlighting.
+     * @param {!string|SearchResult} item
+     * @param {?string} matchClass CSS class for highlighting matched text
+     * @param {?function(number, string):string} rangeFilter
+     * @return {!string} bolded, HTML-escaped result
+     */
+    function highlightMatch(item, matchClass, rangeFilter) {
         var label = item.label || item;
-        var displayName = StringUtils.htmlEscape(label);
-
-        if (query.length > 0) {
-            // make the user's query bold within the item's text
-            displayName = displayName.replace(
-                new RegExp(StringUtils.regexEscape(query), "gi"),
-                "<strong>$&</strong>"
-            );
+        matchClass = matchClass || "quicksearch-namematch";
+        
+        var stringRanges = item.stringRanges;
+        if (!stringRanges) {
+            // If result didn't come from stringMatch(), highlight nothing
+            stringRanges = [{
+                text: label,
+                matched: false,
+                segment: 0
+            }];
         }
-
-        return "<li>" + displayName + "</li>";
-    }
-    
-    function _filenameResultsFormatter(item, query) {
-        // Use the filename formatter
-        query = StringUtils.htmlEscape(query);
-    
-        // put the path pieces together, highlighting the matched parts
-        // of the string
+        
         var displayName = "";
-        var displayPath = "";
 
-        item.stringRanges.forEach(function (range) {
+        // Put the path pieces together, highlighting the matched parts
+        stringRanges.forEach(function (range) {
             if (range.matched) {
-                displayPath += '<span class="quicksearch-pathmatch">';
-                displayName += '<span class="quicksearch-namematch">';
-            } else {
-                displayPath += '<span>';
-                displayName += '<span>';
+                displayName += "<span class='" + matchClass + "'>";
             }
-            displayPath += StringUtils.breakableUrl(StringUtils.htmlEscape(range.text));
-            displayPath += '</span>';
 
-            if (range.segment === 0) {
-                var rightmostSlash = range.text.lastIndexOf('/');
-                if (rightmostSlash > -1) {
-                    displayName += StringUtils.htmlEscape(range.text.substring(rightmostSlash + 1));
-                } else {
-                    displayName += StringUtils.htmlEscape(range.text);
-                }
+            var rangeText = rangeFilter ? rangeFilter(range.segment, range.text) : range.text;
+            displayName += StringUtils.breakableUrl(StringUtils.htmlEscape(rangeText));
+            
+            if (range.matched) {
+                displayName += "</span>";
             }
-            displayName += '</span>';
         });
+        return displayName;
+    }
+    
+    function defaultResultsFormatter(item, query) {
+        query = query.slice(query.indexOf("@") + 1, query.length);
+
+        var displayName = highlightMatch(item);
+        return "<li>" + displayName + "</li>";
+    }
+    
+    function _filenameResultsFormatter(item, query) {
+        // For main label, we just want filename: drop most of the string
+        function fileNameFilter(segment, rangeText) {
+            if (segment === 0) {
+                var rightmostSlash = rangeText.lastIndexOf('/');
+                return rangeText.substring(rightmostSlash + 1);  // safe even if rightmostSlash is -1
+            } else {
+                return "";
+            }
+        }
+        var displayName = highlightMatch(item, null, fileNameFilter);
+        var displayPath = highlightMatch(item, "quicksearch-pathmatch");
 
         return "<li>" + displayName + "<br /><span class='quick-open-path'>" + displayPath + "</span></li>";
     }
@@ -938,4 +950,5 @@ define(function (require, exports, module) {
     exports.stringMatch         = stringMatch;
     exports.basicMatchSort      = basicMatchSort;
     exports.multiFieldSort      = multiFieldSort;
+    exports.highlightMatch      = highlightMatch;
 });