Re: [Zope-dev] z3c.formwidget.query - incorrect interface implementation?
Martin Aspeli wrote: I'm trying to build a widget that allows for auto-complete of items in a vocabulary, but also allows additional (string) values to be added. To understand how that works, I am digging into z3c.formwidget.query, and it looks to me like it's making incorrect assumptions about its own interfaces. This can very well be; the defense would be that the entire vocabulary code is full of odd interfaces that don't really form a complete story. If I'm not reading this wrong, it seems to me that z3c.formwidget.query is using getTermByValue() (which I can't find anywhere else) instead of getTermByToken() as defined by IVocabularyTokenized. I don't remember the details, but the implementation may be incorrect although functional. Certainly, it's made to good use by ``plone.app.z3cform`` and a number of other modules. You're more than welcome to improve or devise an alternative implementation. \malthe ___ Zope-Dev maillist - Zope-Dev@zope.org http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] z3c.formwidget.query - incorrect interface implementation?
Malthe Borch wrote: Martin Aspeli wrote: I'm trying to build a widget that allows for auto-complete of items in a vocabulary, but also allows additional (string) values to be added. To understand how that works, I am digging into z3c.formwidget.query, and it looks to me like it's making incorrect assumptions about its own interfaces. This can very well be; the defense would be that the entire vocabulary code is full of odd interfaces that don't really form a complete story. For what it's worth, I agree. :-) If I'm not reading this wrong, it seems to me that z3c.formwidget.query is using getTermByValue() (which I can't find anywhere else) instead of getTermByToken() as defined by IVocabularyTokenized. I don't remember the details, but the implementation may be incorrect although functional. Certainly, it's made to good use by ``plone.app.z3cform`` and a number of other modules. You're more than welcome to improve or devise an alternative implementation. Have you ever tested it with a source other than the one in queryselect's example code and the one in plone.app.z3cform.queryselect? Those are the only places I can see getTermByValue(). The fix would probably be to make sure it uses tokens everywhere and implement getTermByToken. Martin -- Author of `Professional Plone Development`, a book for developers who want to work with Plone. See http://martinaspeli.net/plone-book ___ Zope-Dev maillist - Zope-Dev@zope.org http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )
Re: [Zope-dev] z3c.formwidget.query - incorrect interface implementation?
On Sun, Aug 24, 2008 at 07:30:24PM +0100, Martin Aspeli wrote: I'm trying to build a widget that allows for auto-complete of items in a vocabulary, but also allows additional (string) values to be added. To understand how that works, I am digging into z3c.formwidget.query, and it looks to me like it's making incorrect assumptions about its own interfaces. snip class IBaseVocabulary(ISource): def getTerm(value): Return the ITerm object for the term 'value'. If 'value' is not a valid term, this method raises LookupError. snip class IVocabularyTokenized(IVocabulary): def getTermByToken(token): Return an ITokenizedTerm for the passed-in token. If `token` is not represented in the vocabulary, `LookupError` is raised. snip However, more importantly, in the README.txt of z3c.formwidget.query we have this: ... def getTermByValue(self, value): ... return self.vocabulary.by_value[value] and similarly, in widget.py: map(terms.add, map(source.getTermByValue, filter(lambda value: value and value not in values, selection))) If I'm not reading this wrong, it seems to me that z3c.formwidget.query is using getTermByValue() (which I can't find anywhere else) instead of getTermByToken() as defined by IVocabularyTokenized. That does look like a bug. However, without looking very deeply at the code,the fix would be to use getTerm, not getTermByToken. I wouldn't mind seeing getTerm renamed to getTermByValue, but that's probably difficult to do without breaking backwards-compatibility. Marius Gedminas -- I used to think I was indecisive, but now I'm not so sure. signature.asc Description: Digital signature ___ Zope-Dev maillist - Zope-Dev@zope.org http://mail.zope.org/mailman/listinfo/zope-dev ** No cross posts or HTML encoding! ** (Related lists - http://mail.zope.org/mailman/listinfo/zope-announce http://mail.zope.org/mailman/listinfo/zope )