Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:24386
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Date: Sat, 15 Jul 2006 12:24:37 +0200
Reply-To: Marcus Boerger <helly@php.net>
Message-ID: <271761716.20060715122437@marcus-boerger.de>
To: Matt W <php_lists@realplain.com>
Cc: internals@lists.php.net
In-Reply-To: <00a201c6a667$147d5c00$0201a8c0@pc1>
References: <009a01c69848$06f22e80$0201a8c0@pc1> <009f01c69cf9$6b493f30$0201a8c0@pc1> <1772356126.20060701125346@marcus-boerger.de> <008701c6a014$ec07ebd0$0201a8c0@pc1> <56141794.20060706101753@marcus-boerger.de> <00fe01c6a4c7$909ad070$0201a8c0@pc1> <1924756948.20060711203719@marcus-boerger.de> <00a201c6a667$147d5c00$0201a8c0@pc1>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] [PATCH] array_fill: Allow an array to specify keys
From: helly@php.net (Marcus Boerger)

Hello Matt,

Thursday, July 13, 2006, 12:28:27 PM, you wrote:

> Hi Marcus,

> ----- Original Message ----- 
> From: "Marcus Boerger"


>> Hello Matt,
>>
>> Tuesday, July 11, 2006, 10:54:05 AM, you wrote:
>>
>> > Hi Marcus,
>>
>> > I'm replying again for clarification about the patch.  When you first
>> > replied and said it looked OK, you mentioned "once we agree to this,"
> which
>> > I assumed meant it could be used/committed.  I saw array.c hasn't been
>> > changed in CVS (though I don't know when that would happen anyway), so I
> was
>> > kinda wondering, that's all. :-)
>>
>> Well a common problem, you would probably get feedback if you would commit
>> it. Since you don't have CVS rights somebody else needs to do that job but
>> risks to get blamed...so you need to find more people that have interest
>> in your patch.

> Heh, understandable.  Can anyone like me get a CVS account if we request it?
> (I see those request messages often...)  Or do you have to be more "known?"
> :-)  Well, even with rights, I'd want to verify a change was OK
> beforehand -- it sounds like you're saying stuff should be commited *first*
> and then feedback comes...

>> > Then about the tests, I still didn't know whether you'd tell me to make
> a
>> > tests file *after* committing the patch or if you need it *first*.
> Sorry.
>> > :-/
>>
>> First of course. You can do "cvs add ext/standard/tests/array_xyz.phpt" to
>> add your new test file "array_xyz.phpt" virtually to cvs. After that you
>> can regenerate the patch with "cvs di -updN" where -N ensures that those
>> tests go into the patch.

> Well, I just tried to use (with WinCvs) "cvs add ..." and it said:

> "add" requires write access to the repository

> :-(

>> > If you (meaning anyone who'd apply the patch) are just waiting for
> tests,
>> > please let me know, and I'll create tests for all array_fill()
> functionality
>> > (old and new) right away!
>>
>> > BTW, since sending the patches, I realized that my code will allow an
> empty
>> > "keys" array.  I'm thinking there should be a warning for that to make
> it
>> > consistent with the old behavior of not returning an empty array.
> Should I
>> > change that and RE-send patches...?
>>
>> Yep, do so please.

> I created array_fill_keys() instead after seeing Andi's e-mail questioning
> the array_fill() modification, and made a few simple tests for it (I see
> array_fill tests were recently added).  So, it's all ready to go, but I
> can't add the test file...  What should I do, e-mail it as a plain file?

There's two ways. First as you mentioned sending them as plain text files.
And second editing the CVS/Entries file in that directory and adding this
line: "/array_fill_keys.phpt/0/dummy timestamp//".

However that's for your next patches, i added the function to HEAD and 5.2
after experimenting with it and mofifying it a tiny bit. I allow empty
arrays as that is easier for user code. Also this is inline with the
ability of array_fill() to specify parameter num as zero.

Best regards,
 Marcus