Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

121 add line ending options #129

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions docs/fixers.rst
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,17 @@ using ``-f``/``--fix``. The ``--six-unicode`` and ``--future-unicode`` options
also disable fixers that are not applicable for those options.


Options affecting all fixers
----------------------------

Normally, output files are written with the usual line endings for the platform
that python-modernize is run on (LF for Unix / Mac OS X, or CRLF for Windows).

The ``-U``/``--unix-line-endings`` option writes Unix line endings regardless of
the curent platform. Similarly, the ``-W``/``--windows-line-endings`` option
writes Windows line endings regardless of the current platform.


Fixers requiring six
++++++++++++++++++++

Expand Down
30 changes: 30 additions & 0 deletions libmodernize/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from __future__ import absolute_import, print_function

import sys
import os
import logging
import optparse

Expand All @@ -28,6 +29,14 @@ def format_usage(usage):
return usage

def main(args=None):
old_os_linesep = os.linesep
try:
return actual_main(args)
finally:
if sys.version_info < (3, 0):
os.linesep = old_os_linesep
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are on Python 3, the monkeypatch is never removed, which will make the tests do different things depending on what order they run in.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Python 3 we never change os.linesep. Also, the monkeypatch to refactor._to_system_newlines should not cause any nondeterminism in the tests because it is always done prior to calling into lib2to3, and is idempotent. (Also, it's only affecting lib2to3, unlike os.linesep which can affect other things.) Nevertheless, I can unpatch refactor._to_system_newlines if you think that would be an improvement.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a test runs with --windows-line-endings, it will monkeypatch refactor._to_system_newlines to convert everything to Windows newlines on write. Then any subsequent test which doesn't specify its newlines will still write files with the Windows style newlines. For most tests, that won't matter, but for instance if your test_mixed_to_native_line_endings ran immediately after test_unix_to_windows_line_endings, I think it would fail.

I'm just finishing up a PR to achieve the same thing without monkeypatching, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I'm wrong, there is nondeterminism and this does need to be unpatched. (The patched function captures the linesep passed into it, so it is not idempotent.)


def actual_main(args=None):
"""Main program.

Returns a suggested exit status (0, 1, 2).
Expand Down Expand Up @@ -63,6 +72,10 @@ def main(args=None):
"(only useful for Python 2.6+).")
parser.add_option("--no-six", action="store_true", default=False,
help="Exclude fixes that depend on the six package.")
parser.add_option("-U", "--unix-line-endings", action="store_true", default=False,
help="Write files with Unix (LF) line endings.")
parser.add_option("-W", "--windows-line-endings", action="store_true", default=False,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think these need short options, especially since we already have lowercase -w. Explicit is better than implicit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay.

help="Write files with Windows (CRLF) line endings.")

fixer_pkg = 'libmodernize.fixes'
avail_fixes = set(refactor.get_fixers_from_package(fixer_pkg))
Expand Down Expand Up @@ -94,6 +107,14 @@ def main(args=None):
if options.print_function:
flags["print_function"] = True

if options.unix_line_endings and options.windows_line_endings:
print("-U/--unix-line-endings and -W/--windows-line-endings options conflict.")
return 2
if options.unix_line_endings:
fix_line_endings('\n')
if options.windows_line_endings:
fix_line_endings('\r\n')

# Set up logging handler
level = logging.DEBUG if options.verbose else logging.INFO
logging.basicConfig(format='%(name)s: %(message)s', level=level)
Expand Down Expand Up @@ -145,3 +166,12 @@ def main(args=None):

# Return error status (0 if rt.errors is zero)
return int(bool(rt.errors))

def fix_line_endings(linesep):
if sys.version_info < (3, 0):
os.linesep = linesep
else:
def _to_system_newlines(s):
return s.replace(os.linesep, linesep)

refactor._to_system_newlines = _to_system_newlines
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure which I like less, changing an important value in the standard library or monkeypatching a private function in 2to3.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative approach is to do the refactor._to_system_newlines patch on Python 2 as well; I think that will work.

30 changes: 30 additions & 0 deletions tests/test_newlines.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
from __future__ import absolute_import

import os
from utils import check_on_input, expect_error
from test_fix_basestring import BASESTRING

TESTCASE = [x.encode('ascii') for x in BASESTRING]


def test_mixed_to_native_line_endings():
check_on_input(b'#\r\n' + TESTCASE[0],
(b'#\n' + TESTCASE[1]).replace(b'\n', os.linesep.encode('ascii')),
mode="b")

def test_windows_to_unix_line_endings():
check_on_input(TESTCASE[0].replace(b'\n', b'\r\n'),
TESTCASE[1],
extra_flags=['--unix-line-endings'],
mode="b")

def test_unix_to_windows_line_endings():
check_on_input(TESTCASE[0],
TESTCASE[1].replace(b'\n', b'\r\n'),
extra_flags=['--windows-line-endings'],
mode="b")

def test_options_conflict():
expect_error(TESTCASE[0],
extra_flags=['--unix-line-endings', '--windows-line-endings'],
mode="b")
38 changes: 29 additions & 9 deletions tests/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from libmodernize.main import main as modernize_main


def check_on_input(input_content, expected_content, extra_flags = []):
def check_on_input(input_content, expected_content, extra_flags=[], mode="t"):
"""
Check that input_content is fixed to expected_content, idempotently.

Expand All @@ -19,24 +19,44 @@ def check_on_input(input_content, expected_content, extra_flags = []):
tmpdirname = tempfile.mkdtemp()
try:
test_input_name = os.path.join(tmpdirname, "input.py")
with open(test_input_name, "wt") as input_file:
with open(test_input_name, "w" + mode) as input_file:
if mode == "b":
input_file = getattr(input_file, 'buffer', input_file)
input_file.write(input_content)

def _check(this_input_content, which_check):
modernize_main(extra_flags + ["-w", test_input_name])
ret = modernize_main(extra_flags + ["-w", test_input_name])
if ret != 0:
raise AssertionError("didn't expect to fail (returned %r)" % (ret,))

output_content = ""
with open(test_input_name, "rt") as output_file:
for line in output_file:
if line:
output_content += line
with open(test_input_name, "r" + mode) as output_file:
if mode == "b":
output_content = output_file.read()
else:
output_content = "".join([line for line in output_file if line])

if output_content != expected_content:
raise AssertionError("%s\nInput:\n%sOutput:\n%s\nExpecting:\n%s" %
raise AssertionError("%s\nInput:\n%s\nOutput:\n%s\nExpecting:\n%s" %
(which_check, this_input_content, output_content, expected_content))

_check(input_content, "output check failed")
if input_content != expected_content:
_check(expected_content, "idempotence check failed")
finally:
shutil.rmtree(tmpdirname)


def expect_error(input_content, extra_flags=[], mode="t"):
tmpdirname = tempfile.mkdtemp()
try:
test_input_name = os.path.join(tmpdirname, "input.py")
with open(test_input_name, "w" + mode) as input_file:
if mode == "b":
input_file = getattr(input_file, 'buffer', input_file)
input_file.write(input_content)

ret = modernize_main(extra_flags + ["-w", test_input_name])
if ret == 0:
raise AssertionError("didn't expect to succeed")
finally:
shutil.rmtree(tmpdirname)