Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Console] Negatable option are null by default#40986

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:5.xfromjderusse:negatable-null
Apr 29, 2021

Conversation

@jderusse
Copy link
Member

@jderussejderusse commentedApr 29, 2021
edited
Loading

QA
Branch?5.3
Bug fix?yes
New feature?no
Deprecations?no
Tickets-
LicenseMIT
Doc PR-

given the command

class TestCommandextends Command{protectedstatic$defaultName ='test';protectedfunctionconfigure()    {$this->addOption('ansi',null, InputOption::VALUE_NEGATABLE,'Force/disable ANSI output');    }protectedfunctionexecute(InputInterface$input,OutputInterface$output):int    {dump($input->getOption('ansi'));return Command::SUCCESS;    }}
callresult
bin/console test --no-ansifalse
bin/console test --ansitrue
bin/console testnull

HypeMC and javiereguiluz reacted with thumbs up emoji
@carsonbotcarsonbot changed the title[console] Negatable option are null by default[Console] Negatable option are null by defaultApr 29, 2021
Copy link
Member

@javiereguiluzjaviereguiluz left a comment

Choose a reason for hiding this comment

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

I like this! It turns "negatable options" into tri-state options, which is something that was asked for by some developers. Thanks Jérémy!

Copy link
Member

@fabpotfabpot left a comment

Choose a reason for hiding this comment

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

This change means that$this->getOption('ansi') can now returnnull, which is a BC break as it always returns a bool right now. Maybe we should havefalse as a default for this internal flag (no need for 3 states here anyway).

derrabus reacted with thumbs up emoji
@javiereguiluz
Copy link
Member

The main difference is about "What's a negatable option?"

(1) A single option with two behaviors (yes/no)
(2) Two different options (yes, no) but defined as a single one for convenience

Incase (1), we need a tri-state to differentiate these cases:

$useAnsi =$input->getOption('ansi');if (true ===$useAnsi) {// force ANSI usage}elseif (false ===$useAnsi) {// don't use ANSI}elseif (null ===$useAnsi) {// user didn't pass any ANSI preference;// we must decide what to do}

Incase (2), you only need two cases (the third one happens automatically):

$useAnsi =$input->getOption('ansi');$dontUseAnsi =$input->getOption('no-ansi');if ($useAnsi) {// force ANSI usage}elseif ($dontUseAnsi) {// don't use ANSI}else {// user didn't pass any ANSI preference;// we must decide what to do}

@jderusse
Copy link
MemberAuthor

The BC break has been solved by setting the default tofalse for OUR--ansi option.
In that way:

  • usinggetOption('ansi') will still return false
  • people can useInputOption::VALUE_NEGATABLE with a defaultnull value and leverage the 3 states of the option.
sstok reacted with laugh emoji

@fabpot
Copy link
Member

Thank you@jderusse.

@fabpotfabpot merged commit694c052 intosymfony:5.xApr 29, 2021
@fabpotfabpot mentioned this pull requestMay 1, 2021
@jderussejderusse deleted the negatable-null branchMay 12, 2021 06:49
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fabpotfabpotfabpot approved these changes

@javiereguiluzjaviereguiluzjaviereguiluz approved these changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@derrabusderrabusderrabus approved these changes

@chalasrchalasrchalasr approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

5.3

Development

Successfully merging this pull request may close these issues.

7 participants

@jderusse@javiereguiluz@fabpot@nicolas-grekas@derrabus@chalasr@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp