From f477bdd911aacffc10cdb9ca46645a54545d6d3f Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Fri, 17 Jan 2025 00:14:13 +0000 Subject: [PATCH 1/4] Remove sorting of allowedHelp maps Closes #845 It is idiomatic to treat the key order of a Dart map as meaningful given that map literals and default Map type preserve key insertion order. It is more useful to allow the caller to decide this order than to mandate an alpha sorting by key. Callers which need this order can construct the map appropriately, and callers which prefer a different order now have the capability. Releasing as a non-breaking change since specific usage output is considered an implementation detail. This is expected to impact some CI statuf for packages with tests hardcoding a strict dependency on the output. No additional tests are necessary since updating the order in existing tests demonstrates the same behavior as adding a non-sorting specific test. --- pkgs/args/CHANGELOG.md | 2 ++ pkgs/args/lib/src/usage.dart | 3 +-- pkgs/args/test/usage_test.dart | 6 +++--- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/pkgs/args/CHANGELOG.md b/pkgs/args/CHANGELOG.md index b97daf577..ec6bd0112 100644 --- a/pkgs/args/CHANGELOG.md +++ b/pkgs/args/CHANGELOG.md @@ -1,5 +1,7 @@ ## 2.6.1-wip +* Remove sorting of the `allowedHelp` argument in usage output. Ordering will + depend on key order for the passed `Map`. * Fix the repository URL in `pubspec.yaml`. * Added option `hideNegatedUsage` to `ArgParser.flag()` allowing a flag to be `negatable` without showing it in the usage text. diff --git a/pkgs/args/lib/src/usage.dart b/pkgs/args/lib/src/usage.dart index bd39f1121..27c85cd57 100644 --- a/pkgs/args/lib/src/usage.dart +++ b/pkgs/args/lib/src/usage.dart @@ -90,8 +90,7 @@ class _Usage { if (option.help != null) _write(2, option.help!); if (option.allowedHelp != null) { - var allowedNames = option.allowedHelp!.keys.toList(); - allowedNames.sort(); + var allowedNames = [...option.allowedHelp!.keys]; _newline(); for (var name in allowedNames) { _write(1, _allowedTitle(option, name)); diff --git a/pkgs/args/test/usage_test.dart b/pkgs/args/test/usage_test.dart index 6f5315b51..95a5ef178 100644 --- a/pkgs/args/test/usage_test.dart +++ b/pkgs/args/test/usage_test.dart @@ -216,10 +216,10 @@ void main() { validateUsage(parser, ''' --suit Like in cards + [spades] Swords of a soldier [clubs] Weapons of war [diamonds] Money for this art [hearts] The shape of my heart - [spades] Swords of a soldier '''); }); @@ -244,10 +244,10 @@ void main() { validateUsage(parser, ''' --suit Like in cards + [spades] Swords of a soldier [clubs] (default) Weapons of war [diamonds] Money for this art [hearts] The shape of my heart - [spades] Swords of a soldier '''); }); @@ -271,10 +271,10 @@ void main() { validateUsage(parser, ''' --suit Like in cards + [spades] Swords of a soldier [clubs] (default) Weapons of war [diamonds] Money for this art [hearts] (default) The shape of my heart - [spades] Swords of a soldier '''); }); From 22011189787775942d3a5650d197d536320e82ca Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Fri, 17 Jan 2025 00:22:09 +0000 Subject: [PATCH 2/4] Remove the variable, no need to copy into a list --- pkgs/args/lib/src/usage.dart | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkgs/args/lib/src/usage.dart b/pkgs/args/lib/src/usage.dart index 27c85cd57..f349cb64e 100644 --- a/pkgs/args/lib/src/usage.dart +++ b/pkgs/args/lib/src/usage.dart @@ -90,9 +90,8 @@ class _Usage { if (option.help != null) _write(2, option.help!); if (option.allowedHelp != null) { - var allowedNames = [...option.allowedHelp!.keys]; _newline(); - for (var name in allowedNames) { + for (var name in option.allowedHelp!.keys) { _write(1, _allowedTitle(option, name)); _write(2, option.allowedHelp![name]!); } From 4f920415fb7a6561e56e9c53d9a9b9636e2c341f Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Fri, 17 Jan 2025 00:30:38 +0000 Subject: [PATCH 3/4] Expand docs --- pkgs/args/lib/src/arg_parser.dart | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/pkgs/args/lib/src/arg_parser.dart b/pkgs/args/lib/src/arg_parser.dart index 50c3991dd..37041d7f4 100644 --- a/pkgs/args/lib/src/arg_parser.dart +++ b/pkgs/args/lib/src/arg_parser.dart @@ -177,6 +177,12 @@ class ArgParser { /// /// The [allowedHelp] argument is a map from values in [allowed] to /// documentation for those values that will be included in [usage]. + /// The map may include a subset of the allowed values. + /// Additional values that are not in [allowed] should be omitted, however + /// there is no validation. + /// When both [allowed] and [allowedHelp] are passed, only [allowed] will + /// be validated at parse time, and only [allowedHelp] will be included in + /// usage output. /// /// The [defaultsTo] argument indicates the value this option will have if the /// user doesn't explicitly pass it in (or `null` by default). @@ -231,6 +237,12 @@ class ArgParser { /// /// The [allowedHelp] argument is a map from values in [allowed] to /// documentation for those values that will be included in [usage]. + /// The map may include a subset of the allowed values. + /// Additional values that are not in [allowed] should be omitted, however + /// there is no validation. + /// When both [allowed] and [allowedHelp] are passed, only [allowed] will + /// be validated at parse time, and only [allowedHelp] will be included in + /// usage output. /// /// The [defaultsTo] argument indicates the values this option will have if /// the user doesn't explicitly pass it in (or `[]` by default). From 585a44e9c900378ec6498b7bb938476549efcccf Mon Sep 17 00:00:00 2001 From: Nate Bosch Date: Fri, 17 Jan 2025 19:24:54 +0000 Subject: [PATCH 4/4] Refactor some null checks into case patterns to avoid ! --- pkgs/args/lib/src/usage.dart | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkgs/args/lib/src/usage.dart b/pkgs/args/lib/src/usage.dart index f349cb64e..c6b531086 100644 --- a/pkgs/args/lib/src/usage.dart +++ b/pkgs/args/lib/src/usage.dart @@ -87,13 +87,13 @@ class _Usage { _write(0, _abbreviation(option)); _write(1, '${_longOption(option)}${_mandatoryOption(option)}'); - if (option.help != null) _write(2, option.help!); + if (option.help case final help?) _write(2, help); - if (option.allowedHelp != null) { + if (option.allowedHelp case final allowedHelp?) { _newline(); - for (var name in option.allowedHelp!.keys) { + for (var MapEntry(key: name, value: content) in allowedHelp.entries) { _write(1, _allowedTitle(option, name)); - _write(2, option.allowedHelp![name]!); + _write(2, content); } _newline(); } else if (option.allowed != null) {