Integration of the new analysis driver, behind a flag.

Integration of the new analysis driver, behind a flag.

The flag is --enable-new-analysis-driver, disabled by default.

WIP, only error notifications for now.

R=brianwilkerson@google.com, paulberry@google.com
BUG=

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

[Brian Wilkerson, 2016-10-31 15:42:58]

lgtm, but I'd like to see the suggested changes made in a future CL (unless you
can convince me that they shouldn't be done at all).

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

https://codereview.chromium.org/2465923002/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/analysis_server.dart:319: final Map<Folder,
nd.AnalysisDriver> driverMap =
I would prefer to see this added to ContextManager so that when we get
ContextManager extracted and made sharable this support will be included.

https://codereview.chromium.org/2465923002/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/analysis_server.dart:1202: // TODO(scheglov)
implement for the new analysis driver
The formatting of the lines above the comment looks strange.

But the real question is: what's left to be implemented? Can you update the
comment to include that?

https://codereview.chromium.org/2465923002/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/analysis_server.dart:1695: //      {
Add a comment explaining why this code was left in?

https://codereview.chromium.org/2465923002/diff/1/pkg/analysis_server/lib/src...
File pkg/analysis_server/lib/src/context_manager.dart (right):

https://codereview.chromium.org/2465923002/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/context_manager.dart:333: AnalysisDriver
addAnalysisDriver(Folder folder, AnalysisOptions options);
I really want to get rid of ContextManagerCallbacks (as part of abstracting
ContextManager), so if there's any way to code this that doesn't involve using
this class, that would be better.

[scheglov, 2016-10-31 16:54:21]

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

https://codereview.chromium.org/2465923002/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/analysis_server.dart:319: final Map<Folder,
nd.AnalysisDriver> driverMap =
On 2016/10/31 15:42:58, Brian Wilkerson wrote:
> I would prefer to see this added to ContextManager so that when we get
> ContextManager extracted and made sharable this support will be included.

Done.

https://codereview.chromium.org/2465923002/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/analysis_server.dart:1202: // TODO(scheglov)
implement for the new analysis driver
On 2016/10/31 15:42:58, Brian Wilkerson wrote:
> The formatting of the lines above the comment looks strange.
> 
> But the real question is: what's left to be implemented? Can you update the
> comment to include that?

I think it's done.
I'm removing the comment.

https://codereview.chromium.org/2465923002/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/analysis_server.dart:1695: //      {
On 2016/10/31 15:42:58, Brian Wilkerson wrote:
> Add a comment explaining why this code was left in?

Done.

https://codereview.chromium.org/2465923002/diff/1/pkg/analysis_server/lib/src...
File pkg/analysis_server/lib/src/context_manager.dart (right):

https://codereview.chromium.org/2465923002/diff/1/pkg/analysis_server/lib/src...
pkg/analysis_server/lib/src/context_manager.dart:333: AnalysisDriver
addAnalysisDriver(Folder folder, AnalysisOptions options);
On 2016/10/31 15:42:58, Brian Wilkerson wrote:
> I really want to get rid of ContextManagerCallbacks (as part of abstracting
> ContextManager), so if there's any way to code this that doesn't involve using
> this class, that would be better.

I'm all for removing extra layers.
But in this CL I'm just following the existing ways.

[scheglov, 2016-10-31 16:54:36]

Description was changed from

==========
Integration of the new analysis driver, behind a flag.

The flag is --enable-new-analysis-driver, disabled by default.

WIP, only error notifications for now.

R=brianwilkerson@google.com, paulberry@google.com
BUG=
==========

to

==========
Integration of the new analysis driver, behind a flag.

The flag is --enable-new-analysis-driver, disabled by default.

WIP, only error notifications for now.

R=brianwilkerson@google.com, paulberry@google.com
BUG=

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

[scheglov, 2016-10-31 16:54:37]

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

[Paul Berry, 2016-10-31 20:09:39]

lgtm

https://codereview.chromium.org/2465923002/diff/1/pkg/analyzer/lib/src/dart/a...
File pkg/analyzer/lib/src/dart/analysis/driver.dart (right):

https://codereview.chromium.org/2465923002/diff/1/pkg/analyzer/lib/src/dart/a...
pkg/analyzer/lib/src/dart/analysis/driver.dart:228: await pumpEventQueue(100);
Can you add a comment explaining why this is needed, and how the number 100 was
chosen?

https://codereview.chromium.org/2465923002/diff/1/pkg/analyzer/lib/src/dart/a...
pkg/analyzer/lib/src/dart/analysis/driver.dart:718: Future pumpEventQueue([int
times = 5000]) {
Can we make this private?  It seems hacky to use it in non-test code and I don't
want calls to it to proliferate without us thinking really hard about it.

[scheglov, 2016-10-31 20:37:11]

https://codereview.chromium.org/2465923002/diff/1/pkg/analyzer/lib/src/dart/a...
File pkg/analyzer/lib/src/dart/analysis/driver.dart (right):

https://codereview.chromium.org/2465923002/diff/1/pkg/analyzer/lib/src/dart/a...
pkg/analyzer/lib/src/dart/analysis/driver.dart:228: await pumpEventQueue(100);
On 2016/10/31 20:09:39, Paul Berry wrote:
> Can you add a comment explaining why this is needed, and how the number 100
was
> chosen?

Done in https://codereview.chromium.org/2466713002