[Console] Argument consumption #30244
Conversation
A new argument has been added in order to consume or not the argument.
The tests have been started Small fix on interface implementation.
The tests have been fixed. Same things for the ArgvInput.
The key is now used.
| @@ -294,7 +294,7 @@ public function hasParameterOption($values, $onlyParams = false) | ||
| /** | ||
| * {@inheritdoc} | ||
| */ | ||
| public function getParameterOption($values, $default = false, $onlyParams = false) | ||
| public function getParameterOption($values, $default = false, $onlyParams = false, $consume = false) |
There was a problem hiding this comment.
Only this class should be changed: all the other changes should be reverted as they are unneeded BC breaks.
On this class, the argument should be commented, mentionned in the docblock and accessed using func_get_arg().
See eg https://github.com/symfony/symfony/pull/30212/files#diff-94f4ef2d14dbabcc4f9c798fea31defbR350
The consumed argument is now optional (BC promise).
The code style has been fixed, consumption to define. Small fix on the dataprovider during tests.
The dataprovider has been updated. The code should be finished.
| [['app/console', 'foo:bar', '-e', 'dev'], '-e', false, false, 'dev'], | ||
| [['app/console', 'foo:bar', '-e', 'dev'], '-e', false, true, ''], | ||
| [['app/console', 'foo:bar', '-e', 'dev', '-f', 'bar'], ['-e', '-f'], false, false, 'dev'], | ||
| [['app/console', 'foo:bar', '-e', 'dev', '-f', 'bar'], ['-e', '-f'], false, true, ''], |
There was a problem hiding this comment.
when consume is true, the first call should return true, and the next ones should return false, because the first call "consumed" the info
| [['app/console', 'foo:bar', '-e', 'dev', '--foo', 'bar'], '-f', false, false, ''], | ||
| [['app/console', 'foo:bar', '-e', 'dev', '--foo', 'bar'], '-f', false, true, ''], | ||
| [['app/console', 'foo:bar', '-e', 'dev', '--foo', 'bar'], '--foo', false, false, 'bar'], | ||
| [['app/console', 'foo:bar', '-e', 'dev', '--foo', 'bar'], '-f', false, true, ''], |
There was a problem hiding this comment.
there are move ways to set the dev env: --env=dev and --env dev - we should test them all, not only -e dev
|
please rebase also, there are unrelated commits in the current history. |
|
Closing as the current approach is not the correct one and this is stalling, I'll take over, thanks for trying! |
Hi everyone,
This PR is linked to #23343, as explained by @nicolas-grekas, a new argument has been added in order to remove the parameter if needed.
Tests should be completed (in progress), same things for the code (not every case are covered).