Navigate between individual search matches in DevTools console

Source/devtools/front_end/console/ConsoleView.js

 /*
  * Copyright (C) 2007, 2008 Apple Inc.  All rights reserved.
  * Copyright (C) 2009 Joseph Pecoraro
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
@@ skipping 30 matching lines @@
 
     this._searchableView = new WebInspector.SearchableView(this);
     this._searchableView.setMinimalSearchQuerySize(0);
     this._searchableView.show(this.element);
 
     this._contentsElement = this._searchableView.element;
     this._contentsElement.classList.add("console-view");
     this._visibleViewMessages = [];
     this._urlToMessageCount = {};
     this._hiddenByFilterCount = 0;
+    this._regexMatchRanges = [];

[lushnikov, 2014/11/07 16:27:16]

please annotate this with the type of elemens in the array. 

You might want to typedef your classname for regexMatch object - please, take a
look at the CSSParser.js file for an example.

[aknudsen, 2014/11/08 11:49:50]

Acknowledged.

[aknudsen, 2014/11/09 00:02:12]

Done.

+
 
     this._clearConsoleButton = new WebInspector.StatusBarButton(WebInspector.UIString("Clear console log."), "clear-status-bar-item");
     this._clearConsoleButton.addEventListener("click", this._requestClearMessages, this);
 
     this._executionContextSelector = new WebInspector.StatusBarComboBox(this._executionContextChanged.bind(this), "console-context");
 
     /**
      * @type {!Map.<!WebInspector.ExecutionContext, !Element>}
      */
     this._optionByExecutionContext = new Map();
@@ skipping 506 matching lines @@
             this._currentGroup = this._currentGroup.parentGroup();
             return;
         }
         if (!this._currentGroup.messagesHidden()) {
             var originatingMessage = viewMessage.consoleMessage().originatingMessage();
             if (lastMessage && originatingMessage && lastMessage.consoleMessage() === originatingMessage)
                 lastMessage.toMessageElement().classList.add("console-adjacent-user-command-result");
 
             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))
+                this._searchableView.updateSearchMatchesCount(this._regexMatchRanges.length);
         }
 
         if (viewMessage.consoleMessage().isGroupStartMessage())
             this._currentGroup = new WebInspector.ConsoleGroup(this._currentGroup, viewMessage);
     },
 
     /**
      * @param {!WebInspector.ConsoleMessage} message
      * @return {!WebInspector.ConsoleViewMessage}
      */
     _createViewMessage: function(message)
     {
         var nestingLevel = this._currentGroup.nestingLevel();
         switch (message.type) {
         case WebInspector.ConsoleMessage.MessageType.Command:
             return new WebInspector.ConsoleCommand(message, this._linkifier, nestingLevel);
         case WebInspector.ConsoleMessage.MessageType.Result:
             return new WebInspector.ConsoleCommandResult(message, this._linkifier, nestingLevel);
         case WebInspector.ConsoleMessage.MessageType.StartGroupCollapsed:
         case WebInspector.ConsoleMessage.MessageType.StartGroup:
             return new WebInspector.ConsoleGroupViewMessage(message, this._linkifier, nestingLevel);
         default:
             return new WebInspector.ConsoleViewMessage(message, this._linkifier, nestingLevel);
         }
     },
 
     _consoleCleared: function()
     {
-        this._clearCurrentSearchResultHighlight();
+        this._clearSearchResultHighlights();
         this._consoleMessages = [];
         this._updateMessageList();
 
         if (this._searchRegex)
             this._searchableView.updateSearchMatchesCount(0);
 
         this._linkifier.reset();
     },
 
     _handleContextMenuEvent: function(event)
@@ skipping 57 matching lines @@
             return true;
         }
 
         return false;
     },
 
     _updateMessageList: function()
     {
         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();
             this._visibleViewMessages[i].resetIncrementRepeatCount();
         }
         this._visibleViewMessages = [];
         for (var i = 0; i < this._consoleMessages.length; ++i)
             this._appendMessageToEnd(this._consoleMessages[i]);
         this._updateFilterStatus();
         this._viewport.invalidate();
@@ skipping 193 matching lines @@
     /**
      * @return {!Array.<!Element>}
      */
     elementsToRestoreScrollPositionsFor: function()
     {
         return [this._messagesElement];
     },
 
     searchCanceled: function()
     {
-        this._clearCurrentSearchResultHighlight();
-        delete this._searchResults;
+        this._clearSearchResultHighlights();
+        this._regexMatchRanges = [];
         delete this._searchRegex;
         this._viewport.refresh();
     },
 
     /**
      * @param {!WebInspector.SearchableView.SearchConfig} searchConfig
      * @param {boolean} shouldJump
      * @param {boolean=} jumpBackwards
      */
     performSearch: function(searchConfig, shouldJump, jumpBackwards)
     {
         var query = searchConfig.query;
         this.searchCanceled();
         this._searchableView.updateSearchMatchesCount(0);
         this._searchRegex = createPlainTextSearchRegex(query, "gi");
 
-        /** @type {!Array.<number>} */
-        this._searchResults = [];
-        for (var i = 0; i < this._visibleViewMessages.length; i++) {
-            if (this._visibleViewMessages[i].matchesRegex(this._searchRegex))
-                this._searchResults.push(i);
-        }
-        this._searchableView.updateSearchMatchesCount(this._searchResults.length);
-        this._currentSearchResultIndex = -1;
-        if (shouldJump && this._searchResults.length)
-            this._jumpToSearchResult(jumpBackwards ? -1 : 0);
+        this._regexMatchRanges = [];
+        this._currentMatchRangeIndex = -1;
+        for (var i = 0; i < this._visibleViewMessages.length; i++)
+            this._searchMessage(i);
+        this._searchableView.updateSearchMatchesCount(this._regexMatchRanges.length);
+        if (shouldJump)
+            this._jumpToMatch(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);
     },
 
     /**
      * @return {boolean}
      */
     supportsCaseSensitiveSearch: function()
     {
         return false;
     },
 
     /**
      * @return {boolean}
      */
     supportsRegexSearch: function()
     {
         return false;
     },
 
-    _clearCurrentSearchResultHighlight: function()
+    _clearSearchResultHighlights: function()
     {
-        if (!this._searchResults)
+        for (var i = 0; i < this._regexMatchRanges.length; ++i) {
+            var matchRange = this._regexMatchRanges[i];
+            var message = this._visibleViewMessages[

[lushnikov, 2014/11/07 16:27:17]

please no wrapping in the code

[aknudsen, 2014/11/08 11:49:50]

Acknowledged.

Out of curiousity though, how do you guys deal with long lines, by enabling
soft-wrap in the editor? I'm totally used to a certain maximum line length,
which is useful in avoiding having to side scroll when diffing for instance.

[aknudsen, 2014/11/09 00:02:12]

Done.

[lushnikov, 2014/11/19 06:55:47]

It depends, one enables line-wrapping, one gets larger display.. All in all, its
a styleguide we all follow.

+                matchRange.messageIndex];
+            if (message)
+                message.clearHighlights();
+        }
+        this._currentMatchRangeIndex = -1;
+    },
+
+    _jumpToMatch: function(index)

[lushnikov, 2014/11/07 16:27:16]

annotate

[aknudsen, 2014/11/08 11:49:51]

Acknowledged.

[aknudsen, 2014/11/09 00:02:13]

Done.

+    {
+        if (!this._regexMatchRanges.length)
             return;
 
-        var highlightedViewMessage = this._visibleViewMessages[this._searchResults[this._currentSearchResultIndex]];
-        if (highlightedViewMessage)
-            highlightedViewMessage.clearHighlight();
-        this._currentSearchResultIndex = -1;
+        index = mod(index, this._regexMatchRanges.length);

[lushnikov, 2014/11/07 16:27:17]

this line could be moved after the "if" 

[aknudsen, 2014/11/08 11:49:51]

Acknowledged.

[aknudsen, 2014/11/09 00:02:13]

Done.

+        var matchRange;
+        if (this._currentMatchRangeIndex >= 0) {
+            // Convert current match highlight to regular match highlight

[lushnikov, 2014/11/07 16:27:17]

remove comment

[aknudsen, 2014/11/08 11:49:50]

Acknowledged.

[aknudsen, 2014/11/09 00:02:13]

Done.

+            matchRange = this._regexMatchRanges[this._currentMatchRangeIndex];
+            matchRange.highlightNode.classList.remove(WebInspector.currentHighlightedSearchResultClassName);
+            matchRange.highlightNode.classList.add(WebInspector.highlightedSearchResultClassName);

[lushnikov, 2014/11/07 16:27:16]

We can make this simpler. This line could be deleted together with line #984. 

After that, your rule for highlighting current-search-result
should have a selector with bigger specificity, e.g. 

.highlighted-search-result.current-search-result { 
...
}

[aknudsen, 2014/11/08 11:49:50]

Acknowledged.

[aknudsen, 2014/11/09 00:02:13]

Done.

+        }
+
+        this._currentMatchRangeIndex = index;
+        this._searchableView.updateCurrentMatchIndex(index);
+        matchRange = this._regexMatchRanges[index];
+        matchRange.highlightNode.classList.remove(WebInspector.highlightedSearchResultClassName);

[lushnikov, 2014/11/07 16:27:16]

this is not needed as per previous comment

[aknudsen, 2014/11/08 11:49:50]

Acknowledged.

[aknudsen, 2014/11/09 00:02:13]

Done.

+        matchRange.highlightNode.classList.add(WebInspector.currentHighlightedSearchResultClassName);
+        // Scroll the message itself into view

[lushnikov, 2014/11/07 16:27:17]

lets remove these comments regarding scroll and add a test instead to verify
that every match is actually shown to the user.

[aknudsen, 2014/11/08 11:49:51]

Why are the comments problematic though? I had no idea initially, for example,
that the message had to be scrolled into view before
matchRange.highlightNode.scrollIntoViewIfNeeded() would have any effect. I agree
that there should be a test in any case.

[lushnikov, 2014/11/19 06:55:47]

In this case, the comment doesn't add any new information to the line it
comments (viewport.scrollItemIntoView is verbose
and clear itself).
In general, the policy is to write as little comments as possible and prefer
self-describing code.

+        this._viewport.scrollItemIntoView(matchRange.messageIndex);
+        // In case the highlight node is in a tree element, it must be scrolled into view, 
+        // the message itself must first be scrolled into view though
+        matchRange.highlightNode.scrollIntoViewIfNeeded();
     },
 
-    _jumpToSearchResult: function(index)
+    _searchMessage: function(index)

[lushnikov, 2014/11/07 16:27:16]

please annotate
also, please move it closer to the performSearch method to improve ease of read

[aknudsen, 2014/11/08 11:49:50]

Acknowledged.

[aknudsen, 2014/11/09 00:02:13]

Done.

     {
-        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);
+        // Reset regex wrt. global search

[lushnikov, 2014/11/07 16:27:16]

please finish the comment with a period

[aknudsen, 2014/11/08 11:49:51]

Acknowledged.

Is this a common style guideline?

[aknudsen, 2014/11/09 00:02:12]

Done.

[lushnikov, 2014/11/19 06:55:47]

Yes, http://www.chromium.org/blink/coding-style#TOC-Comments

+        this._searchRegex.lastIndex = 0;
+
+        var message = this._visibleViewMessages[index];
+        var text = message.getText();
+        var match;
+        var matchRange;
+        while ((match = this._searchRegex.exec(text)) && match[0]) {
+            matchRange = {
+                messageIndex: index,
+                start: match.index,
+                end: match.index + match[0].length - 1,

[lushnikov, 2014/11/07 16:27:16]

you compute "end" here, and afterward couple of layers below (in highlightMatch
function) compute range length again. Lets store length here?

[aknudsen, 2014/11/08 11:49:50]

Acknowledged.

[aknudsen, 2014/11/09 00:02:13]

I simplified, by sending an array of SourceRanges to message.highlightMatches
instead.

+            };
+            matchRange.highlightNode = message.highlightMatch(matchRange.start, matchRange.end)[0];

[lushnikov, 2014/11/07 16:27:16]

Bad news: this calls WebInspector.highlightSearchResults for every match range. 
And this is inefficient, as it is iterating through the DOM every time.

Good news: WebInspector.highlightSearchResults accepts an array of ranges to be
highlighted, so this is easily fixable.

[aknudsen, 2014/11/08 11:49:50]

Acknowledged. Thanks.

[aknudsen, 2014/11/09 00:02:12]

Done.

+            this._regexMatchRanges.push(matchRange);
+        }
+
+        return !!matchRange;
     },
 
     __proto__: WebInspector.VBox.prototype
 }
 
 /**
  * @constructor
  * @extends {WebInspector.Object}
  * @param {!WebInspector.ConsoleView} view
  */
@@ skipping 133 matching lines @@
  * @param {!WebInspector.ConsoleMessage} message
  * @param {!WebInspector.Linkifier} linkifier
  * @param {number} nestingLevel
  */
 WebInspector.ConsoleCommand = function(message, linkifier, nestingLevel)
 {
     WebInspector.ConsoleViewMessage.call(this, message, linkifier, nestingLevel);
 }
 
 WebInspector.ConsoleCommand.prototype = {
-    clearHighlight: function()
+    clearHighlights: function()
     {
         var highlightedMessage = this._formattedCommand;
         delete this._formattedCommand;
         this._formatCommand();
         this._element.replaceChild(this._formattedCommand, highlightedMessage);
     },
 
+    // XXX: Can we remove this?

[lushnikov, 2014/11/07 16:27:17]

Yes! 

[aknudsen, 2014/11/08 11:49:51]

Acknowledged. :)

[aknudsen, 2014/11/09 00:02:13]

Done.

     /**
      * @param {!RegExp} regexObject
      */
     highlightSearchResults: function(regexObject)
     {
         regexObject.lastIndex = 0;
         var match = regexObject.exec(this.text);
         var matchRanges = [];
         while (match) {
             matchRanges.push(new WebInspector.SourceRange(match.index, match[0].length));
@@ skipping 27 matching lines @@
         }
         return this._element;
     },
 
     _formatCommand: function()
     {
         this._formattedCommand = createElementWithClass("span", "console-message-text source-code");
         this._formattedCommand.textContent = this.text;
     },
 
+    /**
+    * @return {!string}

[lushnikov, 2014/11/07 16:27:16]

wrong indent for comments (asterisks should form a column)

[apavlov, 2014/11/07 16:30:07]

Non-object types should not have ! in front of them ({!Object} but not {!string}
or {!function()}), as they are non-nullable by default (as opposed to object
types, which are nullable by default).

[aknudsen, 2014/11/08 11:49:50]

Acknowledged.

[aknudsen, 2014/11/08 11:49:51]

Acknowledged.

[aknudsen, 2014/11/09 00:02:12]

Done.

[aknudsen, 2014/11/09 00:02:13]

Done.

+    */
+    getText: function()
+    {
+        return this.text;
+    },
+
+    /**
+    * @return {!Array.<!Element>}
+    */
+    highlightMatch: function(start, end)

[lushnikov, 2014/11/07 16:27:17]

arguments should be annotated as well

[aknudsen, 2014/11/08 11:49:51]

Acknowledged.

[aknudsen, 2014/11/09 00:02:13]

Done.

+    {
+        var highlightNodes = [];
+        if (this._formattedCommand) {
+            highlightNodes = WebInspector.highlightSearchResults(this._formattedCommand,

[lushnikov, 2014/11/07 16:27:16]

please no wrap

[aknudsen, 2014/11/08 11:49:50]

Acknowledged.

[aknudsen, 2014/11/09 00:02:13]

Done.

+                [new WebInspector.SourceRange(start, end - start + 1)]);
+            this._element.scrollIntoViewIfNeeded();

[lushnikov, 2014/11/07 16:27:17]

Why do you need this call? It should not be needed.

[aknudsen, 2014/11/09 00:02:12]

Done.

+        }
+        return highlightNodes;
+    },
+
     __proto__: WebInspector.ConsoleViewMessage.prototype
 }
 
 /**
  * @constructor
  * @extends {WebInspector.ConsoleViewMessage}
  * @param {!WebInspector.ConsoleMessage} message
  * @param {!WebInspector.Linkifier} linkifier
  * @param {number} nestingLevel
  */
@@ skipping 84 matching lines @@
 WebInspector.ConsoleView.ShowConsoleActionDelegate.prototype = {
     /**
      * @return {boolean}
      */
     handleAction: function()
     {
         WebInspector.console.show();
         return true;
     }
 }