On Wed, Oct 22, 2008 at 1:16 PM, Ilia Alshanetsky <[EMAIL PROTECTED]> 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 >> [EMAIL PROTECTED] >> 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 <[EMAIL PROTECTED]> 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: [EMAIL PROTECTED] >>> 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 [EMAIL PROTECTED] | http://jmertic.wordpress.com
file.c.patch
Description: application/force-download
-- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
