Use reasonable defaults for docgen if invoked on a single package

pkg/docgen/bin/docgen.dart

Index: pkg/docgen/bin/docgen.dart
diff --git a/pkg/docgen/bin/docgen.dart b/pkg/docgen/bin/docgen.dart
index f1275c56620f0bed59691568a5493d86d5bef428..038baa929ceb756925ce32ce7d2521aec3431385 100644
--- a/pkg/docgen/bin/docgen.dart
+++ b/pkg/docgen/bin/docgen.dart
@@ -13,31 +13,61 @@ import 'package:path/path.dart' as path;
 List<String> excludedLibraries = [];
 
 /**
+ * If we look like we're asking to document a single package without
+ * any other options being set, do the sensible thing.
+ */
+bool _useSinglePackageDefaults = false;
+
+/**
+ * If we've specified just a package and no other command-line options,
+ * use the single package name as the start page.
+ */
+String get _defaultStartPage {

[Emily Fortuna, 2014/02/11 22:35:32]

nit: I'd put this function down below the main, so that the main is at the top,
aside from the global variables.

+  if (_useSinglePackageDefaults) {
+    var pubspec = new File(path.join(_files.first, 'pubspec.yaml'));
+    if (!pubspec.existsSync()) return null;
+    return Library.packageNameFor(_files.first);
+  } else {
+    return null;
+  }
+}
+
+/**
+ * The files/directories that we're being asked to document.
+ */
+List<String> _files;
+
+/**
  * Analyzes Dart files and generates a representation of included libraries,
  * classes, and members.
  */
 void main(List<String> arguments) {
-  var results = _initArgParser().parse(arguments);
-
-  var includeSdk = results['parse-sdk'] || results['include-sdk'];
+  var options = _initArgParser().parse(arguments);
+  _files = options.rest.map(path.normalize).toList();
+  if (_files.isEmpty) _printHelpAndExit();
+  _useSinglePackageDefaults = _singlePackageNoOptions(options,
+      _initArgParser(), _files);
+  // If we've changed the options based on it being a single package,
+  // re-parse the arguments so we get different defaults. Ugh.
+  options = _initArgParser().parse(arguments);

[Emily Fortuna, 2014/02/11 22:35:32]

I think you're missing some logic here.  (or I'm misreading this) The only place
this result is used seems to be in _defaultStartPage. If you're re-parsing,
don't you want to use that result somewhere in your parse?  Also can't we always
do this by default?

+  var includeSdk = options['parse-sdk'] || options['include-sdk'];
   var scriptDir = path.dirname(Platform.script.toFilePath());
-  var introduction = includeSdk ? '' : results['introduction'];
-  var files = results.rest.map(path.normalize).toList();
-  if (files.isEmpty) _printHelpAndExit();
-  docgen(files,
-      packageRoot: results['package-root'],
-      outputToYaml: !results['json'],
-      includePrivate: results['include-private'],
+  var introduction = includeSdk ? '' : options['introduction'];
+
+  docgen(_files,
+      packageRoot: options['package-root'],
+      outputToYaml: !options['json'],
+      includePrivate: options['include-private'],
       includeSdk: includeSdk,
-      parseSdk: results['parse-sdk'],
-      append: results['append'] && new Directory(results['out']).existsSync(),
+      parseSdk: options['parse-sdk'],
+      append: options['append'] && new Directory(options['out']).existsSync(),
       introFileName: introduction,
-      out: results['out'],
+      out: options['out'],
       excludeLibraries: excludedLibraries,
-      includeDependentPackages: results['include-dependent-packages'],
-      serve: results['serve'],
-      noDocs: results['no-docs'],
-      startPage: results['startPage']);
+      includeDependentPackages: options['include-dependent-packages'],
+      serve: options['serve'],
+      noDocs: options['no-docs'],
+      startPage: options['start-page']);
 }
 
 /**
@@ -50,6 +80,40 @@ void _printHelpAndExit() {
 }
 
 /**
+ * If the user provided no arguments and
+ * seems to have given us a single package to document, use some
+ * reasonable arguments for what they probably meant.
+ */
+bool _singlePackageNoOptions(ArgResults options, ArgParser parser, List files) {
+  if (!_allOptionsAreDefaults(options, parser)) return false;
+  if (files.length != 1) return false;
+  var pubspec = new File(path.join(files.first, 'pubspec.yaml'));
+  if (!pubspec.existsSync()) return false;
+  var packageName = Library.packageNameFor(files.first);
+  print("Using default options for documenting a single package: "
+      "--start-page=$packageName");
+  return true;
+}
+
+/**
+ * Return true if all the options are at their default value. It would
+ * be nice if there was a simpler way to test this but ArgResult is
+ * very opaque.
+ */
+bool _allOptionsAreDefaults(ArgResults options, ArgParser parser) {
+  for (var optionName in options.options) {
+    var defaultValue = parser.getDefault(optionName);
+    var actualValue = options[optionName];
+    var matchAnyway = defaultValue == null &&
+        actualValue is List && actualValue.isEmpty;
+    if (actualValue != null && actualValue != defaultValue && !matchAnyway) {
+      return false;
+    }
+  }
+  return true;
+}
+
+/**
  * Creates parser for docgen command line arguments.
  */
 ArgParser _initArgParser() {
@@ -69,11 +133,13 @@ ArgParser _initArgParser() {
       help: 'Outputs to JSON. Files are outputted to YAML by default. '
         'If --append is used, it takes the file-format of the previous '
         'run stated in library_list.json ignoring the flag.',
-      negatable: true);
+      negatable: true, defaultsTo: true);
   parser.addFlag('include-private',
       help: 'Flag to include private declarations.', negatable: false);
   parser.addFlag('include-sdk',
-      help: 'Flag to parse SDK Library files.', negatable: false);
+      help: 'Flag to parse SDK Library files.',
+      defaultsTo: true,
+      negatable: true);
   parser.addFlag('parse-sdk',
       help: 'Parses the SDK libraries only.',
       defaultsTo: false, negatable: false);
@@ -101,13 +167,13 @@ ArgParser _initArgParser() {
       help: 'Assumes we are documenting a single package and are running '
         'in the directory with its pubspec. Includes documentation for all '
         'of its dependent packages.',
-      defaultsTo: false, negatable: false);
-  parser.addOption('startPage',
+      defaultsTo: true, negatable: true);
+  parser.addOption('start-page',
       help: 'By default the viewer will start at the SDK introduction page.'
         'To start at some other page, e.g. for a package, provide the name '
-        'of the package in this argument, e.g. --startPage=intl will make '
+        'of the package in this argument, e.g. --start-page=intl will make '
         'the start page of the viewer be the intl package.',
-        defaultsTo: null);
+        defaultsTo: _defaultStartPage);
 
   return parser;
 }