diff --git a/doc/tool-composition.md b/doc/tool-composition.md index f0a4cb11..81ceed68 100644 --- a/doc/tool-composition.md +++ b/doc/tool-composition.md @@ -140,6 +140,70 @@ final config = { }; ``` +### Mapping args to tools + +`CompoundTool.addTool()` supports an optional `argMapper` parameter that can be +used to customize the `ArgResults` instance that the tool gets when it runs. + +The typedef for this `argMapper` function is: + +```dart +typedef ArgMapper = ArgResults Function(ArgParser parser, ArgResults results); +``` + +By default, subtools added to a `CompoundTool` will _only_ receive option args +that are defined by their respective `ArgParser`: + +```dart +// tool/dart_dev/config.dart +import 'package:dart_dev/dart_dev.dart'; + +final config = { + 'example': CompoundTool() + // This subtool has an ArgParser that only supports the --foo flag. + ..addTool(DevTool.fromFunction((_) => 0, + argParser: ArgParser()..addFlag('foo'))) + + // This subtool has an ArgParser that only supports the --bar flag. + ..addTool(DevTool.fromFunction((_) => 0, + argParser: ArgParser()..addFlag('bar'))) +}; +``` + +With the above configuration, running `ddev example --foo --bar` will result in +the compound tool running the first subtool with only the `--foo` option +followed by the second subtool with only the `--bar` option. Any positional args +would be discarded. + +You may want one of the subtools to also receive the positional args. To +illustrate this, our test tool example from above can be updated to allow +positional args to be sent to the `TestTool` portion so that individual test +files can be targeted. + +To do this, we can use the `takeAllArgs` function provided by dart_dev: + +```dart +// tool/dart_dev/config.dart +import 'package:dart_dev/dart_dev.dart'; + +final config = { + 'test': CompoundTool() + ..addTool(DevTool.fromFunction(startServer), alwaysRun: true) + // Using `takeAllArgs` on this subtool will allow it to receive + // the positional args passed to `ddev test` as well as any + // option args specific to the `TestTool`. + ..addTool(TestTool(), argMapper: takeAllArgs) + ..addTool(DevTool.fromFunction(stopServer), alwaysRun: true), +}; + +int startServer([DevToolExecutionContext context]) => 0; +int stopServer([DevToolExecutionContext context]) => 0; +``` + +The default behavior for subtools along with using `takeAllArgs` for the subtool +that needs the positional args should cover most use cases. However, you may +write your own `ArgMapper` function if further customization is needed. + ### Sharing state across tools With more complex use cases, it may be necessary to share or use state across diff --git a/lib/dart_dev.dart b/lib/dart_dev.dart index 1e913f5a..6ee4869a 100644 --- a/lib/dart_dev.dart +++ b/lib/dart_dev.dart @@ -2,7 +2,8 @@ export 'src/core_config.dart' show coreConfig; export 'src/dart_dev_tool.dart' show DevTool, DevToolCommand, DevToolExecutionContext; export 'src/tools/analyze_tool.dart' show AnalyzeTool; -export 'src/tools/compound_tool.dart' show CompoundTool, CompoundToolMixin; +export 'src/tools/compound_tool.dart' + show ArgMapper, CompoundTool, CompoundToolMixin, takeAllArgs; export 'src/tools/format_tool.dart' show FormatMode, Formatter, FormatTool; export 'src/tools/process_tool.dart' show ProcessTool; export 'src/tools/test_tool.dart' show TestTool; diff --git a/lib/src/dart_dev_tool.dart b/lib/src/dart_dev_tool.dart index 46fc9a88..57d90157 100644 --- a/lib/src/dart_dev_tool.dart +++ b/lib/src/dart_dev_tool.dart @@ -86,6 +86,8 @@ class DevToolExecutionContext { : _usageException = usageException, verbose = verbose ?? false; + final void Function(String message) _usageException; + /// The results from parsing the arguments passed to a [Command] if this tool /// was executed via a command-line app. /// @@ -104,7 +106,20 @@ class DevToolExecutionContext { /// This will not be null; it defaults to `false`. final bool verbose; - final void Function(String message) _usageException; + /// Return a copy of this instance with optional updates; any field that does + /// not have an updated value will remain the same. + DevToolExecutionContext update({ + ArgResults argResults, + String commandName, + void Function(String message) usageException, + bool verbose, + }) => + DevToolExecutionContext( + argResults: argResults ?? this.argResults, + commandName: commandName ?? this.commandName, + usageException: usageException ?? this.usageException, + verbose: verbose ?? this.verbose, + ); /// Calling this will throw a [UsageException] with [message] that should be /// caught by [CommandRunner] and used to set the exit code accordingly and @@ -115,11 +130,6 @@ class DevToolExecutionContext { } throw UsageException(message, ''); } - - DevToolExecutionContext withoutArgs() => DevToolExecutionContext( - commandName: commandName, - usageException: usageException, - verbose: verbose); } class DevToolCommand extends Command { diff --git a/lib/src/tools/compound_tool.dart b/lib/src/tools/compound_tool.dart index 29324022..e20d59bd 100644 --- a/lib/src/tools/compound_tool.dart +++ b/lib/src/tools/compound_tool.dart @@ -9,6 +9,43 @@ import '../dart_dev_tool.dart'; final _log = Logger('CompoundTool'); +typedef ArgMapper = ArgResults Function(ArgParser parser, ArgResults results); + +/// Return a parsed [ArgResults] that only includes the option args (flags, +/// single options, and multi options) supported by [parser]. +/// +/// Positional args and any option args not supported by [parser] will be +/// excluded. +/// +/// This [ArgMapper] is the default for tools added to a [CompoundTool]. +ArgResults takeOptionArgs(ArgParser parser, ArgResults results) => + parser.parse(optionArgsOnly(results, allowedOptions: parser.options.keys)); + +/// Return a parsed [ArgResults] that includes the option args (flags, single +/// options, and multi options) supported by [parser] as well as any positional +/// args. +/// +/// Option args not supported by [parser] will be excluded. +/// +/// Use this with a [CompoundTool] to indicate which tool should receive the +/// positional args given to the compound target. +/// +/// // tool/dart_dev/config.dart +/// import 'package:dart_dev/dart_dev.dart'; +/// +/// final config = { +/// 'test': CompoundTool() +/// // This tool will not receive any positional args +/// ..addTool(startServerTool) +/// // This tool will receive the test-specific option args as well as +/// // any positional args given to `ddev test`. +/// ..addTool(TestTool(), argMapper: takeAllArgs) +/// }; +ArgResults takeAllArgs(ArgParser parser, ArgResults results) => parser.parse([ + ...optionArgsOnly(results, allowedOptions: parser.options.keys), + ...results.rest, + ]); + class CompoundTool extends DevTool with CompoundToolMixin {} mixin CompoundToolMixin on DevTool { @@ -25,9 +62,9 @@ mixin CompoundToolMixin on DevTool { set description(String value) => _description = value; String _description; - void addTool(DevTool tool, {bool alwaysRun}) { + void addTool(DevTool tool, {bool alwaysRun, ArgMapper argMapper}) { final runWhen = alwaysRun ?? false ? RunWhen.always : RunWhen.passing; - _specs.add(DevToolSpec(runWhen, tool)); + _specs.add(DevToolSpec(runWhen, tool, argMapper: argMapper)); if (tool.argParser != null) { _argParser.addParser(tool.argParser); } @@ -40,7 +77,8 @@ mixin CompoundToolMixin on DevTool { int code = 0; for (var i = 0; i < _specs.length; i++) { if (!shouldRunTool(_specs[i].when, code)) continue; - final newCode = await _specs[i].tool.run(context); + final newCode = + await _specs[i].tool.run(contextForTool(context, _specs[i])); _log.fine('Step ${i + 1}/${_specs.length} done (code: $newCode)'); _log.info('\n\n'); if (code == 0) { @@ -52,6 +90,34 @@ mixin CompoundToolMixin on DevTool { } } +List optionArgsOnly(ArgResults results, + {Iterable allowedOptions}) { + final args = []; + for (final option in results.options) { + if (!results.wasParsed(option)) continue; + if (allowedOptions != null && !allowedOptions.contains(option)) continue; + final value = results[option]; + if (value is bool) { + args.add('--${value ? '' : 'no-'}$option'); + } else if (value is Iterable) { + args.addAll([for (final v in value as List) '--$option=$v']); + } else { + args.add('--$option=$value'); + } + } + return args; +} + +DevToolExecutionContext contextForTool( + DevToolExecutionContext baseContext, DevToolSpec spec) { + if (baseContext.argResults == null) return baseContext; + + final parser = spec.tool.argParser ?? ArgParser(); + final argMapper = spec.argMapper ?? takeOptionArgs; + return baseContext.update( + argResults: argMapper(parser, baseContext.argResults)); +} + bool shouldRunTool(RunWhen runWhen, int currentExitCode) { switch (runWhen) { case RunWhen.always: @@ -64,11 +130,13 @@ bool shouldRunTool(RunWhen runWhen, int currentExitCode) { } class DevToolSpec { + final ArgMapper argMapper; + final DevTool tool; final RunWhen when; - DevToolSpec(this.when, this.tool); + DevToolSpec(this.when, this.tool, {this.argMapper}); } enum RunWhen { always, passing } diff --git a/test/tools/compound_tool_test.dart b/test/tools/compound_tool_test.dart index 31d47200..f1565890 100644 --- a/test/tools/compound_tool_test.dart +++ b/test/tools/compound_tool_test.dart @@ -59,20 +59,35 @@ void main() { expect(toolsRan, orderedEquals([0, 2])); }); - test('runs tools with a shared, compound context', () async { - final parser = ArgParser()..addFlag('foo'); + test('runs tools with their own ArgResults by default', () async { final tool1 = DevTool.fromFunction((context) { expect(context.argResults['foo'], isTrue); + expect(() => context.argResults['bar'], throwsArgumentError); + expect(context.argResults.rest, isEmpty); return 0; - }, argParser: parser); + }, argParser: ArgParser()..addFlag('foo')); final tool2 = DevTool.fromFunction((context) { - expect(context.argResults['foo'], isTrue); + expect(context.argResults['bar'], isTrue); + expect(() => context.argResults['foo'], throwsArgumentError); + expect(context.argResults.rest, isEmpty); return 0; - }); + }, argParser: ArgParser()..addFlag('bar')); final ct = CompoundTool()..addTool(tool1)..addTool(tool2); - await ct - .run(DevToolExecutionContext(argResults: parser.parse(['--foo']))); + await ct.run(DevToolExecutionContext( + argResults: ct.argParser.parse(['--foo', '--bar', 'baz']))); + }); + + test('runs tools with a custom ArgMapper, if provided', () async { + final tool = DevTool.fromFunction((context) { + expect(context.argResults['foo'], isTrue); + expect(context.argResults.rest, orderedEquals(['bar', 'baz'])); + return 0; + }, argParser: ArgParser()..addFlag('foo')); + + final ct = CompoundTool()..addTool(tool, argMapper: takeAllArgs); + await ct.run(DevToolExecutionContext( + argResults: ct.argParser.parse(['--foo', 'bar', 'baz']))); }); }); @@ -149,4 +164,135 @@ void main() { expect(shouldRunTool(RunWhen.passing, 1), isFalse); }); }); + + group('contextForTool', () { + test('with null argResults', () { + final context = DevToolExecutionContext(); + expect(context, same(contextForTool(context, null))); + }); + + test('maps the argResults', () { + final fooParser = ArgParser()..addFlag('foo'); + final fooTool = DevTool.fromFunction((_) => 0, argParser: fooParser); + + final barParser = ArgParser()..addFlag('bar'); + final barTool = DevTool.fromFunction((_) => 0, argParser: barParser); + + final compoundTool = CompoundTool()..addTool(fooTool)..addTool(barTool); + final baseContext = DevToolExecutionContext( + argResults: compoundTool.argParser.parse(['--foo', '--bar'])); + + final spec = DevToolSpec(RunWhen.passing, fooTool); + final result = contextForTool(baseContext, spec); + expect(result, isNot(same(baseContext))); + expect(result.argResults.options, unorderedEquals(['foo'])); + }); + }); + + group('optionArgsOnly', () { + test('recreates the list of option args without positional args', () { + final originalParser = ArgParser() + ..addFlag('flag') + ..addOption('opt') + ..addMultiOption('multi'); + final originalResults = originalParser.parse([ + '--flag', + '--opt', + 'opt', + '--multi', + 'one', + '--multi', + 'two', + 'foo', + 'bar', + ]); + final args = optionArgsOnly(originalResults); + expect( + args, + unorderedEquals([ + '--flag', + '--opt=opt', + '--multi=one', + '--multi=two', + ])); + }); + + test('filters by allowedOptions if given', () { + final originalParser = ArgParser() + ..addFlag('flag') + ..addFlag('bad-flag') + ..addOption('opt') + ..addOption('bad-opt') + ..addMultiOption('multi') + ..addMultiOption('bad-multi'); + final originalResults = originalParser.parse([ + '--flag', + '--bad-flag', + '--opt', + 'opt', + '--bad-opt', + 'bad', + '--multi', + 'one', + '--multi', + 'two', + '--bad-multi', + 'bad', + 'foo', + 'bar', + ]); + final args = optionArgsOnly(originalResults, + allowedOptions: ['flag', 'opt', 'multi']); + expect( + args, + unorderedEquals([ + '--flag', + '--opt=opt', + '--multi=one', + '--multi=two', + ])); + }); + }); + + group('takeOptionArgs', () { + test('filters out unsupported options and positional args', () { + final fooParser = ArgParser()..addFlag('foo'); + final barParser = ArgParser()..addFlag('bar'); + final compoundParser = CompoundArgParser() + ..addParser(fooParser) + ..addParser(barParser); + final results = compoundParser.parse(['--foo', '--bar', 'baz']); + final mapped = takeOptionArgs(fooParser, results); + expect(mapped.options, unorderedEquals(['foo'])); + expect(mapped.rest, isEmpty); + }); + + test('with empty parser and results', () { + final parser = ArgParser(); + final mapped = takeOptionArgs(parser, parser.parse([])); + expect(mapped.options, isEmpty); + expect(mapped.rest, isEmpty); + }); + }); + + group('takeAllArgs', () { + test('filters out unsupported options but includes positional args', () { + final fooParser = ArgParser()..addFlag('foo'); + final barParser = ArgParser()..addFlag('bar'); + final compoundParser = CompoundArgParser() + ..addParser(fooParser) + ..addParser(barParser); + final results = compoundParser.parse(['--foo', '--bar', 'baz']); + final mapped = takeAllArgs(fooParser, results); + expect(mapped.options, unorderedEquals(['foo'])); + expect(mapped.rest, orderedEquals(['baz'])); + }); + + test('with empty parser and results', () { + final parser = ArgParser(); + final mapped = takeAllArgs(parser, parser.parse([])); + expect(mapped.options, isEmpty); + expect(mapped.rest, isEmpty); + }); + }); }