[Grok-dev] Re: Heads up: Renamed AddForm to Form

[Martijn Faassen]

Martijn Faassen wrote:
> Philipp von Weitershausen wrote:
>>  From the commit log message:
>>
>> - Renamed grok.AddForm to grok.Form because there was no additional
>>   functionality in that base class that was specific to adding. Yes, it
>>   used formlib's AddForm, but none of the features that has to make
>>   adding objects easy for you were used (adding objects in Zope 3 is
>>   overly complicated anyway, thanks to IAdding). Everybody writing add
>>   forms was writing his/her own actions anyway, which is exactly what
>>   the point of a general grok.Form base class is.
>>
>> - Got rid of the "form in form" weirdness. It turns out that a very,
>>   very simple mix-in with a custom render() method makes it possible to
>>   mix formlib's base classes with grok.View. This makes the ViewGrokker
>>   simpler and exposes more of formlib directly in Grok forms.
> 
> This is a serious design change and we've done it the way it works for 
> actual reasons! It would've been nice to at least discuss these reasons 
> before we dive into a massive change like this.
> 
> I'm seriously surprised it is that easy - does overriding update() work? 
> What about overriding render()? I strongly suspect both now don't work. 
> My oversight is that we should've had tests for both cases to make the 
> intent clear. The point for the containment relationship was to avoid 
> the extensive name clashes between the formlib base classes and 
> fundamental grok assumptions.

Reviewing it a bit more, I see we did have tests for 'render()', which 
you removed. I'm not sure why we decided to completely forbid the use of 
'render' in this context anymore. Did you remove the tests as you 
couldn't make them work? :)

The reasons we went through the whole rigamole with form-in-form in the 
first place was that formlib base classes define a whole bunch of 
methods and attributes that enter the view's namespace. The original 
design by Christian and Wolfgang was this way, but it was a very 
primitive prototype and didn't work very well.

JW and I thought for a long time on how to make this work by a 
subclassing relationship, but concluded it was not a good approach 
because of the namespace pollution problem. It makes it impossible for 
Grok to inspect the view class for things like 'render()' and 
'update()', as they are already defined on base classes. Instead I then 
went to the design you just wiped out.

I'm not sure whether update() was a problem at the time we did the 
design actually, as I think it was still called before(), but it 
certainly is now.

The idea of grok base classes is that what's in there is strictly 
controlled and limited. We don't want to surprise users with a lot of 
stuff when they subclass something from Grok. Pulling in formlib base 
classes tends to break this assumption.

Besides that philosophical issue, the biggest problem is that you can't 
use 'update()' in form subclasses at all with the current design. With 
the original design it does work (I just tried).

Regards,

Martijn