Re: Review Request 123669: Add KPluginLoader::findPluginsById convenience API

2015-05-07 Thread Alex Richardson

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


Seems like this is duplicated in a few places already so I agree we should add 
it. But won't most users of the API want only a single plugin returned?
Maybe also add a function `KPluginMetaData KPluginLoader::findPluginById(const 
QString directory, const QString pluginId)`?
Do we also need the function that returns a vector for a given ID?


src/lib/plugin/kpluginloader.cpp (line 278)
https://git.reviewboard.kde.org/r/123669/#comment54900

QVector KPluginMetaData   - QVectorKPluginMetaData


- Alex Richardson


On May 7, 2015, 12:21 a.m., Sebastian Kügler wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123669/
 ---
 
 (Updated May 7, 2015, 12:21 a.m.)
 
 
 Review request for KDE Frameworks, Alex Richardson and David Faure.
 
 
 Repository: kcoreaddons
 
 
 Description
 ---
 
 Add findPluginsById convenience API
 
 It's a quite common case to load a plugin from an ID. This makes it
 easy.
 
 CHANGELOG:New KPluginLoader::findPluginById() convenience API
 REVIEW:123669
 
 
 Diffs
 -
 
   autotests/kpluginloadertest.cpp 3ded0ebca2e0fd20e09bf6e4eca152d13ac11f46 
   src/lib/plugin/kpluginloader.h 124bfab7e207b17d3c8ab4d5a88321af476aad42 
   src/lib/plugin/kpluginloader.cpp 4310d6cd7792329511f12b28d7c68b0fdd013538 
 
 Diff: https://git.reviewboard.kde.org/r/123669/diff/
 
 
 Testing
 ---
 
 Added autotests, everything passes.
 
 
 Thanks,
 
 Sebastian Kügler
 


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


Re: Review Request 123626: port libplasma away from sycoca as much as possible

2015-05-07 Thread Marco Martin


 On May 6, 2015, 10:54 p.m., Sebastian Kügler wrote:
  src/plasma/pluginloader.cpp, line 892
  https://git.reviewboard.kde.org/r/123626/diff/4/?file=366900#file366900line892
 
  Can this lead to containmentactions being listed twice? (I haven't seen 
  it in the UI, but seems plausible as a corner case.)

this lists the plugins that are in plasma/containmentactions (of which at the 
moment there aren't any)
and then adds the ones fetched with sycoca.
in our case, the plugins are all from plasma-workspace and they would get moved 
from one version to the other. If one reinstalls without cleaning then yup, it 
would duplicate entries.
On a clean installation this should never happen, but i can add a check on the 
plugin name to not add it two times, may be slower but shouldn't have a big 
impact.


- Marco


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


On May 6, 2015, 5:21 p.m., Marco Martin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123626/
 ---
 
 (Updated May 6, 2015, 5:21 p.m.)
 
 
 Review request for KDE Frameworks and Plasma.
 
 
 Repository: plasma-framework
 
 
 Description
 ---
 
 this ports most of libplasma away from sycoca, using instead a combination of 
 KPluginLoader and KPackage::PackageLoader instead (so eventually using their 
 own little caches instead of the global sycoca cache)
 a kservicetypetrader call is left in the loading of containmentactions since 
 is the only way to make an older workspace still work, but is only a 
 fallback, so containmentactions in plasma-workspace can be ported eventually 
 as well
 
 
 Diffs
 -
 
   KF5PlasmaConfig.cmake.in dee79ca 
   src/declarativeimports/plasmacomponents/qml/private/TabBarLayout.qml 
 be76a43 
   src/plasma/applet.cpp 2dda381 
   src/plasma/containment.cpp 7eabbb8 
   src/plasma/containmentactions.cpp f24bdac 
   src/plasma/dataengine.cpp 4b3d671 
   src/plasma/package.cpp 4e0be88 
   src/plasma/packagestructure.cpp b2686b6 
   src/plasma/pluginloader.cpp 0ce349a 
   src/plasma/private/applet_p.cpp 2cbfd06 
   src/plasma/private/dataenginemanager.cpp b9c5d8e 
   src/plasma/scripting/appletscript.h 863d707 
   src/plasma/scripting/dataenginescript.h f7ad3c2 
   src/plasma/scripting/scriptengine.cpp cc407e7 
   src/plasma/service.h b5d9b5a 
   src/plasma/service.cpp 3e9d852 
   src/plasmapkg/plasmapkg.cpp d606365 
   src/scriptengines/CMakeLists.txt f566406 
   src/scriptengines/qml/CMakeLists.txt e7130db 
   src/scriptengines/qml/plasmoid/appletinterface.cpp 07ecb3d 
   src/scriptengines/qml/plasmoid/containmentinterface.cpp c6986f2 
   src/scriptengines/qml/plasmoid/declarativeappletscript.cpp 24c39dd 
   src/scriptengines/qml/plasmoid/wallpaperinterface.cpp a406d45 
 
 Diff: https://git.reviewboard.kde.org/r/123626/diff/
 
 
 Testing
 ---
 
 still needs a lot of testing, so far, with a master plasma-workspace notmal 
 workspace functions seems fine
 
 latest version still installs metadata files in kservices5. I would keep this 
 for safety until Plasma 5.4 is released, then try to remove it again
 
 
 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 123626: port libplasma away from sycoca as much as possible

2015-05-07 Thread Marco Martin

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


solved issues, still there the magic self() call, for that, an api addition in 
KPackage may be needed.

- Marco Martin


On May 7, 2015, 9:06 a.m., Marco Martin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123626/
 ---
 
 (Updated May 7, 2015, 9:06 a.m.)
 
 
 Review request for KDE Frameworks and Plasma.
 
 
 Repository: plasma-framework
 
 
 Description
 ---
 
 this ports most of libplasma away from sycoca, using instead a combination of 
 KPluginLoader and KPackage::PackageLoader instead (so eventually using their 
 own little caches instead of the global sycoca cache)
 a kservicetypetrader call is left in the loading of containmentactions since 
 is the only way to make an older workspace still work, but is only a 
 fallback, so containmentactions in plasma-workspace can be ported eventually 
 as well
 
 
 Diffs
 -
 
   KF5PlasmaConfig.cmake.in dee79ca 
   src/declarativeimports/plasmacomponents/qml/private/TabBarLayout.qml 
 be76a43 
   src/plasma/applet.cpp 2dda381 
   src/plasma/containment.cpp 7eabbb8 
   src/plasma/containmentactions.cpp f24bdac 
   src/plasma/dataengine.cpp 4b3d671 
   src/plasma/package.cpp 4e0be88 
   src/plasma/packagestructure.cpp b2686b6 
   src/plasma/pluginloader.cpp 0ce349a 
   src/plasma/private/applet_p.cpp 2cbfd06 
   src/plasma/private/dataenginemanager.cpp b9c5d8e 
   src/plasma/scripting/appletscript.h 863d707 
   src/plasma/scripting/dataenginescript.h f7ad3c2 
   src/plasma/scripting/scriptengine.cpp cc407e7 
   src/plasma/service.h b5d9b5a 
   src/plasma/service.cpp 3e9d852 
   src/plasmapkg/plasmapkg.cpp d606365 
   src/scriptengines/CMakeLists.txt f566406 
   src/scriptengines/qml/CMakeLists.txt e7130db 
   src/scriptengines/qml/plasmoid/appletinterface.cpp 07ecb3d 
   src/scriptengines/qml/plasmoid/containmentinterface.cpp c6986f2 
   src/scriptengines/qml/plasmoid/declarativeappletscript.cpp 24c39dd 
   src/scriptengines/qml/plasmoid/wallpaperinterface.cpp a406d45 
 
 Diff: https://git.reviewboard.kde.org/r/123626/diff/
 
 
 Testing
 ---
 
 still needs a lot of testing, so far, with a master plasma-workspace notmal 
 workspace functions seems fine
 
 latest version still installs metadata files in kservices5. I would keep this 
 for safety until Plasma 5.4 is released, then try to remove it again
 
 
 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 123677: Bring back use of KEncodingFileDialog

2015-05-07 Thread Christoph Feck


 On May 7, 2015, 8:48 p.m., David Rosca wrote:
  Sorry, didn't know of the bug. Should I discard this rr?

Why? What you propose here was discussed as one possible option.


- Christoph


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


On May 7, 2015, 8:45 p.m., David Rosca wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123677/
 ---
 
 (Updated May 7, 2015, 8:45 p.m.)
 
 
 Review request for Kate and KDE Frameworks.
 
 
 Bugs: 343255
 https://bugs.kde.org/show_bug.cgi?id=343255
 
 
 Repository: ktexteditor
 
 
 Description
 ---
 
 This reverts commits 6e57274917f146ac233f50fcb902e8f2569bbbd2 
 (document/katedocument.cpp)
 and 4ef4063a34314480287540a0a4f58127bd6523f2 (link to KIOFileWidgets).
 
 It does use native file dialogs now (not sure if it didn't at the time of 
 that commit). And mainly this fixes
 regression from KDE4, it is now possible to change encoding of files again.
 
 
 Diffs
 -
 
   src/CMakeLists.txt 70c4d89 
   src/document/katedocument.cpp 4f66a47 
 
 Diff: https://git.reviewboard.kde.org/r/123677/diff/
 
 
 Testing
 ---
 
 Save file dialogs are working fine and saving to different encoding works too.
 
 
 Thanks,
 
 David Rosca
 


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


Re: Review Request 123493: Fix leaky incubation controller

2015-05-07 Thread David Edmundson

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

(Updated May 7, 2015, 9:20 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Plasma.


Changes
---

Submitted with commit 3ec8ccd49943f513d5fbcb15ad169339360ad1a7 by David 
Edmundson to branch master.


Repository: kdeclarative


Description
---

Docs for QQmlEngine explcitly say

The engine can only have one active controller and it does not take
ownership of it. therefore we need to set a parent.

The incubator controls the deletion of generated objects, so this means we're 
leaking all QML items created by the KDeclarative::QmlObject.


Diffs
-

  src/kdeclarative/qmlobject.cpp c483665c43985ba57459a880895ee8bf7ff92041 

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


Testing
---

Plasma shell still loads and runs fine. Valgrind is a lot less angry.
Will merge after 5.10 to play safe.


Thanks,

David Edmundson

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


Review Request 123679: Fix build with Qt 5.5

2015-05-07 Thread Jan Kundrát

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

Review request for KDE Frameworks and Stephen Kelly.


Repository: kitemmodels


Description
---

Turns out that there's apparently no overload accepting a QPointer as
the first argument.

This is on Gentoo with qtbase e374ffc29c67493a51527117b55a53dfa5dd4267
and GCC 4.8.3.


Diffs
-

  src/kselectionproxymodel.cpp 0755fb1ca527dde8287ba2607d915a4e18c20134 

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


Testing
---


Thanks,

Jan Kundrát

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


Re: Review Request 123626: port libplasma away from sycoca as much as possible

2015-05-07 Thread Marco Martin


 On May 7, 2015, 11:59 a.m., Sebastian Kügler wrote:
  src/declarativeimports/plasmacomponents/qml/private/TabBarLayout.qml, line 
  197
  https://git.reviewboard.kde.org/r/123626/diff/5/?file=367344#file367344line197
 
  This line may have slipped in?

ouch, i pushed a commit in the wrong branch :/


- Marco


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


On May 7, 2015, 9:06 a.m., Marco Martin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123626/
 ---
 
 (Updated May 7, 2015, 9:06 a.m.)
 
 
 Review request for KDE Frameworks and Plasma.
 
 
 Repository: plasma-framework
 
 
 Description
 ---
 
 this ports most of libplasma away from sycoca, using instead a combination of 
 KPluginLoader and KPackage::PackageLoader instead (so eventually using their 
 own little caches instead of the global sycoca cache)
 a kservicetypetrader call is left in the loading of containmentactions since 
 is the only way to make an older workspace still work, but is only a 
 fallback, so containmentactions in plasma-workspace can be ported eventually 
 as well
 
 
 Diffs
 -
 
   KF5PlasmaConfig.cmake.in dee79ca 
   src/declarativeimports/plasmacomponents/qml/private/TabBarLayout.qml 
 be76a43 
   src/plasma/applet.cpp 2dda381 
   src/plasma/containment.cpp 7eabbb8 
   src/plasma/containmentactions.cpp f24bdac 
   src/plasma/dataengine.cpp 4b3d671 
   src/plasma/package.cpp 4e0be88 
   src/plasma/packagestructure.cpp b2686b6 
   src/plasma/pluginloader.cpp 0ce349a 
   src/plasma/private/applet_p.cpp 2cbfd06 
   src/plasma/private/dataenginemanager.cpp b9c5d8e 
   src/plasma/scripting/appletscript.h 863d707 
   src/plasma/scripting/dataenginescript.h f7ad3c2 
   src/plasma/scripting/scriptengine.cpp cc407e7 
   src/plasma/service.h b5d9b5a 
   src/plasma/service.cpp 3e9d852 
   src/plasmapkg/plasmapkg.cpp d606365 
   src/scriptengines/CMakeLists.txt f566406 
   src/scriptengines/qml/CMakeLists.txt e7130db 
   src/scriptengines/qml/plasmoid/appletinterface.cpp 07ecb3d 
   src/scriptengines/qml/plasmoid/containmentinterface.cpp c6986f2 
   src/scriptengines/qml/plasmoid/declarativeappletscript.cpp 24c39dd 
   src/scriptengines/qml/plasmoid/wallpaperinterface.cpp a406d45 
 
 Diff: https://git.reviewboard.kde.org/r/123626/diff/
 
 
 Testing
 ---
 
 still needs a lot of testing, so far, with a master plasma-workspace notmal 
 workspace functions seems fine
 
 latest version still installs metadata files in kservices5. I would keep this 
 for safety until Plasma 5.4 is released, then try to remove it again
 
 
 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 123492: Fix memory leak in AppletQuickItem

2015-05-07 Thread David Edmundson

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

(Updated May 7, 2015, 9:18 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Plasma.


Changes
---

Submitted with commit 04f96779c119d67aa636a7435176775aff5ccf0f by David 
Edmundson to branch master.


Repository: plasma-framework


Description
---

Change-Id: If96689f937cf3b7e46c6eeacf8e8f850e9db571a


Diffs
-

  src/plasmaquick/appletquickitem.cpp b608a80adbc9f737027e59ff5c9f9933df8de06e 

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


Testing
---


Thanks,

David Edmundson

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


Review Request 123677: Bring back use of KEncodingFileDialog

2015-05-07 Thread David Rosca

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

Review request for KDE Frameworks.


Repository: ktexteditor


Description
---

This reverts commits 6e57274917f146ac233f50fcb902e8f2569bbbd2 
(document/katedocument.cpp)
and 4ef4063a34314480287540a0a4f58127bd6523f2 (link to KIOFileWidgets).

It does use native file dialogs now (not sure if it didn't at the time of that 
commit). And mainly this fixes
regression from KDE4, it is now possible to change encoding of files again.


Diffs
-

  src/CMakeLists.txt 70c4d89 
  src/document/katedocument.cpp 4f66a47 

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


Testing
---

Save file dialogs are working fine and saving to different encoding works too.


Thanks,

David Rosca

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


Re: Review Request 123677: Bring back use of KEncodingFileDialog

2015-05-07 Thread David Rosca

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

(Updated May 7, 2015, 8:45 p.m.)


Review request for Kate and KDE Frameworks.


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


Repository: ktexteditor


Description
---

This reverts commits 6e57274917f146ac233f50fcb902e8f2569bbbd2 
(document/katedocument.cpp)
and 4ef4063a34314480287540a0a4f58127bd6523f2 (link to KIOFileWidgets).

It does use native file dialogs now (not sure if it didn't at the time of that 
commit). And mainly this fixes
regression from KDE4, it is now possible to change encoding of files again.


Diffs
-

  src/CMakeLists.txt 70c4d89 
  src/document/katedocument.cpp 4f66a47 

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


Testing
---

Save file dialogs are working fine and saving to different encoding works too.


Thanks,

David Rosca

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


Re: Review Request 123677: Bring back use of KEncodingFileDialog

2015-05-07 Thread David Rosca


 On May 7, 2015, 8:48 p.m., David Rosca wrote:
  Sorry, didn't know of the bug. Should I discard this rr?
 
 Christoph Feck wrote:
 Why? What you propose here was discussed as one possible option.

Yes, but from the bug discussion it seems reverting those commits is not 
something that Christoph Cullmann would agree with.


- David


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


On May 7, 2015, 8:45 p.m., David Rosca wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123677/
 ---
 
 (Updated May 7, 2015, 8:45 p.m.)
 
 
 Review request for Kate and KDE Frameworks.
 
 
 Bugs: 343255
 https://bugs.kde.org/show_bug.cgi?id=343255
 
 
 Repository: ktexteditor
 
 
 Description
 ---
 
 This reverts commits 6e57274917f146ac233f50fcb902e8f2569bbbd2 
 (document/katedocument.cpp)
 and 4ef4063a34314480287540a0a4f58127bd6523f2 (link to KIOFileWidgets).
 
 It does use native file dialogs now (not sure if it didn't at the time of 
 that commit). And mainly this fixes
 regression from KDE4, it is now possible to change encoding of files again.
 
 
 Diffs
 -
 
   src/CMakeLists.txt 70c4d89 
   src/document/katedocument.cpp 4f66a47 
 
 Diff: https://git.reviewboard.kde.org/r/123677/diff/
 
 
 Testing
 ---
 
 Save file dialogs are working fine and saving to different encoding works too.
 
 
 Thanks,
 
 David Rosca
 


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


Re: Review Request 123677: Bring back use of KEncodingFileDialog

2015-05-07 Thread David Rosca

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


Sorry, didn't know of the bug. Should I discard this rr?

- David Rosca


On May 7, 2015, 8:45 p.m., David Rosca wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123677/
 ---
 
 (Updated May 7, 2015, 8:45 p.m.)
 
 
 Review request for Kate and KDE Frameworks.
 
 
 Bugs: 343255
 https://bugs.kde.org/show_bug.cgi?id=343255
 
 
 Repository: ktexteditor
 
 
 Description
 ---
 
 This reverts commits 6e57274917f146ac233f50fcb902e8f2569bbbd2 
 (document/katedocument.cpp)
 and 4ef4063a34314480287540a0a4f58127bd6523f2 (link to KIOFileWidgets).
 
 It does use native file dialogs now (not sure if it didn't at the time of 
 that commit). And mainly this fixes
 regression from KDE4, it is now possible to change encoding of files again.
 
 
 Diffs
 -
 
   src/CMakeLists.txt 70c4d89 
   src/document/katedocument.cpp 4f66a47 
 
 Diff: https://git.reviewboard.kde.org/r/123677/diff/
 
 
 Testing
 ---
 
 Save file dialogs are working fine and saving to different encoding works too.
 
 
 Thanks,
 
 David Rosca
 


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


debugfull - debug

2015-05-07 Thread Jaroslaw Staniek
Hi,
As with some other bits of knowledge during porting today I just share this one:

https://community.kde.org/index.php?title=Calligra/Building/3diff=42353oldid=42349

When you use Qt4/KF5, debugfull is dead [1] so you'd be surprised
debugging isn't working for you.

We don't have any precise recipe from the core folks involved in the
kf5, I can only say the above works like before when you also
set
CMAKE_CXX_FLAGS_DEBUG:STRING=-g3
CMAKE_C_FLAGS_DEBUG:STRING=-g3
in your cmake cache.

CC'd the kf list so maybe my lame solution could be improved and find
its way to the porting notes [2].

[1] 
http://quickgit.kde.org/?p=extra-cmake-modules.gita=commith=4068592ad9aa3f241027f6db
[2] https://community.kde.org/Frameworks/Porting_Notes

-- 
regards, Jaroslaw Staniek

KDE:
: A world-wide network of software engineers, artists, writers, translators
: and facilitators committed to Free Software development - http://kde.org
Calligra Suite:
: A graphic art and office suite - http://calligra.org
Kexi:
: A visual database apps builder - http://calligra.org/kexi
Qt Certified Specialist:
: http://www.linkedin.com/in/jstaniek
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 123669: Add KPluginLoader::findPluginsById convenience API

2015-05-07 Thread Sebastian Kügler


 On May 7, 2015, 7:24 a.m., Alex Richardson wrote:
  Seems like this is duplicated in a few places already so I agree we should 
  add it. But won't most users of the API want only a single plugin returned?
  Maybe also add a function `KPluginMetaData 
  KPluginLoader::findPluginById(const QString directory, const QString 
  pluginId)`?
  Do we also need the function that returns a vector for a given ID?
 
 Sebastian Kügler wrote:
 At least our changes in libplasma need a QVectorKPluginMetaData. 
 Otherwise, a list seems easy enough to check if something's found. Returning 
 a single metadata won't be very useful for us at this point (but I see it 
 making sense).

Ow, also, and perhaps more importantly, multiple ids are technically perfectly 
valid (only the plugin name and directory are important here). I think that 
fact should be reflected in the API. Perhaps a word on ordering would be in 
place in the API docs, plugin locations are cascaded properly in code using it. 
The most local plugin is at the end of the list, system-widely installed ones 
at the beginning, so code that uses plugins.first() would not allow the user to 
override plugins installed for example into /usr/lib with a plugin with the 
same id and relative path installed into ~/.local). So an extra method 
returning the .last() plugin found might take away this caveat from the API. 
We'll still need the method returning a vector for libplasma, though (and I 
think it's a semantically useful addition).

About adding another method to return the most-local plugin, I'm on the edge. 
If others think it's useful and we think the additional API (with its long-term 
maintainance implications) is worth it, I'm happy to add it as well. (Perhaps 
in a separate review.)

Opinions welcome.


- Sebastian


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


On May 6, 2015, 11:21 p.m., Sebastian Kügler wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123669/
 ---
 
 (Updated May 6, 2015, 11:21 p.m.)
 
 
 Review request for KDE Frameworks, Alex Richardson and David Faure.
 
 
 Repository: kcoreaddons
 
 
 Description
 ---
 
 Add findPluginsById convenience API
 
 It's a quite common case to load a plugin from an ID. This makes it
 easy.
 
 CHANGELOG:New KPluginLoader::findPluginById() convenience API
 REVIEW:123669
 
 
 Diffs
 -
 
   autotests/kpluginloadertest.cpp 3ded0ebca2e0fd20e09bf6e4eca152d13ac11f46 
   src/lib/plugin/kpluginloader.h 124bfab7e207b17d3c8ab4d5a88321af476aad42 
   src/lib/plugin/kpluginloader.cpp 4310d6cd7792329511f12b28d7c68b0fdd013538 
 
 Diff: https://git.reviewboard.kde.org/r/123669/diff/
 
 
 Testing
 ---
 
 Added autotests, everything passes.
 
 
 Thanks,
 
 Sebastian Kügler
 


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


Re: Review Request 123626: port libplasma away from sycoca as much as possible

2015-05-07 Thread Marco Martin

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

(Updated May 7, 2015, 2:31 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Plasma.


Changes
---

Submitted with commit ea924b14691da3819ca17d876f63ffc686fcf840 by Marco Martin 
to branch master.


Repository: plasma-framework


Description
---

this ports most of libplasma away from sycoca, using instead a combination of 
KPluginLoader and KPackage::PackageLoader instead (so eventually using their 
own little caches instead of the global sycoca cache)
a kservicetypetrader call is left in the loading of containmentactions since is 
the only way to make an older workspace still work, but is only a fallback, so 
containmentactions in plasma-workspace can be ported eventually as well


Diffs
-

  KF5PlasmaConfig.cmake.in dee79ca 
  src/declarativeimports/plasmacomponents/qml/private/TabBarLayout.qml be76a43 
  src/plasma/applet.cpp 2dda381 
  src/plasma/containment.cpp 7eabbb8 
  src/plasma/containmentactions.cpp f24bdac 
  src/plasma/dataengine.cpp 4b3d671 
  src/plasma/package.cpp 4e0be88 
  src/plasma/packagestructure.cpp b2686b6 
  src/plasma/pluginloader.cpp 0ce349a 
  src/plasma/private/applet_p.cpp 2cbfd06 
  src/plasma/private/dataenginemanager.cpp b9c5d8e 
  src/plasma/scripting/appletscript.h 863d707 
  src/plasma/scripting/dataenginescript.h f7ad3c2 
  src/plasma/scripting/scriptengine.cpp cc407e7 
  src/plasma/service.h b5d9b5a 
  src/plasma/service.cpp 3e9d852 
  src/plasmapkg/plasmapkg.cpp d606365 
  src/scriptengines/CMakeLists.txt f566406 
  src/scriptengines/qml/CMakeLists.txt e7130db 
  src/scriptengines/qml/plasmoid/appletinterface.cpp 07ecb3d 
  src/scriptengines/qml/plasmoid/containmentinterface.cpp c6986f2 
  src/scriptengines/qml/plasmoid/declarativeappletscript.cpp 24c39dd 
  src/scriptengines/qml/plasmoid/wallpaperinterface.cpp a406d45 

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


Testing
---

still needs a lot of testing, so far, with a master plasma-workspace notmal 
workspace functions seems fine

latest version still installs metadata files in kservices5. I would keep this 
for safety until Plasma 5.4 is released, then try to remove it again


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 123626: port libplasma away from sycoca as much as possible

2015-05-07 Thread Sebastian Kügler

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

Ship it!


Looks good.


src/declarativeimports/plasmacomponents/qml/private/TabBarLayout.qml (line 197)
https://git.reviewboard.kde.org/r/123626/#comment54913

This line may have slipped in?


- Sebastian Kügler


On May 7, 2015, 9:06 a.m., Marco Martin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123626/
 ---
 
 (Updated May 7, 2015, 9:06 a.m.)
 
 
 Review request for KDE Frameworks and Plasma.
 
 
 Repository: plasma-framework
 
 
 Description
 ---
 
 this ports most of libplasma away from sycoca, using instead a combination of 
 KPluginLoader and KPackage::PackageLoader instead (so eventually using their 
 own little caches instead of the global sycoca cache)
 a kservicetypetrader call is left in the loading of containmentactions since 
 is the only way to make an older workspace still work, but is only a 
 fallback, so containmentactions in plasma-workspace can be ported eventually 
 as well
 
 
 Diffs
 -
 
   KF5PlasmaConfig.cmake.in dee79ca 
   src/declarativeimports/plasmacomponents/qml/private/TabBarLayout.qml 
 be76a43 
   src/plasma/applet.cpp 2dda381 
   src/plasma/containment.cpp 7eabbb8 
   src/plasma/containmentactions.cpp f24bdac 
   src/plasma/dataengine.cpp 4b3d671 
   src/plasma/package.cpp 4e0be88 
   src/plasma/packagestructure.cpp b2686b6 
   src/plasma/pluginloader.cpp 0ce349a 
   src/plasma/private/applet_p.cpp 2cbfd06 
   src/plasma/private/dataenginemanager.cpp b9c5d8e 
   src/plasma/scripting/appletscript.h 863d707 
   src/plasma/scripting/dataenginescript.h f7ad3c2 
   src/plasma/scripting/scriptengine.cpp cc407e7 
   src/plasma/service.h b5d9b5a 
   src/plasma/service.cpp 3e9d852 
   src/plasmapkg/plasmapkg.cpp d606365 
   src/scriptengines/CMakeLists.txt f566406 
   src/scriptengines/qml/CMakeLists.txt e7130db 
   src/scriptengines/qml/plasmoid/appletinterface.cpp 07ecb3d 
   src/scriptengines/qml/plasmoid/containmentinterface.cpp c6986f2 
   src/scriptengines/qml/plasmoid/declarativeappletscript.cpp 24c39dd 
   src/scriptengines/qml/plasmoid/wallpaperinterface.cpp a406d45 
 
 Diff: https://git.reviewboard.kde.org/r/123626/diff/
 
 
 Testing
 ---
 
 still needs a lot of testing, so far, with a master plasma-workspace notmal 
 workspace functions seems fine
 
 latest version still installs metadata files in kservices5. I would keep this 
 for safety until Plasma 5.4 is released, then try to remove it again
 
 
 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 123669: Add KPluginLoader::findPluginsById convenience API

2015-05-07 Thread Sebastian Kügler


 On May 6, 2015, 11:24 p.m., David Edmundson wrote:
  autotests/kpluginloadertest.cpp, line 357
  https://git.reviewboard.kde.org/r/123669/diff/1/?file=367268#file367268line357
 
  not that it really matters, but invalid is spelt wrong.
  Double the invalidity.

it says invalid id, without the space. :D


- Sebastian


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


On May 6, 2015, 11:21 p.m., Sebastian Kügler wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123669/
 ---
 
 (Updated May 6, 2015, 11:21 p.m.)
 
 
 Review request for KDE Frameworks, Alex Richardson and David Faure.
 
 
 Repository: kcoreaddons
 
 
 Description
 ---
 
 Add findPluginsById convenience API
 
 It's a quite common case to load a plugin from an ID. This makes it
 easy.
 
 CHANGELOG:New KPluginLoader::findPluginById() convenience API
 REVIEW:123669
 
 
 Diffs
 -
 
   autotests/kpluginloadertest.cpp 3ded0ebca2e0fd20e09bf6e4eca152d13ac11f46 
   src/lib/plugin/kpluginloader.h 124bfab7e207b17d3c8ab4d5a88321af476aad42 
   src/lib/plugin/kpluginloader.cpp 4310d6cd7792329511f12b28d7c68b0fdd013538 
 
 Diff: https://git.reviewboard.kde.org/r/123669/diff/
 
 
 Testing
 ---
 
 Added autotests, everything passes.
 
 
 Thanks,
 
 Sebastian Kügler
 


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


Re: Review Request 123669: Add KPluginLoader::findPluginsById convenience API

2015-05-07 Thread Sebastian Kügler


 On May 7, 2015, 7:24 a.m., Alex Richardson wrote:
  Seems like this is duplicated in a few places already so I agree we should 
  add it. But won't most users of the API want only a single plugin returned?
  Maybe also add a function `KPluginMetaData 
  KPluginLoader::findPluginById(const QString directory, const QString 
  pluginId)`?
  Do we also need the function that returns a vector for a given ID?

At least our changes in libplasma need a QVectorKPluginMetaData. Otherwise, a 
list seems easy enough to check if something's found. Returning a single 
metadata won't be very useful for us at this point (but I see it making sense).


 On May 7, 2015, 7:24 a.m., Alex Richardson wrote:
  src/lib/plugin/kpluginloader.cpp, line 278
  https://git.reviewboard.kde.org/r/123669/diff/1/?file=367270#file367270line278
 
  QVector KPluginMetaData   - QVectorKPluginMetaData

I'll forego submitting a patch with just two spaces removed. Fixed it locally.


- Sebastian


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


On May 6, 2015, 11:21 p.m., Sebastian Kügler wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/123669/
 ---
 
 (Updated May 6, 2015, 11:21 p.m.)
 
 
 Review request for KDE Frameworks, Alex Richardson and David Faure.
 
 
 Repository: kcoreaddons
 
 
 Description
 ---
 
 Add findPluginsById convenience API
 
 It's a quite common case to load a plugin from an ID. This makes it
 easy.
 
 CHANGELOG:New KPluginLoader::findPluginById() convenience API
 REVIEW:123669
 
 
 Diffs
 -
 
   autotests/kpluginloadertest.cpp 3ded0ebca2e0fd20e09bf6e4eca152d13ac11f46 
   src/lib/plugin/kpluginloader.h 124bfab7e207b17d3c8ab4d5a88321af476aad42 
   src/lib/plugin/kpluginloader.cpp 4310d6cd7792329511f12b28d7c68b0fdd013538 
 
 Diff: https://git.reviewboard.kde.org/r/123669/diff/
 
 
 Testing
 ---
 
 Added autotests, everything passes.
 
 
 Thanks,
 
 Sebastian Kügler
 


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