[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: A KSelectAction dedicated to QTextCodec selection ?
From:       David Faure <faure () kde ! org>
Date:       2006-10-20 17:18:37
Message-ID: 200610201918.38357.faure () kde ! org
[Download RAW message or body]

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


[prev in list] [next in list] [prev in thread] [next in thread]