[CONSOLE] Breaking change ?

[Neirda24]

Q	A
Bug report?	yes
Feature request?	no
BC Break report?	yes
RFC?	no
Symfony version	3.4.5

Hi...

when coding I was used to setting an alias as so:

alias dev='./bin/console -e=dev'

But since I upgraded to 3.4.5 I got this error:

$ bin/console -e=dev c:c

In FileLocator.php line 44:

  The file "/var/www/html/app/config/config_=dev.yml" does not exist.

It seems now the good syntax is either

./bin/console -edev c:c
# Or
./bin/console --env=dev c:c

It seems to me that it is a breaking change (not a huge one of course but still...)

Regards,
Adrien

[stof]

It looks like the case of -e=dev was forgotten when refactoring hasParameterOption to support -edev properly, because there was no test for it.

@Neirda24 a syntax working on both the old version and the new one is also -e dev

[Simperfit]

@greg-1-anderson Can we fix this or does it need a big refactoring ?

[stof]

it can be fixed for the next releases (due in 1 month). Removing support for this was just a mistake (and the lack of test coverage allowed it to go through)

[greg-1-anderson]

Actually, -e=dev was a bug that was removed as a bugfix. The bug should not be re-introduced into Symfony Console IMO. Everyone who was using -e=dev should switch to the correct syntax, -edev or -e dev. This is how getOption() has always worked. -e=dev only worked in getParameterOption, which was an inconsistency with the primary implementation in getOption(). Unfortunately, the Symfony console documentation on args and options does not illustrate the syntax for short options with values.

It would be technically possible to allow -e=dev to work by silently skipping = in getParameterOption when encountered. However, I think it is for the best if we just acknowledge that this behavior in getParameterOption() was a mistake, and move forward with the current, now-consistent implementation, as strictly preserving b/c maintains the inconsistent behavior. (As an aside, I'd also recommend that folks avoid using getParameterOption(), and instead refactor their code to use getOption() at some point after the Application has called bind on the input object.)

For testing, please try:

#!/usr/bin/env php
<?php
require __DIR__.'/vendor/autoload.php';

use Symfony\Component\Console\Application;
use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;

(new Application('echo', '1.0.0'))
  ->register('echo')
      ->addArgument('foo', InputArgument::OPTIONAL, 'The directory')
      ->addOption('bar', 'b', InputOption::VALUE_REQUIRED, 'The bar', null)
      ->setCode(function(InputInterface $input, OutputInterface $output) {
          // output arguments and options
        print "bar is " . var_export($input->getOption('bar'), true) . "\n";
      })
  ->getApplication()
  ->setDefaultCommand('echo', true) // Single command application
  ->run();

Since this application uses getOption() instead of getParameterOption, echo -bfoo is the correct way to set the value of the bar option to foo in all versions of Symfony.

I'd be happy to make a documentation PR to clarify how short options with values should be represented if everyone agrees that the bugfix should remain.

[fabpot]

Thanks @greg-1-anderson for the great explanation. I agree with you, let's close.

[Neirda24]

Sorry @fabpot & @greg-1-anderson but

$ ./bin/console -e dev c:c


  Command "dev" is not defined.

or even

$ ./bin/console --env dev c:c


  Command "dev" is not defined.

For me it does not work as well. only

$ ./bin/console -edev c:c
# and
$ ./bin/console --env=dev c:c

And besides I don't really have an opinion on which one is better. But I would have a appreciated to be informed on the changelog on even on the best practice part or anywhere else that this was a bug or even to recommended or anything... Maybe a lack of communication on this point ?

Regards,
Adrien

[greg-1-anderson]

Using the example program I posted above:

$ ./echo -bfoo
bar is 'foo'
$ ./echo -b foo
bar is 'foo'
$ ./echo --bar=foo
bar is 'foo'
$ ./echo --bar foo
bar is 'foo'

There may still be bugs in getParameterOption(). I thought that -e dev had a test. My basic opinion is that getParameterOption() should be avoided.

Regarding documentation, I submitted symfony/symfony-docs#9387 for consideration.

[chalasr]

I think the problem with bin/console -e dev [command_name] is the order, ArgvInput::getFirstArgument() is the method used to get the command name, which might be buggy as it seems to not take care of a potential value if the first part of the token is an option with value and a space separator. I suggest to open an issue about that, it probably never worked.
What should work is bin/console [command_name] -e dev.

About the original issue (using = as separator for short options), the docopt states that it should not be supported:

• Short options can be "stacked" meaning that -abc is equivalent to -a -b -c.
  Long options can have arguments specified after space or equal "=" sign:
  --input=ARG is equivalent to --input ARG.
• Short options can have arguments specified after optional space:
  -f FILE is equivalent to -fFILE.

If it worked before, that's probably by chance, hence it is not tested, but it should be forbidden rather than leading to obscur errors.
An error should be reported for unsupported behaviors when possible, reopening if you don't mind.

[greg-1-anderson]

The problem with ArgvInput::getFirstArgument() is that it must work in absence of any information about the option definitions. I suspect that bin/console -e dev [command_name] cannot be made to work due to this limitation, and folks will need to be content with either bin/console [command_name] -e dev or bin/console -edev [command_name].

It might be possible to make ArgvInput::getFirstArgument() recognize the short form of global options; however, I have not investigated. If this is not possible, though, or if the option is defined by the current command, then I suspect it would also be very difficult to report an error for the unsupported form.