Skip to content

Commit

Permalink
Remove sorting of allowedHelp maps (#852)
Browse files Browse the repository at this point in the history
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.

Remove the additional list copy and iterate the map keys directly.

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.

Refactor a few null check conditions to variable assignment patterns.
  • Loading branch information
natebosch authored Jan 17, 2025
1 parent a59cbea commit 72a2060
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 9 deletions.
2 changes: 2 additions & 0 deletions pkgs/args/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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.
Expand Down
12 changes: 12 additions & 0 deletions pkgs/args/lib/src/arg_parser.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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).
Expand Down
10 changes: 4 additions & 6 deletions pkgs/args/lib/src/usage.dart
Original file line number Diff line number Diff line change
Expand Up @@ -87,15 +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) {
var allowedNames = option.allowedHelp!.keys.toList();
allowedNames.sort();
if (option.allowedHelp case final allowedHelp?) {
_newline();
for (var name in allowedNames) {
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) {
Expand Down
6 changes: 3 additions & 3 deletions pkgs/args/test/usage_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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
''');
});

Expand All @@ -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
''');
});

Expand All @@ -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
''');
});

Expand Down

0 comments on commit 72a2060

Please sign in to comment.