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

2016-02-26 Thread Kai Uwe Broulik

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

(Updated Feb. 26, 2016, 8:56 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, KDEPIM, Daniel Vrátil, Sebastian Kügler, and 
Vishesh Handa.


Changes
---

Submitted with commit 905bf987247cc1d80c319ab44ccaf0306c714f74 by Kai Uwe 
Broulik to branch master.


Repository: kfilemetadata


Description
---

This adds support for the user.xdg.origin.email.subject, 
user.xdg.origin.email.sender, user.xdg.origin.email.message-id xattrs [1] to 
KFileMetadata.

This can (should :P) be populated by KMail when you save an attachment.

Not too happy about the "displayName" I chose. Also we'll need to figure out 
what to index and how we can relate this information and present it to the user 
in a meaningful way. But at least let's collect the information first and then 
we can think about ways to handle this.

[1] https://wiki.freedesktop.org/www/CommonExtendedAttributes/


Diffs
-

  src/properties.h 6ceaca5 
  src/propertyinfo.cpp 4d1fac4 
  src/usermetadata.h 9e10d2a 
  src/usermetadata.cpp 5485d0e 

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


Testing
---

Not really. Tests pass, though.


Thanks,

Kai Uwe Broulik

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


Re: Review Request 127187: Fix build for MSVC (2013) on Windows

2016-02-26 Thread Dāvis Mosāns

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




src/lib/activitiesmodel.h (line 96)


Wouldn't it be better to use _MSC_VER <= 1800 /* MSVC2013 */

as maybe it works for MSVC2015?


- Dāvis Mosāns


On Feb. 26, 2016, 11:32 a.m., Thomas Friedrichsmeier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127187/
> ---
> 
> (Updated Feb. 26, 2016, 11:32 a.m.)
> 
> 
> Review request for KDE Frameworks, kdewin and Ivan Čukić.
> 
> 
> Repository: kactivities
> 
> 
> Description
> ---
> 
> MSVC (2013) complains that beginResetModel(), etc. are protected. This patch 
> allows the code to compile. It is clearly not very elegant, but both my 
> templating- and MSVC-foo is extremely limited. Not sure, if there is a better 
> fix.
> 
> 
> Diffs
> -
> 
>   src/lib/activitiesmodel.h 7258b73 
> 
> Diff: https://git.reviewboard.kde.org/r/127187/diff/
> 
> 
> Testing
> ---
> 
> Now compiles with MSVC 2013 on Windows.
> 
> 
> Thanks,
> 
> Thomas Friedrichsmeier
> 
>

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


Re: Review Request 126291: initial implementation of a platform plugin for OS X (WIP)

2016-02-26 Thread René J . V . Bertin


> On Feb. 25, 2016, 8:26 a.m., Martin Gräßlin wrote:
> > I don't like the introduction of the SCRAPBOOK. The repository is not the 
> > place for dead and old code. That's what git already supports with keeping 
> > the whole history.
> > 
> > We have some autotests for kwindowsystem. Could you try whether the tests 
> > work also on OSX?
> 
> René J.V. Bertin wrote:
> I agree to a certain extent. Git's history feature isn't exactly 
> comparable to a history book in which you can go look around to see if at 
> some point maybe someone contributed some code that was never finished. At 
> the very least you'd need to leave comments in the code that "here used to be 
> some junk DNA that maybe could have led to a whole better world" (and in that 
> case, why not just leave in the code #ifdeffed-out ...)
> 
> As to the autotests: they're built only when `X11_FOUND`. Are you in fact 
> asking me to port them?
> 
> Martin Gräßlin wrote:
> > At the very least you'd need to leave comments in the code that "here 
> used to be some junk DNA that maybe could have led to a whole better world" 
> (and in that case, why not just leave in the code #ifdeffed-out ...)
> 
> eh no, we are not StarOffice. We have a version control system. No need 
> to reference old code.
> 
> > they're built only when X11_FOUND. Are you in fact asking me to port 
> them?
> 
> No, of course not. There are some which might not be X11 specific. E.g. 
> kwindowsystem_threadtest.cpp. It would be good to know which ones can work on 
> OSX
> 
> René J.V. Bertin wrote:
> I'll see for the autotests.
> 
> My point is that you cannot expect that a future developer will study the 
> old commit diffs to check if maybe they contained latent/experimental code 
> that implemented an idea s/he was planning to test.
> The experimental code I've moved is exactly of that nature; it aimed to 
> implement a mechanism by which a table would be maintained of all windows 
> belonging to all running applications. You pointed out correctly that it 
> wouldn't compile and on my end I am not sure whether such a feature would 
> actually be useful (never got feedback on the question I asked about that). 
> So yeah, when the idea of cleaning out inactive code came up and you didn't 
> seem adverse to the idea of keeping code snippets in another place for future 
> reference I came up with the SCRAPBOOK idea.
> Apparently I should have left in the code until I got the green light, 
> and then remove it in an additional commit that does only that, with a 
> sufficiently detailed commit message that the revision history could actually 
> be of some real help here. I can't say I feel like moving all that code back 
> to where it came from, so if you don't want the SCRAPBOOK file I guess it'll 
> just do down the drain.

Re: kwindowsystem_threadtest : that's the only one that doesn't require 
rewriting outside of the autotests CMake file. And it confirms a fear I had: 
there currently is no such thing as an internal list of owned windows, despite 
some efforts to maintain internal structures that allow to return something 
useful in certain situations (that I'll really need to look into again).
Maintaining such a list of windows feels a bit like a chicken-and-egg problem; 
can I rely on a Qt signal or do I need to use a lower-level API. The fact that 
most Qt/KDE applications do not have a Window menu like most Cocoa applications 
do 
(https://developer.apple.com/library/mac/documentation/Cocoa/Conceptual/WinPanel/Tasks/UsingWindowsMenu.html)
 may be an indication that Qt does something peculiar.

Anyway, supposing I'm going to sit down and try to come up with a mechanism to 
maintain a list of the running app's open windows (`WId`s). Can I presume that 
the list will be created (= plugin loaded) when there are no windows yet, 
meaning I only have to listen to window creation events? Or will I need to 
fetch the current list, add that, and then listen for window open events? I 
know how to go from a `WId` to the underlying Cocoa instance, but the other way 
round may be quite a bit more tricky.


- René J.V.


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


On Feb. 23, 2016, 10:54 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126291/
> ---
> 
> (Updated Feb. 23, 2016, 10:54 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and KDE Frameworks.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> KWindowSystem has been lacking a platform plugin for OS X. This RR presents a 
> "backport" of the modified KDE4 

Re: Review Request 126813: Fix build with older polkit

2016-02-26 Thread Alex Richardson

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

(Updated Feb. 26, 2016, 3:59 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Changes
---

Submitted with commit 15c7867e88c5278f61896b5531ac2b544add8220 by Alex 
Richardson to branch master.


Repository: polkit-qt-1


Description
---

Seems like the function exists, but the header declaration is missing


Diffs
-

  CMakeLists.txt bb91bdedc96b8211eb29a1180c2e451dc60fae18 
  core/polkitqt1-subject.h 03028f636d7efc154138821419a704b661a7aeac 
  core/polkitqt1-subject.cpp ecb4c0e216d5bacf5dff5cf100611b941d3e8fbd 
  polkitqt1-config.h.cmake PRE-CREATION 

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


Testing
---

compiles now


Thanks,

Alex Richardson

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


Re: Review Request 127166: Fix xcb port of klauncher and clean up the code.

2016-02-26 Thread Xuetian Weng

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



ping

- Xuetian Weng


On Feb. 24, 2016, 5:19 a.m., Xuetian Weng wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127166/
> ---
> 
> (Updated Feb. 24, 2016, 5:19 a.m.)
> 
> 
> Review request for KDE Frameworks, David Faure and Martin Gräßlin.
> 
> 
> Repository: kinit
> 
> 
> Description
> ---
> 
> The ported klauncher at least has two bugs:
> 1. IMHO, it should compare the cached display string with == instead of != 
> (though != has been there since the old kdelibs, but it just doesn't look 
> correct to me, what's the point of assign dpy = mCached_dpy when dpy_str != 
> XDisplayString(dpy)? And what's the point use == in one place and != in other 
> two places? ).
> 2. it might free the cached connection without setting mCached.conn back to 
> nullptr, which could lead to double free or freeze when system logout.
> 
> This patch refactor the code a little bit with a helper function to update 
> the cached connection instead of duplicate the same logic everywhere. 
> getXCBConnection() will make sure the returned connection is error-free, 
> reuse the cached connection if possible, and update the cached connection if 
> needed.
> 
> 
> Diffs
> -
> 
>   src/klauncher/klauncher.h 31bfaaa 
>   src/klauncher/klauncher.cpp 7ea9da9 
> 
> Diff: https://git.reviewboard.kde.org/r/127166/diff/
> 
> 
> Testing
> ---
> 
> Compiles, startup notify works.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

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


Re: Review Request 127191: KCompletionBox should *not* be a tooltip

2016-02-26 Thread Marco Martin


> On Feb. 26, 2016, 2:14 p.m., Thomas Lübking wrote:
> > src/kcompletionbox.cpp, line 66
> > 
> >
> > q->setAttribute(Qt::WA_X11NetWmWindowTypeCombo); // broken??

ok, i'm blind :p


- Marco


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


On Feb. 26, 2016, 2:18 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127191/
> ---
> 
> (Updated Feb. 26, 2016, 2:18 p.m.)
> 
> 
> Review request for KDE Frameworks and kwin.
> 
> 
> Repository: kcompletion
> 
> 
> Description
> ---
> 
> KCompletionbox it's actually more of  a combobox popup than a tooltip.
> this sets it the proper type (unfortunately with an hack due the type not 
> being available to Qt::WindowFlags)
> 
> without this, the new morphingpopups KWin effect will animate completion 
> boxes (such as in file open dialog, kate, dolphin adress bar etc)
> 
> 
> Diffs
> -
> 
>   src/kcompletionbox.cpp 99afc8e 
> 
> Diff: https://git.reviewboard.kde.org/r/127191/diff/
> 
> 
> Testing
> ---
> 
> Same behavior, due to the Window and bypasswindowmanager that are those two 
> that made the desired behavior when tooltip was picked
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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


Re: Review Request 127191: KCompletionBox should *not* be a tooltip

2016-02-26 Thread Marco Martin

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

(Updated Feb. 26, 2016, 2:18 p.m.)


Review request for KDE Frameworks and kwin.


Repository: kcompletion


Description
---

KCompletionbox it's actually more of  a combobox popup than a tooltip.
this sets it the proper type (unfortunately with an hack due the type not being 
available to Qt::WindowFlags)

without this, the new morphingpopups KWin effect will animate completion boxes 
(such as in file open dialog, kate, dolphin adress bar etc)


Diffs (updated)
-

  src/kcompletionbox.cpp 99afc8e 

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


Testing
---

Same behavior, due to the Window and bypasswindowmanager that are those two 
that made the desired behavior when tooltip was picked


Thanks,

Marco Martin

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


Re: Review Request 127177: Compilation fixes for MSVC (2013)

2016-02-26 Thread Thomas Friedrichsmeier

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

(Updated Feb. 26, 2016, 2:14 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and kdewin.


Changes
---

Submitted with commit 1c2aa25fb179fe3350bee166f2d8e69404d0cc7f by Thomas 
Friedrichsmeier to branch master.


Repository: ki18n


Description
---

Fix compilation on Windows with MSVC.

a) MSVC accepts declspec at global scope, only.
b) QStringLiteral does not work with concatenated literals in MSVC (see 
http://blog.qt.io/blog/2014/06/13/qt-weekly-13-qstringliteral/). Since the 
occurences seem to be mostly for debugging, going through QString::fromLatin1() 
in these cases should not hurt much.


Diffs
-

  src/kcatalog.cpp 083443d 
  src/ktranscript.cpp 72c3755 

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


Testing
---

Now builds with MSVC 2013.

No other compilers and no other platforms tested for this patch.


Thanks,

Thomas Friedrichsmeier

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


Re: Review Request 127191: KCompletionBox should *not* be a tooltip

2016-02-26 Thread Thomas Lübking

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




src/kcompletionbox.cpp (line 66)


q->setAttribute(Qt::WA_X11NetWmWindowTypeCombo); // broken??


- Thomas Lübking


On Feb. 26, 2016, 1:23 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127191/
> ---
> 
> (Updated Feb. 26, 2016, 1:23 p.m.)
> 
> 
> Review request for KDE Frameworks and kwin.
> 
> 
> Repository: kcompletion
> 
> 
> Description
> ---
> 
> KCompletionbox it's actually more of  a combobox popup than a tooltip.
> this sets it the proper type (unfortunately with an hack due the type not 
> being available to Qt::WindowFlags)
> 
> without this, the new morphingpopups KWin effect will animate completion 
> boxes (such as in file open dialog, kate, dolphin adress bar etc)
> 
> 
> Diffs
> -
> 
>   src/kcompletionbox.cpp 99afc8e 
> 
> Diff: https://git.reviewboard.kde.org/r/127191/diff/
> 
> 
> Testing
> ---
> 
> Same behavior, due to the Window and bypasswindowmanager that are those two 
> that made the desired behavior when tooltip was picked
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

___
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

2016-02-26 Thread Pinak Ahuja


> On Feb. 25, 2016, 9:18 p.m., Pinak Ahuja wrote:
> > src/externalextractor.cpp, line 104
> > 
> >
> > 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?
> 
> Boudhayan Gupta wrote:
> 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.

Oh, wait I think you'll have to keep it the way it is. As you said the native 
plugins block the event loop as well and KFileMetadata only provides only a 
synchronous API and the caller is responsible for handling the plugin 
asynchronously, thats what we do in baloo as well, we have a seperate QProcess 
which uses the Extractor plugins.

Anyways closing the isssue, sorry for the noise.


- Pinak


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


On Feb. 25, 2016, 6:32 p.m., Varun Joshi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127179/
> ---
> 
> (Updated Feb. 25, 2016, 6:32 p.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 127191: KCompletionBox should *not* be a tooltip

2016-02-26 Thread Martin Gräßlin

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



+1

- Martin Gräßlin


On Feb. 26, 2016, 2:23 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127191/
> ---
> 
> (Updated Feb. 26, 2016, 2:23 p.m.)
> 
> 
> Review request for KDE Frameworks and kwin.
> 
> 
> Repository: kcompletion
> 
> 
> Description
> ---
> 
> KCompletionbox it's actually more of  a combobox popup than a tooltip.
> this sets it the proper type (unfortunately with an hack due the type not 
> being available to Qt::WindowFlags)
> 
> without this, the new morphingpopups KWin effect will animate completion 
> boxes (such as in file open dialog, kate, dolphin adress bar etc)
> 
> 
> Diffs
> -
> 
>   src/kcompletionbox.cpp 99afc8e 
> 
> Diff: https://git.reviewboard.kde.org/r/127191/diff/
> 
> 
> Testing
> ---
> 
> Same behavior, due to the Window and bypasswindowmanager that are those two 
> that made the desired behavior when tooltip was picked
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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


Review Request 127191: KCompletionBox should *not* be a tooltip

2016-02-26 Thread Marco Martin

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

Review request for KDE Frameworks and kwin.


Repository: kcompletion


Description
---

KCompletionbox it's actually more of  a combobox popup than a tooltip.
this sets it the proper type (unfortunately with an hack due the type not being 
available to Qt::WindowFlags)

without this, the new morphingpopups KWin effect will animate completion boxes 
(such as in file open dialog, kate, dolphin adress bar etc)


Diffs
-

  src/kcompletionbox.cpp 99afc8e 

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


Testing
---

Same behavior, due to the Window and bypasswindowmanager that are those two 
that made the desired behavior when tooltip was picked


Thanks,

Marco Martin

___
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

2016-02-26 Thread Boudhayan Gupta


> On Feb. 26, 2016, 2:48 a.m., Pinak Ahuja wrote:
> > src/externalextractor.cpp, line 104
> > 
> >
> > 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 127177: Compilation fixes for MSVC (2013)

2016-02-26 Thread Kevin Funk

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


Fix it, then Ship it!





src/kcatalog.cpp (line 46)


Remove indentation. Same below.


- Kevin Funk


On Feb. 25, 2016, 5:16 p.m., Thomas Friedrichsmeier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127177/
> ---
> 
> (Updated Feb. 25, 2016, 5:16 p.m.)
> 
> 
> Review request for KDE Frameworks and kdewin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> Fix compilation on Windows with MSVC.
> 
> a) MSVC accepts declspec at global scope, only.
> b) QStringLiteral does not work with concatenated literals in MSVC (see 
> http://blog.qt.io/blog/2014/06/13/qt-weekly-13-qstringliteral/). Since the 
> occurences seem to be mostly for debugging, going through 
> QString::fromLatin1() in these cases should not hurt much.
> 
> 
> Diffs
> -
> 
>   src/kcatalog.cpp 083443d 
>   src/ktranscript.cpp 72c3755 
> 
> Diff: https://git.reviewboard.kde.org/r/127177/diff/
> 
> 
> Testing
> ---
> 
> Now builds with MSVC 2013.
> 
> No other compilers and no other platforms tested for this patch.
> 
> 
> Thanks,
> 
> Thomas Friedrichsmeier
> 
>

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


Re: QString -> QStringLiteral conversions might make applications crash on exit

2016-02-26 Thread Jan Kundrát

On Friday, 26 February 2016 01:37:57 CET, Frank Reininghaus wrote:

This becomes a problem if the read-only data that the QString refers
to are not there any more, which can happen if the QString was stored
in a global static object from one library, and the QStringLiteral is
from another library, which might have been unloaded before the global
static object was destroyed.


Are you 100% sure that this is not "just" due to icon loaders which are 
implemented as plugins [1]?


Upstream says [2] that they do not want to support unloading of plugins 
which "leak" Qt data. There's a patch [3] which implements this for all 
plugins (including the icon loaders). Apparently, it's been so ever since 
Qt 5.0.


For an example on how an application can workaround this thing, see my 
patch to Trojitá [4].


Cheers,
Jan

[1] https://bugreports.qt.io/browse/QTBUG-50829
[2] 
https://bugreports.qt.io/browse/QTBUG-49061?focusedCommentId=297937=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-297937

[3] https://codereview.qt-project.org/140750
[4] 
https://gerrit.vesnicky.cesnet.cz/r/#/c/595/5/src/UiUtils/IconLoader.cpp


--
Trojitá, a fast Qt IMAP e-mail client -- http://trojita.flaska.net/
___
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

2016-02-26 Thread Varun Joshi


> On Feb. 25, 2016, 9:18 p.m., Pinak Ahuja wrote:
> > src/externalextractor.cpp, line 104
> > 
> >
> > 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.

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?


- Varun


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


On Feb. 25, 2016, 6:32 p.m., Varun Joshi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127179/
> ---
> 
> (Updated Feb. 25, 2016, 6:32 p.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: QString -> QStringLiteral conversions might make applications crash on exit

2016-02-26 Thread Nick Shaforostoff
> a) Everyone who makes QString -> QStringLiteral replacements should be
> extremely careful (which is very difficult, since it is not always
> obvious if passing a QString to a function will result in the string
> being stored in a global static object). Automated tools like clazy
> should then not recommend to use QStringLiteral any more.
> 
> b) Classes like KIconLoader, which are used as global static objects,
> should copy all strings that they get to the heap in order to prevent
> such crashes (which might also be quite difficult to do consistently).

from what i have seen, at least icons, fonts and regexps are the ones for which 
stringliterals should be avoided in libraries.
i have created a qt issue for mentioning this in qt documentation
https://bugreports.qt.io/browse/QTBUG-51418 

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


Re: polkit-qt-1 does not build

2016-02-26 Thread Ben Cooksley
On Fri, Feb 26, 2016 at 11:13 PM, Martin Graesslin  wrote:
> On Friday, February 26, 2016 10:37:10 PM CET Ben Cooksley wrote:
>> Hi all,
>>
>> I've been hammering away at some of the builds on our Jenkins Sandbox,
>> and have found it isn't possible to get polkit-qt-1 to build.
>>
>> This blocks the entire KDE 4 chain, among other things.
>>
>> Could someone please investigate?
>> https://build-sandbox.kde.org/job/polkit-qt-1%20master%20latest-qt4/2/PLATFO
>> RM=Linux,compiler=gcc/console
>
> This should be fixed with https://git.reviewboard.kde.org/r/126813/

Excellent. Thanks for the quick response.

>
> Cheers
> Martin

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


Re: Review Request 126813: Fix build with older polkit

2016-02-26 Thread Martin Gräßlin

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


Ship it!




Ship It!

- Martin Gräßlin


On Jan. 21, 2016, 5:24 p.m., Alex Richardson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126813/
> ---
> 
> (Updated Jan. 21, 2016, 5:24 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: polkit-qt-1
> 
> 
> Description
> ---
> 
> Seems like the function exists, but the header declaration is missing
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt bb91bdedc96b8211eb29a1180c2e451dc60fae18 
>   core/polkitqt1-subject.h 03028f636d7efc154138821419a704b661a7aeac 
>   core/polkitqt1-subject.cpp ecb4c0e216d5bacf5dff5cf100611b941d3e8fbd 
>   polkitqt1-config.h.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126813/diff/
> 
> 
> Testing
> ---
> 
> compiles now
> 
> 
> Thanks,
> 
> Alex Richardson
> 
>

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


polkit-qt-1 does not build

2016-02-26 Thread Ben Cooksley
Hi all,

I've been hammering away at some of the builds on our Jenkins Sandbox,
and have found it isn't possible to get polkit-qt-1 to build.

This blocks the entire KDE 4 chain, among other things.

Could someone please investigate?
https://build-sandbox.kde.org/job/polkit-qt-1%20master%20latest-qt4/2/PLATFORM=Linux,compiler=gcc/console

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


Review Request 127187: Fix build for MSVC (2013) on Windows

2016-02-26 Thread Thomas Friedrichsmeier

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

Review request for KDE Frameworks, kdewin and Ivan Čukić.


Repository: kactivities


Description
---

MSVC (2013) complains that beginResetModel(), etc. are protected. This patch 
allows the code to compile. It is clearly not very elegant, but both my 
templating- and MSVC-foo is extremely limited. Not sure, if there is a better 
fix.


Diffs
-

  src/lib/activitiesmodel.h 7258b73 

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


Testing
---

Now compiles with MSVC 2013 on Windows.


Thanks,

Thomas Friedrichsmeier

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


Re: Review Request 126291: initial implementation of a platform plugin for OS X (WIP)

2016-02-26 Thread René J . V . Bertin


> On Feb. 25, 2016, 8:26 a.m., Martin Gräßlin wrote:
> > I don't like the introduction of the SCRAPBOOK. The repository is not the 
> > place for dead and old code. That's what git already supports with keeping 
> > the whole history.
> > 
> > We have some autotests for kwindowsystem. Could you try whether the tests 
> > work also on OSX?
> 
> René J.V. Bertin wrote:
> I agree to a certain extent. Git's history feature isn't exactly 
> comparable to a history book in which you can go look around to see if at 
> some point maybe someone contributed some code that was never finished. At 
> the very least you'd need to leave comments in the code that "here used to be 
> some junk DNA that maybe could have led to a whole better world" (and in that 
> case, why not just leave in the code #ifdeffed-out ...)
> 
> As to the autotests: they're built only when `X11_FOUND`. Are you in fact 
> asking me to port them?
> 
> Martin Gräßlin wrote:
> > At the very least you'd need to leave comments in the code that "here 
> used to be some junk DNA that maybe could have led to a whole better world" 
> (and in that case, why not just leave in the code #ifdeffed-out ...)
> 
> eh no, we are not StarOffice. We have a version control system. No need 
> to reference old code.
> 
> > they're built only when X11_FOUND. Are you in fact asking me to port 
> them?
> 
> No, of course not. There are some which might not be X11 specific. E.g. 
> kwindowsystem_threadtest.cpp. It would be good to know which ones can work on 
> OSX

I'll see for the autotests.

My point is that you cannot expect that a future developer will study the old 
commit diffs to check if maybe they contained latent/experimental code that 
implemented an idea s/he was planning to test.
The experimental code I've moved is exactly of that nature; it aimed to 
implement a mechanism by which a table would be maintained of all windows 
belonging to all running applications. You pointed out correctly that it 
wouldn't compile and on my end I am not sure whether such a feature would 
actually be useful (never got feedback on the question I asked about that). So 
yeah, when the idea of cleaning out inactive code came up and you didn't seem 
adverse to the idea of keeping code snippets in another place for future 
reference I came up with the SCRAPBOOK idea.
Apparently I should have left in the code until I got the green light, and then 
remove it in an additional commit that does only that, with a sufficiently 
detailed commit message that the revision history could actually be of some 
real help here. I can't say I feel like moving all that code back to where it 
came from, so if you don't want the SCRAPBOOK file I guess it'll just do down 
the drain.


- René J.V.


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


On Feb. 23, 2016, 10:54 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126291/
> ---
> 
> (Updated Feb. 23, 2016, 10:54 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and KDE Frameworks.
> 
> 
> Repository: kwindowsystem
> 
> 
> Description
> ---
> 
> KWindowSystem has been lacking a platform plugin for OS X. This RR presents a 
> "backport" of the modified KDE4 KWindowSystem implementation that has been 
> used in the MacPorts kdelibs4 port for the last 2 or 3 (or more) years.
> 
> I have made some initial steps to remove deprecated Carbon API calls, but 
> this is clearly a work in progress.
> 
> Open questions include
> - is there any justification to run an event handler (or Cocoa observer) to 
> keep track of running applications, possibly even listing all their open 
> windows?
> - is there any use for the Qt event listener framework (cf. the 
> NETEventFilter in the X11 plugin)? I haven't yet had time to try to figure 
> out what this "could be good for", and am very open to suggestions in this 
> departments.
> - one application for such an event filter would be a listener that catches 
> the opening and closing of all windows by the running process, and keeps 
> track of their `WId`s. A new method could then be added (to `KWindowInfo`?) 
> to distinguish `WId`s created by the running application from "foreign" ones 
> (useful also on Wayland and MS Windows).
> 
> `KWindowSystem::setMainWindow` should become a front for payload provided by 
> the plugins. I'll leave that to the regular/official maintainer(s) of this 
> framework.
> 
> This code could probably do with *lots* of comments; I'll try to add them as 
> questions about this or that bit of code arise.
> 
> 
> Diffs
> -
> 
>   src/debug_p.h 5ef8996 
>