On Thu, Jul 1, 2010 at 10:26 PM, Will Thompson <will.thomp...@collabora.co.uk> wrote: > Hello, > > I would like us to be able to implement this convenience API: > > class SimpleTextThingy { > public: > SimpleTextThingy(Tp::Contact &contact); > > /* Send a message to contact, optionally spawning/presenting the > * UI for the channel. */ > void sendMessage(Tp::Message &message, bool presentUI); > > /* Present a conversation window to contact, with a message ready > * to send. Think "share link with...": you want to give the user > * the chance to edit the message before sending it. */ > void presentMessageForSending(Tp::Message &message); > > public signals: > void messageSent(Tp::Message &message); > void messageReceived(Tp::Message &message); > } > > The signals are easy: just be an observer for Text channels to contact. The > methods are harder. Simon and I talked about them today. > > If we add a variation of the ChannelRequest.Succeeded signal which also > gives you the channel path, this lets us implement sendMessage("foo", true), > namely popping up the conversation window to that contact (by calling > EnsureChannel() and letting the default handler get it) and then poking a > message into the channel. Of course there's a race here: if the handler > Close()s the channel before you call SendMessage(), you lose. Hack: the > library could just try again, but that would probably piss the user off. > This will happen only rarely in practice, but it would still be unpleasant. > > sendMessage("foo", false); - that is, without presenting the normal UI > whether or not it's already open — is more problematic. If you use > ChannelDispatcher.CreateChannel() with yourself as the preferred handler, it > fails if the user has a conversation window open to that contact. If you use > CD.EnsureChannel(), it refocuses the conversation window if there is one. > Options, none of which I like: > > • We could add Connection.SendAMessage(). This is ugly, and would break > loggers logging outgoing messages. Vetoed. > • We could add send-only text channels, which you can always Create > regardless of whether there's another channel going on. No incoming messages > are reported by them; you can just call SendMessage(). MessageSent fires on > the send-only channel, as probably do delivery reports for the messages you > send on that channel. Incoming messages don't show up. > • We could allow more than one fully-functional text channel to a contact, > with MessageSent and MessageReceived firing on all of them. This involves > loggers de-duplicating messages, but they kind of have to do that anyway. > • Your really clever option here?
Andre pointed out on IRC that the Channel Dispatcher implementation is in a rather good spot to perhaps implement this without any CM modifications. In essence, CD would implement a method, say CD.I.Messages.Send(s:TargetID, s:message, <and perhaps some misc args for flags, and maybe throw an a{sv} there just for the kicks>), in the following way: - EnsureChannel a suitable text channel for sending the message to the recipient - Send the message on the resulting Channel - If sending failed, which in particular can happen at least when Yours wasn't true and somebody else Closed it when we tried to send, just request the channel again, and try sending again The resulting channels shouldn't be dispatched to any Handlers, but they should be dispatched to Observers for logging purposes. However, there's a problem: what if somebody makes a request to the CD which should return a channel we're currently using internally for sending a message? This can perhaps be solved: - Have CD always issue a EnsureChannel to the CM, if the requested ChannelType was Chan.T.Text (even if CreateChannel was used) - If that returned an object path we're not using internally, then all is fine: just dispatch to Observers, a Handler and and CR.SucceededWithChannel() normally - OTHERWISE: don't Succeed the ChannelRequest or dispatch the channel to Handlers (Observers have it already!) until we're done sending through that Channel, at which point dispatch it normally - though again, note that Observers have it already at this point - If nobody made a request matching a Text channel we were internally using while we were using it, just Close it to prevent leaking it This depends on a few things: - If it is safe to implement CreateChannel in the CD as Ensure + not Closing - the CD knows that it's the only other user of the Ensured channel (unless somebody else is directly poking the Connection, but that's broken anyway), so it could be the only one Closing it, but it can avoid that because it knows of the new request - If it is safe to Close the channel when we're done if nobody requested it in the meantime: It should be, as if we use Close instead of Destroy, if there were incoming messages the Channel should then be closed and redispatched, right? - If it is problematic that the Channel might be redispatched after Closing with its pending message queue intact, namely for loggers not wanting duplicate events? We haven't given the Channel to a Handler which could ack the messages beforehand, that is. Is there any other obvious race or misc problem I'm not seeing here? Also, do you think this would complicate the CD implementation too much (or be in fact impossible)? I'm fine with send-only Channels - they'd solve the issue equally well, and without any CD changes, but would in turn require changes in ALL of the CMs, and non-trivial tp-glib support, which looks problematic at this point to say the least. Not to mention the fact that "send-only" channels are an awkward concept at best. Then again, so is bolting a message sending method to the otherwise generic CD, just because it has some extra knowledge to make that slightly less distantly possible. > > presentMessageForSending is actually the easiest of the bunch: if we allow > associating hints with channel requests — > https://bugs.freedesktop.org/show_bug.cgi?id=28866 — then we can just define > a key which means "oi, present this message for sending". > > Thoughts very welcome, > -- > Will > _______________________________________________ > telepathy mailing list > telepathy@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/telepathy > -- Br, Olli Salli _______________________________________________ telepathy mailing list telepathy@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/telepathy