DevTools: Omit the "Show documentation" context menu item creation for non-existent JS methods

Now context menu item for documentation is not created if WebInspector.DocumentationURLProvider cannot provide a URL for selected property.

BUG=391593
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=180056

[lushnikov, 2014-08-11 14:59:47]

https://codereview.chromium.org/458983002/diff/20001/Source/devtools/front_en...
File Source/devtools/front_end/documentation/DocumentationView.js (right):

https://codereview.chromium.org/458983002/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/documentation/DocumentationView.js:67: _getSelection:
function(textEditor)
no need for the separate function - please, inline

[apavlov, 2014-08-11 15:04:14]

<"Documentation" module context menu provider fixed> does not tell the reader
what this patch does. According to the description, it should say something
like:

DevTools: Omit the "Show documentation" context menu item for non-existent JS
methods

https://codereview.chromium.org/458983002/diff/20001/Source/devtools/front_en...
File Source/devtools/front_end/documentation/DocumentationView.js (right):

https://codereview.chromium.org/458983002/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/documentation/DocumentationView.js:53: var
selectedText = this._getSelection(textEditor);
Blink does not use "get" prefixes. This should be something akin to
"_editorSelection"

https://codereview.chromium.org/458983002/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/documentation/DocumentationView.js:56: var urlProvider
= new WebInspector.DocumentationURLProvider();
Can we make it a singleton? It has no state

https://codereview.chromium.org/458983002/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/documentation/DocumentationView.js:70: if (!selection
|| selection.isEmpty() || selection.startLine !== selection.endLine)
Shouldn't this detect the word at cursor if the selection is empty?

[semeny, 2014-08-11 15:34:28]

https://codereview.chromium.org/458983002/diff/20001/Source/devtools/front_en...
File Source/devtools/front_end/documentation/DocumentationView.js (right):

https://codereview.chromium.org/458983002/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/documentation/DocumentationView.js:56: var urlProvider
= new WebInspector.DocumentationURLProvider();
On 2014/08/11 15:04:14, apavlov wrote:
> Can we make it a singleton? It has no state

Did not really understand what is wrong and what should I do.

https://codereview.chromium.org/458983002/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/documentation/DocumentationView.js:67: _getSelection:
function(textEditor)
On 2014/08/11 14:59:47, lushnikov wrote:
> no need for the separate function - please, inline

Done.

https://codereview.chromium.org/458983002/diff/20001/Source/devtools/front_en...
Source/devtools/front_end/documentation/DocumentationView.js:70: if (!selection
|| selection.isEmpty() || selection.startLine !== selection.endLine)
On 2014/08/11 15:04:14, apavlov wrote:
> Shouldn't this detect the word at cursor if the selection is empty?

No, only if property is selected.

[lushnikov, 2014-08-12 08:18:30]

lgtm

[lushnikov, 2014-08-12 08:20:27]

On 2014/08/12 08:18:30, lushnikov wrote:
> lgtm

Before landing this, please make sure that the first line of issue description
is equal to the issue title.

[semeny, 2014-08-12 08:24:10]

On 2014/08/12 08:20:27, lushnikov wrote:
> On 2014/08/12 08:18:30, lushnikov wrote:
> > lgtm
> 
> Before landing this, please make sure that the first line of issue description
> is equal to the issue title.

Whole patch can be described only by title. Unfortunately, blink_presubmit
crashes if there is no description. So, I have to write something in
description. What can I do in this case?

[semeny, 2014-08-12 08:36:37]

The CQ bit was checked by semeny@google.com

[commit-bot: I haz the power, 2014-08-12 08:37:20]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/semeny@google.com/458983002/40001

[yurys, 2014-08-12 08:50:02]

lgtm

[commit-bot: I haz the power, 2014-08-12 09:50:41]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  blink_presubmit on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/1...)

[commit-bot: I haz the power, 2014-08-12 09:58:06]

Change committed as 180056