Devtools: API To set the red/yellow squiggles for a file via DI

API To set the red/yellow squiggles for a file via DI

Known internally as "LineMessage"s. Additionally, Extends them to
take start and end line/column sets, to prepare for a
future where the UI actually supports rendering multiline
squiggles.

This work was split from https://codereview.chromium.org/1264133002/

BUG=484261

[wes, 2015-09-23 00:23:35]

wwigham@gmail.com changed reviewers:
+ paulirish@chromium.org, pfeldman@chromium.org

[pfeldman, 2015-10-21 23:47:53]

https://codereview.chromium.org/1361863002/diff/1/Source/devtools/front_end/s...
File Source/devtools/front_end/source_frame/SourceFrame.js (right):

https://codereview.chromium.org/1361863002/diff/1/Source/devtools/front_end/s...
Source/devtools/front_end/source_frame/SourceFrame.js:609: setMessagesForSource:
function(messages) {
This requires JSDoc and { to be on the next line.

https://codereview.chromium.org/1361863002/diff/1/Source/devtools/front_end/s...
Source/devtools/front_end/source_frame/SourceFrame.js:708: this._start = {
We have WebInspector.TextRange that could be reused for that.

https://codereview.chromium.org/1361863002/diff/1/Source/devtools/front_end/s...
Source/devtools/front_end/source_frame/SourceFrame.js:736: var location = {line:
lineNumber, column: columnNumber};
Should this also be a range now?

https://codereview.chromium.org/1361863002/diff/1/Source/devtools/front_end/s...
Source/devtools/front_end/source_frame/SourceFrame.js:760: start: function() {
{ goes the next line.

https://codereview.chromium.org/1361863002/diff/1/Source/devtools/front_end/s...
Source/devtools/front_end/source_frame/SourceFrame.js:903: if (!start) {
drop {}

https://codereview.chromium.org/1361863002/diff/1/Source/devtools/front_end/s...
File Source/devtools/front_end/sources/SourcesPanel.js (right):

https://codereview.chromium.org/1361863002/diff/1/Source/devtools/front_end/s...
Source/devtools/front_end/sources/SourcesPanel.js:1377: if
(!(container.messages() instanceof Array))
There is a compile-time check for this.

https://codereview.chromium.org/1361863002/diff/1/Source/devtools/front_end/s...
Source/devtools/front_end/sources/SourcesPanel.js:1383:
frame.setMessagesForSource(container.messages().map(function(m){
Revealers are for revealing things, not for populating messages. You should also
not replace the messages, but rather add them. It seems like what you need is an
addMessage on the UISourceCode that would store message in the internal list and
dispatch corresponding message that SourceFrame would hear.

https://codereview.chromium.org/1361863002/diff/1/Source/devtools/front_end/w...
File Source/devtools/front_end/workspace/UISourceCode.js (right):

https://codereview.chromium.org/1361863002/diff/1/Source/devtools/front_end/w...
Source/devtools/front_end/workspace/UISourceCode.js:742: this._location =
location;
Range?

https://codereview.chromium.org/1361863002/diff/1/Source/devtools/front_end/w...
Source/devtools/front_end/workspace/UISourceCode.js:749: text: function() {
{ goes next line.

https://codereview.chromium.org/1361863002/diff/1/Source/devtools/front_end/w...
Source/devtools/front_end/workspace/UISourceCode.js:773:
WebInspector.UISourceCodeMessages = function(source, messages)
Looks like unusual data structure. Why would you store pointer to the source in
it?

[wes, 2015-10-23 04:51:01]

https://codereview.chromium.org/1361863002/diff/1/Source/devtools/front_end/s...
File Source/devtools/front_end/sources/SourcesPanel.js (right):

https://codereview.chromium.org/1361863002/diff/1/Source/devtools/front_end/s...
Source/devtools/front_end/sources/SourcesPanel.js:1383:
frame.setMessagesForSource(container.messages().map(function(m){
On 2015/10/21 23:47:53, pfeldman wrote:
> Revealers are for revealing things, not for populating messages. You should
also
> not replace the messages, but rather add them. It seems like what you need is
an
> addMessage on the UISourceCode that would store message in the internal list
and
> dispatch corresponding message that SourceFrame would hear.

I'd actually wavered on if squiggles should be stateful myself... initially I
had them event based and stateful, but it resulted in a lot of state management
(and a lot more API surface) just to say "clear out all the old ones and then
display this set of messages". Which is all it turns out I ever wanted to do.

But yeah, I could see it better propagated as an event - though I don't think
it's completely inappropriate to call it something reveal-able - the messages
are a lot like an overlay on the given file.

[wes, 2015-10-23 20:35:30]

https://codereview.chromium.org/1361863002/diff/1/Source/devtools/front_end/s...
File Source/devtools/front_end/source_frame/SourceFrame.js (right):

https://codereview.chromium.org/1361863002/diff/1/Source/devtools/front_end/s...
Source/devtools/front_end/source_frame/SourceFrame.js:609: setMessagesForSource:
function(messages) {
On 2015/10/21 23:47:53, pfeldman wrote:
> This requires JSDoc and { to be on the next line.

Acknowledged.

https://codereview.chromium.org/1361863002/diff/1/Source/devtools/front_end/s...
Source/devtools/front_end/source_frame/SourceFrame.js:708: this._start = {
On 2015/10/21 23:47:53, pfeldman wrote:
> We have WebInspector.TextRange that could be reused for that.

Acknowledged.

https://codereview.chromium.org/1361863002/diff/1/Source/devtools/front_end/s...
Source/devtools/front_end/source_frame/SourceFrame.js:736: var location = {line:
lineNumber, column: columnNumber};
On 2015/10/21 23:47:53, pfeldman wrote:
> Should this also be a range now?

Yep.

https://codereview.chromium.org/1361863002/diff/1/Source/devtools/front_end/s...
Source/devtools/front_end/source_frame/SourceFrame.js:760: start: function() {
On 2015/10/21 23:47:53, pfeldman wrote:
> { goes the next line.

Acknowledged.

https://codereview.chromium.org/1361863002/diff/1/Source/devtools/front_end/s...
Source/devtools/front_end/source_frame/SourceFrame.js:903: if (!start) {
On 2015/10/21 23:47:53, pfeldman wrote:
> drop {}

Acknowledged.

https://codereview.chromium.org/1361863002/diff/1/Source/devtools/front_end/s...
File Source/devtools/front_end/sources/SourcesPanel.js (right):

https://codereview.chromium.org/1361863002/diff/1/Source/devtools/front_end/s...
Source/devtools/front_end/sources/SourcesPanel.js:1377: if
(!(container.messages() instanceof Array))
On 2015/10/21 23:47:53, pfeldman wrote:
> There is a compile-time check for this.

How so? container is cast from Object to the data structure on the line before -
and other Revealers do type assertions, such as
WebInspector.LayersPanel.LayerTreeRevealer,
WebInspector.NetworkPanel.RequestRevealer,
WebInspector.ResourcesPanel.ResourceRevealer, and so on. Though I should
probably be asserting is that it's a WebInspector.UISourceCodeMessages, rather
than checking its members' return types.

https://codereview.chromium.org/1361863002/diff/1/Source/devtools/front_end/w...
File Source/devtools/front_end/workspace/UISourceCode.js (right):

https://codereview.chromium.org/1361863002/diff/1/Source/devtools/front_end/w...
Source/devtools/front_end/workspace/UISourceCode.js:742: this._location =
location;
On 2015/10/21 23:47:53, pfeldman wrote:
> Range?

Acknowledged.

https://codereview.chromium.org/1361863002/diff/1/Source/devtools/front_end/w...
Source/devtools/front_end/workspace/UISourceCode.js:749: text: function() {
On 2015/10/21 23:47:53, pfeldman wrote:
> { goes next line.

Acknowledged.

https://codereview.chromium.org/1361863002/diff/1/Source/devtools/front_end/w...
Source/devtools/front_end/workspace/UISourceCode.js:773:
WebInspector.UISourceCodeMessages = function(source, messages)
On 2015/10/21 23:47:53, pfeldman wrote:
> Looks like unusual data structure. Why would you store pointer to the source
in
> it?

The source is how the source frame can lookup the source's associated view on
which it can set and display the messages.