[Grok-dev] grokcore.view feedback

[Martijn Faassen]

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.

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. 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.

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.

* 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.

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.

* 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.

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).

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?

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.

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.

Regards,

Martijn