[Console] Support binary / negatable options #39642

jderusse · 2020-12-28T22:16:51Z

Q	A
Branch?	5.x
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets	Fix #24314
License	MIT
Doc PR	TODO

This PR take over #26292 to introduce a new option mode NEGATABLE that ease creation of option and their negation (ie. --foo, --no-foo)

Negated options applies only to flag options (VALUE_NONE)

./cli.php --foo
var_dump(getOption('foo')); // true

./cli.php --no-foo
var_dump(getOption('foo')); // false

….g. --foo and --no-foo.

fabpot · 2020-12-29T07:42:05Z

I would only allow the --no- prefix for options that act as flags (the ones that do not take a value).

src/Symfony/Component/Console/Input/InputDefinition.php

src/Symfony/Component/Console/Tests/Command/ListCommandTest.php

src/Symfony/Component/Console/Tests/Fixtures/application_1.xml

OskarStark · 2020-12-30T08:00:22Z

Can you please make fabbot happy? 😊

jderusse · 2020-12-30T09:02:35Z

Can you please make fabbot happy? 😊

I think, fabbot is a false positive here.
Fixing it will break the test

OskarStark · 2020-12-30T09:15:21Z

Indeed it looks weird 🧐

javiereguiluz · 2020-12-30T16:34:57Z

Jérémy thanks for this contribution.

As a super minor comment: is "negatable" the best word to describe this? Could we use "boolean" like the Golang boolean flags?

src/Symfony/Component/Console/Tests/Command/ListCommandTest.php

OskarStark · 2020-12-30T17:47:41Z

I like the Boolean naming a lot! Thank you for the proposal Javier 😃

chalasr · 2020-12-30T21:50:25Z

Boolean looks confusing IMO.
Actually, Go' boolean flags are not about options that have a --no- alternative, but just options whose value can either be true or false.
Negatable options can be found in Perl and node and other languages. I would stick with it.

jderusse · 2020-12-30T21:50:59Z

~~renamed VALUE_NEGATABLE into VALUE_BOOLEAN~~

I also Replaced --[no-]foo notation by --foo|no-foo for consistency with shortcut -v|vv|vvv and respects docopt (| means either X or X).
But it lloks like we are not 100% compliant with the spec:

-v|vv|vvv should be -v | -vv | -vvv,
-e, --env=ENV should be -e --env=ENV or -e, --env <env>

wouterj · 2020-12-30T23:05:17Z

You're correct about -v|vv|vvv (let's fix that in another PR), but -e, --env=ENV is correct afaics: "Follow either <angular-brackets> or UPPER-CASE convention for options' arguments. You can use a comma if you want to separate options." Seems like all variations of comma/no-comma, and <angular-brackets>/UPPER-CASE are valid.

(and sorry for being nitpicky about this, I just remember this as it was my first big PR to Symfony: #13220)

src/Symfony/Component/Console/Application.php

src/Symfony/Component/Console/Input/InputOption.php

fabpot

Small nitpick before merging

src/Symfony/Component/Console/Descriptor/XmlDescriptor.php

fabpot · 2021-01-05T07:59:31Z

Thank you @greg-1-anderson.

chalasr · 2021-01-05T11:20:24Z

Thanks @jderusse also :)

src/Symfony/Component/Console/Input/InputDefinition.php

greg-1-anderson · 2021-01-05T15:29:31Z

Is a backport to Symfony 4.4.x permissible?

stof · 2021-01-05T15:38:26Z

@greg-1-anderson no. New features are never backported in patch releases.

driesvints · 2021-06-01T08:21:49Z

src/Symfony/Component/Console/Application.php

@@ -1033,8 +1033,7 @@ protected function getDefaultInputDefinition()
            new InputOption('--quiet', '-q', InputOption::VALUE_NONE, 'Do not output any message'),
            new InputOption('--verbose', '-v|vv|vvv', InputOption::VALUE_NONE, 'Increase the verbosity of messages: 1 for normal output, 2 for more verbose output and 3 for debug'),
            new InputOption('--version', '-V', InputOption::VALUE_NONE, 'Display this application version'),
-            new InputOption('--ansi', '', InputOption::VALUE_NONE, 'Force ANSI output'),
-            new InputOption('--no-ansi', '', InputOption::VALUE_NONE, 'Disable ANSI output'),


Why was this flag removed?

driesvints · 2021-06-01T08:23:36Z

@jderusse @fabpot the no-ansi flag was removed in this PR and broke the Laravel installer: laravel/installer#202. That seems like a breaking change to me on a minor release? Although it seems like we can work around it for now easily.

stof · 2021-06-01T08:37:13Z

Well, was Lavaral checking the no-ansi option directly instead of checking whether the output was decorated ? If yes, it was not dealing with the smart default when neither --no-ansi or --ansi was passed.
The BC promise for application-level options is for the caller (and there, --no-ansi still works as --ansi is now a negatable option).

driesvints · 2021-06-01T08:43:19Z

We switched from getOption to hasOption:

stof · 2021-06-01T09:08:55Z

@driesvints this is not equivalent. hasOption tells you whether the input has a definition for this option. It won't tell you whether the option was used when calling the command.

stof · 2021-06-01T09:09:43Z

what you really need to check is $output->isDecorated() to pass either --ansi or --no-ansi to force the subcommand to have the same decoration.

driesvints · 2021-06-01T10:00:25Z

@stof never have had to do that before. This has always worked like this. I still feel this is a breaking change, sorry.

driesvints · 2021-06-01T10:06:16Z

@stof I can't seem to find any docs about isDecorated on symfony.com. How would one use something like that?

stof · 2021-06-01T10:13:59Z

@driesvints there is no documentation about accessing --no-ansi directly in your own command either. and as I said, passing an explicit --no-ansi is not the only way to get a decorated output. The default is a smart one (which is why we also have a --ansi to force colors).
And the case of executing a command in a subprocess like you do is precisely the case where the smart default will probably be different in the subprocess (probably never enabling colors).

How would one use something like that?

call $output->isDecorated() (which returns a boolean) to know whether messages will be decorated (i.e. whether they support being formatted with color, bold, etc...)

driesvints · 2021-06-01T10:20:49Z

Thanks @stof, that was clear 👍

[WIP] Implements symfony#24314: Support binary / negatable options, e…

8d455db

….g. --foo and --no-foo.

jderusse requested a review from chalasr as a code owner December 28, 2020 22:16

carsonbot added Status: Needs Review Console Feature labels Dec 28, 2020

jderusse added this to the 5.x milestone Dec 28, 2020

jderusse force-pushed the binary-options branch from 20fa14c to 89a7e89 Compare December 29, 2020 09:58

OskarStark reviewed Dec 29, 2020

View reviewed changes

src/Symfony/Component/Console/Input/InputDefinition.php Show resolved Hide resolved

src/Symfony/Component/Console/Input/InputDefinition.php Show resolved Hide resolved

src/Symfony/Component/Console/Tests/Command/ListCommandTest.php Outdated Show resolved Hide resolved

jderusse force-pushed the binary-options branch 2 times, most recently from 497f03c to 82d2a9f Compare December 29, 2020 16:56

jderusse commented Dec 29, 2020

View reviewed changes

src/Symfony/Component/Console/Tests/Fixtures/application_1.xml Show resolved Hide resolved

jderusse force-pushed the binary-options branch from 82d2a9f to 0c54aa7 Compare December 29, 2020 22:24

OskarStark approved these changes Dec 30, 2020

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Dec 30, 2020

wouterj reviewed Dec 30, 2020

View reviewed changes

src/Symfony/Component/Console/Tests/Command/ListCommandTest.php Outdated Show resolved Hide resolved

jderusse force-pushed the binary-options branch from 0c54aa7 to ca2d46a Compare December 30, 2020 21:46

fabpot requested changes Jan 1, 2021

View reviewed changes

src/Symfony/Component/Console/Application.php Outdated Show resolved Hide resolved

carsonbot added Status: Needs Work and removed Status: Reviewed labels Jan 1, 2021

chalasr reviewed Jan 1, 2021

View reviewed changes

src/Symfony/Component/Console/Input/InputOption.php Outdated Show resolved Hide resolved

jderusse force-pushed the binary-options branch from ad5f315 to 95bfea2 Compare January 4, 2021 09:08

fabpot requested changes Jan 5, 2021

View reviewed changes

src/Symfony/Component/Console/Descriptor/XmlDescriptor.php Outdated Show resolved Hide resolved

Implement negatable option

1a9a3c3

jderusse force-pushed the binary-options branch from 95bfea2 to 1a9a3c3 Compare January 5, 2021 07:47

fabpot approved these changes Jan 5, 2021

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Work labels Jan 5, 2021

fabpot mentioned this pull request Jan 5, 2021

[Console] Support binary / negatable options symfony/symfony-docs#14783

Closed

fabpot merged commit a0bbf53 into symfony:5.x Jan 5, 2021

jderusse deleted the binary-options branch January 5, 2021 08:04

stof reviewed Jan 5, 2021

View reviewed changes

src/Symfony/Component/Console/Input/InputDefinition.php Show resolved Hide resolved

fabpot mentioned this pull request Apr 18, 2021

Release v5.3.0-BETA1 #40853

Merged

driesvints reviewed Jun 1, 2021

View reviewed changes

driesvints mentioned this pull request Jun 1, 2021

Fix new command failing without no-ansi option laravel/installer#202

Merged

driesvints mentioned this pull request Jun 1, 2021

Use isDecorated for no-ansi detection laravel/installer#203

Merged

driesvints mentioned this pull request Jun 22, 2021

Tinker lost colors laravel/tinker#127

Closed

eliashaeussler mentioned this pull request Aug 15, 2022

Add support for negatable input options phpstan/phpstan-symfony#300

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Console] Support binary / negatable options #39642

[Console] Support binary / negatable options #39642

jderusse commented Dec 28, 2020 •

edited

Loading

fabpot commented Dec 29, 2020

OskarStark commented Dec 30, 2020 •

edited

Loading

jderusse commented Dec 30, 2020

OskarStark commented Dec 30, 2020

javiereguiluz commented Dec 30, 2020

OskarStark commented Dec 30, 2020

chalasr commented Dec 30, 2020

jderusse commented Dec 30, 2020 •

edited

Loading

wouterj commented Dec 30, 2020 •

edited

Loading

fabpot left a comment

fabpot commented Jan 5, 2021

chalasr commented Jan 5, 2021

greg-1-anderson commented Jan 5, 2021

stof commented Jan 5, 2021

driesvints Jun 1, 2021

driesvints commented Jun 1, 2021

stof commented Jun 1, 2021

driesvints commented Jun 1, 2021

stof commented Jun 1, 2021

stof commented Jun 1, 2021

driesvints commented Jun 1, 2021

driesvints commented Jun 1, 2021

stof commented Jun 1, 2021

driesvints commented Jun 1, 2021

[Console] Support binary / negatable options #39642

[Console] Support binary / negatable options #39642

Conversation

jderusse commented Dec 28, 2020 • edited Loading

fabpot commented Dec 29, 2020

OskarStark commented Dec 30, 2020 • edited Loading

jderusse commented Dec 30, 2020

OskarStark commented Dec 30, 2020

javiereguiluz commented Dec 30, 2020

OskarStark commented Dec 30, 2020

chalasr commented Dec 30, 2020

jderusse commented Dec 30, 2020 • edited Loading

wouterj commented Dec 30, 2020 • edited Loading

fabpot left a comment

Choose a reason for hiding this comment

fabpot commented Jan 5, 2021

chalasr commented Jan 5, 2021

greg-1-anderson commented Jan 5, 2021

stof commented Jan 5, 2021

driesvints Jun 1, 2021

Choose a reason for hiding this comment

driesvints commented Jun 1, 2021

stof commented Jun 1, 2021

driesvints commented Jun 1, 2021

stof commented Jun 1, 2021

stof commented Jun 1, 2021

driesvints commented Jun 1, 2021

driesvints commented Jun 1, 2021

stof commented Jun 1, 2021

driesvints commented Jun 1, 2021

jderusse commented Dec 28, 2020 •

edited

Loading

OskarStark commented Dec 30, 2020 •

edited

Loading

jderusse commented Dec 30, 2020 •

edited

Loading

wouterj commented Dec 30, 2020 •

edited

Loading