Add access to AST and element model data to the status pages

Add access to AST ane element model data to the status pages

R=devoncarew@google.com

Committed: https://github.com/dart-lang/sdk/commit/c0617d20158955d99d6447036237fe2639ba088c

[Brian Wilkerson, 2017-06-10 18:40:49]

brianwilkerson@google.com changed reviewers:
+ devoncarew@google.com

[devoncarew, 2017-06-11 06:13:30]

lgtm

lgtm!

Several comments, mostly style related re: using sentences for the page
descriptions, and that the blankslate() element is more a 'there are none or
these' element than an error condition.

https://codereview.chromium.org/2928033006/diff/1/pkg/analysis_server/lib/src...
File pkg/analysis_server/lib/src/status/diagnostics.dart (right):

https://codereview.chromium.org/2928033006/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/status/diagnostics.dart:138: : super(site, 'ast',
'AST', description: 'The AST for a file');
We should end this in a period - the other descriptions are full sentences.

https://codereview.chromium.org/2928033006/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/status/diagnostics.dart:147: _description = 'The AST
for $path';
Same re: ending in a period.

If the file is unspecified, perhaps just null out _description?

https://codereview.chromium.org/2928033006/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/status/diagnostics.dart:158: blankslate('No file
path provided.');
I'd suggest something like:

p('No file path provided.');

blankslate() means more something like 'there are none of these items' rather
than some type of error condition.

http://primercss.io/blankslate/

https://codereview.chromium.org/2928033006/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/status/diagnostics.dart:163: blankslate('The file
"$path" is not being analyzed.');
Same, I'd recommend a p() element instead of blankslate(). The OverlaysPage
class has similar code, where it presents some error conditions for a file
param. Here's how it presents its not found message:

p('<code>${escape(overlayPath)}</code> not found.', raw: true);

https://codereview.chromium.org/2928033006/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/status/diagnostics.dart:168: blankslate('An AST
could not be produced for the file "$path".');
p()?

https://codereview.chromium.org/2928033006/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/status/diagnostics.dart:211: secondaryPages.add(new
AstPage(this));
I don't have a strong opinion here, but another way to accomplish this would
have been to just keep a single list of pages, and change the logic in
DiagnosticPageWithNav re: how it decides what pages show up in the nav on the
left. Currently it does a subclass check, but it change that to allow pages to
opt-out of the nav.

A list of pages and secondaryPages works as well :)

https://codereview.chromium.org/2928033006/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/status/diagnostics.dart:348: description: 'The
element model for a file');
nit: end in period

https://codereview.chromium.org/2928033006/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/status/diagnostics.dart:356: String path =
params['file'] ?? 'unspecified file';
when unspecified, null out _description?

[Brian Wilkerson, 2017-06-11 16:44:19]

Description was changed from

==========
Add access to AST ane element model data to the status pages
==========

to

==========
Add access to AST ane element model data to the status pages

R=devoncarew@google.com

Committed:
https://github.com/dart-lang/sdk/commit/c0617d20158955d99d6447036237fe2639ba088c
==========

[Brian Wilkerson, 2017-06-11 16:44:21]

Committed patchset #1 (id:1) manually as
c0617d20158955d99d6447036237fe2639ba088c (presubmit successful).

[Brian Wilkerson, 2017-06-11 16:45:25]

Thanks for the feedback. I didn't know about blankslate so I just made a guess.

https://codereview.chromium.org/2928033006/diff/1/pkg/analysis_server/lib/src...
File pkg/analysis_server/lib/src/status/diagnostics.dart (right):

https://codereview.chromium.org/2928033006/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/status/diagnostics.dart:138: : super(site, 'ast',
'AST', description: 'The AST for a file');
Done

https://codereview.chromium.org/2928033006/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/status/diagnostics.dart:147: _description = 'The AST
for $path';
Done

https://codereview.chromium.org/2928033006/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/status/diagnostics.dart:158: blankslate('No file
path provided.');
Done

https://codereview.chromium.org/2928033006/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/status/diagnostics.dart:163: blankslate('The file
"$path" is not being analyzed.');
Done

https://codereview.chromium.org/2928033006/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/status/diagnostics.dart:168: blankslate('An AST
could not be produced for the file "$path".');
Done

https://codereview.chromium.org/2928033006/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/status/diagnostics.dart:211: secondaryPages.add(new
AstPage(this));
I think I'll leave it for now, because it's working, but the other might be
better because it would reduce code duplication.

https://codereview.chromium.org/2928033006/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/status/diagnostics.dart:348: description: 'The
element model for a file');
Done

https://codereview.chromium.org/2928033006/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/status/diagnostics.dart:356: String path =
params['file'] ?? 'unspecified file';
Done