[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    RFC: Merge branch new_kcm_code into HEAD
From:       Frans Englich <frans.englich () telia ! com>
Date:       2004-11-26 1:29:07
Message-ID: 200411260129.07220.frans.englich () telia ! com
[Download RAW message or body]

Hello all,

The "KCM technologies" were originally invented for KControl only, but has 
since then slowly grown towards being generic: modules in 
configuration dialogs, for example. That concept -- modules are atomic units 
which goes anywhere -- should now be lived up to fully, and the classic KDE 
integration pushed one step further.

--- Detailed Description ---

Previously, the KCModuleProxy class(in kutils) was a lightweight helper class, 
while KControl and kcmshell implemented their own powerfull classes. Now, 
because of the KCM technology being generic, the flexibility only KControl 
had, is useful on a wider basis. Since these two sets of classes tried to do 
the same thing more or less, but did it in different ways, it led to trouble 
and duplication. 

KCModuleProxy is now the one and only wrapper class, and takes care of all 
interals(with steady help from KCModuleLoader). These are the advantages this 
refactoring, combined with extensions, brings:

* Root modules can be loaded anywhere: Be it kcmshell, configuration dialogs 
or any other place. K3b can now integrate its "Setup 2" module into its 
ordinary configuration dialog, for example.

* Module uniqueness is ensured everywhere. If a module is loaded in an 
application, and then tried to be loaded anywhere else, the loading is 
aborted, the user informed, and the module is automatically loaded when it is 
unloaded in the other application. IIRC, a Kontact crash fixed by this.

* Delayed module loading have in some cases been improved(performance).

* An improved error reporting API by Matthias Kretz and I, which makes coding 
easier, and also results in a better user interface.

* Usability/GUI improvements, especially surrounding root mode

* Currently, a common problem with modules/configuration dialogs is they 
expand outside the viewing area of smaller screens. This is a cause of faulty 
designed dialogs, since they violate the HIG.[1] In either case, the new 
proxy class workarounds this in a bearable way by wrapping modules in a 
QScrollView(see BR #72112, #89940). If a configuration dialog have scrollbars 
after merging, it's because it violates the HIG(the new code only hack around 
it). If that's the case, it is an excellent time to find out what options 
that can be removed by selecting good defaults, clever coding, and conscious 
user targeting.


--- New Features ---

One new feature is Conditional Module Loading(CML, buzz buzz), which allows 
modules to themselves probe, and report back, whether they should be shown. 
For example, on my system(in my KControl) the Joystick module doesn't show up 
because I don't have a joystick connected. This is implemented in 
the various helper classes, and is hence available everywhere.

Practically, it is achieved by a boolean desktop file directive which tells 
whether the module should be tested, and (eventually) a boolean test 
function, similar to create_, whose return value tells if the module should 
be loaded. 

CML will help keep the module count lower and the modules relevant, but it is 
also a great way to mess up an interface. All relevant modules support CML(in 
HEAD), except the laptop modules which fundamentally requires a refactoring, 
although changes have happened lately.

Another new feature is the utility class KCModuleContainer, which is common 
code centralized, as well as a mechanism for organizing modules. But again, 
also a formidable way to create havoc when not used properly. As constructor 
arguments it takes a list of module names, which it then creates a module of, 
which has a tab widget containing the specified modules. 

KCModuleContainer is a KCModule which wraps real modules -- it's a 
"meta-module". It does this in a complete way: What's This text is handled, 
icons on tabs, efficient saving, etc. KCModuleContainer is as generic as 
KCModuleProxy; it can mix root modules with regulars, its test function tests 
the contained modules, etc. 

It is accompanied by a macro which takes care of everything, such as library 
declaration. For example, this creates the "Peripherals/Display" module with 
library name "display":
KCMODULECONTAINER("randr, xinerama, power, kgamma", display);

--

The features and other advantages are supported in all module wrappers, such 
as kcmshell, KCMultiDialog, and KCModuleContainer -- except KControl. E.g, 
modules won't be conditionally loaded in current KControl even if the modules 
support it. In fact, none of the changes affects KControl. kcmshell is 
backwards compatible(behavior wise, for example).

KControl with its 5500 LOC is a sinking ship, and no matter what needs to be 
replaced in KDE 4 to properly adapt to the changes which have happened over 
time. KDE 4 will be the release where these features and advantages are 
widely used. 

I want the code merged to ease maintenance, have the code available for 
distributors and 3rd parties, and the bugs/fixes it brings to KCMultiDialog. 

All in all, have the lines of code count dropped by a couple of thousands(when 
including an replacement of the current KControl), and that should rise 
further as code is upgraded. Writing KControl prototypes should also be 
simpler, easier, and allowing to focus on other issues. For example, I have a 
KControl, feature complete, in 400 LOC. 

The branch is simply a refactoring which makes functionality widely available, 
combined with extensions.


--- Code condition & future changes ---

The majority of code haven't been touched for months, and it feels 
reasonably stable. As usual, it's difficult to do proper testing, so I 
wouldn't be surprised if regressions pops up. Some of the things KCModuleProxy 
does is neither exactly straight forward; while a large goal with the branch 
was to simplify and compact code, it still looks complex, although I wonder 
if the code by its nature can be done simple in a snap..

An API cleanup is of course needed(KDE 4). Many modules needs fresh ups to 
take advantage of the API changes in KCModule(among other things), but it 
would be wise to wait with that until KDE 4 is in development.

--- Concerns for Merging ---

* Touched code is all KCM classes in kdelibs/kutils which have either been 
extended or adapted to the new code, and a more or less new kcmshell in 
kdelibs/kcmshell(disentangled from KControl). 

* The code uses kdesu for loading root modules, and it currently resides in 
kdebase. Moving it is no technical problem(AFAICT) and would probably be 
helpful in other cases. This was discussed on kde-core-devel a long time ago 
and agreed upon was sensible, but I had to go offline and the move was never 
done. (And no, kdelibs is not only for libraries.) 

* Security implications: From that perspective the situation is not changed 
AFAICT; kdesu and the root modules are what is of concern. Those are the one 
to audit, and eventually the KCModuleProxy class(which sits inbetween).

* Binary Compatibility: The interfaces are heavily extended, so a watchful 
eye on BC is necessary. _AFAICT_ BC haven't been broken. 
I've attached a diff of kcmoduleproxy.h against HEAD, in case anyone wants to 
review. If I were you, I would review.

I am interested in objections for merging the new_kcm_code branch and moving 
kdesu. Questions and suggestions are also appreciated.


		Frans


1.
http://usability.kde.org/hig/current/windows-size.php

["kcmoduleproxy.h.diff" (text/x-diff)]

Index: kcmoduleproxy.h
===================================================================
RCS file: /home/kde/kdelibs/kutils/kcmoduleproxy.h,v
retrieving revision 1.1
retrieving revision 1.2.2.5
diff -u -3 -p -r1.1 -r1.2.2.5
--- kcmoduleproxy.h	27 Sep 2003 10:14:30 -0000	1.1
+++ kcmoduleproxy.h	25 Nov 2004 20:48:09 -0000	1.2.2.5
@@ -1,5 +1,6 @@
 /*  This file is part of the KDE project
     Copyright (C) 2003 Matthias Kretz <kretz@kde.org>
+    Copyright (C) 2004 Frans Englich <frans.englich@telia.com>
 
     This library is free software; you can redistribute it and/or
     modify it under the terms of the GNU Library General Public
@@ -23,45 +24,315 @@
 #include <qwidget.h>
 #include <qstringlist.h>
 
-class KCModuleInfo;
 class KAboutData;
-class KInstance;
 class KCModule;
+class KCModuleInfo;
+class KInstance;
+class KProcess;
+#include <kservice.h>
 
-class KCModuleProxy : public QWidget
+/**
+ *
+ * @brief Encapsulates a @ref KCModule for embedding.
+ *
+ * @description @ref KCModuleProxy is a wrapper for KCModule intended for cases \
where  + * modules are to be displayed. It ensures layout is consistent, handles 
+ * root/administrator modules and in general takes care of the details 
+ * needed for making a module available in an interface. A KCModuleProxy 
+ * can be treated as a QWidget, without worrying about the details specific 
+ * for modules such as library loading. KCModuleProxy is not a sub class of KCModule \
 + * but its API closely resembles KCModule's.\n
+ * Usually, an instance is created by passing one of the constructors a @ref \
KService::Ptr,  + * @ref KCModuleInfo or simply the name of the module and then added \
to the layout as any  + * other widget. \n
+ * When the user have changed the module, @ref changed( bool ) as well as @ref \
changed ( KCModuleProxy * ) + * is emitted. KCModuleProxy does not take care of \
prompting for saving - if the object is deleted while  + * changes is not saved the \
changes will be lost. @ref changed() returns true if changes are unsaved. \n + * \n
+ * KCModuleProxy does not take care of authorization of KCModules. \n
+ * KCModuleProxy do lazy loading, meaning the library will not be loaded or 
+ * any other initialization done before its show() function is called. This means 
+ * modules will only be loaded when they are actually needed as well as it is \
possible to  + * load many KCModuleProxy without any speed penalty.
+ *
+ * KCModuleProxy should be used in all cases where modules are embedded in order to 
+ * promote code efficiency and usability consistency.
+ * 
+ * @author Frans Englich <frans.englich@telia.com>
+ * @author Matthias Kretz <kretz@kde.org>
+ *
+ */
+class KUTILS_EXPORT KCModuleProxy : public QWidget
 {
-	Q_OBJECT
-	public:
-		KCModuleProxy( const KCModuleInfo & info, bool withfallback = false,
-				QWidget * parent = 0, const char * name = 0,
-				const QStringList & args = QStringList() );
-		~KCModuleProxy();
-		void load();
-		void save();
-		void defaults();
-		QString quickHelp() const;
-		const KAboutData * aboutData() const;
-		int buttons() const;
-		QString rootOnlyMsg() const;
-		bool useRootOnlyMsg() const;
-		KInstance * instance() const;
-		bool changed() const;
-		KCModule * realModule() const;
-
-	signals:
-		void changed( bool );
-
-	protected:
-		void showEvent( QShowEvent * );
-
-	private slots:
-		void moduleChanged( bool );
-		void moduleDestroyed();
-
-	private:
-		class KCModuleProxyPrivate;
-		KCModuleProxyPrivate * d;
+Q_OBJECT
+
+	friend class KCModuleProxyRootCommunicatorImpl;
+
+public:
+
+	/**
+	 * Constructs a KCModuleProxy from a KCModuleInfo class.
+	 *
+	 * @param info The KCModuleInfo to construct the module from.
+	 *
+	 * @param withfallback If set to true and loading of the module fails, 
+	 * a alternative will be tried, resulting in the module appearing in its 
+	 * own window, if at all.
+	 * The embedded module will be load()ed.
+	 *
+	 * @param args This is used in the implementation and is internal. Use the 
+	 * default.
+	 */
+	KCModuleProxy( const KCModuleInfo & info, bool withFallback = true,
+			QWidget * parent = 0, const char * name = 0,
+			const QStringList & args = QStringList() );
+
+	/**
+	 * Constructs a KCModuleProxy from a module's service name, which is 
+	 * equivalent to the desktop file for the kcm without the ".desktop" part. 
+	 * Otherwise equal to the one above.
+	 *
+	 * @param serviceName The module's service name to construct from.
+	 */
+	KCModuleProxy( const QString& serviceName, bool withFallback = true, 
+			QWidget * parent = 0, const char * name = 0,
+			const QStringList & args = QStringList() );
+			
+	/**
+	 * Constructs a KCModuleProxy from KService. Otherwise equal to the one above.
+	 *
+	 * @param service The KService to construct from.
+	 */
+	KCModuleProxy( const KService::Ptr& service, bool withFallback = true, 
+			QWidget  * parent = 0, const char * name = 0,
+			const QStringList & args = QStringList() );
+
+	/**
+	 * Default destructor
+	 */
+	~KCModuleProxy();
+
+	/**
+	 * Calling it will cause the contained module to 
+	 * run its load() routine.
+	 */
+	void load();
+
+	/**
+	 * Calling it will cause the contained module to 
+	 * run its save() routine.
+	 *
+	 * If the module was not modified, it will not be asked
+	 * to save.
+	 */
+	void save();
+
+	/**
+	 * @return the module's quickHelp();
+	 */
+	QString quickHelp() const;
+
+	/**
+	 * @return the module's aboutData()
+	 */
+	const KAboutData * aboutData() const;
+
+	/**
+	 * @return what buttons the module
+	 * needs
+	 */
+	int buttons() const;
+
+	/**
+	 * @return The module's custom root 
+	 * message, if it has one
+	 * @deprecated
+	 */
+	QString rootOnlyMsg() const;
+	//KDE4 remove. There's a limit for convenience functions, 
+	// this one's available via moduleInfo()-> and realModule()->
+
+	/**
+	 * @return If the module is a root module.
+	 * @deprecated
+	 */
+	bool useRootOnlyMsg() const;
+	//KDE4 remove. There's a limit for convenience functions, 
+	// this one's available via moduleInfo()-> and realModule()->
+
+	/**
+	 * Returns the embedded KCModule's KInstance.
+	 * @return The module's KInstance.
+	 * @deprecated
+	 */
+	KInstance * instance() const;
+	//KDE4 remove. There's a limit for convenience functions, 
+	// this one's available via realModule()
+
+	/**
+	 * @return true if the module is modified 
+	 * and needs to be saved.
+	 */
+	bool changed() const;
+
+	/**
+	 * Returns whether the module is running in root mode. A module is in root mode
+	 * when @ref runAsRoot() has been called. A session under root user will never \
reach  +	 * root mode.
+	 *
+	 * @note realModule() will return null when the module is running in root mode.
+	 *
+	 * @return true if the module is running with root privileges
+	 * @since 3.4
+	 */
+	bool rootMode() const;
+
+	/**
+	 * Access to the actual module. However, if the module is 
+	 * running in root mode, see @ref rootMode(), this function returns 
+	 * a NULL pointer, since the module is in another process. It may also 
+	 * return NULL if anything goes wrong.
+	 *
+	 * @return the encapsulated module. 
+	 */
+	KCModule* realModule() const;
+
+	/**
+	 * @return a KCModuleInfo for the encapsulated
+	 * module
+	 */
+	const KCModuleInfo& moduleInfo() const;
+
+	/**
+	 * Returns the DCOP the module's @ref DCOPClient 
+	 * and @ref DCOPObject has(they are identical).
+	 *
+	 * @since 3.4
+	 */
+	QCString dcopName() const;
+
+public slots:
+	
+	/**
+	 * Calling this will cause the module to be run in 
+	 * "administrator mode".
+	 *
+	 * @since 3.4
+	 */
+	void runAsRoot();
+
+	/**
+	 * Calling it will cause the contained module to 
+	 * load its default values.
+	 */
+	void defaults();
+	
+	/**
+	 * Calling this, results in deleting the contained 
+	 * module, and unregistering from DCOP. A similar result is achieved
+	 * by deleting the KCModuleProxy itself.
+	 *
+	 * @since 3.4
+	 */
+	void deleteClient();
+
+signals:
+
+	/*
+	 * This signal is emitted when the contained module is changed.
+	 */
+	void changed( bool state );
+	
+	/**
+	 * This is emitted in the same situations as in the one above. Practical 
+	 * when several KCModuleProxys are loaded.
+	 *
+	 * @since 3.4
+	 */
+	void changed( KCModuleProxy* mod );
+
+	/**
+	 * When a module running with root privileges and exits, returns to normal mode, \
the  +	 * childClosed() signal is emitted.
+	 *
+	 * @since 3.4
+	 */
+	void childClosed();
+
+	/*
+	 * This signal is relayed from the encapsulated module, and 
+	 * is equivalent to the module's own quickHelpChanged() signal.
+	 *
+	 * @since 3.4
+	 */
+	void quickHelpChanged();
+
+protected:
+
+	/**
+	 * Reimplemented for internal purposes. Makes sure the encapsulated 
+	 * module is loaded before the show event is taken care of.
+	 */
+	void showEvent( QShowEvent * );
+
+	/**
+	 * Internal intialization function, called by the constructors.
+	 *
+	 * @internal
+	 * @since 3.4
+	 */
+	void init( const KCModuleInfo& info );
+
+
+	/**
+	 * Emits the quickHelpChanged signal.
+	 * @since 3.4
+	 */
+	void emitQuickHelpChanged();
+
+private slots:
+
+   /**
+	* Calls the function @p function of the root module's KCModuleProxy 
+	* DCOP interface.
+	* 
+	* @param function the function signature of the function to call.
+	* @since 3.4
+	*/
+	void callRootModule( const QCString& function );
+
+	/**
+	 * This is called when the module exits from root mode. It zeroes 
+	 * pointers, deletes the embed window, and so forth.
+	 *
+	 * @since 3.4
+	 */
+	void rootExited();
+
+	/**
+	 * Makes sure the proper variables is set and signals are emitted.
+	 */
+	void moduleChanged( bool );
+
+	/**
+	 * Zeroes d->kcm
+	 */
+	void moduleDestroyed();
+
+	/**
+	 * Gets called by DCOP when an application closes.
+	 * Is used to (try to) reload a KCM which previously 
+	 * was loaded.
+	 *
+	 * @since 3.4
+	 */
+	void applicationRemoved( const QCString& app );
+
+private:
+	
+	class KCModuleProxyPrivate;
+	KCModuleProxyPrivate * d;
 };
 
-// vim: sw=4 ts=4 noet
 #endif // KCMODULEPROXY_H
+// vim: sw=4 ts=4 noet



[prev in list] [next in list] [prev in thread] [next in thread]