Re: Re: plua merged into lua

Ah and an additional comment: I won't go to "stable" directly but release as beta first and (try to) get some people to test. johannes On Mon, 2011-08-15 at 23:34 +0200, Johannes Schlüter wrote: > Hi, > > I'm unfortunately missing the time to go through this properly but to > answer at least a bit more than I did on IRC a few comments: > > In php_lua.h line 90-110: > This code is dependent on C99 and some compilers. Even when more code I > prefer having the long version in the code instead of these non-portable > shortcuts. > Additionally one should use php_error_docref() this formats the error a > bit nicer and can directly link to the documentation. Also: why is > lua_error() using E_USER_ERROR not E_ERROR? The USER ones are meant to > be used from userspace. > > In lua.c I do not understand why you need the LUA_G(instances) table. > The individual instances should be stored in the object (as is > lua_obj->L = L; ) and manged via that object. > > In lua.c I see that Lua::register seems to only accept global PHP > functions as a callback. Is there no way to attach custom data to the > lua closure, so we can store zend function pointers (as returned by the > "f" modifier to zend_parse_parameters) so object methods and closures > work, too? > Related to that I also see the usage of a property _callbacks. If the > information is needed it should probably stored in the object structure, > this actually might be what is needed for what i said before. Secodly if > the property is kept it is strange: The class Lua is marked as final, > while the property is set to protected. This combination doesn't really > make sense. Maybe it should be private. Secondly it is static, while > callbacks are set by object instance. > > In lua.c php_lua_send_zval_to_lua's case IS_ARRAY should have a > recursion protection. (see HashTable.nApplyCount, recursion is easy: $a > = array(); $a[] &= $a; ) > > In lua.c php_lua_send_zval_to_lua I'm not sure about the array > conversion. This will only work for objects making use of "normal" > properties. It will cause trouble with private/protected properties (You > get \0-bytes in the result). Maybe it makes sense to create an interface > like this: > interface LuaConvertable { public function getElements() } > which returns a list of all properties and their name and value as they > should be exported. If not one has to think about iterators and using > get_property object handlers. > > In lua.c PHP_METHOD(lua, call) - why does this function exist? I think > it should be an alias in the function table (PHP_MALIAS). I also think > call should be the primary name and __call the alias to it. > > In lua.c in php_lua_get_zval_from_lua, if a TTABLE is returned and the > key is neither numeric nor a string there should be a warning for the > user. Maybe it also makes sense not to use key = > php_lua_get_zval_from_lua(L, -2 TSRMLS_CC); but a specific handling > routine. > > In lua.c proto Lua::getVersion() should be renamed from getversion to > getVersion ;-) > > In lua.c Lua::register probably should be renamed to something like > Lua::registerCallback. > > In lua.c all(?) PHP_METHODs: After a failure in zend_parse_parameters > (zpp) do a simple "return;" not WRONG_PARAM_COUNT. zpp will already > report an error, way more precise than WRONG_PARAM_COUNT. > > As an additional comment, not tooo relevant here: the php_{$extname}.h > header should be short and contain little more than the > phpext_{$extname}_ptr as this file is included from > main/internal_functions.c along with all other extension's headers when > doing a static build and therefore increases compile times and increases > the risk of symbol conflicts (especially when including "random" library > headers) which can be avoided. > > > johannes > > On Thu, 2011-08-11 at 11:48 +0800, Laruence wrote: > > Hi: > > johanees, do you think it's the time to make a release now? > > (the issues you metioned last time: version $Id, I will subtitute > > it before release); > > > > > > thanks > > > > 2011/8/6 Andreas Streichardt <andreas.streichardt@googlemail.com>: > > > Hi! > > > I just had a quick look at the code and it looks very good and way more > > > professional than my code :) > > > So i would say: release it. I will update a few links i set to my extension > > > to the pecl homepage. > > > Kind regards, > > > Andreas Streichardt > > > > > > > > > Laruence <laruence@php.net> hat geschrieben: > > > > > > > > > Hi: > > > if I can get your agreement before next sunday(14 Aug), > > > I will make the first relase with version 1.0.. > > > > > > alos doc is in process. > > > > > > thanks > > > > > > 2011/8/4 Laruence <laruence@php.net>: > > >> Hi: > > >> > > >> I have merged plua src into lua, add add package2.xm to build pecl > > >> package. > > >> > > >> could you plz review my codes when you are free? > > >> > > >> thanks > > >> > > >> -- > > >> Laruence Xinchen Hui > > >> http://www.laruence.com/ > > >> > > > > > > > > > > > > -- > > > Laruence Xinchen Hui > > > http://www.laruence.com/ > > > > > > > > > >