Summary: | use TpBaseProtocol | ||
---|---|---|---|
Product: | Telepathy | Reporter: | Simon McVittie <smcv> |
Component: | haze | Assignee: | Simon McVittie <smcv> |
Status: | RESOLVED FIXED | QA Contact: | Telepathy bugs list <telepathy-bugs> |
Severity: | normal | ||
Priority: | medium | Keywords: | patch |
Version: | git master | ||
Hardware: | Other | ||
OS: | All | ||
URL: | http://git.collabora.co.uk/?p=user/smcv/haze-smcv.git;a=shortlog;h=refs/heads/protocol | ||
Whiteboard: | review+ | ||
i915 platform: | i915 features: | ||
Bug Depends on: | 27997, 29076, 29079 | ||
Bug Blocks: |
Description
Simon McVittie
2010-07-15 08:26:46 UTC
+ /* FIXME: is it safe to pass a NULL account to prpl_info->account for all + * prpls? If it is, we could do that to be more likely to normalize right */ + return g_strdup (purple_normalize (NULL, contact)); You mean ->normalize, not ->account. And, it seems to be, except for Zephyr (which will critical). A bunch of the other normalization functions have comments suggesting that it's not accidental that they work with account == NULL. :) So I think we should do that; or, make purple_normalize() do that. Your call. This stuff with haze_foo_manager_append_channel_classes, all with identical implementations, suggests that that should live in tp-glib. Not clear exactly how, though... (In reply to comment #1) > + /* FIXME: is it safe to pass a NULL account to prpl_info->account for all > + * prpls? If it is, we could do that to be more likely to normalize right */ > + return g_strdup (purple_normalize (NULL, contact)); > > You mean ->normalize, not ->account. And, it seems to be, except for Zephyr > (which will critical). A bunch of the other normalization functions have > comments suggesting that it's not accidental that they work with account == > NULL. :) > > So I think we should do that; or, make purple_normalize() do that. Your call. purple_normalize() can't do that, because it doesn't know which prpl we're dealing with (it'd normally get that from @account). My vote would be to add: gchar * purple_plugin_protocol_info_normalize (PurplePluginProtocolInfo *prpl, const char *contact) { if (PURPLE_PROTOCOL_PLUGIN_HAS_FUNC (prpl, normalize)) { return prpl->normalize (NULL, contact); } else { return purple_normalize (NULL, contact); } } and fix the Zephyr plugin in the same Pidgin release. In the meantime, we could optionally do the same in Haze, and either assume that nobody uses Zephyr or insert some sort of hack. I think purple_normalize is good enough for now, though. > This stuff with haze_foo_manager_append_channel_classes, all with identical > implementations, suggests that that should live in tp-glib. Not clear exactly > how, though... Perhaps. The problem is that Protocol wants to enumerate all possible-to-implement channel classes statelessly, whereas Requests wants to enumerate all channel classes that will actually work, given a connected Connection, and the TpChannelManager API was designed for the latter. More precisely, what we want is: * HazeProtocol has a list of TpChannelManager *classes*: HazeConnection's create_channel_managers() will not create channel managers not chosen from that list * Each TpChannelManager has a new class-method (i.e. first parameter is TpChannelManagerClass *, not TpChannelManager *) that returns a list of all possible channel classes, which is a superset of the result of foreach_channel_class() I'll have a look at implementing that in tp-glib. (In reply to comment #2) > More precisely, what we want is: > > * HazeProtocol has a list of TpChannelManager *classes*: HazeConnection's > create_channel_managers() will not create channel managers not chosen from that > list > > * Each TpChannelManager has a new class-method (i.e. first parameter is > TpChannelManagerClass *, not TpChannelManager *) that returns a list of all > possible channel classes, which is a superset of the result of > foreach_channel_class() > > I'll have a look at implementing that in tp-glib. It turns out to be easier to use GTypes rather than classes. I did this (see Bug #27997), updated Haze to cope, and fixed the typo you noticed. I looked at adding that method to libpurple, but then I noticed that none of the functions defined in prpl.h actually take a PurplePluginProtocolInfo * as the first argument: they either take a PurpleAccount or a PurpleConnection. So adding this more-useful function would be inconsistent. Sigh. So I didn't. Ship the rest. Fixed in git for 0.5.0 |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.