Hi Chris, Em Qui, 2006-11-16 às 07:33 +0000, Chris Withers escreveu:

Leonardo Rochael Almeida wrote:

...

@@ -615,8 +615,8 @@ def getobject(self, rid, REQUEST=None): """Return a cataloged object given a 'data_record_id_' """ - obj = self.aq_parent.unrestrictedTraverse(self.getpath(rid)) - if not obj: + obj = self.aq_parent.unrestrictedTraverse(self.getpath(rid), None) + if obj is None:

Please revert this change. You've changed the semantics of this statement and it will now mask errors in traversing to the path.

This is bad...

I agree with you the semantics have changed, but I argue that they've changed to what they were intended to be in the first place :-) This is the full method before my changes: def getobject(self, rid, REQUEST=None): """Return a cataloged object given a 'data_record_id_' """ obj = self.aq_parent.unrestrictedTraverse(self.getpath(rid)) if not obj: if REQUEST is None: REQUEST=self.REQUEST obj = self.resolve_url(self.getpath(rid), REQUEST) return obj Here it is, after my changes def getobject(self, rid, REQUEST=None): """Return a cataloged object given a 'data_record_id_' """ obj = self.aq_parent.unrestrictedTraverse(self.getpath(rid), None) if obj is None: if REQUEST is None: REQUEST=self.REQUEST obj = self.resolve_url(self.getpath(rid), REQUEST) return obj Traversing with a default argument will only mask traversal errors, not any other errors that might happen during traversing. If the intent was not to mask any errors at all, even the ones normal to traversal, then the "if not obj" that was there before would only be triggered in situations where there it was a bug: an object was found but it's got a .__len__ or .__nonzero__ method. In the version with my changes, but with the old unrestrictedTraverse call, the "if obj is None" would only ever be reached in the pathological situation where the attribute at the end of the path is None. Which means the ".resolve_url(self.getpath(rid), REQUEST)" would hardly ever be called at all. I'm open to reversing the unrestrictedTraverse call to the old semantics, as soon as we agree what those where, so I propose 2 options and I'll implement whatever this list decides: A) Keep my changes, but use a marker "object()" instead of None for the "default" parameter of the .unrestrictedTraverse() call. B) Reverse my changes, but also remove the "if not obj" block, which is buggy whichever way you look at it with the old unrestrictedTraverse call. Which one should it be? Cheers, Leo -- Leonardo Rochael Almeida Enfold Systems http://www.enfoldsystems.com phone. +1.713.942.2377 Ext 215 fax. +1.832.201.8856