[Bug 33546] Refactore and cleanup TplEvent/TplEventText
bugzilla-daemon at freedesktop.org
bugzilla-daemon at freedesktop.org
Mon Feb 7 19:42:44 CET 2011
https://bugs.freedesktop.org/show_bug.cgi?id=33546
--- Comment #5 from Emilio Pozuelo Monfort <pochu27 at gmail.com> 2011-02-07 10:42:44 PST ---
Looks good. I have a few comments though
> +/*
> * tpl_event_text_get_pending_msg_id
> - * @self: a #TplEventText
> + * @self: a #TplEvent
It's still a TplEventText. Also add a '_' to the function name.
> -_tpl_event_text_get_message_type (TplEventText * self)
> +tpl_event_text_get_message_type (TplEventText * self)
Not your fault, but while you're at it, make that '*self' (without the space).
_tpl_event_text_equal() did more checks than what tpl_event_equal_default()
does. Is that alright?
> + const gchar *room_id = _tpl_channel_text_get_chatroom_id (tpl_text);
> + receiver = _tpl_entity_new_from_room_id (room_id);
> + target_id = room_id;
You can just do:
target_id = _tpl_channel_...
receiver = _tpl_entity_... (target_id);
> + data = g_slice_new0(ReceivedData);
Add an space after _new0
> - TplEventText *self = TPL_EVENT_TEXT (object);
> + TplEventTextPriv *priv = TPL_EVENT_TEXT (object)->priv;
I'd rather you did self->priv->foo instead.
> + "(maybe NULL with some log store)",
s/maybe/may be/
s/store/stores/
> + /* FIXME: in text format it's not possible to guess who is the
> + * reveiver (unless we are in a room). In this case the receiver will
receiver
> + tp_proxy_get_object_path (TP_PROXY(account)),
missing space
> + /* Determin if it's a chatroom */
determine
> + log_id = _tpl_create_message_token (instead_of_channel_path,\
that backslash seems wrong
> - regex = g_regex_new ("^\\((.+)\\) (.+):", 0, 0, NULL);
> + regex = g_regex_new ("^\\((.+)\\) (.+): (.+)", 0, 0, NULL);
what's this about?
> - if (hits == NULL || g_strv_length (hits) < 3)
> + if (hits == NULL || g_strv_length (hits) < (4 + (guint)is_html))
Same here.
TBH I haven't closely looked at the pidgin log store changes...
--
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
More information about the telepathy-bugs
mailing list