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

[generate:form] When I load a service, the console generate twice the service in create method #2131

Closed
igasi opened this issue Apr 12, 2016 · 11 comments

Comments

@igasi
Copy link
Contributor

igasi commented Apr 12, 2016

Problem/Motivation
I loaded 1 service and the console generate

Do you want to load services from the container (yes/no) [no]:
 > yes           


Type the service name or use keyup or keydown.
This is optional, press enter to continue

 Enter your service [ ]:
 > config.factory
 Enter your service [ ]:
 > 

screenshot from 2016-04-12 15-39-36

Propose solution

Review the template for render this and remove double service in create method.

@igasi
Copy link
Contributor Author

igasi commented Apr 12, 2016

the config?

@omero
Copy link
Contributor

omero commented Apr 12, 2016

It looks that the form template always injects the config.factory service https://github.com/hechoendrupal/DrupalConsole/blob/master/templates/module/src/Form/form.php.twig#L41

in the generate:form:config you need this service so, is correct, otherwise on a generic form is not necessary, any thoughts @jmolivas ?

@igasi
Copy link
Contributor Author

igasi commented Apr 12, 2016

Yes, in this case, is a generic form.

screenshot from 2016-04-12 16-31-09
screenshot from 2016-04-12 16-29-59

You're right, we are injecting at first but it is not used in the constructor.

@omero
Copy link
Contributor

omero commented Apr 12, 2016

👍 for removing the line on the template at least for generate:form command.

@igasi
Copy link
Contributor Author

igasi commented Apr 12, 2016

This template only use for generate:form, or in other side?

@omero
Copy link
Contributor

omero commented Apr 12, 2016

yes! is used on generate:form and generate:form:config commands

@igasi
Copy link
Contributor Author

igasi commented Apr 12, 2016

There are any way to differentiate between both?

For implements some thing this: (and send my PR)

{% if services is not empty %}
  public static function create(ContainerInterface $container) {
    return new static(
    {% if isGenerateFormConfigTemplate %}
      $container->get('config.factory'),
    {% endif %}
{{ serviceClassInjection(services) }}
    );
  }

{% endif %}

@omero
Copy link
Contributor

omero commented Apr 12, 2016

Sorry my mistake, when looking the FormGenerator class https://github.com/hechoendrupal/DrupalConsole/blob/master/src/Generator/FormGenerator.php#L42, uses a different template for configuration forms, removing only the line can work

@igasi
Copy link
Contributor Author

igasi commented Apr 12, 2016

Nice, Thx.

igasi added a commit to igasi/DrupalConsole that referenced this issue Apr 12, 2016
@igasi
Copy link
Contributor Author

igasi commented Apr 12, 2016

PR sent.

@jmolivas
Copy link
Member

Fixed with #2133

jmolivas pushed a commit that referenced this issue Mar 15, 2017
* issue #2131 Removed config.factory service extra in generate:form command

* #3223 Fixed config:import:single

* #3223 Fixed config:import:single - improved validation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants