Re: Become a mantainer of wxphp

Johannes thanks a lot for reviewing the code and sharing a lot of knowledge that now I can implement on the extension :) [quote=Johannes]Did you talk to Mário when taking it over? Is he ok with that? Is there a "migration path" for people coming from the old version to the new one? Will everything still work? (not needed, but useful ...)[/quote] I maintain contact with Mário, If IIRC I already talked about this with him but I wanted to have something working and familiarize more with the zend engine before becoming a maintainer. Old version of this extension didn't supported a lot of things, and had bugs that I fixed. Also wxWidgets interface is mostly consistent so if somebody used old version this new one should work with little or no changes at all. [quote=Johannes]If PHP_METHOD(php_wxApp, Yield) in app.cpp is a hand-written it is strange (for instance it sets "valid=1" and then does "if (valid)"it also does an "if (ZEND_NUM_ARGS()==0)" followed by a switch based on the same value. Functions not expecting parameters should use zend_parse_parameters_none() to produce a nicer error.[/quote] I haven't touched the wxApp wrapper much but I guess that Mário maybe used parts of the code generator to create that class and then tweak it, so maybe thats why it looks like that. [quote=Johannes]zend_parse_parameters is always set to quiet mode, therefore the user won't get a proper error when passing wrong arguments..[/quote] Thats due to the fact that wxWidgets class methods sometimes (almost all the time :D) have different overloads, so the method can expect any number of combinations of arguments, I don't know if theres a better way to handle that situation. [quote=Johannes]Preprocessor macros should always start in the first column.[/quote] Did some indentation to some macros that enclose debugging code because I found it easier to read that starting the macro at first column then huge indentation to see actual code inside the macro, so is mandatory to have it of first column ? :) [quote=Johannes]You're using call_user_function_ex() the zend_call_method[_with_[1-3]_param[s] family of functions from zend_interfaces.h can be nicer.[/quote] Ahh thanks a lot for that one, I noticed that using call_user_function_ex doesn't works well when calling a private method from a public virtual method when working with classes. So that should fix it I guess :) [quote=Johannes]Why are you using objects and resources?[/quote] Thats actually for historical reasons thats how it was coded originally, and I'm still a noob with the zend engine, I have also asked that question my self, why it was done that way, and use of "wxResource" string as push objects as resources. But still I don't know which other way can be done. [quote=Johannes]I think bool wxLogWindow_php::OnFrameClose(wxFrame* frame) won't freearguments[0] and [1] properly. Did you check for leaks? [/quote] I actually focused on code that handles destruction of objects when they become out of scope, since on the older version this wasn't handle resulting in a lot of memory leaks. So I have to check those that you mention since I thing I introduced them trying to fix other issues X_X [quote=Johannes]Generated code is hard to read ;-)[/quote] Well I have tried to keep it as readable as possible with indentations and everything, but at least it pays off that those almost 1 millions lines of code doing the same repetitive task doesn't have to be written by hand, and future changes would require a change on the code generator to take effect on the whole code :) Again thanks a lot for your review, really helpful, it means a lot :)