Implement new combined dartdoc option class #1666

jcollins-g · 2018-04-12T19:25:33Z

A new class, "DartdocOption", and its subclasses are intended to replace the existing DartDocConfig and DartdocOptions classes. This class automatically handles arguments that can override configuration file options, check for the existence of files, and print useful error messages. It condenses some of the ugly things one typically has to do to make a config object work, so that you can declare them like this:

DartdocOptionSet all_my_flags = new DartdocOptionSet('dartdoc')..addAll([
  new DartdocOptionArgOnly('help', false, abbr: 'h',
      help: 'Displays help for the program'),
  new DartdocOptionBoth<int>('numberOfBugs', 12,
      help: 'How many bugs should dartdoc have for this run'),
  new DartdocOptionBoth<String>('mySpecialFile', null,
      help: 'If you want your special file documented, add it here.',
      mustExist: true, isFile: true),
  new DartdocOptionSet('warning')..addAll([
    new DartdocOptionBoth<bool>('dayIsThursday', false,
        help: 'Emit warning on thursdays.  I could never get the hang of them.',
        negatable: true)
  ]),
]);

This will read a dartdoc_options.yaml file like this:

dartdoc:
  numberOfBugs: 27
  mySpecialFile: "file/needs/to/exist.dart"
  warning:
    dayIsThursday: true

and command line options like this:

dart binary.dart -h
dart binary.dart --number-of-bugs 35 --no-warning-day-is-thursday

overriding the file with the command line options and throwing an assert if files declared that must exist don't, when accessing via:

all_my_files[numberOfBugs].valueAt(Directory.current);

The valueAt allows dartdoc to compute this value for different combinations of config files and arguments, and it should be sufficiently cached to minimize performance hits.

This PR does not actually use this anywhere besides the tests, that's for a followup.

pq

Substantially, 👍.

A few nits and style conversation starters...

pq · 2018-04-13T12:53:42Z

lib/src/dartdoc_options.dart

 import 'package:path/path.dart' as pathLib;
 import 'package:yaml/yaml.dart';

 import 'logging.dart';

+/// Constants to help with type checking, because T is int and so forth
+/// don't work in Dart.


pq · 2018-04-13T12:54:44Z

lib/src/dartdoc_options.dart

+const double _kDoubleVal = 0.0;
+const bool _kBoolVal = true;
+
+class DartdocOptionError extends DartDocFailure {


Maybe it would be more consistent to have them both named Dartdoc* or DartDoc*?

Changed all names to DartdocThing.

pq · 2018-04-13T12:56:20Z

lib/src/dartdoc_options.dart

+
+  // The choice not to use reflection means there's some ugly type checking,
+  // somewhat more ugly than we'd have to do anyway to automatically convert
+  // command line arguments and yaml data to real types.


pq · 2018-04-13T12:57:43Z

lib/src/dartdoc_options.dart

+}
+
+/// A [DartdocOption] that only contains other [DartdocOption]s and is not an option itself.
+class DartdocOptionSet extends DartdocOption<Null> {


Looking at this makes me think the name DartDoc was a typo (meaning Dartdoc instead).

Between Dartdoc and DartDoc I prefer Dartdoc, because the command is a single word in my mind. So I've switched them to Dartdoc -- I think it's really a matter of personal preference. I'm interested in consistency though so I've changed them all.

pq · 2018-04-13T12:58:23Z

lib/src/dartdoc_options.dart

+      dartdocYaml = pathLib.canonicalize(pathLib.join(
+          valueWithContext.canonicalDirectoryPath,
+          valueWithContext.definingFile));
+      description = "Field ${fieldName} from ${dartdocYaml}";


=> single quotes?

I think I keep doing this because in bash single quotes mean no resolution of expressions like '${foo}'. I'll try to reprogram myself differently. Fixed.

pq · 2018-04-13T13:02:42Z