Re: CI system maintainability

[Boudhayan Gupta]

Hi,

On Thu, 28 Mar 2019 at 15:21, Kevin Ottens  wrote:

> Hello,
>
> On Thursday, 28 March 2019 14:33:59 CET laurent Montel wrote:
> > I am against to force mandatory review, as it will create a lot of lose
> of
> > time,
>
> As I said, unpopular.
>

I don't get why mandatory code reviews are so unpopular.

I don't care if you lose time. I don't want the guys building my house to
cut corners mixing my concrete because it's going to save time. Why are you
in such a massive hurry to make changes to software which for example holds
access to my Google Account password? In fact, the very fact that you make
this argument makes me wonder if I'm running trustable code on my computer
at all, because apparently doing it quickly is far more important than
doing it right.

As a user, I simply do not want unreviewed crap running on my computer.
Yes, crap, because no software engineer writes bug-free code all the time,
and if you're so overconfident that you don't need reviews on even your
one-liners, you're probably too overconfident to be writing good code
anyway, so I'm going to operate on the presumption that if the code hasn't
had more than one pair of eyeballs ever looking at it, it's crap.

As a developer, I know that even one-liners, and especially one-liners, the
sort where you think "meh, this is a tiny little thing, I don't have to be
careful" are the ones that have the most dangerous typos and unintended
bugs. Reviews catch that.

In a project like PIM, if the code hasn't been through review, which
independent party do I trust to verify that you're not, for example,
leaking my Google password to some world-readable tempfile? Do you really
expect every user to read the entire codebase for themselves and make sure
that's not being done? The whole point of having all the code out in the
open for independent audit purposes, to protect your security and privacy
and what not is completely moot if no one else actually looks at the code
anyway. And let's be honest, the code quality of some of KDE's projects - I
wouldn't touch them with a six-foot pole. The ones I would touch though,
all have multiple people looking at the code and reviewing everything that
goes in.

Let me be very clear - even if you're the best damn programmer on the
planet, if *you* wrote the code, I do not trust *you* one inch to tell me
that that code is correct. That verification needs to come from someone
else, someone who does not have a conflict of interest in seeing that code
get into production. This is nothing personal, this is confirmation bias on
the author's part which leads to issues that even though they might be
infrequent, usually have catastrophic impact.

And if "culture" trumps over engineering best practices, it follows that I
should just stop using software produced by this entity because who knows
what it's doing.

Thanks,
Boudhayan

Re: libkcompactdisc test (testkcd) finds no tracks - wrong initialiser sent to the Phonon backend?

[Boudhayan Gupta]

libkcompactdisc doesn't work and should either be deprecated or rewritten
by someone who has access to an audio cd and a drive.

Nothing actually uses libkcompactdisc.


Freundliche Grüße
Boudhayan Gupta
KDE e.V. - Community Working Group
+49 151 71032970

On 18 August 2017 at 09:24, René J.V. Bertin <rjvber...@gmail.com> wrote:

> Hi,
>
> I'm not certain where to raise issues with libkcompactdisc, I hope this ML
> isn't the worst choice.
>
> I can't say I understand very well exactly how this library is suppose to
> integrate with Solid and the backends. It evidently works with the audiocd
> kio on Linux, but while trying to get that to work on Mac too (part of some
> work I'm doing on Solid) I discovered that testkcd doesn't find any (audio)
> CD tracks on Linux either.
>
> It looks to me like that could well be because it hands off a Solid UDI to
> Phonon and from there to the VLC Phonon backend. I've lost track of the
> control flow somewhere in there but I don't think Phonon knows how to
> handle such UDIs. Annoyingly there appears to be no way to get useful
> debugging done via the audiocd kio so I cannot corroborate this intuition.
>
> Thoughts?
>
> Thanks,
> René
>

Re: RE : Re: Send Email with attachment with my Qt Application

[Boudhayan Gupta]

Hi Charles-Elie,

On 17 November 2016 at 12:08, Charles-Élie G  wrote:
> Hi,
>
> In fact I think that KService must be built with CMake and after be used
> with QMake or CMake. No ?

Yes, that is correct. It must be built with CMake. We generate PRI
files that enable it to be used by QMake based projects later, but
KService itself must be built with CMake.

>  Message d'origine 
> De : Christoph Feck 
> Date : 16/11/2016 21:36 (GMT+01:00)
> À : kde-frameworks-devel@kde.org
> Objet : Re: Send Email with attachment with my Qt Application
>
> This is not true. KDE Frameworks support both qmake and cmake. On the
> API pages, the required instructions for use are explained.
>
> https://api.kde.org/frameworks-api/frameworks-apidocs/frameworks/kservice/html/index.html

Really? I don't see instructions telling me to do "qmake && make &&
make install" to build KService. Neither do I see the .pro file in the
repository.

-- Boudhayan

Re: Send Email with attachment with my Qt Application

[Boudhayan Gupta]

Hi Charles-Elie,

KService, and all of KDE software written in C++/Qt, are configured
with CMake and not QMake. There's no .pro files, but CMakeLists.txt
files. You'll need to install CMake, and build by doing:

cd /path/to/sources
cmake .
make && sudo make install

Hope this helps.

Thanks,
Boudhayan Gupta

On 16 November 2016 at 23:01, Charles-Elie Gentil <m...@jiyuusoft.net> wrote:
> In fact I did not understand how to build KService, but I think I have
> found: I probably have to create the .pro file myself.
>
> For the direct use of invokMailer, I have already tried but I can not use
> ShellExecute on my Mac. But I will try again.
>
> Best regards,
>
> Charlie
>
> m...@jiyuusoft.net
> http://blog.jiyuusoft.net
>
> Le 16 nov. 2016 à 17:24, David Edmundson <da...@davidedmundson.co.uk> a
> écrit :
>
> Sorry, didn't read that second line properly about attachments properly.
>
> What error do you get when buildling kservice?
> You may find this tool that builds dependencies useful: https://inqlude.org.
>
> Alternatively, you can probably just extract this one invokeMailer here:
> https://api.kde.org/frameworks/kservice/html/ktoolinvocation__win_8cpp_source.html
>
> It appears KToolInvocation doesn't have an OS X backend.
>
> David
>
>

Re: Scrap Baloo Thread Feedback

[Boudhayan Gupta]

Hi,

Unfortunately I've been hit my multiple pretty severe health scares in the
last month, and have no idea when I'm going to be at 100% again.

For the time being I'd rather not hold up any development, so don't hold
back anything on my account.

-- Boudhayan

On 16 October 2016 at 17:46, Christoph Cullmann  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?
>
> Greetings
> Christoph
>
> - Am 7. Okt 2016 um 20:08 schrieb cullmann cullm...@absint.com:
>
> > Hi,
> >
> >> Hey
> >>
> >> On Fri, Oct 7, 2016 at 6:34 PM, Christoph Cullmann 
> 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 not true.
> >
> > 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
> >
> >>
> >>>
> >>> => 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.
> >
> > And yes, there are "it hogs" 100% memory or time bugs open, thought you
> can
> > hardly reproduce them
> > as people are somehow scared to pack their home and send it to us. Not
> that a
> > lot of that bugs
> > got touched at all in Bugzilla.
> >
> >>
> >>>
> 
>  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.
> > That is ok, to see it as hobby.
> >
> > But I am a bit unnerved that one proposes this as the generic index
> solution
> > for our desktop, which should be stable, if nothing else, and knows 

Re: Scrap baloo?

[Boudhayan Gupta]

Hi,

On 28 September 2016 at 20:33, Christoph Cullmann  wrote:
> Hi,
>
>> Hi,
>>
>> On 28 September 2016 at 02:36, Christoph Cullmann  
>> wrote:
>>> any update?
>>
>> Yep. In all the happennings of the week I just forgot to write this email.
>>
>> If Baloo is going to be an integral part of the Plasma experience, do
>> we really want to depend on an external project where we don't have
>> control (and indeed, sentiments may prevent unrestricted contributions
>> based only on merit). This is the political reason why I don't want to
>> depend against Tracker. The technical reason is that it's based on
>> SQLite, which is incredibly slow compared to what we do now.
> I don't see really that it is slow compared to what we do, if you have 
> benchmarks
> for that, I would be pleased to see them.

So would I. You already have Tracker based code, could you spare some
time and run some?

>> At the same time, LMDB needs to be replaced, and fast. I'm building a
>> new KVDB as an university project (it should be able to do 256GB
>> indexes on 32bit machines), and if that doesn't work out there's
>> Sophia (http://sophia.systems/). I'll be evaluating both as a
>> replacement to LMDB.
> Do we really want to maintain a own DB system?
> IMHO that will never work out, all DB systems around need more maintenance 
> power
> than we have.

This is something I'm not sure about. The DB will be build anyway, my
graduation depends on it :D And if I'm going to do something I will do
it well, so it'll be simple and clean.

If it doesn't work out, there's always Sophia to fall back on.

>> Vishesh also wanted to separate out the engine and make it public API
>> (apparently other projects want to make use of it as a general data
>> storage library - and the engine offers fulltext search capabilities
>> and other fancy logical operators that make it particularly
>> attractive. My plan is to move towards that, and eventually also not
>> only index files but also other kinds of objects - contacts, or
>> people, for example.
>>
>> I don't want to move back into the "semantic desktop" idea at all, but
>> I do want some sort of infrastructure that allows for an "action on
>> object" metaphor - file objects can be opened with an application,
>> people objects can be sent mails, and so on.
>>
>> Hope this makes sense.
> I still not see how that should work out, atm, IMHO facts are:
>
> 1) baloo is not maintained

It will, now.

> 2) lmdb will e.g. never work for us on NFS homes and the code needs major 
> overhaul
> to handle errors (which you confirm)

LMDB goes away, either way.

> 3) you said you have "some time" left to maintain it, but you now propose in 
> addition to maintain
> Baloo to write a DB system from scratch, I don't really see that working

I have a personal interest, an academic interest, and now a
KDE-related interest in the KVDB. It *will* work, because I'm the kind
of guy who puts a lot of time and effort into things (maybe even
disproportionately so) into things that genuinely interest me. My
challenge will be to make the codebase so that after I'm done with
this (say in about 5 years or so) it'll be comprehensible to the next
maintainer.

> 4) tracker on the other side is maintained and in use and we can share the 
> index data with GNOME and others
>
> I really doubt that doing the work to remove lmdb, replace it with an "own 
> one" and then starting
> to fix all other issues (like indexer running amok, broken file extractors, 
> ...) will work out if
> we don't clone some more people.
>
> But that is only my opinion.

*Sigh*

I don't want to take the easy way out here. Half the fun in KDE is
doing crazy things and seeing your baby work. That's the entire
motivation for being here.

And right now I'm volunteering to do this.

-- Boudhayan

Re: Scrap baloo?

[Boudhayan Gupta]

Hi,

On 28 September 2016 at 02:36, Christoph Cullmann  wrote:
> any update?

Yep. In all the happennings of the week I just forgot to write this email.

If Baloo is going to be an integral part of the Plasma experience, do
we really want to depend on an external project where we don't have
control (and indeed, sentiments may prevent unrestricted contributions
based only on merit). This is the political reason why I don't want to
depend against Tracker. The technical reason is that it's based on
SQLite, which is incredibly slow compared to what we do now.

At the same time, LMDB needs to be replaced, and fast. I'm building a
new KVDB as an university project (it should be able to do 256GB
indexes on 32bit machines), and if that doesn't work out there's
Sophia (http://sophia.systems/). I'll be evaluating both as a
replacement to LMDB.

Vishesh also wanted to separate out the engine and make it public API
(apparently other projects want to make use of it as a general data
storage library - and the engine offers fulltext search capabilities
and other fancy logical operators that make it particularly
attractive. My plan is to move towards that, and eventually also not
only index files but also other kinds of objects - contacts, or
people, for example.

I don't want to move back into the "semantic desktop" idea at all, but
I do want some sort of infrastructure that allows for an "action on
object" metaphor - file objects can be opened with an application,
people objects can be sent mails, and so on.

Hope this makes sense.

Thanks,
Boudhayan

Re: Scrap baloo?

[Boudhayan Gupta]

Hi,

On 16 September 2016 at 01:04, Christoph Cullmann  wrote:
>>> => Opinions?
>>
>> It would be nice to hear what Vishesh, Pinak and Boudhayan think about
>> this.

I prefer getting familiar with the code as well as having a mile-high
view of what does what before I start laying down code, hence why I
haven't submitted a single line of code yet. What I have been doing so
far, though...

I've been trying to track down the history of Baloo and which part
does what. Also looking at past backends and what decisions were taken
that ended up with Baloo in the state that it is today.

I spoke to Pinak privately and he's volunteered to write some
documentation over the weekend. I've also been looking at why Baloo
doesn't use something like Xapian (which is made for use-cases like
Baloo) and I was pointed to this:
https://community.kde.org/Baloo/XapianProblems. Looks like the whole
rationale boils down to - we have so much custom code over Xapian we
might as well just have custom code over LMDB instead, it's faster.

I have a long journey (an actual journey, flights, buses, the whole
deal) ahead of me and I'll have some time to think about a long-term
plan for Baloo. It may involve killing it and working against Tracker,
or it may even involve putting more custom code into it. Whatever the
decision is, I'm going to try and justify the effort involved before I
present my plan to everyone here.

So far I'm leaning towards either a safer wrapper around LMDB or a
custom Key Value Database (basically a reimplimentation of Bitcask in
C++) for which we can ensure the presence of a Qt-friendly "safe" C++
API, to armour the database code. I actually think Baloo in its
current form is very well designed; for example, using the filesystem
as a database is pure genius. It's just sloppily executed and the code
quality could do with... some work.

I think wrapping the Database bits into a more modular and reusable
component was part of Vishesh's plans anyway, because other bits of
KDE would like to re-use them.

Anyway, enough rambling. I should have something in a few days.

Thanks,
Boudhayan

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

[Boudhayan Gupta]

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



Can you add Vishesh and poke him if possible? This seems like a gigantic patch 
and I'm not sure I understand all of it right now.

- Boudhayan Gupta


On Sept. 12, 2016, 12:42 a.m., Christoph Cullmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128892/
> ---
> 
> (Updated Sept. 12, 2016, 12:42 a.m.)
> 
> 
> Review request for KDE Frameworks, Plasma and Boudhayan Gupta.
> 
> 
> 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 128891: Transaction not created => don't try to abort them

[Boudhayan Gupta]

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


Ship it!




LGTM.

Long term I plan to replace all of this with liblmdb++, but that may or may not 
happen soon-ish.

- Boudhayan Gupta


On Sept. 12, 2016, 12:21 a.m., Christoph Cullmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128891/
> ---
> 
> (Updated Sept. 12, 2016, 12:21 a.m.)
> 
> 
> Review request for KDE Frameworks and Boudhayan Gupta.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> Transaction not created => don't try to abort them, avoid wrong usage of lmdb 
> api
> 
> 
> Diffs
> -
> 
>   src/engine/database.cpp 6a433c7 
> 
> Diff: https://git.reviewboard.kde.org/r/128891/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Christoph Cullmann
> 
>

Re: Review Request 128890: Make e.g. Baloo::Query thread safe.

[Boudhayan Gupta]

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


Ship it!




Ship It!

- Boudhayan Gupta


On Sept. 11, 2016, 11:49 p.m., Christoph Cullmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128890/
> ---
> 
> (Updated Sept. 11, 2016, 11:49 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma and Boudhayan Gupta.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> lmdb itself is thread safe (e.g. you can use the same env in multiple 
> threads).
> Unfortunately, the Baloo::Database itself not, as open() might race against 
> other open calls (we have one unique db object in baloo).
> 
> => add non-recursive mutex (recursive mutex not needed, one just must avoid 
> to call isOpen() or path() inside open, that is done, else no unit test 
> works).
> 
> 
> Diffs
> -
> 
>   src/engine/database.h e3bb175 
>   src/engine/database.cpp ec7ae2e 
> 
> Diff: https://git.reviewboard.kde.org/r/128890/diff/
> 
> 
> Testing
> ---
> 
> Unit tests still work.
> 
> 
> Thanks,
> 
> Christoph Cullmann
> 
>

Re: Taking over maintainership of Baloo

[Boudhayan Gupta]

Hi,

On 11 September 2016 at 22:59, Christoph Cullmann  wrote:
> I think the main issue is: There is no need to reproduce them, it is clear, 
> that baloo will crash
> on any problem with lmdb, as "no" errors are handled (or lets say 99% of the 
> errors are not handled).
>
> The code is full of Q_ASSERT, but no handling of the return codes beside that.

Oh, it gets worse. Q_ASSERTs compile to nothing in release mode so you
don't even have meaningful crashes on those asserts, things just
mysteriously fail elsewhere. I spent a whole bunch of time coding in
actual checks after those asserts because Dolphin would crash when
selecting multiple files with Baloo disabled, and other such magical
bugs.

> Actually, I think, in most cases the code shall just catch the error cases 
> and do nothing (or purge data
> until all is fine again) but not like now: crash or assert.
>
> That is not acceptable for something that runs per default ;=)
>
> Just looking at the code, you see things like:
>
> 1) Baloo::Database: needs a mutex, as many threads might call ::open (e.g. in 
> krunner) => corruption, dead
>
> 2) inproper cleanup: I doubt one can  mdb_txn_abort(txn); if already 
> mdb_txn_begin(...) failed
>
> 3) needs everywhere return code checks, e.g. in PostingList 
> PostingDB::get(const QByteArray& term), we have
> tons of bugs about random crashs afterwards: 
> https://bugs.kde.org/show_bug.cgi?id=367480
>
> 4) 32-bit systems supported at all? ATM, after 1GB of indexing, that was it, 
> no more baloo or any other application
> calling any of the accessors of e.g. Query (if you have bad luck).

If you have the time to fix those things you've listed above, awesome
- I'd love to look at the RRs. Lessens my workload.

Otherwise I'll look at this on the weekends when I have no sysadmin
things to do.

>
>>
>>> Beside that, could we get some baloo-b...@kde.org list as default assignee
>>> for all baloo bugs instead of "one" person?
>>
>> If we're going to do it this way, why not just make it the default
>> frameworks mailing list, or the frameworks bugs list if there is one?
> I am not sure, if that will make people happy ;=)

Let's get other people involved, too ;-)

Thanks,
Boudhayan

Re: Taking over maintainership of Baloo

[Boudhayan Gupta]

Hi,

On 11 September 2016 at 22:44, Christoph Cullmann <cullm...@absint.com> wrote:
> Hi,
>
> it would be very nice to have some active maintainership there,
> given a lot of VERY major crash bugs pile up there since > 1 year.

The issue is that neither Pinak nor I can reproduce much of the bugs,
making solving them difficult. I'll try harder to nowadays, though ;-)

> Beside that, could we get some baloo-b...@kde.org list as default assignee
> for all baloo bugs instead of "one" person?

If we're going to do it this way, why not just make it the default
frameworks mailing list, or the frameworks bugs list if there is one?

> Greetings
> Christoph

-- Boudhayan

>
> - Am 11. Sep 2016 um 19:08 schrieb Boudhayan Gupta bgu...@kde.org:
>
>> Hi all,
>>
>> Since Vishesh effectively stopped maintaining Baloo full-time, mostly
>> Pinak and to some extent I have been looking over bugs and reviewing
>> RRs pertaining to Baloo. If there are no objections to the same
>> (including if Pinak declines to either co-maintain it or would rather
>> maintain it alone, sans me), I'd like to officially take over
>> maintainership of Baloo, co-maintaining it with Pinak.
>>
>> Thanks,
>> Boudhayan
>
> --
> - Dr.-Ing. Christoph Cullmann -
> AbsInt Angewandte Informatik GmbH  Email: cullm...@absint.com
> Science Park 1 Tel:   +49-681-38360-22
> 66123 Saarbrücken  Fax:   +49-681-38360-20
> GERMANYWWW:   http://www.AbsInt.com
> 
> Geschäftsführung: Dr.-Ing. Christian Ferdinand
> Eingetragen im Handelsregister des Amtsgerichts Saarbrücken, HRB 11234

Re: Review Request 128888: Improve epub extractor, less segfaults

[Boudhayan Gupta]

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


Ship it!




It looks sane enough to me. I'm going to assume you tested things and nothing 
obvious regressed, and give you the ship-it.

I'm also very interested in knowing how we have come about having a variable 
called `tit` ;-)

- Boudhayan Gupta


On Sept. 11, 2016, 10:32 p.m., Christoph Cullmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/12/
> ---
> 
> (Updated Sept. 11, 2016, 10:32 p.m.)
> 
> 
> Review request for KDE Frameworks, Boudhayan Gupta and Pinak Ahuja.
> 
> 
> Repository: kfilemetadata
> 
> 
> Description
> ---
> 
> Improve epub extractor:
> 
> 1) check for more nullpointers (e.g. data can be null for some fields, 
> iterators, ...)
> 2) actually close the epub file again at all
> 3) iterator seems to handle clink as stated in docs, fix double free
> 
> e.g. see bug 361727
> could be the double freed clink in the last iterator
> 
> https://bugs.kde.org/show_bug.cgi?id=361727
> 
> 
> Diffs
> -
> 
>   src/extractors/epubextractor.cpp 88a4caa 
> 
> Diff: https://git.reviewboard.kde.org/r/12/diff/
> 
> 
> Testing
> ---
> 
> Unit test still works, no idea how to craft files broken enough to trigger 
> the faults
> 
> 
> Thanks,
> 
> Christoph Cullmann
> 
>

Taking over maintainership of Baloo

[Boudhayan Gupta]

Hi all,

Since Vishesh effectively stopped maintaining Baloo full-time, mostly
Pinak and to some extent I have been looking over bugs and reviewing
RRs pertaining to Baloo. If there are no objections to the same
(including if Pinak declines to either co-maintain it or would rather
maintain it alone, sans me), I'd like to officially take over
maintainership of Baloo, co-maintaining it with Pinak.

Thanks,
Boudhayan

Re: Review Request 128885: Increase size limit of baloo index for 64-bit machines

[Boudhayan Gupta]

> On Sept. 11, 2016, 10:20 p.m., Boudhayan Gupta wrote:
> > src/engine/database.cpp, line 107
> > <https://git.reviewboard.kde.org/r/128885/diff/1/?file=476712#file476712line107>
> >
> > This is clever, but I had to read it thrice to figure out what exactly 
> > it does. Maybe use this:
> > 
> > ```
> > ((sizeof(size_t) == 4) ? 1 : 256) * size_t(1024 * 1024 * 1024)
> > ```
> > 
> > Seems more readable to me - the significant digits of the size followed 
> > by the multiplier.
> > 
> > If there's a performance or strictness issue I'm not seeing, please 
> > tell me.

Err, could you also calculate the effective size in a const on the line above? 
Sorry about the nitpick, I just want readability.


- Boudhayan


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


On Sept. 11, 2016, 4:27 p.m., Christoph Cullmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128885/
> -------
> 
> (Updated Sept. 11, 2016, 4:27 p.m.)
> 
> 
> Review request for KDE Frameworks, Boudhayan Gupta, David Faure, and Pinak 
> Ahuja.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> Increase size limit of baloo index for 64-bit machines to avoid crashs after 
> > 5GB of index size.
> (Better would be additional out-of-space handling, but ATM baloo has zero 
> checks for that)
> 
> The size limit for 32-bit is still 1GB, like before (there was a silent 
> overflow from 5GB to 1GB in the computation), people with large homes will 
> still get random segfaults on 32-bit.
> 
> Patch based on patch from Hao Zhang, Bug 364475
> https://bugs.kde.org/show_bug.cgi?id=364475
> 
> 
> Diffs
> -
> 
>   src/engine/database.cpp 89e2e03 
>   src/engine/databasesize.h aa180fb 
>   src/engine/transaction.cpp 0af20be 
>   src/tools/balooctl/statuscommand.cpp 73289c4 
> 
> Diff: https://git.reviewboard.kde.org/r/128885/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Christoph Cullmann
> 
>

Re: Review Request 128885: Increase size limit of baloo index for 64-bit machines

[Boudhayan Gupta]

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


Fix it, then Ship it!




Ship It!


src/engine/database.cpp (line 106)
<https://git.reviewboard.kde.org/r/128885/#comment66716>

This is clever, but I had to read it thrice to figure out what exactly it 
does. Maybe use this:

```
((sizeof(size_t) == 4) ? 1 : 256) * size_t(1024 * 1024 * 1024)
```

Seems more readable to me - the significant digits of the size followed by 
the multiplier.

If there's a performance or strictness issue I'm not seeing, please tell me.


- Boudhayan Gupta


On Sept. 11, 2016, 4:27 p.m., Christoph Cullmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128885/
> ---
> 
> (Updated Sept. 11, 2016, 4:27 p.m.)
> 
> 
> Review request for KDE Frameworks, Boudhayan Gupta, David Faure, and Pinak 
> Ahuja.
> 
> 
> Repository: baloo
> 
> 
> Description
> ---
> 
> Increase size limit of baloo index for 64-bit machines to avoid crashs after 
> > 5GB of index size.
> (Better would be additional out-of-space handling, but ATM baloo has zero 
> checks for that)
> 
> The size limit for 32-bit is still 1GB, like before (there was a silent 
> overflow from 5GB to 1GB in the computation), people with large homes will 
> still get random segfaults on 32-bit.
> 
> Patch based on patch from Hao Zhang, Bug 364475
> https://bugs.kde.org/show_bug.cgi?id=364475
> 
> 
> Diffs
> -
> 
>   src/engine/database.cpp 89e2e03 
>   src/engine/databasesize.h aa180fb 
>   src/engine/transaction.cpp 0af20be 
>   src/tools/balooctl/statuscommand.cpp 73289c4 
> 
> Diff: https://git.reviewboard.kde.org/r/128885/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Christoph Cullmann
> 
>

baloo + out-of-memory in file extractor

[Boudhayan Gupta]

Hi Christoph,

On 10 September 2016 at 23:46, Christoph Cullmann  wrote:
>>> Would it be a good idea to restrict the file extractor process to some
>> fixed amount of memory
>>> to use via setrlimit? (or more fancy stuff?)
>>
>> That would probably just make Baloo crash, so fixing the bug is probably
>> the better option.
> Actually, that we don't limit the resources + sandbox baloo_file_indexer is 
> the bug,
> not that some meta info extractor is buggy (which should be fixed, too).
>
> ATM, the state is:
>
> 1) baloo is on per default
> 2) it will index at least your home
> 3) if it encounters any "bad" file, it will OOM you, in my case in a way
> that a normal user is doomed, as 1-2 seconds after login the machine is 
> already halted.
>
> Given that e.g. your "Downloads" might even contain "evil" files from the net,
> at least some resource limit would be good and even better some sandbox, to 
> avoid that
> the indexer which is easily pwn'd pwn's your session.

You've got a point there. In that case, what I'd do is:

1) Limit resources on baloo_file_extractor.
2) Try to detect if it crashes because it exceeded limits, not sure if
this is easily possible.
3) Mark files causing such crashes as files that should be skipped,
and the user notified somehow (?).

> Beside that, a real other problem is, that baloo has close to zero error 
> handling for its
> database, once one error happens, all further things will go down and never 
> recover.
>
> e.g. one time balooctl wrongly use => goodbye
> https://bugs.kde.org/show_bug.cgi?id=368557
>
> Interesting too: We use lmdb, which means, we memory map always, aka 32-bit 
> machines will
> be out of memory if you have large indices like > 2GB :/

Nah, 32bit machines should have PAE. If they don't... I'm not willing
to make fundamental changes to how indexes are kept to support edge
cases like this. Disabling Baloo automatically if you detect machines
with a 32-bit address space is the way to go.

I'd still wait for Pinak to comment on all of the above though.

> I would like to help out with fixing this issues, but I think first some 
> consensus would be needed
> how we want to go on with that.

-- Boudhayan

Re: baloo + out-of-memory in file extractor

[Boudhayan Gupta]

Hi,

On 10 Sep 2016 19:42, "Christoph Cullmann" <cullm...@absint.com> wrote:
>
> Hi,
>
> during the last night of the akademy for me in Berlin, baloo file
extractor OOM'd my notebook
> during extracting the tag info of the taglib unit test test-files.

Is it possible to reproduce the issue and get a stack trace?

If you can consistently repro this, running under valgrind to check for
leaks will also go a long way.

>
> Would it be a good idea to restrict the file extractor process to some
fixed amount of memory
> to use via setrlimit? (or more fancy stuff?)

That would probably just make Baloo crash, so fixing the bug is probably
the better option.

>
> Greetings
> Christoph

-- Boudhayan Gupta

>
> --
> - Dr.-Ing. Christoph Cullmann -
> AbsInt Angewandte Informatik GmbH  Email: cullm...@absint.com
> Science Park 1 Tel:   +49-681-38360-22
> 66123 Saarbrücken  Fax:   +49-681-38360-20
> GERMANYWWW:   http://www.AbsInt.com
> 
> Geschäftsführung: Dr.-Ing. Christian Ferdinand
> Eingetragen im Handelsregister des Amtsgerichts Saarbrücken, HRB 11234

Re: KMessageBox runtime dependency on FrameworkIntegrationPlugin makes it useless

[Boudhayan Gupta]

On 13 June 2016 at 00:08, Hugo Pereira Da Costa
<hugo.pereira.da.co...@gmail.com> wrote:
> On 06/12/2016 07:51 PM, Albert Astals Cid wrote:
>>
>> Having KMessageBox "dontShowMeAgain" feature depend on an integration
>> plugin
>> is a very bad idea.
>>
>> Basically it means programs that use KMessageBox can never asusme it will
>> work
>> so basically they have to use alternative methods to have the
>> "dontShowMeAgain" feature or not have it at all.
>>
>> I understand someone thought that it was a better idea having a feature
>> that
>> may work or not randomly that increasing the dependency chain of
>> KMessageBox,
>> but I disagree.
>>
>> I don't think the status quo is good at all, my program basically gets a
>> runtime dependency that is not specified anywhere and that makes some
>> features
>> work or not randomly.
>>
>> The options I can see are:
>>   * Remove the "dontShowMeAgain" feature from KMessageBox
>>   * Make the "dontShowMeAgain" feature use QSettings (always or if
>> FrameworkIntegrationPlugin is not available)
>>   * Show a KMessageBox warning when trying to use the "dontShowMeAgain"
>> feature
>> and the FrameworkIntegrationPlugin is not available saying the user to
>> install
>> that package if he wants to get the functionality.
>>
>> Suggestions?
>>
>> For those that say this is an hipothetical situation that makes no sense
>> to
>> fix, please go and read https://bugs.kde.org/show_bug.cgi?id=362082
>>
>> I lost hours of my life trying to figure out what was wrong.
>
> Hello Albert,
>
> Just a naive question (sorry if I miss the point). If the use of the
> don't-show-again pluggin is critical to your app, can't you also add a
> strong dependency on frameworkintegration to your app ?

The point here is, "Don't show me again" is a core feature, from an
usability perspective, and it should always just work. Therefore, the
onus is on KMessageBox to ensure all dependencies are met, and if
they're not met an adequate fallback exists. This is not for the
application to ensure.

-- 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

[Boudhayan Gupta]

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

(Updated March 26, 2016, 2:45 p.m.)


Status
--

This change has been discarded.


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 127179: Add external extractor plugin support

[Boudhayan Gupta]

> On Feb. 26, 2016, 2:48 a.m., Pinak Ahuja wrote:
> > src/externalextractor.cpp, line 104
> > <https://git.reviewboard.kde.org/r/127179/diff/1/?file=445488#file445488line104>
> >
> > You should use the asynchronous api instead of blocking the event loop 
> > by calling waitForFinished()
> > 
> > Connect a slot to the QProcess::finished signal and let it handle the 
> > output of the QProcess.
> 
> Varun Joshi wrote:
> Since native plugins block the event loop, I thought it would be sensible 
> to let the caller handle calling all types of plugins asynchronously. What do 
> you think?

Good point! Remember that the event loop is not ecxlisive to the KFM library 
but is shared (and owned) by the application that uses this library. Not 
yeilding to the event loop will freeze the parent application too.


- Boudhayan


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


On Feb. 26, 2016, 12:02 a.m., Varun Joshi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127179/
> ---
> 
> (Updated Feb. 26, 2016, 12:02 a.m.)
> 
> 
> Review request for Baloo, KDE Frameworks, Boudhayan Gupta, and Vishesh Handa.
> 
> 
> Repository: kfilemetadata
> 
> 
> Description
> ---
> 
> 1. Add the ExternalExtractor class that wrap the external extractor process 
> into the standard Extractor interface
> 2. Modify ExtractorCollection to enable it to support ExternalExtractors
> 3. Added an example PyPDF2 extractor plugin
> 
> 
> Diffs
> -
> 
>   README.md 19b1a26a241e6a35c636aaf8162afe762018f073 
>   src/CMakeLists.txt a5490856a51aa2f59389ee963f3430c1ce5c60d5 
>   src/config-kfilemetadata.h.in PRE-CREATION 
>   src/externalextractor.h PRE-CREATION 
>   src/externalextractor.cpp PRE-CREATION 
>   src/extractorcollection.h 8542aed576102be2b0c86bbdf3d65d756d468c6e 
>   src/extractorcollection.cpp a1bde65bf57e493918cd3e85ccdb23c4cd623401 
>   src/extractorplugin.h 65abad3b2397628ba42a40d9ef2970e02114e250 
>   src/extractors/CMakeLists.txt 5dd223e1cf6864a943e848664ad5fae0d0603e77 
>   src/extractors/externalextractors/CMakeLists.txt PRE-CREATION 
>   src/extractors/externalextractors/pdfextractor/main.py PRE-CREATION 
>   src/extractors/externalextractors/pdfextractor/manifest.json PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/127179/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Varun Joshi
> 
>

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

Re: Review Request 127179: Add external extractor plugin support

[Boudhayan Gupta]

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



The logic is more or less sound. You'll have to solve these issues.

I think you should try to get this patch merged first, before the writers. The 
code looks fine enough - I don't think when you apply the patch it'll depend on 
the writers RR.

Just edit the RR and remove the depends-on RR number.


src/externalextractor.h (line 40)
<https://git.reviewboard.kde.org/r/127179/#comment63200>

This isn't the proper way to do private classes.

Private classes are done like this:

1) In the header:
```
private:
struct ExternalExtractorPrivate;
ExternalExtractorPrivate *d_ptr;
Q_DECLARE_PRIVATE(ExternalExtractor);
```

Basically, the private class should be named ParentClassPrivate, the next 
line should create a ParentClassPrivate pointer called d_ptr (the variable name 
is important), and then you use the Q_DECLARE_PRIVATE(ParentClass) macro call 
(not ParentClassPrivate) to set up the accessor.

I've used the struct keyword instead of class because everything inside the 
public class is typically public - and in C++ structs are just classes where 
everything is public by default.

Doing it this way is better because the macros take care of casting the 
pointer correctly - suppose someone subclasses ParentClass to ChildClass, raw 
pointers won't work because it's still a pointer to ParentClass::Private, not 
ChildClass::Private.

2) When you use the d-pointer inside a function, do this:
```
someFunc()
{
Q_D(ParentClass) // not ParentClassPrivate
d->whatever = whatever;
}
```

Note that you're using d-> and not d_ptr->. The Q_D() macro automatically 
static_casts() the d_ptr into the appropriate type and makes a variable d 
(local to your function) that you can use to access the private class.

For more information, refer to:
https://wiki.qt.io/D-Pointer



src/externalextractor.cpp (line 46)
<https://git.reviewboard.kde.org/r/127179/#comment63201>

You'll need to create an object for the private class here too.



src/externalextractor.cpp (line 61)
<https://git.reviewboard.kde.org/r/127179/#comment63207>

All text needs to be translatable.

1) ```#include ```
2) Replace all ```string``` by ```i18n(string)```



src/externalextractor.cpp (line 83)
<https://git.reviewboard.kde.org/r/127179/#comment63208>

You're leaking memory. Have a destructor delete the private class object.



src/externalextractor.cpp (line 104)
<https://git.reviewboard.kde.org/r/127179/#comment63202>

You may want to wait for more than 5 seconds for large files. 30 seconds is 
a good idea.



src/externalextractor.cpp (line 108)
<https://git.reviewboard.kde.org/r/127179/#comment63203>

Clean this up. Vomit out the error output only if there's an error. There's 
no need to dump the extractor output in case there's no error.



src/extractorcollection.cpp (line 4)
<https://git.reviewboard.kde.org/r/127179/#comment63204>

Copyright who?



src/extractorcollection.cpp (line 81)
<https://git.reviewboard.kde.org/r/127179/#comment63205>

Cat-on-keyboard? ;-)



src/extractors/externalextractors/pdfextractor/main.py (line 40)
<https://git.reviewboard.kde.org/r/127179/#comment63206>

Remove empty space from this line.


- Boudhayan Gupta


On Feb. 26, 2016, 12:02 a.m., Varun Joshi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127179/
> ---
> 
> (Updated Feb. 26, 2016, 12:02 a.m.)
> 
> 
> Review request for Baloo, KDE Frameworks, Boudhayan Gupta, and Vishesh Handa.
> 
> 
> Repository: kfilemetadata
> 
> 
> Description
> ---
> 
> 1. Add the ExternalExtractor class that wrap the external extractor process 
> into the standard Extractor interface
> 2. Modify ExtractorCollection to enable it to support ExternalExtractors
> 3. Added an example PyPDF2 extractor plugin
> 
> 
> Diffs
> -
> 
>   README.md 19b1a26a241e6a35c636aaf8162afe762018f073 
>   src/CMakeLists.txt a5490856a51aa2f59389ee963f3430c1ce5c60d5 
>   src/config-kfilemetadata.h.in PRE-CREATION 
>   src/externalextractor.h PRE-CREATION 
>   src/externalextractor.cpp PRE-CREATION 
>   src/extractorcollection.h 8542aed576102be2b0c86bbdf3d65d756d468c6e 
>   src/extractorcollection.cpp a1bde65bf57e493918cd3e85ccdb23c4cd623401 
>   src/extractorplugin.h 65abad3b2397628ba42a40d9ef2970e02114e250 
>   src/extractors/CMakeLists.txt 

Re: Review Request 127158: Adds writer and external (extractor and writer) plugin support

[Boudhayan Gupta]

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



I think the very first thing you should do is divide the whole thing into 
multiple patches, and thus 3 review requests. Discard this one and file 3 new 
ones.

The three should be - one that adds all the write functionality, one that adds 
the external reader, and one that adds the external writer. In any case, as 
they're three different major features, you'll have to commit them in three 
different commits anyway.

- Boudhayan Gupta


On Feb. 24, 2016, 3:33 a.m., Varun Joshi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127158/
> ---
> 
> (Updated Feb. 24, 2016, 3:33 a.m.)
> 
> 
> Review request for Baloo, KDE Frameworks and Vishesh Handa.
> 
> 
> Repository: kfilemetadata
> 
> 
> Description
> ---
> 
> Builds upon Boudhyan Gupta's work on external extractors, creating a process 
> that runs a script that extracts/writes data, accepting input and producing 
> an output (JSON-formatted) via stdin/stdout.
> 
> Adds Writer, WriterData, WriterPlugin and WriterCollection classes, analogous 
> to the Extractor classes. The only notable difference is that consumer 
> applications will be able to use WriteData (analogous to ExtractionResult) 
> without subclassing it.
> 
> Adds ExternalWriter and ExternalExtractor classes. These handle input/output 
> and communication with the external plugins.
> 
> 
> Diffs
> -
> 
>   README.md 19b1a26a241e6a35c636aaf8162afe762018f073 
>   autotests/CMakeLists.txt 9d308367838680f1081cfa3b583d3401412c21c1 
>   autotests/taglibwritertest.h PRE-CREATION 
>   autotests/taglibwritertest.cpp PRE-CREATION 
>   src/CMakeLists.txt a5490856a51aa2f59389ee963f3430c1ce5c60d5 
>   src/config-kfilemetadata.h.in PRE-CREATION 
>   src/externalextractor.h PRE-CREATION 
>   src/externalextractor.cpp PRE-CREATION 
>   src/externalwriter.h PRE-CREATION 
>   src/externalwriter.cpp PRE-CREATION 
>   src/extractorcollection.h 8542aed576102be2b0c86bbdf3d65d756d468c6e 
>   src/extractorcollection.cpp a1bde65bf57e493918cd3e85ccdb23c4cd623401 
>   src/extractorplugin.h 65abad3b2397628ba42a40d9ef2970e02114e250 
>   src/extractors/CMakeLists.txt 5dd223e1cf6864a943e848664ad5fae0d0603e77 
>   src/extractors/externalextractors/CMakeLists.txt PRE-CREATION 
>   src/extractors/externalextractors/exampleextractor/main.py PRE-CREATION 
>   src/extractors/externalextractors/exampleextractor/manifest.json 
> PRE-CREATION 
>   src/extractors/externalextractors/pdfextractor/main.py PRE-CREATION 
>   src/extractors/externalextractors/pdfextractor/manifest.json PRE-CREATION 
>   src/writedata.h PRE-CREATION 
>   src/writedata.cpp PRE-CREATION 
>   src/writer.h PRE-CREATION 
>   src/writer.cpp PRE-CREATION 
>   src/writer_p.h PRE-CREATION 
>   src/writercollection.h PRE-CREATION 
>   src/writercollection.cpp PRE-CREATION 
>   src/writerplugin.h PRE-CREATION 
>   src/writerplugin.cpp PRE-CREATION 
>   src/writers/CMakeLists.txt PRE-CREATION 
>   src/writers/externalwriters/CMakeLists.txt PRE-CREATION 
>   src/writers/externalwriters/pdfwriter/main.py PRE-CREATION 
>   src/writers/externalwriters/pdfwriter/manifest.json PRE-CREATION 
>   src/writers/taglibwriter.h PRE-CREATION 
>   src/writers/taglibwriter.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/127158/diff/
> 
> 
> Testing
> ---
> 
> Created a sample PDF reader and extractor in Python, both of which work 
> successfully.
> 
> 
> Thanks,
> 
> Varun Joshi
> 
>

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

Re: building everything with a set of specific preprocessor token(s)/definition(s)?

[Boudhayan Gupta]

On 14 February 2016 at 19:31, René J.V.  wrote:
> Hi,
>
> I'd like to investigate something Qt-related that would require me to build 
> all frameworks with a specific set of preprocessor tokens defined. Rather 
> than patching all toplevel CMake files I'm hoping for way to define those 
> tokens in the cmake invocation, IOW on the cmake command line.
> It seems CMake itself doesn't make that easy because of a lack incremental 
> mechanisms, but maybe there's a hook in the ECM that I've overlooked?
>
> Thanks,
> René

CC="gcc -DWHATEVER_DEFINE" cmake ...

It really should be as simple as that.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: building everything with a set of specific preprocessor token(s)/definition(s)?

[Boudhayan Gupta]

On 14 February 2016 at 22:00, René J.V. <rjvber...@gmail.com> wrote:
> On Sunday February 14 2016 15:20:14 David Faure wrote:
>
>> export CXXFLAGS="-DMYDEFINE=1" before running cmake should do the trick.
>
> That's what I'm using ATM, but cmake has a nasty habit of not taking the 
> user's full CXXFLAGS and/or adding its own stuff to the resulting 
> CMAKE_CXX_COMPILER_FLAGS_XXX, depending on what build type you're using.
>
> There's also an interaction with the build system I'm targeting; not really 
> relevant on here but it does make it less trivial to rely on settings passed 
> through the environment.

Hence my idea of using CC/CXX rather than the *FLAGS env-vars.

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

Re: Review Request 126949: Remove Air and Oxygen themes

[Boudhayan Gupta]

> On Feb. 2, 2016, 1:42 a.m., andreas kainz wrote:
> > Hi, I know I say I will keep oxygen-icons5 up to date, and I hope I get the 
> > time I need the plasma oxygen theme too to give the user the one single 
> > click feature (look & feel package). Is there an space where you can move 
> > "unmaintained" stuff and the distros can choose what they want to ship. now 
> > all distros ship oxygen.
> > 
> > I love breeze but oxygen is also really sexy in addition a lot of uers work 
> > with kde4 (which is unmainted too) and update to kf5 with Kubuntu 16.04 
> > LTS. It would be bad to support oxygen for an additional LTS cycle, but for 
> > the switch it would be nice to give the user the posibility for an soft 
> > switch.
> 
> andreas kainz wrote:
> in addition a lot of dev's make screenshots with oxygen so there is an 
> user group for oxygen.
> 
> Kai Uwe Broulik wrote:
> There'a an oxygen repository where the widget style and everything 
> resides, I'd say the Plasma theme would fit in there as well.
> 
> andreas kainz wrote:
> ok. that would fit for me. since now we have not that much look and feel 
> packages most distros use it for there look and feel stuff and than breeze 
> and breeze-dark for people how don't like monochrome (there are a lot) is 
> oxygen. it would be nice to have more packages there available so move the 
> oxygen stuff in an spacific area is fine for me but don't move them to 
> kde-look cause than oxygen is more broken than now and no one can fix it any 
> more.
> 
> thanks Kai
> 
> Martin Klapetek wrote:
> > in addition a lot of dev's make screenshots with oxygen so there is an 
> user group for oxygen.
> 
> Really? Got any examples?
> 
> > There'a an oxygen repository where the widget style and everything 
> resides, I'd say the Plasma theme would fit in there as well.
> 
> 
> Ok, I'll put it there then.
> 
> David Edmundson wrote:
> Andreas, note the oxygen widget style and plasma theme are different 
> things. 
> The Oxygen plasma style is this theme: 
> https://www.kde.org/announcements/4.0/screenshots/desktop.jpg
> 
> the widget style isn't being touched.
> 
> andreas kainz wrote:
> can we add ALL oxygen stuff in one "unmaintained" area?
> 
> Hugo Pereira Da Costa wrote:
> Wait wait, neither oxygen widget style nor window decoration are 
> unmaintained. (nor broken as far as I know)
> They are in "maintenance" mode on the contrary. Meaning: they are "done", 
> there won't be any new feature, but bugs get fixed when reported. 
> Please 
> - dont move them anywhere else,
> - dont move broken, nor truely unmaintained things in there.
> This is not a trash can :)
> 
> Thx,
> 
> Hugo
> 
> Kai Uwe Broulik wrote:
> Yes, Hugo is doing a fine job of maintaining Oxygen stuff and 
> cherry-picking fixes from Breeze. :) It's really just the Plasma theme that 
> is utterly broken and unmaintained.
> 
> andreas kainz wrote:
> I'v never done a plasma theme but I saw how the breeze theme get broken 
> and how it was fixed again. As Hugo maintaine oxygen stuff and I maintain 
> oxygen icon stuff, it would be good to have the plasma theme also available. 
> I can offer to maintain the oxygen plasma theme (one of them).
> 
> Marco Martin wrote:
> oxygen icon or oxygen style repository? for me both would be good.
> 
> note that if you chose the oxygen look and feel it sets the oxygen widget 
> style and the Air plasma theme, that should be still the case.
> 
> Thomas Pfeiffer wrote:
> I'd say we need one theme to go with the Oxygen Look & Feel package. If 
> Air is still maintainted and used in the L, then killing Oxygen from our 
> repos should be fine.
> I'm against putting it on kde-look, however, because we don't want 
> unmaintained and broken stuff on there, either. kde-look currently is used 
> kind of as a junkyard by some, but this has to change if we want to showcase 
> it more.

I'm still of the opinion that it makes more sense to put everything Oxygen in 
one package; given that Oxygen is not the default theme for Plasma anymore we 
shouldn't ship it with the default look-n-feel package at all.


- Boudhayan


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


On Feb. 2, 2016, 1 a.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126949/
> ---
> 
> (Updated Feb. 2, 2016, 1 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Bugs: 358533
> http://bugs.kde.org/show_bug.cgi?id=358533
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---

Re: Review Request 126949: Remove Air and Oxygen themes

[Boudhayan Gupta]

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



+1

Ideally the default look-n-feel package should include only two themes - the 
default one and a high-contrast counterpart. In fact all other themes should 
reside elsewhere.

If we still want to "officially" provide Air and Oxygen, maybe we should move 
them to an extra-themes package instead instead of kde-look?

- Boudhayan Gupta


On Feb. 2, 2016, 1 a.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126949/
> ---
> 
> (Updated Feb. 2, 2016, 1 a.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Bugs: 358533
> http://bugs.kde.org/show_bug.cgi?id=358533
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> They are untested, unmaintained and most importantly, broken.
> 
> Let's remove them from our releases and move them to kde-look.org for grabs.
> 
> 
> Diffs
> -
> 
>   src/desktoptheme/air/CMakeLists.txt 40ea382 
>   src/desktoptheme/air/colors 5c4e53d 
>   src/desktoptheme/air/dialogs/background.svgz 81ef460 
>   src/desktoptheme/air/dialogs/kickoff.svgz 40e2ae6 
>   src/desktoptheme/air/dialogs/krunner.svgz 1f6a64f 
>   src/desktoptheme/air/icons/amarok.svgz e3c157e 
>   src/desktoptheme/air/icons/audio.svgz 540cc77 
>   src/desktoptheme/air/icons/battery.svgz 88f864e 
>   src/desktoptheme/air/icons/configure.svgz a433b3c 
>   src/desktoptheme/air/icons/device.svgz 23df094 
>   src/desktoptheme/air/icons/document.svgz 6e89dd8 
>   src/desktoptheme/air/icons/edit.svgz d0e1ca4 
>   src/desktoptheme/air/icons/go.svgz ad3a4b2 
>   src/desktoptheme/air/icons/kget.svgz 1f1702f 
>   src/desktoptheme/air/icons/klipper.svgz 4afd33c 
>   src/desktoptheme/air/icons/konv_message.svgz 10e31cd 
>   src/desktoptheme/air/icons/konversation.svgz 376211a 
>   src/desktoptheme/air/icons/kopete.svgz 5f41c4c 
>   src/desktoptheme/air/icons/korgac.svgz 8c8f700 
>   src/desktoptheme/air/icons/kpackagekit.svgz 234e6da 
>   src/desktoptheme/air/icons/ktorrent.svgz dc623d8 
>   src/desktoptheme/air/icons/list.svgz 0a25bb0 
>   src/desktoptheme/air/icons/media.svgz 23653bd 
>   src/desktoptheme/air/icons/nepomuk.svgz 5c8545e 
>   src/desktoptheme/air/icons/network.svgz a8e8b16 
>   src/desktoptheme/air/icons/notification.svgz db12260 
>   src/desktoptheme/air/icons/preferences.svgz 12e6588 
>   src/desktoptheme/air/icons/printer.svgz 199ab95 
>   src/desktoptheme/air/icons/quassel.svgz b004749 
>   src/desktoptheme/air/icons/slc.svgz 9fd376b 
>   src/desktoptheme/air/icons/wallet.svgz 4ad3691 
>   src/desktoptheme/air/icons/window.svgz eecc37f 
>   src/desktoptheme/air/icons/zoom.svgz c159491 
>   src/desktoptheme/air/metadata.desktop 0be4bd5 
>   src/desktoptheme/air/opaque/dialogs/background.svgz d2329ff 
>   src/desktoptheme/air/opaque/dialogs/krunner.svgz 2a2134f 
>   src/desktoptheme/air/opaque/widgets/extender-background.svgz e810ba4 
>   src/desktoptheme/air/opaque/widgets/panel-background.svgz 2a2134f 
>   src/desktoptheme/air/opaque/widgets/tooltip.svgz 2a2134f 
>   src/desktoptheme/air/translucent/dialogs/background.svgz 9b68062 
>   src/desktoptheme/air/translucent/dialogs/krunner.svgz 4c48b52 
>   src/desktoptheme/air/translucent/widgets/extender-background.svgz 799e798 
>   src/desktoptheme/air/translucent/widgets/panel-background.svgz 7783e3b 
>   src/desktoptheme/air/translucent/widgets/tooltip.svgz 7783e3b 
>   src/desktoptheme/air/widgets/action-overlays.svgz 4259105 
>   src/desktoptheme/air/widgets/actionbutton.svgz e1713ea 
>   src/desktoptheme/air/widgets/analog_meter.svgz f0d2163 
>   src/desktoptheme/air/widgets/arrows.svgz b987b32 
>   src/desktoptheme/air/widgets/background.svgz 174a8fc 
>   src/desktoptheme/air/widgets/bar_meter_horizontal.svgz 8031b77 
>   src/desktoptheme/air/widgets/bar_meter_vertical.svgz 1b0660d 
>   src/desktoptheme/air/widgets/branding.svgz c6316fb 
>   src/desktoptheme/air/widgets/busywidget.svgz f638cfb 
>   src/desktoptheme/air/widgets/button.svgz 2c529bf 
>   src/desktoptheme/air/widgets/calendar.svgz 2d80a49 
>   src/desktoptheme/air/widgets/checkmarks.svgz dcf2924 
>   src/desktoptheme/air/widgets/clock.svgz 3839dac 
>   src/desktoptheme/air/widgets/configuration-icons.svgz 9b212d3 
>   src/desktoptheme/air/widgets/containment-controls.svgz a3166ce 
>   src/desktoptheme/air/widgets/dragger.svgz 3629591 
>   src/desktopth

Re: Review Request 126324: [MSWin/OS X] save and restore window geometry instead of only size (WIP/Suggestion)

[Boudhayan Gupta]

> On Dec. 14, 2015, 12:23 p.m., Martin Gräßlin wrote:
> > src/gui/CMakeLists.txt, line 2
> > 
> >
> > this introduces a QtWidgets dependency and thus changes the integration 
> > level of the framework. I highly recommend to not go this way.
> > 
> > Looking at the code there is no reason for this. The usage of 
> > QDesktopWidget is not needed as QScreen provides the same. Similar one 
> > doesn't need the QWidget usage as QWindow is already there.
> 
> René J.V. Bertin wrote:
> I'm all for reducing the number of dependencies, but haven't found 
> another way to get at QWidget::saveGeometry and QWidget::restoreGeometry.
> You're probably much more familiar with what those functions really save 
> and restore, and thus to what extent they're essentially convenience 
> functions here for something I could just as well access via 
> QWindow::geometry or QWindow::frameGeometry. I'd have to figure out on my end 
> which of the two I'd need to use because that'd be specific to OS X (knowing 
> there is no QWindow::setFrameGeometry). I won't be able to test that on MS 
> Windows though.
> 
> What integration level are you invoking? This dependency doesn't make 
> kconfig a Tier 2 framework, does it? Is it so bad to add a dependency on 
> Qt5Widgets to something that already depends on Qt5Gui?
> 
> A more fundamental question would be why this is in KConfig. One could 
> argue that window size (and position) are not application configuration 
> parameters when they're saved automatically; they're a reflection of 
> application interface state (@). Maybe a subtle difference (and maybe a 
> debate that was already held a long time ago), but doesn't this rather (or 
> better) belong in KWindowSystem?
> 
> @) Off-topic, but like other state variables saved automatically it might 
> even be wise to save them in a separate file so it's easier to reset state 
> without doing a full "factory reset".

In Qt5, a dependency on QtWidgets is the difference between having to use a 
QGuiApplication (without) and QApplication (with). QtWidgets is a 20MB or so 
dependency, so in terms of library load times at runtime the difference is 
somewhat significant.


- Boudhayan


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


On Dec. 13, 2015, 7:24 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126324/
> ---
> 
> (Updated Dec. 13, 2015, 7:24 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and KDE Frameworks.
> 
> 
> Repository: kconfig
> 
> 
> Description
> ---
> 
> In KDElibs4, the KMainWindow::saveWindowSize() and 
> KMainWindow::restoreWindowSize() function saved and restored not only the 
> size but also the position (i.e. the geometry) of windows, using 
> QWidget::saveGeometry and QWidget::restoreGeometry.
> 
> 2 main reasons for this (according to the comments):
> - Under X11 restoring the position is tricky
> - X11 has a window manager which might be considered responsible for that 
> functionality (and I suppose most modern WMs have the feature enabled by 
> default?)
> 
> Both arguments are moot on MS Windows and OS X, and on both platforms users 
> expect to see window positions restored as well as window size. On OS X there 
> is also little choice in the matter: most applications offer the geometry 
> restore without asking (IIRC it is the same on MS Windows).
> 
> I would thus like to propose to port the platform-specific code that existed 
> for MS Windows (and for OS X as a MacPorts patch that apparently was never 
> submitted upstreams). I realise that this violates the message conveyed by 
> the function names but I would like to think that this is a case where 
> function is more important.
> 
> You may also notice that the Mac version does not store resolution-specific 
> settings. This happens to work best on OS X, where multi-screen support has 
> been present since the early nineties, and where window geometry is restored 
> regardless of the screen resolution (i.e. connect a different external screen 
> with a different resolution, and windows will reopen as they were on that 
> screen, not with some default geometry).
> I required I can update the comments in the header to reflect this subtlety.
> 
> Note that for optimal functionality a companion patch to `KMainWindow::event` 
> is required:
> ```
> --- a/src/kmainwindow.cpp
> +++ b/src/kmainwindow.cpp
> @@ -772,7 +772,7 @@ bool KMainWindow::event(QEvent *ev)
>  {
>  K_D(KMainWindow);
>  switch (ev->type()) {
> -#ifdef Q_OS_WIN
> 

Re: Review Request 126324: [MSWin/OS X] save and restore window geometry instead of only size (WIP/Suggestion)

[Boudhayan Gupta]

> On Dec. 14, 2015, 12:23 p.m., Martin Gräßlin wrote:
> > src/gui/CMakeLists.txt, line 2
> > <https://git.reviewboard.kde.org/r/126324/diff/3/?file=421934#file421934line2>
> >
> > this introduces a QtWidgets dependency and thus changes the integration 
> > level of the framework. I highly recommend to not go this way.
> > 
> > Looking at the code there is no reason for this. The usage of 
> > QDesktopWidget is not needed as QScreen provides the same. Similar one 
> > doesn't need the QWidget usage as QWindow is already there.
> 
> René J.V. Bertin wrote:
> I'm all for reducing the number of dependencies, but haven't found 
> another way to get at QWidget::saveGeometry and QWidget::restoreGeometry.
> You're probably much more familiar with what those functions really save 
> and restore, and thus to what extent they're essentially convenience 
> functions here for something I could just as well access via 
> QWindow::geometry or QWindow::frameGeometry. I'd have to figure out on my end 
> which of the two I'd need to use because that'd be specific to OS X (knowing 
> there is no QWindow::setFrameGeometry). I won't be able to test that on MS 
> Windows though.
> 
> What integration level are you invoking? This dependency doesn't make 
> kconfig a Tier 2 framework, does it? Is it so bad to add a dependency on 
> Qt5Widgets to something that already depends on Qt5Gui?
> 
> A more fundamental question would be why this is in KConfig. One could 
> argue that window size (and position) are not application configuration 
> parameters when they're saved automatically; they're a reflection of 
> application interface state (@). Maybe a subtle difference (and maybe a 
> debate that was already held a long time ago), but doesn't this rather (or 
> better) belong in KWindowSystem?
> 
> @) Off-topic, but like other state variables saved automatically it might 
> even be wise to save them in a separate file so it's easier to reset state 
> without doing a full "factory reset".
> 
> Boudhayan Gupta wrote:
> In Qt5, a dependency on QtWidgets is the difference between having to use 
> a QGuiApplication (without) and QApplication (with). QtWidgets is a 20MB or 
> so dependency, so in terms of library load times at runtime the difference is 
> somewhat significant.
> 
> René J.V. Bertin wrote:
> I'm sensitive to that kind of argument. Interesting btw; my OS X 
> QtWidgets library is 8.6Mb (which should include debug info), but on Linux 
> using the same script for installing Qt it is 97Mb (!). So there is indeed 
> some reason to introduce the extra dependency only on platforms where it's 
> needed, if it's needed.
> 
> But (something I cannot NOT ask in this context) : just how many 
> applications are there that present a user interface without any need for 
> QtWidgets in any of their dependencies?

The idea is that you can write OpenGL apps without introducing the QtWidgets 
dependency, by using Qt's event loop and drawing on a QWindow. The KDE 
Frameworks follow this philosophy - because they're supposed to be used 
independently of KDE, the frameworks strive to minimise their dependency 
footprint.


- Boudhayan


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


On Dec. 13, 2015, 7:24 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126324/
> ---
> 
> (Updated Dec. 13, 2015, 7:24 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and KDE Frameworks.
> 
> 
> Repository: kconfig
> 
> 
> Description
> ---
> 
> In KDElibs4, the KMainWindow::saveWindowSize() and 
> KMainWindow::restoreWindowSize() function saved and restored not only the 
> size but also the position (i.e. the geometry) of windows, using 
> QWidget::saveGeometry and QWidget::restoreGeometry.
> 
> 2 main reasons for this (according to the comments):
> - Under X11 restoring the position is tricky
> - X11 has a window manager which might be considered responsible for that 
> functionality (and I suppose most modern WMs have the feature enabled by 
> default?)
> 
> Both arguments are moot on MS Windows and OS X, and on both platforms users 
> expect to see window positions restored as well as window size. On OS X there 
> is also little choice in the matter: most applications offer the geometry 
> restore without as

Re: Review Request 126324: [MSWin/OS X] save and restore window geometry instead of only size (WIP/Suggestion)

[Boudhayan Gupta]

> On Dec. 14, 2015, 5:17 p.m., Marco Martin wrote:
> > src/gui/CMakeLists.txt, line 22
> > <https://git.reviewboard.kde.org/r/126324/diff/3/?file=421934#file421934line22>
> >
> > we really can't add a dependency to QWidgets here.
> > depending on qwidgets will make many potential interested project not 
> > able to use it
> 
> Alex Richardson wrote:
> Also shouldn't all that information be available using QScreen/QWindow? 
> It doesn't seem to save much: 
> http://code.woboq.org/qt5/qtbase/src/widgets/kernel/qwidget.cpp.html#_ZNK7QWidget12saveGeometryEv
> 
> René J.V. Bertin wrote:
> Are you more or less inviting me to "clone" QWidget::saveGeometry? It 
> *does* do more than simply saving frame position and window size (I have a 
> working prototype that saves and restores just that, but I guess you'd want 
> me to include maximised state and what else?)
> 
> Boudhayan Gupta wrote:
> Don't just lift code. See what it does and then implement it your own 
> way. Licensing between Qt and KDE are incompatible (quote: code may not be 
> copied from Qt into KDE Platform as Qt is LGPL 2.1 only which would prevent 
> it being used under LGPL 3 - from 
> https://techbase.kde.org/Policies/Licensing_Policy)

Actually, René, I think it would be prudent to at least try and push a patch 
upstream to Qt implementing the window geometry APIs on QWindow.


- Boudhayan


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


On Dec. 13, 2015, 7:24 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126324/
> ---
> 
> (Updated Dec. 13, 2015, 7:24 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and KDE Frameworks.
> 
> 
> Repository: kconfig
> 
> 
> Description
> ---
> 
> In KDElibs4, the KMainWindow::saveWindowSize() and 
> KMainWindow::restoreWindowSize() function saved and restored not only the 
> size but also the position (i.e. the geometry) of windows, using 
> QWidget::saveGeometry and QWidget::restoreGeometry.
> 
> 2 main reasons for this (according to the comments):
> - Under X11 restoring the position is tricky
> - X11 has a window manager which might be considered responsible for that 
> functionality (and I suppose most modern WMs have the feature enabled by 
> default?)
> 
> Both arguments are moot on MS Windows and OS X, and on both platforms users 
> expect to see window positions restored as well as window size. On OS X there 
> is also little choice in the matter: most applications offer the geometry 
> restore without asking (IIRC it is the same on MS Windows).
> 
> I would thus like to propose to port the platform-specific code that existed 
> for MS Windows (and for OS X as a MacPorts patch that apparently was never 
> submitted upstreams). I realise that this violates the message conveyed by 
> the function names but I would like to think that this is a case where 
> function is more important.
> 
> You may also notice that the Mac version does not store resolution-specific 
> settings. This happens to work best on OS X, where multi-screen support has 
> been present since the early nineties, and where window geometry is restored 
> regardless of the screen resolution (i.e. connect a different external screen 
> with a different resolution, and windows will reopen as they were on that 
> screen, not with some default geometry).
> I required I can update the comments in the header to reflect this subtlety.
> 
> Note that for optimal functionality a companion patch to `KMainWindow::event` 
> is required:
> ```
> --- a/src/kmainwindow.cpp
> +++ b/src/kmainwindow.cpp
> @@ -772,7 +772,7 @@ bool KMainWindow::event(QEvent *ev)
>  {
>  K_D(KMainWindow);
>  switch (ev->type()) {
> -#ifdef Q_OS_WIN
> +#if defined(Q_OS_WIN) || defined(Q_OS_OSX)
>  case QEvent::Move:
>  #endif
>  case QEvent::Resize:
> ```
> 
> This ensures that the window geometry save is performed also after a move (to 
> update the position) without requiring a dummy resizing operation.
> Do I need to create a separate RR for this change or is it small enough that 
> I can push it if and when this RR is accepted?
> 
> 
> Diffs
> -
> 
>   src/gui/CMakeLists.txt 9663e09 
>   src/gui/kwindowconfig.h 48a8f3c 
>   src/gui/kwindowconfig.cpp d2f355c 
> 
> Diff: https://git.reviewboa

Re: Review Request 126324: [MSWin/OS X] save and restore window geometry instead of only size (WIP/Suggestion)

[Boudhayan Gupta]

> On Dec. 14, 2015, 5:17 p.m., Marco Martin wrote:
> > src/gui/CMakeLists.txt, line 22
> > 
> >
> > we really can't add a dependency to QWidgets here.
> > depending on qwidgets will make many potential interested project not 
> > able to use it
> 
> Alex Richardson wrote:
> Also shouldn't all that information be available using QScreen/QWindow? 
> It doesn't seem to save much: 
> http://code.woboq.org/qt5/qtbase/src/widgets/kernel/qwidget.cpp.html#_ZNK7QWidget12saveGeometryEv
> 
> René J.V. Bertin wrote:
> Are you more or less inviting me to "clone" QWidget::saveGeometry? It 
> *does* do more than simply saving frame position and window size (I have a 
> working prototype that saves and restores just that, but I guess you'd want 
> me to include maximised state and what else?)

Don't just lift code. See what it does and then implement it your own way. 
Licensing between Qt and KDE are incompatible (quote: code may not be copied 
from Qt into KDE Platform as Qt is LGPL 2.1 only which would prevent it being 
used under LGPL 3 - from https://techbase.kde.org/Policies/Licensing_Policy)


- Boudhayan


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


On Dec. 13, 2015, 7:24 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126324/
> ---
> 
> (Updated Dec. 13, 2015, 7:24 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and KDE Frameworks.
> 
> 
> Repository: kconfig
> 
> 
> Description
> ---
> 
> In KDElibs4, the KMainWindow::saveWindowSize() and 
> KMainWindow::restoreWindowSize() function saved and restored not only the 
> size but also the position (i.e. the geometry) of windows, using 
> QWidget::saveGeometry and QWidget::restoreGeometry.
> 
> 2 main reasons for this (according to the comments):
> - Under X11 restoring the position is tricky
> - X11 has a window manager which might be considered responsible for that 
> functionality (and I suppose most modern WMs have the feature enabled by 
> default?)
> 
> Both arguments are moot on MS Windows and OS X, and on both platforms users 
> expect to see window positions restored as well as window size. On OS X there 
> is also little choice in the matter: most applications offer the geometry 
> restore without asking (IIRC it is the same on MS Windows).
> 
> I would thus like to propose to port the platform-specific code that existed 
> for MS Windows (and for OS X as a MacPorts patch that apparently was never 
> submitted upstreams). I realise that this violates the message conveyed by 
> the function names but I would like to think that this is a case where 
> function is more important.
> 
> You may also notice that the Mac version does not store resolution-specific 
> settings. This happens to work best on OS X, where multi-screen support has 
> been present since the early nineties, and where window geometry is restored 
> regardless of the screen resolution (i.e. connect a different external screen 
> with a different resolution, and windows will reopen as they were on that 
> screen, not with some default geometry).
> I required I can update the comments in the header to reflect this subtlety.
> 
> Note that for optimal functionality a companion patch to `KMainWindow::event` 
> is required:
> ```
> --- a/src/kmainwindow.cpp
> +++ b/src/kmainwindow.cpp
> @@ -772,7 +772,7 @@ bool KMainWindow::event(QEvent *ev)
>  {
>  K_D(KMainWindow);
>  switch (ev->type()) {
> -#ifdef Q_OS_WIN
> +#if defined(Q_OS_WIN) || defined(Q_OS_OSX)
>  case QEvent::Move:
>  #endif
>  case QEvent::Resize:
> ```
> 
> This ensures that the window geometry save is performed also after a move (to 
> update the position) without requiring a dummy resizing operation.
> Do I need to create a separate RR for this change or is it small enough that 
> I can push it if and when this RR is accepted?
> 
> 
> Diffs
> -
> 
>   src/gui/CMakeLists.txt 9663e09 
>   src/gui/kwindowconfig.h 48a8f3c 
>   src/gui/kwindowconfig.cpp d2f355c 
> 
> Diff: https://git.reviewboard.kde.org/r/126324/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.6 through 10.9 with various KDElibs4 versions and now with Qt 
> 5.5.1 and frameworks 5.16.0 (and Kate as a test application).
> I presume that the MS Windows code has been tested sufficiently in KDELibs4; 
> I have only adapted it to Qt5 and tested if it builds.
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org

Re: Moving KWayland to frameworks

[Boudhayan Gupta]

On 10 December 2015 at 12:32, Martin Graesslin  wrote:
> yes, that's the annoying one which is lots of work and lots of possible
> regressions. I'm not sure whether I'm willing to do that work which I consider
> a useless exercise.

In my opinion this discussion is an useless exercise, because:

> Well it doesn't need to be a global bump of compiler requirements. But we
> could consider different compiler requirements for frameworks which are non-
> portable. KWayland will never be built on Windows neither on OSX. So any
> compiler restrictions on it just shouldn't matter.

Exactly. Anything that's Wayland related is supposed to be on the
bleeding edge right now. Setting compiler restrictions on this
particular framework will serve only to (apologies for my language,
but there really is not a more apt term for this) cockblock KWayland
development. Developing new technologies from scratch is hard; being
unable to use easier constructs for solving problems not only makes
your job harder but frustrates you more (it's there, why can't I use
this?)

I may be wrong, but I'm guessing Wayland came quite a while after
gcc-4.5 did, and that they use features that require gcc>4.5.

IMO non-portable frameworks should not be subject to the global
compiler requirements at all, but use more platform-appropriate
compiler requirements.

-- Boudhayan
___
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

[Boudhayan Gupta]

> On Dec. 6, 2015, 5:32 p.m., Vishesh Handa wrote:
> > 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?

I'm going to sit and think on this for a while.

I wanted to get a minimally invasive solution to the problem, but if you want 
it doen **right**, I'd have to think things through.

I don't think 3 will ever be a problem - I'm suggesting that people will 
install only one plugin per mimetype, if someone wants a plugin using different 
tech, they'll swap out the current plugin (by removing it from their system). 
For 4, we can put the somewhere in libexec. For 5, I don't want to use .desktop 
files. Maybe KPackage based solutions? Or even simpler, a bunch of json files 
in some subdir of /etc.


- Boudhayan


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


On Oct. 24, 2015, 5:49 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, 5:49 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: Purpose as a KDE Framework

[Boudhayan Gupta]

Hi Aleix,

On 3 December 2015 at 23:51, Aleix Pol <aleix...@kde.org> wrote:
> On Tue, Dec 1, 2015 at 5:10 PM, Boudhayan Gupta <bgu...@kde.org> wrote:
>> On 1 December 2015 at 20:42, Aleix Pol <aleix...@kde.org> wrote:
>>>> 1. I'd like to be able to load the list of plugins available per
>>>> mimetype a priori before the filename to share becomes available.
>>> You can pass { "mimetype": "image/png", "urls": [] }.
>>>
>>> That should work.
>>> I'm not sure why you'd want to list plugins before having anything to
>>> share though...
>>
>> That lists the plugins; how do I now execute them with a URL?
> Just show me the code you have to port it to Purpose and I'll try and
> see how to fit it in the API... Is it in a branch?

Nope, it's in master right now. Basically the whole KIPI integration
code - right now I load the plugins at one go and later execute a
plugin with the file name - which is generated at the last moment
because I defer saving the temp file as far as possible.

Maybe we can find some time on IRC and discuss this realtime?

>> Another thing - What if I want to exclude a specific plugin from
>> displaying in the menu, such as SaveAs?
>
> We can look into this. It's not done so far. Do you think it's /that/
> terrible that it's offered by default?

Well the plugin itself isn't a bad idea, but I'd want to blacklist
some plugins if I do have native functionality in my app for that.
Saving is one such thing, in KIPI I blacklist printing because I do it
better in my code.

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

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[Boudhayan Gupta]

> On Dec. 2, 2015, 3:15 p.m., Boudhayan Gupta wrote:
> > src/platformtheme/kdemactheme.mm, line 39
> > <https://git.reviewboard.kde.org/r/126198/diff/3/?file=420466#file420466line39>
> >
> > This makes me very nervous.
> > 
> > Using private APIs is almost always a guarantee the application won't 
> > preserve binary compatibility even across point releases (eg Qt 5.5.0 - 
> > 5.5.1), let alone across major releases.
> > 
> > What makes it even more dangerous is that this file won't be built only 
> > on MacPorts/Fink but also by people making normal AppBundle releases - 
> > where you have no control over what versions of dependencies are being used.
> > 
> > Please try to find a public API alternative, even if it ends up being a 
> > giant hack. I once found a very elegant private API solution to making a 
> > QQuickWidget emit a release event on the mouse cursor, but just because of 
> > the whole binary compat problem (some Linux distros don't even ship apps 
> > depending on private headers) I had to create a dummy item inside the 
> > QtQuick code and interact with it to get the cursor released. Giant hack, 
> > but at least no private API usage.
> > 
> > Private API is **private** for a reason.
> 
> Luigi Toscano wrote:
> Afaik frameworkintegration is supposed to be an extension of the internal 
> Qt world and it has already been the case (I asked the same question few 
> Akademys ago). If you want to use QPA, you need to go down in the stack.
> 
> Martin Gräßlin wrote:
> yeah frameworkintegration is bound to use private API. So not really a 
> problem in this special case.
> 
> AFAIK some distros have adjusted the packaging to force a recompile for 
> new Qt releases.
> 
> Boudhayan Gupta wrote:
> Ah, that makes sense. Okay, I'm dropping this issue.
> 
> René J.V. Bertin wrote:
> Whew - Hacking is so much more fun if you can do it there where you're 
> not supposed to go, rather than to avoid exactly that ;)
> However, it might be possible to replace the direct call of the private 
> function by a call through a function pointer to `createPlatformTheme` 
> obtained through a dynamic resolver. Any idea if that is bound to fail 
> through a Qt function because it'd detect a request for a private API?
> 
> In any case, you (Boudhayan) are right that this modified framework might 
> end up being used also in *standalone* AppBundle builds rather than only in 
> builds using shared resources (in which applications are also built as app 
> bundles!). At least I hope it will, for the very reason I've been pushing the 
> patch. But I think you're wrong to assume that using private APIs is a 
> problem there. If anything it should be (much) less the case, because 
> standalone app bundles form a much more protected environment. They're not 
> meant to be upgraded internally, except maybe by some special mechanism 
> controlled by the developer(s) in charge. Updates to Qt without an 
> accompanying forced rebuild ("revbump" in MacPorts speak) to 
> frameworkintegration are much more likely to occur in a shared environment.

There's no reason to go through a dynamic function resolver if the rest of FWI 
uses private APIs happily. Luigi says, "frameworkintegration is supposed to be 
an extension of the internal Qt world" - in that case it makes perfect sense to 
make use of private APIs to get your job done.

As for the AppBundles point - I'm thinking if it's possible Qt is installed 
from e.g. the SDKs provided by qt.io and standalone AppBundles end up depending 
on that. If not, using private APIs in standalone AppBundles makes no 
difference - you won't upgrade Qt without upgrading the entire AppBundle.


- Boudhayan


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


On Dec. 2, 2015, 1:59 a.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126198/
> ---
> 
> (Updated Dec. 2, 2015, 1:59 a.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie 
> Zimmerman.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to 
> Qt5; all that is required is to include the `qgenericunixservices` and 
> `qgenericunixthemes`

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[Boudhayan Gupta]

> On Dec. 2, 2015, 3:15 p.m., Boudhayan Gupta wrote:
> > src/platformtheme/kdemactheme.mm, line 39
> > <https://git.reviewboard.kde.org/r/126198/diff/3/?file=420466#file420466line39>
> >
> > This makes me very nervous.
> > 
> > Using private APIs is almost always a guarantee the application won't 
> > preserve binary compatibility even across point releases (eg Qt 5.5.0 - 
> > 5.5.1), let alone across major releases.
> > 
> > What makes it even more dangerous is that this file won't be built only 
> > on MacPorts/Fink but also by people making normal AppBundle releases - 
> > where you have no control over what versions of dependencies are being used.
> > 
> > Please try to find a public API alternative, even if it ends up being a 
> > giant hack. I once found a very elegant private API solution to making a 
> > QQuickWidget emit a release event on the mouse cursor, but just because of 
> > the whole binary compat problem (some Linux distros don't even ship apps 
> > depending on private headers) I had to create a dummy item inside the 
> > QtQuick code and interact with it to get the cursor released. Giant hack, 
> > but at least no private API usage.
> > 
> > Private API is **private** for a reason.
> 
> Luigi Toscano wrote:
> Afaik frameworkintegration is supposed to be an extension of the internal 
> Qt world and it has already been the case (I asked the same question few 
> Akademys ago). If you want to use QPA, you need to go down in the stack.
> 
> Martin Gräßlin wrote:
> yeah frameworkintegration is bound to use private API. So not really a 
> problem in this special case.
> 
> AFAIK some distros have adjusted the packaging to force a recompile for 
> new Qt releases.

Ah, that makes sense. Okay, I'm dropping this issue.


- Boudhayan


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


On Dec. 2, 2015, 1:59 a.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126198/
> ---
> 
> (Updated Dec. 2, 2015, 1:59 a.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie 
> Zimmerman.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to 
> Qt5; all that is required is to include the `qgenericunixservices` and 
> `qgenericunixthemes` components in the build, and to append `"kde"` to the 
> list returned by `QCocoaIntegration::themeNames()` for instance under the 
> condition that `KDE_SESSION_VERSION` is set to a suitable value in the 
> environment.
> 
> This will allow KF5 and Qt5 applications to use the theme selected through 
> KDE if FrameworkIntegration is installed and KDE_SESSION_VERSION is set, 
> which seems like a sufficiently specific set of conditions that is easy to 
> avoid by users who prefer to use the Mac native theme.
> 
> While requestion the KDE theme is also possible through `-style kde` or `env 
> QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be 
> the only way to get the full theme, including the font and colour selection. 
> In my opinion it is above all the font customisation which is a very welcome 
> feature for Qt/Mac; by default Qt applications use the default system font 
> (Lucida Grande 13pt or even 14pt) throughout. This is a good UI font, but not 
> at that size (and most "native" OS X applications indeed use a range of 
> smaller sizes, depending on role.
> 
> It does have introduce a number of regressions, which the current patch aims 
> to address. The most visible and problematic of these regressions is the loss 
> of the Mac-style menu bar and thus of all menu items (actions).
> 
> The fix is straightforward : on OS X (and similarly affected platforms?), an 
> instance of the native Cocoa platform theme is created through the private 
> API, and used as a fallback rather than immediately falling back to the 
> default implementations from `QPlatformTheme`. In addition, methods missing 
> from (not overridden by) `KdePlatformTheme` are provided on OS X and call the 
> corresponding methods from the native theme. It is this change which restores 
> the menubar and even the Dock menu functionality.
> One minor regression remains but should 

Re: Review Request 126198: [OS X] adaptations for the KdePlatformTheme (and autotests)

[Boudhayan Gupta]

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



src/platformtheme/kdemactheme.mm (line 39)
<https://git.reviewboard.kde.org/r/126198/#comment60954>

This makes me very nervous.

Using private APIs is almost always a guarantee the application won't 
preserve binary compatibility even across point releases (eg Qt 5.5.0 - 5.5.1), 
let alone across major releases.

What makes it even more dangerous is that this file won't be built only on 
MacPorts/Fink but also by people making normal AppBundle releases - where you 
have no control over what versions of dependencies are being used.

Please try to find a public API alternative, even if it ends up being a 
giant hack. I once found a very elegant private API solution to making a 
QQuickWidget emit a release event on the mouse cursor, but just because of the 
whole binary compat problem (some Linux distros don't even ship apps depending 
on private headers) I had to create a dummy item inside the QtQuick code and 
interact with it to get the cursor released. Giant hack, but at least no 
private API usage.

Private API is **private** for a reason.


- Boudhayan Gupta


On Dec. 2, 2015, 1:59 a.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126198/
> ---
> 
> (Updated Dec. 2, 2015, 1:59 a.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Valorie 
> Zimmerman.
> 
> 
> Repository: frameworkintegration
> 
> 
> Description
> ---
> 
> The KdePlatformTheme plugin can be enabled on OS X with a minimal patch to 
> Qt5; all that is required is to include the `qgenericunixservices` and 
> `qgenericunixthemes` components in the build, and to append `"kde"` to the 
> list returned by `QCocoaIntegration::themeNames()` for instance under the 
> condition that `KDE_SESSION_VERSION` is set to a suitable value in the 
> environment.
> 
> This will allow KF5 and Qt5 applications to use the theme selected through 
> KDE if FrameworkIntegration is installed and KDE_SESSION_VERSION is set, 
> which seems like a sufficiently specific set of conditions that is easy to 
> avoid by users who prefer to use the Mac native theme.
> 
> While requestion the KDE theme is also possible through `-style kde` or `env 
> QT_STYLE_OVERRIDE=kde` the use of the KdePlatformTheme plugin appears to be 
> the only way to get the full theme, including the font and colour selection. 
> In my opinion it is above all the font customisation which is a very welcome 
> feature for Qt/Mac; by default Qt applications use the default system font 
> (Lucida Grande 13pt or even 14pt) throughout. This is a good UI font, but not 
> at that size (and most "native" OS X applications indeed use a range of 
> smaller sizes, depending on role.
> 
> It does have introduce a number of regressions, which the current patch aims 
> to address. The most visible and problematic of these regressions is the loss 
> of the Mac-style menu bar and thus of all menu items (actions).
> 
> The fix is straightforward : on OS X (and similarly affected platforms?), an 
> instance of the native Cocoa platform theme is created through the private 
> API, and used as a fallback rather than immediately falling back to the 
> default implementations from `QPlatformTheme`. In addition, methods missing 
> from (not overridden by) `KdePlatformTheme` are provided on OS X and call the 
> corresponding methods from the native theme. It is this change which restores 
> the menubar and even the Dock menu functionality.
> One minor regression remains but should be easy to fix (elsewhere?): the 
> Preferences menu loses its keyboard shortcut (Command-,).
> 
> Given the fallback nature of the native platform instance I have preferred to 
> print a warning rather than using something like `qFatal`, above all because 
> the message printed by qFatal tends to get lost on OS X. I can replace my use 
> of `qWarning` with a dialog giving the choice between continuing or exiting 
> the application - code that would be called in the menu methods because only 
> there is it certain that the application actually needs a menubar.
> 
> In line with experience and feedback from the KDE(4)-Mac community I have 
> decided to force the use of native dialogs rather than the ones from the 
> KdePlatformPlugin.
> 
> In addition I set the fallback value for `ShowIconsOnPushButtons` to false in 
> line w

Re: Purpose as a KDE Framework

[Boudhayan Gupta]

On 1 December 2015 at 19:31, Aleix Pol  wrote:
> Hi,
> I've been working on Purpose since some months now, with the intention
> of becoming a framework some day.

+1. I intend to port Spectacle to use it instead of KIPI for uploading
images to online sites, and I love the API's simplicity.

I only have two concerns:

1. I'd like to be able to load the list of plugins available per
mimetype a priori before the filename to share becomes available.

2. I'd also like some feature to load the plugins in some parallel
thread or something. Currently KIPI has about 30 plugins and it blocks
the GUI thread for 5-6 seconds while it loads them. I'd want to avoid
that when the plugins grow.

Otherwise, looking forward to seeing it as a Framework! Splitting the
plugins into a separate package (purpose-plugins) as Martin said
sounds like a snazzy idea as well.


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

Re: Purpose as a KDE Framework

[Boudhayan Gupta]

On 1 December 2015 at 20:42, Aleix Pol  wrote:
>> 1. I'd like to be able to load the list of plugins available per
>> mimetype a priori before the filename to share becomes available.
> You can pass { "mimetype": "image/png", "urls": [] }.
>
> That should work.
> I'm not sure why you'd want to list plugins before having anything to
> share though...

That lists the plugins; how do I now execute them with a URL?

Another thing - What if I want to exclude a specific plugin from
displaying in the menu, such as SaveAs?

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

Re: Scope of framework integration plugin?

[Boudhayan Gupta]

On 30 November 2015 at 17:32, Aleix Pol  wrote:
> On Mon, Nov 30, 2015 at 12:48 PM, Martin Graesslin  wrote:
>> Opinions?
>
> Hi,
> I agree with you, I've proposed the same thing as you in the past,
> although there's some issues that would then need to be sorted.
>
> What you actually want is, IMHO, to move the QPlatformTheme plugin
> together with Plasma, but frameworksintegration is not entirely about
> that nowadays: it has code that integrates different frameworks with
> each other:
> - FrameworkIntegrationPlugin: this doesn't really belong in plasma,
> integrates KMessageBox with KNotifications.
> - infopage: this is used by applications as well as plasma
> - KStyle: Plasma can provide styles, but this seems more like a QStyle
> creation framework.
> - platformtheme: Plasma integration. There I agree.

Hi,

Unless I'm much mistaken, the KDE HIG encourages application UIs to be
designed in such a way that they don't look out of place on other
platforms. Therefore the KDE theming bits shouldn't even need to exist
on other platforms (unless you're one of those people who'd rather
spend time making your "Windows XP look like KDE" over using your
computer for actual work - and then blogging about said endeavour).

Applications are supposed to use the platform's native style, icons,
font settings etc. We don't want users on other platforms to open a
KDE application and suddenly feel a jarring difference between that
and their regular apps. We want all KDE apps to blend in.

So I'm gonna have to agree with Martin here - the PlatformTheme bits
should be moved over to Workspace, since that's where they belong. If
other platforms don't provide a PlatformTheme plugin - I'm tempted to
say "their problem, not ours", but we can at least help out if they
stumble. Not to say Qt already provides generic fallbacks.

Yours,
Boudhayan
___
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()

[Boudhayan Gupta]

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

(Updated Nov. 24, 2015, 11:42 a.m.)


Status
--

This change has been marked as submitted.


Review request for Baloo, KDE Frameworks, Pinak Ahuja, and Vishesh Handa.


Changes
---

Submitted with commit 22ade345757b7c080597f5f062b7ebb6e45a2c3d by Boudhayan 
Gupta to branch master.


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 126091: Remove printscreen.khotkeys from KHotkeys package

[Boudhayan Gupta]

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

(Updated Nov. 24, 2015, 11:53 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, Plasma and David Edmundson.


Changes
---

Submitted with commit a8c177911dea1e099bfb499f199ee7039c756cde by Boudhayan 
Gupta to branch master.


Repository: khotkeys


Description
---

After moving KSnapshot to Extragear I've modified it to install its own 
khotkeys file (which it does correctly). Spectacle already installs its own 
KHotkeys file, so there's no reason anymore for KHotkeys to include its own 
PrintScreen hotkey file.

I'll push out a special release of KSnapshot for distributions to use once the 
Plasma 5.5 version of KHotkeys is pushed out, so printscreen functionality 
won't be broken on the desktop (assuming, that is, distros push out the updates 
together).


Diffs
-

  data/CMakeLists.txt f3bcb57 
  data/printscreen.khotkeys e583311 

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


Testing
---

Doesn't install the printscreen.khotkeys file anymore


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 126091: Remove printscreen.khotkeys from KHotkeys package

[Boudhayan Gupta]

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


Since there has been no negative comments from the kde-distro-packagers mailing 
list posting (in fact, no comments at all), I'm committing this to Plasma/5.5 
and merging to master.

Reference to posting: 
https://mail.kde.org/pipermail/kde-distro-packagers/2015-November/000107.html

- Boudhayan Gupta


On Nov. 16, 2015, 10:36 p.m., Boudhayan Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126091/
> ---
> 
> (Updated Nov. 16, 2015, 10:36 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma and David Edmundson.
> 
> 
> Repository: khotkeys
> 
> 
> Description
> ---
> 
> After moving KSnapshot to Extragear I've modified it to install its own 
> khotkeys file (which it does correctly). Spectacle already installs its own 
> KHotkeys file, so there's no reason anymore for KHotkeys to include its own 
> PrintScreen hotkey file.
> 
> I'll push out a special release of KSnapshot for distributions to use once 
> the Plasma 5.5 version of KHotkeys is pushed out, so printscreen 
> functionality won't be broken on the desktop (assuming, that is, distros push 
> out the updates together).
> 
> 
> Diffs
> -
> 
>   data/CMakeLists.txt f3bcb57 
>   data/printscreen.khotkeys e583311 
> 
> Diff: https://git.reviewboard.kde.org/r/126091/diff/
> 
> 
> Testing
> ---
> 
> Doesn't install the printscreen.khotkeys file anymore
> 
> 
> 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()

[Boudhayan Gupta]

> On Nov. 24, 2015, 4:20 p.m., Vishesh Handa wrote:
> > src/engine/database.cpp, line 73
> > <https://git.reviewboard.kde.org/r/126110/diff/1/?file=417424#file417424line73>
> >
> > Please remove the comment. It's not longer valid.

We still are checking if the index exists and bailing out early if not, but 
sure, I'll remove the comment.


> On Nov. 24, 2015, 4:20 p.m., Vishesh Handa wrote:
> > src/engine/database.cpp, line 199
> > <https://git.reviewboard.kde.org/r/126110/diff/1/?file=417424#file417424line199>
> >
> > Perhaps you want to close the env here as well? The assert can fail in 
> > production.

Done (in both RW and RO cases).


- Boudhayan


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


On Nov. 19, 2015, 5:09 p.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, 5:09 p.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 126091: Remove printscreen.khotkeys from KHotkeys package

[Boudhayan Gupta]

> On Nov. 24, 2015, 5:32 p.m., Bhushan Shah wrote:
> > You commited it to wrong branch.. should be Plasma/5.5 then master.

Pushed master first, sorry :-D


- Boudhayan


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


On Nov. 24, 2015, 5:23 p.m., Boudhayan Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126091/
> ---
> 
> (Updated Nov. 24, 2015, 5:23 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma and David Edmundson.
> 
> 
> Repository: khotkeys
> 
> 
> Description
> ---
> 
> After moving KSnapshot to Extragear I've modified it to install its own 
> khotkeys file (which it does correctly). Spectacle already installs its own 
> KHotkeys file, so there's no reason anymore for KHotkeys to include its own 
> PrintScreen hotkey file.
> 
> I'll push out a special release of KSnapshot for distributions to use once 
> the Plasma 5.5 version of KHotkeys is pushed out, so printscreen 
> functionality won't be broken on the desktop (assuming, that is, distros push 
> out the updates together).
> 
> 
> Diffs
> -
> 
>   data/CMakeLists.txt f3bcb57 
>   data/printscreen.khotkeys e583311 
> 
> Diff: https://git.reviewboard.kde.org/r/126091/diff/
> 
> 
> Testing
> ---
> 
> Doesn't install the printscreen.khotkeys file anymore
> 
> 
> 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 126091: Remove printscreen.khotkeys from KHotkeys package

[Boudhayan Gupta]

> On Nov. 16, 2015, 10:46 p.m., Hrvoje Senjan wrote:
> > >assuming, that is, distros push out the updates together
> > 
> > don't assume this ;-) at least send a mail to kde-distro-packagers about 
> > the sideeffects if those 3 are pushed asynchronously
> 
> Boudhayan Gupta wrote:
> 'course, I'll post there as well as add a big fat notice to the 
> kde-apps-announce posting that announces this special release of KSnapshot.
> 
> So ship it?
> 
> David Edmundson wrote:
> Lets email them first. Check they're ok with it, before we do it.

Haven't had any negative comments (or any comments at all) on the thread yet. 
Should I commit it now (and generate the KSnapshot tarballs)? The Plasma 
release is just days away.


- Boudhayan


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


On Nov. 16, 2015, 10:36 p.m., Boudhayan Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126091/
> ---
> 
> (Updated Nov. 16, 2015, 10:36 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma and David Edmundson.
> 
> 
> Repository: khotkeys
> 
> 
> Description
> ---
> 
> After moving KSnapshot to Extragear I've modified it to install its own 
> khotkeys file (which it does correctly). Spectacle already installs its own 
> KHotkeys file, so there's no reason anymore for KHotkeys to include its own 
> PrintScreen hotkey file.
> 
> I'll push out a special release of KSnapshot for distributions to use once 
> the Plasma 5.5 version of KHotkeys is pushed out, so printscreen 
> functionality won't be broken on the desktop (assuming, that is, distros push 
> out the updates together).
> 
> 
> Diffs
> -
> 
>   data/CMakeLists.txt f3bcb57 
>   data/printscreen.khotkeys e583311 
> 
> Diff: https://git.reviewboard.kde.org/r/126091/diff/
> 
> 
> Testing
> ---
> 
> Doesn't install the printscreen.khotkeys file anymore
> 
> 
> 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 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[Boudhayan Gupta]

> On Nov. 18, 2015, 12:52 p.m., Martin Gräßlin wrote:
> > src/plugins/osx/macpoller.cpp, lines 222-227
> > 
> >
> > seriously? You care about idle timeouts below 5 msec? This is a 
> > framework to tell the application whether the user doesn't use input 
> > devices. I don't know how fast you type, but I'm relatively certain that it 
> > takes more than 5 msec to move my finger from one key to another. What 
> > exactly is that you want to detect here? An event after each key press, 
> > maybe even between a key press and a key release? Because that's probably 
> > already more about 5 msec.
> 
> René J.V. Bertin wrote:
> The timer is used only for the detection of idle timeouts (so *absence* 
> of user input), and the precision with which it fires determines the accuracy 
> with which those events can be timed. This is currently inaccessible anyway, 
> but I don't believe in doing things half-bakedly. One might question whether 
> it makes sense to allow the framework to work at high precision (I more or 
> less agree that remains to be seen). But I don't think it makes sense to do 
> that and then leave the QTimer in a mode where the requested precision cannot 
> be met.
> 
> Martin Gräßlin wrote:
> To get this quite clear: you care about a precision of below 5 msec, this 
> is a third of the time it takes at least till the next frame is rendered. If 
> you are lucky you get the result of a key press or mouse move represented on 
> screen after 16 msec, more likely it's more. And you care about a wrong timer 
> precision below 5 msec. Sorry that's ridiculous. Please don't include such 
> non-sense code. You make that code more difficult to all other developers to 
> maintain. This code has cost if it's included.
> 
> Aleix Pol Gonzalez wrote:
> It's not a matter of doing things "right" or "wrong". It's a matter of 
> priorities.
> 
> KIdleTime is a framework for figuring out whether the system is idle. I 
> don't consider 5ms not using a system as it's being idling.
> 
> René J.V. Bertin wrote:
> Again, we're talking about detecting how long a system has been idle, not 
> whether it is (the framework is called KIdle*Time*, right? :)). The precision 
> in question here applies just as well to (very) short periods (timeouts) as 
> well as to long ones.
> Also, don't confound the 5*ms* lower limit under which I switch to a high 
> precision timer with the 5`%` error of a Qt::CoarseTimer. The precision of a 
> high precision timer may be overkill for longer timeout durations in a 
> framework that has a 1ms granularity, but the effect of 5% error scales with 
> duration.
> 
> I consider that it's up to the user of a feature to know what s/he 
> expects of it, not up to a framework. (Turning that around would probably 
> remove the raison d'être of a lot of software.)
> 
> Finally, I have not been able to measure any increase in overhead when 
> using a (single) high precision timer.
> 
> Granted, this is a bit a matter of principle for me, but as I've said 
> elsewere, I could easily see me having used this in a previous job.
> 
> Martin Gräßlin wrote:
> Rene, please stop it. Don't argue! You won't get this code in with 
> polling and setting the qtimer precision based on clearly irrelevant value. 
> It is our responsibility as fellow KDE developers to prevent such code to get 
> it. I absolutely don't care how you intend to use this code, but this is a 
> user idle framework. A system which had an input event in the last five milli 
> seconds is not idle per definition. Thus the code is idiotic and wrong. 
> Please stop arguing and change the code given the feedback you get here. Same 
> above for the nullptr check. That one has to go, it's a wrong check at that 
> place, remove it.
> 
> Nicolás Alvarez wrote:
> FYI, while you fight and argue about the best way to do high-precision 
> frequent polling on something that doesn't need any precision, I'm waist-deep 
> in a disassembler making good progress figuring out how to do this without 
> any polling at all.

Talk is cheap, show me the code!

**wink**


- Boudhayan


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


On Nov. 18, 2015, 10:05 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126078/
> ---
> 
> (Updated Nov. 18, 2015, 10:05 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Dario Freddi.
> 
> 
> Repository: kidletime
> 
> 
> Description
> ---
> 
> I noticed that the KIdleTime example 

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

[Boudhayan Gupta]

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

Review request for Baloo, KDE Frameworks, Pinak Ahuja, and Vishesh Handa.


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()

[Boudhayan Gupta]

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

(Updated Nov. 19, 2015, 5:09 p.m.)


Review request for Baloo, KDE Frameworks, Pinak Ahuja, and Vishesh Handa.


Changes
---

May possibly fix BUG 353757


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 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[Boudhayan Gupta]

Hi all,

On 19 November 2015 at 15:06, René J.V.  wrote:
> On Thursday November 19 2015 08:10:08 Martin Graesslin wrote:
>> the code else where, but because the issues pointed out would be unresolved
>> giving users of your code a bad framework and thus harming both your users as
>> well as the frameworks project for distributing bad code. The problem of
>> polling is real. Please accept this feedback.
>
> Now it's just bad code, eh?

Thou shalt not poll.

This is for one reason and one reason only - polling causes wakeups,
which consumes both power and processor resources. You're running
warmer, slower and consuming more energy. Microwatts count, because
they add up.

The irony here is that you're polling in a library with the purpose of
increasing power usage efficiency.

That said, here's a concrete way for you to detect both Idletime and
catch resume events:

For idletime, run a QTimer with the interval set to the idle timeout.
Poll the HID time since last event twice - once at timer start and
once at timer end. Check the difference. If it's round about the same
as the timer difference, the system has been idle. Trigger the idle
signal. If the difference is significantly less, adjust the QTimer
interval to check again at a shorter interval - the time at which you
expect the HID time since last event to be the expected value.
Basically, do some math, keep adjusting the timeout value everytime
you reach the timeout.

Now once you're in idle mode, you'll need some way to resume from
idle. This needs a high-precision event source. Not a timer, an event
source. For this, get a keylogger, like the one here -
https://github.com/caseyscarborough/keylogger/blob/master/keylogger.c
- and adjust it so that it responds to key as well as mouse events.
Instead of actually logging the key though, make it emit a tick
through a socket pair or a named pipe, with KIdleTime on the listening
end. Or just use QProcess - start the process when you enter idle, end
the process on the first instance you get some output from the stdout,
which is in response to an input event.

Let's hope this helps you out.

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

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

[Boudhayan Gupta]

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

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

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

[Boudhayan Gupta]

> On Nov. 19, 2015, 1:36 a.m., Thomas Lübking wrote:
> > Errr... you intend to crash applications depending on whether some file is 
> > present??
> > Please fix the actual bug instead of such workaround, got a backtrace?

Err... it **doesn't** cause a crash with the patch, causes a crash without it. 
This is a perfectly valid fix - this function is **supposed** to return false 
if the database could not be opened, for any reason whatsoever.


- Boudhayan


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


On Nov. 19, 2015, 1:10 a.m., Boudhayan Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126105/
> ---
> 
> (Updated Nov. 19, 2015, 1:10 a.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

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

[Boudhayan Gupta]

> On Nov. 19, 2015, 1:36 a.m., Thomas Lübking wrote:
> > Errr... you intend to crash applications depending on whether some file is 
> > present??
> > Please fix the actual bug instead of such workaround, got a backtrace?
> 
> Boudhayan Gupta wrote:
> Err... it **doesn't** cause a crash with the patch, causes a crash 
> without it. This is a perfectly valid fix - this function is **supposed** to 
> return false if the database could not be opened, for any reason whatsoever.
> 
> Thomas Lübking wrote:
> I didn't say this patch would introduce a crash.
> I said that "something" crashes for "some" reason and you work around 
> that by testing whether a file is present.
> Your own commit message clearly says that if - for any reason - 
> ~/.local/share/baloo/index exists (eg. i touch it), things will crash again, 
> what means that the actual problem is not resolved but just shadowed.

Why would you want to touch ~/.local/share/baloo/index, if you disable baloo?

Let me explain - balooctl disable works by disabling indexing and removing 
~/.local/share/baloo/index. If baloo is disabled, that file is not supposed to 
exist. A couple of months ago we were even trying to figure out how that file 
came to exist on systems with Baloo disabled and now I've found out. So this 
fixes that too.

Now the real problem. Before this patch, Baloo::Database::open() would just 
create an empty database. Fair enough, except that db->open() would return true 
in places where you clearly have an invalid database. This would still work 
(i.e., not crash), because invalid in this context is empty. However, once you 
select a ton of files (the number is very weird  - 158 or something) LMDB would 
scream at you with this problem - **MDB_READERS_FULL: Environment maxreaders 
limit reached**

There are two ways to solve this - increase the maxreaders limit in LMDB, or 
return false if the database doesn't exist. The first option is when you want 
to go all Linus - *we do not break userspace*, but this way is the more correct 
way of solving this, because again, on systems where Baloo is disabled, the 
index file isn't supposed to exist at all.

I was afraid I was opening up a can of worms by adding yet another condition 
for db->open() to return false, because I was afraid there were places where a 
check on db->open()'s return value was missing (and someone would try to do 
operations on an invalid database). However, I've done all the things that used 
to trigger crashes before and nothing's crashed yet. With Baloo being both 
enabled and disabled.


- Boudhayan


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


On Nov. 19, 2015, 1:10 a.m., Boudhayan Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126105/
> ---
> 
> (Updated Nov. 19, 2015, 1:10 a.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

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

[Boudhayan Gupta]

> On Nov. 19, 2015, 6:03 a.m., Vishesh Handa wrote:
> > 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.

I concur, the errant calls are being made by Baloo-Widgets and we should 
probably fix Baloo-Widgets to not go through the hassle of invoking lower-level 
baloo functions if it's disabled. I'll look into it - is there a simple API in 
Baloo which I can check to see if indexing is disabled? Can I use 
Baloo::IndexerConfig::fileIndexingEnabled() from Baloo-Widgets to do this check?


- Boudhayan


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


On Nov. 19, 2015, 1:10 a.m., Boudhayan Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126105/
> ---
> 
> (Updated Nov. 19, 2015, 1:10 a.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

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

[Boudhayan Gupta]

> On Nov. 19, 2015, 1:36 a.m., Thomas Lübking wrote:
> > Errr... you intend to crash applications depending on whether some file is 
> > present??
> > Please fix the actual bug instead of such workaround, got a backtrace?
> 
> Boudhayan Gupta wrote:
> Err... it **doesn't** cause a crash with the patch, causes a crash 
> without it. This is a perfectly valid fix - this function is **supposed** to 
> return false if the database could not be opened, for any reason whatsoever.
> 
> Thomas Lübking wrote:
> I didn't say this patch would introduce a crash.
> I said that "something" crashes for "some" reason and you work around 
> that by testing whether a file is present.
> Your own commit message clearly says that if - for any reason - 
> ~/.local/share/baloo/index exists (eg. i touch it), things will crash again, 
> what means that the actual problem is not resolved but just shadowed.
> 
> Boudhayan Gupta wrote:
> Why would you want to touch ~/.local/share/baloo/index, if you disable 
> baloo?
> 
> Let me explain - balooctl disable works by disabling indexing and 
> removing ~/.local/share/baloo/index. If baloo is disabled, that file is not 
> supposed to exist. A couple of months ago we were even trying to figure out 
> how that file came to exist on systems with Baloo disabled and now I've found 
> out. So this fixes that too.
> 
> Now the real problem. Before this patch, Baloo::Database::open() would 
> just create an empty database. Fair enough, except that db->open() would 
> return true in places where you clearly have an invalid database. This would 
> still work (i.e., not crash), because invalid in this context is empty. 
> However, once you select a ton of files (the number is very weird  - 158 or 
> something) LMDB would scream at you with this problem - **MDB_READERS_FULL: 
> Environment maxreaders limit reached**
> 
> There are two ways to solve this - increase the maxreaders limit in LMDB, 
> or return false if the database doesn't exist. The first option is when you 
> want to go all Linus - *we do not break userspace*, but this way is the more 
> correct way of solving this, because again, on systems where Baloo is 
> disabled, the index file isn't supposed to exist at all.
> 
> I was afraid I was opening up a can of worms by adding yet another 
> condition for db->open() to return false, because I was afraid there were 
> places where a check on db->open()'s return value was missing (and someone 
> would try to do operations on an invalid database). However, I've done all 
> the things that used to trigger crashes before and nothing's crashed yet. 
> With Baloo being both enabled and disabled.
> 
> Thomas Lübking wrote:
> That sounds as if a ton of jobs is started which all try to access the 
> database?
> W/o knowing the details on what is going on, there should probably:
> 
> a) a sane limit per client on how many DB jobs to perform at once 
> (general performance issue)
> b) a sanity check on the index file ie. testing whether it contains some 
> significant bytes or at least isn't empty.
> 
> I guess (b) should actually done by lmdb, but it won't hurt to do it on 
> baloo as well
> 
> Is this also a problem if the file is actually a valid DB, but baloo just 
> disabled?
> 
> 
> ---
> General remark: I guess valid databases should not be deleted but at best 
> be renamed with de/activation, no matter what else happens with this patch.

I'll put this down to a LMDB bug (I suspect some race condition) because when 
the db is active and indexing is enabled, the crash doesn't happen. Therefore I 
see no sense in increasing the limit. As for b), then I'd have to know the LMDB 
file format to do it in Baloo (by default an empty database as created by this 
method is 8KB in size, so there's some sort of data in it).

For your general remark (rename dbs instead of deleting them) - you'd have to 
ask Vishesh. That's a design decision left to the maintainer. I'm just a 
drive-by patcher who's somewhat familiar with the codebase because I've fixed 
quite a few crashes in other apps when Baloo is disabled ;-)


- Boudhayan


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


On Nov. 19, 2015, 1:10 a.m., Boudhayan Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126105/
> --

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

[Boudhayan Gupta]

> On Nov. 19, 2015, 10:35 a.m., Sune Vuorela wrote:
> > without actualy knowing the code in question, a brief look over gives me 
> > the impression that the error handling could be improved and that would 
> > make this patch not needed.

The assumption here is that if (rc != 0) in normal operation the errors are 
serious enough to warrant crashing the process, not handling it gracefully. 
Granted, I've just discovered one case where this isn't true, but that's when 
indexing is disabled and the db file shouldn't exist, so I've added a special 
check for that.

Also, that patch prevents an empty index from being created even when indexing 
is disabled, which is correct behaviour.


- Boudhayan


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


On Nov. 19, 2015, 1:10 a.m., Boudhayan Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126105/
> ---
> 
> (Updated Nov. 19, 2015, 1:10 a.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

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

[Boudhayan Gupta]

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

(Updated Nov. 19, 2015, 7:11 a.m.)


Status
--

This change has been marked as submitted.


Review request for Baloo, KDE Frameworks, Pinak Ahuja, and Vishesh Handa.


Changes
---

Submitted with commit 1e880e29883561c8330cb81c03b48716ea68616c by Boudhayan 
Gupta to branch master.


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

Re: Review Request 126091: Remove printscreen.khotkeys from KHotkeys package

[Boudhayan Gupta]

> On Nov. 17, 2015, 12:51 p.m., Martin Gräßlin wrote:
> > should the file be added to ksnapshot?

Already done, installs to both {CMAKE_INSTALL_PREFIX}/share/khotkeys and 
{CMAKE_INSTALL_PREFIX}/share/apps/khotkeys (for kde4 compatibility)


- Boudhayan


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


On Nov. 16, 2015, 10:36 p.m., Boudhayan Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126091/
> ---
> 
> (Updated Nov. 16, 2015, 10:36 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma and David Edmundson.
> 
> 
> Repository: khotkeys
> 
> 
> Description
> ---
> 
> After moving KSnapshot to Extragear I've modified it to install its own 
> khotkeys file (which it does correctly). Spectacle already installs its own 
> KHotkeys file, so there's no reason anymore for KHotkeys to include its own 
> PrintScreen hotkey file.
> 
> I'll push out a special release of KSnapshot for distributions to use once 
> the Plasma 5.5 version of KHotkeys is pushed out, so printscreen 
> functionality won't be broken on the desktop (assuming, that is, distros push 
> out the updates together).
> 
> 
> Diffs
> -
> 
>   data/CMakeLists.txt f3bcb57 
>   data/printscreen.khotkeys e583311 
> 
> Diff: https://git.reviewboard.kde.org/r/126091/diff/
> 
> 
> Testing
> ---
> 
> Doesn't install the printscreen.khotkeys file anymore
> 
> 
> Thanks,
> 
> Boudhayan Gupta
> 
>

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

Review Request 126091: Remove printscreen.khotkeys from KHotkeys package

[Boudhayan Gupta]

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

Review request for KDE Frameworks, Plasma and David Edmundson.


Repository: khotkeys


Description
---

After moving KSnapshot to Extragear I've modified it to install its own 
khotkeys file (which it does correctly). Spectacle already installs its own 
KHotkeys file, so there's no reason anymore for KHotkeys to include its own 
PrintScreen hotkey file.

I'll push out a special release of KSnapshot for distributions to use once the 
Plasma 5.5 version of KHotkeys is pushed out, so printscreen functionality 
won't be broken on the desktop (assuming, that is, distros push out the updates 
together).


Diffs
-

  data/CMakeLists.txt f3bcb57 
  data/printscreen.khotkeys e583311 

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


Testing
---

Doesn't install the printscreen.khotkeys file anymore


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 126091: Remove printscreen.khotkeys from KHotkeys package

[Boudhayan Gupta]

> On Nov. 16, 2015, 10:46 p.m., Hrvoje Senjan wrote:
> > >assuming, that is, distros push out the updates together
> > 
> > don't assume this ;-) at least send a mail to kde-distro-packagers about 
> > the sideeffects if those 3 are pushed asynchronously

'course, I'll post there as well as add a big fat notice to the 
kde-apps-announce posting that announces this special release of KSnapshot.

So ship it?


- Boudhayan


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


On Nov. 16, 2015, 10:36 p.m., Boudhayan Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126091/
> ---
> 
> (Updated Nov. 16, 2015, 10:36 p.m.)
> 
> 
> Review request for KDE Frameworks, Plasma and David Edmundson.
> 
> 
> Repository: khotkeys
> 
> 
> Description
> ---
> 
> After moving KSnapshot to Extragear I've modified it to install its own 
> khotkeys file (which it does correctly). Spectacle already installs its own 
> KHotkeys file, so there's no reason anymore for KHotkeys to include its own 
> PrintScreen hotkey file.
> 
> I'll push out a special release of KSnapshot for distributions to use once 
> the Plasma 5.5 version of KHotkeys is pushed out, so printscreen 
> functionality won't be broken on the desktop (assuming, that is, distros push 
> out the updates together).
> 
> 
> Diffs
> -
> 
>   data/CMakeLists.txt f3bcb57 
>   data/printscreen.khotkeys e583311 
> 
> Diff: https://git.reviewboard.kde.org/r/126091/diff/
> 
> 
> Testing
> ---
> 
> Doesn't install the printscreen.khotkeys file anymore
> 
> 
> 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

[Boudhayan Gupta]

> On Oct. 30, 2015, 12:43 p.m., Boudhayan Gupta wrote:
> > Ping? :-)

Err, ship it or drop it?

It's been about two weeks sitting, so I was hoping to get some feedback now, 
either way.


- Boudhayan


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


On Oct. 24, 2015, 5:49 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, 5:49 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: Failure while executing KTar::open while using KCompressionDevice as the device

[Boudhayan Gupta]

I'd first ascertain whether the file is corrupt. Why don't you dump
the tar to a QFile and manually use tar -xvf to extract it, just to
test whether the download is working?

On 2 November 2015 at 23:23, Luiz Romário Santana Rios
 wrote:
> Hello,
>
> I'm trying to decompress a XZ archive downloaded using
> QNetworkAccessManager, so, according to the documents, I have to pass
> the QNetworkReply pointer to a KCompressionDevice and, then, use it as
> Ktar's device like this:
>
> https://gist.github.com/anonymous/b8fb686367f518a7dbb5
>
> The problem is that KTar::open() fails and returns false. The file I'm
> trying to extract has the following structure more or less:
> /root
> /root/dir
> /root/dir/file1
> /root/dir/file2
> ...
>
> So, as far as I've seen, the code runs normally when entering /root
> and /root/dir, but, pretty high in the stack, at
> KXzFilter::uncompress(), the call to lzma_code returns
> LZMA_FORMAT_ERROR while trying to uncompress file1 (or file2, I'm not
> sure). Here's the call stack:
>
> https://gist.github.com/anonymous/9ea380cfe48daadb5971
>
> Is this a bug? If it's a bug, how can I proceed to fix it?
>
> Thanks for the attention.
>
> --
> Luiz Romário Santana Rios
> ___
> Kde-frameworks-devel mailing list
> Kde-frameworks-devel@kde.org
> https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
___
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

[Boudhayan Gupta]

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


Ping? :-)

- Boudhayan Gupta


On Oct. 24, 2015, 5:49 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, 5:49 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

[Boudhayan Gupta]

> On Oct. 24, 2015, 6 p.m., Vishesh Handa wrote:
> > 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.

Do you mean the entire current extractor interface, or just this patch?

What do you mean, "write them into a cohesive blob"? Do you mean all the 
plugins would be compiled into one giant SO/DLL?

I wrote this patch because it was a 4-hour job, but if you want to overhaul the 
entire plugin API I suppose the SOC project would be a better place to do it.


> On Oct. 24, 2015, 6 p.m., Vishesh Handa wrote:
> > src/extractors/externalextractor.cpp, line 69
> > <https://git.reviewboard.kde.org/r/125762/diff/4/?file=412432#file412432line69>
> >
> > Why move from the C++ for syntax to Q_FOREACH? There doesn't seem to be 
> > any advantage in this case.

I was told on IRC (IIRC, by Luca Beltrame) that we still can't use range-based 
for loops in frameworks because of compiler requirements. My initial patch used 
range-based for loops.


- Boudhayan


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


On Oct. 24, 2015, 5:49 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, 5:49 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

[Boudhayan Gupta]

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

(Updated Oct. 24, 2015, 5:49 p.m.)


Review request for Baloo, KDE Frameworks, Pinak Ahuja, and Vishesh Handa.


Changes
---

Update diff based on offline comments from pinakahuja.

* Comment explaining methods in cpp above ctor/dtor are static
* Look at cwd, KFILEMETADATA_EXTRACTOR_PATH and PATH, in that order, for 
extractors.
* Fail on malformed JSON input


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 (updated)
-

  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 125641: Allow PAM unlocking of a running wallet

[Boudhayan Gupta]

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



src/runtime/kwalletd/main.cpp (line 173)
<https://git.reviewboard.kde.org/r/125641/#comment59969>

Is this necessary? Shouldn't KWallet::Wallet::isOpen() fail nicely if the 
wallet isn't running?


- Boudhayan Gupta


On Oct. 16, 2015, 10:22 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125641/
> ---
> 
> (Updated Oct. 16, 2015, 10:22 p.m.)
> 
> 
> Review request for KDE Frameworks and Valentin Rusu.
> 
> 
> Repository: kwallet
> 
> 
> Description
> ---
> 
> A use-case: kwallet gets locked with lockscreen, eg. on Plasma Mobile, 
> unlocking the screen would also unlock kwallet through PAM.
> 
> Another use-case: automatic login that shows lockscreen after booting, 
> unlocking that session would also unlock kwallet through PAM.
> 
> This requires a small change in kwallet-pam.
> 
> Now to the patch itself. When a user authenticates via lockscreen, PAM can 
> start the kwalletd process and pass the auth hash token to it. In case the 
> kwalletd process is already running, this patch would check if the wallet is 
> opened and if not, it would pass the PAM hash token over dbus to the running 
> kwallet instance which would unlock the running wallet. If it is unlocked, 
> nothing would happen.
> 
> I originally didn't want to pass it over dbus, but in the end it doesn't 
> matter because as soon as the session is unlocked (at this point the hash is 
> sent), the wallet would be unlocked and a possible attacker would have access 
> to its data anyway. But I'm open to suggestions on improvements.
> 
> 
> Diffs
> -
> 
>   src/runtime/kwalletd/main.cpp fbab58d 
> 
> Diff: https://git.reviewboard.kde.org/r/125641/diff/
> 
> 
> Testing
> ---
> 
> I've created a special PAM profile which has
> 
> auth   optionalpam_kwallet5.so lockscreen 
> kwalletd=/opt/kde5/bin/kwalletd5
> 
> ran kcheckpass -c myprofile and kwallet5 got started and unlocked. Then I 
> locked the wallet using kwalletmanager5, ran kcheckpass -c myprofile again 
> and the running kwallet5 instance got unlocked.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

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

Re: Review Request 125763: Add Skip Close Animation to NET::Property2

[Boudhayan Gupta]

> On Oct. 23, 2015, 4:41 p.m., Martin Gräßlin wrote:
> > I understand the changes to NETRootInfo, but I fail to see what the changes 
> > to NETWinInfo are supposed to do.

If the changes to NETWinInfo aren't required, it simplifies the patch a lot. 
I'll post a fixed patch tomorrow.


- Boudhayan


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


On Oct. 23, 2015, 4:11 p.m., Boudhayan Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125763/
> ---
> 
> (Updated Oct. 23, 2015, 4:11 p.m.)
> 
> 
> Review request for KDE Frameworks and Martin Gräßlin.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> Add NET::Property2::WM2SkipCloseAnimation and associated resolvers.
> 
> 
> Diffs
> -
> 
>   src/netwm_def.h 0938e2b 
>   src/platforms/xcb/atoms_p.h b5a6e7e 
>   src/platforms/xcb/netwm.cpp 2335c29 
>   src/platforms/xcb/netwm_p.h e0645bb 
> 
> Diff: https://git.reviewboard.kde.org/r/125763/diff/
> 
> 
> Testing
> ---
> 
> Only build-tested.
> 
> 
> 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

[Boudhayan Gupta]

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

(Updated Oct. 23, 2015, 6:58 p.m.)


Review request for Baloo, KDE Frameworks, Pinak Ahuja, and Vishesh Handa.


Changes
---

* Add TypeInfo support
* Look in PATH instead of just /usr/bin, semantics for Q_OS_UNIX and Q_OS_WIN
* Add docs


Repository: kfilemetadata


Description (updated)
---

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 (updated)
-

  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

[Boudhayan Gupta]

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

(Updated Oct. 23, 2015, 7:10 p.m.)


Review request for Baloo, KDE Frameworks, Pinak Ahuja, and Vishesh Handa.


Changes
---

change range-based for loops to Q_FOREACH


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 (updated)
-

  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

Review Request 125763: Add Skip Close Animation to NET::Property2

[Boudhayan Gupta]

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

Review request for KDE Frameworks and Martin Gräßlin.


Repository: kwindowsystem


Description
---

Add NET::Property2::WM2SkipCloseAnimation and associated resolvers.


Diffs
-

  src/netwm_def.h 0938e2b 
  src/platforms/xcb/atoms_p.h b5a6e7e 
  src/platforms/xcb/netwm.cpp 2335c29 
  src/platforms/xcb/netwm_p.h e0645bb 

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


Testing
---

Only build-tested.


Thanks,

Boudhayan Gupta

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

Review Request 125762: External extractor plugin support for KFileMetaData

[Boudhayan Gupta]

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

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
-

  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

[Boudhayan Gupta]

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

(Updated Oct. 23, 2015, 4:17 p.m.)


Review request for Baloo, KDE Frameworks, Pinak Ahuja, and Vishesh Handa.


Changes
---

Change description


Repository: kfilemetadata


Description (updated)
---

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.

TODO:
* Add docs for writing external extractors
* Look in PATH for extractors instead of in /usr/bin


Diffs
-

  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 125736: Add Skip Window Close Animation to KWindowEffects

[Boudhayan Gupta]

> On Oct. 23, 2015, 12:11 p.m., Martin Gräßlin wrote:
> > src/kwindoweffects.h, lines 162-168
> > <https://git.reviewboard.kde.org/r/125736/diff/4/?file=412108#file412108line162>
> >
> > Maybe we should extend the documentation a little bit? E.g. that it's 
> > only a hint to the WM, that it's free to ignore it, that not all WMs might 
> > support it.
> > 
> > Maybe also give an example for a use case? The screen shot case?

Yep, that sounds like a good idea. I'll do that.


> On Oct. 23, 2015, 12:11 p.m., Martin Gräßlin wrote:
> > src/platforms/xcb/kwindoweffects.cpp, lines 73-75
> > <https://git.reviewboard.kde.org/r/125736/diff/4/?file=412114#file412114line73>
> >
> > Just as a note: KWin doesn't set the atom name on the root window, so 
> > it will always return false in this case.
> > 
> > At the moment I don't have an idea for how to do it. Normal way would 
> > be to add it to the NET_SUPPORTED in NETWM.

So should I always return true for this for now? This does mean that the API 
will be lying, at least for now.


> On Oct. 23, 2015, 12:11 p.m., Martin Gräßlin wrote:
> > src/platforms/xcb/kwindoweffects.cpp, line 333
> > <https://git.reviewboard.kde.org/r/125736/diff/4/?file=412114#file412114line333>
> >
> > nitpick: unrelated whitespace change.

That's my text-editor (Atom) being a smart-ass. If you really don't want this 
in the patch, I'll use a second text-editor that doesn't automatically trim 
empty lines (nano?) to re-add the space.


- Boudhayan


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


On Oct. 21, 2015, 8:53 p.m., Boudhayan Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125736/
> ---
> 
> (Updated Oct. 21, 2015, 8:53 p.m.)
> 
> 
> Review request for KDE Frameworks and Martin Gräßlin.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> Adds a Skip Window Close Animation effect to KWindowEffects, with an 
> implementation for X11.
> 
> Binary compatibility with older plugins is **not** preserved. I welcome 
> comments on how exactly to do this - creating a new base class 
> (KWindowEffectsPrivate2 based on KWindowEffectsPrivate and basing the plugins 
> off that sounds kind of complicated). Maybe we can just drop internal ABI 
> compatibility and update kwayland-integration to implement the new method too?
> 
> 
> Diffs
> -
> 
>   autotests/kwindoweffectstest.cpp 0e83bdc 
>   src/CMakeLists.txt 1598b4f 
>   src/kwindoweffects.h bf0ea1e 
>   src/kwindoweffects.cpp 0c6600f 
>   src/kwindoweffects_dummy.cpp 65924ae 
>   src/kwindoweffects_dummy_p.h 2beabdd 
>   src/kwindoweffects_extensions_p.h PRE-CREATION 
>   src/platforms/xcb/atoms_p.h b5a6e7e 
>   src/platforms/xcb/kwindoweffects.cpp c8da6d2 
>   src/platforms/xcb/kwindoweffects_x11.h c240ddf 
> 
> Diff: https://git.reviewboard.kde.org/r/125736/diff/
> 
> 
> Testing
> ---
> 
> * make test succeeds with the new plugin installed
> * make test succeeds on all tests in the kwindoweffects test except the 
> skipCloseAnimation unit with the old plugin installed. skipCloseAnimation 
> segfaults.
> 
> 
> 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 125641: Allow PAM unlocking of a running wallet

[Boudhayan Gupta]

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


"I originally didn't want to pass it over dbus, but in the end it doesn't 
matter because as soon as the session is unlocked (at this point the hash is 
sent), the wallet would be unlocked and a possible attacker would have access 
to its data anyway. But I'm open to suggestions on improvements."

I don't understand. If the wallet is unlocked, why are you trying to unlock it?

+1 on the use-cases; I don't know much about kwallet's or PAM's internals to 
thoroughly review the code. This is important - needs a review.

- Boudhayan Gupta


On Oct. 16, 2015, 10:22 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125641/
> ---
> 
> (Updated Oct. 16, 2015, 10:22 p.m.)
> 
> 
> Review request for KDE Frameworks and Valentin Rusu.
> 
> 
> Repository: kwallet
> 
> 
> Description
> ---
> 
> A use-case: kwallet gets locked with lockscreen, eg. on Plasma Mobile, 
> unlocking the screen would also unlock kwallet through PAM.
> 
> Another use-case: automatic login that shows lockscreen after booting, 
> unlocking that session would also unlock kwallet through PAM.
> 
> This requires a small change in kwallet-pam.
> 
> Now to the patch itself. When a user authenticates via lockscreen, PAM can 
> start the kwalletd process and pass the auth hash token to it. In case the 
> kwalletd process is already running, this patch would check if the wallet is 
> opened and if not, it would pass the PAM hash token over dbus to the running 
> kwallet instance which would unlock the running wallet. If it is unlocked, 
> nothing would happen.
> 
> I originally didn't want to pass it over dbus, but in the end it doesn't 
> matter because as soon as the session is unlocked (at this point the hash is 
> sent), the wallet would be unlocked and a possible attacker would have access 
> to its data anyway. But I'm open to suggestions on improvements.
> 
> 
> Diffs
> -
> 
>   src/runtime/kwalletd/main.cpp fbab58d 
> 
> Diff: https://git.reviewboard.kde.org/r/125641/diff/
> 
> 
> Testing
> ---
> 
> I've created a special PAM profile which has
> 
> auth   optionalpam_kwallet5.so lockscreen 
> kwalletd=/opt/kde5/bin/kwalletd5
> 
> ran kcheckpass -c myprofile and kwallet5 got started and unlocked. Then I 
> locked the wallet using kwalletmanager5, ran kcheckpass -c myprofile again 
> and the running kwallet5 instance got unlocked.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

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

Re: Review Request 125736: Add Skip Window Close Animation to KWindowEffects

[Boudhayan Gupta]

> On Oct. 21, 2015, 8:08 p.m., Martin Gräßlin wrote:
> > src/kwindoweffects.h, line 172
> > <https://git.reviewboard.kde.org/r/125736/diff/3/?file=412098#file412098line172>
> >
> > looks like you removed an empty line
> 
> Boudhayan Gupta wrote:
> I don't understand this issue. Removed an empty line?

Re-checked in two text-editors - I added a new line at end of file, not removed 
an empty line. Seems like a reviewboard issue.


- Boudhayan


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


On Oct. 21, 2015, 8:53 p.m., Boudhayan Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125736/
> ---
> 
> (Updated Oct. 21, 2015, 8:53 p.m.)
> 
> 
> Review request for KDE Frameworks and Martin Gräßlin.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> Adds a Skip Window Close Animation effect to KWindowEffects, with an 
> implementation for X11.
> 
> Binary compatibility with older plugins is **not** preserved. I welcome 
> comments on how exactly to do this - creating a new base class 
> (KWindowEffectsPrivate2 based on KWindowEffectsPrivate and basing the plugins 
> off that sounds kind of complicated). Maybe we can just drop internal ABI 
> compatibility and update kwayland-integration to implement the new method too?
> 
> 
> Diffs
> -
> 
>   autotests/kwindoweffectstest.cpp 0e83bdc 
>   src/CMakeLists.txt 1598b4f 
>   src/kwindoweffects.h bf0ea1e 
>   src/kwindoweffects.cpp 0c6600f 
>   src/kwindoweffects_dummy.cpp 65924ae 
>   src/kwindoweffects_dummy_p.h 2beabdd 
>   src/kwindoweffects_extensions_p.h PRE-CREATION 
>   src/platforms/xcb/atoms_p.h b5a6e7e 
>   src/platforms/xcb/kwindoweffects.cpp c8da6d2 
>   src/platforms/xcb/kwindoweffects_x11.h c240ddf 
> 
> Diff: https://git.reviewboard.kde.org/r/125736/diff/
> 
> 
> Testing
> ---
> 
> * make test succeeds with the new plugin installed
> * make test succeeds on all tests in the kwindoweffects test except the 
> skipCloseAnimation unit with the old plugin installed. skipCloseAnimation 
> segfaults.
> 
> 
> 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 125736: Add Skip Window Close Animation to KWindowEffects

[Boudhayan Gupta]

> On Oct. 21, 2015, 8:08 p.m., Martin Gräßlin wrote:
> > src/kwindoweffects.h, line 172
> > <https://git.reviewboard.kde.org/r/125736/diff/3/?file=412098#file412098line172>
> >
> > looks like you removed an empty line

I don't understand this issue. Removed an empty line?


- Boudhayan


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


On Oct. 21, 2015, 5:25 p.m., Boudhayan Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125736/
> ---
> 
> (Updated Oct. 21, 2015, 5:25 p.m.)
> 
> 
> Review request for KDE Frameworks and Martin Gräßlin.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> Adds a Skip Window Close Animation effect to KWindowEffects, with an 
> implementation for X11.
> 
> Binary compatibility with older plugins is **not** preserved. I welcome 
> comments on how exactly to do this - creating a new base class 
> (KWindowEffectsPrivate2 based on KWindowEffectsPrivate and basing the plugins 
> off that sounds kind of complicated). Maybe we can just drop internal ABI 
> compatibility and update kwayland-integration to implement the new method too?
> 
> 
> Diffs
> -
> 
>   autotests/kwindoweffectstest.cpp 0e83bdc 
>   src/kwindoweffects.h bf0ea1e 
>   src/kwindoweffects.cpp 0c6600f 
>   src/kwindoweffects_dummy.cpp 65924ae 
>   src/kwindoweffects_dummy_p.h 2beabdd 
>   src/kwindoweffects_extensions.h PRE-CREATION 
>   src/platforms/xcb/atoms_p.h b5a6e7e 
>   src/platforms/xcb/kwindoweffects.cpp c8da6d2 
>   src/platforms/xcb/kwindoweffects_x11.h c240ddf 
> 
> Diff: https://git.reviewboard.kde.org/r/125736/diff/
> 
> 
> Testing
> ---
> 
> * make test succeeds with the new plugin installed
> * make test succeeds on all tests in the kwindoweffects test except the 
> skipCloseAnimation unit with the old plugin installed. skipCloseAnimation 
> segfaults.
> 
> 
> 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 125736: Add Skip Window Close Animation to KWindowEffects

[Boudhayan Gupta]

> On Oct. 21, 2015, 8:19 p.m., Boudhayan Gupta wrote:
> > src/platforms/xcb/kwindoweffects_x11.h, line 26
> > <https://git.reviewboard.kde.org/r/125736/diff/3/?file=412105#file412105line26>
> >
> > This was my idea, which sitter also liked and is used in Phonon. The 
> > idea is that every individual added feature has its own class which is 
> > MI-ed with the core class, that way plugins can choose to implement only 
> > the extra features they want.
> > 
> > For example if I add feature A in class 1, then feature B in class 2, 
> > but a plugin only wants to implement feature B without implementing feature 
> > A, they can't if you do what you're asking.
> 
> Martin Gräßlin wrote:
> I see what you mean. But that could also be achieved with default 
> implementation instead of pure virtuals. And I do see a problem with that 
> approach: what if you want to add another method one day to e.g. enable close 
> animation again? Then you need to extend KWindowEffectsSkipCloseAnimation in 
> exactly the same way as I proposed originally.

Then I add a KWindowEffectsEnableCloseAnimation extension.

This is why I put the extensions in another header. kwindoweffects_p.h is going 
to get very ugly very fast if I keep adding micro-features - let's confine the 
ugliness to this one place.

If you go the chain of inheritence way, it'll end up like this one day:

https://quickgit.kde.org/?p=phonon.git=blob=6b5d878ec9e6e605557b5de12c179d6ac3d5daf8=phonon%2Faudiooutputinterface.h=plain


- Boudhayan


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


On Oct. 21, 2015, 5:25 p.m., Boudhayan Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125736/
> ---
> 
> (Updated Oct. 21, 2015, 5:25 p.m.)
> 
> 
> Review request for KDE Frameworks and Martin Gräßlin.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> Adds a Skip Window Close Animation effect to KWindowEffects, with an 
> implementation for X11.
> 
> Binary compatibility with older plugins is **not** preserved. I welcome 
> comments on how exactly to do this - creating a new base class 
> (KWindowEffectsPrivate2 based on KWindowEffectsPrivate and basing the plugins 
> off that sounds kind of complicated). Maybe we can just drop internal ABI 
> compatibility and update kwayland-integration to implement the new method too?
> 
> 
> Diffs
> -
> 
>   autotests/kwindoweffectstest.cpp 0e83bdc 
>   src/kwindoweffects.h bf0ea1e 
>   src/kwindoweffects.cpp 0c6600f 
>   src/kwindoweffects_dummy.cpp 65924ae 
>   src/kwindoweffects_dummy_p.h 2beabdd 
>   src/kwindoweffects_extensions.h PRE-CREATION 
>   src/platforms/xcb/atoms_p.h b5a6e7e 
>   src/platforms/xcb/kwindoweffects.cpp c8da6d2 
>   src/platforms/xcb/kwindoweffects_x11.h c240ddf 
> 
> Diff: https://git.reviewboard.kde.org/r/125736/diff/
> 
> 
> Testing
> ---
> 
> * make test succeeds with the new plugin installed
> * make test succeeds on all tests in the kwindoweffects test except the 
> skipCloseAnimation unit with the old plugin installed. skipCloseAnimation 
> segfaults.
> 
> 
> 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 125736: Add Skip Window Close Animation to KWindowEffects

[Boudhayan Gupta]

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

(Updated Oct. 21, 2015, 8:53 p.m.)


Review request for KDE Frameworks and Martin Gräßlin.


Changes
---

fix issues


Repository: kwindowsystem


Description
---

Adds a Skip Window Close Animation effect to KWindowEffects, with an 
implementation for X11.

Binary compatibility with older plugins is **not** preserved. I welcome 
comments on how exactly to do this - creating a new base class 
(KWindowEffectsPrivate2 based on KWindowEffectsPrivate and basing the plugins 
off that sounds kind of complicated). Maybe we can just drop internal ABI 
compatibility and update kwayland-integration to implement the new method too?


Diffs (updated)
-

  autotests/kwindoweffectstest.cpp 0e83bdc 
  src/CMakeLists.txt 1598b4f 
  src/kwindoweffects.h bf0ea1e 
  src/kwindoweffects.cpp 0c6600f 
  src/kwindoweffects_dummy.cpp 65924ae 
  src/kwindoweffects_dummy_p.h 2beabdd 
  src/kwindoweffects_extensions_p.h PRE-CREATION 
  src/platforms/xcb/atoms_p.h b5a6e7e 
  src/platforms/xcb/kwindoweffects.cpp c8da6d2 
  src/platforms/xcb/kwindoweffects_x11.h c240ddf 

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


Testing
---

* make test succeeds with the new plugin installed
* make test succeeds on all tests in the kwindoweffects test except the 
skipCloseAnimation unit with the old plugin installed. skipCloseAnimation 
segfaults.


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 125736: Add Skip Window Close Animation to KWindowEffects

[Boudhayan Gupta]

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



src/kwindoweffects.cpp (line 95)
<https://git.reviewboard.kde.org/r/125736/#comment59910>

I had a few qDebug() statements afterwards for testing. I'll remove this.



src/kwindoweffects_extensions.h (lines 26 - 35)
<https://git.reviewboard.kde.org/r/125736/#comment59911>

I could, but isn't it better to organize all extensions in one place?



src/platforms/xcb/kwindoweffects_x11.h (line 26)
<https://git.reviewboard.kde.org/r/125736/#comment59912>

This was my idea, which sitter also liked and is used in Phonon. The idea 
is that every individual added feature has its own class which is MI-ed with 
the core class, that way plugins can choose to implement only the extra 
features they want.

For example if I add feature A in class 1, then feature B in class 2, but a 
plugin only wants to implement feature B without implementing feature A, they 
can't if you do what you're asking.


- Boudhayan Gupta


On Oct. 21, 2015, 5:25 p.m., Boudhayan Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125736/
> ---
> 
> (Updated Oct. 21, 2015, 5:25 p.m.)
> 
> 
> Review request for KDE Frameworks and Martin Gräßlin.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> Adds a Skip Window Close Animation effect to KWindowEffects, with an 
> implementation for X11.
> 
> Binary compatibility with older plugins is **not** preserved. I welcome 
> comments on how exactly to do this - creating a new base class 
> (KWindowEffectsPrivate2 based on KWindowEffectsPrivate and basing the plugins 
> off that sounds kind of complicated). Maybe we can just drop internal ABI 
> compatibility and update kwayland-integration to implement the new method too?
> 
> 
> Diffs
> -
> 
>   autotests/kwindoweffectstest.cpp 0e83bdc 
>   src/kwindoweffects.h bf0ea1e 
>   src/kwindoweffects.cpp 0c6600f 
>   src/kwindoweffects_dummy.cpp 65924ae 
>   src/kwindoweffects_dummy_p.h 2beabdd 
>   src/kwindoweffects_extensions.h PRE-CREATION 
>   src/platforms/xcb/atoms_p.h b5a6e7e 
>   src/platforms/xcb/kwindoweffects.cpp c8da6d2 
>   src/platforms/xcb/kwindoweffects_x11.h c240ddf 
> 
> Diff: https://git.reviewboard.kde.org/r/125736/diff/
> 
> 
> Testing
> ---
> 
> * make test succeeds with the new plugin installed
> * make test succeeds on all tests in the kwindoweffects test except the 
> skipCloseAnimation unit with the old plugin installed. skipCloseAnimation 
> segfaults.
> 
> 
> 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 125736: Add Skip Window Close Animation to KWindowEffects

[Boudhayan Gupta]

> On Oct. 21, 2015, 8:19 p.m., Boudhayan Gupta wrote:
> > src/platforms/xcb/kwindoweffects_x11.h, line 26
> > <https://git.reviewboard.kde.org/r/125736/diff/3/?file=412105#file412105line26>
> >
> > This was my idea, which sitter also liked and is used in Phonon. The 
> > idea is that every individual added feature has its own class which is 
> > MI-ed with the core class, that way plugins can choose to implement only 
> > the extra features they want.
> > 
> > For example if I add feature A in class 1, then feature B in class 2, 
> > but a plugin only wants to implement feature B without implementing feature 
> > A, they can't if you do what you're asking.
> 
> Martin Gräßlin wrote:
> I see what you mean. But that could also be achieved with default 
> implementation instead of pure virtuals. And I do see a problem with that 
> approach: what if you want to add another method one day to e.g. enable close 
> animation again? Then you need to extend KWindowEffectsSkipCloseAnimation in 
> exactly the same way as I proposed originally.
> 
> Boudhayan Gupta wrote:
> Then I add a KWindowEffectsEnableCloseAnimation extension.
> 
> This is why I put the extensions in another header. kwindoweffects_p.h is 
> going to get very ugly very fast if I keep adding micro-features - let's 
> confine the ugliness to this one place.
> 
> If you go the chain of inheritence way, it'll end up like this one day:
> 
> 
> https://quickgit.kde.org/?p=phonon.git=blob=6b5d878ec9e6e605557b5de12c179d6ac3d5daf8=phonon%2Faudiooutputinterface.h=plain
> 
> Martin Gräßlin wrote:
> right, I see your point. Even if I consider it as unlikely that we ever 
> get to a two digit number (based on history of the development of 
> KWindowEffects) ;-)
> 
> Then please use a "_p.h" naming. And it also needs to be installed 
> through CMake. Otherwise we cannot use it from kwayland-integration.

kwindoweffects_extensions_p.h?


- Boudhayan


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


On Oct. 21, 2015, 5:25 p.m., Boudhayan Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125736/
> ---
> 
> (Updated Oct. 21, 2015, 5:25 p.m.)
> 
> 
> Review request for KDE Frameworks and Martin Gräßlin.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> Adds a Skip Window Close Animation effect to KWindowEffects, with an 
> implementation for X11.
> 
> Binary compatibility with older plugins is **not** preserved. I welcome 
> comments on how exactly to do this - creating a new base class 
> (KWindowEffectsPrivate2 based on KWindowEffectsPrivate and basing the plugins 
> off that sounds kind of complicated). Maybe we can just drop internal ABI 
> compatibility and update kwayland-integration to implement the new method too?
> 
> 
> Diffs
> -
> 
>   autotests/kwindoweffectstest.cpp 0e83bdc 
>   src/kwindoweffects.h bf0ea1e 
>   src/kwindoweffects.cpp 0c6600f 
>   src/kwindoweffects_dummy.cpp 65924ae 
>   src/kwindoweffects_dummy_p.h 2beabdd 
>   src/kwindoweffects_extensions.h PRE-CREATION 
>   src/platforms/xcb/atoms_p.h b5a6e7e 
>   src/platforms/xcb/kwindoweffects.cpp c8da6d2 
>   src/platforms/xcb/kwindoweffects_x11.h c240ddf 
> 
> Diff: https://git.reviewboard.kde.org/r/125736/diff/
> 
> 
> Testing
> ---
> 
> * make test succeeds with the new plugin installed
> * make test succeeds on all tests in the kwindoweffects test except the 
> skipCloseAnimation unit with the old plugin installed. skipCloseAnimation 
> segfaults.
> 
> 
> 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 125736: Add Skip Window Close Animation to KWindowEffects

[Boudhayan Gupta]

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

(Updated Oct. 21, 2015, 5:25 p.m.)


Review request for KDE Frameworks and Martin Gräßlin.


Changes
---

Add kdeframeworks group


Repository: kwindowsystem


Description
---

Adds a Skip Window Close Animation effect to KWindowEffects, with an 
implementation for X11.

Binary compatibility with older plugins is **not** preserved. I welcome 
comments on how exactly to do this - creating a new base class 
(KWindowEffectsPrivate2 based on KWindowEffectsPrivate and basing the plugins 
off that sounds kind of complicated). Maybe we can just drop internal ABI 
compatibility and update kwayland-integration to implement the new method too?


Diffs
-

  autotests/kwindoweffectstest.cpp 0e83bdc 
  src/kwindoweffects.h bf0ea1e 
  src/kwindoweffects.cpp 0c6600f 
  src/kwindoweffects_dummy.cpp 65924ae 
  src/kwindoweffects_dummy_p.h 2beabdd 
  src/kwindoweffects_extensions.h PRE-CREATION 
  src/platforms/xcb/atoms_p.h b5a6e7e 
  src/platforms/xcb/kwindoweffects.cpp c8da6d2 
  src/platforms/xcb/kwindoweffects_x11.h c240ddf 

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


Testing
---

* make test succeeds with the new plugin installed
* make test succeeds on all tests in the kwindoweffects test except the 
skipCloseAnimation unit with the old plugin installed. skipCloseAnimation 
segfaults.


Thanks,

Boudhayan Gupta

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

Re: Naming scheme for Qt5/KF5-based libraries outside of KF5

[Boudhayan Gupta]

On 27 September 2015 at 10:29, Alexander Potashev <aspotas...@gmail.com> wrote:
> 2015-09-27 1:39 GMT+03:00 Albert Astals Cid <aa...@kde.org>:
>> El Diumenge, 27 de setembre de 2015, a les 04:01:26, Boudhayan Gupta va
>> escriure:
>>> On 27 September 2015 at 03:36, Albert Astals Cid <aa...@kde.org> wrote:
>>> > El Dissabte, 26 de setembre de 2015, a les 16:27:22, Sune Vuorela va
>> escriure:
>>> >> On 2015-09-26, Alexander Potashev <aspotas...@gmail.com> wrote:
>>> >> > 1. Many people prefer a "KF5" prefix, e.g. libKF5Screen.so).
>>> >> > 2. Another way of naming is a -qt5 suffix, e.g. libmarblewidget-qt5.so.
>>> >> > 3. (probably some others?)
>>> >> >
>>> >> > Friedrikh said in [1] that using a KF5 prefix for all libraries will
>>> >> > "blur the hint by the name if something is part of KF5 or not".
>>> >> >
>>> >> > Any thoughts? I believe we can have guidelines for library names.
>>> >>
>>> >> I do think that having things named KF5 that aren't actual frameworks is
>>> >> bad for several reasons.
>>> >>
>>> >> 1) It blurs what's a framework
>>> >> 2) We promise ABI and API compatibility for frameworks, but not for
>>> >> other things
>>> >> 3) Moving something from "not a KDE Framework" to "KDE Framework" gives
>>> >> a last chance for fixing up abi/api.
>>> >>
>>> >> so. foo-qt5 is fine for a qt5 version of foo.
>>> >
>>> > I agree, the problem is that there's few exceptions to copy from, so
>>> > that's
>>> > the exact reason libkdegames has that KF5 thing in the name, the guy that
>>> > did the port just copied what the frmeworks do.
>>> >
>>> > So anyone up for write what "a library that is not frameworks should do to
>>> > be nice in the KDE land"?
>>>
>>> We could kill two birds with one stone here, creating a new KDE module
>>> just for libraries (say, KDE Companion Libraries or something) and put
>>> everything in the KC5 (or whatever we decide) namespace.
>>>
>>> I'm all for just putting everything in KDE Support, using the KS5
>>> namespace and removing the tier0 restriction from Support.
>>
>> I don't see which birds it kills, as far as I see it it only gives you the
>> problem of having yet another product to release.
>
> Sune, Boudhayan, Albert,
>
> Thanks for your feedback! I think we already have consensus on the
> "-qt5" suffix. I'll go rename the shared libraries in a few repos...
> :)
>
> Albert, do you mean you want a Techbase page with guidelines for libraries?
>
> Regarding the library product, Boudhayan almost repeated my proposal
> [1]. But using a namespace (e.g. KC5::) would not be a good idea
> because this product may contain completely disconnected libraries.
> "-qt5" suffixes should be enough. For KF5 the namespace makes sense
> because the frameworks have numerous dependencies between one another,
> thus KF5 feels and is promoted as an all-in-one product.
>
> [1] https://mail.kde.org/pipermail/release-team/2015-June/008628.html

Putting hyphens in library names is just ugly, when the rest of the
product name is neat and tidy CamelCase with an initial uppercase
letter.

I'm still in favour of a new product, or reusing KDESupport, or even
Extragear libs. If you must use a suffix though, please consider using
Qt5, not -qt5, so that the lib becomes libSomeThingQt5, not
libSomeThing-qt5.

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

Re: Naming scheme for Qt5/KF5-based libraries outside of KF5

[Boudhayan Gupta]

On 27 September 2015 at 15:29, Sune Vuorela <nos...@vuorela.dk> wrote:
> On 2015-09-26, Boudhayan Gupta <bgu...@kde.org> wrote:
>> We could kill two birds with one stone here, creating a new KDE module
>> just for libraries (say, KDE Companion Libraries or something) and put
>> everything in the KC5 (or whatever we decide) namespace.
>
> By doing this, we kind of make it a thing to .. become. I do think that
> shared cross-repository libraries that aren't frameworks should be
> avoided as much as possible, and for the few ones where it isn't
> possible for specific functionality we should still try to limit it.

What exactly is wrong with shared cross-repo libraries? Take
libPurpose for example. It's a great little utility that many apps can
use, but it's certainly not mature enough to be a framework. I'd want
it in a place where many people can use it.

> It is libraries that might change abi and api, and that's generally a
> mess, especially if the users have different release schedules. And
> people will use an available shared library.

What's wrong with people using a shared library? That's what they're for.

A new component must be aligned with the Applications release-unit,
and since Applications are primary (only?) users of the libraries, and
API/ABI break shouldn't cause any problems at all, given that apps in
the (eg) 15.12 release will only depend on the 15.12 version of the
library.

Also, why are we assuming API/ABI will be broken? Can't developers be
trusted to be strict with their APIs?

>> I'm all for just putting everything in KDE Support, using the KS5
>> namespace and removing the tier0 restriction from Support.
>
> KDE Support is our 'low level' libraries, but many of them has become
> frameworks, which I think should also be the road ahead.

Again, if it can become a framework, make it a framework. If it can't,
put it in Support and release it with Applications.

>
> /Sune

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

Re: Naming scheme for Qt5/KF5-based libraries outside of KF5

[Boudhayan Gupta]

On 27 September 2015 at 04:09, Albert Astals Cid <aa...@kde.org> wrote:
> El Diumenge, 27 de setembre de 2015, a les 04:01:26, Boudhayan Gupta va
> escriure:
>> On 27 September 2015 at 03:36, Albert Astals Cid <aa...@kde.org> wrote:
>> > El Dissabte, 26 de setembre de 2015, a les 16:27:22, Sune Vuorela va
> escriure:
>> >> On 2015-09-26, Alexander Potashev <aspotas...@gmail.com> wrote:
>> >> > 1. Many people prefer a "KF5" prefix, e.g. libKF5Screen.so).
>> >> > 2. Another way of naming is a -qt5 suffix, e.g. libmarblewidget-qt5.so.
>> >> > 3. (probably some others?)
>> >> >
>> >> > Friedrikh said in [1] that using a KF5 prefix for all libraries will
>> >> > "blur the hint by the name if something is part of KF5 or not".
>> >> >
>> >> > Any thoughts? I believe we can have guidelines for library names.
>> >>
>> >> I do think that having things named KF5 that aren't actual frameworks is
>> >> bad for several reasons.
>> >>
>> >> 1) It blurs what's a framework
>> >> 2) We promise ABI and API compatibility for frameworks, but not for
>> >> other things
>> >> 3) Moving something from "not a KDE Framework" to "KDE Framework" gives
>> >> a last chance for fixing up abi/api.
>> >>
>> >> so. foo-qt5 is fine for a qt5 version of foo.
>> >
>> > I agree, the problem is that there's few exceptions to copy from, so
>> > that's
>> > the exact reason libkdegames has that KF5 thing in the name, the guy that
>> > did the port just copied what the frmeworks do.
>> >
>> > So anyone up for write what "a library that is not frameworks should do to
>> > be nice in the KDE land"?
>>
>> We could kill two birds with one stone here, creating a new KDE module
>> just for libraries (say, KDE Companion Libraries or something) and put
>> everything in the KC5 (or whatever we decide) namespace.
>>
>> I'm all for just putting everything in KDE Support, using the KS5
>> namespace and removing the tier0 restriction from Support.
>
> I don't see which birds it kills, as far as I see it it only gives you the
> problem of having yet another product to release.

Release it with the Applications release-unit, since the users of the
libraries are applications.

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

Re: Naming scheme for Qt5/KF5-based libraries outside of KF5

[Boudhayan Gupta]

Hi Friedrich,

On 27 September 2015 at 20:55, Friedrich W. H. Kossebau
 wrote:
> Some bummer here:
> a) not all libraries are in repositories of their own
> b) not all libraries are released on the same cycle
>
> E.g. a) happens because the libs could be shared libs for sharing between
> multiple executables/plugins developed in a single repo. Or they are only
> slowly established as shared code and still developed strongly coupled with
> their main user executable/plugin and for that live in the same repo.
> And b) is because there are a few libs in extragear or playground repos, so
> not covered by the "KDE Applications" release cycle or forced to follow.

So let's clean this mess up.

For applications, Extragear seems to be the place to live in if you
manage your own release cycles. I see no reason libraries should also
be treated the same way.

What I propose is that all libraries which want to manage their own
release cycles and their own namespaces, be moved to Extragear Libs
and release from there. All the libraries which can stick to the
Applications release-unit, move to Support or a new module and be
released with them.

This has the advantage of Application libs not having to maintain
API/ABI stability, since the Applications from one release will depend
on the libs from the same release. Extragear Libs devs, who want to
manage their own cycles etc, will however have to make API/ABI
guarantees.

Also, I get the feeling that Extragear is treated as a second-class
citizen. It doesn't have to be. Apps and libs in Extragear should be
held to the same standards as the rest of the KDE modules. The only
difference ever should be the release cycles.

> So while I agree that having all libs nicely separated would be good to have,
> if only for discoverability, doing that with a single module at least
> currently would not work.

Can you elaborate on why a single module would not work?

> ((Long-term we should perhaps look into that, because right now the layout of
> our repository structure does not make a difference between repos with
> executables, plugins and libraries (and mixed ones).
> And IMHO if we have libs, thus code intended to be shared between other
> software, it would be great if it would be easy to developers to simply browse
> all of the libs to find something perhaps matching. If that list would be a
> virtual one, fine as well, but having the real repositories ordering also
> follow that grouping helps shaping minds and reduces complexicity needed due
> to the mapping.
> (Would also make it simpler to know which libs there are that should be also
> noted at inqlude.org)

+1

Side note here - I'm very interested in getting Vlc-Qt under the KDE
umbrella, if the maintainers of Vlc-Qt and KDE are in support. A
dedicated library module would be a great place for it.

> But different topic, with quite some details and strings attached, worth at
> least a different thread (and someone who can and would drive any planning for
> that for some time, not me).))

I'd love to help. I love organization ;-)

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

Re: Naming scheme for Qt5/KF5-based libraries outside of KF5

[Boudhayan Gupta]

On 27 September 2015 at 22:51, Sune Vuorela <nos...@vuorela.dk> wrote:
> On 2015-09-27, Boudhayan Gupta <bgu...@kde.org> wrote:
>> What I propose is that all libraries which want to manage their own
>> release cycles and their own namespaces, be moved to Extragear Libs
>> and release from there. All the libraries which can stick to the
>> Applications release-unit, move to Support or a new module and be
>> released with them.
>
> What happens if an Applications application uses an extragear lib? or an
> extragear application uses an application lib?
>
> Yes. this will happen.
>
> And then you have a abi/api unstable library out of sync with the
> application it uses. And that's highly annoying.
>
> Seen before with e.g. Digikam/Gwenview/KPhotoAlbum and
> libkipi/libkexiv2/...

"Seen before" is no reason to not move forward if we can actually fix
this. As I said, Extragear library developers will *have* to provide
API/ABI guarantees.

> I don't see why we need a extra organizational level to house something
> we should try to avoid to have in the fist place.
>
> Let's get all good libraries made frameworks.

That's the ideal scenario, but isn't becoming a framework... hard?

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

Re: Naming scheme for Qt5/KF5-based libraries outside of KF5

[Boudhayan Gupta]

On 27 September 2015 at 03:36, Albert Astals Cid  wrote:
> El Dissabte, 26 de setembre de 2015, a les 16:27:22, Sune Vuorela va escriure:
>> On 2015-09-26, Alexander Potashev  wrote:
>> > 1. Many people prefer a "KF5" prefix, e.g. libKF5Screen.so).
>> > 2. Another way of naming is a -qt5 suffix, e.g. libmarblewidget-qt5.so.
>> > 3. (probably some others?)
>> >
>> > Friedrikh said in [1] that using a KF5 prefix for all libraries will
>> > "blur the hint by the name if something is part of KF5 or not".
>> >
>> > Any thoughts? I believe we can have guidelines for library names.
>>
>> I do think that having things named KF5 that aren't actual frameworks is
>> bad for several reasons.
>>
>> 1) It blurs what's a framework
>> 2) We promise ABI and API compatibility for frameworks, but not for
>> other things
>> 3) Moving something from "not a KDE Framework" to "KDE Framework" gives
>> a last chance for fixing up abi/api.
>>
>> so. foo-qt5 is fine for a qt5 version of foo.
>
> I agree, the problem is that there's few exceptions to copy from, so that's
> the exact reason libkdegames has that KF5 thing in the name, the guy that did
> the port just copied what the frmeworks do.
>
> So anyone up for write what "a library that is not frameworks should do to be
> nice in the KDE land"?

We could kill two birds with one stone here, creating a new KDE module
just for libraries (say, KDE Companion Libraries or something) and put
everything in the KC5 (or whatever we decide) namespace.

I'm all for just putting everything in KDE Support, using the KS5
namespace and removing the tier0 restriction from Support.

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