jacmoe / yii2-tale-jade

A Tale Jade for PHP integration for the Yii2 framework (Retired)
MIT License
4 stars 0 forks source link

Wide-compatible adapter #2

Closed kylekatarnls closed 6 years ago

kylekatarnls commented 6 years ago

Hi, continuously to https://github.com/Talesoft/tale-jade/issues/84#issuecomment-339641920 and #1 I propose you to do the same time you will rename the project to make it evolve to more extensibility and global compatibility.

Pug-php also has a Yii2 adapter: https://github.com/rmrevin/yii2-pug but not yet compatible with Pug-php 3 and so Phug. The maintainer did not reply to my issue about that https://github.com/rmrevin/yii2-pug/issues/1 so I did a quick adapter to make it compatible: https://gist.github.com/kylekatarnls/5b748c8fd351906447305703339bf5cc As you maintain https://github.com/jacmoe/yii2-tale-jade, maybe you would be interested in a more global project:

What about creating a new adapter that would replace them all and be referenced by tale-pug, pug-php and the phug documentation as the officiel adapter to use to get pug templates to work with Yii. I have the "pug" namespace on composer and so you could have a package name like "pug/yii" that would be well referenced (and it would take into account the renaming of jade into pug).

What about the following usage:

return [
    //....
    'components' => [
        'view' => [
            'renderers' => [
                'pug' => [
                    'class' => 'Pug\YiiViewRenderer',
                    'engine' => 'Tale\Pug\Renderer',
                    'cachePath' => '@runtime/pug/cache',
                    'options' => [...],
                ],
            ],
        ],
    ],
];

With the engine Tale\Pug\Renderer option specified, it would use Tale\Pug\Renderer instead of the default that would be Phug\Renderer and you would just have to adapt the small differencies of thr render method:

$method = method_exists($this->parser, 'renderFile')
    ? array($this->parser, 'renderFile')
    : array($this->parser, 'render);

return call_user_func($method, $file, $params + ['app' => Yii::$app, 'view' => $view]);

This way, your adapter would be extensible and customizable (allow user to extend the renderer class), it would support old and new tale-pug, old and new pug-php, and phug or any engine that would provide and renderFile or a render method.

About your ->addPath it seems to me like a breaking pattern and unneeded since you can access a file with relative path, so adding a path that would make all relative paths available with a absolute path (with a / can have conflict with truly absolute paths), the base path should not be added at each render but only once and should be the framework views directory, not the parent of the rendered file.

Of course I can help you in this if you need.

What do you think of it?

jacmoe commented 6 years ago

Rmrevin started his project way after I started mine, and it looks like it just does regular Pug templates, which is fine!

This project, however, makes full use of Tale-Jade's extended features, like calling PHP directly inside the templates, or including PHP or JS files as-is.

It's like regular Pug, but on steoroids.

I am not sure about a super-adapter, because the aim of this project is to take max advantage of Tale-Jade's (Tale-Pug's) special powers.

jacmoe commented 6 years ago

Yes, I know that I need to rename (i.e. create a new project) - I have been waiting for Tale-Jade - apparently Torben forgot to ping me when he created Tale-Pug 😄

jacmoe commented 6 years ago

But, yes: I will think about it! It's just that I am slightly allergic to premature generalization - especially since I have no real interest in supporting other flavours.

jacmoe commented 6 years ago

On another note: it is a pity that the Yii 2 community is so reluctant to use a template language other than just plain PHP - I have been trying to convert them, but alas ...

kylekatarnls commented 6 years ago

In fact, if I wrote the pug-php/phug Yii adapter, it would support de facto old and new Tale-pug versions and so it would be the solution we would promote, so it would make your project obsolete. I would prefer to let you the ownership of this.

It's also the approach we had by merging engine of both tale-pug and pug-php, instead of letting user have a difficult choices between many partial isolate projects, we preferred to propose a standard but customizable solution maintained by a bigger community that join the effort.

First, think about the main need of user: I don't think they're looking for a tale-pug, pug-php or phug adapter for Yii, most are looking for a way to get pug working in Yii with no particular knowledge about existing PHP engine for that. Once they reach the referenced project for that, it should provide them the most full-featured engine by default, then let them the possibility to use something else and inform them of what other common solutions that exist.

If you don't like the method_exists solution (I can understand that, it's the quick-fix solution), you can consider having an EngineAdapter directory, with TaleAdapter.php and PhugAdapter.php and a common interface for both. So to extends, user would have to create a class that implement the EngineAdapterInterface and plug it with an Yii option or a method of the ViewRenderer. As you prefer.

jacmoe commented 6 years ago

So, basically you are threatening me and giving me an offer that I can't refuse ? 😃

If I can do exactly what I do with your grandiose solution as I do now - see the readme for the motivation behind this project - then I wouldn't mind looking into it, or (better!) review a pull request.

I had (and still do) a good reason for basing my wrapper on Tale-Jade (and not one of the other PHP jade wrappers/libs).

jacmoe commented 6 years ago

To be totally honest: I am in the dark with regards to where Tale-Jade is going - it seems that it is going to Tale-Pug at the moment, but where will it go from there?

You say that the back-end is going to be Phug, is that right? - is that permanent or just a stop-gap until everything has migrated directly to Phug?

kylekatarnls commented 6 years ago

Answer for https://github.com/jacmoe/yii2-tale-jade/issues/2#issuecomment-339685384

That's not the way I see the things. Yii is not my priority but when we will promote Phug (when the documentation will be ready) I would like to propose implementations in each frameworks (it will also be promoted on the pugjs README probably, we're are discussing about that with pugjs staff). If at this time, the making of the Yii Phug adapter does not interest anyone else, I will do it in a cross-compatibility way. But I would be very sad to do it alone against other solutions, I prefer team-work and I'm ready to do compromises if it can help to make the community work together for the pug ecosystem.

My main goal is giving easier access to Pug to PHP users and help for support. (Right now, people ask questions on stackoverflow and github issues, and use different engines and adapters), That makes things difficult for users. They found a solution that is good for some specific engine but not another. I think Pug PHP solutions need to merge to get more common and standard options/features when it's possible.

kylekatarnls commented 6 years ago

Answer for https://github.com/jacmoe/yii2-tale-jade/issues/2#issuecomment-339689780

Tale-pug is a mirror of tale-jade (just renamed) and will implement phug as soon as possible, as you can see @TorbenKoehn is participating as actively as me in https://github.com/phug-php

@TorbenKoehn would probably confirm that we discussed about: we prefer to work together of making Phug better and make all user benefit of improvements by upgrading to new versions instead of spending effort to make same things in parallel as we did before.

So tale-pug 1 and pug-php 2 will be fixed when small issues are found to bring backward support to existing users. But phug throught tale-pug 2 and pug-php 3 will be our priority over older versions.

jacmoe commented 6 years ago

If I can freely mix PHP and Pug in my templates like I can with Tale-Jade/Pug, if I can create mixins, and if I can include PHP/JS as well - i.e. if it does everything that Tale-Pug does - then I will be happy about it all! If it is just a plain Pug engine for PHP, then it is IMO too limiting.

If you give me an interface that I need to adhere to when wrapping, I can switch to that, no problem. 😄

kylekatarnls commented 6 years ago

If you read on in-progress documentation, I think you can have a more clear idea of what Phug exactly is: http://phug.selfbuild.fr/

Mixins are availables like any feature in pugjs, Phug pass 100% of pugjs unit tests, it means basically, we support at least all the pugjs features. So yes mixins, dynamic mixins, mixins attributes/assignments, mixins recursion and variables propagation are supported.

It's also pure-PHP and handle by default every expressions as PHP expressions. To handle them as JS expressions, it's an optional extension to plug.

About the naming, are you sure, it could not be compatible if someday Yii 3 is out? Else the 2 should not be in the name. Then, I would propose to simply use pug and reather have phug in repository keyword, but I think users more likely search for pug than anything else.

jacmoe commented 6 years ago

Ok, since I have to create a new repository for this with a new name, I am going to name it yii2-phug (based on Tale-Pug) - it will be more future proof in the long run, right?

jacmoe commented 6 years ago

Scrap that: yii2-pug it is, then.

I would like to just call it yii-pug but I don't think I am able to get away with that just yet. :)

All Yii extensions/whatevers follows that naming convention because Yii 1 and Yii 2 are so different.

kylekatarnls commented 6 years ago

But versioning is not supposed to be handled by project name. It should be done with composer by upgrading the version number. If your repo is the official adapter, other developers may help you for next release by providing pull-request, it does not mean you're force to maintain it alone forever ^^

jacmoe commented 6 years ago

I agree fully! But what can you do :p

Thanks for the link to the Phug docs - I can see where you're coming from. It definitely makes a lot of sense to join forces!

Also, it looks like I can have my cake and eat a better one as well. 😄

jacmoe commented 6 years ago

Should I base my adapter on the 4 Pug-php 3 adapters?

I will (try and) do that once I create the new repository.

jacmoe commented 6 years ago

I will grab the gist first, of course - thanks! 👍

jacmoe commented 6 years ago

About your ->addPath it seems to me like a breaking pattern and unneeded since you can access a file with relative path, so adding a path that would make all relative paths available with a absolute path (with a / can have conflict with truly absolute paths), the base path should not be added at each render but only once and should be the framework views directory, not the parent of the rendered file.

The problem is that views can be outside of the main base path, because of the way that view file overrides works in Yii.

Perhaps I'll add an array of view paths..

kylekatarnls commented 6 years ago

Multi paths seems to work well with the new adapter I recently made: https://github.com/pug-php/pug-yii2 please report it if you find some bugs on it.

jacmoe commented 6 years ago

I will! :)

Does this mean that I can safely abandon all my Yii2 jade/pug extensions? I mean, your adapter seems pretty official.

I have been busy with NaNoWriMo this November, mind you. And just now I am slowly getting back into programming/projects. Code does rot, doesn't it?

kylekatarnls commented 6 years ago

Your work was pretty helpful to me to create this adapter, it's some kind of combo of rmrevin/yii2-pug and jacmoe/yii2-tale-pug.

What we do with @TorbenKoehn aims to bring a standard ecosystem for Pug in PHP. So yes, I would prefer to all of us @rmrevin, you and me working together on the global solution that each user can customize with options over individual solutions for each engine and each version of those engines.

I would like a lot you tell me about features you may loose from jacmoe/yii2-tale-pug to pug/yii2 and pull-requests are welcome as always!

jacmoe commented 6 years ago

I will!

The only problem that I have now is PHP 7.2 compatibility, but I see that we both have pull requests open on tale-pug that deals with the count issue.

I will test, and contribute, as needed. Of course! Open source is all about scratching your own itch, and work together.

jacmoe commented 6 years ago

I am not impressed by this:

$path = realpath(Yii::getAlias('@app/views'));

There is no 'app/views' in my application.

jacmoe commented 6 years ago

Fixes here: https://github.com/pug-php/pug-yii2/pull/1

kylekatarnls commented 6 years ago

To be honest, I never made an application with Yii2 and found @app/views in the documentation: http://www.yiiframework.com/doc-2.0/guide-structure-views.html

jacmoe commented 6 years ago

They usually are, though. Except when you are using a theme :)

kylekatarnls commented 6 years ago

Hi, before I tag the version, can you please review my last changes (https://github.com/pug-php/pug-yii2/compare/50fa0ad...master) or retry to update to the last dev-master to check nothing is broken for you?

jacmoe commented 6 years ago

Will do!

IIRC, $app and $view are exposed to the view by default in Yii, so that's why they were passed in yii2-tale-jade. :)

jacmoe commented 6 years ago

It works perfectly! 😄

kylekatarnls commented 6 years ago

pug/yii2 1.1.0 released with $app and $view exposed.

jacmoe commented 6 years ago

In case I haven't thanked you for taking over the Yii2 Pug renderer, let me thank you now ;-)