[RFC] Allow to override the skeleton files?

[javiereguiluz]

When creating this bundle we decided to not require Twig to keep things simple. That means that our skeleton files can't have complex logic and we need to interpolate the placeholder variables ourselves. Not a big deal ... but not using Twig and not defining skeleton files as Twig templates has another problem...

... you can't easily override the skeleton files in your project (see #32 and lots of related issues in the SensioGeneratorBundle). Should we...

... add Twig as dependency and turn skeletons into templates?
... try to make the skeleton files overridable using the kernel file locator ?
... not support this feature explicitly?

[yceruto]

add Twig as dependency and turn skeletons into templates?

👍 adding logic to the skeleton template (like conditional code and blocks) look good to me and solve ideas like described by @weaverryan here #22 (comment)

[weaverryan]

I disagree with adding Twig as a dependency. I don't want API-only projects to end up with a templates/ directory just so they can generate some guard authenticators or something else.

And I also don't think that our templates should be overridable - and opinion that I'm sure won't make me very popular with some people :). Making a generation command is very easy, and you can even extend from our AbstractCommand. If you want a different template for a generator, I think you should just create your own command.

[eugenekurasov]

@weaverryan so if I need add for example add to top

<?php declare(strict_types = 1); 
....

I need copy of exist command with change only skeleton? If I don't need change logic or add new variables, I think is violates the principle DRY, isn't it?

So I think I need have some solution when we need change only template.

[javiereguiluz]

I agree with Ryan ... but I'd also love to find a simple solution to help solve the needs of developers like @eugenekurasov.

[weaverryan]

@weaverryan so if I need add for example add to top

<?php declare(strict_types = 1); 
....

For now, yes. We have a tricky balance to strike: if we make the bundle very extendible, then we need to be much more careful when adding/changing features so we don't break those extensions. By saying "create your own custom commands", the bundle can stay innovative. We learned this lesson with SensioGeneratorBundle. Of course, if you want to make your own command, we should make it as easy as possible. Right now, it's pretty easy, since we have the AbstractCommand, but perhaps we could make it even easier.

[eugenekurasov]

So problem only with BC, if it is so this is another problem. I don't give solution for this issue now, but you can see how trying resolved same issue in doctrineMigrationsBundle - they give option for override they skeleton. I think you can found other solution.
So yes it's not easy to find the right solution but this is not good solution create new commands.

[yceruto]

Could be #50 enough to solve the issue?

[ChangePlaces]

--- after a bit of thought, I'd like to recommend,

• final is removed from the command classes
• instead of hardcoding the 'template' directory, the return valid is abstracted to a method. in combination with final being removed, this would allow a class to extend e.g. make crud and only overwrite this method to return a new directory for the 'templates'

[ckrack]

As with the proposed target option (#51) to use a different place for the generated code, I'd suggest using an origin option, which could be used by the makers to set/override the path for the Resources directory.

php bin/console make:crud Foobar --origin=src/Resources/Maker

Pro:

• We can stay free of dependencies like Twig or iterating over possible paths.
• It would stay in the background
• It would even allow us to have packages, that just generates skeletons for specific makers, allowing a lot of customization.

Con:

• We need to change every maker to include this.
• It doesn't allow customizing help-files (they are read in the configureCommand method, where the input is not yet available). But the behaviour shouldn't change anyway, only the skeleton.
• Templates can not have an interface and an update could break skeletons

The last con would probably require us to define which variables are available inside the templates.
Skeleton-Packages would then dependend on a specific version of the maker-bundle.

[MUbedaSJ]

Hello,
I propose an auto-detection owerwriting CRUD "skeleton" templates, with only 3 lines :
Into "MakeCrup.php" file, you can add this line (at ~134), for detecting if custom skeleton files are defined on project...:

 $customSkeletonDir=(file_exists(getcwd()."/templates/bundles/MakerBundle/skeleton/")?getcwd()."/templates/bundles/MakerBundle/skeleton/":"");

Then use this variable '$customSkeletonDir' when calling templates:
at line ~137:

 $generator->generateController(
            $controllerClassDetails->getFullName(),
            $customSkeletonDir.'crud/controller/Controller.tpl.php',
            ...

and lines ~199:

foreach ($templates as $template => $variables) {
            $generator->generateTemplate(
                customSkeletonDir.$templatesPath.'/'.$template.'.html.twig',
                $customSkeletonDir.'crud/templates/'.$template.'.tpl.php',
                $variables
            );
        }

I've tested for "*.tpl.php" templates, and it work like a charm !

[WilcoBrandSync]

Perfect!

Two small changes:

Hello,
I propose an auto-detection owerwriting CRUD "skeleton" templates, with only 3 lines :
Into "MakeCrup.php" file, you can add this line (at ~134), for detecting if custom skeleton files are defined on project...:

• It's the "MakeCrud.php" file and not the "MakeCrup.php" file;

and lines ~199:

foreach ($templates as $template => $variables) {
$generator->generateTemplate(
customSkeletonDir.$templatesPath.'/'.$template.'.html.twig',
$customSkeletonDir.'crud/templates/'.$template.'.tpl.php',
$variables
);
}

• Remove the 'customSkeletonDir.', otherwise it will generate a complete folder structure from root to the generated templates;

This is really a handy solution! Thank you for this.

[johnpancoast]

I'm a bit late, but I think this is/was a perfect use case for Twig. I think that https://github.com/symfony/maker-bundle/pull/520/files should be held off.

No offense meant @weaverryan, we just disagree, but I don't see how having a templates directory in an API-only project matters. What if I have an API application that is also heavy on emails or Console commands (or anything else that one can imagine) and I find that Twig is useful for those cases, for whatever reason? Then my API app might have a templates directory which is fine if there are Twig templates, regardless of if they're used for the web responses or not. The beauty of Symfony is in its independent libraries and how they can either coalesce into a framework or how they can be used independently for any case that a developer finds useful.

Twig is a templating language, nothing more, and code to be generated belongs in templates. In fact, all of the code that's generated already lives in templates, it's just that they are in PHP templates which loses the awesome behavior that Twig allows.

Going further, the maker bundle is for generating code in my application. I don't see why overriding templates or generation code should be made difficult for the developer. Sure, I can make my own makers/generators, but why force that on me if overriding templates can be done easier?
[EDIT] - My bad, I obviously glossed over some of this too quickly and didn't see that Twig wasn't chosen to keep things simple at the time. Obviously this RFC is to ask the question if skeletons should be overridable and if Twig should be used for that. No offense meant to devs or design choices in the beginning. But my vote is (clearly) in favor of Twig.

[EDIT AGAIN] - I also didn't see the Feature tag... wow, I really should've read this more before I wrote anything. Apologies!

For an example, my current case is that I want to remove the commented out examples in generated Repositories like the one below. Even if it's easy to create my own maker command, I don't think I should be forced to do that for something as simple as this. I also don't think that changing the directory of the PHP templates is the answer as in #520. I think we should definitely still consider Twig's use case here.

Another good example of something where I shouldn't have to create a whole command is if I want to just add PHPDoc blocks. It's trivial to just add them after the fact, but I'd love if I could just override a Twig template/block instead.

<?php

namespace App\Repository;

use App\Entity\Foo;
use Doctrine\Bundle\DoctrineBundle\Repository\ServiceEntityRepository;
use Doctrine\Common\Persistence\ManagerRegistry;

/**
 * @method Foo|null find($id, $lockMode = null, $lockVersion = null)
 * @method Foo|null findOneBy(array $criteria, array $orderBy = null)
 * @method Foo[]    findAll()
 * @method Foo[]    findBy(array $criteria, array $orderBy = null, $limit = null, $offset = null)
 */
class FooRepository extends ServiceEntityRepository
{
    public function __construct(ManagerRegistry $registry)
    {
        parent::__construct($registry, Foo::class);
    }

    // /**
    //  * @return Foo[] Returns an array of Foo objects
    //  */
    /*
    public function findByExampleField($value)
    {
        return $this->createQueryBuilder('f')
            ->andWhere('f.exampleField = :val')
            ->setParameter('val', $value)
            ->orderBy('f.id', 'ASC')
            ->setMaxResults(10)
            ->getQuery()
            ->getResult()
        ;
    }
    */

    /*
    public function findOneBySomeField($value): ?Foo
    {
        return $this->createQueryBuilder('f')
            ->andWhere('f.exampleField = :val')
            ->setParameter('val', $value)
            ->getQuery()
            ->getOneOrNullResult()
        ;
    }
    */
}

[MrYamous]

What would be the benefits of using Twig as a dependency?
Currently "skeleton files can't have complex logic" as javiereguiluz said, but I'm not sure that's a problem. I mean this bundle has been made to be very extensible, considering that I think that the default generated code should remain simple but give developers the possibility to adapt it to their needs.

MUbedaSJ's proposal seems to me to be a good compromise, which gives everyone the possibility to override default templates, without having to create a new command, and also allows to use, or not, Twig in the custom templates.

[MagnusDot]

What would be the benefits of using Twig as a dependency?
Currently "skeleton files can't have complex logic" as javiereguiluz said, but I'm not sure that's a problem. I mean this bundle has been made to be very extensible, considering that I think that the default generated code should remain simple but give developers the possibility to adapt it to their needs.

MUbedaSJ's proposal seems to me to be a good compromise, which gives everyone the possibility to override default templates, without having to create a new command, and also allows to use, or not, Twig in the custom templates.

that's exactly what i did in my PR

[johnpancoast]

@MrYamous, @MagnusDot. Oops, responded with another account. Responding with correct one. Remember that I'm late to this RFC and to this bundle, but it was still open so I felt like adding my thoughts. I haven't reviewed all of the maker code yet, but I'll expand on my thoughts.

that's exactly what i did in my PR

So far, it looks like your PR is changing the crud maker to allow it to read from a directory relative to your current working directory. I don't know if that's the right approach. My CWD is not always relative to the project, meaning I have to cd into a certain directory for that command to load a template from a certain directory, and it's only for the crud maker at the moment.

Currently "skeleton files can't have complex logic" as javiereguiluz said, but I'm not sure that's a problem. I mean this bundle has been made to be very extensible, considering that I think that the default generated code should remain simple but give developers the possibility to adapt it to their needs.

The bundle doesn't seem extensible in the way I need. How would I change the template for the repository that's created from the make:entity command, for example? The make:entity command calls the EntityClassGenerator which is a final class (cannot be extended, although you couldn't extend here anyway since the class is called directly). That class says where the template is but I cannot change that, at least not in a way that I can immediately see.

Writing my own command simply to change minor template content seems like overkill. And perhaps I can override the generator services themselves but that definitely seems like overkill. I know that changing the template directories is a possibility discussed and coded a bit in the PR, but we're still just left with PHP templates that would need to be copied completely, the PHP templates in maker bundle do not have the power of Twig templates.

What would be the benefits of using Twig as a dependency?

The code is generated from templates that already have conditions, there are use cases for inheritance of these templates when people just want to override certain parts or the whole template, and twig is a templating language built for things like that. I don't see the harm in adding dependencies that are used for their intended purpose.

I definitely appreciate the challenges, but it seems as though there's hesitancy to Twig for whatever reason and I'm curious why. Although these are templates for maker-generated coded, they are still just templates with conditions where inheritance would be beneficial for minor tweaks. These are perfect use cases for Twig (twig can be used for many template needs, web, email, others) and I don't think it makes anything more complicated. Perhaps there's a bit of dev to do in maker bundle to add Twig, but it's not that much work, and I think the flexibility it would provide users/clients is worth it.

[calls9-tylersmith]

Anyway, for the developers that disagree with the decision to distrust their ability to write their own templates, enjoy the following ridiculous workaround which exploits the brilliant code that we're not allowed to override:

        $controllerPath = $generator->generateController(
            $controllerClassNameDetails->getFullName(),
            '../../../../../../src/Maker/skeleton/controller/MyController.tpl.php',
            [
                'use_statements' => $controllerUseStatements,
                'route_path' => Str::asRoutePath($controllerClassNameDetails->getRelativeNameWithoutSuffix()),
                'route_name' => Str::asRouteName($controllerClassNameDetails->getRelativeNameWithoutSuffix()),
            ]
        );

(Along with creating src/Maker/skeleton/controller/MyController.tpl.php)

[javiereguiluz]

We didn't agree on this ... so let's close as "won't fix" ... and for those needing it, they can use the "hack" shown in the previous comment. Thanks.