DevTools: Split off SourcesTextEditor from CodeMirrorTextEditor

DevTools: Split off SourcesTextEditor from CodeMirrorTextEditor
This is in preperation for CodeMirror-based console

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

[einbinder, 2016-08-12 18:16:32]

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

[einbinder, 2016-08-12 18:16:32]

Ptal

[dgozman, 2016-08-15 23:16:06]

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

[dgozman, 2016-08-15 23:16:07]

https://codereview.chromium.org/2238883004/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/source_frame/CodeMirrorTextEditor.js
(left):

https://codereview.chromium.org/2238883004/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/source_frame/CodeMirrorTextEditor.js:590:
WebInspector.moduleSetting("showWhitespacesInEditor").removeChangeListener(this._updateCodeMirrorMode,
this);
All the settings should be in SourcesTextEditor.

https://codereview.chromium.org/2238883004/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/source_frame/CodeMirrorTextEditor.js
(right):

https://codereview.chromium.org/2238883004/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/source_frame/CodeMirrorTextEditor.js:860:
scrollIntoView: function(lineNumber)
scrollLineIntoView

https://codereview.chromium.org/2238883004/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/source_frame/SourcesTextEditor.js
(right):

https://codereview.chromium.org/2238883004/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/source_frame/SourcesTextEditor.js:1:
/*
New-style copyright.

https://codereview.chromium.org/2238883004/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/source_frame/SourcesTextEditor.js:64:
this.element.addEventListener("mousedown", updateAnticipateJumpFlag.bind(this,
true), true);
style: empty lines please.

[einbinder, 2016-08-16 19:00:59]

ptal

https://codereview.chromium.org/2238883004/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/source_frame/CodeMirrorTextEditor.js
(left):

https://codereview.chromium.org/2238883004/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/source_frame/CodeMirrorTextEditor.js:590:
WebInspector.moduleSetting("showWhitespacesInEditor").removeChangeListener(this._updateCodeMirrorMode,
this);
On 2016/08/15 at 23:16:07, dgozman wrote:
> All the settings should be in SourcesTextEditor.

I moved whitespaces, but bracket matching will be shared by the console (with
its own setting flag) in another patch.

https://codereview.chromium.org/2238883004/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/source_frame/CodeMirrorTextEditor.js
(right):

https://codereview.chromium.org/2238883004/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/source_frame/CodeMirrorTextEditor.js:860:
scrollIntoView: function(lineNumber)
On 2016/08/15 at 23:16:07, dgozman wrote:
> scrollLineIntoView

Done.

https://codereview.chromium.org/2238883004/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/source_frame/SourcesTextEditor.js
(right):

https://codereview.chromium.org/2238883004/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/source_frame/SourcesTextEditor.js:1:
/*
On 2016/08/15 at 23:16:07, dgozman wrote:
> New-style copyright.

Done.

https://codereview.chromium.org/2238883004/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/source_frame/SourcesTextEditor.js:64:
this.element.addEventListener("mousedown", updateAnticipateJumpFlag.bind(this,
true), true);
On 2016/08/15 at 23:16:07, dgozman wrote:
> style: empty lines please.

Done.

[lushnikov, 2016-08-18 21:43:03]

That's a huge patch! Let's make it in a few steps?

https://codereview.chromium.org/2238883004/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/source_frame/CodeMirrorTextEditor.js
(left):

https://codereview.chromium.org/2238883004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/source_frame/CodeMirrorTextEditor.js:1351:
editRange: function(range, text, origin)
this method makes sense for CMTE - it's extremely general

https://codereview.chromium.org/2238883004/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/source_frame/CodeMirrorTextEditor.js
(right):

https://codereview.chromium.org/2238883004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/source_frame/CodeMirrorTextEditor.js:74:
"Esc": "dismiss",
why this change?

https://codereview.chromium.org/2238883004/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/source_frame/SourcesTextEditor.js
(right):

https://codereview.chromium.org/2238883004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/source_frame/SourcesTextEditor.js:14:
this._codeMirror.addKeyMap({
we should either codeMirror() all over the place; let's also mark it as
@protected

https://codereview.chromium.org/2238883004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/source_frame/SourcesTextEditor.js:35:
this._elementToWidget = new Map();
this has to be rebased atop of your previous CM change

[einbinder, 2016-08-18 22:52:23]

ptal

https://codereview.chromium.org/2238883004/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/source_frame/CodeMirrorTextEditor.js
(left):

https://codereview.chromium.org/2238883004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/source_frame/CodeMirrorTextEditor.js:1351:
editRange: function(range, text, origin)
On 2016/08/18 at 21:43:03, lushnikov wrote:
> this method makes sense for CMTE - it's extremely general

Done.

https://codereview.chromium.org/2238883004/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/source_frame/CodeMirrorTextEditor.js
(right):

https://codereview.chromium.org/2238883004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/source_frame/CodeMirrorTextEditor.js:74:
"Esc": "dismiss",
On 2016/08/18 at 21:43:02, lushnikov wrote:
> why this change?

I was confused when I saw this function name, because it does a lot more that
just dismiss multiple selections.

https://codereview.chromium.org/2238883004/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/front_end/source_frame/SourcesTextEditor.js
(right):

https://codereview.chromium.org/2238883004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/source_frame/SourcesTextEditor.js:14:
this._codeMirror.addKeyMap({
On 2016/08/18 at 21:43:03, lushnikov wrote:
> we should either codeMirror() all over the place; let's also mark it as
@protected

It is protected, I just missed one _codeMirror.

https://codereview.chromium.org/2238883004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/source_frame/SourcesTextEditor.js:35:
this._elementToWidget = new Map();
On 2016/08/18 at 21:43:03, lushnikov wrote:
> this has to be rebased atop of your previous CM change

Yep, I'm waiting for it to land.

[lushnikov, 2016-08-22 19:47:04]

Let's land this in two steps:
1. Make all the changes in the code you'd like to make
2. Make a split, which does only code moves

https://codereview.chromium.org/2238883004/diff/100001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/inspector/editor/editor-test.js (right):

https://codereview.chromium.org/2238883004/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector/editor/editor-test.js:6: var textEditor
= new WebInspector.SourcesTextEditor(textEditorDelegate || new
WebInspector.SourcesTextEditorDelegate());
hackish, but works

https://codereview.chromium.org/2238883004/diff/100001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/inspector/editor/text-editor-indent-autodetection.html
(right):

https://codereview.chromium.org/2238883004/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector/editor/text-editor-indent-autodetection.html:28:
var indent = textEditor._indent();
why this change?

[einbinder, 2016-08-23 18:21:49]

Ptal. I merged with the previous patch that had the CMTE changes.

https://codereview.chromium.org/2238883004/diff/100001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/inspector/editor/text-editor-indent-autodetection.html
(right):

https://codereview.chromium.org/2238883004/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector/editor/text-editor-indent-autodetection.html:28:
var indent = textEditor._indent();
On 2016/08/22 at 19:47:04, lushnikov wrote:
> why this change?

It wasn't used externally except in tests, but I moved it back.

[lushnikov, 2016-08-23 21:05:19]

lgtm

https://codereview.chromium.org/2238883004/diff/120001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/source_frame/CodeMirrorTextEditor.js
(left):

https://codereview.chromium.org/2238883004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/source_frame/CodeMirrorTextEditor.js:1692:
this._setEditorIndentation(text.split("\n").slice(0,
WebInspector.CodeMirrorTextEditor.LinesToScanForIndentationGuessing));
this was in-between of setText on purpose - setting options on empty editor
works
way faster. (@see git-blame for details). Let's make sure we don't regress this
behavior.

https://codereview.chromium.org/2238883004/diff/120001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/source_frame/SourcesTextEditor.js
(right):

https://codereview.chromium.org/2238883004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/source_frame/SourcesTextEditor.js:53:

stray line

[einbinder, 2016-08-23 21:08:58]

https://codereview.chromium.org/2238883004/diff/120001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/source_frame/CodeMirrorTextEditor.js
(left):

https://codereview.chromium.org/2238883004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/source_frame/CodeMirrorTextEditor.js:1692:
this._setEditorIndentation(text.split("\n").slice(0,
WebInspector.CodeMirrorTextEditor.LinesToScanForIndentationGuessing));
On 2016/08/23 at 21:05:19, lushnikov wrote:
> this was in-between of setText on purpose - setting options on empty editor
works
> way faster. (@see git-blame for details). Let's make sure we don't regress
this behavior.

All good.

https://codereview.chromium.org/2238883004/diff/120001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/front_end/source_frame/SourcesTextEditor.js
(right):

https://codereview.chromium.org/2238883004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/source_frame/SourcesTextEditor.js:53:

On 2016/08/23 at 21:05:19, lushnikov wrote:
> stray line

Done.

[einbinder, 2016-08-23 21:09:01]

The CQ bit was checked by einbinder@chromium.org

[einbinder, 2016-08-23 21:09:02]

The patchset sent to the CQ was uploaded after l-g-t-m from
lushnikov@chromium.org
Link to the patchset: https://codereview.chromium.org/2238883004/#ps140001
(title: "Stray line")

[commit-bot: I haz the power, 2016-08-23 21:10:06]

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-23 22:46:36]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-08-23 22:48:37]

Description was changed from

==========
DevTools: Split off SourcesTextEditor from CodeMirrorTextEditor
This is in preperation for CodeMirror-based console

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

to

==========
DevTools: Split off SourcesTextEditor from CodeMirrorTextEditor
This is in preperation for CodeMirror-based console

BUG=607690

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

[commit-bot: I haz the power, 2016-08-23 22:48:38]

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