stecman / symfony-console-completion

Automatic tab-key completion for Symfony console application options, arguments and parameters
MIT License
420 stars 26 forks source link

remove command shortcuts #86

Closed Seretos closed 5 years ago

Seretos commented 5 years ago

Hi, i use this nice tool for my command line app. This app has many global arguments (for all commands) with shortcuts.

Now i cant use this package, because the auto complete command uses the same shortcuts.

I think the most common usecase for shortcuts is for applications, that use this package :) so, can them removed?

aik099 commented 5 years ago

Possible alternative solutions, because there is always a collision possibility if your app is using options/shortcuts with same names as used by _completion command:

  1. add _completion command to your app as first/last (try both scenarios) so that your global options/shortcuts would win the conflict
  2. change hook generation template (in this package) to always use option names and not shortcuts (that would allow this package to work even when your global shortcuts would replace this package shortcuts)
stecman commented 5 years ago

Interesting problem. Keeping compatibility with existing shell profiles is important, so this will need to be solved without removing the shortcuts. The same conflict issue can occur with the regular option names as well.

If @aik099's first suggestion doesn't help, it's a fairly trivial change to make the completion command ignore any global options from user code. Something along the lines of this in CompletionCommand:

/**
 * Ignore user-defined global options
 *
 * Any global options defined by user-code are meaningless to this command.
 * Options outside of the core defaults are ignored to avoid name and shortcut conflicts.
 */
public function mergeApplicationDefinition($mergeArgs = true)
{
    $appDefinition = $this->getApplication()->getDefinition();
    $originalOptions = $appDefinition->getOptions();

    // Temporarily override application options with basic defaults
    // Note the "option not found" exception isn't handled here, and other default options should probably be included.
    $appDefinition->setOptions([
        $appDefinition->getOption('verbose'),
        $appDefinition->getOption('version'),
        $appDefinition->getOption('help'),
    ]);

    parent::mergeApplicationDefinition($mergeArgs);

    // Restore original application options (not strictly necessary)
    $appDefinition->setOptions($originalOptions);
}

You can update your PR, @Seretos, or I'm happy to make this change.

stecman commented 5 years ago

@Seretos I'll close this PR as I've made another that solves it with the above fix :+1: