Remove whitespaces of whitespace-only files (#3348)
Currently, empty and whitespace-only (with or without newlines) are not modified. In some discussions (issues and pull requests) consensus was to reformat whitespace-only files to empty or single-character files, preserving line endings when possible. With that said, this commit introduces the following behaviors: * Empty files are left as is * Whitespace-only files (no newline) reformat into empty files * Whitespace-only files (1 or more newlines) reformat into a single newline character To implement these changes, we moved the initial check at `format_file_contents` that raises `NothingChanged` if the source (with no whitespaces) is an empty string. In the case of *.ipynb files, `format_ipynb_string` checks a similar condition and removed whitespaces. In the case of Python files, `format_str_once` includes a check on the output that returns the correct newline character if possible or an empty string otherwise. Signed-off-by: Antonio Ossa Guerra <aaossa@uc.cl>
This commit is contained in:
parent
c23a5c15aa
commit
d97b7898b3
@ -15,6 +15,8 @@
|
|||||||
<!-- Changes that affect Black's preview style -->
|
<!-- Changes that affect Black's preview style -->
|
||||||
|
|
||||||
- Enforce empty lines before classes and functions with sticky leading comments (#3302)
|
- Enforce empty lines before classes and functions with sticky leading comments (#3302)
|
||||||
|
- Reformat empty and whitespace-only files as either an empty file (if no newline is
|
||||||
|
present) or as a single newline character (if a newline is present) (#3348)
|
||||||
- Implicitly concatenated strings used as function args are now wrapped inside
|
- Implicitly concatenated strings used as function args are now wrapped inside
|
||||||
parentheses (#3307)
|
parentheses (#3307)
|
||||||
- Correctly handle trailing commas that are inside a line's leading non-nested parens
|
- Correctly handle trailing commas that are inside a line's leading non-nested parens
|
||||||
|
@ -917,7 +917,7 @@ def format_file_contents(src_contents: str, *, fast: bool, mode: Mode) -> FileCo
|
|||||||
valid by calling :func:`assert_equivalent` and :func:`assert_stable` on it.
|
valid by calling :func:`assert_equivalent` and :func:`assert_stable` on it.
|
||||||
`mode` is passed to :func:`format_str`.
|
`mode` is passed to :func:`format_str`.
|
||||||
"""
|
"""
|
||||||
if not src_contents.strip():
|
if not mode.preview and not src_contents.strip():
|
||||||
raise NothingChanged
|
raise NothingChanged
|
||||||
|
|
||||||
if mode.is_ipynb:
|
if mode.is_ipynb:
|
||||||
@ -1014,6 +1014,9 @@ def format_ipynb_string(src_contents: str, *, fast: bool, mode: Mode) -> FileCon
|
|||||||
Operate cell-by-cell, only on code cells, only for Python notebooks.
|
Operate cell-by-cell, only on code cells, only for Python notebooks.
|
||||||
If the ``.ipynb`` originally had a trailing newline, it'll be preserved.
|
If the ``.ipynb`` originally had a trailing newline, it'll be preserved.
|
||||||
"""
|
"""
|
||||||
|
if mode.preview and not src_contents:
|
||||||
|
raise NothingChanged
|
||||||
|
|
||||||
trailing_newline = src_contents[-1] == "\n"
|
trailing_newline = src_contents[-1] == "\n"
|
||||||
modified = False
|
modified = False
|
||||||
nb = json.loads(src_contents)
|
nb = json.loads(src_contents)
|
||||||
@ -1106,6 +1109,13 @@ def _format_str_once(src_contents: str, *, mode: Mode) -> str:
|
|||||||
dst_contents = []
|
dst_contents = []
|
||||||
for block in dst_blocks:
|
for block in dst_blocks:
|
||||||
dst_contents.extend(block.all_lines())
|
dst_contents.extend(block.all_lines())
|
||||||
|
if mode.preview and not dst_contents:
|
||||||
|
# Use decode_bytes to retrieve the correct source newline (CRLF or LF),
|
||||||
|
# and check if normalized_content has more than one line
|
||||||
|
normalized_content, _, newline = decode_bytes(src_contents.encode("utf-8"))
|
||||||
|
if "\n" in normalized_content:
|
||||||
|
return newline
|
||||||
|
return ""
|
||||||
return "".join(dst_contents)
|
return "".join(dst_contents)
|
||||||
|
|
||||||
|
|
||||||
|
6
tests/data/preview/whitespace.py
Normal file
6
tests/data/preview/whitespace.py
Normal file
@ -0,0 +1,6 @@
|
|||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
# output
|
@ -25,6 +25,7 @@
|
|||||||
List,
|
List,
|
||||||
Optional,
|
Optional,
|
||||||
Sequence,
|
Sequence,
|
||||||
|
Type,
|
||||||
TypeVar,
|
TypeVar,
|
||||||
Union,
|
Union,
|
||||||
)
|
)
|
||||||
@ -153,6 +154,34 @@ def test_empty_ff(self) -> None:
|
|||||||
os.unlink(tmp_file)
|
os.unlink(tmp_file)
|
||||||
self.assertFormatEqual(expected, actual)
|
self.assertFormatEqual(expected, actual)
|
||||||
|
|
||||||
|
@patch("black.dump_to_file", dump_to_stderr)
|
||||||
|
def test_one_empty_line(self) -> None:
|
||||||
|
mode = black.Mode(preview=True)
|
||||||
|
for nl in ["\n", "\r\n"]:
|
||||||
|
source = expected = nl
|
||||||
|
assert_format(source, expected, mode=mode)
|
||||||
|
|
||||||
|
def test_one_empty_line_ff(self) -> None:
|
||||||
|
mode = black.Mode(preview=True)
|
||||||
|
for nl in ["\n", "\r\n"]:
|
||||||
|
expected = nl
|
||||||
|
tmp_file = Path(black.dump_to_file(nl))
|
||||||
|
if system() == "Windows":
|
||||||
|
# Writing files in text mode automatically uses the system newline,
|
||||||
|
# but in this case we don't want this for testing reasons. See:
|
||||||
|
# https://github.com/psf/black/pull/3348
|
||||||
|
with open(tmp_file, "wb") as f:
|
||||||
|
f.write(nl.encode("utf-8"))
|
||||||
|
try:
|
||||||
|
self.assertFalse(
|
||||||
|
ff(tmp_file, mode=mode, write_back=black.WriteBack.YES)
|
||||||
|
)
|
||||||
|
with open(tmp_file, "rb") as f:
|
||||||
|
actual = f.read().decode("utf8")
|
||||||
|
finally:
|
||||||
|
os.unlink(tmp_file)
|
||||||
|
self.assertFormatEqual(expected, actual)
|
||||||
|
|
||||||
def test_experimental_string_processing_warns(self) -> None:
|
def test_experimental_string_processing_warns(self) -> None:
|
||||||
self.assertWarns(
|
self.assertWarns(
|
||||||
black.mode.Deprecated, black.Mode, experimental_string_processing=True
|
black.mode.Deprecated, black.Mode, experimental_string_processing=True
|
||||||
@ -971,8 +1000,8 @@ def err(msg: str, **kwargs: Any) -> None:
|
|||||||
)
|
)
|
||||||
|
|
||||||
def test_format_file_contents(self) -> None:
|
def test_format_file_contents(self) -> None:
|
||||||
empty = ""
|
|
||||||
mode = DEFAULT_MODE
|
mode = DEFAULT_MODE
|
||||||
|
empty = ""
|
||||||
with self.assertRaises(black.NothingChanged):
|
with self.assertRaises(black.NothingChanged):
|
||||||
black.format_file_contents(empty, mode=mode, fast=False)
|
black.format_file_contents(empty, mode=mode, fast=False)
|
||||||
just_nl = "\n"
|
just_nl = "\n"
|
||||||
@ -990,6 +1019,17 @@ def test_format_file_contents(self) -> None:
|
|||||||
black.format_file_contents(invalid, mode=mode, fast=False)
|
black.format_file_contents(invalid, mode=mode, fast=False)
|
||||||
self.assertEqual(str(e.exception), "Cannot parse: 1:7: return if you can")
|
self.assertEqual(str(e.exception), "Cannot parse: 1:7: return if you can")
|
||||||
|
|
||||||
|
mode = black.Mode(preview=True)
|
||||||
|
just_crlf = "\r\n"
|
||||||
|
with self.assertRaises(black.NothingChanged):
|
||||||
|
black.format_file_contents(just_crlf, mode=mode, fast=False)
|
||||||
|
just_whitespace_nl = "\n\t\n \n\t \n \t\n\n"
|
||||||
|
actual = black.format_file_contents(just_whitespace_nl, mode=mode, fast=False)
|
||||||
|
self.assertEqual("\n", actual)
|
||||||
|
just_whitespace_crlf = "\r\n\t\r\n \r\n\t \r\n \t\r\n\r\n"
|
||||||
|
actual = black.format_file_contents(just_whitespace_crlf, mode=mode, fast=False)
|
||||||
|
self.assertEqual("\r\n", actual)
|
||||||
|
|
||||||
def test_endmarker(self) -> None:
|
def test_endmarker(self) -> None:
|
||||||
n = black.lib2to3_parse("\n")
|
n = black.lib2to3_parse("\n")
|
||||||
self.assertEqual(n.type, black.syms.file_input)
|
self.assertEqual(n.type, black.syms.file_input)
|
||||||
@ -1281,8 +1321,51 @@ def test_reformat_one_with_stdin_and_existing_path(self) -> None:
|
|||||||
report.done.assert_called_with(expected, black.Changed.YES)
|
report.done.assert_called_with(expected, black.Changed.YES)
|
||||||
|
|
||||||
def test_reformat_one_with_stdin_empty(self) -> None:
|
def test_reformat_one_with_stdin_empty(self) -> None:
|
||||||
|
cases = [
|
||||||
|
("", ""),
|
||||||
|
("\n", "\n"),
|
||||||
|
("\r\n", "\r\n"),
|
||||||
|
(" \t", ""),
|
||||||
|
(" \t\n\t ", "\n"),
|
||||||
|
(" \t\r\n\t ", "\r\n"),
|
||||||
|
]
|
||||||
|
|
||||||
|
def _new_wrapper(
|
||||||
|
output: io.StringIO, io_TextIOWrapper: Type[io.TextIOWrapper]
|
||||||
|
) -> Callable[[Any, Any], io.TextIOWrapper]:
|
||||||
|
def get_output(*args: Any, **kwargs: Any) -> io.TextIOWrapper:
|
||||||
|
if args == (sys.stdout.buffer,):
|
||||||
|
# It's `format_stdin_to_stdout()` calling `io.TextIOWrapper()`,
|
||||||
|
# return our mock object.
|
||||||
|
return output
|
||||||
|
# It's something else (i.e. `decode_bytes()`) calling
|
||||||
|
# `io.TextIOWrapper()`, pass through to the original implementation.
|
||||||
|
# See discussion in https://github.com/psf/black/pull/2489
|
||||||
|
return io_TextIOWrapper(*args, **kwargs)
|
||||||
|
|
||||||
|
return get_output
|
||||||
|
|
||||||
|
mode = black.Mode(preview=True)
|
||||||
|
for content, expected in cases:
|
||||||
|
output = io.StringIO()
|
||||||
|
io_TextIOWrapper = io.TextIOWrapper
|
||||||
|
|
||||||
|
with patch("io.TextIOWrapper", _new_wrapper(output, io_TextIOWrapper)):
|
||||||
|
try:
|
||||||
|
black.format_stdin_to_stdout(
|
||||||
|
fast=True,
|
||||||
|
content=content,
|
||||||
|
write_back=black.WriteBack.YES,
|
||||||
|
mode=mode,
|
||||||
|
)
|
||||||
|
except io.UnsupportedOperation:
|
||||||
|
pass # StringIO does not support detach
|
||||||
|
assert output.getvalue() == expected
|
||||||
|
|
||||||
|
# An empty string is the only test case for `preview=False`
|
||||||
output = io.StringIO()
|
output = io.StringIO()
|
||||||
with patch("io.TextIOWrapper", lambda *args, **kwargs: output):
|
io_TextIOWrapper = io.TextIOWrapper
|
||||||
|
with patch("io.TextIOWrapper", _new_wrapper(output, io_TextIOWrapper)):
|
||||||
try:
|
try:
|
||||||
black.format_stdin_to_stdout(
|
black.format_stdin_to_stdout(
|
||||||
fast=True,
|
fast=True,
|
||||||
|
Loading…
Reference in New Issue
Block a user