Preserve line separator format (Unix vs. Windows)

[petsuter]

I tried running python-modernize on Windows on source files that use Unix line separators. This changed them over to Windows line separators. That's not very helpful. A diff now shows every single line as changed. The previous line separator format should be preserved.

[takluyver]

Or maybe we should modernise them all to Unix style ;-)

We'll need to look at 2to3 and see how much code we'd need to duplicate in order to fix this ourselves. If it doesn't require duplicating too much, it should be OK.

[forivall]

For now, you can set your .gitattributes file appropriately so that it ignores line ending type changes and always commits them as unix line separators.

[techtonik]

Most likely that 2to3 need to be opened in binary mode for writing. https://github.com/python/cpython/blob/master/Lib/lib2to3/refactor.py#L527 (write_file)

[daira]

The implementation is different depending on whether python-modernize is running on Python 2 or 3: https://github.com/python/cpython/blob/master/Lib/lib2to3/refactor.py#L113 . On Python 2 at least, setting os.linesep to "\n" will make it use Unix newlines when writing, without any duplication of code.

The Right Thing is to preserve the line-ending style of each file, but that looks significantly more difficult to do, unless we detect the line-ending style ourselves and set os.linesep accordingly.

[techtonik]

@daira why not just read input file as binary? You will get the strings, but without any newline transformations.

[techtonik]

So, it looks like subclassing of lib2to3.RefactoringTool is required.

[takluyver]

2to3 already opens the file as binary, but it normalises the line endings to Unix style before processing the code, and then converts to platform native when writing it again. That presumably means that Windows style line endings can't be passed through 2to3's machinery, or 2to3 wouldn't bother normalising them.

We'd have to subclass RefactoringTool, duplicate _read_python_source, refactor_file, processed_file and write_file to detect and pass through the original newlines. I think the extra code outweighs the benefits, since it's easy to convert back to your preferred line endings after running modernize.

[techtonik]

It is easy to do once, but not every single time. So, how about hack to read the linefeed stats when "-w" is supplied and rewrite of the file after processing?

[takluyver]

It is easy to do once, but not every single time.

That's what scripting is for.

[daira]

As I pointed out, setting os.linesep (as an option, say --linesep=unix or --linesep=windows) may be sufficient to make the existing implementation do what we want.

[Edit: the monkey-patch below is probably better, if it works.]

[petsuter]

It's easy to do, but too much code? Everyone should script it, but doing it right once from the beginning is not worth it? :(

[daira]

I think this should work (untested):

from lib2to3 import refactor

def _identity(obj):
    return obj

refactor._from_system_newlines = _identity
refactor._to_system_newlines = _identity

if sys.version_info >= (3, 0):
    # Force newline mode to '', i.e. 
    # * on input, "universal newline mode is enabled, but line endings 
    #   are returned to the caller untranslated";
    # * on output, "no translation takes place".

    def _open_with_encoding(file, mode='r', buffering=-1, encoding=None, 
                            errors=None, newline=None, closefd=True):
        return open(file, mode=mode, buffering=buffering, encoding=encoding,
                    errors=errors, newline='', closefd=closefd)

    refactor._open_with_encoding = _open_with_encoding

[daira]

Hmm, not sure that will do the right thing on Windows for lines created by a fixer, though.

[takluyver]

I suspect that 2to3's parser expects Unix style newlines, otherwise it wouldn't bother normalising them in the first place. That could be a red herring, though.

[daira]

I used the os.linesep approach for the pull request. I believe this should also work on Python 3 based on the API docs; we'll see what Travis says.

[daira]

Nope, setting os.linesep doesn't work on Python 3, despite the docs implying that it should. I've run out of time to work on this now; perhaps someone else can have a go.

[daira]

I had another idea. Fixed now -- please review.

[techtonik]

https://github.com/python/cpython/blob/master/Lib/lib2to3/refactor.py#L540 explicitly converts file to system-specific newlines without any option to turn this off! What is the ill logic behind that?

[daira]

Hmm, although the approach I used in #132 works, subclassing RefactoringTool has the advantage that we can also log which files changed, as needed for #127. So I'm leaning toward that approach now.

[daira]

I think we should use something like the auto-detection method in @techtonik's patch, rather than the -line-endings options. (@techtonik makes a good argument that they look like fixer options and someone might expect them to operate for files that are not otherwise fixed.) I see how to do that (and how to test it) now; I'll have time to work on it tomorrow this weekend.

[techtonik]

@daira you can take my patch and work on top of it.

[daira]

Actually it looks as though I won't have time to do this before I go on holiday on Thursday (until the 28th November). So someone else should probably look at it.