From kfm-devel  Thu Jan 30 11:59:04 2014
From: Frank Reininghaus <frank78ac () googlemail ! com>
Date: Thu, 30 Jan 2014 11:59:04 +0000
To: kfm-devel
Subject: Re: Dolphin and Baloo
Message-Id: <CAFoZWWhiEJ5XSNp-RrVmuvOr4t5cTEEq0ezU8c=jWdsXUs+www () mail ! gmail ! com>
X-MARC-Message: https://marc.info/?l=kfm-devel&m=139108316229683

Hi,

2014-01-29 Vishesh Handa:
> On Wednesday 29 January 2014 17:59:45 Frank Reininghaus wrote:
[..]
>> 4. Could you point out why you commented out some code in
>> KFileItemModelRolesUpdater::applyChangedBalooRoles(const QString&
>> itemUrl), please?
>>
>
> I should have added a comment. We no longer have detailed information about
> what exactly changed so I have no way of doing data.insert(role, QVariant())
> because I do not know the variant.

OK, I see. Thanks for the explanation.

> I'm not sure how to fix this.

Hm, one (untested) solution could be to replace the commented code by

foreach (const QByteArray& role, m_roles) {
    if (rolesProvider.roles().contains(role)) {
        // Overwrite the the value of all Baloo roles with an empty
QVariant, because the roles
        // provider doesn't overwrite them when the property value
list is empty.
        // See bug 322348
        data.insert(role, QVariant());
    }
}

So the idea would be:

(a) First, clear the values of all Baloo roles.

(b) Then set the values of all Baloo roles to those that are obtained
from Baloo::File::properties().

This would ensure that no obsolete "Baloo role" values remain.

>> 5. About the FIXME comments in KFileItemModelRolesUpdater: I agree
>> that these things should be fixed because I don't like nested event
>> loops much. It should be quite straightforward to do, right? I don't
>> mind if it's fixed after merging the branch, but it should be done
>> before the next beta releases.
>
> I tried using a thread, moving the event loop to that thread and then blocking
> until the thread is finished. I was not successful. I'll run it by someone who
> understands threads better than I do.
>
> Unless you can think of a better way?

AFAIK, the usual way of dealing with a KJob is to start it and then
connect to its result() signal, isn't it?

So the code that handles the Baloo roles could be factored from

KFileItemModelRolesUpdater::applyChangedBalooRoles() and

KFileItemModelRolesUpdater::rolesData()

to a new slot

KFileItemModelRolesUpdater::slotBalooJobResult()

(or something like that). The two methods would then start the job and
connect its "result()" signal to the new slot.


Regards,
Frank