Closed GoogleCodeExporter closed 9 years ago
To be sure, what platform are you running IWYU on when this happens? Unix,
right?
Original comment by kim.gras...@gmail.com
on 15 Mar 2013 at 5:21
Unix is right. But our source is unfortunately with Win endings. That's why I
noticed the problem.
Original comment by andre.st...@gmail.com
on 15 Mar 2013 at 5:41
I attached a patch which improves fix_include to first detect the line ending
and writes the fixed file accordingly.
Original comment by andre.st...@gmail.com
on 18 Mar 2013 at 10:07
Attachments:
Thank you! I assume it works in your CRLF-on-Unix environment? I've tried it
with LF-on-Windows and it works well there.
I think it's fine that we get unspecified line endings if the file has mixed
line endings to begin with.
Could you extract the line-ending detection out into its own function and wrap
the body in a try-finally block to make sure f is closed, e.g.
def _DetectLineEndings(filename):
# Find out which file ending is used first. The
# first lines indicate the line ending for the whole file
# so pathological files with mixed endings aren't handled properly!
f = open(filename, 'U')
try:
while f.newlines is None:
if f.readline() == '':
break
return f.newlines if f.newlines != None and type(f.newlines) is not tuple else '\n'
finally:
f.close()
Original comment by kim.gras...@gmail.com
on 19 Mar 2013 at 1:45
You're welcome. I made the changes as you suggested.
Cheers,
André
Original comment by andre.st...@gmail.com
on 20 Mar 2013 at 2:27
Attachments:
Oh, just noticed -- we have a hard 80-column limit for both Python and C++
code. I can reformat that for you before committing, just FYI if you want to
make more changes.
Thanks for your contribution, I'll commit it as soon we get sign-off from
project admin.
Original comment by kim.gras...@gmail.com
on 20 Mar 2013 at 8:20
Okay thanks. I'll have that 80-column limit in mind for my next contribution ;)
And of course thanks to all contributors for an awesome IWYU! Didn't find
anything comparable in the C++ world.
Original comment by andre.st...@gmail.com
on 21 Mar 2013 at 1:20
Patch looks good to me. Thanks André for your contribution.
Original comment by vsap...@gmail.com
on 24 Mar 2013 at 10:45
Committed as r460, thank you!
André, could you update to HEAD and make sure the problem is solved, and I can
close the issue. Thanks!
Original comment by kim.gras...@gmail.com
on 25 Mar 2013 at 6:18
I can confirm that the problem is solved on HEAD. Thanks for merging!
Original comment by andre.st...@gmail.com
on 25 Mar 2013 at 8:10
Original comment by kim.gras...@gmail.com
on 25 Mar 2013 at 12:45
Original issue reported on code.google.com by
andre.st...@gmail.com
on 15 Mar 2013 at 4:29