Build script views programmatically.

Build script views programmatically.

Preparation for displaying source annotations attached to locations more specific than a line number.

R=johnmccutchan@google.com

Committed: https://code.google.com/p/dart/source/detail?r=44140

[rmacnak, 2015-02-27 01:35:17]

rmacnak@google.com changed reviewers:
+ johnmccutchan@google.com

[rmacnak, 2015-02-27 01:35:17]

https://codereview.chromium.org/959043003/diff/60001/runtime/observatory/lib/...
File runtime/observatory/lib/src/elements/script_view.dart (left):

https://codereview.chromium.org/959043003/diff/60001/runtime/observatory/lib/...
runtime/observatory/lib/src/elements/script_view.dart:20: void attached() {
Redundant.

[Cutch, 2015-02-27 14:44:34]

You seemed to have removed the block of empty lines being replaced by ellipses,
let's get that back in.

https://codereview.chromium.org/959043003/diff/60001/runtime/observatory/lib/...
File runtime/observatory/lib/src/elements/script_inset.dart (right):

https://codereview.chromium.org/959043003/diff/60001/runtime/observatory/lib/...
runtime/observatory/lib/src/elements/script_inset.dart:24: print("Annotated
${element.text} as $title");
Remove before committing.

https://codereview.chromium.org/959043003/diff/60001/runtime/observatory/lib/...
runtime/observatory/lib/src/elements/script_inset.dart:122: if (script == null)
return;
curlies- if ( ... ) { return; }

should you ensure the script children are cleared?

https://codereview.chromium.org/959043003/diff/60001/runtime/observatory/lib/...
runtime/observatory/lib/src/elements/script_inset.dart:167:
table.style.fontFamily = "monospace";
use css classes defined in html instead of setting style properties from code.

https://codereview.chromium.org/959043003/diff/60001/runtime/observatory/lib/...
runtime/observatory/lib/src/elements/script_inset.dart:215: if (line.hits ==
null) hitsUnknown(node);
braces and new lines please

[rmacnak, 2015-02-27 20:55:34]

Restored ellipses for consecutive blank lines. Added cache test.

https://codereview.chromium.org/959043003/diff/60001/runtime/observatory/lib/...
File runtime/observatory/lib/src/elements/script_inset.dart (right):

https://codereview.chromium.org/959043003/diff/60001/runtime/observatory/lib/...
runtime/observatory/lib/src/elements/script_inset.dart:24: print("Annotated
${element.text} as $title");
On 2015/02/27 14:44:33, Cutch wrote:
> Remove before committing.

Done.

https://codereview.chromium.org/959043003/diff/60001/runtime/observatory/lib/...
runtime/observatory/lib/src/elements/script_inset.dart:122: if (script == null)
return;
On 2015/02/27 14:44:33, Cutch wrote:
> curlies- if ( ... ) { return; }

Done.
 
> should you ensure the script children are cleared?

I don't follow. The whole subtree is swapped at the end.

https://codereview.chromium.org/959043003/diff/60001/runtime/observatory/lib/...
runtime/observatory/lib/src/elements/script_inset.dart:167:
table.style.fontFamily = "monospace";
On 2015/02/27 14:44:33, Cutch wrote:
> use css classes defined in html instead of setting style properties from code.

Done.

https://codereview.chromium.org/959043003/diff/60001/runtime/observatory/lib/...
runtime/observatory/lib/src/elements/script_inset.dart:215: if (line.hits ==
null) hitsUnknown(node);
On 2015/02/27 14:44:33, Cutch wrote:
> braces and new lines please

Done.

[Cutch, 2015-02-28 00:37:04]

https://codereview.chromium.org/959043003/diff/80001/runtime/observatory/lib/...
File runtime/observatory/lib/src/elements/script_inset.dart (right):

https://codereview.chromium.org/959043003/diff/80001/runtime/observatory/lib/...
runtime/observatory/lib/src/elements/script_inset.dart:111: return;
In the case that a script was not null and then replaced with the null script,
the UI should be cleared before returning from the function.

https://codereview.chromium.org/959043003/diff/80001/runtime/observatory/lib/...
runtime/observatory/lib/src/elements/script_inset.dart:153: a.title = "Current
invocation";
currentPos doesn't imply invocation. It is just the token position that the
script_inset is asked to focus on.

https://codereview.chromium.org/959043003/diff/80001/runtime/observatory/lib/...
runtime/observatory/lib/src/elements/script_inset.dart:193: return table;
You no longer update linesReady- why?

https://codereview.chromium.org/959043003/diff/80001/runtime/observatory/lib/...
File runtime/observatory/lib/src/service/object.dart (right):

https://codereview.chromium.org/959043003/diff/80001/runtime/observatory/lib/...
runtime/observatory/lib/src/service/object.dart:2111: notifyChange(null);
Add a comment about what this does.

[rmacnak, 2015-03-02 19:08:26]

https://codereview.chromium.org/959043003/diff/80001/runtime/observatory/lib/...
File runtime/observatory/lib/src/elements/script_inset.dart (right):

https://codereview.chromium.org/959043003/diff/80001/runtime/observatory/lib/...
runtime/observatory/lib/src/elements/script_inset.dart:111: return;
On 2015/02/28 00:37:04, Cutch wrote:
> In the case that a script was not null and then replaced with the null script,
> the UI should be cleared before returning from the function.

Done.

https://codereview.chromium.org/959043003/diff/80001/runtime/observatory/lib/...
runtime/observatory/lib/src/elements/script_inset.dart:153: a.title = "Current
invocation";
On 2015/02/28 00:37:04, Cutch wrote:
> currentPos doesn't imply invocation. It is just the token position that the
> script_inset is asked to focus on.

We should replace use currentPos with specific annotations in a future CL.
Changed to "Point of interest" for now.

https://codereview.chromium.org/959043003/diff/80001/runtime/observatory/lib/...
runtime/observatory/lib/src/elements/script_inset.dart:193: return table;
On 2015/02/28 00:37:04, Cutch wrote:
> You no longer update linesReady- why?

Unused. Removed the field.

https://codereview.chromium.org/959043003/diff/80001/runtime/observatory/lib/...
File runtime/observatory/lib/src/service/object.dart (right):

https://codereview.chromium.org/959043003/diff/80001/runtime/observatory/lib/...
runtime/observatory/lib/src/service/object.dart:2111: notifyChange(null);
On 2015/02/28 00:37:04, Cutch wrote:
> Add a comment about what this does.

Done.

[Cutch, 2015-03-02 19:11:45]

LGTMwC

https://codereview.chromium.org/959043003/diff/120001/runtime/observatory/lib...
File runtime/observatory/lib/src/elements/script_inset.dart (right):

https://codereview.chromium.org/959043003/diff/120001/runtime/observatory/lib...
runtime/observatory/lib/src/elements/script_inset.dart:253: if (stop <=
position) return;  // Empty gap between annotations/boundries.
column limit

[Cutch, 2015-03-02 19:13:24]

lgtm

[rmacnak, 2015-03-02 19:20:01]

Committed patchset #8 (id:140001) manually as 44140 (presubmit successful).