D9712: balooctl status: produce parseable output

[Vishesh Handa]

vhanda added a comment.


  In https://phabricator.kde.org/D9712#191746, @ltoscano wrote:
  
  > May I observe that relying on the command line output is fragile in the 
best case?
  >  Unless the  command can produce the output in some formatted way (like the 
openstack CLI, which can enable a special mode to produce output in various 
formats like csv, table-formatted, or direct values, and select the columns).
  
  
  I agree. Having a special mode which gives json/csv or some other machine 
readable output is a far better option.

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D9712

To: michaelh, #frameworks, vhanda, smithjd
Cc: ltoscano, lbeltrame, elvisangelaccio, lueck, dfaure, ngraham

D8007: popplerextractor: don't try to guess the title if there isn't one.

[Vishesh Handa]

vhanda added a comment.


  To give some history - Strigi used to do this guessing, and when I 
implemented KFileMetaData, I decided to do the same. I don't have an opinion on 
whether it is a good idea or not.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D8007

To: flameeyes, #frameworks, mgallien
Cc: mgallien, vhanda, ngraham, #frameworks

D8007: popplerextractor: don't try to guess the title if there isn't one.

[Vishesh Handa]

vhanda added a comment.


  > @vhanda do you know if there is something to do to update Baloo database 
when metadata returned by KFileMetaData are changed even if the file itself did 
not change ?
  
  I'm starting to forget the code base and what all I implemented. But I don't 
think such a thing was implemented.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D8007

To: flameeyes, #frameworks, mgallien
Cc: mgallien, vhanda, ngraham, #frameworks

D8527: Display "Downloaded From" by default, if file supports it

[Vishesh Handa]

vhanda accepted this revision.
vhanda added a comment.
This revision is now accepted and ready to land.


  Looks good.
  
  Maybe we should wait and see if someone who works on Dolphin has an opinion. 
This widget is only used in Dolphin.

REPOSITORY
  R824 Baloo Widgets

BRANCH
  origin_url_386261

REVISION DETAIL
  https://phabricator.kde.org/D8527

To: ngraham, #dolphin, #frameworks, broulik, dfaure, markg, vhanda
Cc: vhanda, spoorun, navarromorales, firef, ngraham, andrebarros, emmanuelp

D8528: Consider DjVu files to be documents

[Vishesh Handa]

vhanda added a comment.


  Since I was added as a reviewer, I thought I'll comment.
  
  I am not currently maintaining Baloo or using it, so I don't want to really 
discuss the specifics. Though from a technical point of view this change will 
work. If nobody has any objections, I would say go for it.

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D8528

To: ngraham, vhanda, #frameworks, rkflx
Cc: rkflx, #frameworks

D4995: Fix DB inconsistency due to some docterms appearing with uppercase symbols

[Vishesh Handa]

vhanda accepted this revision.
vhanda added a comment.
This revision is now accepted and ready to land.


  This is awesome. Good work.
  
  Ship it! (If you don't have commit access, please ask for it, you can add me 
as a reference)
  
  (Optionally, one could even add a unit test)

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D4995

To: poboiko, pinakahuja, vhanda
Cc: #frameworks

D4911: add Baloo DBus signals for moved or removed files

[Vishesh Handa]

vhanda added a comment.


  I'm not the maintainer of Baloo any more, so I don't want to give it a clear 
Yes / No.
  
  This patch is going to be a big CPU hog. For files this will barely have an 
impact, but for folders of a large enough size, it's going to result in tons of 
dbus signals, and more importantly, tons of database lookups and full path 
constructions. It really will add up. In an earlier version of Baloo, we used 
to store the full file paths, which meant when a folder moved, every 
sub-file/folder's URL had to be updated. That was a huge CPU burner. This patch 
isn't that bad, but it's half way there.
  
  I cannot see what advantage these signals add, apart from possibly making 
Baloo more introspect-able. If it's to build applications on top of Baloo, I 
would recommend against it. The kernel provides file system monitoring APIs 
which should be used instead.

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D4911

To: mgallien, vhanda
Cc: apol, #frameworks

Re: Review Request 122183: [KUnitConversion] Currency: Fetch the currency file properly

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122183/
---

(Updated Feb. 26, 2017, 8:47 p.m.)


Status
--

This change has been discarded.


Review request for KDE Frameworks.


Bugs: 340819
https://bugs.kde.org/show_bug.cgi?id=340819


Repository: kunitconversion


Description
---

Currency: Fetch the currency file properly

Properly run an event loop and wait for the file to be fetched.

Also add a test to make sure currency conversion is working.

This patch also contains -
* https://git.reviewboard.kde.org/r/122182/
* https://git.reviewboard.kde.org/r/122181/
* https://git.reviewboard.kde.org/r/122180/

This is because reviewboard refuses to upload only a part of the diff. Please 
only look at currency.cpp w.r.t the EventLoop.


Diffs
-

  autotests/convertertest.h 8129a48 
  autotests/convertertest.cpp ae4298e 
  src/currency.cpp 715233c 

Diff: https://git.reviewboard.kde.org/r/122183/diff/


Testing
---

Test now passes.


Thanks,

Vishesh Handa

Re: Scrap Baloo Thread Feedback

[Vishesh Handa]

On Sun, Oct 16, 2016 at 2:16 PM, Christoph Cullmann <cullm...@absint.com> wrote:
> Hi,
>
> (evil top posting)
>
> given the silence, I assume any interest in baloo has stopped once more, or?
> Or are there any plans how to fixup the current situation?

I'm not going to be very involved with this.

I've already expressed my opinion. You all are free to make a decision.

--
Vishesh Handa

Re: Scrap Baloo Thread Feedback

[Vishesh Handa]

On Fri, Oct 7, 2016 at 8:08 PM, Christoph Cullmann  wrote:
>
> I did experiments and search works with tracker, but yes, a problem is 
> tagging,+
> which ATM doesn't work. Nor do I say that is a ready solution now, just a 
> possibility
> to avoid having to maintain low level code with at most 1 person (how it 
> looks ATM).
>
> And I don't propose to go that road now, but ATM I see nobody doing any other 
> experiments.
>
> Besides, tracker is constantly maintained and used since >> 5 years:
>
> https://github.com/GNOME/tracker/graphs/contributors

ok. Baloo clearly isn't being maintained.

>
>>
>>>
>>> => That is good that we agree, but I find it very astonishing that we use 
>>> baloo
>>> in its
>>> current state more or less mandatory on all that systems were it by design 
>>> will
>>> fail.
>>>
>>> (and it fails if you read the bugs)
>>>
>>
>> There is a certain amount of failure, but it's not "by-design". But
>> maybe I'm not seeing things clearly.
> You yourself stated that neither 32-bit issues nor NFS nor > 32-bit inodes 
> have any
> error handling. And that seems to have been known even during design and still
> we have this now as a framework per default used by any Plasma installation on
> systems exactly featuring that without error checking.
>
>>

>>
>> How about requirements such as resource consumption, ease of
>> integration, search speed are taken into consideration? Come on guys.
>> We're engineers over here.
>>
> What is the argument here? If you take a look at bugs.kde.org, you see 
> that
> people are complaining about all
> of that with baloo. I see no evidence nowhere that e.g. baloo is 
> "superior" to
> what GNOME uses
> or any other solution (perhaps beside nepomuk, ok...).
>>
>> What tests have been to obtain the evidence?
> What tests have been done to obtain the inverse evidence? I only hear here 
> the complaint
> about not taking requirements like resource consumption or speed into 
> account, but
> there is ATM zero evidence that e.g. tracker is slower.
>

I did do a lot of tests during the design of Baloo. I don't have hard numbers.

Even if I didn't, that doesn't mean a decision should be made without
gathering proper data.

> And the typical error check is:
>
> void MTimeDB::put(quint32 mtime, quint64 docId)
> {
> Q_ASSERT(mtime > 0);
> Q_ASSERT(docId > 0);
>
> MDB_val key;
> key.mv_size = sizeof(quint32);
> key.mv_data = static_cast();
>
> MDB_val val;
> val.mv_size = sizeof(quint64);
> val.mv_data = static_cast();
>
> int rc = mdb_put(m_txn, m_dbi, , , 0);
> Q_ASSERT_X(rc == 0, "MTimeDB::put", mdb_strerror(rc));
> }
>
> without any way to pass an error to the outside, nor any error handling code 
> at the outside,
> as no error can ever occur that is non-fatal.
>

ok. The API isn't exported. It can be changed.

But we both seem to have different opinions of how much work this would be.

>
> Besides I don't see any documentation of the DB format, but I could miss that.
> (at least not in the git nor https://community.kde.org/Baloo)
>

There isn't any.

>
> What would be highly appreciated would be a bit of documentation what the
> different pieces do and stuff like that, even if you have no time to code.
>

If you can send me specific questions about different parts I can
answer them. For "general documentation", I don't know where to start.
I usually much prefer just going through the code.


> Greetings
> Christoph
>

Re: Scrap Baloo Thread Feedback

[Vishesh Handa]

Hey

On Fri, Oct 7, 2016 at 6:34 PM, Christoph Cullmann <cullm...@absint.com> wrote:
> Hi,
>
>> On Fri, Oct 7, 2016 at 5:58 PM, Christoph Cullmann <cullm...@absint.com> 
>> wrote:
>>>
>
> 1) No handling of DB errors beside asserting
> 2) No handling of errors in the extractors (e.g. see the fixes I did, all 
> extractors will need more of that)
> 3) No handling of NFS/large inodes/inconsistencies => crash
>
> In the end, in my opinion, you can rewrite close to all parts dealing with 
> the DB or
> any other thing internally. If ever any thing gots inconsistent, ATM you are 
> doomed, forever,
> if not by luck my new startup code deletes the index, then you live again 
> until it is reindexed.
>
>>
> I am not sure, I am all for removing complete indexing and use a other indexer
> like tracker to exactly avoid the excurse into DB world and how to handle it
> in a safe way with close to zero person manpower.
>

It's avoiding the problem and hoping for the best, without any experiments.

>
> => That is good that we agree, but I find it very astonishing that we use 
> baloo in its
> current state more or less mandatory on all that systems were it by design 
> will fail.
>
> (and it fails if you read the bugs)
>

There is a certain amount of failure, but it's not "by-design". But
maybe I'm not seeing things clearly.

>>
>>>>
>>>> How about requirements such as resource consumption, ease of
>>>> integration, search speed are taken into consideration? Come on guys.
>>>> We're engineers over here.

>>> What is the argument here? If you take a look at bugs.kde.org, you see that
>>> people are complaining about all
>>> of that with baloo. I see no evidence nowhere that e.g. baloo is "superior" 
>>> to
>>> what GNOME uses
>>> or any other solution (perhaps beside nepomuk, ok...).

What tests have been to obtain the evidence?

>
>>
>> Yup, you have. It's awesome. I no longer have the motivation to work on 
>> Baloo.
> Thanks, but that makes me very sad, btw.
> Baloo came up to replace nepomuk, which was dead because it had too many bugs 
> and all maintainers left.
> Now we have baloo, which has many bugs, some even by design, and the 
> maintainer left, too.
>

Actually, Nepomuk was not dead. I was maintaining it. I killed it
because it had too many structural problems.

This is how the open source world works. People work on projects and
when it no longer scratches their itch (I no longer use Baloo), they
loose interest. This is "supposed" to be a hobby.

>
>> (This is why they run on a separate process)
> That doesn't help, it just OOMs your system => dead, it needs resource 
> restrictions,
> which is tricky to get right.
>

You're right. It needs a better thought out solution. A separate
process is the bare minimum.

Btw, have you looked if Tracker actually does any of this?

>> My hostility was because the proposal ignores key points such as -
>>
>> * Indexing Speed
>> * Search speed
>> * Database size
> => If you look at the bugs, people complain we are inferior and I see not
> that the proposal ignores it, I just see not how to compare, given there are 
> no
> hard facts that we are faster than e.g. tracker in any way.
>

Data can be gathered about it. Not all data is publicly available.

>> * Ease of use with our existing components
> My proposal did not change the interface at all, it has zero impact on "ease 
> of use".
>
>> * Ease of fixing problems in the code
> My estimate would be: rewrite close to everything. Even the basic 64-bit int 
> id won't work
> with 64-bit inodes, each DB call must be touched to check for errors, at each 
> place
> one will need to check for potential inconsistencies and exit gracefully...
>

I don't follow why everything needs to be re-written? Am I missing
something or do we just need to check for more errors and use a higher
integer id? This certainly doesn't seem super trivial, but it sounds
like less work than implementing a shim on top of Tracker.

I could be wrong.

>>
>> Baloo has certain speed requirements if it is to be used with krunner,
>> and we want instant feedback. This was an integral requirement.
> I doubt e.g. tracker has different requirements, as it is used in similar 
> places by GNOME.
>
> But all that left besides, have you an proposal how to fixup the current 
> situation?
> Are you willing to invest some work to fix the current issues or an idea what
> would be a good way to tackle them?
>

I probably will not work more in Baloo.

I'll have to investigate the problems a bit more. From the cursory
look of this thread, it doesn't seem that the problems are that dire.
But I may not be reading into it correctly.


--
Vishesh Handa

Re: Scrap Baloo Thread Feedback

[Vishesh Handa]

On Fri, Oct 7, 2016 at 6:20 PM, Aleix Pol <aleix...@kde.org> wrote:
>>>
>>
>> I don't understand why all framework discussions must happen on the
>> same list. It just adds to a crazy amount of noise, which one then
>> needs to parse through.
>
> Arguing that it should be elsewhere because you'd like to ignore the
> rest of the traffic in kde-frameworks doesn't sound very constructive,
> especially considering how they're the "noise" that actually improves
> the frameworks.
>
> Maybe you can better configure your e-mail client differently so we
> can focus on the issue at matter?

This is not about how it should be. I'm informing them why it was
chosen to be somewhere else. This decision can be changed.

Frameworks collectively may or may not improve by having everything in
one place. Lets not treat it as a axiom.

An analogy could be that we get commit emails, but we get to choose
which projects we are interested in. We don't make everyone subscribe
to kde-commits, and then put their own complex filters on top.

--
Vishesh Handa

Re: Scrap Baloo Thread Feedback

[Vishesh Handa]

On Fri, Oct 7, 2016 at 6:14 PM, Christoph Cullmann <cullm...@absint.com> wrote:
>>>
>>
>> I don't understand why all framework discussions must happen on the
>> same list. It just adds to a crazy amount of noise, which one then
>> needs to parse through.
>
> If you would have baloo-devel I could understand that point,
> but not with some other generic mailing list like kde-devel which
> has the same amount of noise and is not even dedicated to 'frameworks'
> or 'baloo'.

If you guys plans to use frameworks devel, then please change the
review requests.

It was just too much noise for me, and I found the noise/signal ratio
way lower in kde-devel. Baloo-devel was specifically not chosen as it
would just an another silo in kde. Nepomuk used to suffer from that.

--
Vishesh Handa

Re: Scrap Baloo Thread Feedback

[Vishesh Handa]

On Fri, Oct 7, 2016 at 6:14 PM, Christoph Cullmann <cullm...@absint.com> wrote:
>>>
>>
>> I don't understand why all framework discussions must happen on the
>> same list. It just adds to a crazy amount of noise, which one then
>> needs to parse through.
>
> If you would have baloo-devel I could understand that point,
> but not with some other generic mailing list like kde-devel which
> has the same amount of noise and is not even dedicated to 'frameworks'
> or 'baloo'.

If you guys plans to use frameworks devel, then please change the
review requests.

It was just too much noise for me, and I found the noise/signal ratio
way lower in kde-devel. Baloo-devel was specifically not chosen as it
would just an another silo in kde. Nepomuk used to suffer from that.

--
Vishesh Handa

Re: Scrap Baloo Thread Feedback

[Vishesh Handa]

On Fri, Oct 7, 2016 at 5:57 PM, Kevin Funk <kf...@kde.org> wrote:
> On Friday, 7 October 2016 17:24:26 CEST Vishesh Handa wrote:
>> Hey guys
>>
>> I was told there is a thread about scrapping Baloo. All Baloo
>> discussion used to happen on kde-devel and that's where the review
>> requests go. It's the only reason I am still subscribed to kde-devel.
>
> Heya,
>
> Baloo is a framework nowadays, therefore it totally makes sense to have the
> discussion on kde-framework-devel.
>
> There's been tons of discussion around Baloo on kde-framework-devel already.
> kde-frameworks-devel is also where the CI messages for the baloo repo go to.
>
> It likely makes sense for you to subscribe, no?
>

I don't understand why all framework discussions must happen on the
same list. It just adds to a crazy amount of noise, which one then
needs to parse through.

> Cheers,
> Kevin
>
>> (snip)
>
>
> --
> Kevin Funk | kf...@kde.org | http://kfunk.org

Scrap Baloo Thread Feedback

[Vishesh Handa]

Hey guys

I was told there is a thread about scrapping Baloo. All Baloo
discussion used to happen on kde-devel and that's where the review
requests go. It's the only reason I am still subscribed to kde-devel.

I must say, the thread is overall quite disappointing. There seems to
be no scientific or rationale cost based analysis of this. How about a
list of requirements and priorities are drawn up and then possible
solutions are evaluated according to it?

Right now, random requirements such as NFS and 32bit systems are
coming up. Are these really that important? I specifically designed
Baloo to not care about both network mounts and 32-bit systems. Yes,
Baloo has bugs and it won't handle more than 32bit-inodes. These
things, as all others, can be fixed. It's really a question of what is
important. Lets not target the outliers. Many of these decisions were
deliberately taken.

How about requirements such as resource consumption, ease of
integration, search speed are taken into consideration? Come on guys.
We're engineers over here.

(If the discussion continues on kde-frameworks-devel, I probably won't see it)

--
Vishesh Handa

Re: baloo metadata scrapers

[Vishesh Handa]

On Oct 7, 2016 12:07, "Martin Bednar" <seraf...@gmail.com> wrote:
>
> Hi,
>
> thanks for answering.
>
> On Friday, 7 October 2016 10:44:02 CEST Vishesh Handa wrote:
> > Hey Martin
> >
> > Baloo is only designed for searching for files. It is not a meta-data
> > store. If the data you're extracting is present in the file, then it
> > could potentially be added to Baloo.
>
> It is not, I get that data from an external source (currently themoviedb).
>
> > The important thing to look at is how to present the data, and how
> > does the data help in searching. For example - providing the name of
> > an actor in a movie could make the movie popup when searching for that
> > name. This is great, however, the user needs to be given a clear
> > explanation of why that file is in the search results.
>
> Agreed, the dream actually is creating something along the lines of the
> multimedia library described on the nepomukport page[1] for potential use
in
> Plasma Media Center.
>
> I found https://community.kde.org/Baloo/Architecture since my email, and
am
> now thinking of creating a new data/search store plugin.
>

Ignore this.

Actually ignore all docs you find online on any wiki. It is too old. The
git repo is the only thing that is valid.

> > Our main search interface, krunner, isn't designed to provide this
> > additional context. Therefore, adding actor information, is not
> > something I would want to ship.
>
> Understood.
>
> I'd like to ask for a few pointers though :
> Does documentation on how to write new search/store plugins exist?
> From what I understood, I'll have to take care of storing the data
myself. I
> see that baloo uses LMDB, Xapian and SQLite. Any pointers as to which db
> system (or a completely different one) is well adapted to this kind of
use? At
> the moment I tend to lean towards LMDB, but have no real technical
argument
> for it...

Basically I would recommend looking at this from the presentation and
usability side first. Technology later.

Do you want some kind of kioslave? Through Krunner? Something else? What
kind of workflow and what kind of users.

Once this is more clear, you can figure out which is the best way to go
about what you want.

>
> Thanks,
>
> Martin
>
> [1] https://community.kde.org/Baloo/NepomukPort#Bangarang

Re: baloo metadata scrapers

[Vishesh Handa]

Hey Martin

Baloo is only designed for searching for files. It is not a meta-data
store. If the data you're extracting is present in the file, then it
could potentially be added to Baloo.

The important thing to look at is how to present the data, and how
does the data help in searching. For example - providing the name of
an actor in a movie could make the movie popup when searching for that
name. This is great, however, the user needs to be given a clear
explanation of why that file is in the search results.

Our main search interface, krunner, isn't designed to provide this
additional context. Therefore, adding actor information, is not
something I would want to ship.

--
Vishesh Handa


On Fri, Sep 30, 2016 at 9:47 PM, Martin Bednar <seraf...@gmail.com> wrote:
> Hi,
>
> having very fond memories of the potential I saw in the tvshows://
> experimental kioslave (from the days of nepomuk), so I was left wondering
> whether something like that would be possible with baloo.
>
> I currently have a working scraper (currently for movies, tvshows should be
> easy to tack on) and am wondering if there is a way to use baloo to store the
> metadata. I read a bit about baloo on api.kde.org, but am not able to find any
> indication how (if) such a project would be feasible.
>
> Thanks,
>
> Martin.
>
>
> --
> Why top posting?
> Why are forums read from top to bottom?

Re: Baloo maintainer required

[Vishesh Handa]

On Mon, Sep 19, 2016 at 1:11 PM, Boudhayan Gupta <m...@baloneygeek.com> wrote:
> Hi,
>
> On 19 Sep 2016 4:34 p.m., "Vishesh Handa" <m...@vhanda.in> wrote:
>> >> If someone else wants to maintain it, I can help with the transition.
>
> I have a ton of questions; I'll send you a list sometime during the week?
>

Go ahead

>> Generally, it's polite the cc the previous maintainer. Or at least
>> inform them in some other manner.
>
> The assumption was that you're still subscribed to the mailing list (which
> you are, given that you posted here) and would have noticed. But true,
> should have poked you harder. My bad.
>

Not on kde-frameworks, which is where it was sent.

Anyway, thanks for stepping up.

--
Vishesh Handa

Re: Baloo maintainer required

[Vishesh Handa]

On Mon, Sep 19, 2016 at 11:43 AM, Bhushan Shah <bhus...@gmail.com> wrote:
> Hey Vishesh,
>
> On Mon, Sep 19, 2016 at 3:06 PM, Vishesh Handa <m...@vhanda.in> wrote:
>> I've been steadily loosing motivation to work on Baloo.
>>
>> I've removed myself from all the bugs, as it doesn't seem like I'm
>> going to look at them. I no longer use Baloo or have a KDE development
>> environment. Therefore, it's not even scratching my own itch.
>>
>> If someone else wants to maintain it, I can help with the transition.
>
> Few days ago Boudhayan volunteered for maintaining it..
>
> See 
> https://mail.kde.org/pipermail/kde-frameworks-devel/2016-September/037674.html
>

Thanks.

Generally, it's polite the cc the previous maintainer. Or at least
inform them in some other manner.

Oh well. At least it is being maintained.

> Thanks
>
> --
> Bhushan Shah
>
> http://blog.bshah.in
> IRC Nick : bshah on Freenode

Re: Review Request 128892: Open baloo lmdb database read-only beside in baloo_file/baloo_file_extractor + balooctl (for some commands) + unit tests

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128892/#review99115
---


Ship it!




This is awesome. Ship it.

- Vishesh Handa


On Sept. 11, 2016, 7:19 p.m., Christoph Cullmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128892/
> ---
> 
> (Updated Sept. 11, 2016, 7:19 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma, Boudhayan Gupta, and Vishesh Handa.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> At the moment, any application that uses baloo can corrupt the db.
> Now, only the things that need to write to it open it with read-write.
> This only works as long as the library exposes only read-only things like 
> Query/...
> 
> 
> Diffs
> -
> 
>   src/engine/database.h 6ccb2a5 
>   src/engine/database.cpp 6a433c7 
>   src/file/extractor/app.cpp 0ca7276 
>   src/lib/file.cpp cbbc912 
>   src/lib/searchstore.cpp 060a4fd 
>   src/lib/taglistjob.cpp 76ac8ff 
>   src/qml/experimental/monitor.cpp 11c06ae 
>   src/tools/balooctl/main.cpp 2a6b175 
>   src/tools/balooctl/statuscommand.cpp 1a56c64 
>   src/tools/balooshow/main.cpp f45f2e0 
> 
> Diff: https://git.reviewboard.kde.org/r/128892/diff/
> 
> 
> Testing
> ---
> 
> Unit tests still work, balooctl seems to do something.
> 
> 
> Thanks,
> 
> Christoph Cullmann
> 
>

Re: Review Request 128665: Nested tags for Baloo Widgets

[Vishesh Handa]

> On Aug. 19, 2016, 8:52 a.m., Vishesh Handa wrote:
> > Could you please attach some screenshots of the before and after?
> 
> James Smith wrote:
> I don't think it's really necessary, the existing indexed tag was erased 
> if a tag higher up was selected, or an expanded first-level indexed tag 
> became unexpanded when erasing an unsaved new tag beyond the last item of the 
> indexed tag.

I cannot visaulize how this looks by just reading at the code. Please add 
images. Otherwise I need to compile this code and try it out manually.


- Vishesh


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128665/#review98495
---


On Aug. 19, 2016, 12:05 a.m., James Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128665/
> ---
> 
> (Updated Aug. 19, 2016, 12:05 a.m.)
> 
> 
> Review request for Baloo.
> 
> 
> Bugs: 334615
> http://bugs.kde.org/show_bug.cgi?id=334615
> 
> 
> Repository: baloo-widgets
> 
> 
> Description
> ---
> 
> Info pane tags are shortened to their last element, with the full tag shown 
> in each tag's tooltip. The tag selector box allows for creating and selecting 
> nested tags.
> 
> 
> Diffs
> -
> 
>   src/kedittagsdialog.cpp 1778d53 
>   src/kedittagsdialog_p.h 3c4de19 
>   src/tagcheckbox.cpp 8273a7f 
> 
> Diff: https://git.reviewboard.kde.org/r/128665/diff/
> 
> 
> Testing
> ---
> 
> Compile, run
> 
> 
> Thanks,
> 
> James Smith
> 
>

Re: Review Request 128664: Nested tags for Baloo

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128664/#review98860
---



Please test out all possible workflows. Ideally it would be great if we could 
have automated tests for all the workflows with tags.


src/file/basicindexingjob.cpp (line 95)
<https://git.reviewboard.kde.org/r/128664/#comment66551>

Please add a test for this -

We have a basicindexingjobtest file where the tests were "temporarily" 
removed. They clearly need to be added again with the behaviour this class 
expects.



src/file/basicindexingjob.cpp (line 107)
<https://git.reviewboard.kde.org/r/128664/#comment66550>

Alright. So now TA instead of being a prefix to store each word, it now 
stores the entire tag.

This breaks existing behaviour. That's typically frowned upon. Example - 
All tags with more than 1 word.

Perhaps it would just be easier if we would intrduce a new prefix.



src/kioslaves/tags/kio_tags.cpp (line 91)
<https://git.reviewboard.kde.org/r/128664/#comment66552>

So if we only have tag in the form -

"Fire/Water"

at that point unless one also has the tag "Fire", it will just never show 
up.


- Vishesh Handa


On Aug. 20, 2016, 10:58 p.m., James Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128664/
> ---
> 
> (Updated Aug. 20, 2016, 10:58 p.m.)
> 
> 
> Review request for Baloo and Vishesh Handa.
> 
> 
> Bugs: 334615
> http://bugs.kde.org/show_bug.cgi?id=334615
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> Index and query each tag as a single full term for generating recursed search 
> results. Represent nested tags as recursed items in the Tags:// KIO Slave.
> 
> 
> Diffs
> -
> 
>   src/file/basicindexingjob.cpp 88bb59a01e5592abb74b1ab345bfc6765d35db57 
>   src/kioslaves/tags/kio_tags.cpp de2e6d71945632e23a85f831878b4c431360731c 
>   src/lib/searchstore.cpp 060a4fd795ab858eb84526f93f827d09ee85db7c 
> 
> Diff: https://git.reviewboard.kde.org/r/128664/diff/
> 
> 
> Testing
> ---
> 
> Compile, run
> 
> 
> Thanks,
> 
> James Smith
> 
>

Re: Review Request 128738: Quote all prefix search values, if operator is = or :.

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128738/#review98859
---



This breaks the default behavior of searching and makes it hard to reason what 
is going on.

Eg - `baloosearch 'tag:Death Fire'` and `baloosearch tag:Death Fire` now yield 
different results since "Fire" is considered part of the tag in the first query.

I would rather not ship this.

- Vishesh Handa


On Aug. 24, 2016, 3:26 p.m., James Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128738/
> ---
> 
> (Updated Aug. 24, 2016, 3:26 p.m.)
> 
> 
> Review request for Baloo and Vishesh Handa.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> Add quotation marks surrounding the search term for all searches matching 
> operator = or :.
> 
> 
> Diffs
> -
> 
>   src/tools/baloosearch/main.cpp 8fabdbb 
> 
> Diff: https://git.reviewboard.kde.org/r/128738/diff/
> 
> 
> Testing
> ---
> 
> Compile, run.
> 
> 
> Thanks,
> 
> James Smith
> 
>

Re: Review Request 128738: Quote all prefix search values, if operator is = or :.

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128738/#review98765
---



Hey James. I'm a little busy right now, but I will have time post Wednesday to 
actually try all your patches out.

- Vishesh Handa


On Aug. 24, 2016, 3:26 p.m., James Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128738/
> ---
> 
> (Updated Aug. 24, 2016, 3:26 p.m.)
> 
> 
> Review request for Baloo and Vishesh Handa.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> Add quotation marks surrounding the search term for all searches matching 
> operator = or :.
> 
> 
> Diffs
> -
> 
>   src/tools/baloosearch/main.cpp 8fabdbb 
> 
> Diff: https://git.reviewboard.kde.org/r/128738/diff/
> 
> 
> Testing
> ---
> 
> Compile, run.
> 
> 
> Thanks,
> 
> James Smith
> 
>

Re: Review Request 128665: Nested tags for Baloo Widgets

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128665/#review98495
---



Could you please attach some screenshots of the before and after?

- Vishesh Handa


On Aug. 19, 2016, 12:05 a.m., James Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128665/
> ---
> 
> (Updated Aug. 19, 2016, 12:05 a.m.)
> 
> 
> Review request for Baloo.
> 
> 
> Bugs: 334615
> http://bugs.kde.org/show_bug.cgi?id=334615
> 
> 
> Repository: baloo-widgets
> 
> 
> Description
> ---
> 
> Info pane tags are shortened to their last element, with the full tag shown 
> in each tag's tooltip. The tag selector box allows for creating and selecting 
> nested tags.
> 
> 
> Diffs
> -
> 
>   src/kedittagsdialog.cpp 1778d53 
>   src/kedittagsdialog_p.h 3c4de19 
>   src/tagcheckbox.cpp 8273a7f 
> 
> Diff: https://git.reviewboard.kde.org/r/128665/diff/
> 
> 
> Testing
> ---
> 
> Compile, run
> 
> 
> Thanks,
> 
> James Smith
> 
>

Re: Review Request 128664: Nested tags for Baloo

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128664/#review98494
---



This is quite an interesting change.

Could you please also elaborate on how this will change the tags kioslave? I'm 
afraid I don't quite follow.


src/kioslaves/tags/kio_tags.cpp (line 103)
<https://git.reviewboard.kde.org/r/128664/#comment66370>

QString::compare returns 0 on match or < 0 and > 0 depending on which term 
is larger. I don't quite understand what is going on over here.



src/kioslaves/tags/kio_tags.cpp (line 395)
<https://git.reviewboard.kde.org/r/128664/#comment66369>

This code is getting hard to follow. Do you think you could write a simple 
unit test for just this function?

That way it will be clear what inputs it can accept.

Also I think this function can be made `const`.



src/lib/searchstore.cpp (line 58)
<https://git.reviewboard.kde.org/r/128664/#comment66367>

I think this line along with the next can now be removed.



src/lib/searchstore.cpp (line 270)
<https://git.reviewboard.kde.org/r/128664/#comment66366>

Please check for both "tag" and "tags".



src/lib/searchstore.cpp (line 276)
<https://git.reviewboard.kde.org/r/128664/#comment66368>

Could you please explain what's going on over here? I don't understand it.

From a cursory glance it seems that this code will assert. `q` is of type 
`EngineQuery::Phrase` and is a leaf query so `Transaction::PostingIterator` 
will assert it `Q_ASSERT(0)`


- Vishesh Handa


On Aug. 18, 2016, 11:56 p.m., James Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128664/
> ---
> 
> (Updated Aug. 18, 2016, 11:56 p.m.)
> 
> 
> Review request for Baloo and Vishesh Handa.
> 
> 
> Bugs: 334615
> http://bugs.kde.org/show_bug.cgi?id=334615
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> Index and query each tag as a single full term for generating recursed search 
> results. Represent nested tags as recursed items in the Tags:// KIO Slave.
> 
> 
> Diffs
> -
> 
>   src/file/basicindexingjob.cpp 88bb59a01e5592abb74b1ab345bfc6765d35db57 
>   src/kioslaves/tags/kio_tags.cpp de2e6d71945632e23a85f831878b4c431360731c 
>   src/lib/searchstore.cpp 060a4fd795ab858eb84526f93f827d09ee85db7c 
> 
> Diff: https://git.reviewboard.kde.org/r/128664/diff/
> 
> 
> Testing
> ---
> 
> Compile, run
> 
> 
> Thanks,
> 
> James Smith
> 
>

Re: Review Request 128554: Check for xattr during config step, otherwise the build might fail (if xattr.h is missing). Missing xattr should now trigger an error message to prompt the user into install

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128554/#review97999
---


Ship it!




Ship It!

- Vishesh Handa


On July 29, 2016, 5:50 p.m., Johan Ouwerkerk wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128554/
> ---
> 
> (Updated July 29, 2016, 5:50 p.m.)
> 
> 
> Review request for Baloo.
> 
> 
> Repository: kfilemetadata
> 
> 
> Description
> ---
> 
> Check for xattr during config step, otherwise the build might fail (if 
> xattr.h is missing). Missing xattr should now trigger an error message to 
> prompt the user into installing libattr + development packages.
> 
> CMake logic is based on: 
> https://github.com/rpm-software-management/librepo/blob/master/cmake/Modules/FindXattr.cmake
> Taking some cues from an older KDE review request that Googling turned up: 
> https://git.reviewboard.kde.org/r/115877/
> 
> Note: the rationale for this change is purely to 'document'/warn about the 
> previously hidden dependency on xattr when building from source. Currently 
> this is a hard dependency, compilation simply errors out if xattr headers 
> aren't available.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 4ec8eebe54fa8220c30930efffd8e76fd5eb0695 
>   cmake/FindXattr.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/128554/diff/
> 
> 
> Testing
> ---
> 
> Without xattr development headers cmake now complains when building with 
> kdesrc-build.
> With xattr development headers installed, cmake & compilation steps pass with 
> kdesrc-build.
> 
> 
> Thanks,
> 
> Johan Ouwerkerk
> 
>

Re: [kde-community] Criteria for PlanetKDE Blogs

[Vishesh Handa]

On Thu, Jul 14, 2016 at 9:13 AM, Ben Cooksley <bcooks...@kde.org> wrote:
> In order for planet to pick up your posts within a reasonable time
> frame, Planet must poll your blog on a regular basis - currently once
> every 30 minutes. A listing of every failure or change in URL is sent
> to Sysadmin on the completion of each run. As such a broken blog can
> cause some problems.

What kind of problems?

I'm guessing it would just be ignored and eventually over time we can
remove ones which have been broken for a very long time.

--
Vishesh Handa

Re: Review Request 128189: DocumentUrlDB::del Only assert when children of dir actually exist

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128189/#review96569
---


Ship it!




It's so strange that no matter what we still keep getting corrupted data.

If possible, it would be nice if you could save cola's baloo db and try to 
diagnose exactly what got corrupted. Or if it is small enough we can save it 
(after removing personal data) and use it as a test case.

- Vishesh Handa


On June 14, 2016, 8:32 p.m., Pinak Ahuja wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128189/
> ---
> 
> (Updated June 14, 2016, 8:32 p.m.)
> 
> 
> Review request for Baloo.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> We can have cases when deleting a directory from DocumentUrlDB when we have 
> some children stored under it which actually don't exist on the file system. 
> This can happen when the dir is being removed from DB because it doesn't 
> exist on the filesystem.
> 
> We do need to work on cleaning up the DB when we encounter such behaviour or 
> even better figure out why it is happening and preventing it from happening.
> 
> 
> Diffs
> -
> 
>   src/engine/documenturldb.h 9bd5bb5 
> 
> Diff: https://git.reviewboard.kde.org/r/128189/diff/
> 
> 
> Testing
> ---
> 
> Fixed crash on CoLa's laptop.
> 
> 
> Thanks,
> 
> Pinak Ahuja
> 
>

Re: Find out whether a file is a remote file or not

[Vishesh Handa]

On Sun, May 1, 2016 at 6:28 PM, Dominik Haumann <dhaum...@kde.org> wrote:
>
> CC: Vishesh, since maybe baloo also had to solve these issues

We mostly just use Solid to query the list of mount points and ignore
non local stuff.

baloo/src/file/storagedevices.h

--
Vishesh Handa
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Could we enable Travis-CI on our github mirrors?

[Vishesh Handa]

On Thu, Apr 21, 2016 at 3:05 AM, Ben Cooksley <bcooks...@kde.org> wrote:
>>>
>>> Also, another -1 from me is because I don't think we should use GitHub
>>> more than a mere mirror.
>>
>>
>> It would still remain a mere mirror imho, i.e. strictly read-only. Btw do we
>> have some written rule about our github presence somewhere? It seems to me
>> that some KDE project uses github as their main development platform.
>
> Urls to those projects which "appear" to be using Github as their
> primary development platform would be appreciated.
>

I most certainly *always* link to code on github and to the documentation.

> Use of Github for anything other than mirror purposes, especially if
> it is being pushed as the primary means of making contributions is
> strictly prohibited under the Manifesto.

Except it isn't. Please point me to where it does.

> The projects included above
> can expect to hear from Sysadmin and will be requested to provide an
> explanation of their activities there to both us and
> kde-commun...@kde.org.
>

Please provide more info about why this would be required.

--
Vishesh Handa

Re: Please add new versions on bugs.kde.org products on KF5 releases

[Vishesh Handa]

On Sat, Apr 9, 2016 at 12:09 PM, David Faure <fa...@kde.org> wrote:
> One following issue remains:
> there is no frameworks-krunner bugzilla product. There is a krunner product, 
> but it's not the same
> thing, it's not KF5-versionned. Should we add one? Who would be the 
> maintainer?
> (the yaml file points to Vishesh).

Whatever is easier.

I am the maintainer, but I haven't been doing anything, so someone
else should take over. I'll write an email announcing that soon.

--
Vishesh Handa
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Product versions on bugs.kde.org

[Vishesh Handa]

On Sun, Mar 13, 2016 at 7:57 PM, David Faure <fa...@kde.org> wrote:
> On Saturday 12 March 2016 20:38:43 Vishesh Handa wrote:
>>
>> And from my point of view the user doesn't care if Baloo is part of
>> something called "Frameworks". There is a bug, they want to report it.
>> It's just added noise.
>
> Typically, users report bugs in the app where they see them, and then
> the maintainer of the app reassigns to the "guilty" underlying framework.
>
> E.g. end users don't report kio bugs, they report dolphin bugs, which then
> get reassigned to frameworks-kio.
>
> So I think frameworks-baloo makes sense (and is consistent). The "users"
> of the framework are application developers, who know how to find it in 
> bugzilla.

Except that Baloo isn't just a library. It provides a daemon, and a
CLI search interface. I also know a few people running Baloo on
servers.

This whole "frameworks-baloo" vs "baloo" seems more pedantic than
anything else. Specially considering the amount of overhead this is
actively causing. I just have a crash report from 2 hours ago filed to
Baloo (not Baloo-frameworks). Bug reports against previous versions of
Baloo won't automatically go to this new "baloo-frameworks". And I
have over 130+ new bug emails, which aren't relevant.

I'm all for consistency, but not when it has a real cost.

--
Vishesh Handa

Re: Product versions on bugs.kde.org

[Vishesh Handa]

On Sat, Mar 12, 2016 at 4:02 AM, Alexander Potashev
<aspotas...@gmail.com> wrote:
> 7. Created product frameworks-baloo and added versions 5.13.0+ [4].
> Bugs for older, out-of-KF5 versions go into the "Baloo" product.
>

For the record, this just doubles the number of places I need to look.

And from my point of view the user doesn't care if Baloo is part of
something called "Frameworks". There is a bug, they want to report it.
It's just added noise.

> 8. Added product "BalooWidgets", it's part of KDE Applications. Moved
> relevant bugs from the "Baloo" product.

Sure .. I guess. I don't see the advantage apart from just being pedantic.

> 9. Added product "Baloo KCM", it's part of Plasma
> (plasma-desktop.git/kcms/baloo). Moved relevant bugs from the "Baloo"
> product.
>

Then move it under Plasma? Or sure.

> I guess now the products "kpeople" and "Baloo" can be obsoleted in Bugzilla.
>
>
> If the above looks good, please add the product "Baloo KCM" into
> releaseme.git/plasma/plasma-add-bugzilla-versions.

In the future, I would appreciate it if such decisions were run by the
maintainer. I now have more work since my existing filters and
bookmarks are now invalid. And the documentation which tells where to
file a bug.

--
Vishesh Handa

Re: Review Request 127023: [KFileMetadata] Support Origin Email subject/sender/id

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127023/#review92404
---


Fix it, then Ship it!





src/propertyinfo.cpp (line 421)
<https://git.reviewboard.kde.org/r/127023/#comment63024>

I'm not too keen on indexing this unless there is a clear use case. It just 
increases the size of the index.

During the Nepomuk days we used to index and collect way too much 
information with the hope that we would be find amazing creative uses for the 
data. It did not end well.


- Vishesh Handa


On Feb. 9, 2016, 9:09 p.m., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127023/
> ---
> 
> (Updated Feb. 9, 2016, 9:09 p.m.)
> 
> 
> Review request for KDE Frameworks, KDEPIM, Daniel Vrátil, Sebastian Kügler, 
> and Vishesh Handa.
> 
> 
> Repository: kfilemetadata
> 
> 
> Description
> ---
> 
> This adds support for the user.xdg.origin.email.subject, 
> user.xdg.origin.email.sender, user.xdg.origin.email.message-id xattrs [1] to 
> KFileMetadata.
> 
> This can (should :P) be populated by KMail when you save an attachment.
> 
> Not too happy about the "displayName" I chose. Also we'll need to figure out 
> what to index and how we can relate this information and present it to the 
> user in a meaningful way. But at least let's collect the information first 
> and then we can think about ways to handle this.
> 
> [1] https://wiki.freedesktop.org/www/CommonExtendedAttributes/
> 
> 
> Diffs
> -
> 
>   src/properties.h 6ceaca5 
>   src/propertyinfo.cpp 4d1fac4 
>   src/usermetadata.h 9e10d2a 
>   src/usermetadata.cpp 5485d0e 
> 
> Diff: https://git.reviewboard.kde.org/r/127023/diff/
> 
> 
> Testing
> ---
> 
> Not really. Tests pass, though.
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126227: Fix incomplete or duplicate search result with mtime

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126227/#review89508
---

Ship it!


- Vishesh Handa


On Dec. 6, 2015, 6:03 p.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126227/
> ---
> 
> (Updated Dec. 6, 2015, 6:03 p.m.)
> 
> 
> Review request for Baloo and Vishesh Handa.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> Test with
> - baloosearch 'modified=>...' -d $HOME
> - baloosearch 'modified=>...'
> 
> This two queries should return same result since I only index home folder. 
> But they are not, because result from mtimedb is not sorted.
> 
> In some other rare case (On my machine, regularly happens to torrent 
> downloaded files), it could also return duplicate result. not sure if this 
> should be considered as a indexer bug.
> 
> Sort and remove duplication from the result in VectorPostingIterator when 
> requesting first result with next().
> 
> 
> Diffs
> -
> 
>   autotests/unit/engine/mtimedbtest.cpp e38022d 
>   src/engine/mtimedb.h 1dc1cb9 
>   src/engine/mtimedb.cpp f7283b5 
> 
> Diff: https://git.reviewboard.kde.org/r/126227/diff/
> 
> 
> Testing
> ---
> 
> No more duplicate result or missing result.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125369: Baloo runner: look also for presentations/spreadsheets/text files

[Vishesh Handa]

> On Sept. 24, 2015, 11:50 p.m., Vishesh Handa wrote:
> > runners/baloo/baloosearchrunner.cpp, line 165
> > <https://git.reviewboard.kde.org/r/125369/diff/1/?file=405159#file405159line165>
> >
> > Have a look at basicindexingjob.txt. We define that all Presentation 
> > and Spreadsheets are also Documents.
> > 
> > For text, we specifically don't do that, as then we get source code as 
> > Documents. Maybe we can make another category caleld "Source Code" or 
> > something. I'm not sure, but just combining the two doesn't seem correct.
> > 
> > Maybe others can chime in?
> 
> Igor Poboiko wrote:
> Dammit, my fault. Sorry for the noise. I just noticed that sometimes 
> KRunner don't look for presentations, so I thought the issue was here. And 
> when I fixed it, it worked.
> But it seems like the problem is with indexing; sometimes mimetype is not 
> determined correctly. And it's not always reproducible. But that's another 
> issue.
> 
> # balooctl index test.ppt
> Indexing /home/eol/test.ppt
>  -- indexing "/home/eol/test.ppt" mimetype "application/x-ole-storage"
> File(s) indexed
> # mimetype test.ppt
> application/vnd.ms-powerpoint
> 
> Regarding indexing of text files: I thought the issue was just due to 
> performance of indexing; but now it doesn't index large text-files.
> I mean, e.g. I have a lot of plain-text documents (which are not source 
> codes, but are proper documents): those are mostly LaTeX files or just .txt 
> files with some notes. I expected them to pop up during search.
> 
> Pinak Ahuja wrote:
> That would me mainly because we skip text files larger than 10 mb, have a 
> look at file/extractor/app.cpp
> Maybe we can experiment with a different limit. Mimetype issue is 
> something we should look into. I've seen audio files with .m4a extension 
> showing mimetype as video in the index.
> 
> Igor Poboiko wrote:
> No-no, that wasn't the case. My .tex-files are much smaller (several 
> KBs), and "balooshow -x" shows that they are indexed properly. They just 
> don't popup because of its type ("T8 Ttext" in index).
> 
> The limit solution looks perfectly fine for me. I mean, I don't write 
> files of size 10Mb+ by myself :) 
> They are 99.99% are not documents but just some machine-generated junk 
> (log-files, or something like that), and I don't want it to be indexed 
> anyways.
> But small text files might be useful for me. Especially if they are 
> already indexed :)
> 
> Talking about mimetypes: right, I had the same issue with mp4 audio-files 
> too. But it seems to be an upstream issue, mimetype handling is 
> shared-mime-database and Qt job...

Ping? Can this be closed.


- Vishesh


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125369/#review85905
---


On Sept. 24, 2015, 7:18 a.m., Igor Poboiko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125369/
> ---
> 
> (Updated Sept. 24, 2015, 7:18 a.m.)
> 
> 
> Review request for Baloo and Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Small usability improvement: since presentations, spreadsheets and text files 
> are also documents (at least, that's what I expect :) ), search for them in 
> KRunner when "documents" category is checked.
> 
> 
> Diffs
> -
> 
>   runners/baloo/baloosearchrunner.cpp 0023a11 
> 
> Diff: https://git.reviewboard.kde.org/r/125369/diff/
> 
> 
> Testing
> ---
> 
> It just works: now I am able to find my LaTeX-files, various presentations, 
> etc with KRunner.
> 
> 
> Thanks,
> 
> Igor Poboiko
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 126006: KF5 compilation failing due to baloo tests/file/CMakeLists.txt

[Vishesh Handa]

> On Nov. 24, 2015, 11:05 a.m., Vishesh Handa wrote:
> > Ping?

I'm closing this review request for now. The changes in the diff are handled by 
release scripts. Please reopen this if you feel otherwise.


- Vishesh


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126006/#review88753
---


On Nov. 9, 2015, 3:51 p.m., Jan Issac wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126006/
> ---
> 
> (Updated Nov. 9, 2015, 3:51 p.m.)
> 
> 
> Review request for Baloo.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> Little fix in baloo tests/file/CMakeLists.txt
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt cc93ed3 
> 
> Diff: https://git.reviewboard.kde.org/r/126006/diff/
> 
> 
> Testing
> ---
> 
> Fixed tests/file/CMakeLists.txt. KF5 build with tests was not completing 
> because of this.
> 
> 
> File Attachments
> 
> 
> missing /src/dbus/extractor_interface.cpp
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/11/09/29b5024d-662c-4bed-826b-3f23bf88ee33__0001-fixed-missing-generated-file.patch
> 
> 
> Thanks,
> 
> Jan Issac
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125762: External extractor plugin support for KFileMetaData

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125762/#review89174
---


If you're planning on pushing this please push it to a testing branch. Untill 
we actually have some plugins using this we might not be sure of the API. My 
points below are quite negative and I was hoping on writing a more positive 
email about the different ways to go about this, but I seem to have been 
procrastinating and have not written it. Sorry.

My main objections with this -

1. One cannot choose between external plugins. Since all of them are in 
1-plugin (called ExternalPlugin), and all the mimtypes are just combined. This 
matters less in the case of Baloo, but when someone else wants to choose 
specific extractors. It's impossible.

2. Extraction of plain text is simply not supported.

3. No way to differentiate in the implementation of a plugin. We may get 
plugins for the same mimetype using different technologies, perhaps we need a 
way to identify which one to choose. Maybe this will never be a problem, but 
I'm skeptical.

4. Contamination of the PATH, maybe we could put them in a different place 
which makes it obvious that users should never execute them.

5. Getting the list of extractors now means running a possibly large number of 
executables with possibly bad implementations. They could just get stuck, and 
all other plugins will suffer. Perhaps the list of mimetypes supported could be 
in a desktop file?

- Vishesh Handa


On Oct. 24, 2015, 12:19 p.m., Boudhayan Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125762/
> ---
> 
> (Updated Oct. 24, 2015, 12:19 p.m.)
> 
> 
> Review request for Baloo, KDE Frameworks, Pinak Ahuja, and Vishesh Handa.
> 
> 
> Repository: kfilemetadata
> 
> 
> Description
> ---
> 
> This patch introduces support for external metadata extractors in 
> KFileMetaData
> 
> The external extractors themselves can be written in any language, provided 
> that it can be executed as a standalone executable (compiled or script with a 
> hashbang), with command line arguments, and can output data to stdout.
> 
> The extractors are executed like so:
> 
> * `extractor --mimetypes` - outputs a list of mimetypes supported by the 
> extractor, one per line.
> * `extractor filename` - outputs a json document with the metadata. The keys 
> are such that they can be directly used with PropertyInfo::fromName().
> 
> At the KFileMetaData end, an additional internal plugin (ExternalExtractor) 
> is provided that forms a conduit between external extractors and the internal 
> API. This plugin looks for executables called 
> kfilemetadata_extractor_ in /usr/bin to find external extractors, 
> and executes them with the --mimetypes arg to find the list of mimetypes each 
> extractor supports. ExternalExtractor then claims to support all of these 
> mimetypes, and then delegates to the extractor executable when doing the 
> actual extraction.
> 
> 
> Diffs
> -
> 
>   README.md 19b1a26 
>   src/extractors/CMakeLists.txt 5dd223e 
>   src/extractors/externalextractor.h PRE-CREATION 
>   src/extractors/externalextractor.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/125762/diff/
> 
> 
> Testing
> ---
> 
> Tested with the sample executable file extractor (as attched, written in 
> python) with the dump manual test in KFileMetaData. Works.
> 
> 
> File Attachments
> 
> 
> kfilemetadata_extractor_executable
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/10/23/146b657f-31d9-4117-a82f-ef966a6339d4__kfilemetadata_extractor_executable
> 
> 
> Thanks,
> 
> Boudhayan Gupta
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125762: External extractor plugin support for KFileMetaData

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125762/#review89174
---


If you're planning on pushing this please push it to a testing branch. Untill 
we actually have some plugins using this we might not be sure of the API. My 
points below are quite negative and I was hoping on writing a more positive 
email about the different ways to go about this, but I seem to have been 
procrastinating and have not written it. Sorry.

My main objections with this -

1. One cannot choose between external plugins. Since all of them are in 
1-plugin (called ExternalPlugin), and all the mimtypes are just combined. This 
matters less in the case of Baloo, but when someone else wants to choose 
specific extractors. It's impossible.

2. Extraction of plain text is simply not supported.

3. No way to differentiate in the implementation of a plugin. We may get 
plugins for the same mimetype using different technologies, perhaps we need a 
way to identify which one to choose. Maybe this will never be a problem, but 
I'm skeptical.

4. Contamination of the PATH, maybe we could put them in a different place 
which makes it obvious that users should never execute them.

5. Getting the list of extractors now means running a possibly large number of 
executables with possibly bad implementations. They could just get stuck, and 
all other plugins will suffer. Perhaps the list of mimetypes supported could be 
in a desktop file?

- Vishesh Handa


On Oct. 24, 2015, 12:19 p.m., Boudhayan Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125762/
> ---
> 
> (Updated Oct. 24, 2015, 12:19 p.m.)
> 
> 
> Review request for Baloo, KDE Frameworks, Pinak Ahuja, and Vishesh Handa.
> 
> 
> Repository: kfilemetadata
> 
> 
> Description
> ---
> 
> This patch introduces support for external metadata extractors in 
> KFileMetaData
> 
> The external extractors themselves can be written in any language, provided 
> that it can be executed as a standalone executable (compiled or script with a 
> hashbang), with command line arguments, and can output data to stdout.
> 
> The extractors are executed like so:
> 
> * `extractor --mimetypes` - outputs a list of mimetypes supported by the 
> extractor, one per line.
> * `extractor filename` - outputs a json document with the metadata. The keys 
> are such that they can be directly used with PropertyInfo::fromName().
> 
> At the KFileMetaData end, an additional internal plugin (ExternalExtractor) 
> is provided that forms a conduit between external extractors and the internal 
> API. This plugin looks for executables called 
> kfilemetadata_extractor_ in /usr/bin to find external extractors, 
> and executes them with the --mimetypes arg to find the list of mimetypes each 
> extractor supports. ExternalExtractor then claims to support all of these 
> mimetypes, and then delegates to the extractor executable when doing the 
> actual extraction.
> 
> 
> Diffs
> -
> 
>   README.md 19b1a26 
>   src/extractors/CMakeLists.txt 5dd223e 
>   src/extractors/externalextractor.h PRE-CREATION 
>   src/extractors/externalextractor.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/125762/diff/
> 
> 
> Testing
> ---
> 
> Tested with the sample executable file extractor (as attched, written in 
> python) with the dump manual test in KFileMetaData. Works.
> 
> 
> File Attachments
> 
> 
> kfilemetadata_extractor_executable
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/10/23/146b657f-31d9-4117-a82f-ef966a6339d4__kfilemetadata_extractor_executable
> 
> 
> Thanks,
> 
> Boudhayan Gupta
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126227: Fix incomplete or duplicate search result with mtime

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126227/#review89170
---


If you have the time, some automated tests for this would be fairly awesome as 
well.

I'm a little charry of adding the removal of duplicate entires, it seems like 
we're patching over a problem instead o figuring out the root cause.


src/engine/mtimedb.cpp (line 98)
<https://git.reviewboard.kde.org/r/126227/#comment61013>

Could you please sort these values as well? This is possible why we are 
getting duplicates. The WriteTransaction assumes the data is sorted when it 
gets it.



src/engine/mtimedb.cpp (line 142)
<https://git.reviewboard.kde.org/r/126227/#comment61014>

This can be removed once `get` is sorted.


- Vishesh Handa


On Dec. 6, 2015, 5:29 a.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126227/
> ---
> 
> (Updated Dec. 6, 2015, 5:29 a.m.)
> 
> 
> Review request for Baloo and Vishesh Handa.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> Test with
> - baloosearch 'modified=>...' -d $HOME
> - baloosearch 'modified=>...'
> 
> This two queries should return same result since I only index home folder. 
> But they are not, because result from mtimedb is not sorted.
> 
> In some other rare case (On my machine, regularly happens to torrent 
> downloaded files), it could also return duplicate result. not sure if this 
> should be considered as a indexer bug.
> 
> Sort and remove duplication from the result in VectorPostingIterator when 
> requesting first result with next().
> 
> 
> Diffs
> -
> 
>   src/engine/mtimedb.cpp f7283b5 
> 
> Diff: https://git.reviewboard.kde.org/r/126227/diff/
> 
> 
> Testing
> ---
> 
> No more duplicate result or missing result.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Sprint: Reorganize the wikis

[Vishesh Handa]

Hey Olivier

I'm so happy to see someone bring this up.

On Thu, Dec 3, 2015 at 12:10 AM, Olivier Churlaud <oliv...@churlaud.com> wrote:
> Hi,
>
> I'm coming here with this observation: the wiki are quite a mess. And very
> often KDE4 and KF5 things are mixed.
> That everyone do it at their scale is impossible. That's why I have a
> proposition. (I've thought about it for a while)
>
> It would be easier to do this like in a sprint, in team.
>
> 1) Define what structure the wiki should have: What is techbase? What is
> Community? Are the manuals in Userbase or on doc? and so on
> 2) then take each page and order => Archive KDE4 | Mess | KF5 | To remove
> 3) Do a list of what is missing and what should be tidied.
> 4) Write to the corresponding teams so that they take care of it.
> 5) Remove what should be
>

Perhaps also -

6) Are wikis the ideal way to present documentation? The have pros and
cons, and perhaps we can be more flexible on different approaches and
see what works out. We seem to have documentation in git repos [1],
wikis, and even a book or two.

[1] https://github.com/KDE/baloo/blob/master/docs/user/searching.md

> What I mean in 2) is: for example there are N pages about how to configure
> Git. We take the good one, put the other in a namespace "To remove"
>
> What I mean in 4) is: for example in https://community.kde.org/Sonnet, it's
> very old, and not much documented (sorry if some of you are reading this, I
> pick it randomly), so we can put a mail on the ML and tell them 'well we saw
> that...'
>
> Basically this is my vision of what could be done.
>
> Since I don't always know what is old what is still usable and so on, people
> with this knowledge would be required.
>
> I think we should plan it very carefully, to have a battle plan, and then
> go.
>
> What do you think?
> Would people be interested to give a hand?
> How and when do we begin?

I would definitely be interested.

However, I doubt I have the time to take time off work and travel for
this, so remotely, rather than physically.

--
Vishesh Handa

>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 126223: Fix date filter used by timeline:/

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126223/#review89142
---

Ship it!



src/lib/searchstore.cpp (line 221)
<https://git.reviewboard.kde.org/r/126223/#comment61008>

I'm guessing I was trying to implement the case when you only specify the 
year + day and not the month, though that doesn't seem to make much sense to me.


- Vishesh Handa


On Dec. 2, 2015, 6:42 p.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126223/
> ---
> 
> (Updated Dec. 2, 2015, 6:42 p.m.)
> 
> 
> Review request for Baloo and Vishesh Handa.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> timeline:/ doesn't return correct result for month 1 or day 1, this is 
> because date filter always takes 1 as whole month or whole year. 0 should be 
> used in such case.
> 
> 
> Diffs
> -
> 
>   src/lib/searchstore.cpp 11fbd3f 
> 
> Diff: https://git.reviewboard.kde.org/r/126223/diff/
> 
> 
> Testing
> ---
> 
> timeline://2015-12/2015-12-01 now returns correct result.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 126227: Fix incomplete or duplicate search result with mtime

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126227/#review89141
---



src/engine/vectorpostingiterator.cpp (line 43)
<https://git.reviewboard.kde.org/r/126227/#comment61007>

Do you think you could sort the values in the mtime posting iterator before 
sending it forward, instead of sorting it over here?

The VectorPostingIteartor just doesn't seem like the correct place.


- Vishesh Handa


On Dec. 3, 2015, 2:09 a.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126227/
> ---
> 
> (Updated Dec. 3, 2015, 2:09 a.m.)
> 
> 
> Review request for Baloo and Vishesh Handa.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> Test with
> - baloosearch 'modified=>...' -d $HOME
> - baloosearch 'modified=>...'
> 
> This two queries should return same result since I only index home folder. 
> But they are not, because result from mtimedb is not sorted.
> 
> In some other rare case (On my machine, regularly happens to torrent 
> downloaded files), it could also return duplicate result. not sure if this 
> should be considered as a indexer bug.
> 
> Sort and remove duplication from the result in VectorPostingIterator when 
> requesting first result with next().
> 
> 
> Diffs
> -
> 
>   src/engine/vectorpostingiterator.cpp 777b7fd 
> 
> Diff: https://git.reviewboard.kde.org/r/126227/diff/
> 
> 
> Testing
> ---
> 
> No more duplicate result or missing result.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 126132: Better bitrate format in filemetadatawidget

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126132/#review88752
---

Ship it!



src/widgetfactory.cpp (line 118)
<https://git.reviewboard.kde.org/r/126132/#comment60856>

Perhaps use QStringLiteral instead?


- Vishesh Handa


On Nov. 22, 2015, 2:26 a.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126132/
> ---
> 
> (Updated Nov. 22, 2015, 2:26 a.m.)
> 
> 
> Review request for Baloo and Vishesh Handa.
> 
> 
> Repository: baloo-widgets
> 
> 
> Description
> ---
> 
> Bitrate can be better formatted.
> 
> Now it's either a strange double number (3.00594e+06), or a long integer 
> (32).
> 
> 
> Diffs
> -
> 
>   src/widgetfactory.cpp 7151952 
> 
> Diff: https://git.reviewboard.kde.org/r/126132/diff/
> 
> 
> Testing
> ---
> 
> Now 3.00594e+06 is displayed as 3.0 MB/s. 
> 32 is displayed as 320 kB/s.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 126110: Clean up and armour Baloo::Database::open()

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126110/#review88751
---

Ship it!



src/engine/database.cpp (line 73)
<https://git.reviewboard.kde.org/r/126110/#comment60854>

Please remove the comment. It's not longer valid.



src/engine/database.cpp (line 192)
<https://git.reviewboard.kde.org/r/126110/#comment60855>

Perhaps you want to close the env here as well? The assert can fail in 
production.


- Vishesh Handa


On Nov. 19, 2015, 11:39 a.m., Boudhayan Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126110/
> ---
> 
> (Updated Nov. 19, 2015, 11:39 a.m.)
> 
> 
> Review request for Baloo, KDE Frameworks, Pinak Ahuja, and Vishesh Handa.
> 
> 
> Bugs: 353757
> http://bugs.kde.org/show_bug.cgi?id=353757
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> * Add checks for failures
> * Add manual checks after Q_ASSERT* (they're not compiled in Release mode)
> * Clean up m_env after failure, not just set it to 0
> 
> 
> Diffs
> -
> 
>   src/engine/database.cpp e39eb86 
> 
> Diff: https://git.reviewboard.kde.org/r/126110/diff/
> 
> 
> Testing
> ---
> 
> Builds, runs, does not crash.
> make test succeeds at 100%
> 
> 
> Thanks,
> 
> Boudhayan Gupta
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126110: Clean up and armour Baloo::Database::open()

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126110/#review88751
---

Ship it!



src/engine/database.cpp (line 73)
<https://git.reviewboard.kde.org/r/126110/#comment60854>

Please remove the comment. It's not longer valid.



src/engine/database.cpp (line 192)
<https://git.reviewboard.kde.org/r/126110/#comment60855>

Perhaps you want to close the env here as well? The assert can fail in 
production.


- Vishesh Handa


On Nov. 19, 2015, 11:39 a.m., Boudhayan Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126110/
> ---
> 
> (Updated Nov. 19, 2015, 11:39 a.m.)
> 
> 
> Review request for Baloo, KDE Frameworks, Pinak Ahuja, and Vishesh Handa.
> 
> 
> Bugs: 353757
> http://bugs.kde.org/show_bug.cgi?id=353757
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> * Add checks for failures
> * Add manual checks after Q_ASSERT* (they're not compiled in Release mode)
> * Clean up m_env after failure, not just set it to 0
> 
> 
> Diffs
> -
> 
>   src/engine/database.cpp e39eb86 
> 
> Diff: https://git.reviewboard.kde.org/r/126110/diff/
> 
> 
> Testing
> ---
> 
> Builds, runs, does not crash.
> make test succeeds at 100%
> 
> 
> Thanks,
> 
> Boudhayan Gupta
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 126006: KF5 compilation failing due to baloo tests/file/CMakeLists.txt

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126006/#review88753
---


Ping?

- Vishesh Handa


On Nov. 9, 2015, 3:51 p.m., Jan Issac wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126006/
> ---
> 
> (Updated Nov. 9, 2015, 3:51 p.m.)
> 
> 
> Review request for Baloo.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> Little fix in baloo tests/file/CMakeLists.txt
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt cc93ed3 
> 
> Diff: https://git.reviewboard.kde.org/r/126006/diff/
> 
> 
> Testing
> ---
> 
> Fixed tests/file/CMakeLists.txt. KF5 build with tests was not completing 
> because of this.
> 
> 
> File Attachments
> 
> 
> missing /src/dbus/extractor_interface.cpp
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/11/09/29b5024d-662c-4bed-826b-3f23bf88ee33__0001-fixed-missing-generated-file.patch
> 
> 
> Thanks,
> 
> Jan Issac
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125868: Baloo Engine: Disable COW on btrfs

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125868/
---

(Updated Nov. 19, 2015, 1:39 a.m.)


Status
--

This change has been marked as submitted.


Review request for Baloo.


Changes
---

Submitted with commit 3d9c33ced6368dc9278ecfe00c92645282d2d5d6 by Vishesh Handa 
to branch master.


Repository: baloo


Description
---

We do not need the extra overhead. Specially since LMDB internally uses
a COW mechanism for the btree. (Append only btree)

code stolen from Akonadi


Diffs
-

  src/engine/CMakeLists.txt 96d63f4 
  src/engine/database.cpp 4f0579f 
  src/engine/fsutils.h PRE-CREATION 
  src/engine/fsutils.cpp PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/125868/diff/


Testing
---


Thanks,

Vishesh Handa


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 126105: Fix yet another crash in Dolphin when Baloo isn't running

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126105/#review88554
---

Ship it!


I'm a little hesitant about this, but ship it.

A couple of things though -
1. Dolphin should probably not be sending Baloo so many requests if Baloo is 
disabled. Perhaps we should fix this in Dolphin.
2. This patch is mostly only for the Baloo::File class which causes the db to 
be opened and closed for each operation. It's expensive, but the main use case 
that I focussed on was searching. Over there the db is only opened once per 
application.

- Vishesh Handa


On Nov. 18, 2015, 7:40 p.m., Boudhayan Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126105/
> ---
> 
> (Updated Nov. 18, 2015, 7:40 p.m.)
> 
> 
> Review request for Baloo, KDE Frameworks, Pinak Ahuja, and Vishesh Handa.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> Add a check in Baloo::Database::open() to return false if we're opening the 
> database in ReadOnly mode and the database doesn't exist. This fixes a crash 
> in Dolphin when Baloo isn't running.
> 
> This isn't the entire fix - one will also have to remove 
> ~/.local/share/baloo/index to not crash anymore; this patch prevents the file 
> from being recreated everytime Baloo::Database::open() is run, and re-causing 
> the crash.
> 
> 
> Diffs
> -
> 
>   src/engine/database.cpp 4f0579f 
> 
> Diff: https://git.reviewboard.kde.org/r/126105/diff/
> 
> 
> Testing
> ---
> 
> Builds, runs, doesn't crash anymore, the works.
> 
> 
> Thanks,
> 
> Boudhayan Gupta
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 126105: Fix yet another crash in Dolphin when Baloo isn't running

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126105/#review88554
---

Ship it!


I'm a little hesitant about this, but ship it.

A couple of things though -
1. Dolphin should probably not be sending Baloo so many requests if Baloo is 
disabled. Perhaps we should fix this in Dolphin.
2. This patch is mostly only for the Baloo::File class which causes the db to 
be opened and closed for each operation. It's expensive, but the main use case 
that I focussed on was searching. Over there the db is only opened once per 
application.

- Vishesh Handa


On Nov. 18, 2015, 7:40 p.m., Boudhayan Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126105/
> ---
> 
> (Updated Nov. 18, 2015, 7:40 p.m.)
> 
> 
> Review request for Baloo, KDE Frameworks, Pinak Ahuja, and Vishesh Handa.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> Add a check in Baloo::Database::open() to return false if we're opening the 
> database in ReadOnly mode and the database doesn't exist. This fixes a crash 
> in Dolphin when Baloo isn't running.
> 
> This isn't the entire fix - one will also have to remove 
> ~/.local/share/baloo/index to not crash anymore; this patch prevents the file 
> from being recreated everytime Baloo::Database::open() is run, and re-causing 
> the crash.
> 
> 
> Diffs
> -
> 
>   src/engine/database.cpp 4f0579f 
> 
> Diff: https://git.reviewboard.kde.org/r/126105/diff/
> 
> 
> Testing
> ---
> 
> Builds, runs, doesn't crash anymore, the works.
> 
> 
> Thanks,
> 
> Boudhayan Gupta
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 125868: Baloo Engine: Disable COW on btrfs

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125868/
---

Review request for Baloo.


Repository: baloo


Description
---

We do not need the extra overhead. Specially since LMDB internally uses
a COW mechanism for the btree. (Append only btree)

code stolen from Akonadi


Diffs
-

  src/engine/CMakeLists.txt 96d63f4 
  src/engine/database.cpp 4f0579f 
  src/engine/fsutils.h PRE-CREATION 
  src/engine/fsutils.cpp PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/125868/diff/


Testing
---


Thanks,

Vishesh Handa


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125777: Monitor lib: Use Kformat::spelloutDuration to localize time string

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125777/#review87608
---

Ship it!


Ship It!

- Vishesh Handa


On Oct. 28, 2015, 10:25 a.m., Pinak Ahuja wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125777/
> ---
> 
> (Updated Oct. 28, 2015, 10:25 a.m.)
> 
> 
> Review request for Baloo and Vishesh Handa.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> KFormat::spelloutDuration works perfectly well for out use case, using it is 
> a no brainer.
> 
> 
> Diffs
> -
> 
>   src/qml/experimental/CMakeLists.txt 0b06ac8 
>   src/qml/experimental/monitor.cpp dfc7037 
> 
> Diff: https://git.reviewboard.kde.org/r/125777/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Pinak Ahuja
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125818: Use KDE_INSTALL_FULL_DBUSINTERFACEDIR to install dbus interfaces

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125818/#review87535
---


I really have no idea about this. If you're sure about this go ahead.

- Vishesh Handa


On Oct. 26, 2015, 8:36 p.m., Heiko Becker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125818/
> ---
> 
> (Updated Oct. 26, 2015, 8:36 p.m.)
> 
> 
> Review request for Baloo and Vishesh Handa.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> Two reasons to do this:
> - DBUS_INTERFACES_INSTALL_DIR is marked deprecated
> - It's helpful on a multiarch layout where the prefix is /usr/${host}
>   but arch-independent files should still be installed to /usr/share.
> 
> 
> Diffs
> -
> 
>   KF5BalooConfig.cmake.in 08413efa29290c37dc501cc2ec1eb86d13218981 
>   src/dbus/CMakeLists.txt 44240ec643eef69f90b3832314ede46bca1820e5 
>   src/dbus/fake/CMakeLists.txt c9735b3d176c4a3c7ee995dac0aa83a3b715bf7c 
> 
> Diff: https://git.reviewboard.kde.org/r/125818/diff/
> 
> 
> Testing
> ---
> 
> Successfully installed it with 
> -DCMAKE_INSTALL_PREFIX:PATH=/usr/x86_64-pc-linux-gnu and 
> -DCMAKE_INSTALL_FULL_DATAROOTDIR:PATH=/usr/share
> 
> 
> Thanks,
> 
> Heiko Becker
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125776: UnindexedFileIndexer: Handle files that have been moved when baloo_file was not running

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125776/#review87537
---



src/engine/transaction.h 
<https://git.reviewboard.kde.org/r/125776/#comment60096>

I'm a little confused. Isn't the metadatamover using this method?

$ ack renameFilePath
src/file/metadatamover.cpp
116:tr->renameFilePath(id, job.document());


- Vishesh Handa


On Oct. 26, 2015, 8:16 a.m., Pinak Ahuja wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125776/
> ---
> 
> (Updated Oct. 26, 2015, 8:16 a.m.)
> 
> 
> Review request for Baloo and Vishesh Handa.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> Since the inode remains same on moving, the fileId should be preserved. We 
> can simply check if the filePath in the db is same as the actual filePath to 
> check if file is moved and update the filePath if needed. This will also 
> handle renames.
> 
> 
> Diffs
> -
> 
>   src/engine/documentoperations.h 6382e53 
>   src/engine/transaction.h 2ae052e 
>   src/engine/transaction.cpp f3c0f4c 
>   src/engine/writetransaction.cpp ae27ad9 
>   src/file/unindexedfileindexer.cpp 6873047 
> 
> Diff: https://git.reviewboard.kde.org/r/125776/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Pinak Ahuja
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125818: Use KDE_INSTALL_DBUSINTERFACEDIR to install dbus interfaces

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125818/#review87546
---

Ship it!


Ship It!

- Vishesh Handa


On Oct. 27, 2015, 6:53 p.m., Heiko Becker wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125818/
> ---
> 
> (Updated Oct. 27, 2015, 6:53 p.m.)
> 
> 
> Review request for Baloo and Vishesh Handa.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> ...and use PATH_VARS to make the config file work with absolute paths.
> 
> Two reasons to do this:
> - DBUS_INTERFACES_INSTALL_DIR is marked deprecated
> - By not hard-coding the packackage prefix, it's helpful on a multiarch
>   layout where the prefix is /usr/${host} but arch-independent files
>   should still be installed to /usr/share (i.e a level below the
>   prefix).
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 9f719667902cd13257b59113f1eb8bb0e8515d28 
>   KF5BalooConfig.cmake.in 08413efa29290c37dc501cc2ec1eb86d13218981 
>   src/dbus/CMakeLists.txt 44240ec643eef69f90b3832314ede46bca1820e5 
>   src/dbus/fake/CMakeLists.txt c9735b3d176c4a3c7ee995dac0aa83a3b715bf7c 
> 
> Diff: https://git.reviewboard.kde.org/r/125818/diff/
> 
> 
> Testing
> ---
> 
> Successfully installed it with 
> -DCMAKE_INSTALL_PREFIX:PATH=/usr/x86_64-pc-linux-gnu and 
> -DCMAKE_INSTALL_FULL_DATAROOTDIR:PATH=/usr/share
> 
> 
> Thanks,
> 
> Heiko Becker
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125776: UnindexedFileIndexer: Handle files that have been moved when baloo_file was not running

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125776/#review87541
---

Ship it!


:)

- Vishesh Handa


On Oct. 27, 2015, 6:35 p.m., Pinak Ahuja wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125776/
> ---
> 
> (Updated Oct. 27, 2015, 6:35 p.m.)
> 
> 
> Review request for Baloo and Vishesh Handa.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> Since the inode remains same on moving, the fileId should be preserved. We 
> can simply check if the filePath in the db is same as the actual filePath to 
> check if file is moved and update the filePath if needed. This will also 
> handle renames.
> 
> 
> Diffs
> -
> 
>   src/engine/documentoperations.h 6382e53 
>   src/engine/transaction.h 2ae052e 
>   src/engine/transaction.cpp f3c0f4c 
>   src/engine/writetransaction.cpp ae27ad9 
>   src/file/metadatamover.cpp adbd696 
>   src/file/unindexedfileindexer.cpp 6873047 
> 
> Diff: https://git.reviewboard.kde.org/r/125776/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Pinak Ahuja
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125776: UnindexedFileIndexer: Handle files that have been moved when baloo_file was not running

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125776/#review87350
---



src/file/unindexedfileindexer.cpp (line 65)
<https://git.reviewboard.kde.org/r/125776/#comment59983>

This is a silly thing, but do you think perhaps we should move this 
'renameFilePath' call inside `replaceDocument`, and just add an extra operation?


- Vishesh Handa


On Oct. 24, 2015, 6:17 p.m., Pinak Ahuja wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125776/
> ---
> 
> (Updated Oct. 24, 2015, 6:17 p.m.)
> 
> 
> Review request for Baloo and Vishesh Handa.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> Since the inode remains same on moving, the fileId should be preserved. We 
> can simply check if the filePath in the db is same as the actual filePath to 
> check if file is moved and update the filePath if needed. This will also 
> handle renames.
> 
> 
> Diffs
> -
> 
>   src/file/unindexedfileindexer.cpp 6873047 
> 
> Diff: https://git.reviewboard.kde.org/r/125776/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Pinak Ahuja
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125762: External extractor plugin support for KFileMetaData

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125762/#review87334
---


I still have many thoughts regarding this particular approach, and third party 
plugins in general. I'm trying to write them into a cohesive blob.


src/extractors/externalextractor.cpp (line 69)
<https://git.reviewboard.kde.org/r/125762/#comment59973>

Why move from the C++ for syntax to Q_FOREACH? There doesn't seem to be any 
advantage in this case.


- Vishesh Handa


On Oct. 24, 2015, 12:19 p.m., Boudhayan Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125762/
> ---
> 
> (Updated Oct. 24, 2015, 12:19 p.m.)
> 
> 
> Review request for Baloo, KDE Frameworks, Pinak Ahuja, and Vishesh Handa.
> 
> 
> Repository: kfilemetadata
> 
> 
> Description
> ---
> 
> This patch introduces support for external metadata extractors in 
> KFileMetaData
> 
> The external extractors themselves can be written in any language, provided 
> that it can be executed as a standalone executable (compiled or script with a 
> hashbang), with command line arguments, and can output data to stdout.
> 
> The extractors are executed like so:
> 
> * `extractor --mimetypes` - outputs a list of mimetypes supported by the 
> extractor, one per line.
> * `extractor filename` - outputs a json document with the metadata. The keys 
> are such that they can be directly used with PropertyInfo::fromName().
> 
> At the KFileMetaData end, an additional internal plugin (ExternalExtractor) 
> is provided that forms a conduit between external extractors and the internal 
> API. This plugin looks for executables called 
> kfilemetadata_extractor_ in /usr/bin to find external extractors, 
> and executes them with the --mimetypes arg to find the list of mimetypes each 
> extractor supports. ExternalExtractor then claims to support all of these 
> mimetypes, and then delegates to the extractor executable when doing the 
> actual extraction.
> 
> 
> Diffs
> -
> 
>   README.md 19b1a26 
>   src/extractors/CMakeLists.txt 5dd223e 
>   src/extractors/externalextractor.h PRE-CREATION 
>   src/extractors/externalextractor.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/125762/diff/
> 
> 
> Testing
> ---
> 
> Tested with the sample executable file extractor (as attched, written in 
> python) with the dump manual test in KFileMetaData. Works.
> 
> 
> File Attachments
> 
> 
> kfilemetadata_extractor_executable
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/10/23/146b657f-31d9-4117-a82f-ef966a6339d4__kfilemetadata_extractor_executable
> 
> 
> Thanks,
> 
> Boudhayan Gupta
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 125762: External extractor plugin support for KFileMetaData

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125762/#review87334
---


I still have many thoughts regarding this particular approach, and third party 
plugins in general. I'm trying to write them into a cohesive blob.


src/extractors/externalextractor.cpp (line 69)
<https://git.reviewboard.kde.org/r/125762/#comment59973>

Why move from the C++ for syntax to Q_FOREACH? There doesn't seem to be any 
advantage in this case.


- Vishesh Handa


On Oct. 24, 2015, 12:19 p.m., Boudhayan Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125762/
> ---
> 
> (Updated Oct. 24, 2015, 12:19 p.m.)
> 
> 
> Review request for Baloo, KDE Frameworks, Pinak Ahuja, and Vishesh Handa.
> 
> 
> Repository: kfilemetadata
> 
> 
> Description
> ---
> 
> This patch introduces support for external metadata extractors in 
> KFileMetaData
> 
> The external extractors themselves can be written in any language, provided 
> that it can be executed as a standalone executable (compiled or script with a 
> hashbang), with command line arguments, and can output data to stdout.
> 
> The extractors are executed like so:
> 
> * `extractor --mimetypes` - outputs a list of mimetypes supported by the 
> extractor, one per line.
> * `extractor filename` - outputs a json document with the metadata. The keys 
> are such that they can be directly used with PropertyInfo::fromName().
> 
> At the KFileMetaData end, an additional internal plugin (ExternalExtractor) 
> is provided that forms a conduit between external extractors and the internal 
> API. This plugin looks for executables called 
> kfilemetadata_extractor_ in /usr/bin to find external extractors, 
> and executes them with the --mimetypes arg to find the list of mimetypes each 
> extractor supports. ExternalExtractor then claims to support all of these 
> mimetypes, and then delegates to the extractor executable when doing the 
> actual extraction.
> 
> 
> Diffs
> -
> 
>   README.md 19b1a26 
>   src/extractors/CMakeLists.txt 5dd223e 
>   src/extractors/externalextractor.h PRE-CREATION 
>   src/extractors/externalextractor.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/125762/diff/
> 
> 
> Testing
> ---
> 
> Tested with the sample executable file extractor (as attched, written in 
> python) with the dump manual test in KFileMetaData. Works.
> 
> 
> File Attachments
> 
> 
> kfilemetadata_extractor_executable
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/10/23/146b657f-31d9-4117-a82f-ef966a6339d4__kfilemetadata_extractor_executable
> 
> 
> Thanks,
> 
> Boudhayan Gupta
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125733: UnindexedFileIndexer: only index required parts of file

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125733/#review87222
---

Ship it!


Nice! Now we just need to handle the entire filePath changing.

- Vishesh Handa


On Oct. 21, 2015, 6:28 a.m., Pinak Ahuja wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125733/
> ---
> 
> (Updated Oct. 21, 2015, 6:28 a.m.)
> 
> 
> Review request for Baloo and Vishesh Handa.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> * Re index file content only if MTime has changed.
> * Re index FileName and Xattrs only if CTime has changed.
> 
> 
> Diffs
> -
> 
>   src/file/unindexedfileindexer.cpp bcd6af1 
>   src/file/unindexedfileiterator.h 90e2342 
>   src/file/unindexedfileiterator.cpp 38c8acd 
> 
> Diff: https://git.reviewboard.kde.org/r/125733/diff/
> 
> 
> Testing
> ---
> 
> manual testing by renaming files/changing xattrs/changing content when baloo 
> is not running and then running 'balooctl check'
> 
> 
> Thanks,
> 
> Pinak Ahuja
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125729: Transaction: add method to return timeInfo struct instead of returning mtime and ctime separately

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125729/#review87165
---

Ship it!


- Vishesh Handa


On Oct. 20, 2015, 6:28 p.m., Pinak Ahuja wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125729/
> ---
> 
> (Updated Oct. 20, 2015, 6:28 p.m.)
> 
> 
> Review request for Baloo and Vishesh Handa.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> Make more sense to fetch both with one method considering they are stored in 
> the same btree.
> 
> 
> Diffs
> -
> 
>   autotests/unit/engine/transactiontest.cpp 478e779 
>   src/engine/transaction.h 77e9ac0 
>   src/engine/transaction.cpp e682fd6 
>   src/file/modifiedfileindexer.cpp 3fa25c0 
>   src/file/unindexedfileiterator.cpp ea3e5a4 
>   src/lib/searchstore.cpp 992c053 
> 
> Diff: https://git.reviewboard.kde.org/r/125729/diff/
> 
> 
> Testing
> ---
> 
> Adapted test passes.
> 
> 
> Thanks,
> 
> Pinak Ahuja
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125643: Databases: Use QByteArray::fromRawData when passing data to a codec

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125643/
---

(Updated Oct. 18, 2015, 10:11 p.m.)


Status
--

This change has been marked as submitted.


Review request for Baloo and Igor Poboiko.


Changes
---

Submitted with commit c67546c4fc8c77d502895cdd240126542e105946 by Vishesh Handa 
to branch master.


Repository: baloo


Description
---

All 3 codecs seem to always copy the data on decoding. We can avoid the
extra copy over here by using QByteArray::fromRawData.


Diffs
-

  src/engine/documentdb.cpp 71c2f62 
  src/engine/positiondb.cpp 032192e 
  src/engine/postingdb.cpp 3045d42 

Diff: https://git.reviewboard.kde.org/r/125643/diff/


Testing
---


Thanks,

Vishesh Handa


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125650: Added exclude mimetypes to balooctl's control

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125650/#review86952
---

Ship it!


A few extra whitespaces on extra lines, but apart from that it seems good. 
Nicely done.

Do you have git access or should I commit it for you?

- Vishesh Handa


On Oct. 16, 2015, 8:13 a.m., Varun Joshi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125650/
> ---
> 
> (Updated Oct. 16, 2015, 8:13 a.m.)
> 
> 
> Review request for Baloo and Vishesh Handa.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> Changed FileIndexerConfig to expose exclude mimetypes
> Changed IndexerConfig to enable getting and setting exclude mimetypes
> Changed ConfigCommand to enable configuration of exclude mimetypes
> 
> 
> Diffs
> -
> 
>   src/file/fileindexerconfig.h 4c7dfcc 
>   src/file/fileindexerconfig.cpp 949ceb2 
>   src/lib/indexerconfig.h d15c2e9 
>   src/lib/indexerconfig.cpp 108390b 
>   src/tools/balooctl/configcommand.cpp 460e318 
> 
> Diff: https://git.reviewboard.kde.org/r/125650/diff/
> 
> 
> Testing
> ---
> 
> Tested by adding and removing mimetypes via balooctl
> 
> 
> Thanks,
> 
> Varun Joshi
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125585: Remove the Baloo icon

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125585/
---

(Updated Oct. 14, 2015, 11:11 p.m.)


Status
--

This change has been discarded.


Review request for Baloo.


Repository: baloo


Description
---

Breeze ships a far better icon, and this icon was supposed to be
temporary. (Committed on 2014-02-13)


Diffs
-

  CMakeLists.txt 90277fd 
  icons/128-apps-baloo.png 2ca1fa8 
  icons/CMakeLists.txt 59e4260 

Diff: https://git.reviewboard.kde.org/r/125585/diff/


Testing
---


Thanks,

Vishesh Handa


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Review Request 125643: Databases: Use QByteArray::fromRawData when passing data to a codec

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125643/
---

Review request for Baloo and Igor Poboiko.


Repository: baloo


Description
---

All 3 codecs seem to always copy the data on decoding. We can avoid the
extra copy over here by using QByteArray::fromRawData.


Diffs
-

  src/engine/documentdb.cpp 71c2f62 
  src/engine/positiondb.cpp 032192e 
  src/engine/postingdb.cpp 3045d42 

Diff: https://git.reviewboard.kde.org/r/125643/diff/


Testing
---


Thanks,

Vishesh Handa


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Baloo patches for review

[Vishesh Handa]

Hey Pinak

Could you please take a quick look at these patches.

I should use reviewboard but it's a pain to upload all these patches.
Specially since most of these are so trivial.

--
Vishesh Handa
From 8637cf5d3555039970e473fe1aa259e0fcc69e70 Mon Sep 17 00:00:00 2001
From: Vishesh Handa <m...@vhanda.in>
Date: Sun, 4 Oct 2015 23:49:57 +0200
Subject: [PATCH 1/8] WriteTransaction: Extra asserts in replaceDocument

---
 src/engine/writetransaction.cpp | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/engine/writetransaction.cpp b/src/engine/writetransaction.cpp
index 1391ba9..f1ff1ad 100644
--- a/src/engine/writetransaction.cpp
+++ b/src/engine/writetransaction.cpp
@@ -205,6 +205,9 @@ void WriteTransaction::replaceDocument(const Document& doc, DocumentOperations o
 }
 
 if (operations & DocumentTime) {
+Q_ASSERT(doc.m_mTime);
+Q_ASSERT(doc.m_cTime);
+
 DocumentTimeDB::TimeInfo info;
 info.mTime = doc.m_mTime;
 info.cTime = doc.m_cTime;
-- 
2.5.0

From 63224dc57a8dbe151ac1ff23e93190a1bd6c387c Mon Sep 17 00:00:00 2001
From: Vishesh Handa <m...@vhanda.in>
Date: Mon, 5 Oct 2015 12:01:28 +0200
Subject: [PATCH 3/8] FileIndexScheduler: Forcibly kill threads on exit

This is not good as we are killing before committing, but as a short
term solution, this works. We no longer block when trying to exit.
---
 src/file/fileindexscheduler.cpp | 5 +
 src/file/fileindexscheduler.h   | 1 +
 2 files changed, 6 insertions(+)

diff --git a/src/file/fileindexscheduler.cpp b/src/file/fileindexscheduler.cpp
index e11656f..28cf6db 100644
--- a/src/file/fileindexscheduler.cpp
+++ b/src/file/fileindexscheduler.cpp
@@ -64,6 +64,11 @@ FileIndexScheduler::FileIndexScheduler(Database* db, FileIndexerConfig* config,
  this, QDBusConnection::ExportScriptableContents);
 }
 
+FileIndexScheduler::~FileIndexScheduler()
+{
+m_threadPool.waitForDone(0); // wait 0 msecs
+}
+
 void FileIndexScheduler::scheduleIndexing()
 {
 if (m_threadPool.activeThreadCount() || m_indexerState == Suspended) {
diff --git a/src/file/fileindexscheduler.h b/src/file/fileindexscheduler.h
index 6292a85..0749f81 100644
--- a/src/file/fileindexscheduler.h
+++ b/src/file/fileindexscheduler.h
@@ -44,6 +44,7 @@ class FileIndexScheduler : public QObject
 Q_PROPERTY(int state READ state NOTIFY stateChanged)
 public:
 FileIndexScheduler(Database* db, FileIndexerConfig* config, QObject* parent = 0);
+~FileIndexScheduler() Q_DECL_OVERRIDE;
 int state() const { return m_indexerState; }
 
 Q_SIGNALS:
-- 
2.5.0

From 745893195c9deaf16be6b432320bee0c6e8905de Mon Sep 17 00:00:00 2001
From: Vishesh Handa <m...@vhanda.in>
Date: Mon, 5 Oct 2015 00:56:38 +0200
Subject: [PATCH 2/8] WriteTransaction commit: Avoid fetching the positionList
 unless required

The postingList is always required, but that is not the case for the
positionList. Lets avoid fetching it unless it is absolutely required.
---
 src/engine/writetransaction.cpp | 22 +-
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/src/engine/writetransaction.cpp b/src/engine/writetransaction.cpp
index f1ff1ad..ae27ad9 100644
--- a/src/engine/writetransaction.cpp
+++ b/src/engine/writetransaction.cpp
@@ -268,7 +268,9 @@ void WriteTransaction::commit()
 const QVector operations = iter.value();
 
 PostingList list = postingDB.get(term);
-QVector positionList = positionDB.get(term); // FIXME: We do not need to fetch this for all the terms
+
+bool fetchedPositionList = false;
+QVector positionList;
 
 for (const Operation& op : operations) {
 quint64 id = op.data.docId;
@@ -277,11 +279,19 @@ void WriteTransaction::commit()
 insert(list, id);
 
 if (!op.data.positions.isEmpty()) {
+if (!fetchedPositionList) {
+positionList = positionDB.get(term);
+fetchedPositionList = true;
+}
 insert(positionList, op.data);
 }
 }
 else {
 list.removeOne(id);
+if (!fetchedPositionList) {
+positionList = positionDB.get(term);
+fetchedPositionList = true;
+}
 positionList.removeOne(PositionInfo(id));
 }
 }
@@ -292,10 +302,12 @@ void WriteTransaction::commit()
 postingDB.del(term);
 }
 
-if (!positionList.isEmpty()) {
-positionDB.put(term, positionList);
-} else {
-positionDB.del(term);
+if (fetchedPositionList) {
+if (!positionList.isEmpty()) {
+positionDB.put(term, positionList);
+} else {
+positionDB.del(term);
+}
 }
 }
 
-- 
2.5.0

From ead963911d70654ed87d6

Review Request 125585: Remove the Baloo icon

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125585/
---

Review request for Baloo.


Repository: baloo


Description
---

Breeze ships a far better icon, and this icon was supposed to be
temporary. (Committed on 2014-02-13)


Diffs
-

  CMakeLists.txt 90277fd 
  icons/128-apps-baloo.png 2ca1fa8 
  icons/CMakeLists.txt 59e4260 

Diff: https://git.reviewboard.kde.org/r/125585/diff/


Testing
---


Thanks,

Vishesh Handa


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125544: Fix limit/offset handling when sort option is no sort.

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125544/#review86535
---


I'm adding the release team since maybe we can delay the release of Baloo or 
make a special bug fix release with this patch. Shipping Baloo with a broken 
kioslave is not good.


src/lib/searchstore.cpp (line 103)
<https://git.reviewboard.kde.org/r/125544/#comment59586>

Say we have 50 results

limit = 10
offset = 5

So we want results from [5, 14]

With this code -
for (i = 5; i < 10; i++)

We we would get [5, 10). Could you please fix this? I think a simple `i < 
limit + offset` would do.



src/lib/searchstore.cpp (line 114)
<https://git.reviewboard.kde.org/r/125544/#comment59587>

The entire code in this else case is absolutely rubbish. The person 
implementing it should feel bad (me).

We seem to be completly ignoring the offset over here.


- Vishesh Handa


On Oct. 7, 2015, 10:27 a.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125544/
> ---
> 
> (Updated Oct. 7, 2015, 10:27 a.m.)
> 
> 
> Review request for Baloo and Vishesh Handa.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> The bug is obvious, old code uses limit instead of (limit > 0), which 
> gracefully handles limit < 0 case (means everything).
> 
> After offset is introduced, limit < 0 case is not handled properly.
> 
> This patch contains following changes:
> 1. make offset uint, m_offset is uint in Query already.
> 2. handle limit < 0 case properly.
> 
> 
> Diffs
> -
> 
>   src/lib/searchstore.h 17b4263 
>   src/lib/searchstore.cpp a2e7749 
> 
> Diff: https://git.reviewboard.kde.org/r/125544/diff/
> 
> 
> Testing
> ---
> 
> baloosearch:/ doesn't return anything because of this, now it properly 
> returns result.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125544: Fix limit/offset handling when sort option is no sort.

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125544/#review86564
---

Ship it!


Awesome work. Thanks a lot for taking care of this.

- Vishesh Handa


On Oct. 9, 2015, 5:54 p.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125544/
> ---
> 
> (Updated Oct. 9, 2015, 5:54 p.m.)
> 
> 
> Review request for Baloo, Release Team and Vishesh Handa.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> The bug is obvious, old code uses limit instead of (limit > 0), which 
> gracefully handles limit < 0 case (means everything).
> 
> After offset is introduced, limit < 0 case is not handled properly.
> 
> This patch contains following changes:
> 1. make offset uint, m_offset is uint in Query already.
> 2. handle limit < 0 case properly.
> 
> Also fix offset with sorting.
> 
> 
> Diffs
> -
> 
>   src/lib/searchstore.h 17b4263 
>   src/lib/searchstore.cpp a2e7749 
> 
> Diff: https://git.reviewboard.kde.org/r/125544/diff/
> 
> 
> Testing
> ---
> 
> baloosearch:/ doesn't return anything because of this, now it properly 
> returns result.
> 
> Play with baloosearch command a little bit, offset and limit returns the 
> [offset, offset + limit) and no crash on the border case.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 119890: [Ark] Implement "Add to archive" option when dragging and dropping onto an archive file in dolphin

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119890/#review86362
---

Ship it!



app/addToArchiveDndPlugin.cpp (line 38)
<https://git.reviewboard.kde.org/r/119890/#comment59486>

Empty new line. Plus this entire function is idented in a strange way.



app/addToArchiveDndPlugin.cpp (line 44)
<https://git.reviewboard.kde.org/r/119890/#comment59489>

const KUrl& i



app/addToArchiveDndPlugin.cpp (line 59)
<https://git.reviewboard.kde.org/r/119890/#comment59487>

Extra indent over here.



app/addToArchiveDndPlugin.cpp (line 70)
<https://git.reviewboard.kde.org/r/119890/#comment59488>

The * is next to the type in the rest of the file. `KAction* action` 
instead of `KAction *action`.


- Vishesh Handa


On Aug. 21, 2014, 9:11 p.m., Arjun AK wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119890/
> ---
> 
> (Updated Aug. 21, 2014, 9:11 p.m.)
> 
> 
> Review request for KDE Base Apps and KDE Utils.
> 
> 
> Bugs: 338414
> http://bugs.kde.org/show_bug.cgi?id=338414
> 
> 
> Repository: ark
> 
> 
> Description
> ---
> 
> This patch implements the "Add to archive" option, which is shown when a user 
> drags and drops files onto an existing archive. 
> 
> 
> See also:
> https://git.reviewboard.kde.org/r/119892
> https://bugs.kde.org/show_bug.cgi?id=338414
> 
> 
> Diffs
> -
> 
>   app/ark_dndaddtoarchive.desktop.cmake PRE-CREATION 
>   app/CMakeLists.txt f1ef01b 
>   app/addToArchiveDndPlugin.h PRE-CREATION 
>   app/addToArchiveDndPlugin.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119890/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Arjun AK
> 
>

Re: Baloo Development for 5.16

[Vishesh Handa]

Hey Stefan

On Sun, Oct 4, 2015 at 10:46 PM, Stefan Bruens
<stefan.bru...@rwth-aachen.de> wrote:
> Wherever it ends up, one more item for the Todo/Idea list:
> - Samba 4.3 as a search store
>   Baloo could access the Spotlight API provided by Samba to allow fast
> searching of content stored on a file server. See [1] for some info.
>

I looked into this a little and it seems that there isn't an API for
us to access. Tracker (Gnome) directly integrates with Samba and has
an optional compile time dependency.

Also I'm quite confused about this whole thing. Spotlight is an osx
only thing. I'm guessing they are trying to use an equivalent system
on linux and have settled on Tracker? The link you mentioned seems to
indicate that they are using Tracker on OSX. Maybe I'm just confused.

--
Vishesh Handa

>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Baloo Development for 5.16

[Vishesh Handa]

I've been making a list of things that would be awesome if we could
get done for 5.16. Maybe others can add to this list and then we have
concrete goals for this month.

From last month -
* https://bugs.kde.org/show_bug.cgi?id=353010

Balooctl Monitor
* Kill monitor when baloo_file dies
* Notify start / end of the file
* Translate strings
* Polish UI (too abstract?)

PostingDB + PositionDB
* get should return an iterator instead of a vector. This way we avoid
the extra lookup when doing a set. We could just use the iterator.

Document
* Make it more generic - Remove fileNameTerms and fileXattr terms.
Instead we can offer fields such as addTerm(fieldNum, ...). From the
DB side those fields are generated as required. Akonadi wants to use
Baloo's engine, and we need to slowly make it more generic.
* Remove the URL code from the document. We can modify the URL
directly from the Transaction.

Misc
* Remove KIO dependency - Move it into Plasma.
* Index Scheduler - Run malloc_trim when idle
* Improve documentation - specially the FAQ.

Anyone wants to add specific things? I'm scared this is already quite
long for one month of development. Well, it depends on how much time
we have to hack on Baloo.

Also, should we put this list on todo.kde.org or start using Phabricator?

--
Vishesh Handa

>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125481: Port balooctl check to work with new architecture

[Vishesh Handa]

> On Oct. 2, 2015, 1:28 p.m., Vishesh Handa wrote:
> > src/file/unindexedfileindexer.cpp, line 59
> > <https://git.reviewboard.kde.org/r/125481/diff/1/?file=409378#file409378line59>
> >
> > Are you sure this is the right thing to be doing? I'm imagining the 
> > case when the filename has changed when `baloo_file` was not running.
> 
> Pinak Ahuja wrote:
> Ah, yes I didn't account for that. Replacing the FilenameTerms along with 
> the DocumentTime should handle that case right?

Yes. I think so.

The mimetype could have also changed, but that will eventually be updated by 
the ContentIndexer.


- Vishesh


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125481/#review86228
---


On Oct. 2, 2015, 10:44 a.m., Pinak Ahuja wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125481/
> ---
> 
> (Updated Oct. 2, 2015, 10:44 a.m.)
> 
> 
> Review request for Baloo and Vishesh Handa.
> 
> 
> Bugs: 353011
> http://bugs.kde.org/show_bug.cgi?id=353011
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> Add a new Runnable which checks for unindexed files and indexes them.
> 
> 
> Diffs
> -
> 
>   src/file/CMakeLists.txt 306a211 
>   src/file/fileindexscheduler.h 1479063 
>   src/file/fileindexscheduler.cpp 5bb55ea 
>   src/file/indexerstate.h 8032280 
>   src/file/unindexedfileindexer.h PRE-CREATION 
>   src/file/unindexedfileindexer.cpp PRE-CREATION 
>   src/tools/balooctl/main.cpp 305c82b 
> 
> Diff: https://git.reviewboard.kde.org/r/125481/diff/
> 
> 
> Testing
> ---
> 
> Manual testing: created a new file with baloo_file killed. Ran baloo_file and 
> then balooctl check, new file was indexed.
> 
> 
> Thanks,
> 
> Pinak Ahuja
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125481: Port balooctl check to work with new architecture

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125481/#review86228
---



src/file/unindexedfileindexer.cpp (line 59)
<https://git.reviewboard.kde.org/r/125481/#comment59433>

Are you sure this is the right thing to be doing? I'm imagining the case 
when the filename has changed when `baloo_file` was not running.


- Vishesh Handa


On Oct. 2, 2015, 10:44 a.m., Pinak Ahuja wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125481/
> ---
> 
> (Updated Oct. 2, 2015, 10:44 a.m.)
> 
> 
> Review request for Baloo and Vishesh Handa.
> 
> 
> Bugs: 353011
> http://bugs.kde.org/show_bug.cgi?id=353011
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> Add a new Runnable which checks for unindexed files and indexes them.
> 
> 
> Diffs
> -
> 
>   src/file/CMakeLists.txt 306a211 
>   src/file/fileindexscheduler.h 1479063 
>   src/file/fileindexscheduler.cpp 5bb55ea 
>   src/file/indexerstate.h 8032280 
>   src/file/unindexedfileindexer.h PRE-CREATION 
>   src/file/unindexedfileindexer.cpp PRE-CREATION 
>   src/tools/balooctl/main.cpp 305c82b 
> 
> Diff: https://git.reviewboard.kde.org/r/125481/diff/
> 
> 
> Testing
> ---
> 
> Manual testing: created a new file with baloo_file killed. Ran baloo_file and 
> then balooctl check, new file was indexed.
> 
> 
> Thanks,
> 
> Pinak Ahuja
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125482: balooctl config: add options to set/view onlyBasicIndexing

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125482/#review86245
---



src/tools/balooctl/configcommand.cpp (line 86)
<https://git.reviewboard.kde.org/r/125482/#comment59443>

whether not weather

Or you could use 'if'.



src/tools/balooctl/configcommand.cpp (line 107)
<https://git.reviewboard.kde.org/r/125482/#comment59441>

I'm not sure if we should translate yes/no since we do not accept the 
translated version as input.



src/tools/balooctl/configcommand.cpp (line 366)
<https://git.reviewboard.kde.org/r/125482/#comment59442>

Could you please split this into its own if block and not combine it with 
'hidden'. There will be a little bit of duplicaiton, but it will be much easier 
to understand.


- Vishesh Handa


On Oct. 2, 2015, 2 p.m., Pinak Ahuja wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125482/
> ---
> 
> (Updated Oct. 2, 2015, 2 p.m.)
> 
> 
> Review request for Baloo and Vishesh Handa.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> syntax: balooctl config [set|ls] contentIndexing (yes/no)
> 
> 
> Diffs
> -
> 
>   src/lib/indexerconfig.h 98b51a8 
>   src/lib/indexerconfig.cpp 86094da 
>   src/tools/balooctl/configcommand.cpp 91f31c2 
> 
> Diff: https://git.reviewboard.kde.org/r/125482/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Pinak Ahuja
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125481: Port balooctl check to work with new architecture

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125481/#review86244
---

Ship it!



src/file/unindexedfileiterator.cpp (line 97)
<https://git.reviewboard.kde.org/r/125481/#comment59437>

In the future lets implement a function to fetch both the ctime and mtime 
in one go. They are internally save in the same btree, so it would be more 
efficient.



src/file/unindexedfileiterator.cpp (line 101)
<https://git.reviewboard.kde.org/r/125481/#comment59438>

Maybe we should update UnindexerFileIterator to tell us what parts need to 
be indexed. We're indexing the xattr each time, even though they might not have 
changed. Ditto with filename.

Anyway, this entire process is a rare case and it makes more sense to 
optimize for the common ones.


- Vishesh Handa


On Oct. 2, 2015, 2:57 p.m., Pinak Ahuja wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125481/
> ---
> 
> (Updated Oct. 2, 2015, 2:57 p.m.)
> 
> 
> Review request for Baloo and Vishesh Handa.
> 
> 
> Bugs: 353011
> http://bugs.kde.org/show_bug.cgi?id=353011
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> Add a new Runnable which checks for unindexed files and indexes them.
> 
> 
> Diffs
> -
> 
>   src/file/CMakeLists.txt 306a211 
>   src/file/fileindexscheduler.h 1479063 
>   src/file/fileindexscheduler.cpp 5bb55ea 
>   src/file/indexerstate.h 8032280 
>   src/file/unindexedfileindexer.h PRE-CREATION 
>   src/file/unindexedfileindexer.cpp PRE-CREATION 
>   src/file/unindexedfileiterator.cpp bb54da3 
>   src/tools/balooctl/main.cpp 305c82b 
> 
> Diff: https://git.reviewboard.kde.org/r/125481/diff/
> 
> 
> Testing
> ---
> 
> Manual testing: created a new file with baloo_file killed. Ran baloo_file and 
> then balooctl check, new file was indexed.
> 
> 
> Thanks,
> 
> Pinak Ahuja
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125496: i18n fixes for balooctl config

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125496/#review86265
---

Ship it!


- Vishesh Handa


On Oct. 2, 2015, 9:29 p.m., Luigi Toscano wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125496/
> ---
> 
> (Updated Oct. 2, 2015, 9:29 p.m.)
> 
> 
> Review request for Baloo, Pinak Ahuja and Vishesh Handa.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> - add missing i18n translation calls;
> - use i18n arguments instead of word puzzles;
> - more consistent wording;
> 
> 
> A bit late in the cycle close the release, at least for the 3rd point; 
> partially the 2nd as the messages landed quite late already, but at least the 
> first point should be fine.
> 
> 
> Diffs
> -
> 
>   src/tools/balooctl/configcommand.cpp dbda4d1 
> 
> Diff: https://git.reviewboard.kde.org/r/125496/diff/
> 
> 
> Testing
> ---
> 
> Compiles, the strings are properly shown with the right arguments
> 
> 
> Thanks,
> 
> Luigi Toscano
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125430: FileContentIndexer: Fix threading issues

[Vishesh Handa]

> On Sept. 29, 2015, 3:10 p.m., Pinak Ahuja wrote:
> > Oh wait, I don't think this is needed all slots that modify m_monitors run 
> > in the creation/main thread also m_currentFile is updated in the 
> > slotIndexingFile and read in the QProperty getter (I should probably make 
> > it const) both of which should run in the creation/main thread.

My bad. I didn't realize slotIndexingFile is called from the main thread and 
not from the content indexer's thread. This patch is not required.


- Vishesh


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125430/#review86118
---


On Sept. 27, 2015, 9:48 p.m., Vishesh Handa wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125430/
> ---
> 
> (Updated Sept. 27, 2015, 9:48 p.m.)
> 
> 
> Review request for Baloo.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> See diff
> 
> 
> Diffs
> -
> 
>   src/file/filecontentindexer.h 6ecd41c 
>   src/file/filecontentindexer.cpp 4323c3f 
> 
> Diff: https://git.reviewboard.kde.org/r/125430/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vishesh Handa
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125424: Balooctl: Add checkDb command

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125424/
---

(Updated Sept. 30, 2015, 4:41 p.m.)


Status
--

This change has been marked as submitted.


Review request for Baloo, Pinak Ahuja and Igor Poboiko.


Changes
---

Submitted with commit 9da558f643eaac1e6c37aba5a2ff1dfe2c84bf98 by Vishesh Handa 
to branch master.


Repository: baloo


Description
---

Balooctl: Add checkDb command

This command can be used by developers to check if their PostingDb and
DocumentUrlDB is in a valid state. We have a few bugs caused because all
the databases are not correctly updated. This will help us diagnose this
better.

I am trying to diagnose the bug revealed by review 125362. Still not entirely 
sure.


Diffs
-

  src/engine/transaction.h 845d9f0 
  src/engine/transaction.cpp 8e9700c 
  src/tools/balooctl/main.cpp 0e74761 

Diff: https://git.reviewboard.kde.org/r/125424/diff/


Testing
---


Thanks,

Vishesh Handa


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125429: Baloo: Fix dbus warnings

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125429/
---

(Updated Sept. 30, 2015, 4:41 p.m.)


Status
--

This change has been marked as submitted.


Review request for Baloo and Pinak Ahuja.


Changes
---

Submitted with commit 3905929f5f8be6dbe23c1e02ebd093e1608245a3 by Vishesh Handa 
to branch master.


Repository: baloo


Description
---

Fix dbus warnings

We cannot emit dbus signals from a different thread than the thread the
object has an affinity to.


Diffs
-

  src/file/filecontentindexer.cpp 4323c3f 

Diff: https://git.reviewboard.kde.org/r/125429/diff/


Testing
---


Thanks,

Vishesh Handa


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125429: Baloo: Fix dbus warnings

[Vishesh Handa]

> On Sept. 29, 2015, 2:24 p.m., Pinak Ahuja wrote:
> > src/file/filecontentindexer.cpp, line 84
> > <https://git.reviewboard.kde.org/r/125429/diff/1/?file=408580#file408580line84>
> >
> > Wouldn't this slot be executed in the main thread? We most probably 
> > don't need this.

I'm confused. We do need it since dbus signals need to be emitted in the main 
thread.


- Vishesh


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125429/#review86113
---


On Sept. 30, 2015, 4:41 p.m., Vishesh Handa wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125429/
> ---
> 
> (Updated Sept. 30, 2015, 4:41 p.m.)
> 
> 
> Review request for Baloo and Pinak Ahuja.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> Fix dbus warnings
> 
> We cannot emit dbus signals from a different thread than the thread the
> object has an affinity to.
> 
> 
> Diffs
> -
> 
>   src/file/filecontentindexer.cpp 4323c3f 
> 
> Diff: https://git.reviewboard.kde.org/r/125429/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vishesh Handa
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125430: FileContentIndexer: Fix threading issues

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125430/
---

(Updated Sept. 30, 2015, 4:41 p.m.)


Status
--

This change has been discarded.


Review request for Baloo.


Repository: baloo


Description
---

See diff


Diffs
-

  src/file/filecontentindexer.h 6ecd41c 
  src/file/filecontentindexer.cpp 4323c3f 

Diff: https://git.reviewboard.kde.org/r/125430/diff/


Testing
---


Thanks,

Vishesh Handa


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125431: WriteTransaction: Fix memory corruption issue

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125431/
---

(Updated Sept. 28, 2015, 10:10 p.m.)


Status
--

This change has been discarded.


Review request for Baloo.


Repository: baloo


Description
---

WriteTransaction: Fix memory corruption issue

In both these functions the QByteArrays are allocated from
QByteArray::fromRawData so that we avoid copies. They come from the LMDB
db which guarentees the memory will not be discarded as long as we are
using it. Unfortunately for us, we mark it for remove in removeTerms and
replaceTerms after this call, so LMDB is free to reuse that memory, and
it probably does.

We should ideally replace QByteArray with our ByteArray class.

This does not fix all our issues. We still have problems.


Diffs
-

  src/engine/writetransaction.cpp 1391ba9 

Diff: https://git.reviewboard.kde.org/r/125431/diff/


Testing
---


Thanks,

Vishesh Handa


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125362: Avoid QByteArray::fromRawData

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125362/#review86076
---

Ship it!


The more I think I about this, the more it makes sense.

I managed to fix one of corruption issues with 
[this](https://git.reviewboard.kde.org/r/125431/), but who knows how many more 
exist. The proper solution is to write our own class where the copy / move 
semantics are cleared. Also most of our strings are quite small and QByteArray 
does not have any small string optimizations.

Ship it!

- Vishesh Handa


On Sept. 23, 2015, 2:42 p.m., Igor Poboiko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125362/
> ---
> 
> (Updated Sept. 23, 2015, 2:42 p.m.)
> 
> 
> Review request for Baloo.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> I've noted following bug: sometypes e.g "PDF" (T5) files had a "Folder" (T9) 
> type in KRunner.
> "balooshow -x" showed that it has a "T5", while "baloosearch --type folder" 
> still listed it.
> 
>  * Debugging showed that it appears somewhere in WritingTransaction::commit()
>  * There wasn't any WritingTransaction::m_pendingOperations["T9"] access at 
> all
>  * This hash contained "T9" key (QHash::keys().contains("T9") == true), but 
> it didn't (QHash::contains("T9") == false and QHash::count("T9") == 0)
>  * Because of that QHashIterator fails miserably iterating over non-existing 
> values (e.g iter.value() returns some value with some data for that 
> non-existing values)
>  * Bisection showed that QHash got corrupted at "documentTermsDB.put(id, 
> docTerms)" (engine/writingtransaction.cpp:185), to be specific - on mdb_put() 
> line
> 
> That was the bug itself. The problem is that QByteArray::fromRawData() is 
> used everywhere, which does not copy data from DB but just stores a pointer 
> to some place in memory-mapped file in DB. And it doesn't know if data where 
> it points to changed, leading to undefined behavior like that.
> 
> This patch removes ::fromRawData() calls replacing it by copy-constructors. 
> (maybe somewhere we can leave it, but I'm not sure it's a optimization we 
> should care about)
> 
> 
> Diffs
> -
> 
>   src/codecs/doctermscodec.cpp e8801f9 
>   src/codecs/postingcodec.cpp 1edb645 
>   src/engine/documentdatadb.cpp 690df70 
>   src/engine/documentdb.cpp ea0cb66 
>   src/engine/idfilenamedb.cpp d4e1eb1 
>   src/engine/positiondb.cpp 568dc54 
>   src/engine/postingdb.cpp e183db5 
> 
> Diff: https://git.reviewboard.kde.org/r/125362/diff/
> 
> 
> Testing
> ---
> 
> After applying this patch I have no more files in wrong category, so the 
> issue is gone.
> 
> 
> Thanks,
> 
> Igor Poboiko
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125431: WriteTransaction: Fix memory corruption issue

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125431/#review86077
---


I'm discarding this patch as review 125362 does a better job at fixing all the 
issues.

- Vishesh Handa


On Sept. 27, 2015, 9:50 p.m., Vishesh Handa wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125431/
> ---
> 
> (Updated Sept. 27, 2015, 9:50 p.m.)
> 
> 
> Review request for Baloo.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> WriteTransaction: Fix memory corruption issue
> 
> In both these functions the QByteArrays are allocated from
> QByteArray::fromRawData so that we avoid copies. They come from the LMDB
> db which guarentees the memory will not be discarded as long as we are
> using it. Unfortunately for us, we mark it for remove in removeTerms and
> replaceTerms after this call, so LMDB is free to reuse that memory, and
> it probably does.
> 
> We should ideally replace QByteArray with our ByteArray class.
> 
> This does not fix all our issues. We still have problems.
> 
> 
> Diffs
> -
> 
>   src/engine/writetransaction.cpp 1391ba9 
> 
> Diff: https://git.reviewboard.kde.org/r/125431/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vishesh Handa
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 120393: [kdelibs4support] Kill dead code

[Vishesh Handa]

> On May 26, 2015, 8:05 p.m., Hrvoje Senjan wrote:
> > Ok. Will update diff to kill nepomuk only. What about forwarding headers? 
> > They are still installed

Ping. What's the status on this?


- Vishesh


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120393/#review80866
---


On March 18, 2015, 6:24 p.m., Hrvoje Senjan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120393/
> ---
> 
> (Updated March 18, 2015, 6:24 p.m.)
> 
> 
> Review request for KDE Frameworks, David Faure and Vishesh Handa.
> 
> 
> Repository: kdelibs4support
> 
> 
> Description
> ---
> 
> Strigi check has been removed in commit 
> c8f4c69650c71276b2a2263212addde63764e58b, and soprano wasn't even ported to 
> Qt5 (afaik), so this was never compiled.
> 
> 
> Diffs
> -
> 
>   autotests/kfilemetainfotest.cpp c751cdd 
>   src/CMakeLists.txt b662893 
>   src/config-kdelibs4support.h.cmake 1af3ee0 
>   src/kio/kfilemetadataconfigurationwidget.cpp 259b205 
>   src/kio/kfilemetadataprovider.cpp 3468546 
>   src/kio/kfilemetadataprovider_p.h 31137b2 
>   src/kio/kfilemetadatawidget.cpp 1edb069 
>   src/kio/kfilemetainfo.cpp eae1295 
>   src/kio/kfilemetainfoitem.cpp 62f760d 
>   src/kio/kfilemetainfoitem_p.h 8929e46 
>   src/kio/knfotranslator.cpp 8eec6a1 
> 
> Diff: https://git.reviewboard.kde.org/r/120393/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Hrvoje Senjan
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 125424: Balooctl: Add checkDb command

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125424/
---

Review request for Baloo, Pinak Ahuja and Igor Poboiko.


Repository: baloo


Description
---

Balooctl: Add checkDb command

This command can be used by developers to check if their PostingDb and
DocumentUrlDB is in a valid state. We have a few bugs caused because all
the databases are not correctly updated. This will help us diagnose this
better.

I am trying to diagnose the bug revealed by review 125362. Still not entirely 
sure.


Diffs
-

  src/engine/transaction.h 845d9f0 
  src/engine/transaction.cpp 8e9700c 
  src/tools/balooctl/main.cpp 0e74761 

Diff: https://git.reviewboard.kde.org/r/125424/diff/


Testing
---


Thanks,

Vishesh Handa


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Review Request 125430: FileContentIndexer: Fix threading issues

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125430/
---

Review request for Baloo.


Repository: baloo


Description
---

See diff


Diffs
-

  src/file/filecontentindexer.cpp 4323c3f 

Diff: https://git.reviewboard.kde.org/r/125430/diff/


Testing
---


Thanks,

Vishesh Handa


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Review Request 125429: Baloo: Fix dbus warnings

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125429/
---

Review request for Baloo and Pinak Ahuja.


Repository: baloo


Description
---

Fix dbus warnings

We cannot emit dbus signals from a different thread than the thread the
object has an affinity to.


Diffs
-

  src/file/filecontentindexer.cpp 4323c3f 

Diff: https://git.reviewboard.kde.org/r/125429/diff/


Testing
---


Thanks,

Vishesh Handa


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125430: FileContentIndexer: Fix threading issues

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125430/
---

(Updated Sept. 27, 2015, 9:48 p.m.)


Review request for Baloo.


Repository: baloo


Description
---

See diff


Diffs (updated)
-

  src/file/filecontentindexer.h 6ecd41c 
  src/file/filecontentindexer.cpp 4323c3f 

Diff: https://git.reviewboard.kde.org/r/125430/diff/


Testing
---


Thanks,

Vishesh Handa


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Review Request 125431: WriteTransaction: Fix memory corruption issue

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125431/
---

Review request for Baloo.


Repository: baloo


Description
---

WriteTransaction: Fix memory corruption issue

In both these functions the QByteArrays are allocated from
QByteArray::fromRawData so that we avoid copies. They come from the LMDB
db which guarentees the memory will not be discarded as long as we are
using it. Unfortunately for us, we mark it for remove in removeTerms and
replaceTerms after this call, so LMDB is free to reuse that memory, and
it probably does.

We should ideally replace QByteArray with our ByteArray class.

This does not fix all our issues. We still have problems.


Diffs
-

  src/engine/writetransaction.cpp 1391ba9 

Diff: https://git.reviewboard.kde.org/r/125431/diff/


Testing
---


Thanks,

Vishesh Handa


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125401: implement windows usermetadata

[Vishesh Handa]

> On Sept. 26, 2015, 10:52 a.m., Vishesh Handa wrote:
> > I cannot really comment on the windows parts. But the ideas seems good. 
> > Ship it?
> 
> Patrick Spendrin wrote:
> Since I moved code around for *nix as well, could you check that it 
> compiles at least (and maybe also works as before)? Otherwise I'll push it 
> tomorrow...

Compiles on Linux, all tests pass, and basic adding, listing and modifying 
xattr seems to work.


- Vishesh


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125401/#review85951
---


On Sept. 27, 2015, 11:17 a.m., Patrick Spendrin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125401/
> ---
> 
> (Updated Sept. 27, 2015, 11:17 a.m.)
> 
> 
> Review request for Baloo and kdewin.
> 
> 
> Repository: kfilemetadata
> 
> 
> Description
> ---
> 
> I implemented all the functions for storing and retrieving user metadata
> using Alternate Data Streams (ADS).
> 
> 
> Diffs
> -
> 
>   src/usermetadata.cpp 28967287d67d3e15a548a4e3fda614568dc932a3 
>   src/extractors/poextractor.h 2929fc1b6e032ceb117efb60ffa05c79b1af0841 
>   src/extractors/poextractor.cpp a521982e4286155e15dc0270892e56c2b0db0cba 
>   src/xattr_p.h 80ea4c7767e5017af92dd5744ed061eedc3c2834 
> 
> Diff: https://git.reviewboard.kde.org/r/125401/diff/
> 
> 
> Testing
> ---
> 
> On Windows; since most of the code is ifdeffed anyway, it shouldn't matter 
> for other platforms
> 
> 
> Thanks,
> 
> Patrick Spendrin
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125424: Balooctl: Add checkDb command

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125424/
---

(Updated Sept. 27, 2015, 11:51 p.m.)


Review request for Baloo, Pinak Ahuja and Igor Poboiko.


Changes
---

1. I didn't land up adding document urls since once can always fetch them via 
'balooshow', but may I still should. It makes the output harder to read.
2. Whoa! This revealed a lot of easily reproducible corruptions


Repository: baloo


Description
---

Balooctl: Add checkDb command

This command can be used by developers to check if their PostingDb and
DocumentUrlDB is in a valid state. We have a few bugs caused because all
the databases are not correctly updated. This will help us diagnose this
better.

I am trying to diagnose the bug revealed by review 125362. Still not entirely 
sure.


Diffs (updated)
-

  src/engine/transaction.h 845d9f0 
  src/engine/transaction.cpp 8e9700c 
  src/tools/balooctl/main.cpp 0e74761 

Diff: https://git.reviewboard.kde.org/r/125424/diff/


Testing
---


Thanks,

Vishesh Handa


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125401: implement windows usermetadata

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125401/#review85951
---


I cannot really comment on the windows parts. But the ideas seems good. Ship it?

- Vishesh Handa


On Sept. 25, 2015, 10:43 p.m., Patrick Spendrin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125401/
> ---
> 
> (Updated Sept. 25, 2015, 10:43 p.m.)
> 
> 
> Review request for Baloo and kdewin.
> 
> 
> Repository: kfilemetadata
> 
> 
> Description
> ---
> 
> I implemented all the functions for storing and retrieving user metadata
> using Alternate Data Streams (ADS).
> 
> 
> Diffs
> -
> 
>   src/extractors/poextractor.h 2929fc1b6e032ceb117efb60ffa05c79b1af0841 
>   src/extractors/poextractor.cpp a521982e4286155e15dc0270892e56c2b0db0cba 
>   src/usermetadata.cpp 28967287d67d3e15a548a4e3fda614568dc932a3 
>   src/xattr_p.h 80ea4c7767e5017af92dd5744ed061eedc3c2834 
> 
> Diff: https://git.reviewboard.kde.org/r/125401/diff/
> 
> 
> Testing
> ---
> 
> On Windows; since most of the code is ifdeffed anyway, it shouldn't matter 
> for other platforms
> 
> 
> Thanks,
> 
> Patrick Spendrin
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125414: balooctl config: Add excludeFilter

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125414/#review85992
---

Ship it!


Awesome work.


src/tools/balooctl/configcommand.cpp (line 201)
<https://git.reviewboard.kde.org/r/125414/#comment59348>

QStringLiteral



src/tools/balooctl/configcommand.cpp (line 326)
<https://git.reviewboard.kde.org/r/125414/#comment59349>

Maybe add a `return 1` after this?


- Vishesh Handa


On Sept. 26, 2015, 7:34 p.m., Pinak Ahuja wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125414/
> ---
> 
> (Updated Sept. 26, 2015, 7:34 p.m.)
> 
> 
> Review request for Baloo and Vishesh Handa.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> Syntax: balooctl config [ls|rm|add] excludeFilters 
> 
> 
> Diffs
> -
> 
>   src/lib/indexerconfig.h b3d265c 
>   src/lib/indexerconfig.cpp 143b6c0 
>   src/tools/balooctl/configcommand.cpp 66eb798 
> 
> Diff: https://git.reviewboard.kde.org/r/125414/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Pinak Ahuja
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125362: Avoid QByteArray::fromRawData

[Vishesh Handa]

> On Sept. 23, 2015, 2:59 p.m., Vishesh Handa wrote:
> > Hmm. This is actually an optimization I care about. 
> > Do you think you could share your `~/.local/share/baloo/index` file. That 
> > way I will be able to reproduce it and see exactly where/why the data is 
> > changing?
> > 
> > Btw, awesome work at diagnosing this! :)
> 
> Igor Poboiko wrote:
> It's not that easy, it takes ~150MB of space. Right now my internet 
> connection won't accept that X_X
> Also I'm not sure you'll be able to reproduce it even having my index, 
> just because for me this bug occurs during index creation (when it actually 
> writes something into DB; btw, isn't it obvious it can change data? or lmdb 
> guarantees it won't?). You'll just be able to see some .pdf-files of type 
> document (in DocumentDB; so it shows correctly in 'balooshow') and of type 
> document (in PostingDB, so it pops up during search)
> 
> But you can ping me on IRC (poboiko at #kde-baloo at freenode), and I 
> will do whatever you want :)
> 
> Vishesh Handa wrote:
> I don't get much IRC time these days :(
> 
> I'm a little confused. `balooshow file` shows a type of T5 (PDF). This 
> means that when the file was indexed, the correct type was stored. However, 
> when calling 'baloosearch type:Folder' the file is in the results. This 
> indicates that it is a problem when searching, and not indexing, no?
> 
> Boudhayan Gupta wrote:
> Given that it's a memory corruption bug, this patch may fix this:
> 
> https://paste.kde.org/pwsj1pbnq
> 
> The stacktrace clearly implicates code that this patch is modifying.
> 
> I cannot test this, however. This bug isn't mine; IRC nick "genstorm" 
> reported this.
> 
> Igor Poboiko wrote:
> Vishesh: as far as I understood, you have several indexes. 
>  * PostingDB, where keys are search terms and values are documents (e.g. 
> key: "T5", value: "123123123", id of "file.pdf")
>  * DocumentDB, where keys are documents and value are list of terms (e.g. 
> key: 123123123, value: "T9")
> They are used differently:
>  * First is used by "baloosearch" for searching and is being populated by 
> "baloo_file" indexer
>  * Second is used by "balooshow" and is being populated by 
> "baloo_file_extractor", which is called by "baloo_file"
>  
>  What I've discovered is that extractor fails (like I've explained), 
> leading to inconsistent DB, such as type "T5" in first DB and "T9" in second 
> DB, because of that memory corruption thing.

You're completely right. And it makese sense.

Gimme a day or two, I want to run a few tests, but if we cannot figure it out, 
maybe we should ship this for 5.15. It's better to loose on performance than 
have crashes and incorrect values.


- Vishesh


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125362/#review85830
---


On Sept. 23, 2015, 2:42 p.m., Igor Poboiko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125362/
> ---
> 
> (Updated Sept. 23, 2015, 2:42 p.m.)
> 
> 
> Review request for Baloo.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> I've noted following bug: sometypes e.g "PDF" (T5) files had a "Folder" (T9) 
> type in KRunner.
> "balooshow -x" showed that it has a "T5", while "baloosearch --type folder" 
> still listed it.
> 
>  * Debugging showed that it appears somewhere in WritingTransaction::commit()
>  * There wasn't any WritingTransaction::m_pendingOperations["T9"] access at 
> all
>  * This hash contained "T9" key (QHash::keys().contains("T9") == true), but 
> it didn't (QHash::contains("T9") == false and QHash::count("T9") == 0)
>  * Because of that QHashIterator fails miserably iterating over non-existing 
> values (e.g iter.value() returns some value with some data for that 
> non-existing values)
>  * Bisection showed that QHash got corrupted at "documentTermsDB.put(id, 
> docTerms)" (engine/writingtransaction.cpp:185), to be specific - on mdb_put() 
> line
> 
> That was the bug itself. The problem is that QByteArray::fromRawData() is 
> used everywhere, which does not copy data from DB but just stores a pointer 
> to 

Re: Review Request 125365: Do not ignore subterms if not found

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125365/
---

(Updated Sept. 24, 2015, 11:45 p.m.)


Status
--

This change has been marked as submitted.


Review request for Baloo, Pinak Ahuja and Igor Poboiko.


Changes
---

Submitted with commit 810820f8ac6a7a025b083f3e88d96851938958d5 by Vishesh Handa 
to branch master.


Repository: baloo


Description
---

When composing different PostingIterators in an AND / OR / PHRASE
combination, we should not ignore the null posting iterators. In the
case of AND that clearly implies that there is no result.

We were just ignoring them and therefore returning invalid results.


Diffs
-

  autotests/unit/engine/andpostingiteratortest.cpp 9a71ae4 
  autotests/unit/engine/orpostingiteratortest.cpp df0bd77 
  autotests/unit/engine/phraseanditeratortest.cpp 6465a1e 
  src/engine/andpostingiterator.h d8aadec 
  src/engine/andpostingiterator.cpp 99b0b9b 
  src/engine/orpostingiterator.cpp 9d16349 
  src/engine/phraseanditerator.h 1f5573e 
  src/engine/phraseanditerator.cpp d00cdcf 
  src/engine/transaction.cpp ad43d69 
  src/lib/searchstore.cpp 806ea5f 

Diff: https://git.reviewboard.kde.org/r/125365/diff/


Testing
---


Thanks,

Vishesh Handa


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<

Re: Review Request 125374: TagListJob: Emit error when failed to open database

[Vishesh Handa]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125374/#review85903
---

Ship it!


- Vishesh Handa


On Sept. 24, 2015, 2:32 p.m., David Rosca wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125374/
> ---
> 
> (Updated Sept. 24, 2015, 2:32 p.m.)
> 
> 
> Review request for Baloo and Vishesh Handa.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> Emit job result when database can't be opened, so TagListJob::exec() won't 
> hang.
> 
> 
> Diffs
> -
> 
>   src/lib/taglistjob.cpp cb9d94e 
> 
> Diff: https://git.reviewboard.kde.org/r/125374/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Rosca
> 
>


>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<