search

HaveF / include-what-you-use

Automatically exported from code.google.com/p/include-what-you-use
Other
0 stars 0 forks source link

fix_includes.py should preserve line endings #95

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
Currently all source code files are saved always with Unix line endings after 
patching the include lines. Instead the original line endings should be 
preserved.

Original issue reported on code.google.com by andre.st...@gmail.com on 15 Mar 2013 at 4:29

GoogleCodeExporter commented 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

GoogleCodeExporter commented 9 years ago 
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

GoogleCodeExporter commented 9 years ago 
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:

GoogleCodeExporter commented 9 years ago 
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

GoogleCodeExporter commented 9 years ago 
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:

GoogleCodeExporter commented 9 years ago 
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

GoogleCodeExporter commented 9 years ago 
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

GoogleCodeExporter commented 9 years ago 
Patch looks good to me. Thanks André for your contribution.

Original comment by vsap...@gmail.com on 24 Mar 2013 at 10:45

GoogleCodeExporter commented 9 years ago 
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

GoogleCodeExporter commented 9 years ago 
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

GoogleCodeExporter commented 9 years ago

Original comment by kim.gras...@gmail.com on 25 Mar 2013 at 12:45