[Zope-dev] [Checkins] SVN: zope.publisher/trunk/ Added some BBB code to setDefaultSkin

[Hanno Schlichting]

Hi.

Roger Ineichen wrote:
>> Roger Ineichen wrote:
>> Then there's some tests and description missing proving that 
>> intent. All I could read in the changelog and the skinnable 
>> tests was pointing in the other direction: Making it possible 
>> to use the Skinnable concept without relying on 
>> IBrowserRequest. The whole JSONRequest (which is not a 
>> BrowserRequest) tests inside skinnable.txt continue to work.
> 
> Yes, I think everything was tested, but probably future
> ideas are not documented. There where some discussion about
> to split each request into it's own packages.

Where did that discussion happen? All I have heard of was discussions at
PyCon, where nobody quite seemed to see any point in the whole different
request types story at all anymore.

I don't mind if the skinnable story gets less intermingled with the
request story in a new zope.skinnable package and breaks some backwards
compatibility at that point. Right now that mix of the two concepts is
so prevalent in all kinds of places, that I'd rather stay on the
backwards compatible side.

All ZCML browser directives by default register everything for
IDefaultBrowserLayer and expect those resources to be available on
"normal" browser requests. The test helpers in zope.app.testing to get
browser resources set up rely on the same mix of concerns. This stuff is
used all over the place without caring about loading zope.publisher's
ZCML right now.

>> All I did here was to move two constructs from ZCML into direct code.
>> The lines I added do exactly the same as the default adapter 
>> registered as:
> 
> Uhh, this is call hard coded and makes it impossible to
> exclude the adapter with the <exclude> directive.

I call that retaining sensible defaults. You can opt-out of the
IDefaultBrowserLayer for browser requests by providing your own
specialized adapter. I prefer no configuration for the most common case
with an opt-out scenario instead of the everything is opt-in behavior.

> I see two things which are bad. Skinnable depends now on
> IBrowserRequest. I moved the skinnable concept out of the
> browser request part. This allows us to separate the skinnable
> and all different request types into own packages if we do future 
> refactoring.

zope.publisher is the package that defines the IBrowserRequest
interface. It might make sense to split those concerns off into
different packages and then straighten out the dependencies. At that
point I can see having a setDefaultSkin method inside zope.skinnable
with a different behavior. But the one inside zope.publisher ought to
play nicely with IBrowserRequest.

Hanno