KConfigXT Flaws

Waldo Bastian bastian at kde.org
Sun Jan 25 14:47:27 GMT 2004

Hash: SHA1

On Sun January 25 2004 15:44, Frerich Raabe wrote:
> Moin,
> I've recently been working a lot with the new KConfig XT framework and
> noticed a few flaws, some of which are rather grave:
> - KConfigDialog crashes if 0 is passed for the "const char *name"
> parameter; this is because the KConfigDialog constructor attempts to insert
> the null pointer into a dictionary.
>   I suggest to generate a generic name if no name is specified - something
>   like this:
>   if(name) {
>     openDialogs.insert(name, this);
>   } else {
>     QCString genericName("SettingsDialog");
>     genericName += QCString().setNum(openDialogs.count());
>     openDialogs.insert(genericName, this);
>     // So that one can use QObject::name() to get the generated name (which
>     // could then used for KConfigDialog::exists(), for instance)
>     setName(genericName);
>   }

Good point, but rather user something like:
	QCString genericName;
	genericName.sprintf("SettingsDialog-%p", this);

> - setMainWidget calls do not make any sense for KConfigDialog and will
>   cause broken dialogs at runtime; this is partly a flaw in KDialogBase
> because setMainWidget is not virtual, so KConfigDialog cannot reimplement
> it. I do not know how to fix this without breaking binary compatibility,
> but a note should be added to the KConfigDialog documentation, something
> along the lines of "Please note that using the setMainWidget method
> inherited from KDialogBase yields broken behaviour as runtime, use addPage
> instead".

I gues so.

> - It is not possible to dictate KConfigDialog not to use any default button
> at all. This is annoying when you have e.g. a KLineEdit in the dialog:
> entering a text there and then pressing return immediately triggers the
> default button (which is often "Ok", which in turn closes the dialog). A
> workaround is to use cfgDlg.actionButton( Foo )->setDefault( false ), but
> that looks clumsy. This flaw is also inherited from KDialogBase.
>   I suggest adding a new value to the KDialogBase::ButtonCode enumeration
> called "None". This value would get ignored in situations other than
> specifying a default value. I attached a patch which demonstrates this.

Don't see a patch

>   IMHO a good (read: transparent to clients) fix would be to add a
> protected non-virtual method
>   "const KJanusWidget *janusWidget() const { return mJanus; }"
>   to KDialogBase. Then, one can use that method in the KConfigDialog
>   constructor to disconnect the aboutToShowPage signals (right now the
> signal is directly forwarded from KJanusWidget to KDialogBase), and instead
> forward it to a private slot. That slot then maps the widget emitted by the
> janus widget back to the widget which was passed to addPage() and finally
> emits the aboutToShowPage signal again.

It think it would be easier if KJanusWidget would make it possible to
insert our own widget as page. Something like
KJanusWidget::insertPage(QWidget *widget), that does a reparent to
KJanusWidget::FindParent() and then calls addPageWidget()

That way KConfigDialog doesn't need to reparent, and doesn't need to add a
layout and the aboutToShowPage signal is the right one.

> - kconfig_compiler assumes "Mutators=false" as the default; this does not
> seem to make much sense, I'd say the point of a configuration facility is
> to make it possible to *change* values. IMHO mutators should be enabled by
> default.

In 99% of the cases you don't need the mutators because mutation happens via 
KConfigDialog which uses setProperty() calls. No need to generate code that 
isn't used.

> - kconfig_compiler expects the name of the XML file listing the
> configuration items on the commandline. In addition, the .kcfgc file
> containing the code generation options has a "File=" key which is supposed
> to enlist the XML file as well - this seems redundant and confusing to me
> (in fact, it seems that the File= entry in the  .kcfgc file is currently
> ignored). IMHO only the File= entry in the .kcfgc file should be honoured
> by kconfig_compiler, which then would take only one argument.

It is passed via the commandline because that way the build system can do its 
magic to point to the correct file. 

- -- 
bastian at kde.org -=|[ KDE: K Desktop for the Enterprise ]|=- bastian at suse.com
Version: GnuPG v1.2.2 (GNU/Linux)


More information about the kde-core-devel mailing list