search

zordius / lightncandy

An extremely fast PHP implementation of handlebars ( http://handlebarsjs.com/ ) and mustache ( http://mustache.github.io/ ),
https://zordius.github.io/HandlebarsCookbook/
MIT License
610 stars 76 forks source link

Valid use for Lightncandy::prepare() #346

Open brunobg opened 4 years ago

brunobg commented 4 years ago

Regarding Lightncandy::prepare() and https://github.com/zordius/lightncandy/pull/323: lightncandy can be used for dynamic code or documentation generation at compile time execution, which will only run once and therefore there's no gain in caching to a file. So prepare() is not a good idea for most use cases, but definitely good for this case, as saving PHP code to a file to include it is cumbersome.

Even if the @deprecate annotation is kept, I suggest at least the @return type fix.

robentwist commented 4 years ago

Completely agree with brunobg. I use this to dynamically generate an email that is used once. LightnCandy is a much better parser than other frameworks/plugins and is perfect for this use. Caching the file is not necessary since it is only ever used once.

jenschude commented 4 years ago

The prepare function is doing exact the same things. Write it to a temp file and include the code. See https://github.com/zordius/lightncandy/blob/5f8dc549e834cee6080b3820f71f1076084fe8ea/src/LightnCandy.php#L152-L157

brunobg commented 4 years ago

Exactly. It's more convenient to call prepare(), which is properly written and tested, than to reimplement it exactly as it is on the target application.

jenschude commented 4 years ago

You could create a small function or Helper class and copy the prepare method in case you need it. This will ensure that you have to the control.

And I meant that prepare is doing exactly what robentwist said is not necessary.

robentwist commented 4 years ago

Why do you have to remove a feature that works, people want it, and does exactly what you're proposing putting into an outside class?

brunobg commented 4 years ago

You could create a small function or Helper class and copy the prepare method in case you need it. This will ensure that you have to the control.

Sure, I could also re-implement substr() to ensure I have control and add to a Helper class. Should I? No.

And I meant that prepare is doing exactly what robentwist said is not necessary.

To be pedantic, it's not cached with prepare(). Caching means storing it. You use a temporary file that is deleted as soon as the function returns. If I can't used it twice, it's not cached.

Why do you have to remove a feature that works, people want it, and does exactly what you're proposing putting into an outside class?

Can't put it better than @robentwist did. And as @danmcadams said, it's not like you don't use the function all over the docs, too.

More time was spent discussing this issue than accepting a trivial one-line patch that corrects the @return decoration that is actually wrong.