馃 chore: sync skills directory from dart-lang/skills - #236
馃 chore: sync skills directory from dart-lang/skills#236flutter-skills-sync-bot wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates plugin versions and significantly expands the Dart and Flutter skills documentation. It introduces comprehensive guides and templates for building Dart CLI applications, safe cross-platform path manipulation, and idiomatic pattern matching. The review feedback correctly identifies that package:args throws an ArgParserException (which implements FormatException) rather than an ArgumentError when a mandatory option is missing. Consequently, the on ArgumentError block in the single-command tool example is dead code, and the corresponding documentation in SKILL.md should be updated to reflect this behavior.
| } on ArgumentError catch (e) { | ||
| // Missing mandatory option error thrown by results.option(): | ||
| stderr | ||
| ..writeln('Error: ${e.message}') | ||
| ..writeln(parser.usage); | ||
| exitCode = ExitCode.usage.code; |
There was a problem hiding this comment.
In package:args, ArgResults.option(name) does not throw an ArgumentError when a mandatory option is missing from the command-line arguments. Instead, parser.parse(args) will throw an ArgParserException (which implements FormatException) during the parsing phase, which is already handled by the on FormatException block.
An ArgumentError is only thrown by ArgResults if the option name itself was never registered with the ArgParser (which is a developer/programmer error, not a user input error). Therefore, catching ArgumentError here and treating it as a user usage error is misleading and acts as dead code for missing options.
You can safely remove this on ArgumentError block.
| * **The Error Usage Rule**: When an argument parsing or mandatory option error | ||
| occurs (`FormatException`, `UsageException`, or `ArgumentError` thrown when | ||
| accessing a missing `mandatory: true` option via `results.option(...)`), **both | ||
| the error message and the usage text must write to `stderr`**, and exit code | ||
| `64` (`EX_USAGE` / `ExitCode.usage.code`) must be returned. `stdout` should |
There was a problem hiding this comment.
The documentation states that an ArgumentError is thrown when accessing a missing mandatory: true option via results.option(...).
In package:args, if a mandatory option is missing, parser.parse(args) will throw an ArgParserException (which implements FormatException) during parsing. The results.option(...) method is never reached, and even if it were, it does not throw an ArgumentError for missing options (it only throws ArgumentError if the option name itself is unregistered/invalid).
Please update this description to remove the reference to ArgumentError for missing mandatory options.
| * **The Error Usage Rule**: When an argument parsing or mandatory option error | |
| occurs (`FormatException`, `UsageException`, or `ArgumentError` thrown when | |
| accessing a missing `mandatory: true` option via `results.option(...)`), **both | |
| the error message and the usage text must write to `stderr`**, and exit code | |
| `64` (`EX_USAGE` / `ExitCode.usage.code`) must be returned. `stdout` should | |
| * **The Error Usage Rule**: When an argument parsing or mandatory option error | |
| occurs (FormatException or UsageException thrown during parsing), **both | |
| the error message and the usage text must write to stderr**, and exit code | |
| 64 (EX_USAGE / ExitCode.usage.code) must be returned. stdout should |
Automated changes by create-pull-request GitHub action