cakephp / bake

The Bake Command Plugin
Other
110 stars 100 forks source link

Coding standards for cake bake templates #185

Closed ionas closed 6 years ago

ionas commented 8 years ago

I'd like to suggest that we do in future stick to either:

Suggestion Personally I'd like to see one of the "ways" of constructs to be used for baking level only and one for the generated code. If seen that way maybe it makes sense to:

Related

markstory commented 8 years ago

I think this is mostly an issue of time and priorities. If you have time and feel its important please charge ahead. We have existing coding standards, and reusing those as much as possible seems like a win to me.

ionas commented 8 years ago

:+1: Thought so.

But first if I find time I ll supply test to the bake composite key issue. Then I'll take a look and try to make things consistent.

What about the generated result. Is regular syntax for anything but view template results AND alternative syntax for view template results okay?

markstory commented 8 years ago

I think using whatever makes it easy to separate the templates/output is reasonable.

dereuromark commented 8 years ago

Sounds to me like the one place where twig would really make sense.

simkimsia commented 8 years ago

My (somewhat related) question is

  1. how do you even test the coding standards of the baked templates? another codesniffer?
  2. is there a testcase for the baked actions?
AD7six commented 8 years ago

Personally I'd like to see one of the "ways" of constructs to be used for baking level only and one for the generated code.

:-1:

For reference, standard constructs are used for php classes and in view templates it depends on whether using standard constructs would mean hopping in and out of php tags - it is not imo a good idea to arbitrarily decide to use one or the other for output and the other for template logic - the factor there is php tags (<?) or tempalte tags (<%)

We already run phpcs against test comparison files - i.e. we are already verifying bake output matches the code standard. If there are examples of baked output not matching the code standard - it indicates missing tests.

dereuromark commented 8 years ago

So RFC for CakePHP4: Please let's use twig or better yet liquid for the template code (which generates our PHP and HTML code), for both there are also cake plugins then (e.g. https://github.com/gourmet/liquid ). The advantages are clear:

Why I think liquid might be better:

but there are sure other options to also evaluate.

Did I miss anything?

For me this is way more important than a few spaces or coding standards. But I wouldn't mind if we got a little bit better documented and organized here at some point, too : ) (More and clear rules, enforcing standards etc.)

ionas commented 8 years ago

I am torn on adding another dependency and making bake templates more complex than it is. PHP is already "THE" templating language*.

So I with @AD7six's custom template tags for now, but I do think that we should specify not only <?= vs <%= but also that for anything but to use the inverse control structures style compare to what the result should be. For Tables, Entities, Controllers (and Views) this would be alternate control structure style in the meta templates. For the view templates it would depend on what we desire the output to be, standard style or alternate style? The alternate style allows to stay in a PHP tag, so I am not totally following there.

Btw. one could do whatever with their own templates. It is about the out of the box templates and conventions.

dereuromark commented 8 years ago

PHP is not a templating engine, but you are missing the point. We are NOT using PHP for templating, it is some pseudo code asp tag thing. Which is not really readable and no IDE understands it and can help us with that.

ionas commented 8 years ago

Thanks for being so kind, leaving it to me to discover if I miss the point. PHP is one of the strongest templating languages, next to not being only that. As far as I understood the pseudo-PHP tags are rewritten to PHP tags in an internal cache so in essence, we are doubling up on PHP templating - correct me if I am wrong.

Don't get me wrong, I do like twig and even more so _liquid_ (which I touched in my Ruby days): https://github.com/Shopify/liquid/wiki - my vote counts little and if you can convince @AD7six @markstory, et al. and then start implementing based on liquid/gourmet I will :clap: x :100: - I am just unsure if its worth adding another technology/layer because it increases complexity and dependencies.

dereuromark commented 8 years ago

Bake is an optional plugin with pretty much independent development from the main framework. I wouldn't wry about that too much, just as Migrations uses phinx internally, or any of those other optional plugins. In these cases we could easily lock the versions to a bugfix even, as those are not productive relevant. There is no such thing as security issue here, if any dependency does weird things, we just stick to the last "good" version.

I would wry about maintainability and usability in itself, and if you put all pro/contra arguments together maybe the decision can be made easier in what direction to go. Right now it does not seem too much fun to work with.

And internal caching I also don't care much about. Speed is not an issue for development baking, the usability and customization options very much are, though.

simkimsia commented 8 years ago

Right now it does not seem too much fun to work with.

Agree

the usability and customization options very much are, though.

Agree

If there are ways to increase the usability and customization options without using liquid or twig, I am also for it.

Right now the usability and customization options are not really quite there yet.

dereuromark commented 8 years ago

It seems, there is only Twig support yet for PHPStorm. For Liquid etc there is still an open ticket: https://plugins.jetbrains.com/wishlist/show?pr=&wid=404

And as for Twig, there are even some nice cheat sheets for a quick overview: http://willthemoor.github.io/Twig-Cheat-Sheet/

ionas commented 8 years ago

As for speed: It should not be much slower than now. I am at times running 150 bake commands concatenated in bash calls generating/updating hundreds of files. That takes only a few seconds and should not change (by much).

As for PHPStorm: one source code editor app is no reason to base a decision on. As for liquid/twig: aside stability, support and learning curve I am concerned about how hard it is to map cakephp helpers, cakephp basic functions and specific cakephp global functions.

dereuromark commented 8 years ago

That is to be found out. But right now, one has to learn this custom asp tag thing, which is not IDE supported ever, and as such it can only be easier to learn an open source template engine. But sure, we will have to investigate how custom functions and CO can be built with those.

markstory commented 8 years ago

If we're going to use a template system, twig gets my vote. It has a significant adoption outside of CakePHP and is a well maintained library.

dereuromark commented 8 years ago

@markstory And since it is a dev only tool and will never be deployed to servers, it is also not a big deal if the license is not MIT.

markstory commented 8 years ago

Twig is BSD, which is roughly the same as MIT.

HavokInspiration commented 8 years ago

As for using a template engine, I'm really in favor of it. I've been writing the bake template for migrations diff this week-end and it is really not pleasant to work with when you are writing "big" files.

I agree with Twig as well : if we are going to adopt a dependency, it might as well be one that has a bigger community and a good adoption outside of "our scope".

dereuromark commented 8 years ago

@HavokInspiration For me the killer feature is still the IDE support (My files are red underlined from the top to the bottom, and doing all kind of wonky things because of that^^). Code-highlighting and Auto-complete would be only two of the many benefits.

Maybe we can make our style an IDE plugin? :)

HavokInspiration commented 8 years ago

IDE support would be a nice side-effect indeed even though I think (re-)usability and readability is the priority since not everyone is using the same IDE.

And yeah, PHPStorm is imploding within bake template. Funny thing is when you copy paste from another bake template and it tries to auto-indent...

dereuromark commented 8 years ago

Yes, the auto-indent is already quite annoying.

markstory commented 8 years ago

Does anyone want to take a pass at updating the current templates to use Twig? I think we will need to keep the current template renderer as well for backwards compatibility.

AD7six commented 8 years ago

I don't understand the desire to use twig/liquid - but I also don't have the time/energy for the debate either. If someone want's to change bake to use twig/liquid instead just to have IDE support (instead of e.g. just changing bake template's file extensions to not be php) I'm fine with that.

htstudios commented 8 years ago

In case this is done :+1: @markstory for keeping the old way for quite some time.

markstory commented 8 years ago

@AD7six Part of the desire comes from IDE's mishandling the current templates, the other comes from a few subtle issues related to the current template syntax.

dereuromark commented 7 years ago

You can see the Spryker code generator use twig templates, they make it very easy to read and write the code IMO

Maybe we could adapt sth like that for the next generation of this plugin?

dereuromark commented 6 years ago

Closing as the main point left here has been extracted to a follow up issue.