Implement analysis.updateOptions

pkg/analysis_server/lib/src/analysis_server.dart

Index: pkg/analysis_server/lib/src/analysis_server.dart
diff --git a/pkg/analysis_server/lib/src/analysis_server.dart b/pkg/analysis_server/lib/src/analysis_server.dart
index 6e7f35d45d2812fcc38d8e7333e4eba6b3449dbc..5fc76c81fbe380e47cf8319bc244560fdc301829 100644
--- a/pkg/analysis_server/lib/src/analysis_server.dart
+++ b/pkg/analysis_server/lib/src/analysis_server.dart
@@ -210,6 +210,23 @@ class AnalysisServer {
   }
 
   /**
+   * Use the given updaters to update the values of the options in every
+   * existing analysis context.
+   */
+  void updateOptions(List<OptionUpdater> optionUpdaters) {

[Paul Berry, 2014/06/17 20:40:54]

Passing in a list of closures seems like overkill to me.  Why not just maintain
a single AnalysisOptionsImpl object with the current set of options, and when we
receive an analysis.updateOptions request, update it and then propagate it to
each analysisContext?

[Brian Wilkerson, 2014/06/17 21:30:12]

That was my first thought as well. Unfortunately, AnalysisOptionsImpl doesn't
allow for specifying a partial set of options, so I couldn't tell whether an
option was implicitly or explicitly set. That meant that every option that
wasn't suppose to be updated was reset to factory defaults.

[Paul Berry, 2014/06/17 22:24:56]

Actually I was thinking of something along these lines:

AnalysisServer:
- add field "AnalysisOptionsImpl currentOptions;"
- add method "updateOptions()", which does "context.analysisOptions =
currentOptions;" for all contexts.

AnalysisServerContextDirectoryManager.addContext(...):
- Populate the new context's options using analysisServer.currentOptions

AnalysisDomainHandler.updateOptions(Request request):
- Modify server.currentOptions in place based on request
- Call server.updateOptions()

So there would be no need to tell which options were implicitly or explicitly
set--AnalysisDomainHandler.updateOptions() simply modifies those options that
are mentioned in the request and leaves the rest alone.

Your call, though.  The code as is looks like it implements the spec correctly.

[Brian Wilkerson, 2014/06/18 17:01:30]

Unfortunately, every context needs its own separate set of options so that we
can set different cache sizes, so we can't just replace all options.

+    // TODO(brianwilkerson) Figure out how to update the defaults for newly

[Paul Berry, 2014/06/17 20:40:54]

You should be able to take care of this in
AnalysisServerContextDirectoryManager.addContext(), after constructing the
ContextDirectory (which creates the context as a side effect) and before the
call to analysisServer.schedulePerformAnalysisOperation().

[Brian Wilkerson, 2014/06/17 21:30:12]

Thanks. Done, but not tested. This solved one testing question and raised one
more...

+    // create contexts.
+    folderMap.forEach((Folder folder, ContextDirectory directory) {
+      AnalysisContext context = directory.context;
+      AnalysisOptionsImpl options = new AnalysisOptionsImpl.con1(context.analysisOptions);
+      optionUpdaters.forEach((OptionUpdater optionUpdater) {
+        optionUpdater(options);
+      });
+      context.analysisOptions = options;
+    });
+  }
+
+  /**
    * Adds the given [ServerOperation] to the queue, but does not schedule
    * operations execution.
    */
@@ -540,6 +557,7 @@ class ContextDirectory {
   AnalysisContext get context => _context;
 }
 
+typedef void OptionUpdater(AnalysisOptionsImpl options);
 
 /**
  * An enumeration of the services provided by the server domain.