KConfigXT Flaws

[Waldo Bastian]

-----BEGIN PGP SIGNED MESSAGE-----
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. 

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

iD8DBQFAE9b/N4pvrENfboIRAuC9AJ9Q50C4oYQ4DyqlEhWawGL+OpCBOgCdHuz/
cxX153fAZecGfn83nl6YqIA=
=Q4cZ
-----END PGP SIGNATURE-----