[lazarus] Enable/CheckMenuItem

[Marc Weustink]

At 20:01 22-11-2003, Micha Nelissen wrote:
>Marc Weustink wrote:
>
>>At 18:56 22-11-2003, Micha Nelissen wrote:
>>
>>>Marc Weustink wrote:
>>>
>>>>At 17:34 22-11-2003, Mattias Gaertner wrote:
>>>>
>>>>>On Sat, 22 Nov 2003 17:18:31 +0100  Marc Weustink <marc at dommelstein.net>
>>>>>wrote:
>>>>>
>>>>> > At 17:08 22-11-2003, Mattias Gaertner wrote:
>>>>> > >On Sat, 22 Nov 2003 12:41:30 +0100  Marc Weustink 
>>>>> <marc at dommelstein.net>
>>>>> > >wrote:
>>>>> > >
>>>>> > > > At 09:04 22-11-2003, Micha Nelissen wrote:
>>>>> > > > >Marc Weustink wrote:
>>>>> > > > >
>>>>> > > > >>At 22:23 21-11-2003, Micha wrote:
>>>>> > > > >>
>>>>> > > > >>>Specification of Enable/CheckMenuItem needs to be more specific.
>>>>> > > > >Due>>to  win32api obscurities it needs to be changed: note 
>>>>> that only
>>>>> > > > >hndMenu
>>>>> > > > >
>>>>> > > > >>>changed:
>>>>> > > > >>>
>>>>> > > > >>>hndMenu: specifies _parent_ of menuitem to change
>>>>> > > > >>
>>>>> > > > >>??? Doesn't the win32 interface keep track of that (or can't 
>>>>> it be
>>>>> > > > >quiried) ?
>>>>> > > > >
>>>>> > > > >(1) No. (2) It can't be, there is no GetParentMenu(handle) call or
>>>>> > > > >something like that.
>>>>> > > >
>>>>> > > > I should have read before asking :)
>>>>> > > >
>>>>> > > > Your original message:
>>>>> > > > >Specification of Enable/CheckMenuItem needs to be more 
>>>>> specific. Due
>>>>> > > > >to  win32api obscurities it needs to be changed: note that only
>>>>> > > > >hndMenu changed:
>>>>> > > > >
>>>>> > > > >hndMenu: specifies _parent_ of menuitem to change
>>>>> > > > >uIDEnableItem: Integer - menu item to check/uncheck
>>>>> > > > >bChecked: Boolean      - new state of the menu item
>>>>> > > >
>>>>> > > >
>>>>> > > > Ehm, now I understand your question. The GTK winapi 
>>>>> implementation of
>>>>> > > > CheckMenuItem is wrong. It uses hndMenu as the item to check and
>>>>> > > > ignores uIDEnableItem. But it should use hndMenu as menu and
>>>>> > > > uIDEnableItem as"command or index" to the Item.
>>>>> > >
>>>>> > >Why that complicated?
>>>>> >
>>>>> > It is not complicated.
>>>>> >
>>>>> > >We want to enable/check a menu item, and not to emulate the win32 api.
>>>>> > >If the win32api function does not do, what we need, then we should 
>>>>> simply
>>>>> > >rename the interface function (e.g. function
>>>>> > >EnableOrCheckMenuItem(AMenuItem: TComponent; EnableCheck: boolean):
>>>>> > >boolean).
>>>>> > >The win32 intf can then simply get the parent handle.
>>>>> >
>>>>> > If functions are close, I prefer winApi compatible functions. It makes
>>>>> > porting easier. I don't feel like introducing a load of
>>>>> > similar_but_not_the_same functions.
>>>>> > We started with a load of api functions, I see no reason why we would
>>>>> > replace them. If we want the api function, we have to implement it 
>>>>> anyhow.
>>>>>
>>>>>We want the most commonly used winapi functions. If you think, that this
>>>>>function is commonly used, then ok, let's emulate it. I doubt it.
>>>>>
>>>>>IMO the win32api function is too win32 specific. I think, nearly all other
>>>>>interfaces will have the ability to enable a menu item directly and don't
>>>>>have an "Index". So, adding this function to the interface means overhead
>>>>>for all interfaces, except the win32. While a direct function means 
>>>>>overhead
>>>>>only for the win32 intf.
>>>>
>>>>
>>>>I don't see overhead as an issue here. There willbe always overhead.
>>>>For these menufuntions we could introduce a MH_HANDLE flag (there is 
>>>>already a MF_INDEX and a MF_COMMAND) to tell the interface to use the handle.
>>>
>>>
>>>Nooo...please, that's an ugly hack imho.
>>
>>I don't see it as a hack. All menu functions require a flag, telling how 
>>to interpret the uID parameter. For win32 it is MF_BYCOMMAND and 
>>MF_BYINDEX. I don't see why introducing another interpretation would be a hack.
>
>We have 2 options here:
>1) Implement a winapi-compatible function. In this case we should take the 
>route suggested by my initial post. We should not add additional 
>behaviour: then the whole point of compatibility is gone.

You don't break compatibility by adding extra functionality as long as you 
also support the original functionality.
What functionality the LCL uses doesn't matter, that won't break anything.

>2) Implement a custom function. It can do whatever we want it to.
>
>Since it's obvious ;) by now we don't want 1), we'll take route 2). We 
>should change the function name to EnableAMenuItem, as mattias suggested.

To me it isn't that obvious yet.

>Shall I make the necessary modifications, or have you already begun, marc?

I already made some changes for option 1.

At this moment it might look like only needed for menus, but I think it 
implies more. By just adding another function we aren't finished.
Today I looked at the winapi unit and a noticed that it is already polluted 
with non winapi functions. This is misleading. IMO the winapi unit should 
only contain winapi compatible functions. All others, including the new 
enablemenuitem should go to another (lclapi ?) unit.
About naming, I don't think EnableAMenuItem is a good name. IMO 
LCLEnableMenuItem would be better.

So I would suggest to clear this first before doing some implementation.

Marc