What to do about KShortcutsEditor and undo

[Michael Jansen]

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 
propose afaict.

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 
correctly.

Mike

-- 
Michael Jansen
Available for contract work ( Development / Configuration Management )
http://www.michael-jansen.biz