Re: [PATCH] Alias stream_context_get_default() as stream_context_set_default()

On Wed, Aug 6, 2008 at 15:11, Davey Shafik <davey@php.net> wrote: > On Aug 6, 2008, at 03:47 AM, Hannes Magnusson wrote: > >> On Wed, Aug 6, 2008 at 06:24, Davey Shafik <davey@php.net> wrote: >>> >>> OK, here's an attempt at a patch[1], I discussed it briefly with Johannes >>> and he felt some discussion was needed with regards to the return value. >>> >>> I personally seem some benefit to returning the "new" context; Johannes >>> wasn't sure that returning "true" might not be a better option in that >>> we >>> cannot return the previous "value" like ini_set(). >> >> Why can't we? >> If there was a default context then return it, otherwise true/false.. >> Hmh. That could be confusing. >> I'm fine returning the new context :) > > There is always a default context :) > >> >>> [1] >>> http://pixelated-dreams.com/~davey/stream_context_set_default.patch >> >> The arginfo is wrong, the parameter is required, not optional. > > I don't see this, but that's just me not knowing, I thought that anything > following > a | (pipe) was optional, otherwise it was implicitly required, > how do you make an argument explicitly required if that is not the case? Correct, but ext/reflaction doesn't read the zend_parse_parameters(), it reads the arginfos, so those have to match the zend_parse_parameters() statement. ZEND_BEGIN_ARG_INFO_EX(arginfo_stream_context_set_default, 0, 0, 0) should be ZEND_BEING_ARG_INF(arginfo_stream_context_set_default, 0) (meaning all ZEND_ARG_INFO() arguments are required) or ZEND_BEGIN_ARG_INFO_EX(arginfo_stream_context_set_default, 0, 0, 1) (meaning only the first ZEND_ARG_INFO() argument is required) -Hannes