 Chromium Code Reviews
 Chromium Code Reviews Issue 23898009:
  Switch polymer's build.dart to use the new linter. This CL does the following  (Closed) 
  Base URL: https://dart.googlecode.com/svn/branches/bleeding_edge/dart
    
  
    Issue 23898009:
  Switch polymer's build.dart to use the new linter. This CL does the following  (Closed) 
  Base URL: https://dart.googlecode.com/svn/branches/bleeding_edge/dart| Index: pkg/polymer/lib/build_helper.dart | 
| diff --git a/pkg/polymer/lib/build_helper.dart b/pkg/polymer/lib/build_helper.dart | 
| new file mode 100644 | 
| index 0000000000000000000000000000000000000000..81d51516c96be1642e006ea2e16546a1cb1b6583 | 
| --- /dev/null | 
| +++ b/pkg/polymer/lib/build_helper.dart | 
| @@ -0,0 +1,236 @@ | 
| +// Copyright (c) 2012, 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. | 
| + | 
| +/** | 
| + * Common logic to make it easy to run the polymer linter and deploy tool. | 
| + * | 
| + * The functions in this library are designed to make it easier to create | 
| + * `build.dart` files. A `build.dart` file is a Dart script that can be invoked | 
| + * from the command line, but that can also invoked automatically by the Dart | 
| + * Editor whenever a file in your project changes or when selecting some menu | 
| + * options, such as 'Reanalyze Sources'. | 
| + * | 
| + * To work correctly, place the `build.dart` in the root of your project (where | 
| + * pubspec.yaml lives). The file must be named exactly `build.dart`. | 
| + * | 
| + * It's quite likely that in the near future `build.dart` will be replaced with | 
| + * something else. For example, `pub deploy` will deal with deploying | 
| + * applications automatically, and the Dart Editor might provide other | 
| + * mechanisms to hook linters. | 
| + * | 
| + * The following examples illustrate common ways to write a `build.dart` file. | 
| + * | 
| + * **Example 1**: Uses build.dart to run the linter tool. | 
| + * | 
| + * import 'dart:io'; | 
| + * import 'package:polymer/build_helper.dart'; | 
| + * | 
| + * main() { | 
| + * lint(); | 
| + * } | 
| + * | 
| + * **Example 2**: Runs the linter and creates a deployable version of the app | 
| + * every time. | 
| + * | 
| + * import 'dart:io'; | 
| + * import 'package:polymer/build_helper.dart'; | 
| + * | 
| + * main() { | 
| + * lint().then(() => deploy()); | 
| + * } | 
| + * | 
| + * **Example 3**: Runs the linter, but conditionally does the deploy step. See | 
| + * [parseOptions] for a description of options parsed automatically by this | 
| + * helper library. | 
| + * | 
| + * import 'dart:io'; | 
| + * import 'package:polymer/build_helper.dart'; | 
| + * | 
| + * main() { | 
| + * var options = parseOptions(); | 
| + * lint().then(() { | 
| + * if (options.forceDeploy) deploy(); | 
| + * }); | 
| + * } | 
| + * | 
| + * **Example 4**: Like the previous example, but indicates to the linter and | 
| + * deploy tool which files are actually used as entrypoint files. See the | 
| + * documentation of [lint] and [deploy] below for more details. | 
| + * | 
| + * import 'dart:io'; | 
| + * import 'package:polymer/build_helper.dart'; | 
| + * | 
| + * main() { | 
| 
Jennifer Messerly
2013/09/10 04:16:51
this looks like the most common pattern. let's com
 
Siggi Cherem (dart-lang)
2013/09/11 01:45:26
Cool idea, added it for now. I think once pub depl
 | 
| + * var options = parseOptions(); | 
| + * lint(entrypoints: ['web/index.html']).then(() { | 
| + * if (options.forceDeploy) deploy(entrypoints: ['web/index.html']); | 
| + * }); | 
| + * } | 
| + */ | 
| +library build_helper; | 
| 
Jennifer Messerly
2013/09/10 04:16:51
library polymer.build_helper;
actually -- instead
 
Siggi Cherem (dart-lang)
2013/09/11 01:45:26
Agree, Note I uploaded 2 patch sets. I did all the
 | 
| + | 
| +import 'dart:async'; | 
| +import 'dart:io'; | 
| +import 'package:args/args.dart'; | 
| + | 
| +import 'package:polymer/src/barback_helper.dart'; | 
| 
Jennifer Messerly
2013/09/10 04:16:51
this should be 'src/barback_helper.dart' for consi
 
Siggi Cherem (dart-lang)
2013/09/11 01:45:26
oops, done.
 | 
| +import 'package:polymer/src/linter.dart'; | 
| +import 'package:polymer/src/transform.dart'; | 
| + | 
| + | 
| +/** | 
| + * Runs the polymer linter on any relevant file in your package, | 
| + * such as any .html file under 'lib/', 'asset/', and 'web/'. | 
| + * | 
| + * The [entrypoints] list contains files under web/ that should be treated as | 
| + * entrypoints. Each entry on this list is a relative path from the package root | 
| + * (for example 'web/index.html'). If emtpy, all files under 'web/' are treated | 
| + * as possible entrypoints. | 
| + * | 
| + * Options are read from the command line arguments, but you can override them | 
| + * passing the [options] argument. | 
| + * | 
| + * Internally, the linter needs to know the name of the [currentPackage] and | 
| + * the location where to find the code for any package it depends on | 
| + * ([packageDirs]). This is inferred automatically, but can be overriden if | 
| + * those arguments are provided. | 
| + */ | 
| +Future lint({List<String> entrypoints, CommandLineOptions options, | 
| + String currentPackage, Map<String, String> packageDirs}) { | 
| + if (options == null) options = _options; | 
| + if (currentPackage == null) currentPackage = readCurrentPackageFromPubspec(); | 
| + var linterOptions = new TransformOptions(currentPackage, entrypoints); | 
| + var linter = options.machineFormat ? new Linter(linterOptions, jsonFormatter) | 
| + : new Linter(linterOptions); | 
| 
Jennifer Messerly
2013/09/10 04:16:51
slight improvement:
var formatter = options.machi
 
Siggi Cherem (dart-lang)
2013/09/11 01:45:26
Done.
 | 
| + return runBarback(new BarbackOptions([[linter]], null, | 
| + currentPackage: currentPackage, packageDirs: packageDirs)).then((assets) { | 
| + for (var asset in assets) { | 
| 
Jennifer Messerly
2013/09/10 04:16:51
how stable is this order? wondering if we should w
 
Siggi Cherem (dart-lang)
2013/09/11 01:45:26
good point. added sorting below.
 | 
| + var id = asset.id; | 
| + if (id.package == currentPackage && id.path.endsWith('.messages')) { | 
| + asset.readAsString().then((content) { | 
| + if (!content.isEmpty) print(content); | 
| 
Jennifer Messerly
2013/09/10 04:16:51
i'm not sure we want to print from an async callba
 
Siggi Cherem (dart-lang)
2013/09/11 01:45:26
Good point. did so below too.
 | 
| + }); | 
| + } | 
| + } | 
| + }); | 
| +} | 
| + | 
| +/** | 
| + * Runs the polymer deploy step. | 
| 
Jennifer Messerly
2013/09/10 04:16:51
"Creates a directory suitable for deploying a Poly
 
Siggi Cherem (dart-lang)
2013/09/11 01:45:26
Done.
 | 
| + * | 
| + * The [entrypoints] list contains files under web/ that should be treated as | 
| 
Jennifer Messerly
2013/09/10 04:16:51
must they be under "web/"?
 
Siggi Cherem (dart-lang)
2013/09/11 01:45:26
Good question. For now that was the convention I w
 | 
| + * entrypoints. Each entry on this list is a relative path from the package root | 
| + * (for example 'web/index.html'). If emtpy, all files under 'web/' are treated | 
| 
Jennifer Messerly
2013/09/10 04:16:51
very minor quibble: i'm not sure about having 0 wr
 
Siggi Cherem (dart-lang)
2013/09/11 01:45:26
Sorry, that is how it's implemented (null = max),
 | 
| + * as possible entrypoints. | 
| + * | 
| + * Options are read from the command line arguments, but you can override them | 
| + * passing the [options] list. | 
| + * | 
| + * Internally, the deploy script needs to know the name of the [currentPackage] | 
| 
Jennifer Messerly
2013/09/10 04:16:51
trivial edit: I think you can remove "internally"
 
Siggi Cherem (dart-lang)
2013/09/11 01:45:26
Done, but used "step" instead of "function".
 | 
| + * and the location where to find the code for any package it depends on | 
| + * ([packageDirs]). This is inferred automatically, but can be overriden if | 
| + * those arguments are provided. | 
| + * | 
| + * Note: we expect this function to disappear in the future when `pub deploy` | 
| 
Jennifer Messerly
2013/09/10 04:16:51
**Note**: this function will be replaced in the fu
 
Siggi Cherem (dart-lang)
2013/09/11 01:45:26
sgtm
 | 
| + * becomes feature complete. | 
| + */ | 
| +Future deploy({List<String> entrypoints, CommandLineOptions options, | 
| + String currentPackage, Map<String, String> packageDirs, | 
| + bool forceDeploy}) { | 
| + if (options == null) options = _options; | 
| + if (currentPackage == null) currentPackage = readCurrentPackageFromPubspec(); | 
| + var barbackOptions = new BarbackOptions( | 
| + createDeployPhases(new TransformOptions(currentPackage, entrypoints)), | 
| + options.outDir, currentPackage: currentPackage, | 
| + packageDirs: packageDirs); | 
| + return runBarback(barbackOptions) | 
| + .then((_) => print('Done! All files written to "${options.outDir}"')); | 
| +} | 
| + | 
| + | 
| +/** | 
| + * Options that may be used either in build.dart or by the linter and deploy | 
| + * tools. | 
| + */ | 
| +class CommandLineOptions { | 
| + /** Files marked as changed. */ | 
| + List<String> changedFiles; | 
| 
Jennifer Messerly
2013/09/10 04:16:51
final fields?
 
Siggi Cherem (dart-lang)
2013/09/11 01:45:26
Done.
 | 
| + | 
| + /** Files marked as removed. */ | 
| + List<String> removedFiles; | 
| + | 
| + /** Whether to clean intermediate artifacts, if any. */ | 
| + bool clean; | 
| + | 
| + /** Whether to do a full build (as if all files have changed). */ | 
| + bool full; | 
| + | 
| + /** Whether to print results using a machine parseable format. */ | 
| + bool machineFormat; | 
| + | 
| + /** Whether the force deploy option was passed in the command line. */ | 
| + bool forceDeploy; | 
| + | 
| + /** Location where to generate output files. */ | 
| + String outDir; | 
| + | 
| + CommandLineOptions(this.changedFiles, this.removedFiles, this.clean, | 
| + this.full, this.machineFormat, this.forceDeploy, this.outDir); | 
| +} | 
| + | 
| +/** Options parsed directly from the command line arguments. */ | 
| +CommandLineOptions _options = parseOptions(); | 
| + | 
| +/** | 
| + * Parse command-line arguments and return a [CommandLineOptions] object. The | 
| + * following flags are parsed by this method. | 
| + * | 
| + * * `--changed file-path`: notify of a file change. | 
| + * * `--removed file-path`: notify that a file was removed. | 
| + * * `--clean`: remove temporary artifacts (if any) | 
| + * * `--full`: build everything, similar to marking every file as changed | 
| + * * `--machine`: produce output that can be parsed by tools, such as the Dart | 
| + * Editor. | 
| + * * `--deploy`: force deploy. | 
| + * * `--help`: print documentation for each option and exit. | 
| + * | 
| + * Currently not all the flags are used by [lint] or [deploy] above, but they | 
| + * are available so they can be used from your `build.dart`. For instance, see | 
| + * the top-level library documentation for an example that uses the force-deploy | 
| + * option to conditionally call [deploy]. | 
| + * | 
| + * If this documentation becomes out of date, the best way to discover which | 
| + * flags are supported is to invoke this function from your build.dart, and run | 
| + * it with the `--help` command-line flag. | 
| + */ | 
| +CommandLineOptions parseOptions([List<String> args]) { | 
| + var parser = new ArgParser() | 
| + ..addOption('changed', help: 'the file has changed since the last build', | 
| + allowMultiple: true) | 
| + ..addOption('removed', help: 'the file was removed since the last build', | 
| + allowMultiple: true) | 
| + ..addFlag('clean', negatable: false, | 
| + help: 'currently a noop, may be used in the future to remove any build' | 
| 
Jennifer Messerly
2013/09/10 04:16:51
long line in help? consider adding \n
 
Siggi Cherem (dart-lang)
2013/09/11 01:45:26
reworded it.
 | 
| + ' artifacts') | 
| + ..addFlag('full', negatable: false, help: 'perform a full build') | 
| + ..addFlag('machine', negatable: false, | 
| + help: 'produce warnings in a machine parseable format') | 
| + ..addFlag('deploy', negatable: false, | 
| + help: 'if using the deploy() function, whether to force deploying') | 
| + ..addOption('out', abbr: 'o', help: 'Directory where to generate files.', | 
| 
Jennifer Messerly
2013/09/10 04:16:51
nit: noticed this one was capitalized and has a pe
 
Siggi Cherem (dart-lang)
2013/09/11 01:45:26
good catch. I normally like lowercase + no period,
 | 
| + defaultsTo: 'out') | 
| + ..addFlag('help', abbr: 'h', | 
| + negatable: false, help: 'displays this help and exit'); | 
| + var results = parser.parse(args == null ? new Options().arguments : args); | 
| + if (results['help']) { | 
| + print('A build script that invokes the polymer linter and deploy tools.'); | 
| + print('Usage: dart build.dart [options]'); | 
| + print('\nThese are valid options expected by build.dart:'); | 
| + print(parser.getUsage()); | 
| + exit(0); | 
| + } | 
| + return new CommandLineOptions(results['changed'], results['removed'], | 
| 
Jennifer Messerly
2013/09/10 04:16:51
trivial: this might be a good case for a shortish
 
Siggi Cherem (dart-lang)
2013/09/11 01:45:26
Done.
 | 
| + results['clean'], results['full'], results['machine'], results['deploy'], | 
| + results['out']); | 
| +} |