-
Notifications
You must be signed in to change notification settings - Fork 34
Description
Premise
As per the official documentation, this section indicates the expected behavior:
I cannot strictly classify my problem as a bug because it does not say "Options for a command must appear immediately after the command in the argument list".var results = parser.parse(['commit', '-a']); print(results.command.name); // "commit" print(results.command['all']); // trueOptions for a command must appear after the command in the argument list. For example, given the above parser,
"git -a commit"is not valid.
Problem
Consider the help command (available by default when using CommandRunner). It accepts no options other than the default -h flag. Therefore, any argument except -h to help is, I expect, to throw a UsageException.
However, if the main command e.g. bugdemo has some options e.g. a flag -f, then running bugdemo help -f:
- does not throw
UsageException - the parent
bugdemoconsumes the-foption from its childhelp
Demonstration:
import 'package:args/command_runner.dart';
void main() async {
final runner = CommandRunner(
'mock-command',
'A mock command to demonstrate a bug.',
);
runner.argParser.addFlag('mock-flag', abbr: 'f');
print('-------------------------------------------------------------------');
await demoCorrectOutput(runner);
print('-------------------------------------------------------------------');
await demoWrongOutput(runner);
print('-------------------------------------------------------------------');
}
Future<void> demoCorrectOutput(final CommandRunner runner) async {
try {
await runner.run(['help', '-x']);
print('>>> Bug: Error not caught <<<');
} on UsageException catch (e) {
print('>>> Error caught as expected <<<');
print(e);
}
}
Future<void> demoWrongOutput(final CommandRunner runner) async {
try {
await runner.run(['help', '-f']);
print('>>> Bug: Error not caught <<<');
} on UsageException catch (e) {
print('>>> Error caught as expected <<<');
print(e);
}
}
Output
-------------------------------------------------------------------
>>> Error caught as expected <<<
Could not find an option or flag "-x".
Usage: mock-command help [command]
-h, --help Print this usage information.
Run "mock-command help" to see global options.
-------------------------------------------------------------------
A mock command to demonstrate a bug.
Usage: mock-command <command> [arguments]
Global options:
-h, --help Print this usage information.
-f, --[no-]mock-flag
Available commands:
help Display help information for mock-command.
Run "mock-command help <command>" for more information about a command.
>>> Bug: Error not caught <<<
-------------------------------------------------------------------
Confusion
I'm not sure why this unexpected behavior is mandatory, because all corresponding implementation code e.g. in bool _handleSoloOption(String opt) appears to do so intentionally:
...
if (option == null) {
// Walk up to the parent command if possible.
_validate(_parent != null, 'Could not find an option or flag "-$opt".',
'-$opt');
return _parent!._handleSoloOption(opt);
}
...So, I decided to test the same behavior in some other popular argument parsing library. I went ahead with Python's standard argparse and got the desired result:
Suggestion
I reckon modifying this behavior would be a breaking change, as I could find one Test Case that confirms this admittedly unexpected behavior is rather expected by design:
test('assigns collapsed options to the proper command', () {
var parser = ArgParser();
parser.addFlag('apple', abbr: 'a');
var command = parser.addCommand('cmd');
command.addFlag('banana', abbr: 'b');
var subcommand = command.addCommand('subcmd');
subcommand.addFlag('cherry', abbr: 'c');
var args = parser.parse(['cmd', 'subcmd', '-abc']);
expect(args['apple'], isTrue);
expect(args.command!.name, equals('cmd'));
expect(args.command!['banana'], isTrue);
expect(args.command!.command!.name, equals('subcmd'));
expect(args.command!.command!['cherry'], isTrue);
});Nonetheless, if you may kindly consider, it's still possible to fix this situation by relying on a feat. For example, a new Constructor-level parameter (could default to the existing behavior) for applicable Classes to let the user explicitly choose which style of parsing to rely on for this scenario. Thus, it would keep the possibility open for other relevant styles/expectations. Thank you.