Skip to content

[Console] Fix allow passing a default argument when InputOption::IS_NEGATABLE is used #52058

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

Open
wants to merge 3 commits into
base: 5.4
Choose a base branch
from

Conversation

jnoordsij
Copy link
Contributor

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets
License MIT

(Encountered on test run for #52055)

Currently, the following call/construction of an InputOption is valid:

new InputOption('foo', null, InputOption::VALUE_NEGATABLE, '', false)

while the following call is not

new InputOption('foo', null, InputOption::VALUE_NONE | InputOption::VALUE_NEGATABLE, '', false)

To me, this seems inconsistent, as I would expect both calls to behave in exactly the same manner. This PR ensures both calls are equally valid.

Note: a follow-up might be ensuring the passed default value is boolean when using InputOption::VALUE_NEGATABLE; I can add that to #52055 once this is merged.

@jnoordsij jnoordsij force-pushed the fix-inputoption-set-default branch from 1aa455d to ba2f4a5 Compare October 14, 2023 09:45
@jnoordsij jnoordsij requested a review from GromNaN October 17, 2023 08:05
@nicolas-grekas
Copy link
Member

I'm not sure I understand the purpose. Why don't you use new InputOption('foo', null, InputOption::VALUE_NONE | InputOption::VALUE_NEGATABLE) if you don't want a value?

@OskarStark OskarStark changed the title [Console] Fix allow passing a default argument when InputOption::IS_NEGATABLE is used [Console] Fix allow passing a default argument when InputOption::IS_NEGATABLE is used Oct 17, 2023
@jnoordsij
Copy link
Contributor Author

I'm not sure I understand the purpose. Why don't you use new InputOption('foo', null, InputOption::VALUE_NONE | InputOption::VALUE_NEGATABLE) if you don't want a value?

I think passing an option for the negatable case does make sense, since it basically ensures the option resolves as boolean instead of nullable boolean, which could be useful or at least clearer in some applications.

I do admit it looks slightly off when combined with an explicit VALUE_NONE, but the main issue I had with this was the inconsistency of how the options were handled, which prompted me to open #52055. To illustrate, the following docblock actually claims not setting VALUE_REQUIRED or VALUE_OPTIONAL is considered to be equal to actually setting VALUE_NONE:
https://github.com/symfony/symfony/blob/5.4/src/Symfony/Component/Console/Input/InputOption.php#L130-L138

So given this, I would expect VALUE_NONE and VALUE_NONE | VALUE_NEGATABLE to behave exactly the same in all cases, as VALUE_NEGATABLE in my eyes is just a slightly more specific variant of VALUE_NONE, i.e. an option that does not allow a manual value, but does allow passing the negated version.

@fabpot fabpot modified the milestones: 5.4, 6.4 Dec 14, 2024
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