------=_Part_87150_11777158.1161372965330
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

Hi,

2006/10/20, David Faure <faure@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.
> 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).

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.

> We need "set this one as current" and "set none as current".
> Why do we need "setCurrentAction(0,LetSelectedMode)" which does -nothing-

I know it's stupid, but it's necessary to allow setCurrentAction(fooaction);
Unless you propose to make the SelectionMode configured by a method,
it's still stupid to make
setSelectionMode(LetSelectedMode);
setCurrentAction(0);

> and setCurrentAction(unrelatedAction,DeselectMode) which has the effect of unchecking the current action (!)
> and setCurrentAction(unrelatedAction,LetSelectedMode) which, well, does nothing too.

Hmmm you are rigth I don't thougth about this, maybe now I can remove
these cases ;)

> Either I'm missing something, or this would be a very confusing API.
>
> setCurrentAction(unrelatedAction) should do nothing IMHO, and that's it.

Sure, here is the patch for this ;)

Maybe, I should also add some checks to check that the actions are
visible, enabled and checkable.

Michel

------=_Part_87150_11777158.1161372965330
Content-Type: application/octet-stream; name=kselectaction.diff
Content-Transfer-Encoding: base64
X-Attachment-Id: f_etizdxzg
Content-Disposition: attachment; filename="kselectaction.diff"

SW5kZXg6IGtzZWxlY3RhY3Rpb24uY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIGtzZWxlY3RhY3Rpb24uY3Bw
CShyZXZpc2lvbiA1OTc0NzkpCisrKyBrc2VsZWN0YWN0aW9uLmNwcAkod29ya2luZyBjb3B5KQpA
QCAtMjEzLDcgKzIxMyw3IEBACiAKIFFBY3Rpb24qIEtTZWxlY3RBY3Rpb246OmN1cnJlbnRBY3Rp
b24oKSBjb25zdAogewotICByZXR1cm4gZC0+bV9hY3Rpb25Hcm91cC0+Y2hlY2tlZEFjdGlvbigp
OworICByZXR1cm4gc2VsZWN0YWJsZUFjdGlvbkdyb3VwKCktPmNoZWNrZWRBY3Rpb24oKTsKIH0K
IAogaW50IEtTZWxlY3RBY3Rpb246OmN1cnJlbnRJdGVtKCkgY29uc3QKQEAgLTIyOSwzMSArMjI5
LDM2IEBACiAgIHJldHVybiBRU3RyaW5nKCk7CiB9CiAKLXZvaWQgS1NlbGVjdEFjdGlvbjo6c2V0
Q3VycmVudEFjdGlvbihRQWN0aW9uKiBhY3Rpb24pCitib29sIEtTZWxlY3RBY3Rpb246OnNldEN1
cnJlbnRBY3Rpb24oUUFjdGlvbiogYWN0aW9uLCBEZXNlbGVjdGlvbk1vZGUgbW9kZSkKIHsKLSAg
Ly9rRGVidWcgKDEyOSkgPDwgIktTZWxlY3RBY3Rpb246OnNldEN1cnJlbnRBY3Rpb24oIiA8PCBh
Y3Rpb24gPDwgIikiIDw8IGVuZGw7Ci0gIGlmICghYWN0aW9uKSB7CisgIC8va0RlYnVnICgxMjkp
IDw8ICJLU2VsZWN0QWN0aW9uOjpzZXRDdXJyZW50QWN0aW9uKCIgPDwgYWN0aW9uIDw8ICIsbW9k
ZT0iIDw8IG1vZGUgPDwgIikiIDw8IGVuZGw7CisgIGlmIChhY3Rpb24pIHsKKyAgICBpZiAoYWN0
aW9ucygpLmNvbnRhaW5zKGFjdGlvbikpIHsKKyAgICAgIGFjdGlvbi0+c2V0Q2hlY2tlZCh0cnVl
KTsKKyAgICAgIHJldHVybiB0cnVlOworICAgIH0gZWxzZSB7CisgICAgICBrV2FybmluZyAoMTI5
KSA8PCBrX2Z1bmNpbmZvIDw8ICJBY3Rpb24gZG9uJ3QgYmVsb25nIHRvIGdyb3VwOiIgPDwgYWN0
aW9uLT50ZXh0KCkgPDwgZW5kbDsKKyAgICAgIHJldHVybiBmYWxzZTsKKyAgICB9CisgIH0KKwor
ICBzd2l0Y2gobW9kZSkKKyAgeworICBjYXNlIERlc2VsZWN0TW9kZToKICAgICBpZiAoY3VycmVu
dEFjdGlvbigpKQogICAgICAgY3VycmVudEFjdGlvbigpLT5zZXRDaGVja2VkKGZhbHNlKTsKLQot
ICB9IGVsc2UgewotICAgIGFjdGlvbi0+c2V0Q2hlY2tlZCh0cnVlKTsKKyAgICBicmVhazsKKyAg
ZGVmYXVsdDoKKyAgICAvLyBEbyBub3RoaW5nCisgICAgYnJlYWs7CiAgIH0KKyAgcmV0dXJuIGZh
bHNlOwogfQogCi1ib29sIEtTZWxlY3RBY3Rpb246OnNldEN1cnJlbnRJdGVtKCBpbnQgaW5kZXgg
KQorYm9vbCBLU2VsZWN0QWN0aW9uOjpzZXRDdXJyZW50SXRlbSggaW50IGluZGV4LCBEZXNlbGVj
dGlvbk1vZGUgbW9kZSApCiB7Ci0gIC8va0RlYnVnICgxMjkpIDw8ICJLU2VsZWN0QWN0aW9uOjpz
ZXRDdXJyZW50SW5kZXgoIiA8PCBpbmRleCA8PCAiKSIgPDwgZW5kbDsKLSAgaWYgKFFBY3Rpb24q
IGEgPSBhY3Rpb24oaW5kZXgpKSB7Ci0gICAgc2V0Q3VycmVudEFjdGlvbihhKTsKLSAgICByZXR1
cm4gdHJ1ZTsKLSAgfQotCi0gIC8va0RlYnVnICgxMjkpIDw8ICJcdGRvaW5nIHRoZSBkZXNlbGVj
dCIgPDwgZW5kbDsKLSAgaWYgKHNlbGVjdGFibGVBY3Rpb25Hcm91cCgpLT5jaGVja2VkQWN0aW9u
KCkpCi0gICAgc2VsZWN0YWJsZUFjdGlvbkdyb3VwKCktPmNoZWNrZWRBY3Rpb24oKS0+c2V0Q2hl
Y2tlZChmYWxzZSk7Ci0KLSAgcmV0dXJuIGZhbHNlOworICAvL2tEZWJ1ZyAoMTI5KSA8PCAiS1Nl
bGVjdEFjdGlvbjo6c2V0Q3VycmVudEluZGV4KCIgPDwgaW5kZXggPDwgIixtb2RlPSIgPDwgbW9k
ZSA8PCAiKSIgPDwgZW5kbDsKKyAgcmV0dXJuIHNldEN1cnJlbnRBY3Rpb24oYWN0aW9uKGluZGV4
KSwgbW9kZSk7CiB9CiAKIFFBY3Rpb24gKiBLU2VsZWN0QWN0aW9uOjphY3Rpb24oIGludCBpbmRl
eCApIGNvbnN0CkBAIC0yODgsMTYgKzI5MywxMCBAQAogICByZXR1cm4gMEw7CiB9CiAKLWJvb2wg
S1NlbGVjdEFjdGlvbjo6c2V0Q3VycmVudEFjdGlvbiggY29uc3QgUVN0cmluZyAmIHRleHQsIFF0
OjpDYXNlU2Vuc2l0aXZpdHkgY3MpCitib29sIEtTZWxlY3RBY3Rpb246OnNldEN1cnJlbnRBY3Rp
b24oIGNvbnN0IFFTdHJpbmcgJiB0ZXh0LCBRdDo6Q2FzZVNlbnNpdGl2aXR5IGNzLCBEZXNlbGVj
dGlvbk1vZGUgbW9kZSkKIHsKLSAgLy9rRGVidWcgKDEyOSkgPDwgIktTZWxlY3RBY3Rpb246OnNl
dEN1cnJlbnRBY3Rpb24oIiA8PCB0ZXh0IDw8ICIsY3M9IiA8PCBjcyA8PCAiKSIgPDwgZW5kbDsK
LSAgaWYgKFFBY3Rpb24qIGEgPSBhY3Rpb24odGV4dCwgY3MpKSB7Ci0gICAgYS0+c2V0Q2hlY2tl
ZCh0cnVlKTsKLSAgICByZXR1cm4gdHJ1ZTsKLSAgfQotCi0gIC8va0RlYnVnICgxMjkpIDw8ICJc
dGZhaWxlZCIgPDwgZW5kbDsKLSAgcmV0dXJuIGZhbHNlOworICAvL2tEZWJ1ZyAoMTI5KSA8PCAi
S1NlbGVjdEFjdGlvbjo6c2V0Q3VycmVudEFjdGlvbigiIDw8IHRleHQgPDwgIixjcz0iIDw8IGNz
IDw8ICIsbW9kZT0iIDw8IG1vZGUgPDwgIikiIDw8IGVuZGw7CisgIHJldHVybiBzZXRDdXJyZW50
QWN0aW9uKGFjdGlvbih0ZXh0LCBjcyksIG1vZGUpOwogfQogCiB2b2lkIEtTZWxlY3RBY3Rpb246
OnNldENvbWJvV2lkdGgoIGludCB3aWR0aCApCkluZGV4OiBrc2VsZWN0YWN0aW9uLmgKPT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PQotLS0ga3NlbGVjdGFjdGlvbi5oCShyZXZpc2lvbiA1OTc0NzkpCisrKyBrc2VsZWN0YWN0
aW9uLmgJKHdvcmtpbmcgY29weSkKQEAgLTEwLDYgKzEwLDcgQEAKICAgICAgICAgICAgICAgKEMp
IDIwMDUtMjAwNiBIYW1pc2ggUm9kZGEgPHJvZGRhQGtkZS5vcmc+CiAgICAgICAgICAgICAgIChD
KSAyMDA2IEFsYmVydCBBc3RhbHMgQ2lkIDxhYWNpZEBrZGUub3JnPgogICAgICAgICAgICAgICAo
QykgMjAwNiBDbGFyZW5jZSBEYW5nIDxkYW5nQGtkZS5vcmc+CisgICAgICAgICAgICAgIChDKSAy
MDA2IE1pY2hlbCBIZXJtaWVyIDxtaWNoZWwuaGVybWllckBnbWFpbC5jb20+CiAKICAgICBUaGlz
IGxpYnJhcnkgaXMgZnJlZSBzb2Z0d2FyZTsgeW91IGNhbiByZWRpc3RyaWJ1dGUgaXQgYW5kL29y
CiAgICAgbW9kaWZ5IGl0IHVuZGVyIHRoZSB0ZXJtcyBvZiB0aGUgR05VIExpYnJhcnkgR2VuZXJh
bCBQdWJsaWMKQEAgLTUxLDcgKzUyLDcgQEAKICAgICBRX1BST1BFUlRZKCBib29sIGVkaXRhYmxl
IFJFQUQgaXNFZGl0YWJsZSBXUklURSBzZXRFZGl0YWJsZSApCiAgICAgUV9QUk9QRVJUWSggaW50
IGNvbWJvV2lkdGggUkVBRCBjb21ib1dpZHRoIFdSSVRFIHNldENvbWJvV2lkdGggKQogICAgIFFf
UFJPUEVSVFkoIFFTdHJpbmcgY3VycmVudFRleHQgUkVBRCBjdXJyZW50VGV4dCApCi0gICAgUV9F
TlVNUyggVG9vbGJhck1vZGUgKQorICAgIFFfRU5VTVMoIERlc2VsZWN0aW9uTW9kZSBUb29sYmFy
TW9kZSApCiAgICAgUV9QUk9QRVJUWSggVG9vbEJhck1vZGUgdG9vbEJhck1vZGUgUkVBRCB0b29s
QmFyTW9kZSBXUklURSBzZXRUb29sQmFyTW9kZSApCiAgICAgUV9QUk9QRVJUWSggUVRvb2xCdXR0
b246OlRvb2xCdXR0b25Qb3B1cE1vZGUgdG9vbEJ1dHRvblBvcHVwTW9kZSBSRUFEIHRvb2xCdXR0
b25Qb3B1cE1vZGUgV1JJVEUgc2V0VG9vbEJ1dHRvblBvcHVwTW9kZSApCiAgICAgUV9QUk9QRVJU
WSggaW50IGN1cnJlbnRJdGVtIFJFQUQgY3VycmVudEl0ZW0gV1JJVEUgc2V0Q3VycmVudEl0ZW0g
KQpAQCAtMTgyLDYgKzE4MywxMyBAQAogICAgICAqLwogICAgIHZpcnR1YWwgfktTZWxlY3RBY3Rp
b24oKTsKIAorICAgIGVudW0gRGVzZWxlY3Rpb25Nb2RlIHsKKyAgICAgIC8vLyBEZXNlbGVjdCBw
cmV2aW91c2x5IGNoZWNrZWQgYWN0aW9uLgorICAgICAgRGVzZWxlY3RNb2RlLAorICAgICAgLy8v
IERvbid0IGRlc2VsZWN0IGFueSBwcmV2aW91c2x5IGNoZWNrZWQgYWN0aW9uLgorICAgICAgTGV0
U2VsZWN0ZWRNb2RlCisgICAgfTsKKwogICAgIGVudW0gVG9vbEJhck1vZGUgewogICAgICAgLy8v
IENyZWF0ZXMgYSBidXR0b24gd2hpY2ggcG9wcyB1cCBhIG1lbnUgd2hlbiBpbnRlcmFjdGVkIHdp
dGgsIGFzIGRlZmluZWQgYnkgdG9vbEJ1dHRvblBvcHVwTW9kZSgpLgogICAgICAgTWVudU1vZGUs
CkBAIC0yNTYsMTEgKzI2NCwxMyBAQAogICAgIFFBY3Rpb24qIGFjdGlvbihjb25zdCBRU3RyaW5n
JiB0ZXh0LCBRdDo6Q2FzZVNlbnNpdGl2aXR5IGNzID0gUXQ6OkNhc2VTZW5zaXRpdmUpIGNvbnN0
OwogCiAgICAgLyoqCi0gICAgICogIFNldHMgdGhlIGN1cnJlbnRseSBjaGVja2VkIGl0ZW0uCisg
ICAgICogU2V0cyB0aGUgY3VycmVudGx5IGNoZWNrZWQgaXRlbS4KICAgICAgKgotICAgICAqICBA
cGFyYW0gaXRlbSB0aGUgUUFjdGlvbiB0byBiZWNvbWUgdGhlIGN1cnJlbnRseSBjaGVja2VkIGl0
ZW0uCisgICAgICogQHBhcmFtIGl0ZW0gdGhlIFFBY3Rpb24gdG8gYmVjb21lIHRoZSBjdXJyZW50
bHkgY2hlY2tlZCBpdGVtLgorICAgICAqCisgICAgICogXHJldHVybiBcZSB0cnVlIGlmIGEgY29y
cmVzcG9uZGluZyBhY3Rpb24gd2FzIGZvdW5kIGFuZCBzdWNjZXNzZnVsbHkgY2hlY2tlZC4KICAg
ICAgKi8KLSAgICB2b2lkIHNldEN1cnJlbnRBY3Rpb24oUUFjdGlvbiogYWN0aW9uKTsKKyAgICBi
b29sIHNldEN1cnJlbnRBY3Rpb24oUUFjdGlvbiogYWN0aW9uLCBEZXNlbGVjdGlvbk1vZGUgbW9k
ZSA9IERlc2VsZWN0TW9kZSk7CiAKICAgICAvKioKICAgICAgKiBcb3ZlcmxvYWQgc2V0Q3VycmVu
dEFjdGlvbihRQWN0aW9uKikKQEAgLTI3Myw3ICsyODMsNyBAQAogICAgICAqCiAgICAgICogXHJl
dHVybiBcZSB0cnVlIGlmIGEgY29ycmVzcG9uZGluZyBhY3Rpb24gd2FzIGZvdW5kIGFuZCB0aHVz
IHNldCB0byB0aGUgY3VycmVudCBhY3Rpb24sIG90aGVyd2lzZSBcZSBmYWxzZQogICAgICAqLwot
ICAgIGJvb2wgc2V0Q3VycmVudEl0ZW0oaW50IGluZGV4KTsKKyAgICBib29sIHNldEN1cnJlbnRJ
dGVtKGludCBpbmRleCwgRGVzZWxlY3Rpb25Nb2RlIG1vZGUgPSBEZXNlbGVjdE1vZGUpOwogCiAg
ICAgLyoqCiAgICAgICogXG92ZXJsb2FkIHNldEN1cnJlbnRBY3Rpb24oUUFjdGlvbiopCkBAIC0y
ODYsNyArMjk2LDcgQEAKICAgICAgKgogICAgICAqIFxyZXR1cm4gXGUgdHJ1ZSBpZiBhIGNvcnJl
c3BvbmRpbmcgYWN0aW9uIHdhcyBmb3VuZCwgb3RoZXJ3aXNlIFxlIGZhbHNlCiAgICAgICovCi0g
ICAgYm9vbCBzZXRDdXJyZW50QWN0aW9uKGNvbnN0IFFTdHJpbmcmIHRleHQsIFF0OjpDYXNlU2Vu
c2l0aXZpdHkgY3MgPSBRdDo6Q2FzZVNlbnNpdGl2ZSk7CisgICAgYm9vbCBzZXRDdXJyZW50QWN0
aW9uKGNvbnN0IFFTdHJpbmcmIHRleHQsIFF0OjpDYXNlU2Vuc2l0aXZpdHkgY3MgPSBRdDo6Q2Fz
ZVNlbnNpdGl2ZSwgRGVzZWxlY3Rpb25Nb2RlIG1vZGUgPSBEZXNlbGVjdE1vZGUpOwogCiAgICAg
LyoqCiAgICAgICogQWRkIFxhIGFjdGlvbiB0byB0aGUgbGlzdCBvZiBzZWxlY3RhYmxlIGFjdGlv
bnMuCg==
------=_Part_87150_11777158.1161372965330--