A KSelectAction dedicated to QTextCodec selection ?

[Michel Hermier]

2006/10/21, Hamish Rodda <rodda at kde.org>:
> On Saturday 21 October 2006 05:36, Michel Hermier wrote:
> > Hi,
> >
> > 2006/10/20, David Faure <faure at kde.org>:
> > > 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.
>
> No, unselecting the current action is needed in the case where we want to
> clear the current selection.  For example, in KRecentFilesAction, when a file
> is closed, its action is deselected so it is no longer checked.

I meant, "there migth be some usage to if I can't set to the current
action, let the selection as before", especially when trying to set
the current action using a text research.

> > > Did you make it configurable "just in case", or is there a real use case
> > > for this?
> >
> > I think it's needed to be configurable if we allow to have multiple
> > selection enabled.
> > Because in this case you have to look at all the actions to search to
> > know wich ones are enabled, since the API don't offer to get all the
> > selected items (directly).
>
> KSelectAction is not designed to allow multiple selection.

Are there any anoying reason, we can't make it multiple selection aware ?

> > And yes there is a real need to allow multiple selection, since
> > usually these groups are quite large (see the font and QTextCodec). So
> > we need an option to configure the removal some of them, so we need to
> > select multiple items.
>
> Either have a separate way to configure the action (eg. configuration dialog),
> or create a new class which does what you want.

Yes, but why not simple use the actions we allready have for these ?

> As to the earlier patch, I think the part which checks that the new current
> action is one of the selectable actions is worthwhile.
>
> Cheers,
> Hamish.

Michel