Re: [Zope-dev] z3c.formwidget.query - incorrect interface implementation?

2008-08-24 Thread Malthe Borch
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?

2008-08-24 Thread Martin Aspeli
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?

2008-08-24 Thread Marius Gedminas
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 )