On Thu, Jan 3, 2013 at 11:58 AM, Daniele E. Domenichelli <[email protected]> wrote: > Sorry for the very long delay in the reply... > > On 29/11/12 10:02, George Kiagiadakis wrote: >> On Thu, Nov 29, 2012 at 12:03 AM, David Edmundson >> <[email protected]> wrote: >>> However, it's quite a large change in direction I think, so I want >>> input from you guys. >> >> +1 from me, I would say. > > +1 from me too, it looks like a good idea. > > >>> - None of the methods in Tp::Contact are virtual, which means if we >>> were to override the block method to show the "are you sure" dialog >>> first (for example), you would get a different behaviour happen >>> depending on whether you had casted or not. >> >> That's the only real problem that I can see. We should be extremely >> careful to always cast objects to what they really are before using >> them. > > Do we really need to override methods? If we do, which ones? Can we have > those fixed in the next TpQt ABI break? > > >>> - Any code which tried to use KTp::Contact with a standard >>> Tp::ContactFactory would crash. >> >> Not very difficult to fix... > > Usually we do not get a KTp::Contact directly but Tp::ContactPtr > instead, so you are sure that you cannot call KTp::Contact methods from > that. > We should have a > > typedef Tp::SharedPtr<KTp::Contact> KTp::ContactPtr > We do :)
> so that if you cast a Tp::ContactPtr to a KTp::ContactPtr and check if > it isNull() (and if you don't do it after a cast, then you are probably > crazy), you are sure that you can use it. > So I don't see a big problem here. > The thing is it is safe to convert, as there can never be a Tp::Contact made with that ContactFactory. Any Tp::Contact* you find, will always be a KTp::Contact underneath without checking. I currently don't check when casting. So far the only issue has come from QVariant. If you have qvariantFromValue(someKTpContact) you _cannot_ do myValue.value<Tp::Contact>() (and vice-versa) I think we should enforce a rule that only one of Tp::ContctPtr or KTp::ContactPtr should have a Q_DECLARE_METATYPE. That way no code can get confused. > > > Just one more idea: in several places we need the Tp::Account together > with the Tp::Contact, (for example all the "actions" require both, and > the contact grid widget requires 2 methods to get the contact and the > account). > Does it make sense to have a method > > Tp::Account KTp::Contact::account() const > That would solve so many problems. Problem is it's not easy to do. I can't find a hook I could use outside Tp. I really don't want some sort of "setAccount" method on the connection that client side code calls because that's horrendous. I think the only way to fix that is inside TpQt, but even there I can't find a neat way to do it. Account::onConnectionBuilt maybe... but it's still not great. Dave > so that we can pass just the KTp::Contact everywhere and always be able > to access to the account? (Actually if I understand it correctly the > GlobalContactManager should allow that too, but I didn't have much time > to look at it yet, sorry) > > > Cheers, > Daniele > > _______________________________________________ > KDE-Telepathy mailing list > [email protected] > https://mail.kde.org/mailman/listinfo/kde-telepathy _______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
