Re: [Parseperl-discuss] [PATCH] preserve newlines

[Chris D. <ch...@ch...>]

Yes, I understand the point of generated content.  I thought about  
that.  It occurred to me that the tokenizer could watch all (well,  
most) of the line endings and store the newline that has the  
plurality as the document newline.  Then, Node.pm (or Normal.pm?)  
could have a set_newline method that traverses the tokens and changes  
the newline.  Generated code could use that value too.

As for the unit tests, I checked and all the files in PPI, and none  
have \r anywhere except inc/Test/ClassAPI.pm.  I've written a bunch  
of mixed "\r\n","\n" regression tests for Perl::Critic that could be  
repurposed for PPI.

Chris

On Oct 4, 2006, at 10:26 PM, Adam Kennedy wrote:

> Also, remembering back to my original decision, there were also  
> some other factors.
>
> For example, parsing is one thing, but what happens when you start  
> generating content? What newline do you use then?
>
> We have to remember the newline scheme (if clean) or somehow make a  
> decision on what to use if messy.
>
> We can't actually insert using \n any more, we need to use some per- 
> document value... $doc->newline or something.
>
> It makes a lot of things messier than they should be.
>
> A lot of that mess isn't covered in the unit tests, because I  
> solved the whole problem by localising newlines, so the tests  
> didn't matter.
>
> Adam K
>
> Adam Kennedy wrote:
>> I'm quite nervous about this one, if only because of the problem  
>> that nobody downstream can use \n either (which is partly why I  
>> localised the newlines in the first place).
>> The look ahead to first end-line isn't an option, it fails in too  
>> many cases.
>> I might be able to be convinced though, but we need find a way to  
>> mitigate the regex soup effect.
>> Maybe if we provided \015{1,2}\012|\012|\015 as an export (as  
>> $XPNL or something) that might make life a little simpler.
>> Adam K
>> Chris Dolan wrote:
>>> Here's another slightly controversial patch that I don't feel  
>>> comfortable just committing without a nod from Adam:
>>>
>>> This patch changes PPI to preserve line endings instead of  
>>> localizing them.  The code passes all PPI and Perl::Critic  
>>> tests.  I implemented this change to enable a new Perl::Critic  
>>> policy, CodeLayout::RequireConsistentNewlines, which is intended  
>>> to help with code signing problems.
>>>
>>> There are still a few un-fixed methods that are not exercised by  
>>> the test suite, like PPI::Token::Pod::merge().  These need to be  
>>> fixed before this patch could be committed.  Additionally, the  
>>> PPI.pm docs need to be updated to reflect this change.
>>>
>>> In the process, I added two new API methods:   
>>> PPI::Node::line_ending() and PPI::Token::HereDoc::Terminator_line().
>>>
>>> This patch will collide a tiny bit with my "$_" patch, but it  
>>> should be easy to repair (I'm happy to do the legwork on that).
>>>
>>>  ***
>>>
>>> This implementation simply changes all "\n" checks to "\015{1,2} 
>>> \012|\012|\015" checks.  An alternative implementation would be  
>>> for the tokenizer to look ahead to the first line ending, store  
>>> that value, and use it throughout tokenization.  That  
>>> implementation would be more like what text editors do, but less  
>>> like Perl (I think).
>>>
>>> Chris
>>>
>>> -- 
>>> Chris Dolan, Software Developer, http://www.chrisdolan.net/
>>> Public key: http://www.chrisdolan.net/public.key
>>> vCard: http://www.chrisdolan.net/ChrisDolan.vcf
>>>
>>>
>>>
>>>
>>> -------------------------------------------------------------------- 
>>> ----
>>>
>>> -------------------------------------------------------------------- 
>>> -----
>>> Take Surveys. Earn Cash. Influence the Future of IT
>>> Join SourceForge.net's Techsay panel and you'll get the chance to  
>>> share your
>>> opinions on IT & business topics through brief surveys -- and  
>>> earn cash
>>> http://www.techsay.com/default.php? 
>>> page=join.php&p=sourceforge&CID=DEVDEV
>>>
>>>
>>> -------------------------------------------------------------------- 
>>> ----
>>>
>>> _______________________________________________
>>> Parseperl-discuss mailing list
>>> Par...@li...
>>> https://lists.sourceforge.net/lists/listinfo/parseperl-discuss
>> --------------------------------------------------------------------- 
>> ----
>> Take Surveys. Earn Cash. Influence the Future of IT
>> Join SourceForge.net's Techsay panel and you'll get the chance to  
>> share your
>> opinions on IT & business topics through brief surveys -- and earn  
>> cash
>> http://www.techsay.com/default.php? 
>> page=join.php&p=sourceforge&CID=DEVDEV
>> _______________________________________________
>> Parseperl-discuss mailing list
>> Par...@li...
>> https://lists.sourceforge.net/lists/listinfo/parseperl-discuss
>

-- 
Chris Dolan, Software Developer, Clotho Advanced Media Inc.
608-294-7900, fax 294-7025, 1435 E Main St, Madison WI 53703
vCard: http://www.chrisdolan.net/ChrisDolan.vcf

Clotho Advanced Media, Inc. - Creators of MediaLandscape Software  
(http://www.media-landscape.com/) and partners in the revolutionary  
Croquet project (http://www.opencroquet.org/)