Navigate between individual search matches in DevTools console

Source/devtools/front_end/console/ConsoleView.js

Index: Source/devtools/front_end/console/ConsoleView.js
diff --git a/Source/devtools/front_end/console/ConsoleView.js b/Source/devtools/front_end/console/ConsoleView.js
index 06954a6cb26f1903bbe80c63c08a085388eb2474..c49e2721c2bad235b268dd09490d9eaab5c08345 100644
--- a/Source/devtools/front_end/console/ConsoleView.js
+++ b/Source/devtools/front_end/console/ConsoleView.js
@@ -48,6 +48,8 @@ WebInspector.ConsoleView = function()
     this._visibleViewMessages = [];
     this._urlToMessageCount = {};
     this._hiddenByFilterCount = 0;
+    this._regexMatchRanges = [];
+
 
     this._clearConsoleButton = new WebInspector.StatusBarButton(WebInspector.UIString("Clear console log."), "clear-status-bar-item");
     this._clearConsoleButton.addEventListener("click", this._requestClearMessages, this);
@@ -574,9 +576,8 @@ WebInspector.ConsoleView.prototype = {
 
             this._visibleViewMessages.push(viewMessage);
 
-            if (this._searchRegex && viewMessage.matchesRegex(this._searchRegex)) {
-                this._searchResults.push(viewMessage);
-                this._searchableView.updateSearchMatchesCount(this._searchResults.length);
+            if (this._searchRegex && this._searchMessage(this._visibleViewMessages.length-1)) {

[robwu, 2014/10/25 23:12:59]

Spaces around minus.

[aknudsen, 2014/10/26 11:22:40]

Done.

+                this._searchableView.updateSearchMatchesCount(this._regexMatchRanges.length);
             }
         }
 
@@ -606,7 +607,7 @@ WebInspector.ConsoleView.prototype = {
 
     _consoleCleared: function()
     {
-        this._clearCurrentSearchResultHighlight();
+        this._clearSearchResultHighlights();
         this._consoleMessages = [];
         this._updateMessageList();
 
@@ -684,7 +685,7 @@ WebInspector.ConsoleView.prototype = {
     {
         this._topGroup = WebInspector.ConsoleGroup.createTopGroup();
         this._currentGroup = this._topGroup;
-        this._searchResults = [];
+        this._regexMatchRanges = [];
         this._hiddenByFilterCount = 0;
         for (var i = 0; i < this._visibleViewMessages.length; ++i) {
             this._visibleViewMessages[i].resetCloseGroupDecorationCount();
@@ -898,8 +899,8 @@ WebInspector.ConsoleView.prototype = {
 
     searchCanceled: function()
     {
-        this._clearCurrentSearchResultHighlight();
-        delete this._searchResults;
+        this._clearSearchResultHighlights();
+        this._regexMatchRanges = [];
         delete this._searchRegex;
         this._viewport.refresh();
     },
@@ -916,31 +917,28 @@ WebInspector.ConsoleView.prototype = {
         this._searchableView.updateSearchMatchesCount(0);
         this._searchRegex = createPlainTextSearchRegex(query, "gi");
 
-        /** @type {!Array.<number>} */
-        this._searchResults = [];
+        this._regexMatchRanges = [];
+        this._currentMatchRangeIndex = -1;
         for (var i = 0; i < this._visibleViewMessages.length; i++) {
-            if (this._visibleViewMessages[i].matchesRegex(this._searchRegex))
-                this._searchResults.push(i);
+            this._searchMessage(i);
+        }

[robwu, 2014/10/25 23:12:59]

Omit braces for single-line for/if/if-else blocks (this comment applies to all
other occurrences of this pattern in your patch).

[aknudsen, 2014/10/26 11:22:40]

Done.

+        this._searchableView.updateSearchMatchesCount(this._regexMatchRanges.length);
+        if (shouldJump) {
+            this._jumpToMatch(jumpBackwards ? -1 : 0);
+        } else {
+            console.log("Not jumping to current search result");
         }
-        this._searchableView.updateSearchMatchesCount(this._searchResults.length);
-        this._currentSearchResultIndex = -1;
-        if (shouldJump && this._searchResults.length)
-            this._jumpToSearchResult(jumpBackwards ? -1 : 0);
         this._viewport.refresh();
     },
 
     jumpToNextSearchResult: function()
     {
-        if (!this._searchResults || !this._searchResults.length)
-            return;
-        this._jumpToSearchResult(this._currentSearchResultIndex + 1);
+        this._jumpToMatch(this._currentMatchRangeIndex + 1);
     },
 
     jumpToPreviousSearchResult: function()
     {
-        if (!this._searchResults || !this._searchResults.length)
-            return;
-        this._jumpToSearchResult(this._currentSearchResultIndex - 1);
+        this._jumpToMatch(this._currentMatchRangeIndex - 1);
     },
 
     /**
@@ -959,26 +957,71 @@ WebInspector.ConsoleView.prototype = {
         return false;
     },
 
-    _clearCurrentSearchResultHighlight: function()
+    _clearSearchResultHighlights: function()
     {
-        if (!this._searchResults)
-            return;
-
-        var highlightedViewMessage = this._visibleViewMessages[this._searchResults[this._currentSearchResultIndex]];
-        if (highlightedViewMessage)
-            highlightedViewMessage.clearHighlight();
-        this._currentSearchResultIndex = -1;
+        console.log("Clearing search result highlights");
+        for (var i = 0; i < this._regexMatchRanges.length; ++i) {
+            var matchRange = this._regexMatchRanges[i];
+            var message = this._visibleViewMessages[
+                matchRange.message];

[robwu, 2014/10/25 23:13:00]

matchRange.message? As said in the comment below, this is confusing and needs to
be change to something more sensible, e.g. matchRange.resultIndex.

[aknudsen, 2014/10/26 11:22:40]

I changed it to messageIndex, since it's actually an index into the list of
messages.

+            if (message) {
+                message.clearHighlights();
+            }
+        }
+        this._currentMatchRangeIndex = -1;
     },
 
-    _jumpToSearchResult: function(index)
+    _jumpToMatch: function(index)

[robwu, 2014/10/25 23:12:59]

"_jumpToSearchResult" was more descriptive than "_jumpToMatch", in my opinion.

[aknudsen, 2014/10/26 11:22:40]

The reason I renamed to jumpToMatch was to tie into the core concept of
'matches', since I identify individual search hits as matches. I think
regardless of which term we choose to identify these occurrences, it makes sense
to name these methods correspondingly.

     {
-        index = mod(index, this._searchResults.length);
-        this._clearCurrentSearchResultHighlight();
-        this._currentSearchResultIndex = index;
-        this._searchableView.updateCurrentMatchIndex(this._currentSearchResultIndex);
-        var currentViewMessageIndex = this._searchResults[index];
-        this._viewport.scrollItemIntoView(currentViewMessageIndex);
-        this._visibleViewMessages[currentViewMessageIndex].highlightSearchResults(this._searchRegex);
+        if (!this._regexMatchRanges.length) {
+            return;
+        }
+
+        index = mod(index, this._regexMatchRanges.length);
+        console.log("Jumping to search match #" + index);
+        var matchRange;
+        var message;
+        if (this._currentMatchRangeIndex >= 0) {
+            // Unmark current match range
+            matchRange = this._regexMatchRanges[this._currentMatchRangeIndex];
+            console.log("Unmarking current search match, in message #" + matchRange.message);
+            message = this._visibleViewMessages[matchRange.message];
+            message.highlightMatch(matchRange.start, matchRange.end);
+        }
+
+        this._currentMatchRangeIndex = index;
+        this._searchableView.updateCurrentMatchIndex(index);
+        matchRange = this._regexMatchRanges[index];
+        message = this._visibleViewMessages[matchRange.message];
+        this._viewport.scrollItemIntoView(message);
+        console.log("Highlighting range " + matchRange.start + "," + matchRange.end + " in message #" +
+            matchRange.message + " as the current match");
+        message.highlightMatch(matchRange.start, matchRange.end, true);
+    },
+
+    _searchMessage: function(index)
+    {
+        // Reset regex wrt. global search
+        this._searchRegex.lastIndex = 0;
+
+        var numMatches = 0;
+        var message = this._visibleViewMessages[index];
+        var text = message.getText();
+        var match = this._searchRegex.exec(text);
+        while (match && match[0]) {

[robwu, 2014/10/25 23:12:59]

The idiomatic way to use .exec in a JS loop is:

var match;
while ((match = this._searchRegex.exec(text)) !== null && match[0]) {
    ....
}

[aknudsen, 2014/10/26 11:22:40]

Done.

+            var matchRange = {
+                message: index,

[robwu, 2014/10/25 23:12:59]

"message" is confusing. I thought that it was a string when I read it elsewhere.
Choose a more appropriate property name, e.g. resultIndex.

[aknudsen, 2014/10/26 11:22:40]

Done.

+                start: match.index,
+                end: match.index + match[0].length-1,

[robwu, 2014/10/25 23:12:59]

Spaces around the minus sign.

[aknudsen, 2014/10/26 11:22:40]

Done.

+            };
+            this._regexMatchRanges.push(matchRange);
+            message.highlightMatch(matchRange.start, matchRange.end);
+
+            match = this._searchRegex.exec(text);
+            ++numMatches;
+        }
+
+        return numMatches > 0;

[robwu, 2014/10/25 23:12:59]

numMatches can be eliminated by using the fact that the matchRange variable is
only truthy when the loop runs at least once.

return !!matchRange;

[aknudsen, 2014/10/26 11:22:40]

Done.

     },
 
     __proto__: WebInspector.VBox.prototype
@@ -1132,7 +1175,7 @@ WebInspector.ConsoleCommand = function(message, linkifier, nestingLevel)
 }
 
 WebInspector.ConsoleCommand.prototype = {
-    clearHighlight: function()
+    clearHighlights: function()
     {
         var highlightedMessage = this._formattedCommand;
         delete this._formattedCommand;
@@ -1187,6 +1230,22 @@ WebInspector.ConsoleCommand.prototype = {
         this._formattedCommand.textContent = this.text;
     },
 
+    /**
+    * @return {!string}
+    */
+    getText: function()
+    {
+        return this.text;
+    },
+
+    highlightMatch: function(start, end, isCurrent)
+    {
+       console.log("Highlighting from position " + start + " to " + end);
+       WebInspector.highlightSearchResults(this._formattedCommand,
+           [new WebInspector.SourceRange(start, end - start + 1)], undefined, isCurrent);
+       this._element.scrollIntoViewIfNeeded();
+    },
+
     __proto__: WebInspector.ConsoleViewMessage.prototype
 }