Re: [PHP-DEV] Re: Proposal: Introduce a new macro:php_error_docref_ex()
Why not leave it as it is? I mean, really, those five strokes? Wherever it's going, I'm -1 on changing the meaning of the existing macro. On 11 Feb 2015 16:01, reeze re...@php.net wrote: Hi, On 11 February 2015 at 19:15, Dmitry Stogov dmi...@zend.com wrote: Hi, I don't think it'll improve something, and may complicate merging from PHP5 (this is not a big issue, because merging is already not simple). Yes it just makes code looks a little better and cleaner. I didn't realized the merging problem. that might affect diffs which changes php_error_docref() In any case, the names should be swapped, to be used as: php_error_docref(E_WARNING, recursion detected); or php_error_docref_ex(NULL, E_WARNING, recursion detected); Yes I prefer this too. But considering the merging problem, I think I should hold this idea of update all of those calls. We may provider a simple version with default null docref for external extesions, but I didn't have proper name for that. also macros with __VA_ARGS__ may be not portable, s it should be implemented differently. Yes I should use ##__VA_ARGS__ instead. Thanks. Dmitry. On Wed, Feb 11, 2015 at 11:41 AM, Xinchen Hui larue...@php.net wrote: Hey: On Wed, Feb 11, 2015 at 4:35 PM, Yasuo Ohgaki yohg...@ohgaki.net wrote: Hi all, On Wed, Feb 11, 2015 at 5:21 PM, reeze re...@php.net wrote: I think it is a cleanup, it works but there are too many duplication. it is the time to make the code clean and easier to understand. I've never used other than NULL also. Everyone is going to remove TSRM macros, it's right time to clean up. fine, if you want to change.. please, _ex means extending, so the name should not be php_error_docref_ex... thanks Regards, -- Yasuo Ohgaki yohg...@ohgaki.net -- Xinchen Hui @Laruence http://www.laruence.com/ -- Reeze Xia http://reeze.cn
[PHP-DEV] Re: Proposal: Introduce a new macro:php_error_docref_ex()
Hi all, On Wed, Feb 11, 2015 at 8:15 PM, Dmitry Stogov dmi...@zend.com wrote: I don't think it'll improve something, and may complicate merging from PHP5 (this is not a big issue, because merging is already not simple). In any case, the names should be swapped, to be used as: php_error_docref(E_WARNING, recursion detected); or php_error_docref_ex(NULL, E_WARNING, recursion detected); also macros with __VA_ARGS__ may be not portable, s it should be implemented differently. It's even better. +1 Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
[PHP-DEV] Re: Proposal: Introduce a new macro:php_error_docref_ex()
Hi, On 11 February 2015 at 19:15, Dmitry Stogov dmi...@zend.com wrote: Hi, I don't think it'll improve something, and may complicate merging from PHP5 (this is not a big issue, because merging is already not simple). Yes it just makes code looks a little better and cleaner. I didn't realized the merging problem. that might affect diffs which changes php_error_docref() In any case, the names should be swapped, to be used as: php_error_docref(E_WARNING, recursion detected); or php_error_docref_ex(NULL, E_WARNING, recursion detected); Yes I prefer this too. But considering the merging problem, I think I should hold this idea of update all of those calls. We may provider a simple version with default null docref for external extesions, but I didn't have proper name for that. also macros with __VA_ARGS__ may be not portable, s it should be implemented differently. Yes I should use ##__VA_ARGS__ instead. Thanks. Dmitry. On Wed, Feb 11, 2015 at 11:41 AM, Xinchen Hui larue...@php.net wrote: Hey: On Wed, Feb 11, 2015 at 4:35 PM, Yasuo Ohgaki yohg...@ohgaki.net wrote: Hi all, On Wed, Feb 11, 2015 at 5:21 PM, reeze re...@php.net wrote: I think it is a cleanup, it works but there are too many duplication. it is the time to make the code clean and easier to understand. I've never used other than NULL also. Everyone is going to remove TSRM macros, it's right time to clean up. fine, if you want to change.. please, _ex means extending, so the name should not be php_error_docref_ex... thanks Regards, -- Yasuo Ohgaki yohg...@ohgaki.net -- Xinchen Hui @Laruence http://www.laruence.com/ -- Reeze Xia http://reeze.cn
[PHP-DEV] Re: Proposal: Introduce a new macro:php_error_docref_ex()
Hi,
On 12 February 2015 at 15:30, Dmitry Stogov dmi...@zend.com wrote:
We don't use macros with variable number of arguments in PHP.
this is not portable.
We could don't have to use variadic macros, we could try something like:
*+PHPAPI void php_error_doc0(int type, const char *format, ...)*
*+{*
*+* *va_list args;*
*+*
*+* *va_start(args, format);*
*+* *php_error_docref0(NULL, type, format, args);*
*+* *va_end(args);*
*+}*
*+*
*+#define php_error_doc php_error_doc0*
*+*
Thanks. Dmitry.
On Wed, Feb 11, 2015 at 6:01 PM, reeze re...@php.net wrote:
Hi,
On 11 February 2015 at 19:15, Dmitry Stogov dmi...@zend.com wrote:
Hi,
I don't think it'll improve something, and may complicate merging from
PHP5 (this is not a big issue, because merging is already not simple).
Yes it just makes code looks a little better and cleaner.
I didn't realized the merging problem. that might affect diffs which
changes php_error_docref()
In any case, the names should be swapped, to be used as:
php_error_docref(E_WARNING, recursion detected);
or
php_error_docref_ex(NULL, E_WARNING, recursion detected);
Yes I prefer this too.
But considering the merging problem, I think I should hold this idea of
update all of those calls.
We may provider a simple version with default null docref for external
extesions, but I didn't have proper name for that.
also macros with __VA_ARGS__ may be not portable, s it should be
implemented differently.
Yes I should use ##__VA_ARGS__ instead.
Thanks. Dmitry.
On Wed, Feb 11, 2015 at 11:41 AM, Xinchen Hui larue...@php.net wrote:
Hey:
On Wed, Feb 11, 2015 at 4:35 PM, Yasuo Ohgaki yohg...@ohgaki.net
wrote:
Hi all,
On Wed, Feb 11, 2015 at 5:21 PM, reeze re...@php.net wrote:
I think it is a cleanup, it works but there are too many
duplication. it
is the time to make the code clean and easier to understand.
I've never used other than NULL also.
Everyone is going to remove TSRM macros, it's right time to clean up.
fine, if you want to change..
please, _ex means extending,
so the name should not be php_error_docref_ex...
thanks
Regards,
--
Yasuo Ohgaki
yohg...@ohgaki.net
--
Xinchen Hui
@Laruence
http://www.laruence.com/
--
Reeze Xia
http://reeze.cn
--
Reeze Xia
http://reeze.cn
[PHP-DEV] Re: Proposal: Introduce a new macro:php_error_docref_ex()
PS: Github search result: https://github.com/search?l=cq=php_error_docrefref=searchresultstype=Codeutf8=%E2%9C%93 It seems that no one need the docref at all. On 11 February 2015 at 16:07, reeze re...@php.net wrote: Hi all, There are a lot of code use php_error_docref() macro, the first parameter mostly is NULL, before PHP7, it looks like: *php_error_docref(NULL TSRML, E_WARNING, recursion detected);* in PHP7 *php_error_docref(NULL, E_WARNING, recursion detected);* looks better, but the first parameter look dumb. I did a simple statics, there are: Null docref: 2530 Not Null docref: 51 I searched on Github, it seems that almost all of the extension use NULL docref. So I propose add a new macro, like: php_error_error_docref_ex(E_WARNING, xxx). this could make code looks better and the extension maintainer's easier. Another option would be just update the php_error_docref() macro to remove the docref parameter, default to NULL but not add a new macro. What do you think about it? [1] https://github.com/php/php-src/pull/1075 -- Reeze Xia http://reeze.cn -- Reeze Xia http://reeze.cn
[PHP-DEV] Re: Proposal: Introduce a new macro:php_error_docref_ex()
On Wed, Feb 11, 2015 at 4:07 PM, reeze re...@php.net wrote: Hi all, There are a lot of code use php_error_docref() macro, the first parameter mostly is NULL, before PHP7, it looks like: php_error_docref(NULL TSRML, E_WARNING, recursion detected); in PHP7 php_error_docref(NULL, E_WARNING, recursion detected); looks better, but the first parameter look dumb. I did a simple statics, there are: Null docref: 2530 Not Null docref: 51 I searched on Github, it seems that almost all of the extension use NULL docref. So I propose add a new macro, like: php_error_error_docref_ex(E_WARNING, xxx). this could make code looks better and the extension maintainer's easier. Another option would be just update the php_error_docref() macro to remove the docref parameter, default to NULL but not add a new macro. What do you think about it? No. I can't see any benefit from it, but lots of changes. everything works well now. thanks [1] https://github.com/php/php-src/pull/1075 -- Reeze Xia http://reeze.cn -- Xinchen Hui @Laruence http://www.laruence.com/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: Proposal: Introduce a new macro:php_error_docref_ex()
I think it is a cleanup, it works but there are too many duplication. it is the time to make the code clean and easier to understand. I myself don't know why should I need the first NULL when I am a newcomer of writing extensions, it just seems everyone do that, before PHP7 there is a strange TSRML_CC macro, it is hard to maintain. On 11 February 2015 at 16:16, Xinchen Hui larue...@php.net wrote: On Wed, Feb 11, 2015 at 4:07 PM, reeze re...@php.net wrote: Hi all, There are a lot of code use php_error_docref() macro, the first parameter mostly is NULL, before PHP7, it looks like: php_error_docref(NULL TSRML, E_WARNING, recursion detected); in PHP7 php_error_docref(NULL, E_WARNING, recursion detected); looks better, but the first parameter look dumb. I did a simple statics, there are: Null docref: 2530 Not Null docref: 51 I searched on Github, it seems that almost all of the extension use NULL docref. So I propose add a new macro, like: php_error_error_docref_ex(E_WARNING, xxx). this could make code looks better and the extension maintainer's easier. Another option would be just update the php_error_docref() macro to remove the docref parameter, default to NULL but not add a new macro. What do you think about it? No. I can't see any benefit from it, but lots of changes. everything works well now. thanks [1] https://github.com/php/php-src/pull/1075 -- Reeze Xia http://reeze.cn -- Xinchen Hui @Laruence http://www.laruence.com/ -- Reeze Xia http://reeze.cn
[PHP-DEV] Re: Proposal: Introduce a new macro:php_error_docref_ex()
Hi all, On Wed, Feb 11, 2015 at 5:21 PM, reeze re...@php.net wrote: I think it is a cleanup, it works but there are too many duplication. it is the time to make the code clean and easier to understand. I've never used other than NULL also. Everyone is going to remove TSRM macros, it's right time to clean up. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
[PHP-DEV] Re: Proposal: Introduce a new macro:php_error_docref_ex()
Hey: On Wed, Feb 11, 2015 at 4:35 PM, Yasuo Ohgaki yohg...@ohgaki.net wrote: Hi all, On Wed, Feb 11, 2015 at 5:21 PM, reeze re...@php.net wrote: I think it is a cleanup, it works but there are too many duplication. it is the time to make the code clean and easier to understand. I've never used other than NULL also. Everyone is going to remove TSRM macros, it's right time to clean up. fine, if you want to change.. please, _ex means extending, so the name should not be php_error_docref_ex... thanks Regards, -- Yasuo Ohgaki yohg...@ohgaki.net -- Xinchen Hui @Laruence http://www.laruence.com/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] Re: Proposal: Introduce a new macro:php_error_docref_ex()
Hi, I don't think it'll improve something, and may complicate merging from PHP5 (this is not a big issue, because merging is already not simple). In any case, the names should be swapped, to be used as: php_error_docref(E_WARNING, recursion detected); or php_error_docref_ex(NULL, E_WARNING, recursion detected); also macros with __VA_ARGS__ may be not portable, s it should be implemented differently. Thanks. Dmitry. On Wed, Feb 11, 2015 at 11:41 AM, Xinchen Hui larue...@php.net wrote: Hey: On Wed, Feb 11, 2015 at 4:35 PM, Yasuo Ohgaki yohg...@ohgaki.net wrote: Hi all, On Wed, Feb 11, 2015 at 5:21 PM, reeze re...@php.net wrote: I think it is a cleanup, it works but there are too many duplication. it is the time to make the code clean and easier to understand. I've never used other than NULL also. Everyone is going to remove TSRM macros, it's right time to clean up. fine, if you want to change.. please, _ex means extending, so the name should not be php_error_docref_ex... thanks Regards, -- Yasuo Ohgaki yohg...@ohgaki.net -- Xinchen Hui @Laruence http://www.laruence.com/
[PHP-DEV] Re: Proposal: Introduce a new macro:php_error_docref_ex()
We don't use macros with variable number of arguments in PHP. this is not portable. Thanks. Dmitry. On Wed, Feb 11, 2015 at 6:01 PM, reeze re...@php.net wrote: Hi, On 11 February 2015 at 19:15, Dmitry Stogov dmi...@zend.com wrote: Hi, I don't think it'll improve something, and may complicate merging from PHP5 (this is not a big issue, because merging is already not simple). Yes it just makes code looks a little better and cleaner. I didn't realized the merging problem. that might affect diffs which changes php_error_docref() In any case, the names should be swapped, to be used as: php_error_docref(E_WARNING, recursion detected); or php_error_docref_ex(NULL, E_WARNING, recursion detected); Yes I prefer this too. But considering the merging problem, I think I should hold this idea of update all of those calls. We may provider a simple version with default null docref for external extesions, but I didn't have proper name for that. also macros with __VA_ARGS__ may be not portable, s it should be implemented differently. Yes I should use ##__VA_ARGS__ instead. Thanks. Dmitry. On Wed, Feb 11, 2015 at 11:41 AM, Xinchen Hui larue...@php.net wrote: Hey: On Wed, Feb 11, 2015 at 4:35 PM, Yasuo Ohgaki yohg...@ohgaki.net wrote: Hi all, On Wed, Feb 11, 2015 at 5:21 PM, reeze re...@php.net wrote: I think it is a cleanup, it works but there are too many duplication. it is the time to make the code clean and easier to understand. I've never used other than NULL also. Everyone is going to remove TSRM macros, it's right time to clean up. fine, if you want to change.. please, _ex means extending, so the name should not be php_error_docref_ex... thanks Regards, -- Yasuo Ohgaki yohg...@ohgaki.net -- Xinchen Hui @Laruence http://www.laruence.com/ -- Reeze Xia http://reeze.cn
