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] Add a Compiler Pass to Easily Add Helpers #16730

Closed
wants to merge 4 commits into from

Conversation

chrisguitarguy
Copy link
Contributor

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #15258
License MIT
Doc PR -

A container compiler pass that looks for services taged `console.helper`
and stores their IDs in a parameter for later use.
Uses the `console.helpers.ids` parameter set up in `ConsoleHelperPass`.
This also changes the `registeredCommands` property in Applicationto be
`registered`. Better reflects that it controls the both helpers and
command registry.
@linaori
Copy link
Contributor

linaori commented Nov 29, 2015

I find helpers extremely annoying on the console. There's virtually no way to get proper autocomplete. When people will start adding custom helpers, I fear it becomes a big mess where nobody knows what to use when and what's even available.

@Tobion
Copy link
Member

Tobion commented Nov 29, 2015

I agree. The new console utilities like ProgressBar, ProgressIndicator and Table are not using the helper stuff anyway. So IMO we should deprecate the helper functionality in 3.1.

@ogizanagi
Copy link
Member

Agree too. Plus you can already add helpers easily from the Application instance in app/console:

$kernel = new AppKernel($env, $debug);
$application = new Application($kernel);
$myHelper = ...; // Create the helper instance or get it from the Container
$application->getHelperSet()->set($myHelper);

The only benefit of such a compiler pass would be lazy-loading or third-party bundles being able to add helpers.
I don't think console helpers need lazy-loading due to what they generally achieve, and third party bundles can already add helpers if it is really needed through the trick you explained in #15258 (comment):

use Symfony\Bundle\FrameworkBundle\Console\Application as FrameworkApplication;
use Symfony\Component\Console\Application;
use Symfony\Component\HttpKernel\Bundle\Bundle;

class AppBundle extends Bundle
{
    /**
     * {@inheritdoc}
     */
    public function registerCommands(Application $application)
    {
        if ($application instanceof FrameworkApplication) {
            $container  = $application->getKernel()->getContainer()
            $application->getHelperSet()->set($container->get('your_helper'));
        }

        parent::registerCommands($application);
    }
}

EDIT: Forgot about my comment about any benefit: it won't even be lazy loaded anyway :/

@fabpot
Copy link
Member

fabpot commented Jan 25, 2016

Closing for the reasons explained by @iltar and @Tobion

@fabpot fabpot closed this Jan 25, 2016
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

6 participants