clang-apply-replacements doesn't handle windows newlines correctly

[Nikola Smiljanic]

Done in r223750, I'll keep an eye on bots.

On Tue, Dec 9, 2014 at 11:37 AM, Nikola Smiljanic <popizdeh at gmail.com>
wrote:

> Yep, fails then passes. I guess you're right, I'll just diff the source
> file without copying it.
>
> On Tue, Dec 9, 2014 at 11:29 AM, Alexander Kornienko <alexfh at google.com>
> wrote:
>
>> I assume, the test breaks before your patch and passes with it? I'd give
>> it a try then.
>>
>> One nit:
>>
>> +// RUN: cp %S/Inputs/crlf/crlf.cpp.expected
>> %T/Inputs/crlf/crlf.cpp.expected
>>
>>
>> you don't seem to change the .expected file. Why copy it to a temporary
>> directory?
>>
>> Otherwise looks good.
>>
>> On Mon, Dec 8, 2014 at 12:54 PM, Nikola Smiljanic <popizdeh at gmail.com>
>> wrote:
>>
>>> We already have tests that rely on CRLF line endings in
>>> Sema/warn-documentation-crlf.c and FixIt/fixit-newline-style.c. They should
>>> be fine once they're in the repo, I'm just concerned that commit process
>>> and syncing with svn might mess it up. If the changes look good I could
>>> give it a try?
>>>
>>> On Mon, Dec 8, 2014 at 10:05 PM, Alexander Kornienko <alexfh at google.com>
>>> wrote:
>>>
>>>> I don't think we can rely on both git and svn deal correctly with
>>>> line-endings on all platforms =\ Can we generate a test file with needed
>>>> line endings during a test run?
>>>>
>>>> On Thu, Dec 4, 2014 at 6:09 AM, Nikola Smiljanic <popizdeh at gmail.com>
>>>> wrote:
>>>>
>>>>> This patch should have mixed newlines, I just hope Git doesn't convert
>>>>> them on commit.
>>>>>
>>>>> On Thu, Dec 4, 2014 at 1:43 PM, Nikola Smiljanic <popizdeh at gmail.com>
>>>>> wrote:
>>>>>
>>>>>> I think it'll only happen when you run it on Windows. Writing files
>>>>>> in text mode will translate each \n into CRLF on this silly platform :) I'm
>>>>>> honestly surprised that nobody noticed this before. I filed a bug for
>>>>>> clang-modernize for this
>>>>>> https://cpp11-migrate.atlassian.net/browse/CM-171
>>>>>>
>>>>>> On Thu, Dec 4, 2014 at 1:29 AM, Alexander Kornienko <
>>>>>> alexfh at google.com> wrote:
>>>>>>
>>>>>>> That may be a good solution, however, could you provide a bit more
>>>>>>> details: does this problem happen when you run clang-apply-replacements on
>>>>>>> Windows or on Linux or Mac as well? It would also be nice if we could have
>>>>>>> a test for this.
>>>>>>>
>>>>>>> On Wed, Dec 3, 2014 at 4:13 AM, Nikola Smiljanic <popizdeh at gmail.com
>>>>>>> > wrote:
>>>>>>>
>>>>>>>> I'm not entirely sure this is the right way to fix the issue, but
>>>>>>>> since SourceManager reads \r \n we should probably write them without
>>>>>>>> newline translation. What happens now is, newline is read as \r \n and when
>>>>>>>> it's written out the \n is translated so we end up with duplicated \r.
>>>>>>>>
>>>>>>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141209/6939c4bc/attachment.html>