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: dev-unsubscr...@openoffice.org For additional commands, e-mail: dev-h...@openoffice.org
