Add informations about all available connections and managers into the help #129

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
6 participants
Contributor

hason commented Oct 15, 2012

No description provided.

@stof stof and 1 other commented on an outdated diff Oct 15, 2012

Command/Proxy/DoctrineCommandHelper.php
+ {
+ if (null === $application) {
+ return;
+ }
+
+ $doctrine = $application->getKernel()->getContainer()->get('doctrine');
+ $options = $command->getDefinition()->getOptions();
+ $help = $command->getHelp();
+
+ if (isset($options['connection'])) {
+ $options['connection']->setDefault($doctrine->getDefaultConnectionName());
+ $help .= "\n\nAvailable connections: ".implode(', ', array_keys($doctrine->getConnectionNames()));
+ }
+
+ if (isset($options['em'])) {
+ $options['em']->setDefault($doctrine->getDefaultManagerName());
@stof

stof Oct 15, 2012

Member

Please don't do it. Let it default to null (which uses the default manager) instead of modifying the default values. Avoiding changes in the definition in unexpected places would make the maintenance easier

@hason

hason Oct 15, 2012

Contributor

But this is the main feature of this PR ;) You see real name of the default connection or manager.

@stof

stof Oct 15, 2012

Member

Well, in your application, you are also using null to access the default manager from the registry.

@stof stof and 1 other commented on an outdated diff Oct 15, 2012

Command/Proxy/CreateSchemaDoctrineCommand.php
@@ -32,6 +33,16 @@ class CreateSchemaDoctrineCommand extends CreateCommand
/**
* {@inheritDoc}
*/
+ public function setApplication(Application $application = null)
+ {
+ parent::setApplication($application);
+
+ DoctrineCommandHelper::configureCommand($application, $this);
@stof

stof Oct 15, 2012

Member

I don't think doing it in setApplication is a good idea. It would instantiate the registry even when running other commands than Doctrine ones, as setApplication is called when adding the command in the application. Couldn't you modify the help in another place running only when needed ?

@hason

hason Oct 15, 2012

Contributor

Fixed

@hason hason Add informations about all available connections and managers into th…
…e help of a command and set default connection name or default manager name
ca14ea2
Contributor

hason commented Oct 24, 2012

@stof What do you think, is everything okay?

Contributor

stloyd commented Jan 5, 2013

Any news preventing this PR from being merged ?

Contributor

jrobeson commented Jan 20, 2013

yes please .. bump! :)

Owner

guilhermeblanco commented Jan 27, 2014

Is this PR still valid?

Member

kimhemsoe commented Sep 22, 2015

Should this get resurrected or buried ?

Owner

guilhermeblanco commented Nov 4, 2015

Closing due to submitter inactivity. I feel this is not valid anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment