[compiz] Re: compiz: Changes to 'master'

[Mike Dransfield]

David Reveman wrote:
> On Wed, 2007-04-25 at 20:45 +0100, Mike Dransfield wrote:
>   
>> David Reveman wrote:
>>     
>>> On Wed, 2007-04-25 at 11:09 -0700, Robert Carr wrote:
>>>   
>>>       
>>>> include/compiz.h |    1 +
>>>>  1 files changed, 1 insertion(+)
>>>>
>>>> New commits:
>>>> commit 2402215a6a3bd50e9d87e99d4a45de14b635ecea
>>>> Merge: 1b0ae38... 7f518da...
>>>> Author: Robert Carr <racarr at gorbie.(none)>
>>>> Date:   Wed Apr 25 14:09:38 2007 -0400
>>>>
>>>>     Merge branch 'master' of git+ssh://racarr@git.freedesktop.org/git/xorg/app/compiz
>>>>
>>>> commit 1b0ae388155a18ea07f148d6163cf7d5deaa0cbd
>>>> Author: Robert Carr <racarr at gorbie.(none)>
>>>> Date:   Wed Apr 25 14:09:25 2007 -0400
>>>>
>>>>     Add a priv entry to CompAction. For a use case see: compiz-scheme. It's neccesary to implement in a proper way actions that have to go through a wrapper C function. In general the idea of having Actions without an assosciated Option / Actions added at run time needs to be explored a bit more because the current code is not well suited for it.
>>>>     
>>>>         
>>> Why did you add it to the CompOption struct? If we need some user data
>>> attached to actions then that should go into the CompAction struct but
>>> I'm not sure we want that at all. If you want to create run-time actions
>>> like your compiz-schema code is doing I suggest that you just use
>>> addScreenAction and check for a matching event manually in handleEvent.
>>>   
>>>       
>> I had the same problem with the python plugin (ie runtime generation of
>> options/actions).  I solved it by comparing the memory address of the
>> action since CompActionInitiateProc has a reference to it.  I am not sure
>> this is a very nice thing to do, but it works well so far.
>>     
>
> Pointer comparison is fine, there's nothing wrong with that but it's not
> currently allowed to add options at run-time in any other way than by
> activating a plugin.
>
>   
>> I might have a look at doing the python plugin this way, but the lazyness
>> inside me is saying not to ;)
>>     
>
> If we want to run-time actions and hooking into handleEvent and manually
> checking if the event matches an action is currently too much trouble or
> not appropriate for some reason an interface for run-time actions should
> be added. It should be very easy to add this.
>   

I think the extra private is fine for my needs and it means
that I do not have to have any ugly lookups.

I only add new options with a new plugin, so I do not think we
would need a new way to add extra actions at runtime.

I was looking at supporting the private data in the CompAction,
but shouldn't it be like this:

CompPrivate priv;

rather than

void *priv;

It would be more consistent with what we have.

This private data could also be used to reduce the amount of code
in some plugins.  eg. zoom in/out could use the same initiate function
but with different user data it would not need the action wrapper
functions that we have now.


> - David
>
>