Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:41722
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 66.249.92.168 as permitted sender)
DomainKey-Status: bad
DomainKey-Signature: a=rsa-sha1; c=nofws;
        d=gmail.com; s=gamma;
        h=message-id:date:from:sender:to:subject:in-reply-to:mime-version
         :content-type:references:x-google-sender-auth;
        b=N1GUf8jIYj7O3vqoIjwiYKwjCiYPkuKMJXcBK/o75cQdMZ7dudHH/Q9slcEezkRSTH
         tLUve7eKrP9BKGOT9W4bR3c+jWyO7t/DMXKdjieH3g+dEuo+EZGhrVLf8NsyavrONQPf
         ifb0t62neoTPA7DfI6O8m4YSqv0Rp6MdsXkiQ=
Message-ID: <2a9adcf0811060552u7486262bua509bde4d9807ff9@mail.gmail.com>
Date: Thu, 6 Nov 2008 05:52:47 -0800
Sender: jmertic@gmail.com
To: internals@lists.php.net
In-Reply-To: <2a9adcf0810221428v29a612a1i5425cbce95acf4a@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; 
	boundary="----=_Part_26007_10198235.1225979568056"
References: <2a9adcf0810221003i70f37558ob5777b5603865cdf@mail.gmail.com>
	 <1224696974.17075.1.camel@localhost>
	 <2a9adcf0810221135h1e6ed4ecub55318cfb26b3e7e@mail.gmail.com>
	 <370571A4-8A89-412C-A8CD-1D7439C761F1@prohost.org>
	 <2a9adcf0810221428v29a612a1i5425cbce95acf4a@mail.gmail.com>
Subject: Re: [PHP-DEV] [PATCH] Bug #46367 - fputcsv does not add the correct newline character on Windows
From: jmertic@php.net ("John Mertic")

------=_Part_26007_10198235.1225979568056
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

On Wed, Oct 22, 2008 at 1:28 PM, John Mertic <jmertic@php.net> wrote:
> On Wed, Oct 22, 2008 at 1:16 PM, Ilia Alshanetsky <ilia@prohost.org> wrote:
>> You cannot use smart_str_appendc() in this case, since the EOL could be a 2
>> byte string "\r\n".
>>
>> smart_str_appendl(&csvline, PHP_EOL, sizeof(PHP_EOL)-1); should be used
>> here.
>>
>> On 22-Oct-08, at 2:35 PM, John Mertic wrote:
>>
>>> Hi Lars,
>>>
>>> Thanks for the pointers, updated the patch and added a test.
>>>
>>> Index: file.c
>>> ===================================================================
>>> RCS file: /repository/php-src/ext/standard/file.c,v
>>> retrieving revision 1.530
>>> diff -u -r1.530 file.c
>>> --- file.c      21 Oct 2008 22:06:48 -0000      1.530
>>> +++ file.c      22 Oct 2008 18:21:10 -0000
>>> @@ -2104,7 +2104,7 @@
>>>                }
>>>        }
>>>
>>> -       smart_str_appendc(&csvline, '\n');
>>> +       smart_str_appendc(&csvline, PHP_EOL);
>>>        smart_str_0(&csvline);
>>>
>>>        ret = php_stream_write(stream, csvline.c, csvline.len);
>>>
>>>
>>> John Mertic
>>> jmertic@gmail.com
>>> http://jmertic.wordpress.com
>>>
>>> "Explaining a joke is like dissecting a frog: you understand it
>>> better, but the frog dies
>>> in the process." --Mark Twain
>>>
>>>
>>>
>>> On Wed, Oct 22, 2008 at 10:36 AM, Lars Strojny <lstrojny@php.net> wrote:
>>>>
>>>> Hi John,
>>>>
>>>>
>>>> Am Mittwoch, den 22.10.2008, 10:03 -0700 schrieb John Mertic:
>>>> [...]
>>>>>
>>>>> Below is a patch to fix this issue; it uses the constant PHP_EOL to
>>>>> get the correct newline to use on the current platform:
>>>>
>>>> Thanks for your patch. A few things to mention, as it is your first
>>>> patch: please use "diff -ru" to create unified diffs. Also we try to
>>>> always add tests for the things we fix or create. Would you mind
>>>> creating a test for the fix you sent to make sure no regression happens
>>>> in the next n years?
>>>>
>>>> Thanks, Lars
>>>> --
>>>>  Jabber: lars@strojny.net
>>>>  Weblog: http://usrportage.de
>>>>
>>>
>>> --
>>> PHP Internals - PHP Runtime Development Mailing List
>>> To unsubscribe, visit: http://www.php.net/unsub.php
>>
>> Ilia Alshanetsky
>>
>>
>>
>>
>>
>
> Hi Ilia,
>
> I can see your point about scanner's having trouble with different EOL
> characters, but I know in our use case the problem is that the CSV
> files created will come out the lines all mashed together, which is a
> problem for readability of the file. And since most CSV scanners know
> how to deal with Windows, I would think we are pretty safe ;-)
>
> Attached is an updated patch that uses this function instead.
>
> --
> John Mertic
> jmertic@gmail.com | http://jmertic.wordpress.com
>

Hi All,

I haven't seen any further comments on this bug; does it seem safe to
commit or should it wait? I've re-attached the patch and test again
for everyone's reference.

Thanks!

John Mertic
jmertic@gmail.com | http://jmertic.wordpress.com

------=_Part_26007_10198235.1225979568056
Content-Type: application/force-download; name=file.c.patch
Content-Transfer-Encoding: base64
X-Attachment-Id: f_fn7gk53m1
Content-Disposition: attachment; filename=file.c.patch

SW5kZXg6IGZpbGUuYwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09
PT09PT09PT09PT09PT09PT09PT09PT09ClJDUyBmaWxlOiAvcmVwb3NpdG9yeS9waHAtc3JjL2V4
dC9zdGFuZGFyZC9maWxlLmMsdgpyZXRyaWV2aW5nIHJldmlzaW9uIDEuNTMwCmRpZmYgLXUgLXIx
LjUzMCBmaWxlLmMKLS0tIGZpbGUuYwkyMSBPY3QgMjAwOCAyMjowNjo0OCAtMDAwMAkxLjUzMAor
KysgZmlsZS5jCTIyIE9jdCAyMDA4IDIxOjIxOjQyIC0wMDAwCkBAIC0yMTA0LDcgKzIxMDQsNyBA
QAogCQl9CiAJfQogCi0Jc21hcnRfc3RyX2FwcGVuZGMoJmNzdmxpbmUsICdcbicpOworCXNtYXJ0
X3N0cl9hcHBlbmRsKCZjc3ZsaW5lLCBQSFBfRU9MLCBzaXplb2YoUEhQX0VPTCkpOwogCXNtYXJ0
X3N0cl8wKCZjc3ZsaW5lKTsKIAogCXJldCA9IHBocF9zdHJlYW1fd3JpdGUoc3RyZWFtLCBjc3Zs
aW5lLmMsIGNzdmxpbmUubGVuKTsKCg==
------=_Part_26007_10198235.1225979568056--