Skip to content

馃 chore: sync skills directory from dart-lang/skills - #236

Open
flutter-skills-sync-bot wants to merge 1 commit into
mainfrom
automation/sync-dart-skills
Open

馃 chore: sync skills directory from dart-lang/skills#236
flutter-skills-sync-bot wants to merge 1 commit into
mainfrom
automation/sync-dart-skills

Conversation

@flutter-skills-sync-bot

Copy link
Copy Markdown
Collaborator

Automated changes by create-pull-request GitHub action

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Comment on lines +56 to +61
} on ArgumentError catch (e) {
// Missing mandatory option error thrown by results.option():
stderr
..writeln('Error: ${e.message}')
..writeln(parser.usage);
exitCode = ExitCode.usage.code;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

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.

Comment on lines +86 to +90
* **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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

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.

Suggested change
* **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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant