What to do about KShortcutsEditor and undo
kde at michael-jansen.biz
Tue May 20 20:29:13 BST 2008
On Tuesday 20 May 2008 02:49:59 Andreas Hartmetz wrote:
> Hi all,
> -Add setUndoCheckpoint() to KShortcutsEditor so that KControl modules can
> call it when asked to save configuration. Calling KShortcutsEditor::undo()
> will then only undo to the state at the checkpoint.
@see KShortcutsEditor::save() and KShortcutsEditor::undChanges and tell me
what additional functionality your methods provide. They do exactly what you
What is the correct behaviour for the dialog upon destruction when there are
unsaved changes? /me thinks undoing the changes. That's what i implemented.
> -Remove automatic undo in KShortcutsEditor on destruction, it's not a very
> good idea if only for the reason that it crashes if the actions are
> destroyed before the editor. This actually happens in the KWin effects
> config modules.
If you provide a something (the collection) to another object (the editor)
it's your responsibilty to ensure it's valid as long as the object lives. If
you destroy the something as long as the object lives we enter the stage of
undefined behaviour. So i see no problem here. The problem is the way the kwin
effects use the kshortcutsdialog.
> KControl modules should call undo() on their destruction
> when the actions are still alive.
If they would do that now ... no core dump. But that's just luck. They still
destroy an object given to another object to work on without making sure they
get destroyed in the right order.
> -Remove (if no protests, otherwise deprecate)
> KGlobalAccel::overrideComponentData(). It was conceived as a special-case
> hack for KControl modules (which configure actions that they don't "own");
> KActionCollection::setComponentData() can replace it and it will be more
> consistent and generally applicable. Credit to mjansen for pointing this
> out. overrideComponentData() is only used in kdelibs and kdebase, according
> to lxr.kde.org.
That has nothing to do with the problem at hand. But i'm all in for removing
it. Btw. i told you if we can agree on the patch i send to you i would fix the
kwin effects. There's nothing else needed. The rest is just to use the api
Available for contract work ( Development / Configuration Management )
More information about the kde-core-devel