pkg/logging: BREAKING: move to stream-based event model

pkg/logging/lib/logging.dart

Index: pkg/logging/lib/logging.dart
diff --git a/pkg/logging/lib/logging.dart b/pkg/logging/lib/logging.dart
index 935b3aeffec2b16d705fc10da992e149f9449e03..b222f0756670f198da51eaaf38949618e910e759 100644
--- a/pkg/logging/lib/logging.dart
+++ b/pkg/logging/lib/logging.dart
@@ -9,6 +9,10 @@
  */
 library logging;
 
+import 'dart:async';
+
+import 'package:meta/meta.dart';
+
 /**
  * Whether to allow fine-grain logging and configuration of loggers in a
  * hierarchy. When false, all logging is merged in the root logger.
@@ -44,7 +48,10 @@ class Logger {
   Map<String, Logger> children;
 
   /** Handlers used to process log entries in this logger. */

[Siggi Cherem (dart-lang), 2013/02/26 21:11:24]

fix comment

-  List<LoggerHandler> _handlers;
+  StreamController<LogRecord> _controller;
+
+  @deprecated

[Siggi Cherem (dart-lang), 2013/02/26 21:11:24]

If you don't mind - let's remove this one here. We mainly need it in the public
APIs.

This technically shouldn't matter, but turns out that the dart_analyzer right
now is printing deprecation warnings that occur anywhere. Ideally it should only
report warnings when a user makes use of something deprecated from their
package, but currently the analyzer doesn't work that way today. By removing
this @deprecated here, I think we'll get less noise if someone calls the
analyzer from the command line.

Here is the relevant bug for the analyzer
https://code.google.com/p/dart/issues/detail?id=8691

+  Map<LoggerHandler, StreamSubscription> _deprecatedSubscriptions;
 
   /**
    * Singleton constructor. Calling `new Logger(name)` will return the same
@@ -110,37 +117,19 @@ class Logger {
    * instance:
    *    logger.on.record.add((record) { ... });

[Siggi Cherem (dart-lang), 2013/02/26 21:11:24]

+ let's add a small comment here, like:

Deprecated. Use [onRecord] instead. 

    */
+  @deprecated
   LoggerEvents get on => new LoggerEvents(this);
 
-  /** Adds a handler to listen whenever a log record is added to this logger. */
-  void _addHandler(LoggerHandler handler) {
-    if (hierarchicalLoggingEnabled || parent == null) {
-      if (_handlers == null) {
-        _handlers = new List<LoggerHandler>();
-      }
-      _handlers.add(handler);
-    } else {
-      root._addHandler(handler);
-    }
-  }
-
-  /** Remove a previously added handler. */
-  void _removeHandler(LoggerHandler handler) {
-    if (hierarchicalLoggingEnabled || parent == null) {
-      if (_handlers == null) return;
-      int index = _handlers.indexOf(handler);
-      if (index != -1) _handlers.removeRange(index, 1);
-    } else {
-      root._removeHandler(handler);
-    }
-  }
+  Stream<LogRecord> get onRecord => _getStream();
 
-  /** Removes all handlers previously added to this logger. */
-  void _clearHandlers() {
-    if (hierarchicalLoggingEnabled || parent == null) {
-      _handlers = null;
+  void clearListeners() {
+    if(hierarchicalLoggingEnabled || parent == null) {

[Siggi Cherem (dart-lang), 2013/02/26 21:11:24]

style: add space after 'if' (same here below)

+      if(_controller != null) {
+        _controller.close();
+        _controller = null;
+      }
     } else {
-      root._clearHandlers();
+      root.clearListeners();
     }
   }
 
@@ -194,9 +183,44 @@ class Logger {
   /** Log message at level [Level.SHOUT]. */
   void shout(String message) => log(Level.SHOUT, message);
 
+  /** Adds a handler to listen whenever a log record is added to this logger. */

[Siggi Cherem (dart-lang), 2013/02/26 21:11:24]

move this comment down to _addHandler?

+  Stream<LogRecord> _getStream() {
+    if (hierarchicalLoggingEnabled || parent == null) {
+      if (_controller == null) {
+        _controller = new StreamController<LogRecord>.broadcast();
+      }
+      return _controller.stream;
+    } else {
+      return root._getStream();
+    }
+  }
+
+  @deprecated

[Siggi Cherem (dart-lang), 2013/02/26 21:11:24]

same here and all the ones below (the user had to use 'on' to get to these APIs,
so they will get a warning anyways).

+  void _addHandler(LoggerHandler handler) {
+    if(_deprecatedSubscriptions == null) {

[Siggi Cherem (dart-lang), 2013/02/26 21:11:24]

style: missing space after 'if'

+      _deprecatedSubscriptions = new Map<LoggerHandler, StreamSubscription>();
+    }
+
+    final sub = onRecord.listen(handler);

[Siggi Cherem (dart-lang), 2013/02/26 21:11:24]

style: even thought it's final, we use 'var' for these local variables. 

Given that this might fit in one line with the next line, consider just merging
them.

+    _deprecatedSubscriptions[handler] = sub;
+  }
+
+  @deprecated
+  void _removeHandler(LoggerHandler handler) {
+    if(_deprecatedSubscriptions != null) {

[Siggi Cherem (dart-lang), 2013/02/26 21:11:24]

style: space after if (same below)

+      final sub = _deprecatedSubscriptions.remove(handler);

[Siggi Cherem (dart-lang), 2013/02/26 21:11:24]

style: final -> var

+      if(sub != null) {
+        sub.cancel();
+      }
+      if(_deprecatedSubscriptions.isEmpty) {
+        _deprecatedSubscriptions = null;
+      }
+    }
+  }
+
   void _publish(LogRecord record) {
-    if (_handlers != null) {
-      _handlers.forEach((h) => h(record));
+    if (_controller != null) {
+      _controller.add(record);
     }
   }
 
@@ -209,10 +233,12 @@ class Logger {
 
 
 /** Handler callback to process log entries as they are added to a [Logger]. */
+@deprecated
 typedef void LoggerHandler(LogRecord);
 
 
 /** Event manager for a [Logger] (holds events that a [Logger] can fire). */
+@deprecated
 class LoggerEvents {
   final Logger _logger;
 
@@ -224,6 +250,7 @@ class LoggerEvents {
 
 
 /** List of handlers that will be called on a logger event. */
+@deprecated

[Siggi Cherem (dart-lang), 2013/02/26 21:11:24]

I doubt anyone will allocate these types directly (other than the APIs that are
already marked deprecated above), but if that's the reason to add these, then
let's move them to the actual methods below ('add', 'remove', 'clear', and
'record')

That way our code in logger doesn't use any deprecated API. 

 class LoggerHandlerList {
   Logger _logger;
 
@@ -231,7 +258,7 @@ class LoggerHandlerList {
 
   void add(LoggerHandler handler) => _logger._addHandler(handler);
   void remove(LoggerHandler handler) => _logger._removeHandler(handler);
-  void clear() => _logger._clearHandlers();
+  void clear() => _logger.clearListeners();
 }