DevTools: Implement the console prompt with CodeMirror

DevTools: Implement the console prompt with CodeMirror

BUG=607690
Committed: https://crrev.com/fd9f42976b2d7c512f5242dffe713bfa06c22a9c
Cr-Commit-Position: refs/heads/master@{#419305}

[einbinder, 2016-09-12 18:02:49]

einbinder@chromium.org changed reviewers:
+ luoe@chromium.org, lushnikov@chromium.org, pfeldman@chromium.org

[einbinder, 2016-09-12 18:02:50]

Ptal

[dgozman, 2016-09-12 21:38:55]

dgozman@chromium.org changed reviewers:
+ dgozman@chromium.org

[dgozman, 2016-09-12 21:38:57]

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js
(right):

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:23:
prompt.setEditor(factory.createEditor({
Let's pass editor/factory in constructor instead.

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:62:
attach: function(element)
Let's maybe make ConsolePrompt a widget itself?

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:71:
addKeyDownHandler: function(handler)
Is this used outside?

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:76:
clearAutoComplete: function()
|clearAutocomplete| to match editor.

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:110:
return (this._editor.selection().endLine === 0);
nit: extra ()

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:126:
setText: function(text){
{ on next line

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:138:
setAddCompletionsFromHistory: function(value)
annotate please.

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:183:
return;
No need to return.

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:191:
historyCompletions: function(prefix)
Is this used outside?

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:219:
hasFocus: function()
Is this used outside?

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js
(right):

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:106:
this._promptElement.addEventListener("input", this._promptInput.bind(this),
false);
Should we use CodeMirror api instead? Does it interact with this correctly?

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:141:
function promptCreated(prompt)
I'd rather move asynchronicity to ConsolePrompt itself, and proxy things like
text/focus before editor is loaded there.

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:194: if
(this._prompt)
Could this even happen without a prompt? What about other places?

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:578:
this._viewport.setStickToBottom(true);
Nice catch!

[einbinder, 2016-09-13 01:08:00]

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js
(right):

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:23:
prompt.setEditor(factory.createEditor({
On 2016/09/12 at 21:38:56, dgozman wrote:
> Let's pass editor/factory in constructor instead.

Done.

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:62:
attach: function(element)
On 2016/09/12 at 21:38:56, dgozman wrote:
> Let's maybe make ConsolePrompt a widget itself?

Done.

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:62:
attach: function(element)
On 2016/09/12 at 21:38:56, dgozman wrote:
> Let's maybe make ConsolePrompt a widget itself?

Done.

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:71:
addKeyDownHandler: function(handler)
On 2016/09/12 at 21:38:56, dgozman wrote:
> Is this used outside?

Removed.

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:76:
clearAutoComplete: function()
On 2016/09/12 at 21:38:57, dgozman wrote:
> |clearAutocomplete| to match editor.

Done.

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:110:
return (this._editor.selection().endLine === 0);
On 2016/09/12 at 21:38:56, dgozman wrote:
> nit: extra ()

Done.

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:126:
setText: function(text){
On 2016/09/12 at 21:38:56, dgozman wrote:
> { on next line

Done.

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:138:
setAddCompletionsFromHistory: function(value)
On 2016/09/12 at 21:38:57, dgozman wrote:
> annotate please.

Done.

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:183:
return;
On 2016/09/12 at 21:38:56, dgozman wrote:
> No need to return.

Done.

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:191:
historyCompletions: function(prefix)
On 2016/09/12 at 21:38:56, dgozman wrote:
> Is this used outside?

Doesn't appear to be anymore. Fixed.

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:219:
hasFocus: function()
On 2016/09/12 at 21:38:56, dgozman wrote:
> Is this used outside?

Removed.

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js
(right):

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:106:
this._promptElement.addEventListener("input", this._promptInput.bind(this),
false);
On 2016/09/12 at 21:38:57, dgozman wrote:
> Should we use CodeMirror api instead? Does it interact with this correctly?

CodeMirror promises to let these events fall through the editor.

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:141:
function promptCreated(prompt)
On 2016/09/12 at 21:38:57, dgozman wrote:
> I'd rather move asynchronicity to ConsolePrompt itself, and proxy things like
text/focus before editor is loaded there.

Done.

https://codereview.chromium.org/2331053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:194: if
(this._prompt)
On 2016/09/12 at 21:38:57, dgozman wrote:
> Could this even happen without a prompt? What about other places?

Removed.

[pfeldman, 2016-09-14 00:31:53]

lgtm % comments

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/BUILD.gn (right):

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/BUILD.gn:396:
"front_end/console/ConsolePrompt.js",
sort alphabetically

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js
(right):

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:1: /**
Where is my copyright?

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:7:
WebInspector.Widget.call(this);
There is no point in making something a widget unless it is styled in shadow or
needs to know when it wasShown/willHide.

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:21: *
@this {!WebInspector.ConsolePrompt}
drop !

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:41: if
(this._needsFocus)
What if I already clicked away? Might need to set focus to this.element while
resolving the widget...

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/main/module.json (right):

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/main/module.json:311:
"settingName": "consoleBracketMatching",
I'd reuse the setting from sources.

[lushnikov, 2016-09-14 01:11:22]

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js
(right):

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:48: 
nit: stray line

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:76: return
!!this._editor &&
this._editor.selection().collapseToEnd().compareTo(this._editor.fullRange().collapseToEnd())
=== 0;
nit: compareTo -> .equal

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:143:
switch (keyboardEvent.keyCode) {
You probably will simplify key handling if you use
KeyboardShortcut in its full glory:

var shortcut = WebInspector.KeyboardShortcut.makeKeyFromEvent(event);
switch(shortcut) {..}

You'll get things like CtrlOrMeta..

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:167: if
(newText !== undefined) {
fast-return?

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:239: if
(!this._editor)
how could there be no editor?

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js
(right):

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:769: if
(this._prompt)
how could there be no prompt?

[dgozman, 2016-09-14 04:26:18]

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js
(right):

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:40:
this.setText(this._initialText)
semicolon

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:41: if
(this._needsFocus)
delete this._initialText;

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:210:
this._needsFocus = true;
Let's instead focus this.element here, and switch from this.element to
this._editor in gotFactory. This avoids the problem of focus going away from
prompt to something else before you got the factory.

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:245: var
before = this._editor.text(new
WebInspector.TextRange(0,0,prefixRange.startLine,prefixRange.startColumn));
spaces

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:247:
WebInspector.ExecutionContextSelector.completionsForTextInCurrentContext(before,
prefix, false, innerWordsWithPrefix);
Let's annotate false /* doSomething */

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:249: /**
empty line please

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js
(right):

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:137: //
this._searchableView.setDefaultFocusedElement(this._prompt.element);
Commented code.

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:338:
this.focus();
Why this? I think the code which shows ConsoleView knows better whether to focus
it.

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:797: if
(!this._prompt || event.altKey || event.ctrlKey || event.shiftKey ||
!this._prompt.hasFocus())
prompt is always there.

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:836: if
(this._prompt)
ditto

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:851: if
(!this._prompt)
ditto

[einbinder, 2016-09-14 20:21:07]

ptal

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/BUILD.gn (right):

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/BUILD.gn:396:
"front_end/console/ConsolePrompt.js",
On 2016/09/14 at 00:31:52, pfeldman wrote:
> sort alphabetically

Done.

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js
(right):

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:1: /**
On 2016/09/14 at 00:31:52, pfeldman wrote:
> Where is my copyright?

Done.

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:7:
WebInspector.Widget.call(this);
On 2016/09/14 at 00:31:53, pfeldman wrote:
> There is no point in making something a widget unless it is styled in shadow
or needs to know when it wasShown/willHide.

Even if it needs a public .element and needs .focus and .hasFocus?

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:21: *
@this {!WebInspector.ConsolePrompt}
On 2016/09/14 at 00:31:52, pfeldman wrote:
> drop !

Done.

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:40:
this.setText(this._initialText)
On 2016/09/14 at 04:26:17, dgozman wrote:
> semicolon

Done.

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:41: if
(this._needsFocus)
On 2016/09/14 at 00:31:52, pfeldman wrote:
> What if I already clicked away? Might need to set focus to this.element while
resolving the widget...

I thought it was acceptable losses. I can make it work by adding tabIndex to the
.element

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:48: 
On 2016/09/14 at 01:11:21, lushnikov wrote:
> nit: stray line

Done.

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:76: return
!!this._editor &&
this._editor.selection().collapseToEnd().compareTo(this._editor.fullRange().collapseToEnd())
=== 0;
On 2016/09/14 at 01:11:21, lushnikov wrote:
> nit: compareTo -> .equal

Done.

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:143:
switch (keyboardEvent.keyCode) {
On 2016/09/14 at 01:11:21, lushnikov wrote:
> You probably will simplify key handling if you use
> KeyboardShortcut in its full glory:
> 
> var shortcut = WebInspector.KeyboardShortcut.makeKeyFromEvent(event);
> switch(shortcut) {..}
> 
> You'll get things like CtrlOrMeta..

I stole this code from TextPrompt. How about I clean up both in a separate
patch?

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:167: if
(newText !== undefined) {
On 2016/09/14 at 01:11:21, lushnikov wrote:
> fast-return?

Done.

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:210:
this._needsFocus = true;
On 2016/09/14 at 04:26:17, dgozman wrote:
> Let's instead focus this.element here, and switch from this.element to
this._editor in gotFactory. This avoids the problem of focus going away from
prompt to something else before you got the factory.

Done.

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:245: var
before = this._editor.text(new
WebInspector.TextRange(0,0,prefixRange.startLine,prefixRange.startColumn));
On 2016/09/14 at 04:26:18, dgozman wrote:
> spaces

Done.

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:247:
WebInspector.ExecutionContextSelector.completionsForTextInCurrentContext(before,
prefix, false, innerWordsWithPrefix);
On 2016/09/14 at 04:26:17, dgozman wrote:
> Let's annotate false /* doSomething */

Done.

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:249: /**
On 2016/09/14 at 04:26:17, dgozman wrote:
> empty line please

Done.

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js
(right):

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:137: //
this._searchableView.setDefaultFocusedElement(this._prompt.element);
On 2016/09/14 at 04:26:18, dgozman wrote:
> Commented code.

Done.

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:338:
this.focus();
On 2016/09/14 at 04:26:18, dgozman wrote:
> Why this? I think the code which shows ConsoleView knows better whether to
focus it.

The old code focused the prompt. I agree that its overeager, but I want to main
consistency with how the old prompt worked and fix these things in another
patch.

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:769: if
(this._prompt)
On 2016/09/14 at 01:11:21, lushnikov wrote:
> how could there be no prompt?

Got lost in a merge.

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:797: if
(!this._prompt || event.altKey || event.ctrlKey || event.shiftKey ||
!this._prompt.hasFocus())
On 2016/09/14 at 04:26:18, dgozman wrote:
> prompt is always there.

Done.

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:836: if
(this._prompt)
On 2016/09/14 at 04:26:18, dgozman wrote:
> ditto

Done.

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:851: if
(!this._prompt)
On 2016/09/14 at 04:26:18, dgozman wrote:
> ditto

Done.

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/main/module.json (right):

https://codereview.chromium.org/2331053002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/main/module.json:311:
"settingName": "consoleBracketMatching",
On 2016/09/14 at 00:31:53, pfeldman wrote:
> I'd reuse the setting from sources.

Bracket matching ins't really the right name for this option, although its what
we call it. It makes it so that brackets are automatically added in. This hurts
the autocomplete in the console, because we don't show grey hint text if you
aren't at the end of a line. With the next patch, we try to enter a newline if
your statement isn't complete. But if we add in the brackets for you, your
statement will always be complete.

[dgozman, 2016-09-14 21:20:17]

lgtm

https://codereview.chromium.org/2331053002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js
(right):

https://codereview.chromium.org/2331053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:1: //
Copyright (c) 2016 The Chromium Authors. All rights reserved.
http://dev.chromium.org/blink/coding-style#TOC-License

https://codereview.chromium.org/2331053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:240: var
resolve;
resolve -> fulfill

https://codereview.chromium.org/2331053002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js
(right):

https://codereview.chromium.org/2331053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:793: if
(event.altKey || event.ctrlKey || event.shiftKey || !this._prompt.hasFocus())
How can this be called when prompt is not focused?

https://codereview.chromium.org/2331053002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console/consoleView.css
(right):

https://codereview.chromium.org/2331053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/consoleView.css:64:
#console-prompt .CodeMirror{
space before {

https://codereview.chromium.org/2331053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/consoleView.css:69:
padding-top:0;
space after :

https://codereview.chromium.org/2331053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/consoleView.css:73:
padding-top:0;
ditto

https://codereview.chromium.org/2331053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/consoleView.css:78:
margin-left:4px;
ditto

https://codereview.chromium.org/2331053002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/devtools.js (right):

https://codereview.chromium.org/2331053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/devtools.js:897:
"consoleTimestampsEnabled", "consoleBracketMatching", "cpuProfilerView",
"cssSourceMapsEnabled", "currentDockState", "customColorPalette",
"customDevicePresets", "customEmulatedDeviceList",
Don't add new stuff here. It's for old frontends.

https://codereview.chromium.org/2331053002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/ui/TextEditor.js (right):

https://codereview.chromium.org/2331053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/TextEditor.js:1: // Copyright
(c) 2016 The Chromium Authors. All rights reserved.
http://dev.chromium.org/blink/coding-style#TOC-License

[einbinder, 2016-09-15 00:14:10]

https://codereview.chromium.org/2331053002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js
(right):

https://codereview.chromium.org/2331053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:1: //
Copyright (c) 2016 The Chromium Authors. All rights reserved.
On 2016/09/14 at 21:20:17, dgozman wrote:
> http://dev.chromium.org/blink/coding-style#TOC-License

Done.

https://codereview.chromium.org/2331053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsolePrompt.js:240: var
resolve;
On 2016/09/14 at 21:20:17, dgozman wrote:
> resolve -> fulfill

Done.

https://codereview.chromium.org/2331053002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js
(right):

https://codereview.chromium.org/2331053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/ConsoleView.js:793: if
(event.altKey || event.ctrlKey || event.shiftKey || !this._prompt.hasFocus())
On 2016/09/14 at 21:20:17, dgozman wrote:
> How can this be called when prompt is not focused?

Done.

https://codereview.chromium.org/2331053002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console/consoleView.css
(right):

https://codereview.chromium.org/2331053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console/consoleView.css:78:
margin-left:4px;
On 2016/09/14 at 21:20:17, dgozman wrote:
> ditto

Fixed all.

https://codereview.chromium.org/2331053002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/devtools.js (right):

https://codereview.chromium.org/2331053002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/devtools.js:897:
"consoleTimestampsEnabled", "consoleBracketMatching", "cpuProfilerView",
"cssSourceMapsEnabled", "currentDockState", "customColorPalette",
"customDevicePresets", "customEmulatedDeviceList",
On 2016/09/14 at 21:20:17, dgozman wrote:
> Don't add new stuff here. It's for old frontends.

Oops.

[einbinder, 2016-09-16 04:33:52]

The CQ bit was checked by einbinder@chromium.org

[einbinder, 2016-09-16 04:33:53]

The patchset sent to the CQ was uploaded after l-g-t-m from
pfeldman@chromium.org, dgozman@chromium.org
Link to the patchset: https://codereview.chromium.org/2331053002/#ps100001
(title: "Style")

[commit-bot: I haz the power, 2016-09-16 04:34:00]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[einbinder, 2016-09-16 04:46:57]

The CQ bit was unchecked by einbinder@chromium.org

[lushnikov, 2016-09-16 17:17:14]

https://codereview.chromium.org/2331053002/diff/100001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/text_editor/CodeMirrorTextEditor.js
(right):

https://codereview.chromium.org/2331053002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/text_editor/CodeMirrorTextEditor.js:985:
this._codeMirror.setSize(width, "auto");
don't we want to do skip this method altogether in case of autoheight?

[lushnikov, 2016-09-16 17:17:49]

lgtm % comment about the resize method

[einbinder, 2016-09-16 18:25:12]

https://codereview.chromium.org/2331053002/diff/100001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/text_editor/CodeMirrorTextEditor.js
(right):

https://codereview.chromium.org/2331053002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/text_editor/CodeMirrorTextEditor.js:985:
this._codeMirror.setSize(width, "auto");
On 2016/09/16 at 17:17:14, lushnikov wrote:
> don't we want to do skip this method altogether in case of autoheight?

The default codemirror.css sets a height of 300px that we have to override.

[einbinder, 2016-09-16 18:25:33]

The CQ bit was checked by einbinder@chromium.org

[einbinder, 2016-09-16 18:25:33]

The patchset sent to the CQ was uploaded after l-g-t-m from
dgozman@chromium.org, lushnikov@chromium.org, pfeldman@chromium.org
Link to the patchset: https://codereview.chromium.org/2331053002/#ps120001
(title: "Remove consoleBracketMatching setting")

[commit-bot: I haz the power, 2016-09-16 18:25:52]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-16 19:23:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-16 19:23:42]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[einbinder, 2016-09-16 19:52:39]

The CQ bit was checked by einbinder@chromium.org

[einbinder, 2016-09-16 19:52:39]

The CQ bit was checked by einbinder@chromium.org

[einbinder, 2016-09-16 19:52:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
dgozman@chromium.org, lushnikov@chromium.org, pfeldman@chromium.org
Link to the patchset: https://codereview.chromium.org/2331053002/#ps140001
(title: "Fix browser tests")

[einbinder, 2016-09-16 19:52:40]

The patchset sent to the CQ was uploaded after l-g-t-m from
dgozman@chromium.org, lushnikov@chromium.org, pfeldman@chromium.org
Link to the patchset: https://codereview.chromium.org/2331053002/#ps140001
(title: "Fix browser tests")

[commit-bot: I haz the power, 2016-09-16 19:53:07]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-16 22:34:04]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-09-16 22:37:22]

Description was changed from

==========
DevTools: Implement the console prompt with CodeMirror

BUG=607690
==========

to

==========
DevTools: Implement the console prompt with CodeMirror

BUG=607690

Committed: https://crrev.com/fd9f42976b2d7c512f5242dffe713bfa06c22a9c
Cr-Commit-Position: refs/heads/master@{#419305}
==========

[commit-bot: I haz the power, 2016-09-16 22:37:24]

Patchset 8 (id:??) landed as
https://crrev.com/fd9f42976b2d7c512f5242dffe713bfa06c22a9c
Cr-Commit-Position: refs/heads/master@{#419305}