Skip to content

[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

Merged
merged 1 commit into from
May 14, 2025

Conversation

kbond
Copy link
Member

@kbond kbond commented May 12, 2025

Q A
Branch? 7.3
Bug fix? no
New feature? yes
Deprecations? no
Issues Fix #59602 (comment)
License MIT

The only way to make an option VALUE_OPTIONAL.

Signature Definition Usage Value
string|bool $opt = false VALUE_OPTIONAL <omitted>
--opt=val
--opt
false (default)
"val"
true

Now throws an exception:

Signature Definition Usage Value
?string $opt = '' VALUE_OPTIONAL <omitted>
--opt=val
--opt
'' (default)
"val"
null

Now is valid:

Signature Definition Usage Value
?string $opt = null VALUE_REQUIRED <omitted>
--opt=val
null (default)
"val"
?array $opt = null VALUE_ARRAY & VALUE_REQUIRED <omitted>
--opt=val1
null (default)
['val1']

@kbond kbond requested a review from chalasr as a code owner May 12, 2025 22:00
@kbond kbond requested review from yceruto and removed request for chalasr May 12, 2025 22:01
@carsonbot carsonbot added this to the 7.3 milestone May 12, 2025
@kbond
Copy link
Member Author

kbond commented May 12, 2025

If this is acceptable - I'll add a bunch of missing tests (related to non-string scalars):

  • int $opt = 0 (valid)
  • ?int $opt = null (valid)
  • ?int $opt = 0 (invalid)
  • int|bool = false (valid)
  • int|bool = 0 (invalid)
  • float $opt = 0.0 (valid)
  • ?float $opt = null (valid)
  • ?float $opt = 0.0 (invalid)
  • float|bool = false (valid)
  • float|bool = 0.0 (invalid)

@chalasr
Copy link
Member

chalasr commented May 12, 2025

Makes sense to me, thank you.

@kbond
Copy link
Member Author

kbond commented May 12, 2025

@yceruto, the full list:

No. Signature Definition Usage Value
1 bool $opt = false VALUE_NONE <omitted>
--opt
false (default)
true
2 bool $opt = true VALUE_NONE &
VALUE_NEGATABLE
<omitted>
--opt
--no-opt
true (default)
true
false
3 ?bool $opt = null VALUE_NONE &
VALUE_NEGATABLE
<omitted>
--opt
--no-opt
null (default)
true
false
4 string $opt = '' VALUE_REQUIRED <omitted>
--opt=val
'' (default)
'val'
5 ?string $opt = null VALUE_REQUIRED <omitted>
--opt=val
null (default)
'val'
6 array $opt = [] VALUE_REQUIRED &
VALUE_ARRAY
<omitted>
--opt=val1
[] (default)
['val1']
7 ?array $opt = null VALUE_REQUIRED &
VALUE_ARRAY
<omitted>
--opt=val1
null (default)
['val1']
8 string|bool $opt = false VALUE_OPTIONAL <omitted>
--opt
--opt=val
false (default)
true
'val'

Any string can be swapped with int or float.

@kbond kbond force-pushed the console-value-optional branch 2 times, most recently from b87a8fe to c312624 Compare May 13, 2025 00:10
@kbond kbond changed the title [Console] #[Option] bool|string $opt = false as VALUE_OPTIONAL [Console] Invokable command #[Option] adjustments May 13, 2025
Copy link
Member

@yceruto yceruto left a 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) 👍

- `#[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...
@kbond kbond force-pushed the console-value-optional branch from 464a5aa to 59a4ae9 Compare May 13, 2025 13:52
@yceruto
Copy link
Member

yceruto commented May 13, 2025

@kbond do you think/confirm we can document these rules like this? + sample table above

Signature rules (<type> $opt = <default-value>)

  • <type> must be one of: bool, string, int, float, or array
    • Nullable types (?<type>) require <default-value> to be null
    • Union types are only allowed for bool|string, bool|int, or bool|float
      • In these cases, <default-value> must be false
  • All options must define a <default-value>

Definition rules

  • VALUE_NONE: set for bool or ?bool
    • VALUE_NEGATABLE: same as VALUE_NONE, but only if <default-value> is true or null
  • VALUE_REQUIRED: set for non-boolean single types (<type>) or nullable types (?<type>)
    • VALUE_ARRAY: same as VALUE_REQUIRED, but specifically for array type
  • VALUE_OPTIONAL: set only for union types combining bool|<allowed-type>

Usage/Value rules

  • If the option is omitted, <default-value> is given
  • If passed without a value (--opt), true is given
  • If negate passed (--no-opt), false is given
  • If passed with a value (--opt=val), val is given

@kbond
Copy link
Member Author

kbond commented May 13, 2025

@yceruto those rules look great to me!

@fabpot
Copy link
Member

fabpot commented May 14, 2025

Thank you @kbond.

@fabpot fabpot merged commit 5955b14 into symfony:7.3 May 14, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants