Repository navigation
refactor(flutter_tools): migrate AttachCommand to OptionDescriptor and OptionBundle - #194359
Conversation
…OptionDescriptor Delegates the remaining shared `FlutterCommand` `uses*` / `add*` helpers and internal `boolArg` / `stringArg` / `stringsArg` lookups to typed `OptionDescriptor` instances and reusable `OptionBundle` classes (`DeviceDiscoveryOptionsBundle`, `FilesystemOptionsBundle`, `VmServicePortOptionsBundle`, `DdsAndDevToolsOptionsBundle`, `ResidentCompilerOptionsBundle`, and `EngineRenderingFlagsBundle`), and enhances `BuildModeOptionsBundle` (`defaultToRelease`, `excludeDebug`, `excludeRelease`). Part of flutter#194338
…classes in attach.dart
…ildModeOptionsBundle
…es (flutter#194356) Moves the remaining shared CLI options on `FlutterCommand` into declarative option descriptors and bundles, and replaces untyped argument lookups in the command runner with typed descriptor reads. - **Shared option bundles for leaf commands**: Groups common CLI flags—device discovery, VM service/DevTools ports, filesystem overrides, resident compiler settings, engine rendering flags, and build mode configuration—into reusable bundles. This sets the stage for follow-up PRs migrating `assemble` (flutter#194358), `attach` (flutter#194359), `drive` (flutter#194360), `test` (flutter#194361), and `run` (flutter#194362). - **Deduplicated build flags**: Consolidates duplicate definitions and drifted help text for `--config-only`, `--target-platform` (`build apk`, `build aar`, `build appbundle`), `--static` (`build ios-framework`, `build macos-framework`, `build swift-package`), and `--performance-measurement-file`. Part of flutter#194338 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.
There was a problem hiding this comment.
Code Review
This pull request refactors 'AttachCommand' to migrate command-line option registration to the 'OptionBundle' and 'OptionDescriptor' patterns, replacing imperative setup methods and direct 'argResults' lookups with 'registerOptionBundles', 'addDescriptors', and 'getValue'. Feedback points out a regression where the '--ipv6' flag is no longer registered because 'usesIpv6Flag' was removed but 'DebuggingOptionDescriptors.ipv6' was not added to the new 'addDescriptors' list.
harryterkelsen
left a comment
There was a problem hiding this comment.
I suggest adding tests to verify that the hidden options are the same before and after this refactoring
…ltering visibility
|
Added a unit test in |
flutter/flutter@5c7e45a...a3d71d8 2026-10-16 [email protected] 0 Initialize variables in the DlComplexityPathReceiver helper (flutter/flutter#194540) 2026-10-16 [email protected] Roll Fuchsia Test Scripts from V-qQL4158Nw6L-QLi... to 2T18KC7-LDQtmQiLP... (flutter/flutter#194527) 2026-10-16 [email protected] Remove BorderMaskBlur (flutter/flutter#194508) 2026-10-16 [email protected] Roll Skia from a577264dc087 to 470b8816a261 (1 revision) (flutter/flutter#194526) 2026-10-16 [email protected] [ios] Use DFS traversal to find engines of a scene automatically (rather than manually manage the engines list) (flutter/flutter#192457) 2026-10-16 [email protected] [Skia][DisplayList] Map local bounds through the local matrix in DlLocalMatrixImageFilter (flutter/flutter#193963) 2026-10-16 [email protected] [framework] Fix avoid_setters_without_getters warnings on mixin declarations (flutter/flutter#193856) 2026-10-16 [email protected] Roll Dart SDK from 84686ea04815 to 55a734a01fb1 (3 revisions) (flutter/flutter#194522) 2026-10-16 [email protected] Roll Skia from d131b7789482 to a577264dc087 (7 revisions) (flutter/flutter#194514) 2026-10-16 [email protected] Roll Fuchsia GN SDK from PDJz3b3CL31uoLOkD... to kFdyRnojbVy0cxvRZ... (flutter/flutter#194510) 2026-10-16 [email protected] Roll GN to f9eca75ae554 (flutter/flutter#194471) 2026-10-16 [email protected] refactor(flutter_tools): migrate RunCommand and RunCommandBase to OptionDescriptor and OptionBundle (flutter/flutter#194362) 2026-10-15 [email protected] refactor(flutter_tools): finish TestCommand OptionDescriptor and OptionBundle migration (flutter/flutter#194361) 2026-10-15 [email protected] [iOS][a11y] Fix VoiceOver announcement for unfocused password fields (flutter/flutter#192851) 2026-10-15 [email protected] Roll pub packages (flutter/flutter#194477) 2026-10-15 [email protected] refactor(flutter_tools): finish DriveCommand OptionDescriptor migration (flutter/flutter#194360) 2026-10-15 [email protected] [flutter_tools] Roll package:vm_service to 15.4.0 (flutter/flutter#194467) 2026-10-15 [email protected] refactor(flutter_tools): migrate AttachCommand to OptionDescriptor and OptionBundle (flutter/flutter#194359) 2026-10-15 [email protected] [flutter_tools] Inject dependencies into update-packages and package_map (flutter/flutter#194046) 2026-10-15 [email protected] [impeller] parallelize opengles shader compiles (flutter/flutter#194022) 2026-10-15 [email protected] Roll pub packages (flutter/flutter#194469) 2026-10-15 [email protected] Restrict web dev server to loopback by default (flutter/flutter#187595) 2026-10-15 [email protected] Roll Packages from a1ed23c to da40eaf (33 revisions) (flutter/flutter#194459) 2026-10-15 [email protected] [web] Render soft hyphens on Skwasm (flutter/flutter#193850) 2026-10-15 [email protected] refactor(flutter_tools): migrate AssembleCommand to OptionDescriptor (flutter/flutter#194358) 2026-10-15 [email protected] [windows] Add OpenGL golden test against ANGLE at D3D11 feature level 10_0 (flutter/flutter#194321) If this roll has caused a breakage, revert this CL and set the roller to dry run mode using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
…es (flutter#194356) Moves the remaining shared CLI options on `FlutterCommand` into declarative option descriptors and bundles, and replaces untyped argument lookups in the command runner with typed descriptor reads. - **Shared option bundles for leaf commands**: Groups common CLI flags—device discovery, VM service/DevTools ports, filesystem overrides, resident compiler settings, engine rendering flags, and build mode configuration—into reusable bundles. This sets the stage for follow-up PRs migrating `assemble` (flutter#194358), `attach` (flutter#194359), `drive` (flutter#194360), `test` (flutter#194361), and `run` (flutter#194362). - **Deduplicated build flags**: Consolidates duplicate definitions and drifted help text for `--config-only`, `--target-platform` (`build apk`, `build aar`, `build appbundle`), `--static` (`build ios-framework`, `build macos-framework`, `build swift-package`), and `--performance-measurement-file`. Part of flutter#194338 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.
…d OptionBundle (flutter#194359) Migrates `flutter attach` to the shared option bundles and typed option descriptors introduced in flutter#194356, replacing imperative flag registration and untyped argument lookups. Part of flutter#194338 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing.
Migrates
flutter attachto the shared option bundles and typed option descriptors introduced in #194356, replacing imperative flag registration and untyped argument lookups.Part of #194338
Pre-launch Checklist
///).