Having a KTp::Contact

[David Edmundson]

On Thu, Jan 3, 2013 at 11:58 AM, Daniele E. Domenichelli
<daniele.domenichelli at gmail.com> 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
>> <david at davidedmundson.co.uk> 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
> KDE-Telepathy at kde.org
> https://mail.kde.org/mailman/listinfo/kde-telepathy