[Bug 27175] Make TpMessage usable in clients

[bugzilla-daemon at freedesktop.org]

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

--- Comment #15 from Guillaume Desmottes <guillaume.desmottes at collabora.co.uk> 2010-11-10 07:28:01 PST ---
(In reply to comment #14)
> 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.

done.

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

It already does.

> Deprecated functions should be G_GNUC_DEPRECATED.

done.

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

renamed.

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

done.

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

done (as part of the deprecation of the old API)

> >   * #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.

changed.

> > + * #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.

changed.

> > + * #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.

changed.

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

Done (using a new _tp_cm_message_new_from_parts())

> > +gchar * tp_message_to_text (TpMessage *message,
> > +    TpChannelTextMessageFlags *out_flags);
> 
> This should have G_GNUC_WARN_UNUSED_RESULT.

done.

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

done.

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

removed. tp_cm_message_new() now completely ignores size_hint. To make use of
it we should destroy parts and recrete the ptr array. Isn't that a bit
overkill for such trivial optimization? If not, we could remove size_hint
completely from tp_cm_message_new().

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

done.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.