Make observable transform a barback transform.

pkg/barback/lib/src/transform.dart

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 library barback.transform;
 
 import 'dart:async';
+import 'package:source_maps/span.dart' show Span;

[nweiz, 2013/08/08 23:34:57]

Style nit: blank line between core library imports and package imports. Also, we
usually don't use "show" unless there's a concrete problem with the default
names that are exposed by a library.

If we're using the source_maps package, it should be added to the pubspec.

[Siggi Cherem (dart-lang), 2013/08/09 00:29:54]

Done.

Funny, I usually like using show if the list is short =), it makes it more
explicit what your dependencies are and makes it easier to understand where does
a type come from.

[Jennifer Messerly, 2013/08/09 05:44:03]

Moreover, it's the only style (along with "as") which reliably avoids name
conflicts due to new top-level members introduced. It is likely tools will start
doing this automatically and it will be generally encouraged style. There have
been various threads on this topic. It's not ideal, but it's probably what we're
stuck with :|

[Bob Nystrom, 2013/08/09 15:42:38]

Since this is a "package:" import, that doesn't worry me. New top-level names
will only appear when a developer upgrades the version of source_maps explicitly
and they can resolve it then.
If tools are going to do it for us (at deploy time), all the more reason to not
do it manually. :)

 
 import 'asset.dart';
 import 'asset_id.dart';
 import 'asset_node.dart';
 import 'asset_set.dart';
 import 'errors.dart';
 import 'transform_node.dart';
 import 'utils.dart';
 
 /// Creates a [Transform] by forwarding to the private constructor.
@@ skipping 20 matching lines @@
   ///
   /// While a transformation can use multiple input assets, one must be a
   /// special "primary" asset. This will be the "entrypoint" or "main" input
   /// file for a transformation.
   ///
   /// For example, with a dart2js transform, the primary input would be the
   /// entrypoint Dart file. All of the other Dart files that that imports
   /// would be secondary inputs.
   AssetId get primaryId => _node.primary.id;
 
+  Log get log => _log;

[Bob Nystrom, 2013/08/08 22:47:09]

Document:

"A logger so that the [Transformer] can report build details."

[Siggi Cherem (dart-lang), 2013/08/08 23:10:41]

Done.

+
   /// Gets the asset for the primary input.
   Future<Asset> get primaryInput => getInput(primaryId);
 
   Transform._(this._node, this._outputs);
 
   /// Gets the asset for for an input [id].
   ///
   /// If an input with that ID cannot be found, throws an
   /// [AssetNotFoundException].
   Future<Asset> getInput(AssetId id) => _node.getInput(id);
 
   /// Stores [output] as the output created by this transformation.
   ///
   /// A transformation can output as many assets as it wants.
   void addOutput(Asset output) {
     // TODO(rnystrom): This should immediately throw if an output with that ID
     // has already been created by this transformer.
     _outputs.add(output);
   }
 }
+
+final Log _log = new Log(true);

[Bob Nystrom, 2013/08/08 22:47:09]

Add a TODO to create a separate one for each Transform.

[Siggi Cherem (dart-lang), 2013/08/08 23:10:41]

Done.

+
+/// Log object used to report warnings and errors encountered while running a

[Bob Nystrom, 2013/08/08 22:47:09]

Add a TODO to move this into its own file.

[Siggi Cherem (dart-lang), 2013/08/08 23:10:41]

Done. I'm happy to move it out too (if we know what we want to do.)

[nweiz, 2013/08/08 23:34:57]

Or just move it now.

[Siggi Cherem (dart-lang), 2013/08/09 00:29:54]

now that it's just TransformLogger, it feels like part of this library. Do you
still want me to put it separate? transform_logger.dart?

[nweiz, 2013/08/09 00:53:48]

I'd still prefer it to be separate.

[Siggi Cherem (dart-lang), 2013/08/09 01:17:30]

Done.

+/// transform.
+class Log {
+
+  bool _shouldPrint;
+
+  Log(this._shouldPrint);
+
+  /// Logs a warning message. If present, [span] indicates the location in an

[Bob Nystrom, 2013/08/08 22:47:09]

"an asset that generated" -> "the input asset that caused"

[Siggi Cherem (dart-lang), 2013/08/08 23:10:41]

Done.

[nweiz, 2013/08/08 23:34:57]

Style nit: the first paragraph of a doc comment should only be one sentence
long. Also below.

[Siggi Cherem (dart-lang), 2013/08/09 00:29:54]

Done.

+  /// asset that generated the warning.
+  void warning(String message, [Span span]) {
+    _printMessage('warning', message, span);
+  }
+
+  /// Logs an error message. If present, [span] indicates the location in an
+  /// asset that generated the error.

[Bob Nystrom, 2013/08/08 22:47:09]

Ditto above change here.

[Siggi Cherem (dart-lang), 2013/08/08 23:10:41]

Done.

+  void error(String message, [Span span]) {

[nweiz, 2013/08/08 23:34:57]

Add a TODO to clarify when an error message should be logged vs thrown as an
error. Eventually we'll probably allow a transformer to report multiple
exceptions from a single run, which will muddy the waters even further.

[Siggi Cherem (dart-lang), 2013/08/09 00:29:54]

Done.

+    _printMessage('error', message, span);
+  }
+
+  // TODO(sigmund,rnystrom): do something better than printing.
+  _printMessage(String prefix, String message, Span span) {
+    if (!_shouldPrint) return;
+    var sb = new StringBuffer()..write(prefix)..write(' ');

[Bob Nystrom, 2013/08/08 22:47:09]

sb -> buffer

[Siggi Cherem (dart-lang), 2013/08/08 23:10:41]

Done. We all have different preferences on this one =)

+    if (span == null) {
+      sb.write(message);
+    } else {
+      sb.write(span.getLocationMessage(message));
+    }
+    print(sb.toString());

[nweiz, 2013/08/08 23:34:57]

Is it worth the extra complexity here to use a StringBuffer?

[Siggi Cherem (dart-lang), 2013/08/09 00:29:54]

good point. I used to have color printing and remvoed it to make things simpler,
at which point the SB it's overkill.

Done.

+  }
+}