Re: Become a mantainer of wxphp

hi, Thanks, that will help for the next steps :) Cheers, On Thu, May 3, 2012 at 2:42 PM, Mário Soares <mariojcs@gmail.com> wrote: > Hi, > > I'm Mário, the original developer for for the wxPHP extension on PECL. > I'm writing just to confirm jgmdev as the main maintainer for the > project. > > Best regards, > Mário > > 2012/5/2 Johannes Schlüter <johannes@schlueters.de>: >> Hi, >> >> On Wed, 2012-05-02 at 13:56 -0400, JGM wrote: >>> [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. >> >> Ok. Just want to make sure we don't go over somebody's head, that's not >> nice, usually. And having a migration path for users is always good. >> (even for new users, who are more likely to trust the new code ifthey >> don't have to fear it will break in future ;-) ) >> >>> [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. >> >> If there's no reason to keep it like this one might clean it up. >> >>> [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. >> >> A typical pattern used sometimes is something like this: (I'm too lazy >> to type long names ;-) ) >> >>   if (zpp_ex(QUIET, "sss", ...) == FAILURE) { >>      if (zpp_ex(QUIET, "sa", ...) == FAILURE) { >>         if (zpp("a", ...) == FAILURE) { >>             return; >>         } >>         /* handle version with a single a */ >>       } else { >>    /* ....*/ >> >> One could also do >> >>   switch (ZEND_NUM_ARGS()) { >>   case 1: if (zpp() == FAILURE) return; >>   case 2: if (zpp() == FAILURE) return; >>   default: WRONG_PARAMETER_COUNT; return; >>   } >> >> one key item is that there should always be an error message if it is >> wrong in some way. In the first case the last one will create an error >> (which can be a bit misleading ...) the second case can provide more >> specific errors in some cases, but gives the imprecise "wrong parameter" >> message. >> >> I don't know what is best in this case :-) >> >>> [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 ? :) >> >> I just checked the C99 standard (don't have C89 or C++ at hand right >> now) and contrary to what I thought whitespaces are allowed before the >> #. I remember some trouble with older compilers ... but I guess the code >> is ok. >> >>> [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 :) >> >> Yes, zend_call_method accepts a class_entry as "scope" to identify which >> priate/protected elements can be accessed. >> >>> [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. >> >> The basic pattern is to extend zend_object in the C way >> >> struct  my_object { >>    zend_object zo; >>    int a_private_integer_not_accesible_from_php; >>    my_data_type* custom_data; >>    void* anything; >> } >> >> When doing this in C++ you have to ensure the zend_object is in the >> beginning of the structure and that the compiler doesn't put any >> metadata in front (in other words (size_t)&object == (size_t)&object->zo >> must be true), probably an  extern "C"   declaration is needed. >> >> Then add a C/C++-level constructor and destructor: >> >> void my_object_free(void *object TSRMLS_DC) >> { >>    struct my_object *intern = (struct my_object*)object; >>    free(intern->anything); >>    zend_object_std_dtor(&intern->zo TSRMLS_CC); >>    efree(intern); >> } >> >> zend_object_value my_object_new(zend_class_entry *class_type TSRMLS_DC) >> { >>    zend_object_value retval; >>    struct my_object* intern; >>    intern = emalloc(sizeof(struct my_oject)); >> >>    zend_object_std_init(&intern->zo, class_type TSRMLS_CC); >>    object_properties_init(&intern->zo, class_type); >> >>    intern->a_private_integer_not_accesible_from_php = 42; >>    intern->anything = some_data; >> >>    retval.handle = zend_objects_store_put(intern, NULL, my_object_free, NULL TSRMLS_CC); >> >>    return retval; >> } >> >> Mind, again, they are called from C and might need extern "C" binding. >> >> Then register this with the class during MINIT: >> >>    INIT_CLASS_ENTRY(my_class_entry, "my_class", my_functions); >>    my_class_entry.create_object = my_object_new; >>    my_class_entry = zend_register_internal_class(&my_class_entry TSRMLS_CC); >> >> And then, whenever you get the object you can cast from zend_object* to >> my_object* and access the private data. Check the PHP source and OO PECL >> exts for occurrences of"create_object" to find samples. >> >>> [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 >> >> :-) >> >> johannes >> >> >> -- >> PECL development discussion Mailing List (http://pecl.php.net/) >> To unsubscribe, visit: http://www.php.net/unsub.php >> > > -- > PECL development discussion Mailing List (http://pecl.php.net/) > To unsubscribe, visit: http://www.php.net/unsub.php > -- Pierre @pierrejoye | http://blog.thepimp.net | http://www.libgd.org