[PATCH] RE: [cfe-commits] r173697 - FileCheck'ize and merge tests

[Benyei, Guy]

Thanks Dmitri,
I've fixed your comments.
About the test - I'm not exactly sure how to test this change - I'm not sure if committing a DOS-style file will work right. Any ideas?

Thanks
   Guy Benyei

-----Original Message-----
From: Dmitri Gribenko [mailto:gribozavr at gmail.com] 
Sent: Wednesday, February 06, 2013 15:44
To: Benyei, Guy
Cc: cfe-commits at cs.uiuc.edu; llvm-commits at cs.uiuc.edu
Subject: Re: [PATCH] RE: [cfe-commits] r173697 - FileCheck'ize and merge tests

On Wed, Feb 6, 2013 at 3:00 PM, Benyei, Guy <guy.benyei at intel.com> wrote:
> Hi all,
> --strict-whitespace also kills the dos to unix end of line canonicalization. This made 3 Clang LIT test fail on my Windows machine.
> Attached a patch that makes this flag affect the horizontal whitespaces only, as it's stated in the flag's description.

This totally makes sense to me.  Nits:

1. Please add to the flag description in docs/CommandGuide/FileCheck.rst "End-of-line sequences are canonicalized to UNIX-style '\r' in all modes."

2. Please remove trailing whitespace from comments in your patch.

3.

+/// CanonicalizeInputFile - Remove duplicate horizontal space and dos 
+style /// '\r' at end of linefrom the specified memory buffer, free it, 
+and return /// a new one.

Please remove function name from the comment.  It would also help to rephrase it like:

"Canonicalize whitespace in the input file.  Line endings are replaced with UNIX-style '\r'.

\param PreserveHorizontal Don't squash consecutive horizontal whitespace characters into a single space."

4. Please add a test to test/FileCheck/

Dmitri

--
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/
---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dos_eol_removal2.patch
Type: application/octet-stream
Size: 2684 bytes
Desc: dos_eol_removal2.patch
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130206/5eb6092b/attachment.obj>