Skip to content

[Console] #[Option] ?string $opt = null as VALUE_REQUIRED #60406

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

Closed
wants to merge 1 commit into from

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

Adds the following rule for invokable command options:

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

@kbond kbond requested a review from chalasr as a code owner May 12, 2025 20:20
@carsonbot carsonbot added this to the 7.3 milestone May 12, 2025
@kbond kbond requested a review from yceruto May 12, 2025 20:20
@@ -86,7 +86,8 @@ public function testCommandInputOptionDefinition()
$timeoutInputOption = $command->getDefinition()->getOption('idle');
self::assertSame('idle', $timeoutInputOption->getName());
self::assertNull($timeoutInputOption->getShortcut());
self::assertTrue($timeoutInputOption->isValueOptional());
self::assertTrue($timeoutInputOption->isValueRequired());
self::assertFalse($timeoutInputOption->isValueOptional());
Copy link
Member Author

Choose a reason for hiding this comment

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

Note this behaviour change - but I think it's ok - VALUE_OPTIONAL didn't make sense here imo.

@yceruto
Copy link
Member

yceruto commented May 12, 2025

All these changes make perfect sense to me Kevin. Thanks again for your work on this!

Now trying to write the documentation for Inputs symfony/symfony-docs#20932, after this, I struggle to explain consistently what makes an option value required or optional, can I get some help on this?

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

Is it the nullable type? the nullable type and a null default? or neither, just hardcoded conventions? I thought any consistent way to reason about the signature would make it easier to remember how to use the options, but that may not be possible for this feature.

@yceruto
Copy link
Member

yceruto commented May 12, 2025

Following the same logic, what about:

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

@kbond
Copy link
Member Author

kbond commented May 12, 2025

@yceruto, if you're good with this and #60407, once merged, I'll compile a new list for you in the doc PR.

?array $opt = null

Yep, that's a fair one to add also.

@chalasr
Copy link
Member

chalasr commented May 12, 2025

I think #60407 forbids the first case of your list and makes string|bool the only way to get VALUE_OPTIONAL, which I agree with

@kbond
Copy link
Member Author

kbond commented May 12, 2025

The changes from this PR are included in #60407.

@kbond kbond closed this May 12, 2025
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.

4 participants