ebrehault / collective.ttw

Happy hacking for Plone
3 stars 1 forks source link

Relation to zpt-fragments? #1

Open datakurre opened 9 years ago

datakurre commented 9 years ago

@ebrehault Just checking, are you aware of Martin's proposed zpt-fragments-feature for plone.app.theming? The original pull to merge it was rejected, but we have been using it with success and and I've been keeping it compatible with the 1.2.x branch (Plone 4.3.x.).

It adds support for in-theme "fragments" folder, which is published via @@theme-fragment traversal view (with the current context). Looks similar to ideas in your README.

It is only for ZPTs, but I see no reason, why it could not be extended to also support restricted python scripts.

datakurre commented 9 years ago

I finally released Martin's zpt-fragments as its own add-on

https://pypi.python.org/pypi/collective.themefragments

It should work with all p.a.theming releases, but a bug in p.a.theming prevents previewing the fragments.

Also, I released a hobby project for embedding GS install (and unistall) profiles with themes

https://pypi.python.org/pypi/collective.themesitesetup

Unfortunately, there has been regression in p.a.theming plugin support and this requires a fix for most p.a.theming versions to be released.

ebrehault commented 9 years ago

That's very interesting ! I am currently working on Diazo enhancements to support content modification. The global idea has been described here: https://github.com/plone/diazo/pull/42 But it will be implemented differently (and the syntax will be different):

Combining the fragment approach with Diazo content manipulation would be extremely powerful.

datakurre commented 9 years ago

@ebrehault Cool, although, I believe that idea of zpt-fragments was to cover most of those use-cases (already years ago, when Martin made the original and rejected pull for p.a.theming).

That aside, I'm preparing to add support for restricted python scripts into collective.themefragments and I would appreciate your comments on it.

In the current implementation

https://github.com/collective/collective.themefragments/commit/75c194b26a5dd9a3827c2d43876042fb3cd860c2

If you have fragment

fragments/navbar.pt

you can have method

fragments/navbar.get_elements.py

and call it in fragments/navbar.pt like

<tal:block repeat="element view/get_elements">
...
</tal:block>

So, I'd prefer the old style "one script per function/method" and pass them context instead of adding multiple funtions into one script. Currently I only pass the view to the method as "self", but probably I should pass at least also context and container to make it more like the legacy PythonScripts.

Does that make sense or did you have some specific reason for planning to support multiple functions in a single file?

Also, I'm not sure if namespacing scripts makes any sense, or view/foobar-lookup simply look for all the scripts in theme's fragments-directory.

ebrehault commented 9 years ago

Well the main reason is it is easier if we allow users to put many functions in the same file. It avoids having a long list of files with tiny code, and it allows to factorize the code:

def clean_up_data(data):
    return blabla

def do_something():
    return clean_up_data(something)

def do_something_else():
    return clean_up_data(something_else)

As @djay mentionned, I have already implemented a secured Python script runner based on zope.untrustedpython.interpreter. You will find it here: https://github.com/plomino/rapido.core/blob/master/rapido/core/formula.py

And just to let you know, I plan to move rapido into the theming editor (just like you did for fragment, we would create a folder /rapido/myapp, and we will edit the app structure through YAML, HTML and Python files. And it will involve this Python scripting mechanism of course.

datakurre commented 9 years ago

Eric BREHAULT wrote:

Well the main reason is it is easier if we allow users to put many functions in the same file.

But that's something completely new when compared to legacy Zope scripts or to class based views... against that I understand @djay's wish for class based TTW views.

But I now finally understand that c.ttw and c.themefragments approach the same problems from opposite angles. In c.ttw you call Python code first and only then may use a template render (agreed that that's more like in Pyramid), but in c.themefragments you call template first (or a template wrapped into a browser view).

So, it seems, I should keep c.themefragments separate from c.ttw. We've been using themefragments already for some time, so I don't want to abandon that approach.

I'll see, if I can get c.themefragments' script support to be similar enough to legacy PythonScripts so that they would be as old school as the original zpt-fragments themselves.

ebrehault commented 9 years ago

I think it is fine to have 2 separated approaches here, as we are still investigating, it will enrich our insight on the problem, and we might merge later (if accurate).

djay commented 9 years ago

I don't see any advantage in maintaining similarity with zmi Python methods. The closer it is to real python the better and the ability the group functionality into one file is a plus. As much as possible we should be reduce the extra people have to learn who are knew to plone. So if they know python then they shouodnt feel frustrated. I guess from that perspective making functions work like pyramid or django is good but it would be nice if they do later want to mice to the filesystem that they don't have to rework it.

And I defiantly agree that not requiring a zpt is a good idea. Ideally you can use zpt without Python or Python without zpt I think. But we should minimise the use of zpt because its confusing to have two templating languages. On 3 Apr 2015 00:10, "Asko Soukka" notifications@github.com wrote:

Eric BREHAULT wrote:

Well the main reason is it is easier if we allow users to put many functions in the same file.

But that's something completely new when compared to legacy Zope scripts or to class based views... against that I understand @djay's wish for class based TTW views.

But I now finally understand that c.ttw and c.themefragments approach the same problems from opposite angles. In c.ttw you call Python code first and only then may use a template render (agreed that that's more like in Pyramid), but in c.themefragments you call template first (or a template wrapped into a browser view).

So, it seems, I should keep c.themefragments separate from c.ttw. We've been using themefragments already for some time, so I don't want to abandon that approach.

I'll see, if I can get c.themefragments' script support to be similar enough to legacy PythonScripts so that they would be as old school as the original zpt-fragments themselves.

— Reply to this email directly or view it on GitHub https://github.com/ebrehault/collective.ttw/issues/1#issuecomment-88977339 .

datakurre commented 9 years ago

Dylan Jay wrote:

I don't see any advantage in maintaining similarity with zmi Python methods.

It's the only existing (and therefore familiar) alternative for full class based views.

I can agree, why class based views are better, but don't know yet, how they should be implemented in Restricted Python (e.g. is it safe to allo importing BrowserView base class). Themefragments were started as restricted alternative for jbot, so they should remain restricted.

Also, extra boilerblate for imports, class definition and function definitions plus the required intending levels makes small scripting without IDE more error prone.

And I defiantly agree that not requiring a zpt is a good idea. Ideally you can use zpt without Python or Python without zpt I think. But we should minimise the use of zpt because its confusing to have two templating languages.

It was not yet clear, how to reach templates from Python, so I stayed with the zpt approach in themefragments. (Partly also, because templates were already mapped into browser views, so in that way it could be implemented now.)

I can understand the reasoning in going all XSLT in templating, but I was there with DSpace XMLUI years ago and don't miss it.