DevTools: Create TextEditor Interface around CodeMirrorTextEditor

DevTools: Create TextEditor Interface around CodeMirrorTextEditor
This is for lazy loading of the text_editor module in preperation of CodeMirror-based console.

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

[einbinder, 2016-08-26 17:46:20]

einbinder@chromium.org changed reviewers:
+ dgozman@chromium.org, lushnikov@chromium.org

[einbinder, 2016-08-26 17:46:21]

Ptal. Two to three patches left! This one has the interface around the CMTE so
it can be lazy loaded.

[dgozman, 2016-08-26 19:03:25]

Looks pretty good!

https://codereview.chromium.org/2281703002/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/text_editor/CodeMirrorTextEditor.js
(right):

https://codereview.chromium.org/2281703002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/text_editor/CodeMirrorTextEditor.js:125:
WebInspector.moduleSetting(options.bracketMatchingFlag).addChangeListener(this._enableBracketMatchingIfNeeded,
this);
Pass a WI.Setting instance directly.

https://codereview.chromium.org/2281703002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/text_editor/CodeMirrorTextEditor.js:157:
if (options.additionalClass)
Can we add this class on the widget's element instead? This way we don't have to
pass it.
Or maybe it should be a flag? What does this class affect?

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

https://codereview.chromium.org/2281703002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/TextEditor.js:30: range:
function(){ },
fullRange?

https://codereview.chromium.org/2281703002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/TextEditor.js:62: onKeyDown:
function(handler) { },
addKeyDownHandler

https://codereview.chromium.org/2281703002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/TextEditor.js:69:
clearAutocomplete: function() { }
What is this one?

https://codereview.chromium.org/2281703002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/TextEditor.js:74: * 
bracketMatchingFlag: (string|undefined),
What does this string mean? Is it actually an enum?

https://codereview.chromium.org/2281703002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/TextEditor.js:85: *    
suggestionsCallback: ((function(!WebInspector.TextRange,
!WebInspector.TextRange):?Promise.<!Array.<{title: string, className:
(string|undefined)}>>)|undefined),
Why not use SuggestBox.Suggestions (or how it's called now)?

[einbinder, 2016-08-26 23:05:33]

ptal

https://codereview.chromium.org/2281703002/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/text_editor/CodeMirrorTextEditor.js
(right):

https://codereview.chromium.org/2281703002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/text_editor/CodeMirrorTextEditor.js:125:
WebInspector.moduleSetting(options.bracketMatchingFlag).addChangeListener(this._enableBracketMatchingIfNeeded,
this);
On 2016/08/26 at 19:03:24, dgozman wrote:
> Pass a WI.Setting instance directly.

Done.

https://codereview.chromium.org/2281703002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/text_editor/CodeMirrorTextEditor.js:157:
if (options.additionalClass)
On 2016/08/26 at 19:03:24, dgozman wrote:
> Can we add this class on the widget's element instead? This way we don't have
to pass it.
> Or maybe it should be a flag? What does this class affect?

Yeah, we should be able to add it to the surrounding class.

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

https://codereview.chromium.org/2281703002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/TextEditor.js:30: range:
function(){ },
On 2016/08/26 at 19:03:24, dgozman wrote:
> fullRange?

Done.

https://codereview.chromium.org/2281703002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/TextEditor.js:62: onKeyDown:
function(handler) { },
On 2016/08/26 at 19:03:24, dgozman wrote:
> addKeyDownHandler

Done.

https://codereview.chromium.org/2281703002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/TextEditor.js:69:
clearAutocomplete: function() { }
On 2016/08/26 at 19:03:25, dgozman wrote:
> What is this one?

It clears the autocomplete, renamed from finishAutocomplete. Things that modify
a text editor often want to hide the suggestbox beforehand.

https://codereview.chromium.org/2281703002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/TextEditor.js:74: * 
bracketMatchingFlag: (string|undefined),
On 2016/08/26 at 19:03:24, dgozman wrote:
> What does this string mean? Is it actually an enum?

Replaced above with WI.Setting.

https://codereview.chromium.org/2281703002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/TextEditor.js:85: *    
suggestionsCallback: ((function(!WebInspector.TextRange,
!WebInspector.TextRange):?Promise.<!Array.<{title: string, className:
(string|undefined)}>>)|undefined),
On 2016/08/26 at 19:03:24, dgozman wrote:
> Why not use SuggestBox.Suggestions (or how it's called now)?

Closure yelled at me a lot and I gave up trying to use the typedef.

[lushnikov, 2016-08-26 23:11:52]

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

https://codereview.chromium.org/2281703002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/TextEditor.js:51: setText:
function(text) { },
nit: we can make this accept a range as a second optional parameter

[einbinder, 2016-08-27 02:20:04]

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

https://codereview.chromium.org/2281703002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/TextEditor.js:51: setText:
function(text) { },
On 2016/08/26 at 23:11:52, lushnikov wrote:
> nit: we can make this accept a range as a second optional parameter

The editRange and setText functions seem similar to the user, but they have very
different side effects. For example, setText refreshes all the whitespace
settings and editRange fires an onTextChanged event to delegates.

[dgozman, 2016-08-29 15:40:27]

lgtm

https://codereview.chromium.org/2281703002/diff/80001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/text_editor/CodeMirrorTextEditor.js
(left):

https://codereview.chromium.org/2281703002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/text_editor/CodeMirrorTextEditor.js:48:
gutters: ["CodeMirror-linenumbers"],
Where did the gutters go?

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

https://codereview.chromium.org/2281703002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/text_editor/CodeMirrorTextEditor.js:1690:
WebInspector.CodeMirrorTextEditorFactory = function(){ }
{} on separate lines

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

https://codereview.chromium.org/2281703002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/TextEditor.js:4:
WebInspector.TextEditorFactory = function() { }
{} on separate lines

https://codereview.chromium.org/2281703002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/TextEditor.js:20:
WebInspector.TextEditor.prototype = {
empty line before this one

[einbinder, 2016-08-29 18:06:18]

https://codereview.chromium.org/2281703002/diff/80001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/text_editor/CodeMirrorTextEditor.js
(left):

https://codereview.chromium.org/2281703002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/text_editor/CodeMirrorTextEditor.js:48:
gutters: ["CodeMirror-linenumbers"],
On 2016/08/29 at 15:40:26, dgozman wrote:
> Where did the gutters go?

this._gutters is moved to SourcesTextEditor. ["CodeMirror-linenumbers"] is the
default value, but I added an explicit call setting it in SourcesTextEditor in
case in changes.

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

https://codereview.chromium.org/2281703002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/text_editor/CodeMirrorTextEditor.js:1690:
WebInspector.CodeMirrorTextEditorFactory = function(){ }
On 2016/08/29 at 15:40:26, dgozman wrote:
> {} on separate lines

Done.

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

https://codereview.chromium.org/2281703002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/TextEditor.js:4:
WebInspector.TextEditorFactory = function() { }
On 2016/08/29 at 15:40:26, dgozman wrote:
> {} on separate lines

Done.

https://codereview.chromium.org/2281703002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/ui/TextEditor.js:20:
WebInspector.TextEditor.prototype = {
On 2016/08/29 at 15:40:26, dgozman wrote:
> empty line before this one

Done.

[einbinder, 2016-08-29 19:06:04]

The CQ bit was checked by einbinder@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-29 19:47:13]

Dry run: 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-08-30 03:37:39]

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

[commit-bot: I haz the power, 2016-08-30 03:37:40]

Dry run: This issue passed the CQ dry run.

[lushnikov, 2016-09-06 19:53:09]

are we going to land this?

[einbinder, 2016-09-06 20:58:14]

On 2016/09/06 at 19:53:09, lushnikov wrote:
> are we going to land this?

Oh I was waiting on you. Ok.

[einbinder, 2016-09-06 20:58:21]

The CQ bit was checked by einbinder@chromium.org

[einbinder, 2016-09-06 20:58:22]

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

[commit-bot: I haz the power, 2016-09-06 20:58:50]

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-06 21:03:15]

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

[commit-bot: I haz the power, 2016-09-06 21:03:17]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[einbinder, 2016-09-06 21:59:25]

The CQ bit was checked by einbinder@chromium.org

[einbinder, 2016-09-06 21:59:26]

The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org
Link to the patchset: https://codereview.chromium.org/2281703002/#ps120001
(title: "Merge")

[einbinder, 2016-09-06 21:59:37]

The CQ bit was unchecked by einbinder@chromium.org

[einbinder, 2016-09-06 22:02:06]

The CQ bit was checked by einbinder@chromium.org

[einbinder, 2016-09-06 22:02:06]

The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org
Link to the patchset: https://codereview.chromium.org/2281703002/#ps140001
(title: "Remvoe accidental devtools.gypi")

[commit-bot: I haz the power, 2016-09-06 22:02:48]

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

[lushnikov, 2016-09-06 22:30:53]

oh ok, lgtm

[commit-bot: I haz the power, 2016-09-06 23:34:49]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-09-06 23:37:07]

Description was changed from

==========
DevTools: Create TextEditor Interface around CodeMirrorTextEditor
This is for lazy loading of the text_editor module in preperation of
CodeMirror-based console.

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

to

==========
DevTools: Create TextEditor Interface around CodeMirrorTextEditor
This is for lazy loading of the text_editor module in preperation of
CodeMirror-based console.

BUG=607690

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

[commit-bot: I haz the power, 2016-09-06 23:37:09]

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