[Bug 33546] Refactore and cleanup TplEvent/TplEventText

[bugzilla-daemon at freedesktop.org]

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.