-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[Console] Invokable command #[Option] adjustments
#60407
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
Conversation
|
If this is acceptable - I'll add a bunch of missing tests (related to non-string scalars):
|
|
Makes sense to me, thank you. |
b66eb97 to
5ba27bb
Compare
|
@yceruto, the full list:
Any |
b87a8fe to
c312624
Compare
#[Option] bool|string $opt = false as VALUE_OPTIONAL#[Option] adjustments
yceruto
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll do a deeper review tomorrow, but the new list seems to follow a consistent pattern at least (which was my major concern) 👍
c312624 to
464a5aa
Compare
src/Symfony/Component/Console/Tests/Command/InvokableCommandTest.php
Outdated
Show resolved
Hide resolved
- `#[Option] ?string $opt = null` as `VALUE_REQUIRED` - `#[Option] bool|string $opt = false` as `VALUE_OPTIONAL` - `#[Option] ?string $opt = ''` throws exception - allow `#[Option] ?array $opt = null` - more tests...
464a5aa to
59a4ae9
Compare
|
@kbond do you think/confirm we can document these rules like this? + sample table above Signature rules (
Definition rules
Usage/Value rules
|
|
@yceruto those rules look great to me! |
|
Thank you @kbond. |
The only way to make an option
VALUE_OPTIONAL.string|bool $opt = falseVALUE_OPTIONAL<omitted>--opt=val--optfalse(default)"val"trueNow throws an exception:
?string $opt = ''VALUE_OPTIONAL<omitted>--opt=val--opt''(default)"val"nullNow is valid:
?string $opt = nullVALUE_REQUIRED<omitted>--opt=valnull(default)"val"?array $opt = nullVALUE_ARRAY&VALUE_REQUIRED<omitted>--opt=val1null(default)['val1']