Skip to content

Remove sorting of allowedHelp maps #852

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jan 17, 2025
Merged

Remove sorting of allowedHelp maps #852

merged 4 commits into from
Jan 17, 2025

Conversation

natebosch
Copy link
Member

@natebosch natebosch commented Jan 17, 2025

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.

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.
Copy link

Package publishing

Package Version Status Publish tag (post-merge)
package:os_detect 2.0.3 ready to publish os_detect-v2.0.3
package:args 2.6.1-wip WIP (no publish necessary)
package:async 2.12.0 already published at pub.dev
package:characters 1.4.0 already published at pub.dev
package:collection 1.20.0-wip WIP (no publish necessary)
package:convert 3.1.2 already published at pub.dev
package:crypto 3.0.6 already published at pub.dev
package:fixnum 1.1.1 already published at pub.dev
package:lints 5.1.1 already published at pub.dev
package:logging 1.3.0 already published at pub.dev
package:path 1.9.2-wip WIP (no publish necessary)
package:platform 3.1.6 already published at pub.dev
package:typed_data 1.4.0 already published at pub.dev

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

Copy link

github-actions bot commented Jan 17, 2025

PR Health

Breaking changes ⚠️
Package Change Current Version New Version Needed Version Looking good?
args Non-Breaking 2.6.0 2.6.1-wip 2.7.0
Got "2.6.1-wip" expected >= "2.7.0" (non-breaking changes)
⚠️

This check can be disabled by tagging the PR with skip-breaking-check.

Changelog Entry ✔️
Package Changed Files

Changes to files need to be accounted for in their respective changelogs.

Coverage ⚠️
File Coverage
pkgs/args/lib/src/arg_parser.dart 💚 94 %
pkgs/args/lib/src/usage.dart 💔 99 % ⬇️ 0 %

This check for test coverage is informational (issues shown here will not fail the PR).

This check can be disabled by tagging the PR with skip-coverage-check.

API leaks ✔️

The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.

Package Leaked API symbols
License Headers ✔️
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
Files
no missing headers

All source files should start with a license header.

_newline();
for (var name in allowedNames) {
for (var name in option.allowedHelp!.keys) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: consider an if case to extract this as a variable and avoid the !

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, did a couple nearby ones as well.

@natebosch natebosch merged commit 72a2060 into main Jan 17, 2025
14 checks passed
@natebosch natebosch deleted the arg-allowed-help-order branch January 17, 2025 19:34
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Jan 23, 2025
Revisions updated by `dart tools/rev_sdk_deps.dart`.

core (https://github.com/dart-lang/core/compare/7a71ad6..72a2060):
  72a20603  2025-01-17  Nate Bosch  Remove sorting of allowedHelp maps (dart-lang/core#852)
  a59cbeaf  2025-01-13  Kevin Moore  [os_detect] move to pkg:web, prepare publish (dart-lang/core#850)
  48c3c458  2025-01-13  Kevin Moore  [path] fix tests to compile/run with wasm (dart-lang/core#851)
  2ac228bd  2025-01-09  Devon Carew  update lint issue templates (dart-lang/core#849)
  513fa2f0  2025-01-08  Devon Carew  fix a line break issue in the package table (dart-lang/core#846)
  eb74f032  2025-01-06  Devon Carew  Update package:collection for the new `strict_top_level_inference` lint (dart-lang/core#735)

ecosystem (https://github.com/dart-lang/ecosystem/compare/efe4ee4..682c8ef):
  682c8ef  2025-01-17  Moritz  Update `dart_apitool` version in health workflow. (dart-lang/ecosystem#338)
  94b4cea  2025-01-14  Kevin Moore  [puppy] actually rename command to `run`, sort directories, better logs (dart-lang/ecosystem#337)
  f2d3178  2025-01-10  Devon Carew  rename the 'map' command to 'run' (dart-lang/ecosystem#336)
  25c57b8  2025-01-10  Kevin Moore  [puppy] Introduce a package for misc Dart CLI tools (dart-lang/ecosystem#335)
  21e81c5  2025-01-08  Kevin Moore  [corpus] update to latest analyzer, bump min SDK (dart-lang/ecosystem#334)

http (https://github.com/dart-lang/http/compare/6ecd13a..27184eb):
  27184eb  2025-01-21  Brian Quinlan  Remove bespoke code that handled blocking callbacks (dart-lang/http#1450)
  7271367  2025-01-20  Kevin Moore  [http] prepare v1.3.0 release (dart-lang/http#1451)
  7f50fc5  2025-01-15  Brian Quinlan  Prepare cupertino_http 2.0.2 for release (dart-lang/http#1449)
  2bc4cc9  2025-01-15  Brian Quinlan  Fix incorrect response processing (dart-lang/http#1448)
  531d3e5  2025-01-09  Kevin Moore  [http2, conformance_tests] update dependencies (dart-lang/http#1443)
  f0bcf02  2025-01-06  Brian Quinlan  Add tests to verify NUL, CR & LF header value behavior (dart-lang/http#1440)

test (https://github.com/dart-lang/test/compare/f364fc8..7fc9521):
  7fc95218  2025-01-17  Nate Bosch  Drop reference to legacy communication protocol (dart-lang/test#2446)
  073ef8bd  2025-01-17  Matan Lurey  Fix a type error that occurs comparing two large maps with `deepEquals`. (dart-lang/test#2442)
  b5bfba54  2025-01-17  Nate Bosch  Unblock CI: Ignore deprecation and skip a test (dart-lang/test#2445)

web (https://github.com/dart-lang/web/compare/af5de5e..fd3d988):
  fd3d988  2025-01-14  KennethHung  Adapt paths for different development environments (dart-lang/web#336)
  7f440e4  2025-01-10  KennethHung  Remove renames where the type doesn't exist anymore (dart-lang/web#334)
  3b06da9  2025-01-09  KennethHung  Add missing FileReader event getters (dart-lang/web#333)

webdev (https://github.com/dart-lang/webdev/compare/e72f365..4a92ba5):
  4a92ba58  2025-01-13  Ben Konyi  [ DWDS ] Reset version to 24.3.3-wip (dart-lang/webdev#2569)
  cc5aff69  2025-01-10  Ben Konyi  [ DWDS ] Relax DDS constraint, prepare for 24.3.2 release
  562ab622  2025-01-10  Ben Konyi  [ DWDS ] Reset version to 24.3.2-wip
  a7c8fe76  2025-01-10  Ben Konyi  Allow for setting custom DDS port (dart-lang/webdev#2546)
  7dc5c3a9  2025-01-09  Ben Konyi  Rename copied SDK directory prefix from 'sdk copy' to 'sdk_copy' (dart-lang/webdev#2557)
  659e1dfc  2025-01-08  Jessy Yameogo  Added support for some debugging APIs with the DDC library bundle format - part 5 (dart-lang/webdev#2549)
  fcd906fc  2025-01-07  Srujan Gaddam  Add ignores for deprecated web libraries with a TODO and move dwds to 24.4.0-wip (dart-lang/webdev#2559)

Change-Id: I3e847d24146bfb6891e77098591b785a0d15a445
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/405243
Auto-Submit: Devon Carew <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Konstantin Shcheglov <[email protected]>
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Jan 23, 2025
This reverts commit a1e5451.

Reason for revert: it looks like golem is broken

Original change's description:
> [deps] rev core, ecosystem, http, test, web, webdev
>
> Revisions updated by `dart tools/rev_sdk_deps.dart`.
>
> core (https://github.com/dart-lang/core/compare/7a71ad6..72a2060):
>   72a20603  2025-01-17  Nate Bosch  Remove sorting of allowedHelp maps (dart-lang/core#852)
>   a59cbeaf  2025-01-13  Kevin Moore  [os_detect] move to pkg:web, prepare publish (dart-lang/core#850)
>   48c3c458  2025-01-13  Kevin Moore  [path] fix tests to compile/run with wasm (dart-lang/core#851)
>   2ac228bd  2025-01-09  Devon Carew  update lint issue templates (dart-lang/core#849)
>   513fa2f0  2025-01-08  Devon Carew  fix a line break issue in the package table (dart-lang/core#846)
>   eb74f032  2025-01-06  Devon Carew  Update package:collection for the new `strict_top_level_inference` lint (dart-lang/core#735)
>
> ecosystem (https://github.com/dart-lang/ecosystem/compare/efe4ee4..682c8ef):
>   682c8ef  2025-01-17  Moritz  Update `dart_apitool` version in health workflow. (dart-lang/ecosystem#338)
>   94b4cea  2025-01-14  Kevin Moore  [puppy] actually rename command to `run`, sort directories, better logs (dart-lang/ecosystem#337)
>   f2d3178  2025-01-10  Devon Carew  rename the 'map' command to 'run' (dart-lang/ecosystem#336)
>   25c57b8  2025-01-10  Kevin Moore  [puppy] Introduce a package for misc Dart CLI tools (dart-lang/ecosystem#335)
>   21e81c5  2025-01-08  Kevin Moore  [corpus] update to latest analyzer, bump min SDK (dart-lang/ecosystem#334)
>
> http (https://github.com/dart-lang/http/compare/6ecd13a..27184eb):
>   27184eb  2025-01-21  Brian Quinlan  Remove bespoke code that handled blocking callbacks (dart-lang/http#1450)
>   7271367  2025-01-20  Kevin Moore  [http] prepare v1.3.0 release (dart-lang/http#1451)
>   7f50fc5  2025-01-15  Brian Quinlan  Prepare cupertino_http 2.0.2 for release (dart-lang/http#1449)
>   2bc4cc9  2025-01-15  Brian Quinlan  Fix incorrect response processing (dart-lang/http#1448)
>   531d3e5  2025-01-09  Kevin Moore  [http2, conformance_tests] update dependencies (dart-lang/http#1443)
>   f0bcf02  2025-01-06  Brian Quinlan  Add tests to verify NUL, CR & LF header value behavior (dart-lang/http#1440)
>
> test (https://github.com/dart-lang/test/compare/f364fc8..7fc9521):
>   7fc95218  2025-01-17  Nate Bosch  Drop reference to legacy communication protocol (dart-lang/test#2446)
>   073ef8bd  2025-01-17  Matan Lurey  Fix a type error that occurs comparing two large maps with `deepEquals`. (dart-lang/test#2442)
>   b5bfba54  2025-01-17  Nate Bosch  Unblock CI: Ignore deprecation and skip a test (dart-lang/test#2445)
>
> web (https://github.com/dart-lang/web/compare/af5de5e..fd3d988):
>   fd3d988  2025-01-14  KennethHung  Adapt paths for different development environments (dart-lang/web#336)
>   7f440e4  2025-01-10  KennethHung  Remove renames where the type doesn't exist anymore (dart-lang/web#334)
>   3b06da9  2025-01-09  KennethHung  Add missing FileReader event getters (dart-lang/web#333)
>
> webdev (https://github.com/dart-lang/webdev/compare/e72f365..4a92ba5):
>   4a92ba58  2025-01-13  Ben Konyi  [ DWDS ] Reset version to 24.3.3-wip (dart-lang/webdev#2569)
>   cc5aff69  2025-01-10  Ben Konyi  [ DWDS ] Relax DDS constraint, prepare for 24.3.2 release
>   562ab622  2025-01-10  Ben Konyi  [ DWDS ] Reset version to 24.3.2-wip
>   a7c8fe76  2025-01-10  Ben Konyi  Allow for setting custom DDS port (dart-lang/webdev#2546)
>   7dc5c3a9  2025-01-09  Ben Konyi  Rename copied SDK directory prefix from 'sdk copy' to 'sdk_copy' (dart-lang/webdev#2557)
>   659e1dfc  2025-01-08  Jessy Yameogo  Added support for some debugging APIs with the DDC library bundle format - part 5 (dart-lang/webdev#2549)
>   fcd906fc  2025-01-07  Srujan Gaddam  Add ignores for deprecated web libraries with a TODO and move dwds to 24.4.0-wip (dart-lang/webdev#2559)
>
> Change-Id: I3e847d24146bfb6891e77098591b785a0d15a445
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/405243
> Auto-Submit: Devon Carew <[email protected]>
> Reviewed-by: Konstantin Shcheglov <[email protected]>
> Commit-Queue: Konstantin Shcheglov <[email protected]>

Change-Id: I27c20ef89095a7d602c1a609d5533a27a7683a47
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/405445
Commit-Queue: Konstantin Shcheglov <[email protected]>
Auto-Submit: Devon Carew <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>
Bot-Commit: Rubber Stamper <[email protected]>
natebosch added a commit to dart-lang/test that referenced this pull request Mar 31, 2025
Previous versions of `package:args` alphabetically sorted the
`allowedHelp` entries for help output. The latest release removes the
sorting and breaks the change-detection tests for the help output.

dart-lang/core#852

Manually sort the keys of the reporters map to match the alpha sort of
the old output.
natebosch added a commit to dart-lang/test that referenced this pull request Mar 31, 2025
Previous versions of `package:args` alphabetically sorted the
`allowedHelp` entries for help output. The latest release removes the
sorting and breaks the change-detection tests for the help output.

dart-lang/core#852

Manually sort the keys of the reporters map to match the alpha sort of
the old output.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[args] Inconsistent order between allowed and allowedHelp
2 participants