From 348eed2649d9eb40c1f732dbf163f33a582a3834 Mon Sep 17 00:00:00 2001 From: Antonio Ossa Guerra Date: Thu, 29 Sep 2022 12:44:04 -0300 Subject: [PATCH 01/10] Add option to skip the first line in source file This commit adds a CLi option to skip the first line in the source files, just like the Cpython command line allows [1]. By enabling the flag, using `-x` or `--skip-source-first-line`, the first line is removed temporarilly while the remaining contents are formatted. The first line is added back before returning the formatted output. [1]: https://docs.python.org/dev/using/cmdline.html#cmdoption-x Signed-off-by: Antonio Ossa Guerra --- src/black/__init__.py | 14 ++++++++++++++ src/black/mode.py | 2 ++ 2 files changed, 16 insertions(+) diff --git a/src/black/__init__.py b/src/black/__init__.py index 5b8c9749119..009462e617f 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -248,6 +248,12 @@ def validate_regex( ), default=[], ) +@click.option( + "-x", + "--skip-source-first-line", + is_flag=True, + help="Skip the first line of the source code.", +) @click.option( "-S", "--skip-string-normalization", @@ -428,6 +434,7 @@ def main( # noqa: C901 pyi: bool, ipynb: bool, python_cell_magics: Sequence[str], + skip_source_first_line: bool, skip_string_normalization: bool, skip_magic_trailing_comma: bool, experimental_string_processing: bool, @@ -528,6 +535,7 @@ def main( # noqa: C901 line_length=line_length, is_pyi=pyi, is_ipynb=ipynb, + skip_source_first_line=skip_source_first_line, string_normalization=not skip_string_normalization, magic_trailing_comma=not skip_magic_trailing_comma, experimental_string_processing=experimental_string_processing, @@ -904,6 +912,10 @@ def format_file_contents(src_contents: str, *, fast: bool, mode: Mode) -> FileCo if not src_contents.strip(): raise NothingChanged + header = sep = "" + if mode.skip_source_first_line: + header, sep, src_contents = src_contents.partition("\n") + if mode.is_ipynb: dst_contents = format_ipynb_string(src_contents, fast=fast, mode=mode) else: @@ -914,6 +926,8 @@ def format_file_contents(src_contents: str, *, fast: bool, mode: Mode) -> FileCo if not fast and not mode.is_ipynb: # Jupyter notebooks will already have been checked above. check_stability_and_equivalence(src_contents, dst_contents, mode=mode) + + dst_contents = header + sep + dst_contents return dst_contents diff --git a/src/black/mode.py b/src/black/mode.py index 6c0847e8bcc..e3c36450ed1 100644 --- a/src/black/mode.py +++ b/src/black/mode.py @@ -170,6 +170,7 @@ class Mode: string_normalization: bool = True is_pyi: bool = False is_ipynb: bool = False + skip_source_first_line: bool = False magic_trailing_comma: bool = True experimental_string_processing: bool = False python_cell_magics: Set[str] = field(default_factory=set) @@ -208,6 +209,7 @@ def get_cache_key(self) -> str: str(int(self.string_normalization)), str(int(self.is_pyi)), str(int(self.is_ipynb)), + str(int(self.skip_source_first_line)), str(int(self.magic_trailing_comma)), str(int(self.experimental_string_processing)), str(int(self.preview)), From 92297b62a628a973975a036afc5a7ad1bbf52fcc Mon Sep 17 00:00:00 2001 From: Antonio Ossa Guerra Date: Thu, 29 Sep 2022 13:10:23 -0300 Subject: [PATCH 02/10] Add tests for `--skip-source-first-line` option When the flag is disabled (default), black formats the entire source file, as in every line. In the other hand, if the flag is enabled, by using `-x` or `--skip-source-first-line`, the first line is retained while the rest of the source is formatted and then is added back. These tests use an empty Python file that contains invalid syntax in its first line (`invalid_header.py`, at `miscellaneous/`). First, Black is invoked without enabling the flag which should result in an exit code different than 0. When the flag is enabled, Black is expected to return a successful exit code and the header is expected to be retained (even if its not valid Python syntax). Signed-off-by: Antonio Ossa Guerra --- tests/data/miscellaneous/invalid_header.py | 1 + tests/test_black.py | 14 ++++++++++++++ 2 files changed, 15 insertions(+) create mode 100644 tests/data/miscellaneous/invalid_header.py diff --git a/tests/data/miscellaneous/invalid_header.py b/tests/data/miscellaneous/invalid_header.py new file mode 100644 index 00000000000..c0936ad4d80 --- /dev/null +++ b/tests/data/miscellaneous/invalid_header.py @@ -0,0 +1 @@ +This is not valid Python syntax diff --git a/tests/test_black.py b/tests/test_black.py index abd4d00b8e8..14dce4474e5 100644 --- a/tests/test_black.py +++ b/tests/test_black.py @@ -341,6 +341,20 @@ def test_string_quotes(self) -> None: black.assert_equivalent(source, not_normalized) black.assert_stable(source, not_normalized, mode=mode) + def test_skip_source_first_line(self) -> None: + source, _ = read_data("miscellaneous", "invalid_header") + tmp_file = Path(black.dump_to_file(source)) + # Full source should fail (invalid syntax at header) + self.invokeBlack([str(tmp_file), "--diff", "--check"], exit_code=123) + # So, skipping the first line should work + result = BlackRunner().invoke( + black.main, [str(tmp_file), "-x", f"--config={EMPTY_CONFIG}"] + ) + self.assertEqual(result.exit_code, 0) + with open(tmp_file, encoding="utf8") as f: + actual = f.read() + self.assertFormatEqual(source.partition("\n")[0], actual.partition("\n")[0]) + def test_skip_magic_trailing_comma(self) -> None: source, _ = read_data("simple_cases", "expression") expected, _ = read_data( From 73eac33526e649be470ddf7d29ecf7a749d42993 Mon Sep 17 00:00:00 2001 From: Antonio Ossa Guerra Date: Thu, 29 Sep 2022 13:01:05 -0300 Subject: [PATCH 03/10] Support skip source first line option for blackd The recently added option can be added as an acceptable header for blackd. The arguments are passed in such a way that using the new header will activate the skip source first line behaviour as expected Signed-off-by: Antonio Ossa Guerra --- src/blackd/__init__.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/blackd/__init__.py b/src/blackd/__init__.py index e52a9917cf3..d92ad9ffa43 100644 --- a/src/blackd/__init__.py +++ b/src/blackd/__init__.py @@ -30,6 +30,7 @@ PROTOCOL_VERSION_HEADER = "X-Protocol-Version" LINE_LENGTH_HEADER = "X-Line-Length" PYTHON_VARIANT_HEADER = "X-Python-Variant" +SKIP_SOURCE_FIRST_LINE = "X-Skip-Source-First-Line" SKIP_STRING_NORMALIZATION_HEADER = "X-Skip-String-Normalization" SKIP_MAGIC_TRAILING_COMMA = "X-Skip-Magic-Trailing-Comma" PREVIEW = "X-Preview" @@ -40,6 +41,7 @@ PROTOCOL_VERSION_HEADER, LINE_LENGTH_HEADER, PYTHON_VARIANT_HEADER, + SKIP_SOURCE_FIRST_LINE, SKIP_STRING_NORMALIZATION_HEADER, SKIP_MAGIC_TRAILING_COMMA, PREVIEW, @@ -111,6 +113,9 @@ async def handle(request: web.Request, executor: Executor) -> web.Response: skip_magic_trailing_comma = bool( request.headers.get(SKIP_MAGIC_TRAILING_COMMA, False) ) + skip_source_first_line = bool( + request.headers.get(SKIP_SOURCE_FIRST_LINE, False) + ) preview = bool(request.headers.get(PREVIEW, False)) fast = False if request.headers.get(FAST_OR_SAFE_HEADER, "safe") == "fast": @@ -119,6 +124,7 @@ async def handle(request: web.Request, executor: Executor) -> web.Response: target_versions=versions, is_pyi=pyi, line_length=line_length, + skip_source_first_line=skip_source_first_line, string_normalization=not skip_string_normalization, magic_trailing_comma=not skip_magic_trailing_comma, preview=preview, From 21933caa2a9351ee46616bb30b0c5bff6c912843 Mon Sep 17 00:00:00 2001 From: Antonio Ossa Guerra Date: Thu, 29 Sep 2022 13:54:23 -0300 Subject: [PATCH 04/10] Add skip source first line option to blackd docs The new option can be passed to blackd as a header. This commit updates the blackd docs to include the new header. Signed-off-by: Antonio Ossa Guerra --- docs/usage_and_configuration/black_as_a_server.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/usage_and_configuration/black_as_a_server.md b/docs/usage_and_configuration/black_as_a_server.md index a2d4252109a..f24fb34d915 100644 --- a/docs/usage_and_configuration/black_as_a_server.md +++ b/docs/usage_and_configuration/black_as_a_server.md @@ -50,6 +50,9 @@ is rejected with `HTTP 501` (Not Implemented). The headers controlling how source code is formatted are: - `X-Line-Length`: corresponds to the `--line-length` command line flag. +- `X-Skip-Source-First-Line`: corresponds to the `--skip-source-first-line` command line + flag. If present and its value is not an empty string, the first line of the source + code will be ignored. - `X-Skip-String-Normalization`: corresponds to the `--skip-string-normalization` command line flag. If present and its value is not the empty string, no string normalization will be performed. From 9a7dd698556c168432c14482e68bcfcb29a48cc2 Mon Sep 17 00:00:00 2001 From: Antonio Ossa Guerra Date: Thu, 29 Sep 2022 14:06:52 -0300 Subject: [PATCH 05/10] Update CHANGES.md Include the new Black option to skip the first line of source code in the configuration section Signed-off-by: Antonio Ossa Guerra --- CHANGES.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index c96d9a6e492..bf25786e448 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -27,6 +27,9 @@ +- Add `--skip-source-first-line` / `-x` option to ignore the first line of source code + while formatting (#3299) + ### Packaging From 1701b6f841eed484d3a3159327e349d783e24bdc Mon Sep 17 00:00:00 2001 From: Antonio Ossa Guerra Date: Wed, 5 Oct 2022 23:40:15 -0300 Subject: [PATCH 06/10] Update skip first line test including valid syntax Including valid Python syntax help us make sure that the file is still actually valid after skipping the first line of the source file (which contains invalid Python syntax) Signed-off-by: Antonio Ossa Guerra --- tests/data/miscellaneous/invalid_header.py | 1 + tests/test_black.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/data/miscellaneous/invalid_header.py b/tests/data/miscellaneous/invalid_header.py index c0936ad4d80..fb49e2f93e7 100644 --- a/tests/data/miscellaneous/invalid_header.py +++ b/tests/data/miscellaneous/invalid_header.py @@ -1 +1,2 @@ This is not valid Python syntax +y = "This is valid syntax" diff --git a/tests/test_black.py b/tests/test_black.py index 86578af05f5..e06f95cd62f 100644 --- a/tests/test_black.py +++ b/tests/test_black.py @@ -353,7 +353,7 @@ def test_skip_source_first_line(self) -> None: self.assertEqual(result.exit_code, 0) with open(tmp_file, encoding="utf8") as f: actual = f.read() - self.assertFormatEqual(source.partition("\n")[0], actual.partition("\n")[0]) + self.assertFormatEqual(source, actual) def test_skip_magic_trailing_comma(self) -> None: source, _ = read_data("simple_cases", "expression") From 7baf898d7bfce3591e88f656282ca5ff878129cd Mon Sep 17 00:00:00 2001 From: Antonio Ossa Guerra Date: Thu, 6 Oct 2022 15:20:31 -0300 Subject: [PATCH 07/10] Skip first source line at `format_file_in_place` Instead of skipping the first source line at `format_file_contents`, do it before. This allow us to find the correct newline and encoding on the actual source code (everything that's after the header). This change is also applied at Blackd: take the header before passing the source to `format_file_contents` and put the header back once we get the formatted result. Signed-off-by: Antonio Ossa Guerra --- src/black/__init__.py | 11 +++++------ src/blackd/__init__.py | 10 ++++++++++ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 009462e617f..3e9157faa3e 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -798,7 +798,10 @@ def format_file_in_place( mode = replace(mode, is_ipynb=True) then = datetime.utcfromtimestamp(src.stat().st_mtime) + header = b"" with open(src, "rb") as buf: + if mode.skip_source_first_line: + header = buf.readline() src_contents, encoding, newline = decode_bytes(buf.read()) try: dst_contents = format_file_contents(src_contents, fast=fast, mode=mode) @@ -808,6 +811,8 @@ def format_file_in_place( raise ValueError( f"File '{src}' cannot be parsed as valid Jupyter notebook." ) from None + src_contents = header.decode() + src_contents + dst_contents = header.decode() + dst_contents if write_back == WriteBack.YES: with open(src, "w", encoding=encoding, newline=newline) as f: @@ -912,10 +917,6 @@ def format_file_contents(src_contents: str, *, fast: bool, mode: Mode) -> FileCo if not src_contents.strip(): raise NothingChanged - header = sep = "" - if mode.skip_source_first_line: - header, sep, src_contents = src_contents.partition("\n") - if mode.is_ipynb: dst_contents = format_ipynb_string(src_contents, fast=fast, mode=mode) else: @@ -926,8 +927,6 @@ def format_file_contents(src_contents: str, *, fast: bool, mode: Mode) -> FileCo if not fast and not mode.is_ipynb: # Jupyter notebooks will already have been checked above. check_stability_and_equivalence(src_contents, dst_contents, mode=mode) - - dst_contents = header + sep + dst_contents return dst_contents diff --git a/src/blackd/__init__.py b/src/blackd/__init__.py index 1a9fe837962..ba4750b8298 100644 --- a/src/blackd/__init__.py +++ b/src/blackd/__init__.py @@ -134,6 +134,12 @@ async def handle(request: web.Request, executor: Executor) -> web.Response: req_str = req_bytes.decode(charset) then = datetime.utcnow() + header = "" + if skip_source_first_line: + first_newline_position: int = req_str.find("\n") + 1 + header = req_str[:first_newline_position] + req_str = req_str[first_newline_position:] + loop = asyncio.get_event_loop() formatted_str = await loop.run_in_executor( executor, partial(black.format_file_contents, req_str, fast=fast, mode=mode) @@ -146,6 +152,10 @@ async def handle(request: web.Request, executor: Executor) -> web.Response: if formatted_str == req_str: raise black.NothingChanged + # Put the source first line back + req_str = header + req_str + formatted_str = header + formatted_str + # Only output the diff in the HTTP response only_diff = bool(request.headers.get(DIFF_HEADER, False)) if only_diff: From 02d1d172ead122bf4e70ede6027d946a62d123a9 Mon Sep 17 00:00:00 2001 From: Antonio Ossa Guerra Date: Thu, 6 Oct 2022 16:32:18 -0300 Subject: [PATCH 08/10] Test output newlines when skipping first line When skipping the first line of source code, the reference newline must be taken from the second line of the file instead of the first one, in case that the file mixes more than one kind of newline character Signed-off-by: Antonio Ossa Guerra --- tests/test_black.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/test_black.py b/tests/test_black.py index e06f95cd62f..5d0175d9d66 100644 --- a/tests/test_black.py +++ b/tests/test_black.py @@ -355,6 +355,16 @@ def test_skip_source_first_line(self) -> None: actual = f.read() self.assertFormatEqual(source, actual) + def test_skip_source_first_line_when_mixing_newlines(self) -> None: + code_mixing_newlines = b"Header will be skipped\r\ni = [1,2,3]\nj = [1,2,3]\n" + expected = b"Header will be skipped\r\ni = [1, 2, 3]\nj = [1, 2, 3]\n" + with TemporaryDirectory() as workspace: + test_file = Path(workspace) / "skip_header.py" + test_file.write_bytes(code_mixing_newlines) + mode = replace(DEFAULT_MODE, skip_source_first_line=True) + ff(test_file, mode=mode, write_back=black.WriteBack.YES) + self.assertEqual(test_file.read_bytes(), expected) + def test_skip_magic_trailing_comma(self) -> None: source, _ = read_data("simple_cases", "expression") expected, _ = read_data( From 6177cb3ec5c9d0bd3165f54fd5c5f2d988761ee6 Mon Sep 17 00:00:00 2001 From: Antonio Ossa Guerra Date: Thu, 6 Oct 2022 16:36:26 -0300 Subject: [PATCH 09/10] Test that Blackd also skips first line correctly Simliarly to the Black tests, we first compare that Blackd fails when the first line is invalid Python syntax and then check that the result is the expected when tha flag is activated Signed-off-by: Antonio Ossa Guerra --- tests/test_blackd.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/test_blackd.py b/tests/test_blackd.py index db9a1652f8c..5b6461f7685 100644 --- a/tests/test_blackd.py +++ b/tests/test_blackd.py @@ -177,6 +177,20 @@ async def test_blackd_invalid_line_length(self) -> None: ) self.assertEqual(response.status, 400) + @unittest_run_loop + async def test_blackd_skip_first_source_line(self) -> None: + invalid_first_line = b"Header will be skipped\r\ni = [1,2,3]\nj = [1,2,3]\n" + expected_result = b"Header will be skipped\r\ni = [1, 2, 3]\nj = [1, 2, 3]\n" + response = await self.client.post("/", data=invalid_first_line) + self.assertEqual(response.status, 400) + response = await self.client.post( + "/", + data=invalid_first_line, + headers={blackd.SKIP_SOURCE_FIRST_LINE: "true"}, + ) + self.assertEqual(response.status, 200) + self.assertEqual(await response.read(), expected_result) + @unittest_run_loop async def test_blackd_preview(self) -> None: response = await self.client.post( From 62dd94129ca3ef8270e9aae19c503f3d37239b74 Mon Sep 17 00:00:00 2001 From: Antonio Ossa Guerra Date: Thu, 6 Oct 2022 18:21:46 -0300 Subject: [PATCH 10/10] Use the content encoding to decode the header When decoding the header to put it back at the top of the contents of the file, use the same encoding used in the content. This should be a better "guess" that using the default value Signed-off-by: Antonio Ossa Guerra --- src/black/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/black/__init__.py b/src/black/__init__.py index 3e9157faa3e..afd71e51916 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -811,8 +811,8 @@ def format_file_in_place( raise ValueError( f"File '{src}' cannot be parsed as valid Jupyter notebook." ) from None - src_contents = header.decode() + src_contents - dst_contents = header.decode() + dst_contents + src_contents = header.decode(encoding) + src_contents + dst_contents = header.decode(encoding) + dst_contents if write_back == WriteBack.YES: with open(src, "w", encoding=encoding, newline=newline) as f: