From 4af6492028baa1df0432b6b032a33ae5ba88c203 Mon Sep 17 00:00:00 2001 From: Renan <6718144+renancaraujo@users.noreply.github.com> Date: Tue, 20 Dec 2022 14:40:18 +0000 Subject: [PATCH] feat: restrict non multi options (#44) --- .../completion_integration_test.dart | 155 ++++++++++++------ example/test/integration/utils.dart | 6 + lib/src/parser/arg_parser_extension.dart | 62 ++++++- lib/src/parser/completion_level.dart | 12 ++ .../src/parser/arg_parser_extension_test.dart | 85 +++++++++- test/src/parser/completion_level_test.dart | 18 +- 6 files changed, 278 insertions(+), 60 deletions(-) diff --git a/example/test/integration/completion_integration_test.dart b/example/test/integration/completion_integration_test.dart index 0e76539..4726080 100644 --- a/example/test/integration/completion_integration_test.dart +++ b/example/test/integration/completion_integration_test.dart @@ -419,42 +419,6 @@ void main() { suggests(noSuggestions), ); }); - - test( - 'suggest all options when previous option is continuous with a value', - () async { - await expectLater( - 'example_cli some_command --continuous="yeahoo" ', - suggests(allOptionsInThisLevel), - ); - }, - ); - - test( - 'suggest all options when previous option is continuous with a value', - () async { - await expectLater( - 'example_cli some_command --continuous yeahoo ', - suggests(allOptionsInThisLevel), - ); - }, - ); - }); - - group('flags', () { - test('suggest all options when a flag was declared', () async { - await expectLater( - 'example_cli some_command --flag ', - suggests(allOptionsInThisLevel), - ); - }); - - test('suggest all options when a negated flag was declared', () async { - await expectLater( - 'example_cli some_command --no-flag ', - suggests(allOptionsInThisLevel), - ); - }); }); }); @@ -532,15 +496,6 @@ void main() { ); }); }); - - group('flag', () { - test('suggest all options when a flag was declared', () async { - await expectLater( - 'example_cli some_command -f ', - suggests(allOptionsInThisLevel), - ); - }); - }); }); group('invalid options', () { @@ -552,15 +507,109 @@ void main() { }); }); - group( - 'repeating options', - tags: 'known-issues', - () { - group('non multi options', () {}); + group('repeating options', () { + group('non multi options', () { + test('do not include option after it is specified', () async { + await expectLater( + 'example_cli some_command --discrete foo ', + suggests(allOptionsInThisLevel.except('--discrete')), + ); + }); + + test('do not include abbr option after it is specified', () async { + await expectLater( + 'example_cli some_command --discrete foo -', + suggests(allAbbreviationsInThisLevel.except('-d')), + ); + }); + + test('do not include option after it is specified as abbr', () async { + await expectLater( + 'example_cli some_command -d foo ', + suggests(allOptionsInThisLevel.except('--discrete')), + ); + }); + + test( + 'do not include option after it is specified as joined abbr', + () async { + await expectLater( + 'example_cli some_command -dfoo ', + suggests(allOptionsInThisLevel.except('--discrete')), + ); + }, + tags: 'known-issues', + ); + + test('do not include flag after it is specified', () async { + await expectLater( + 'example_cli some_command --flag ', + suggests(allOptionsInThisLevel.except('--flag')), + ); + }); + + test('do not include flag after it is specified (abbr)', () async { + await expectLater( + 'example_cli some_command -f ', + suggests(allOptionsInThisLevel.except('--flag')), + ); + }); - group('multi options', () {}); - }, - ); + test('do not include negated flag after it is specified', () async { + await expectLater( + 'example_cli some_command --no-flag ', + suggests(allOptionsInThisLevel.except('--flag')), + ); + }); + + test('do not regard negation of non negatable flag', () async { + await expectLater( + 'example_cli some_command --no-trueflag ', + suggests(allOptionsInThisLevel), + ); + }); + }); + + group('multi options', () { + test('include multi option after it is specified', () async { + await expectLater( + 'example_cli some_command --multi-c yeahoo ', + suggests(allOptionsInThisLevel), + ); + }); + + test('include multi option after it is specified (abbr)', () async { + await expectLater( + 'example_cli some_command -n yeahoo ', + suggests(allOptionsInThisLevel), + ); + }); + + test( + 'include option after it is specified (abbr joined)', + () async { + await expectLater( + 'example_cli some_command -nyeahoo ', + suggests(allOptionsInThisLevel), + ); + }, + tags: 'known-issues', + ); + + test('include discrete multi option value after it is specified', + () async { + await expectLater( + 'example_cli some_command --multi-d bar -m ', + suggests({ + 'fii': 'fii help', + 'bar': 'bar help', + 'fee': 'fee help', + 'i have space': 'an allowed option with space on it' + }), + ); + }); + }); + }); }); group('some_other_command', () { diff --git a/example/test/integration/utils.dart b/example/test/integration/utils.dart index d595633..b00e8d1 100644 --- a/example/test/integration/utils.dart +++ b/example/test/integration/utils.dart @@ -88,3 +88,9 @@ Future> runCompletionCommand( return map; } + +extension CompletionUtils on Map { + Map except(String key) { + return Map.from(this)..remove(key); + } +} diff --git a/lib/src/parser/arg_parser_extension.dart b/lib/src/parser/arg_parser_extension.dart index 04fa66d..8d0449b 100644 --- a/lib/src/parser/arg_parser_extension.dart +++ b/lib/src/parser/arg_parser_extension.dart @@ -16,6 +16,20 @@ bool isAbbr(String string) => _abbrRegex.hasMatch(string); /// Extends [ArgParser] with utility methods that allow parsing a completion /// input, which in most cases only regards part of the rules. extension ArgParserExtension on ArgParser { + /// Tries to parse the minimal subset of valid [args] as valid options. + ArgResults? findValidOptions(List args) { + final loosenOptionsGramamar = _looseOptions(); + var currentArgs = args; + while (currentArgs.isNotEmpty) { + try { + return loosenOptionsGramamar.parse(currentArgs); + } catch (_) { + currentArgs = currentArgs.take(currentArgs.length - 1).toList(); + } + } + return null; + } + /// Parses [args] with this [ArgParser]'s command structure only, ignore /// option strict rules (mandatory, allowed values, non negatable flags, /// default values, etc); @@ -25,7 +39,7 @@ extension ArgParserExtension on ArgParser { /// Returns null if there is an error when parsing, which means the given args /// do not respect the known command structure. ArgResults? tryParseCommandsOnly(Iterable args) { - final commandsOnlyGrammar = _looseOptions(); + final commandsOnlyGrammar = _cloneCommandsOnly(); final filteredArgs = args.where((element) { return !isAbbr(element) && !isOption(element) && element.isNotEmpty; @@ -40,16 +54,58 @@ extension ArgParserExtension on ArgParser { } /// Recursively copies this [ArgParser] without options. - ArgParser _looseOptions() { + ArgParser _cloneCommandsOnly() { final clonedArgParser = ArgParser( allowTrailingOptions: allowTrailingOptions, ); for (final entry in commands.entries) { - final parser = entry.value._looseOptions(); + final parser = entry.value._cloneCommandsOnly(); clonedArgParser.addCommand(entry.key, parser); } return clonedArgParser; } + + /// Copies this [ArgParser] with a less strict option mapping. + /// + /// It preserves only the options names, types, abbreviations and aliases. + /// + /// It disregard subcommands. + ArgParser _looseOptions() { + final clonedArgParser = ArgParser( + allowTrailingOptions: allowTrailingOptions, + ); + + for (final entry in options.entries) { + final option = entry.value; + + if (option.isFlag) { + clonedArgParser.addFlag( + option.name, + abbr: option.abbr, + aliases: option.aliases, + negatable: option.negatable ?? true, + ); + } + + if (option.isSingle) { + clonedArgParser.addOption( + option.name, + abbr: option.abbr, + aliases: option.aliases, + ); + } + + if (option.isMultiple) { + clonedArgParser.addMultiOption( + option.name, + abbr: option.abbr, + aliases: option.aliases, + ); + } + } + + return clonedArgParser; + } } diff --git a/lib/src/parser/completion_level.dart b/lib/src/parser/completion_level.dart index a753dee..12aa675 100644 --- a/lib/src/parser/completion_level.dart +++ b/lib/src/parser/completion_level.dart @@ -18,6 +18,7 @@ class CompletionLevel { @visibleForTesting const CompletionLevel({ required this.grammar, + this.parsedOptions, required this.rawArgs, required this.visibleSubcommands, required this.visibleOptions, @@ -90,17 +91,24 @@ class CompletionLevel { rawArgs = rootArgs.toList(); } + final validOptionsResult = originalGrammar.findValidOptions(rawArgs); + final visibleSubcommands = subcommands?.values.where((command) { return !command.hidden; }).toList() ?? []; final visibleOptions = originalGrammar.options.values.where((option) { + final wasParsed = validOptionsResult?.wasParsed(option.name) ?? false; + if (wasParsed) { + return option.isMultiple; + } return !option.hide; }).toList(); return CompletionLevel( grammar: originalGrammar, + parsedOptions: validOptionsResult, rawArgs: rawArgs, visibleSubcommands: visibleSubcommands, visibleOptions: visibleOptions, @@ -111,6 +119,10 @@ class CompletionLevel { /// needs completion. final ArgParser grammar; + /// An [ArgResults] that includes the valid options passed to the command on + /// completion level. Null if no valid options were passed. + final ArgResults? parsedOptions; + /// The user input that needs completion starting from the /// command/sub_command being completed. final List rawArgs; diff --git a/test/src/parser/arg_parser_extension_test.dart b/test/src/parser/arg_parser_extension_test.dart index 275f663..ee540e9 100644 --- a/test/src/parser/arg_parser_extension_test.dart +++ b/test/src/parser/arg_parser_extension_test.dart @@ -39,7 +39,7 @@ void main() { final subArgPasrser = ArgParser(); rootArgParser.addCommand('subcommand', subArgPasrser); - test('parses commands only disregarding strict option rules', () { + test('parses commands only disregarding all option rules', () { final args = '--rootFlag subcommand'.split(' '); final results = rootArgParser.tryParseCommandsOnly(args); @@ -72,12 +72,93 @@ void main() { expect(results, isNull); }); - test('returns null when args make no sense', () { + test('parses args with leading spaces', () { final args = ' subcommand'.split(' '); final results = rootArgParser.tryParseCommandsOnly(args); expect(results, isNotNull); }); }); + group('findValidOptions', () { + final argParser = ArgParser() + ..addFlag( + 'flag', + abbr: 'f', + aliases: ['flagalias'], + ) + ..addOption('mandatoryOption', mandatory: true) + ..addMultiOption('multiOption', allowed: ['a', 'b', 'c']) + ..addCommand('subcommand', ArgParser()); + + group('parses the minimal amount of valid options', () { + test('options values and abbr', () { + final args = '-f --mandatoryOption="yay" --multiOption'.split(' '); + final results = argParser.findValidOptions(args); + + expect( + results, + isA() + .having((res) => res.wasParsed('flag'), 'parsed flag', true) + .having( + (res) => res.wasParsed('mandatoryOption'), + 'parsed mandatoryOption', + true, + ) + .having( + (res) => res.wasParsed('multiOption'), + 'parsed multiOption', + false, + ), + ); + }); + test('alias', () { + final args = '--flagalias --mandatoryOption'.split(' '); + final results = argParser.findValidOptions(args); + + expect( + results, + isA() + .having((res) => res.wasParsed('flag'), 'parsed flag', true) + .having( + (res) => res.wasParsed('mandatoryOption'), + 'parsed mandatoryOption', + false, + ) + .having( + (res) => res.wasParsed('multiOption'), + 'parsed multiOption', + false, + ), + ); + }); + }); + test('returns null when there is no valid options', () { + final args = '--extraneousOption --flag'.split(' '); + final results = argParser.findValidOptions(args); + + expect(results, isNull); + }); + test('disregard sub command', () { + final args = '--flag subcommand'.split(' '); + final results = argParser.findValidOptions(args); + + expect( + results, + isA() + .having((res) => res.wasParsed('flag'), 'parsed flag', true) + .having( + (res) => res.wasParsed('mandatoryOption'), + 'parsed mandatoryOption', + false, + ) + .having( + (res) => res.wasParsed('multiOption'), + 'parsed multiOption', + false, + ) + .having((res) => res.command, 'command', isNull), + ); + }); + }); }); } diff --git a/test/src/parser/completion_level_test.dart b/test/src/parser/completion_level_test.dart index 41c46ef..8a0082b 100644 --- a/test/src/parser/completion_level_test.dart +++ b/test/src/parser/completion_level_test.dart @@ -26,7 +26,12 @@ class _TestCompletionCommandRunner extends CompletionCommandRunner { description: 'subcommand level 2', ); subCommand.addSubcommand(subSubCommand); - subSubCommand.argParser.addFlag('level2Flag'); + subSubCommand.argParser + ..addFlag('level2Flag') + ..addOption( + 'level2Option', + mandatory: true, + ); final subSubSubCommand = _TestCommand( name: 'subsubsubcommand', @@ -75,6 +80,15 @@ void main() { '--level2Flag', ]); + expect( + completionLevel.parsedOptions, + isA().having( + (results) => results.wasParsed('level2Flag'), + 'parsed level2Flag', + true, + ), + ); + expect( completionLevel.visibleOptions, isA>() @@ -91,7 +105,7 @@ void main() { .having( (list) => list[1].name, 'second option in the last level', - 'level2Flag', + 'level2Option', ), );