Analysis server connect fixes

pkg/analysis_server/lib/src/analysis_manager.dart

Index: pkg/analysis_server/lib/src/analysis_manager.dart
diff --git a/pkg/analysis_server/lib/src/analysis_manager.dart b/pkg/analysis_server/lib/src/analysis_manager.dart
index 44bd0a1e7211e8240aacb5122653d956bc696a5e..7dd69301d86f0b296d430b42d2c966bab12ba22f 100644
--- a/pkg/analysis_server/lib/src/analysis_manager.dart
+++ b/pkg/analysis_server/lib/src/analysis_manager.dart
@@ -15,7 +15,8 @@ class AnalysisManager {
   static const int PORT = 3333;
 
   /**
-   * The analysis server process being managed.
+   * The analysis server process being managed
+   * or [:null:] if managing an analysis server that was already running.

[Brian Wilkerson, 2014/02/22 16:57:06]

nit: We have been using backquotes (`) in the rest of the server code base.

[danrubel, 2014/02/24 17:39:24]

Done.

    */
   final Process process;
 
@@ -31,13 +32,22 @@ class AnalysisManager {
   static Future<AnalysisManager> start(String pathToServer) {
     // TODO dynamically allocate port and/or allow client to specify port
     return Process.start(Platform.executable, [pathToServer, "--port",
-        PORT.toString()]).then(_connect);
+        PORT.toString()]).then(_attach);

[Brian Wilkerson, 2014/02/22 16:57:06]

We should have a 'catchError' to report when the server could not be started.

[danrubel, 2014/02/24 17:39:24]

Done.

   }
 
   /**
-   * Open a connection to the analysis server.
+   * Open a connection to a running analysis server
    */
-  static Future<AnalysisManager> _connect(Process process) {
+  static Future<AnalysisManager> connect(String url) {
+    return WebSocket.connect(url)
+        .then((WebSocket socket) => new AnalysisManager(null, socket));
+  }
+
+  /**
+   * Attach this process to the newly launched analysis server process,
+   * and open a connection to the analysis server.
+   */
+  static Future<AnalysisManager> _attach(Process process) {

[Brian Wilkerson, 2014/02/22 16:57:06]

I think we also want to use the 'exitCode' getter in order to be notified when
the process exits.

[danrubel, 2014/02/24 17:39:24]

Done.

     var url = 'ws://${InternetAddress.LOOPBACK_IP_V4.address}:$PORT/';
     process.stderr.pipe(stderr);
     Stream out = process.stdout.transform(UTF8.decoder).asBroadcastStream();
@@ -68,8 +78,8 @@ class AnalysisManager {
    * Stop the analysis server.
    *
    * Returns [:true:] if the signal is successfully sent and process is killed.

[Brian Wilkerson, 2014/02/22 16:57:06]

nit: As above

[danrubel, 2014/02/24 17:39:24]

Done.

-   * Otherwise the signal could not be sent, usually meaning that the process
-   * is already dead.
+   * Otherwise there was no attached process or the signal could not be sent,
+   * usually meaning that the process is already dead.
    */
-  bool stop() => process.kill();
+  bool stop() => process != null ? process.kill() : false;

[Brian Wilkerson, 2014/02/22 16:57:06]

I still think you should ask the server nicely to stop running.

[danrubel, 2014/02/24 17:39:24]

Agreed. Not quite ready to wire that up. Added a comment that this needs to be
done.

 }