Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(60)

Issue 1243893002: Separate the API's used by ContextManager. (Closed)

Created:
5 years, 5 months ago by Paul Berry
Modified:
5 years, 5 months ago
Reviewers:
Brian Wilkerson
CC:
reviews_dartlang.org
Base URL:
git@github.com:dart-lang/sdk.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Separate the API's used by ContextManager. Previously, ContextManager had two API's: one used by analysis server to tell the ContextManager what to do, and one used by the ContextManager to make callbacks to analysis server in response to its requests. The first API was implemented in the class AbstractContextManager, which derived from ContextManager, and the second API was implemented in ServerContextManager, which derived from AbstractContextManager. In addition to causing confusion, this made it impossible to provide an alternate implementation of ContextManager as a plug-in, since the plug in would have had to re-implement the second API, and that would have required accessing private implementation details of the ContextManager. This CL separates the API's: the first API is specified in ContextManager and implemented in ContextManagerImpl, and the second API is specified in ContextManagerCallbacks and implemented in ServerContextManagerCallbacks. In the long run I hope to eliminate the ContextManagerCallbacks class entirely, by having the ContextManager tell its client what to do using return values rather than callbacks. R=brianwilkerson@google.com Committed: https://github.com/dart-lang/sdk/commit/128dec84b5528d268cb25d622a3ea0dd5a9332e9

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+473 lines, -459 lines) Patch
M pkg/analysis_server/lib/src/analysis_server.dart View 9 chunks +28 lines, -34 lines 2 comments Download
M pkg/analysis_server/lib/src/context_manager.dart View 22 chunks +172 lines, -154 lines 0 comments Download
M pkg/analysis_server/lib/src/get_handler.dart View 1 chunk +2 lines, -1 line 0 comments Download
M pkg/analysis_server/lib/src/status/get_handler.dart View 1 chunk +2 lines, -1 line 0 comments Download
M pkg/analysis_server/test/analysis/get_errors_test.dart View 1 chunk +1 line, -1 line 0 comments Download
M pkg/analysis_server/test/analysis/get_navigation_test.dart View 1 chunk +1 line, -1 line 0 comments Download
M pkg/analysis_server/test/context_manager_test.dart View 59 chunks +263 lines, -263 lines 0 comments Download
M pkg/analysis_server/test/domain_execution_test.dart View 2 chunks +2 lines, -1 line 0 comments Download
M pkg/analysis_server/test/operation/operation_queue_test.dart View 3 chunks +2 lines, -3 lines 0 comments Download

Messages

Total messages: 5 (1 generated)
Paul Berry
5 years, 5 months ago (2015-07-20 22:56:13 UTC) #2
Brian Wilkerson
LGTM https://codereview.chromium.org/1243893002/diff/1/pkg/analysis_server/lib/src/analysis_server.dart File pkg/analysis_server/lib/src/analysis_server.dart (right): https://codereview.chromium.org/1243893002/diff/1/pkg/analysis_server/lib/src/analysis_server.dart#newcode355 pkg/analysis_server/lib/src/analysis_server.dart:355: (contextManager.callbacks as ServerContextManagerCallbacks).onContextsChanged; Do we plan on eventually ...
5 years, 5 months ago (2015-07-21 13:56:44 UTC) #3
Paul Berry
https://codereview.chromium.org/1243893002/diff/1/pkg/analysis_server/lib/src/analysis_server.dart File pkg/analysis_server/lib/src/analysis_server.dart (right): https://codereview.chromium.org/1243893002/diff/1/pkg/analysis_server/lib/src/analysis_server.dart#newcode355 pkg/analysis_server/lib/src/analysis_server.dart:355: (contextManager.callbacks as ServerContextManagerCallbacks).onContextsChanged; On 2015/07/21 13:56:44, Brian Wilkerson wrote: ...
5 years, 5 months ago (2015-07-21 17:22:21 UTC) #4
Paul Berry
5 years, 5 months ago (2015-07-21 17:41:01 UTC) #5
Message was sent while issue was closed.
Committed patchset #1 (id:1) manually as
128dec84b5528d268cb25d622a3ea0dd5a9332e9 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698