[Code-review] review: Gadget alias branch

[Dafydd Harries]

Ar 14/08/2008 am 11:30, ysgrifennodd Guillaume Desmottes:
> Le mercredi 13 août 2008 à 19:03 +0100, Dafydd Harries a écrit :
> > >-    __slots__ = ('jid', 'properties', 'current_activity')
> > >+    __slots__ = ('jid', 'properties', 'current_activity', 'alias')
> > 
> > Having special cases for this worries me somewhat. I think ideally, activities
> > and buddies would both just be a{sv}s. Special cases add a lot of complexity.
> > Do you think it would be possible for Gadget to communicate aliases by
> > including them in the properties dict?
> 
> With current protocol, Gadget doesn't send the alias to user. Gabble's
> Gadget code doesn't interact with the alias iface. Anyway, PS already
> request alias for each new buddy (by looking at vcard) so it works as
> expected.
> 
> We could store the alias in a props dict of course, but then we should
> special case it when sending "real" properties to the user, so I don't
> think that's a smart move.
> 
> > >+    def buddy_view(self, size, properties, handler, alias=None):
> > >+        def test(buddy):
> > >+            if alias is not None and buddy.alias != alias:
> > >+                return False
> > 
> > I don't think exact string match is the right approach here. I should be able
> > to find you if I search for "Guillaume", even if your alias is actually
> > "Guillaume Desmottes". Doing a substring match seems like a reasonable
> > approach, though perhaps stripping combining characters would be even better.
> > 
> 
> Nice code. I included and used it.

This branch mostly looks great. Again, just one thing to change:

>+            alias = elem.getAttribute('alias')
>+
>+            q.append(('search', jid, props, alias))

We should insert a:

  if alias == '':
      alias = None

If the alias attribute is '', then it will match every buddy, which I don't
think is what we want.

-- 
Dafydd