[Grok-dev] Re: grokcore.view feedback

[Godefroid Chapelle]

Martijn Faassen wrote:
> Hi there,
> 
> Thanks for doing the work on grokcore.view! This will not only be useful 
> for those developing on Zope 2, but also for those who will want to 
> develop with Zope 3 directly and mix Grok facilities into their own 
> applications.
> 
> grokcore.view does look like a different beast than grokcore.component 
> right now - it's not usable out of the box in a Zope 3 application. If I 
> read it right you have to subclass various components and grokkers to 
> make it work. This bothers me - I'd like to tell a Zope 3 developer to 
> just use grokcore.view if they want to use Grok-style views and forms, 
> but one can't use it that way right now. It's only there to support the 
> use case of reuse in five.grok right now. We should fix that and support 
> both use cases.
> 
> A note on naming: I don't like the names 'GrokView' and 'GrokForm'. I 
> think one of the rules of Grok should be that we don't prefix things 
> after the package, as we already have namespaces. Grok hasn't been 
> perfect at following this, but we shouldn't be making it worse. I think 
> you've been following the convention of the non-exposed internal class 
> GrokForm and generalizing it to GrokView, and now expose it to users of 
> grokcore.view.

Exactly.

> 
> I think, reading the branch of grok, that these are intended to be base 
> classes, in which case I would much prefer ViewBase and FormBase. That 
> would make the intent of these classes a lot clearer. 

+1

> This also matches 
> it better to ViewGrokkerBase, which is introduced as well.
> 
> Note by the way that I think you should use martian.baseclass() on your 
> base classes, to make sure they aren't grokked directly.

Thanks reminding us.

> Looking at the implementation of grok.View and grok.*Form on your 
> grokcore.xxx branch, I don't really see a great reason from the 
> dependency perspective why those couldn't be moved into grokcore.view as 
> well. Let's review them:
> 
> grok.Form doesn't seem to depend on anything in Grok. It just has an 
> implementation of applyData and also is a subclass of grok.View (which 
> we'll get to later). It also pulls in a default template, but 
> grokcore.view should have the support for that.
> 
> grok.AddForm is just a subclass of grok.Form, without adding anything.
> 
> grok.EditForm doesn't really add much new either. It has some response 
> on self.request.locale existing which may lead to issues in a Zope 2 
> context, not sure.
> 
> grok.View sets up a default_namespace. I think this should be ported 
> over to the base class with, probably, the exception of adding static. 
> default_namespace in the subclass could be just the bit that adds 
> self.static to the dictionary.
> 
> As for flash and application_url:
> 
> * we could investigate whether z3c.flash just works on Zope 2. It 
> depends on zope.app.session I believe, so that'll be the trickiest. If 
> it works, we should move it to the ViewBase class.

Even if z3c.flash does work, I think this is wrong : Plone has its own 
'message' package and is one of the main targets for five.grok.

> * application_url is the hardest to support cleanly. It presupposes a 
> knowledge of grok.Application that grokcore.view shouldn't really have. 
> I think we should just bite the bullet and move at least the 
> IApplication interface over into grokcore.view, and make application_url 
> check for IApplication instead of isinstance on grok.Application directly.

Checking for IApplication makes sense to me : +1

> My proposal would be to:
> 
> * move grok.View, grok.Form, grok.EditForm, grok.AddForm into 
> grokcore.view. At most it will introduce a new dependency on 
> z3c.flashmessage, but that seems reasonably harmless.

I planned to propose to move Forms in a separate package 
('grokcore.formlib' ?) to avoid unneeded dependencies.

And similarly, to move viewlets to 'grokcore.viewlets'.

> * rename GrokView and GrokForm to ViewBase and FormBase
> 
> * push as much as makes sense from grok.View and friends into the base 
> classes
> 
> When this package is used in a Zope 3 context (such as with Grok), 
> depending on grokcore.view should just make the regular grok.View and 
> grok.*Form available to the programmer, just like grokcore.component 
> makes grok.Adapter and so on available.

+1 if we keep grok.*Form out of scope and move them...

> When this package is used in the context of five.grok, we can't expose 
> these, so we shouldn't register the grokkers for them. I think that 
> could be fairly easily accomplished by simply not loading meta.zcml from 
> grokcore.view. You'd simply subclass appropriately in five.grok. Perhaps 
> for convenience meta.py in grokcore.view should be split up into 
> something you'd like to grok always (in Zope 2 *and* Zope 3) and those 
> bits that only make sense in Zope 3 proper (which would remain in meta.py).

IOW five.grok would decide by itself what it wants to grok. Seems ok.

> Alternatively, we could explore expanding the functionality of the 
> 'grok' directive to allow the grokking of individual classes in a 
> module, though that will be harder to implement as some grokkers expect 
> global grokkers to have done their work.
> 
> grokcore.view also needs documentation like grokcore.component has. This 
> documentation should describe the two use cases:
> 
> * use out of the box by Grok and Zope 3 applications
> 
> * reuse of base classes for use in five.grok
> 
> By describing these things clearly we make it clear to everybody 
> (ourselves, future maintainers) what the different forces are that pull 
> on this package.
> 
> We can more cleanly support these two use cases from an import 
> perspective as well. What about making __init__.py only expose those 
> things that Grok itself exposes concerning views, and introducing a 
> grokcore.view.base that exposes those things reusable as base classes or 
> are needed in the implementation of sub classes?

I like that as well.

> Finally, hopefully in time we can just make grok.View and friends work 
> straight-up in Zope 2, simplifying grokcore.view again. Not today, however.

Rather, later...

> Thanks for all the work. I hope this feedback is not too intimidating. 
> I'm trying to make sure that grokcore.view is the best it can be.

Appreciated a lot !


> Regards,
> 
> Martijn


-- 
Godefroid Chapelle (aka __gotcha) http://bubblenet.be