Skip to content

Commit 72a2060

Browse files
authored
Remove sorting of allowedHelp maps (#852)
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.
1 parent a59cbea commit 72a2060

File tree

4 files changed

+21
-9
lines changed

4 files changed

+21
-9
lines changed

pkgs/args/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
## 2.6.1-wip
22

3+
* Remove sorting of the `allowedHelp` argument in usage output. Ordering will
4+
depend on key order for the passed `Map`.
35
* Fix the repository URL in `pubspec.yaml`.
46
* Added option `hideNegatedUsage` to `ArgParser.flag()` allowing a flag to be
57
`negatable` without showing it in the usage text.

pkgs/args/lib/src/arg_parser.dart

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,12 @@ class ArgParser {
177177
///
178178
/// The [allowedHelp] argument is a map from values in [allowed] to
179179
/// documentation for those values that will be included in [usage].
180+
/// The map may include a subset of the allowed values.
181+
/// Additional values that are not in [allowed] should be omitted, however
182+
/// there is no validation.
183+
/// When both [allowed] and [allowedHelp] are passed, only [allowed] will
184+
/// be validated at parse time, and only [allowedHelp] will be included in
185+
/// usage output.
180186
///
181187
/// The [defaultsTo] argument indicates the value this option will have if the
182188
/// user doesn't explicitly pass it in (or `null` by default).
@@ -231,6 +237,12 @@ class ArgParser {
231237
///
232238
/// The [allowedHelp] argument is a map from values in [allowed] to
233239
/// documentation for those values that will be included in [usage].
240+
/// The map may include a subset of the allowed values.
241+
/// Additional values that are not in [allowed] should be omitted, however
242+
/// there is no validation.
243+
/// When both [allowed] and [allowedHelp] are passed, only [allowed] will
244+
/// be validated at parse time, and only [allowedHelp] will be included in
245+
/// usage output.
234246
///
235247
/// The [defaultsTo] argument indicates the values this option will have if
236248
/// the user doesn't explicitly pass it in (or `[]` by default).

pkgs/args/lib/src/usage.dart

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -87,15 +87,13 @@ class _Usage {
8787
_write(0, _abbreviation(option));
8888
_write(1, '${_longOption(option)}${_mandatoryOption(option)}');
8989

90-
if (option.help != null) _write(2, option.help!);
90+
if (option.help case final help?) _write(2, help);
9191

92-
if (option.allowedHelp != null) {
93-
var allowedNames = option.allowedHelp!.keys.toList();
94-
allowedNames.sort();
92+
if (option.allowedHelp case final allowedHelp?) {
9593
_newline();
96-
for (var name in allowedNames) {
94+
for (var MapEntry(key: name, value: content) in allowedHelp.entries) {
9795
_write(1, _allowedTitle(option, name));
98-
_write(2, option.allowedHelp![name]!);
96+
_write(2, content);
9997
}
10098
_newline();
10199
} else if (option.allowed != null) {

pkgs/args/test/usage_test.dart

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -216,10 +216,10 @@ void main() {
216216
validateUsage(parser, '''
217217
--suit Like in cards
218218
219+
[spades] Swords of a soldier
219220
[clubs] Weapons of war
220221
[diamonds] Money for this art
221222
[hearts] The shape of my heart
222-
[spades] Swords of a soldier
223223
''');
224224
});
225225

@@ -244,10 +244,10 @@ void main() {
244244
validateUsage(parser, '''
245245
--suit Like in cards
246246
247+
[spades] Swords of a soldier
247248
[clubs] (default) Weapons of war
248249
[diamonds] Money for this art
249250
[hearts] The shape of my heart
250-
[spades] Swords of a soldier
251251
''');
252252
});
253253

@@ -271,10 +271,10 @@ void main() {
271271
validateUsage(parser, '''
272272
--suit Like in cards
273273
274+
[spades] Swords of a soldier
274275
[clubs] (default) Weapons of war
275276
[diamonds] Money for this art
276277
[hearts] (default) The shape of my heart
277-
[spades] Swords of a soldier
278278
''');
279279
});
280280

0 commit comments

Comments
 (0)