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
Appearance settings

[Console] Revert "bug #41952 fix handling positional arguments" #41966

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jul 4, 2021
Merged

[Console] Revert "bug #41952 fix handling positional arguments" #41966

merged 3 commits into from
Jul 4, 2021

Conversation

chalasr
Copy link
Member

@chalasr chalasr commented Jul 3, 2021 •
edited
Loading

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets -
License MIT
Doc PR -

Reverts #41952 as it breaks tests on 5.3 due to an implicit string cast:
We added the string typehint to the $name argument in 5.x, removing it now to allow for string|int would be a BC break which I don't think is worth it.

@chalasr
Revert "bug symfony#41952 [Console] fix handling positional arguments…
8fc58aa 
… (nicolas-grekas)"

This reverts commit e93f8c0, reversing
changes made to eb83be4.
@carsonbot carsonbot added this to the 4.4 milestone Jul 3, 2021
@carsonbot carsonbot changed the title Revert "bug #41952 [Console] fix handling positional arguments" [Console] Revert "bug #41952 fix handling positional arguments" Jul 3, 2021
@nicolas-grekas
Copy link
Member

Then we need to also revert the change on the interface, and I'm in favor of deprecating positions in hasArgument and in InputDefinition. The current behavior doesn't make sense when integers are passed. Can you give it a try?

@nicolas-grekas
Copy link
Member

Thank you @chalasr.

@nicolas-grekas nicolas-grekas merged commit 2abf5a4 into symfony:4.4 Jul 4, 2021
@chalasr chalasr deleted the revert-int-arg branch July 4, 2021 09:36
@chalasr
Copy link
Member Author

chalasr commented Jul 4, 2021

@nicolas-grekas It appears that handling positions in InputDefinition is required for parsing the raw input (for IS_ARRAY arguments especially), see e.g.

symfony/src/Symfony/Component/Console/Input/ArgvInput.php

Line 161 in 5010ebd

} elseif ($this->definition->hasArgument($c - 1) && $this->definition->getArgument($c - 1)->isArray()) {
Nothing to do I'd say.

This was referenced Jul 26, 2021
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Bug Console Status: Needs Review
Projects
None yet
Milestone
4.4
Development

Successfully merging this pull request may close these issues.
3 participants
@chalasr @nicolas-grekas @carsonbot
Morty Proxy This is a proxified and sanitized view of the page, visit original site.