On 01/30/2014 08:08 PM, Christopher Jones wrote:
>
>
> On 01/30/2014 10:55 AM, Stefan Neufeind wrote:
>> Hi,
>>
>> On 01/30/2014 07:08 PM, Christopher Jones wrote:
>>>
>>> On 01/30/2014 01:11 AM, Stefan Neufeind wrote:
>>>> On 01/12/2014 09:40 PM, Johannes Schlüter wrote:
>>>>> On Sat, 2014-01-11 at 20:15 +0100, Stefan Neufeind wrote:
>>>>>> -static int php_do_open_temporary_file(const char *path, const char
>>>>>> *pfx, char **opened_path_p TSRMLS_DC)
>>>>>> +static int php_do_open_temporary_file(const char *path, const char
>>>>>> *pfx, char **opened_path_p TSRMLS_DC, const char *suffix)
>>>>>
>>>>> TSRM should be the last parameter, move your new one in front of it.
>>>>>
>>>>>> -PHPAPI int php_open_temporary_fd_ex(const char *dir, const char
>>>>>> *pfx, char **opened_path_p, zend_bool open_basedir_check TSRMLS_DC)
>>>>>> +PHPAPI int php_open_temporary_fd_ex(const char *dir, const char
>>>>>> *pfx, char **opened_path_p, zend_bool open_basedir_check TSRMLS_DC,
>>>>>> const char *suffix)
>>>>>
>>>>>> -PHPAPI int php_open_temporary_fd(const char *dir, const char *pfx,
>>>>>> char **opened_path_p TSRMLS_DC)
>>>>>> +PHPAPI int php_open_temporary_fd(const char *dir, const char *pfx,
>>>>>> char **opened_path_p TSRMLS_DC, const char *suffix)
>>>>>
>>>>>> -PHPAPI FILE *php_open_temporary_file(const char *dir, const char
>>>>>> *pfx, char **opened_path_p TSRMLS_DC)
>>>>>> +PHPAPI FILE *php_open_temporary_file(const char *dir, const char
>>>>>> *pfx, char **opened_path_p TSRMLS_DC, const char *suffix)
>>>>> [...]
>>>>>
>>>>> These are API changes so it could only be in 5.6.0. Eventually it
>>>>> might
>>>>> be added if instead of changing APIs new functions might be introduced
>>>>> and the old ones call the new ones.
>>>>> (also TSRM should be last last there, too)
>>>>>
>>>>> Aside for that: A test would be good.
>>>>>
>>>>> For not loosing it either create a feature request at bugs.php.net or
>>>>> pull request at github.
>>>>
>>>> Hi,
>>>>
>>>> I've sent a reply to Ferenc and David some days ago. But maybe somebody
>>>> else could also help get this forward a bit.
>>>>
>>>> RFC is here, together with a pull-request:
>>>> https://wiki.php.net/rfc/tempnam-suffix
>>>>
>>>> I also tried creating a testcase. For some reason it fails - but from
>>>> the commandline I don't see why. I guess it's because my
>>>> ".png"-example
>>>> fails to validate against the expected result or something?
>>>> The actual output of the script for testing is fine for me though.
>>>>
>>>> Would be awesome if somebody could pick this up please.
>>>
>>> The RFC needs some basic info, e.g. is there a default for the
>>> optional suffix? What are any potential porting issues for users
>>> (or for the implementation)? Also, make it easy for readers by
>>> giving an
>>> example of what happens now and what happens with the new
>>> implementation.
>>
>> Well, it's an additional option. If you don't give it, it behaves as
>> before. And as commonly with a suffix: If you don't specify one, you
>> don't get a suffix.
>>
>> Sorry I don't know what is unclear. But maybe it's just an
>> experience-thing how "explicitly" you usually handle such things in an
>> RFC.
>
> Prior PHP RFCs are not the best examples of RFCs in the world. Feel free
> to do better!
>
> One rule would be if something has to be explained on the mail list, then
> it should be in the RFC.
>
> Also, from experience, RFCs should state what they do, and also what they
> don't do. It's better not to leave any gray areas.
>
> Don't forget to have content that can be reused for the PHP manual
> (See #8 in
> https://blogs.oracle.com/opal/entry/the_mysterious_php_rfc_process).
I'll shortly try to write at least a small bit more. Thanks for your
pointers.
>>> Having a failing testcase isn't going to help tempt someone to spend
>>> time merging the code. Perhaps you can revisit this and get it
>>> working? (Or state that if the RFC is accepted, you will do the work
>>> to get it working). Are any existing tests broken by the change?
>>
>> I tried to find out, but didn't manage to get a "verbose" output of the
>> testcase-runner on the console. It just said "failed" for me but didn't
>> give me the returned / expected output or so. So it's more a question of
>> "how to get it right". The testcode in the test itself works. I suspect
>> a notation-thing that for somebody experienced will be peanuts.
>
> I'd never take that for granted. It will take some fiddling, I'm sure.
> I can understand you might not want to spend time on it if there is a
> possibility the RFC isn't going to be accepted. However I don't see
> your RFC as something that would fail to pass a vote.
>
> Check the .log, .exp files etc from the test run and post questions
> on the mail list.
>
>>
>> I currently run it with:
>> TESTS=ext/standard/tests/file/tempnam_suffix.phpt make test
>
> That should be fine.
Thanks to Gernot Vormayr it works now. "--show-all" was what I missed.
Passing it as part of "TESTS" was what shows the small glitch.
Test working as well now.
Kind regards,
Stefan