[Bug 27175] Make TpMessage usable in clients

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Mon Nov 8 19:55:24 CET 2010


https://bugs.freedesktop.org/show_bug.cgi?id=27175

--- Comment #14 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-11-08 10:55:20 PST ---
+  //TpCMMessage *self = TP_CM_MESSAGE (object);

Fixed later, but for reference: no C99 comments please. I'd prefer

    (void) self;

if you really must have a stub dispose().

I think message-mixin.h should probably #include <telepathy-glib/cm-message.h>,
which would mean echo-message-parts' chan.c wouldn't have to.

telepathy-glib.h should #include <telepathy-glib/signalled-message.h>.

Deprecated functions should be G_GNUC_DEPRECATED.

>  static void
> -tp_message_ref_handles (TpMessage *self,
> +tp_message_ref_handles (TpMessage *msg,

Not API, so it could be tp_cm_message_ref_handles if that'd be any easier.

> +  tp_cm_message_set_handle (self, 0, "message-sender", TP_HANDLE_TYPE_CONTACT,
> +      handle);

Assuming the relevant commit from Bug #26838 is merged (opinions welcome btw),
this method should add { "message-sender-id": inspected identifier } too.

> +  if (handle_or_0 != 0)
> +    tp_message_ref_handle (self, handle_type, handle_or_0);

This should call the non-deprecated one (tp_cm_message_ref_handle) if possible.

>   * #TpCMMessage represent a message send or received using the Message
>   * interface.

I'd prefer: #TpCMMessage is used within connection managers to represent a
message sent or received using the Messages interface.

> + * #TpClientMessage represent a message send using the Message interface.

I'd prefer: #TpClientMessage is used within Telepathy clients to represent a
message composed by a client, which it will send using the Messages interface.
Its subclass #TpSignalledMessage represents messages as signalled by a
connection manager.

> + * #TpSignalledMessage represent a message received using the Message interface.

#TpSignalledMessage is used within Telepathy clients to represent a message
signalled by a connection manager. This can either be a message received from
someone else, confirmation that a message has been sent by the local user, or a
delivery report indicating that delivery of a message has succeeded or failed.

> +          echo_msg = _tp_signalled_message_new (echo);

This seems like an abuse of TpSignalledMessage - this is within a CM, so the
refcounting will be all wrong.

> +gchar * tp_message_to_text (TpMessage *message,
> +    TpChannelTextMessageFlags *out_flags);

This should have G_GNUC_WARN_UNUSED_RESULT.

It seems as though it'd be cleaner to have TpSignalledMessage take the sending
TpContact as a construct-time parameter, rather than sellotaping it on
afterwards?

I don't think initial-parts should be a property: I think it's fair to say that
if you call g_object_new() or equivalent instead of using the
tp_foo_message_new "C binding", you get one part, and have to append to get
more (which is very easy, particularly in high-level languages).

I certainly don't think size-hint (which is a probably-premature optimization)
deserves to be a property.

> - make client-side received-message objects read-only? (we still need
> the set methods to work client-side while sending a message, though)

That'd mean having a "mutable" flag (property? class member?) which is true for
TpClientMessage and TpCMMessage but false for TpSignalledMessage.

-- 
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