Re: Patch: FvwmAnimate using ParseModuleArgs() - NOT WORKING YET
On 2/4/06, Dan Espen [EMAIL PROTECTED] wrote: seventh guardian [EMAIL PROTECTED] writes: On 2/4/06, Dan Espen [EMAIL PROTECTED] wrote: I think you need to keep the asterisk at the front of the name. There are times when you need it there, and times when you don't. It's easier to put it there in the first place rather than trying to stick it back when you need it. Yes, it is far more efficient that way. But the problem is that the existing structure only stores the name, and not the asterisk. Of course we could use the existing char* MyName, but that would defeat the whole purpose of using the ModuleArgs struct. I wonder if we need the asterisk in the first place. Wouldn't it be easyer new-code-wise to use only the name for pattern matching? It could be stripped off by fvwm or simply not used in the config files. (since this is only experimental code we are allowed to forget backward compatibility issues) Eventually you have to deal with compatibility. FvwmAnimate mallocs the storage for MyName and never frees it. I don't consider that a leak since it does it one time. We could arrange to free it though. You can change ParseModuleArgs to malloc the space for the name including the *. Then you have 2 choices: Pass the address of the second byte and change FvwmAnimate to refer to the full name as name-1 or Pass the address of the first byte and change all the modules that want the name without the asterisk to reference name+1. Giving it a better thought, probably using the first option will be better than using CatString3. Since the work I'm doing will eventually allow us to replace the args for InitGetConfigLine with the whole struct, and probably to remove the *FvwmAnimate part of the line before it is given to the user (maybe using something similar to GlobalConfig and ModuleConfig in perl), the module-name-1 string will only be used inside the Module.c functions. Being so it makes sense to have module-name availiable in a straightforward way for user output, and having module-name-1 still availiable for some obscure user function. For now I would ask you to tolerate the current use of CatString3 to avoid making those changes to Module.h, as it makes things easier for me. Cheers! Renato Caldas
Re: Patch: FvwmAnimate using ParseModuleArgs() - NOT WORKING YET
On 2/4/06, seventh guardian [EMAIL PROTECTED] wrote: On 2/2/06, Dominik Vogt [EMAIL PROTECTED] wrote: So, the work can be split into several smaller steps: 1. Make all the modules use ParseModuleArgs() and copy the fds from the ModuleArgs struct to the arrays that are currently used by the modules. 2. Remove the fd arrays in the modules and use the fds in the ModuleArgs instead. 3. Make the service functions in Module.c use a (const ModuleArgs *) instead of passing individual arguments and adapt the modules. FvwmAnimate is mostly done for the 1st item in the list. I also made the code use the struct-name instead of the char *MyName global var. As for what isn't done yet in this module: FvwmAnimate uses some macros (yuch!) for the communication with fvwm. I used CatString3() for the places where we need the * char before the module name, but I really don't know what to do in with the macros. I thought of making them inline functions, but I would like to know what you think.. Search for these macros are not done yet in the patch and you will find it. With that problem aside, I hope FvwmAnimate users could test the new code to see if it works ok. Note that it is _not_ working right now because of those macros. Cheers! Renato Caldas OOPS forgot to send the patch itself.. here it goes. The patch goes against Fvwm-2.5.16. I guess FvwmAnimate hasn't changed in CVS.. If it has please tell me so that I can diff it against CVS. Cheers FvwmAnimate.patch Description: Binary data
Re: Patch: FvwmAnimate using ParseModuleArgs() - NOT WORKING YET
I think you need to keep the asterisk at the front of the name. There are times when you need it there, and times when you don't. It's easier to put it there in the first place rather than trying to stick it back when you need it. Since I put the macros there and I like to use macros, I have to ask what you think is wrong with them. Personally, I find that the more compact the code is, the easier it is to read. Inline functions for C are not portable. -Original Message- Date: Sat, 4 Feb 2006 14:29:17 + From: seventh guardian [EMAIL PROTECTED] To: FVWM Workers fvwm-workers@fvwm.org Subject: Patch: FvwmAnimate using ParseModuleArgs() - NOT WORKING YET Sender: [EMAIL PROTECTED] On 2/2/06, Dominik Vogt [EMAIL PROTECTED] wrote: So, the work can be split into several smaller steps: 1. Make all the modules use ParseModuleArgs() and copy the fds from the ModuleArgs struct to the arrays that are currently used by the modules. 2. Remove the fd arrays in the modules and use the fds in the ModuleArgs instead. 3. Make the service functions in Module.c use a (const ModuleArgs *) instead of passing individual arguments and adapt the modules. FvwmAnimate is mostly done for the 1st item in the list. I also made the code use the struct-name instead of the char *MyName global var. As for what isn't done yet in this module: FvwmAnimate uses some macros (yuch!) for the communication with fvwm. I used CatString3() for the places where we need the * char before the module name, but I really don't know what to do in with the macros. I thought of making them inline functions, but I would like to know what you think.. Search for these macros are not done yet in the patch and you will find it. With that problem aside, I hope FvwmAnimate users could test the new code to see if it works ok. Note that it is _not_ working right now because of those macros. Cheers! Renato Caldas
Re: Patch: FvwmAnimate using ParseModuleArgs() - NOT WORKING YET
On 2/4/06, Dan Espen [EMAIL PROTECTED] wrote: I think you need to keep the asterisk at the front of the name. There are times when you need it there, and times when you don't. It's easier to put it there in the first place rather than trying to stick it back when you need it. Yes, it is far more efficient that way. But the problem is that the existing structure only stores the name, and not the asterisk. Of course we could use the existing char* MyName, but that would defeat the whole purpose of using the ModuleArgs struct. I wonder if we need the asterisk in the first place. Wouldn't it be easyer new-code-wise to use only the name for pattern matching? It could be stripped off by fvwm or simply not used in the config files. (since this is only experimental code we are allowed to forget backward compatibility issues) Since I put the macros there and I like to use macros, I have to ask what you think is wrong with them. Personally, I find that the more compact the code is, the easier it is to read. Sorry, it's just my personal liking. Macros are also ok, what I don't like in macros is that the code appears to be standard C but it's not (it has its own syntax..). And if it worked perfectly with the old code, it's puzling me with the new one: Assuming we are to replace char* MyName with the struct, I currently don't know what to do with one of the existing macros (the one that uses the asterisk'ed name). Forgive my macros' ignorance, but I don't know what to do to call CatString3() from within the macro.
Re: Patch: FvwmAnimate using ParseModuleArgs() - NOT WORKING YET
seventh guardian [EMAIL PROTECTED] writes: On 2/4/06, Dan Espen [EMAIL PROTECTED] wrote: I think you need to keep the asterisk at the front of the name. There are times when you need it there, and times when you don't. It's easier to put it there in the first place rather than trying to stick it back when you need it. Yes, it is far more efficient that way. But the problem is that the existing structure only stores the name, and not the asterisk. Of course we could use the existing char* MyName, but that would defeat the whole purpose of using the ModuleArgs struct. I wonder if we need the asterisk in the first place. Wouldn't it be easyer new-code-wise to use only the name for pattern matching? It could be stripped off by fvwm or simply not used in the config files. (since this is only experimental code we are allowed to forget backward compatibility issues) Eventually you have to deal with compatibility. FvwmAnimate mallocs the storage for MyName and never frees it. I don't consider that a leak since it does it one time. We could arrange to free it though. You can change ParseModuleArgs to malloc the space for the name including the *. Then you have 2 choices: Pass the address of the second byte and change FvwmAnimate to refer to the full name as name-1 or Pass the address of the first byte and change all the modules that want the name without the asterisk to reference name+1. Since I put the macros there and I like to use macros, I have to ask what you think is wrong with them. Personally, I find that the more compact the code is, the easier it is to read. Sorry, it's just my personal liking. Macros are also ok, what I don't like in macros is that the code appears to be standard C but it's not (it has its own syntax..). C macros are part of standard C. And if it worked perfectly with the old code, it's puzling me with the new = one: Assuming we are to replace char* MyName with the struct, I currently don't know what to do with one of the existing macros (the one that uses the asterisk'ed name). Forgive my macros' ignorance, but I don't know what to do to call CatString3() from within the macro. Since CatString3 returns the addess of the static buffer, it should work fine. Yep, just tested it, it works just like you are using it with macros. I think you have something else going on. -- Dan Espen E-mail: [EMAIL PROTECTED]
Re: Patch: FvwmAnimate using ParseModuleArgs() - NOT WORKING YET
On 2/4/06, Dan Espen [EMAIL PROTECTED] wrote: seventh guardian [EMAIL PROTECTED] writes: On 2/4/06, Dan Espen [EMAIL PROTECTED] wrote: I think you need to keep the asterisk at the front of the name. There are times when you need it there, and times when you don't. It's easier to put it there in the first place rather than trying to stick it back when you need it. Yes, it is far more efficient that way. But the problem is that the existing structure only stores the name, and not the asterisk. Of course we could use the existing char* MyName, but that would defeat the whole purpose of using the ModuleArgs struct. I wonder if we need the asterisk in the first place. Wouldn't it be easyer new-code-wise to use only the name for pattern matching? It could be stripped off by fvwm or simply not used in the config files. (since this is only experimental code we are allowed to forget backward compatibility issues) Eventually you have to deal with compatibility. Yes, but unless there's a reason for the use of the asterisk, it is cleaner not to have it. Configuration keywords are changing all the time, it won't be that difficult to strip the * from the config files. By the way, anyone knows why is the asterisk there for? FvwmAnimate mallocs the storage for MyName and never frees it. I don't consider that a leak since it does it one time. Me neither. But it definitely makes sense to only use one location for the same data. We could arrange to free it though. You can change ParseModuleArgs to malloc the space for the name including the *. Then you have 2 choices: Pass the address of the second byte and change FvwmAnimate to refer to the full name as name-1 or Pass the address of the first byte and change all the modules that want the name without the asterisk to reference name+1. Yes, it's possible. The question is, is it worth the complication? Since we are redoing things let's cut the problems from the root: is there any reason for the existance of the asterisk? If not it would be _a lot_ simpler just to use the name. What it seems to me is that the module side has no need for it. It in fact makes thigs trickier. On the other hand, fvwm side may rely on it to sort out the module configs. Why can't fvwm just strip the leading * from the configs? Since I put the macros there and I like to use macros, I have to ask what you think is wrong with them. Personally, I find that the more compact the code is, the easier it is to read. Sorry, it's just my personal liking. Macros are also ok, what I don't like in macros is that the code appears to be standard C but it's not (it has its own syntax..). C macros are part of standard C. Wrong choice of words. I meant they have their special syntax, which can be misleading sometimes. And if it worked perfectly with the old code, it's puzling me with the new = one: Assuming we are to replace char* MyName with the struct, I currently don't know what to do with one of the existing macros (the one that uses the asterisk'ed name). Forgive my macros' ignorance, but I don't know what to do to call CatString3() from within the macro. Since CatString3 returns the addess of the static buffer, it should work fine. Yep, just tested it, it works just like you are using it with macros. I think you have something else going on. My fault. It's working now. The working patch goes attached. FvwmAnimate.patch Description: Binary data
Re: Patch: FvwmAnimate using ParseModuleArgs() - NOT WORKING YET
On 04 Feb 2006 16:56:47 +, seventh guardian wrote: existing structure only stores the name, and not the asterisk. Of course we could use the existing char* MyName, but that would defeat the whole purpose of using the ModuleArgs struct. I wonder if we need the asterisk in the first place. Wouldn't it be easyer new-code-wise to use only the name for pattern matching? It could be stripped off by fvwm or simply not used in the config files. (since this is only experimental code we are allowed to forget backward compatibility issues) Eventually you have to deal with compatibility. Yes, but unless there's a reason for the use of the asterisk, it is cleaner not to have it. Configuration keywords are changing all the time, it won't be that difficult to strip the * from the config files. Here you speak about the fvwm configuration. You can't omit the asterisk without inventing a new syntax that does not conflict with the syntax of fvwm commands and functions. I see no problem in the asterisks syntax, it seems to be more optimal than other possible syntaxes for module configs. By the way, anyone knows why is the asterisk there for? Here I suppose you ask about the asterisk sent in M_CONFIG_INFO packets. I suggest you to learn the fvwm-to-module protocol more in depth. You may run FvwmGtkDebug or FvwmDebug to see what is sent by fvwm to a module on its Send_ConfigInfo request. You may discover that the asterisk is needed to distinguish from other M_CONFIG_INFO packets that carry a non module configuration. BTW, perllib solves this by having two tracker types, GlobalConfig and ModuleConfig, so a module in perl has a nice API. Redesigning the module protocol may be fine in the future, but this would mean much larger changes than you think. A good suggestion may be, for example, to add a new module packet MX_MODULE_CONFIG carrying the configuration line without the leading *FvwmModule: , and not just omitting an asterisk, as you suggest. Please do not break any compatibility right now. Let's not forget, we are in the freeze before 2.6.0. Regards, Mikhael.
Re: Patch: FvwmAnimate using ParseModuleArgs() - NOT WORKING YET
Mikhael Goikhman [EMAIL PROTECTED] writes: On 04 Feb 2006 16:56:47 +, seventh guardian wrote: existing structure only stores the name, and not the asterisk. Of course we could use the existing char* MyName, but that would defeat the whole purpose of using the ModuleArgs struct. I wonder if we need the asterisk in the first place. Wouldn't it be easyer new-code-wise to use only the name for pattern matching? It could be stripped off by fvwm or simply not used in the config files. (since this is only experimental code we are allowed to forget backward compatibility issues) Eventually you have to deal with compatibility. Yes, but unless there's a reason for the use of the asterisk, it is cleaner not to have it. Configuration keywords are changing all the time, it won't be that difficult to strip the * from the config files. Here you speak about the fvwm configuration. You can't omit the asterisk without inventing a new syntax that does not conflict with the syntax of fvwm commands and functions. I see no problem in the asterisks syntax, it seems to be more optimal than other possible syntaxes for module configs. The asterisk tells Fvwm to save up the module config commands, not parse them. It then sends out the commands to a module when the module asks for them. There's more to it than what I just described. Check the module documentation. -- Dan Espen E-mail: [EMAIL PROTECTED]
Re: Patch: FvwmAnimate using ParseModuleArgs() - NOT WORKING YET
On 2/4/06, Mikhael Goikhman [EMAIL PROTECTED] wrote: On 04 Feb 2006 16:56:47 +, seventh guardian wrote: existing structure only stores the name, and not the asterisk. Of course we could use the existing char* MyName, but that would defeat the whole purpose of using the ModuleArgs struct. I wonder if we need the asterisk in the first place. Wouldn't it be easyer new-code-wise to use only the name for pattern matching? It could be stripped off by fvwm or simply not used in the config files. (since this is only experimental code we are allowed to forget backward compatibility issues) Eventually you have to deal with compatibility. Yes, but unless there's a reason for the use of the asterisk, it is cleaner not to have it. Configuration keywords are changing all the time, it won't be that difficult to strip the * from the config files. Here you speak about the fvwm configuration. You can't omit the asterisk without inventing a new syntax that does not conflict with the syntax of fvwm commands and functions. I see no problem in the asterisks syntax, it seems to be more optimal than other possible syntaxes for module configs. By the way, anyone knows why is the asterisk there for? Here I suppose you ask about the asterisk sent in M_CONFIG_INFO packets. I suggest you to learn the fvwm-to-module protocol more in depth. You may run FvwmGtkDebug or FvwmDebug to see what is sent by fvwm to a module on its Send_ConfigInfo request. You may discover that the asterisk is needed to distinguish from other M_CONFIG_INFO packets that carry a non module configuration. BTW, perllib solves this by having two tracker types, GlobalConfig and ModuleConfig, so a module in perl has a nice API. Yes, but as I found in the source code, modules distinguish them by looking for the *FvwmMyname string instead of just the asterisk. I don't know about perl, but c modules usually do that. In this case it would be as easy to look for just for FvwmMyname, without needing either the asterisked string or calling CatString3. Redesigning the module protocol may be fine in the future, but this would mean much larger changes than you think. A good suggestion may be, for example, to add a new module packet MX_MODULE_CONFIG carrying the configuration line without the leading *FvwmModule: , and not just omitting an asterisk, as you suggest. Please do not break any compatibility right now. Let's not forget, we are in the freeze before 2.6.0. I guess you're right. Breaking compatibility now would be a waste of time, since it's going to be redesigned again for 2.6.. So we're back to the issue of using CatString3 or not..
Re: Patch: FvwmAnimate using ParseModuleArgs() - NOT WORKING YET
On 04 Feb 2006 19:42:58 +, seventh guardian wrote: Yes, but as I found in the source code, modules distinguish them by looking for the *FvwmMyname string instead of just the asterisk. I don't know about perl, but c modules usually do that. In this case it would be as easy to look for just for FvwmMyname, without needing either the asterisked string or calling CatString3. As I said, a module gets M_CONFIG_INFO packets from fvwm, with text like: DesktopSize ... ImagePath ... Colorset ... *FvwmAnimate... You may add a higher level API similar to what perllib does (to work with GlobalConfig, ModuleConfig, Colorsets), but you can't just omit an asterisk when you send or match the text, because a user may have a module with any name. Regards, Mikhael.
