Re: svn: /php/php-src/ branches/PHP_5_4/Zend/zend_compile.c trunk/Zend/zend_compile.c

Hi, 2011/11/18 Stefan Marr <php@stefan-marr.de>: > Hi Felipe: > > On 18 Nov 2011, at 14:56, Felipe Pena wrote: > >> felipe                                   Fri, 18 Nov 2011 13:56:41 +0000 >> >> Revision: ®xÚwéÿÔs‹€ >> €http://svn.php.net/viewvc?view=revision&revision=319487 >> >> -     else if (!strncmp(mname, ZEND_DESTRUCTOR_FUNC_NAME,  mname_len)) {     >>  ce->destructor  = fe; fe->common.fn_flags |= ZEND_ACC_DTOR; } >> -     else if (!strncmp(mname, ZEND_GET_FUNC_NAME,         mname_len)) ce->__get >>       = fe; >> -     else if (!strncmp(mname, ZEND_SET_FUNC_NAME,         mname_len)) ce->__set >>       = fe; >> -     else if (!strncmp(mname, ZEND_CALL_FUNC_NAME,        mname_len)) ce->__call >>      = fe; >> -     else if (!strncmp(mname, ZEND_UNSET_FUNC_NAME,       mname_len)) ce->__unset >>     = fe; >> -     else if (!strncmp(mname, ZEND_ISSET_FUNC_NAME,       mname_len)) ce->__isset >>     = fe; >> -     else if (!strncmp(mname, ZEND_CALLSTATIC_FUNC_NAME,  mname_len)) >> ce->__callstatic= fe; >> -     else if (!strncmp(mname, ZEND_TOSTRING_FUNC_NAME,    mname_len)) >> ce->__tostring  = fe; >> -     else if (ce->name_length + 1 == mname_len) { >> +     } else if (!strncmp(mname, ZEND_DESTRUCTOR_FUNC_NAME,  mname_len)) { >> +             ce->destructor = fe; fe->common.fn_flags |= ZEND_ACC_DTOR; >> +     } else if (!strncmp(mname, ZEND_GET_FUNC_NAME, mname_len)) { >> +             ce->__get = fe; >> +     } else if (!strncmp(mname, ZEND_SET_FUNC_NAME, mname_len)) { >> +             ce->__set = fe; >> +     } else if (!strncmp(mname, ZEND_CALL_FUNC_NAME, mname_len)) { >> +             ce->__call = fe; >> +     } else if (!strncmp(mname, ZEND_UNSET_FUNC_NAME, mname_len)) { >> +             ce->__unset = fe; >> +     } else if (!strncmp(mname, ZEND_ISSET_FUNC_NAME, mname_len)) { >> +             ce->__isset = fe; >> +     } else if (!strncmp(mname, ZEND_CALLSTATIC_FUNC_NAME, mname_len)) { >> +             ce->__callstatic = fe; >> +     } else if (!strncmp(mname, ZEND_TOSTRING_FUNC_NAME, mname_len)) { >> +             ce->__tostring = fe; >> +     } else if (ce->name_length + 1 == mname_len) { > > How is that change an improvement for readability? > The previous formatting was an implicit tabled arrangement, which makes it very easy to see > what the commonalities are and where the code differs. > Breaking that up into multiple lines, does IMHO not improve readability or clarity. If you just > missed the braces, well, they could have been added inline, no? > And I do not really buy that 'CS' (that is coding style?) applies to such a special > piece of code. > > Would it be a problem to revert that change, or at least 'table' it again? CS = Coding standards Well, our codind standards in the CODING_STANDARDS file states: -------8<----------------------------------------------------------------------------------------------------------- 3. Be generous with whitespace and braces. Keep one empty line between the variable declaration section and the statements in a block, as well as between logical statement groups in a block. Maintain at least one empty line between two functions, preferably two. Always prefer:: if (foo) { bar; } to: if(foo)bar; -------8<----------------------------------------------------------------------------------------------------------- So I just did it in conforming whole rest of the code. It's not a question of readability, but the following an adopted standards. -- Regards, Felipe Pena