Skip to content
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

[Console] Prevent ArgvInput::getFirstArgument() from returning an option value #30277

Merged
merged 1 commit into from Feb 21, 2019

Conversation

chalasr
Copy link
Member

@chalasr chalasr commented Feb 17, 2019

Q A
Branch? 3.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #23343
License MIT
Doc PR n/a

Fixes the case where the passed input string contains no command name but one or more global (i.e. application-defined) options accepting values.

@chalasr chalasr added this to the 3.4 milestone Feb 17, 2019
@chalasr chalasr force-pushed the cli-opts-val-are-not-args branch 4 times, most recently from 5076733 to e4d4a92 Compare February 18, 2019 18:44
@nicolas-grekas
Copy link
Member

Do we really need that logic? To me, an approach like #30244 will achieve the same with greater flexibility.

@chalasr
Copy link
Member Author

chalasr commented Feb 20, 2019

I think yes, it fixes a real bug. ArrayInput does not suffer from this issue.
#30244 looks like a feature to me as it requires code change for being used, we would need to document it. And the fact it involves to remove an option from the input makes it less flexible to me, IMHO it is not the desired behavior for the 80% use case, keeping in mind that this is not only about --env but any option defined at the application level which are generally meant to be accessed from the final input object at command runtime.

@stof
Copy link
Member

stof commented Feb 20, 2019

how does this handle cases where multiple shortcut gets grouped together ?

@chalasr
Copy link
Member Author

chalasr commented Feb 20, 2019

@stof in case the token is a short option (group), this checks that the last char of the token has a value and, if it has one, the next token is skipped. Just added a test + inline comment.

@fabpot
Copy link
Member

fabpot commented Feb 21, 2019

Thank you @chalasr.

@fabpot fabpot merged commit 46461e9 into symfony:3.4 Feb 21, 2019
fabpot added a commit that referenced this pull request Feb 21, 2019
…ning an option value (chalasr)

This PR was merged into the 3.4 branch.

Discussion
----------

[Console] Prevent ArgvInput::getFirstArgument() from returning an option value

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #23343
| License       | MIT
| Doc PR        | n/a

Fixes the case where the passed input string contains no command name but one or more global (i.e. application-defined) options accepting values.

Commits
-------

46461e9 [Console] Prevent ArgvInput::getFirstArgument() from returning an option value
@chalasr chalasr deleted the cli-opts-val-are-not-args branch February 21, 2019 08:55
This was referenced Mar 3, 2019
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.

None yet

5 participants