And now that I just read Bjoerns last mail wrt old/current behavior: At least for the EditEngine I am quite sure these items have been in some ItemPool, but never in an ItemSet. So that might be the reason why I never had an issue with that.
Not sure what would happen with the VoidItems listed below in other places. Malte. Malte Timmermann wrote, On 06/16/10 11:07: > > Mathias Bauer wrote, On 06/16/10 10:11: >> On 15.06.2010 11:47, Björn Michaelsen wrote: >> >>> Well, the implementation in new_itemsets only checks for the which id >>> being 0 and never checks for the type being an SfxVoidItem (outside >>> assertions). So an disabled item is consistently defined as any item >>> returning an which id of 0 in the new itemset implementation. >> That's an improvement and it allows to use the "API" consistently. OTOH >> it is still ugly. :-) I consider the way how the ItemSet codes the >> "disabled" status an implementation detail. >> >>> Now the interesting part is finding the clients of the itemset that >>> rely on void items being interpreted as "disabled" by the itemset and >>> fixing those .... >> Question is if that is more effort then replacing all code that puts >> items with ID = 0 with code calling "DisableItem" instead. That would >> make putting an item with ID=0 an invalid action. >> >> BTW: I don't remember if any of the two ItemSet implementations returns >> an VoidItem in a GetItem call for a disabled item or if NULL is returned >> then. IMHO it always should be the latter. > > We have > const SfxPoolItem& Get(....) > and > const SfxPoolItem* GetItem(...) > > Not sure why we have two different Get Methods, but the first doesn't > allow to return NULL, so I assume the second one would also have to > return the same VoidItem for consistency. > >> Back to your primary topic. If we identify the places where a VoidItem >> with ID != 0 is put, and if the developers state that this is by intent, >> we can remove that assertion and make putting VoidItems with ID != 0 a >> valid action that does not disable that item. > > Yes, I guess it would be better to keep allowing VoidItems with Which != > 0. Or we search for all places where we might have != 0 VoidItems. > > Places I found: > EditEngine has some, but that could be changed. > > I have no clue here: > http://svn.services.openoffice.org/opengrok/xref/DEV300_m81/sfx2/source/control/sfxstatuslistener.cxx#233 > if ( pType == ::getVoidCppuType() ) > 232 { > 233 pItem = new SfxVoidItem( m_nSlotID ); > 234 eState = SFX_ITEM_UNKNOWN; > 235 } > > I don't think the slot id would be zero... > But this looks like we really would mix up slot and which ids. > Some more such things in sfx2/source/control/ > > SW: > rReq.SetReturnValue(SfxVoidItem(bLabel ? FN_LABEL : FN_BUSINESS_CARD)); > rSet.Put( SfxVoidItem( SID_ATTR_ZOOMSLIDER ) ); > rReq.SetReturnValue( SfxVoidItem( rReq.GetSlot() ) ); > > SVX: > new SfxVoidItem(SDRATTR_SHADOW3D ); > new SfxVoidItem(SDRATTR_SHADOWPERSP ); > > I didn't check the [all...] links in OpenGrok, but with these hits I > already feel that we simply should keep the old behavior... > >> The question why ItemSet::Put must allow that nWhich differs from the >> item's which is still open. I assume that it's related to different >> pools having different WhichIds for the same item. But knowing it for >> sure would perhaps allow us to define a less fuzzy interface. > > Putting them on a different ID than it's own ID doesn't sound right to me. > For the different which ID stuff we have the SlotID mechanism. > But that would probably need cloning the item and changing the which > when putting it into a pool/itemset where different which ids are use. > So the situations where we put items on different IDs should be > investigated more closely. > > But again, looking at > pItem = new SfxVoidItem( m_nSlotID ); > rReq.SetReturnValue( SfxVoidItem( rReq.GetSlot() ) ); > it seems to be a supported and wanted scenario. > > Malte. > >> Regards, >> Mathias >> > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
