fixes #610, incorrect help output

lib/src/compiler/command.dart

Index: lib/src/compiler/command.dart
diff --git a/lib/src/compiler/command.dart b/lib/src/compiler/command.dart
index 545d1a4850ad19c930dca6b3ac9e67219b025c72..66bf2ef1282885c311ef72a231f862845a43c48c 100644
--- a/lib/src/compiler/command.dart
+++ b/lib/src/compiler/command.dart
@@ -4,49 +4,88 @@
 
 import 'dart:convert' show JSON;
 import 'dart:io';
-import 'package:args/command_runner.dart';
 import 'package:analyzer/src/generated/source.dart' show Source;
 import 'package:analyzer/src/summary/package_bundle_reader.dart'
     show InSummarySource;
+import 'package:args/args.dart' show ArgParser, ArgResults;
+import 'package:args/command_runner.dart' show UsageException;
+import 'package:path/path.dart' as path;
+
 import 'compiler.dart'
     show BuildUnit, CompilerOptions, JSModuleFile, ModuleCompiler;
 import '../analyzer/context.dart' show AnalyzerOptions;
-import 'package:path/path.dart' as path;
 
 typedef void MessageHandler(Object message);
 
 /// The command for invoking the modular compiler.
-class CompileCommand extends Command {
-  final MessageHandler messageHandler;
+class CompileCommand {
+  final MessageHandler _print;
   CompilerOptions _compilerOptions;
+  final ArgParser _argParser = new ArgParser();

[nweiz, 2016/08/12 20:30:57]

Nit: this doesn't need to be type-annotated.

[Jennifer Messerly, 2016/08/12 21:08:01]

true. Although it unfortunately needs the type annotation after removing the
class & having addArguments methods return "void". (see updated version of this
file)

 
   CompileCommand({MessageHandler messageHandler})
-      : this.messageHandler = messageHandler ?? print {
-    argParser.addOption('out', abbr: 'o', help: 'Output file (required)');
-    argParser.addOption('module-root',
-        help: 'Root module directory. '
+      : _print = messageHandler ?? print {
+    _argParser.addOption('out', abbr: 'o', help: 'Output file (required)');
+    _argParser.addOption('module-root',
+        help: 'Root module directory.\n'
             'Generated module paths are relative to this root.');
-    argParser.addOption('library-root',
-        help: 'Root of source files. '
+    _argParser.addOption('library-root',
+        help: 'Root of source files.\n'
             'Generated library names are relative to this root.');
-    argParser.addOption('build-root',
-        help: 'Deprecated in favor of --library-root');
-    CompilerOptions.addArguments(argParser);
-    AnalyzerOptions.addArguments(argParser);
+    _argParser.addOption('build-root',
+        help: 'Deprecated in favor of --library-root', hide: true);
+    CompilerOptions.addArguments(_argParser);
+    AnalyzerOptions.addArguments(_argParser);
   }
 
-  get name => 'compile';
-  get description => 'Compile a set of Dart files into a JavaScript module.';
+  int run(List<String> args) {
+    try {
+      _run(_argParser.parse(args));
+      return 0;
+    } on UsageException catch (error) {
+      // Incorrect usage, input file not found, etc.
+      _print(error);
+      return 64;
+    } on CompileErrorException catch (error) {
+      // Code has error(s) and failed to compile.
+      _print(error);
+      return 1;
+    } catch (error, stackTrace) {
+      // Anything else is likely a compiler bug.
+      //
+      // --unsafe-force-compile is a bit of a grey area, but it's nice not to
+      // crash while compiling
+      // (of course, output code may crash, if it had errors).
+      //
+      _print("");
+      _print("We're sorry, you've found a bug in our compiler.");
+      _print("You can report this bug at:");
+      _print("    https://github.com/dart-lang/dev_compiler/issues");
+      _print("");
+      _print(
+          "Please include the information below in your report, along with");
+      _print(
+          "any other information that may help us track it down. Thanks!");
+      _print("");
+      _print("    dartdevc arguments: " + args.join(' '));
+      _print("    dart --version: ${Platform.version}");
+      _print("");
+      _print("```");
+      _print(error);
+      _print(stackTrace);
+      _print("```");

[nweiz, 2016/08/12 20:30:57]

Make this a multi-line string rather than calling _print over and over. If a
MessageHandler only supports a single line at a time, add a helper method to
split a string and call _print on the results rather than doing it manually.

[Jennifer Messerly, 2016/08/12 21:08:01]

Done. As a general rule I try and not fix-all-the-things when doing a refactor.
But that said I'm happy to fix this case as it's easy enough and a lot nicer
looking.

[nweiz, 2016/08/12 21:16:37]

My philosophy is, if I don't fix incidental stuff as part of other changes, I'll
never find time to fix them at all. :)

+      return 70;
+    }
+  }
 
-  @override
-  void run() {
+  void _run(ArgResults argResults) {
     var compiler =
         new ModuleCompiler(new AnalyzerOptions.fromArguments(argResults));
     _compilerOptions = new CompilerOptions.fromArguments(argResults);
     var outPath = argResults['out'];
 
     if (outPath == null) {
-      usageException('Please include the output file location. For example:\n'
+      _usageException('Please include the output file location. For example:\n'
           '    -o PATH/TO/OUTPUT_FILE.js');
     }
 
@@ -62,7 +101,7 @@ class CompileCommand extends Command {
     if (moduleRoot != null) {
       moduleRoot = path.absolute(moduleRoot);
       if (!path.isWithin(moduleRoot, outPath)) {
-        usageException('Output file $outPath must be within the module root '
+        _usageException('Output file $outPath must be within the module root '
             'directory $moduleRoot');
       }
       modulePath =
@@ -76,7 +115,7 @@ class CompileCommand extends Command {
         (source) => _moduleForLibrary(moduleRoot, source));
 
     JSModuleFile module = compiler.compile(unit, _compilerOptions);
-    module.errors.forEach(messageHandler);
+    module.errors.forEach(_print);
 
     if (!module.isValid) throw new CompileErrorException();
 
@@ -104,15 +143,23 @@ class CompileCommand extends Command {
         return path.relative(buildUnitPath, from: moduleRoot);
       }
 
-      throw usageException(
+      _usageException(
           'Imported file ${source.uri} is not within the module root '
           'directory $moduleRoot');
     }
 
-    throw usageException(
+    _usageException(
         'Imported file "${source.uri}" was not found as a summary or source '
         'file. Please pass in either the summary or the source file '
         'for this import.');
+    return null; // unreachable
+  }
+
+  void _usageException(String message) {
+    throw new UsageException(
+        message,
+        'Dart Development Compiler compiles Dart into a JavaScript module.'
+        '\n\n${_argParser.usage}');
   }
 }