Repository navigation
feat(flutter_tools): add IntOptionDescriptor and DefaultedIntOptionDescriptor - #192455
Conversation
…scriptor Adds IntOptionDescriptor (OptionDescriptor<int?>) and DefaultedIntOptionDescriptor (OptionDescriptor<int>) to option_descriptor.dart. - Parses single-value integer CLI options and throws a descriptive FormatException on invalid numeric strings. - Migrates DebuggingOptionDescriptors.ddsPort, WebOptions.webPort, and WebOptions.webBrowserDebugPort from StringOptionDescriptor to IntOptionDescriptor. - Eliminates redundant manual int.tryParse / int.parse call sites in FlutterCommand.ddsPort and RunCommand. - Adds comprehensive unit tests in option_descriptor_test.dart.
…ults.getValue - Catches FormatException in SafeArgResults.getValue and calls throwToolExit(e.message) so invalid integer flags (--dds-port=abc, --web-port=abc) exit cleanly instead of triggering unhandled tool crash reports. - Deduplicates integer parsing logic into _IntOptionDescriptorBase._parseValue. - Removes redundant wasParsed(WebOptions.webBrowserDebugPort) check in RunCommandBase. - Adds unit test verifying SafeArgResults.getValue converts FormatException into ToolExit.
There was a problem hiding this comment.
Code Review
This pull request introduces IntOptionDescriptor and DefaultedIntOptionDescriptor to natively support integer command-line options, replacing manual string parsing for options such as dds-port, web-port, and web-browser-debug-port. Additionally, SafeArgResults.getValue is updated to catch FormatException and throw a ToolExit. The review feedback suggests restricting the defaultsTo parameter type to int? in the _IntOptionDescriptorBase constructor to ensure compile-time type safety.
bkonyi
left a comment
There was a problem hiding this comment.
LGTM other than the DDS port concern.
…default 0 - Updates DebuggingOptionDescriptors.ddsPort from IntOptionDescriptor to DefaultedIntOptionDescriptor with defaultsTo: 0. - Simplifies FlutterCommand.ddsPort to directly return getValue(DebuggingOptionDescriptors.ddsPort) when explicit ddsPort or fallback random port 0 is used.
|
autosubmit label was removed for flutter/flutter/192455, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label. |
…lutter#193323) Migrate `BuildLinuxCommand` (`build linux`) and `BuildWindowsCommand` (`build windows`) to typed `OptionDescriptor`s and `OptionBundle`s (following flutter#191018, flutter#191510, flutter#191760, flutter#192307, flutter#192455, flutter#192524, and flutter#192818): - Add `DesktopBuildOptionsBundle` in `common_options.dart`, encapsulating `CommonBuildOptionsBundle`, `BuildModeOptionsBundle`, `DartCompileOptionsBundle`, `BuildInfoOptions.flavor`, `splitDebugInfo`, `obfuscate`, `extraFrontEndOptions`, `extraGenSnapshotOptions`, `performanceMeasurementFile`, `analyzeSize`, `codeSizeDirectory`, `trackWidgetCreation`, and `configOnly`. - Migrate `BuildLinuxCommand` (`build_linux.dart`) to `DesktopBuildOptionsBundle`, `DefaultedEnumOptionDescriptor<TargetPlatform>` (`_targetPlatformOption`), and `DefaultedStringOptionDescriptor` (`_targetSysroot`), replacing untyped `boolArg('config-only')`, `stringArg('target-platform')!`, and `stringArg('target-sysroot')!` queries. - Migrate `BuildWindowsCommand` (`build_windows.dart`) to `DesktopBuildOptionsBundle` and `getValue(DesktopBuildOptionsBundle.configOnly)`. - Delete the obsolete `FlutterCommand.addCommonDesktopBuildOptions` helper method from `flutter_command.dart` (no remaining callers in `flutter_tools`). ## 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]. - [ ] 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.
Adds IntOptionDescriptor (OptionDescriptor<int?>) and DefaultedIntOptionDescriptor (OptionDescriptor) to option_descriptor.dart.