Hello!

On Friday, October 20, 2006 01:19:05 PM Michel Hermier wrote:
> I noticed while preparing the changes locally, that I need one of my
> local change for KSelectAction (well only a return change, but the
> patch adds a little more).
> So I delay the commit for now.
> 
> Can someone maintaining KSelectAction review the patch.
> What changed with this patch:
> - bool setCurrentAction(QAction *, DeselectionMode mode) should be
> more safe, checking that the action really belongs to the action group
> before activating the action. Also added an extra parameter with
> default value to mimic the old behaviour. This extra parameter allow
> to not deselect the previous action in case of falure to select the
> action.

Why does this need to be configurable? Deselection the previous action
and not selecting any new one instead looks like a bug, not a feature.
Did you make it configurable "just in case", or is there a real use case for this?

We need "set this one as current" and "set none as current".
Why do we need "setCurrentAction(0,LetSelectedMode)" which does -nothing-
and setCurrentAction(unrelatedAction,DeselectMode) which has the effect of unchecking the current action (!)
and setCurrentAction(unrelatedAction,LetSelectedMode) which, well, does nothing too.
Either I'm missing something, or this would be a very confusing API.

setCurrentAction(unrelatedAction) should do nothing IMHO, and that's it.

-- 
David Faure, faure@kde.org, sponsored by Trolltech to work on KDE,
Konqueror (http://www.konqueror.org), and KOffice (http://www.koffice.org).