Re: Applet::load and friends ... or PluginLoader::load..

2011-07-15 Thread Marco Martin
On Friday 15 July 2011, Aaron J. Seigo wrote:
> 
> anyways.. the thing i'm contemplating is this:
> 
> Removing all the "load(..)" methods from the classes, and instead have
> everything use PluginManager directly.

+1

> pros:
>   * fewer methods in general in the API, particularly ones that plugin
> writers don't care about
>   * it follows the pattern now done with Package which handles all package
> loading instead of per-class API for that (in fact, it goes one better:
> Package::load would also disappeaer)
>   * consistent way for loading plugins: use PluginLoader
> 
> cons:
>   * code like Applet::load becomes PluginLoader::self()->loadApplet. in
> other words, rather more verbose. i could change PluginLoader to instead
> of "having loadApplet" or "loadDataEngine" just "applet" and "dataEngine"

i don't see it *so* a con:
- it's very clear what it does
- it's not exactly code that will be used all over the place, mostly in 
libplasma itself and in the shells, "normal" users of libplasma (plasmoids, 
dataengines etc) would barely see this, with the possible exception of who 
uses the kpart, but even then they would have this code only in a couple of 
places

Cheers,
Marco Martin
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Applet::load and friends ... or PluginLoader::load..

2011-07-15 Thread Ryan Rix
Hi+1, that all makes sense to me, and sounds like a Good Thing.R--Ryan Rix http://rix.Sihttp://opensource.com : Where Open Source multipliesOn Jul 15, 2011 4:27, Aaron J. Seigo  wrote: hi...

so, we have the PluginLoader class now which is there to allow extending the 
loading of plugins by the application.

there are methods like: PluginLoader::loadApplet and PluginLoader::loadPackage

these are generally only used by the internals of classes like Applet::load, 
which is currently this:

Applet *Applet::load(const QString &appletName, uint appletId, const 
QVariantList &args)
{
return PluginLoader::pluginLoader()->loadApplet(appletName, appletId, 
args);
}

not much to it, is there? :)

this is the same for AbstractRunner::load, Package::load, and on and on ..

the one exception to this pattern is DataEngineManager, which does some 
internal bookkeeping around the PluginLoader method. that may be going away, 
however, in libplasma2 if we manage to get reference counted implicitly shared 
DataEngines.

anyways.. the thing i'm contemplating is this:

Removing all the "load(..)" methods from the classes, and instead have 
everything use PluginManager directly.

pros:
  * fewer methods in general in the API, particularly ones that plugin writers 
don't care about
  * it follows the pattern now done with Package which handles all package 
loading instead of per-class API for that (in fact, it goes one better: 
Package::load would also disappeaer)
  * consistent way for loading plugins: use PluginLoader

cons:
  * code like Applet::load becomes PluginLoader::self()->loadApplet. in other 
words, rather more verbose. i could change PluginLoader to instead of "having 
loadApplet" or "loadDataEngine" just "applet" and "dataEngine"

thoughts?

-- 
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43

KDE core developer sponsored by Qt Development Frameworks
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: Move screensaver and locking functionality to kwin from krunner

2011-07-15 Thread Martin Gräßlin

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101943/#review4741
---


The code from the SoK project is now in an own branch: 
https://projects.kde.org/projects/kde/kdebase/kde-workspace/repository/revisions/2780fd91810bad353ac33422ce4b3eab291c4b47

Comparing the two I prefer the SoK commit as it doesn't add files to kwin's 
root directory (which I don't like any more and get's currently more and more 
changed)

- Martin


On July 13, 2011, 4:07 p.m., Alex Merry wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101943/
> ---
> 
> (Updated July 13, 2011, 4:07 p.m.)
> 
> 
> Review request for kwin, Plasma, Aaron J. Seigo, Martin Gräßlin, and Farhad 
> Hedayati Fard.
> 
> 
> Summary
> ---
> 
> This transfers the screen locking and screensaver funcitonality wholesale 
> from krunner to kwin.  This has been OK'd in principle by the relevant 
> maintainers.
> 
> Most of the code is simply moved untouched.  Points of note:
> 
> I've introduced a KWIN_BUILD_SCREENSAVER option, to match the other kwin 
> build options, like KWIN_BUILD_TABBOX.  I disabled it for Plasma active, but 
> that may not be appropriate.
> 
> I put the screensaver stuff (creating the SaverEngine object and setting up 
> the shortcut) in KWin::Workspace.  The shortcut stuff is actually in 
> useractions.cpp, but this implements methods in KWin::Workspace.
> 
> I'm using the kglobalaccel D-Bus interface directly to steal the existing 
> shortcut from KRunner.  I'm doing it like this because the 
> KAction/KGlobalAccel interface is not rich enough to do this (probably wisely 
> - this isn't something apps should generally be doing).  The shortcut 
> deregistration happens in KWin rather than KRunner because I don't want to 
> rely on the order in which KWin and KRunner are started (or even on KRunner 
> being started at all).
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 89d97cd 
>   kcontrol/screensaver/CMakeLists.txt e4dcc3a 
>   krunner/CMakeLists.txt 4455847 
>   krunner/README c8b9740 
>   krunner/config-xautolock.h.cmake eadb0a6 
>   krunner/dbus/org.freedesktop.ScreenSaver.xml 5efd943 
>   krunner/dbus/org.kde.screensaver.xml e700b88 
>   krunner/kcfg/kscreensaversettings.kcfg c8f76f3 
>   krunner/kcfg/kscreensaversettings.kcfgc af9133d 
>   krunner/krunnerapp.h 82db725 
>   krunner/krunnerapp.cpp c143be5 
>   krunner/lock/CMakeLists.txt cf9a67e 
>   krunner/lock/autologout.h 0c44405 
>   krunner/lock/autologout.cc c86e29a 
>   krunner/lock/config-krunner-lock.h.cmake 7bfdfd6 
>   krunner/lock/kscreenlocker.notifyrc 2955c5f 
>   krunner/lock/lockdlg.h f25e55f 
>   krunner/lock/lockdlg.cc 6367216 
>   krunner/lock/lockprocess.h 8b6d9a8 
>   krunner/lock/lockprocess.cc ecc632f 
>   krunner/lock/main.h 8a60353 
>   krunner/lock/main.cc 9f1c9b8 
>   krunner/main.cpp 84a547b 
>   krunner/screensaver/saverengine.h 3384d4a 
>   krunner/screensaver/saverengine.cpp 6c15be6 
>   krunner/screensaver/xautolock.h 3db3233 
>   krunner/screensaver/xautolock.cpp 7124215 
>   krunner/screensaver/xautolock_c.h 3b82f5c 
>   krunner/screensaver/xautolock_diy.c b9df2f8 
>   krunner/screensaver/xautolock_engine.c d6d0cf5 
>   kscreenlocker/CMakeLists.txt PRE-CREATION 
>   kscreenlocker/autologout.h PRE-CREATION 
>   kscreenlocker/autologout.cc PRE-CREATION 
>   kscreenlocker/config-kscreenlocker.h.cmake PRE-CREATION 
>   kscreenlocker/kscreenlocker.notifyrc PRE-CREATION 
>   kscreenlocker/lockdlg.h PRE-CREATION 
>   kscreenlocker/lockdlg.cc PRE-CREATION 
>   kscreenlocker/lockprocess.h PRE-CREATION 
>   kscreenlocker/lockprocess.cc PRE-CREATION 
>   kscreenlocker/main.h PRE-CREATION 
>   kscreenlocker/main.cc PRE-CREATION 
>   kwin/CMakeLists.txt 7d6ea52 
>   kwin/config-kwin.h.cmake a291859 
>   kwin/config-xautolock.h.cmake PRE-CREATION 
>   kwin/kscreensaversettings.kcfg PRE-CREATION 
>   kwin/kscreensaversettings.kcfgc PRE-CREATION 
>   kwin/screensaver/org.freedesktop.ScreenSaver.xml PRE-CREATION 
>   kwin/screensaver/org.kde.screensaver.xml PRE-CREATION 
>   kwin/screensaver/saverengine.h PRE-CREATION 
>   kwin/screensaver/saverengine.cpp PRE-CREATION 
>   kwin/screensaver/xautolock.h PRE-CREATION 
>   kwin/screensaver/xautolock.cpp PRE-CREATION 
>   kwin/screensaver/xautolock_c.h PRE-CREATION 
>   kwin/screensaver/xautolock_diy.c PRE-CREATION 
>   kwin/screensaver/xautolock_engine.c PRE-CREATION 
>   kwin/useractions.cpp 387e499 
>   kwin/workspace.h 66b9830 
>   kwin/workspace.cpp 8cf5299 
>   plasma/desktop/applets/kickoff/CMakeLists.txt bc5fa2e 
>   plasma/generic/applets/lock_logout/CMakeLists.txt 8381d46 
>   plasma/generic/containmentactions/contextmenu/CMakeLists.txt a1fc205 
>   plasma/generic/runners/sessions

Re: Applet::load and friends ... or PluginLoader::load..

2011-07-15 Thread Sebastian Kügler
On Friday, July 15, 2011 13:26:45 Aaron J. Seigo wrote:
> thoughts?

Having used this kind of API in Plasma a few times, I'd say the using 
PluginLoader directly would not be a huge burden. I wouldn't mind this change.
-- 
sebas

http://www.kde.org | http://vizZzion.org | GPG Key ID: 9119 0EF9
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Applet::load and friends ... or PluginLoader::load..

2011-07-15 Thread Aaron J. Seigo
hi...

so, we have the PluginLoader class now which is there to allow extending the 
loading of plugins by the application.

there are methods like: PluginLoader::loadApplet and PluginLoader::loadPackage

these are generally only used by the internals of classes like Applet::load, 
which is currently this:

Applet *Applet::load(const QString &appletName, uint appletId, const 
QVariantList &args)
{
return PluginLoader::pluginLoader()->loadApplet(appletName, appletId, 
args);
}

not much to it, is there? :)

this is the same for AbstractRunner::load, Package::load, and on and on ..

the one exception to this pattern is DataEngineManager, which does some 
internal bookkeeping around the PluginLoader method. that may be going away, 
however, in libplasma2 if we manage to get reference counted implicitly shared 
DataEngines.

anyways.. the thing i'm contemplating is this:

Removing all the "load(..)" methods from the classes, and instead have 
everything use PluginManager directly.

pros:
  * fewer methods in general in the API, particularly ones that plugin writers 
don't care about
  * it follows the pattern now done with Package which handles all package 
loading instead of per-class API for that (in fact, it goes one better: 
Package::load would also disappeaer)
  * consistent way for loading plugins: use PluginLoader

cons:
  * code like Applet::load becomes PluginLoader::self()->loadApplet. in other 
words, rather more verbose. i could change PluginLoader to instead of "having 
loadApplet" or "loadDataEngine" just "applet" and "dataEngine"

thoughts?

-- 
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43

KDE core developer sponsored by Qt Development Frameworks


signature.asc
Description: This is a digitally signed message part.
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel