From eec44f5977f195a10b81676525f463d0b634bd80 Mon Sep 17 00:00:00 2001 From: Bryan Bugyi Date: Sun, 30 May 2021 15:32:28 -0400 Subject: [PATCH 1/6] Fix --experiemental-string-processing crash when matching parens not found (#2283) Fixes #2271 --- CHANGES.md | 1 + src/black/trans.py | 14 ++++++++++---- tests/data/long_strings__regression.py | 20 ++++++++++++++++++++ 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index d9800a6979c..63c7a2cf30d 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -10,6 +10,7 @@ `.gitignore` rules like `git` does) (#2225) - Restored compatibility with Click 8.0 on Python 3.6 when LANG=C used (#2227) - Add extra uvloop install + import support if in python env (#2258) +- Fix --experimental-string-processing crash when matching parens are not found (#2283) ### _Blackd_ diff --git a/src/black/trans.py b/src/black/trans.py index 7ecc31d6d31..169b675be3d 100644 --- a/src/black/trans.py +++ b/src/black/trans.py @@ -25,7 +25,7 @@ from black.mode import Feature from black.nodes import syms, replace_child, parent_type from black.nodes import is_empty_par, is_empty_lpar, is_empty_rpar -from black.nodes import CLOSING_BRACKETS, STANDALONE_COMMENT +from black.nodes import OPENING_BRACKETS, CLOSING_BRACKETS, STANDALONE_COMMENT from black.lines import Line, append_leaves from black.brackets import BracketMatchError from black.comments import contains_pragma_comment @@ -1398,6 +1398,11 @@ class StringParenWrapper(CustomSplitMapMixin, BaseStringSplitter): def do_splitter_match(self, line: Line) -> TMatchResult: LL = line.leaves + if line.leaves[-1].type in OPENING_BRACKETS: + return TErr( + "Cannot wrap parens around a line that ends in an opening bracket." + ) + string_idx = ( self._return_match(LL) or self._else_match(LL) @@ -1665,9 +1670,10 @@ def do_transform(self, line: Line, string_idx: int) -> Iterator[TResult[Line]]: right_leaves.pop() if old_parens_exist: - assert ( - right_leaves and right_leaves[-1].type == token.RPAR - ), "Apparently, old parentheses do NOT exist?!" + assert right_leaves and right_leaves[-1].type == token.RPAR, ( + "Apparently, old parentheses do NOT exist?!" + f" (left_leaves={left_leaves}, right_leaves={right_leaves})" + ) old_rpar_leaf = right_leaves.pop() append_leaves(string_line, line, right_leaves) diff --git a/tests/data/long_strings__regression.py b/tests/data/long_strings__regression.py index 2e7f2483b63..231d88651b6 100644 --- a/tests/data/long_strings__regression.py +++ b/tests/data/long_strings__regression.py @@ -396,6 +396,16 @@ def xxxxxxx_xxxxxx(xxxx): " it has now" ) + +def _legacy_listen_examples(): + text += ( + " \"listen for the '%(event_name)s' event\"\n" + "\n # ... (event logic logic logic) ...\n" + % { + "since": since, + } + ) + # output @@ -886,3 +896,13 @@ def xxxxxxx_xxxxxx(xxxx): " it goes over 88 characters which" " it has now" ) + + +def _legacy_listen_examples(): + text += ( + " \"listen for the '%(event_name)s' event\"\n" + "\n # ... (event logic logic logic) ...\n" + % { + "since": since, + } + ) From 4ca4407b4adc49b96c9536b16ed7d0a1e0b2deca Mon Sep 17 00:00:00 2001 From: Bryan Bugyi Date: Sun, 30 May 2021 17:41:03 -0400 Subject: [PATCH 2/6] Make sure to split lines that start with a string operator (#2286) Fixes #2284 --- CHANGES.md | 1 + src/black/trans.py | 78 +++++++++++++++++------- tests/data/long_strings__regression.py | 84 ++++++++++++++++++++++++++ 3 files changed, 142 insertions(+), 21 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 63c7a2cf30d..c761d14624a 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -11,6 +11,7 @@ - Restored compatibility with Click 8.0 on Python 3.6 when LANG=C used (#2227) - Add extra uvloop install + import support if in python env (#2258) - Fix --experimental-string-processing crash when matching parens are not found (#2283) +- Make sure to split lines that start with a string operator (#2286) ### _Blackd_ diff --git a/src/black/trans.py b/src/black/trans.py index 169b675be3d..80e88a2d2fb 100644 --- a/src/black/trans.py +++ b/src/black/trans.py @@ -920,9 +920,9 @@ class StringSplitter(CustomSplitMapMixin, BaseStringSplitter): lines by themselves). Requirements: - * The line consists ONLY of a single string (with the exception of a - '+' symbol which MAY exist at the start of the line), MAYBE a string - trailer, and MAYBE a trailing comma. + * The line consists ONLY of a single string (possibly prefixed by a + string operator [e.g. '+' or '==']), MAYBE a string trailer, and MAYBE + a trailing comma. AND * All of the requirements listed in BaseStringSplitter's docstring. @@ -952,6 +952,16 @@ class StringSplitter(CustomSplitMapMixin, BaseStringSplitter): CustomSplit objects and add them to the custom split map. """ + STRING_OPERATORS = [ + token.PLUS, + token.STAR, + token.EQEQUAL, + token.NOTEQUAL, + token.LESS, + token.LESSEQUAL, + token.GREATER, + token.GREATEREQUAL, + ] MIN_SUBSTR_SIZE = 6 # Matches an "f-expression" (e.g. {var}) that might be found in an f-string. RE_FEXPR = r""" @@ -972,8 +982,20 @@ def do_splitter_match(self, line: Line) -> TMatchResult: idx = 0 - # The first leaf MAY be a '+' symbol... - if is_valid_index(idx) and LL[idx].type == token.PLUS: + # The first two leaves MAY be the 'not in' keywords... + if ( + is_valid_index(idx) + and is_valid_index(idx + 1) + and [LL[idx].type, LL[idx + 1].type] == [token.NAME, token.NAME] + and str(LL[idx]) + str(LL[idx + 1]) == "not in" + ): + idx += 2 + # Else the first leaf MAY be a string operator symbol or the 'in' keyword... + elif is_valid_index(idx) and ( + LL[idx].type in self.STRING_OPERATORS + or LL[idx].type == token.NAME + and str(LL[idx]) == "in" + ): idx += 1 # The next/first leaf MAY be an empty LPAR... @@ -1023,23 +1045,26 @@ def do_transform(self, line: Line, string_idx: int) -> Iterator[TResult[Line]]: ) first_string_line = True - starts_with_plus = LL[0].type == token.PLUS - def line_needs_plus() -> bool: - return first_string_line and starts_with_plus + string_op_leaves = self._get_string_operator_leaves(LL) + string_op_leaves_length = ( + sum([len(str(prefix_leaf)) for prefix_leaf in string_op_leaves]) + 1 + if string_op_leaves + else 0 + ) - def maybe_append_plus(new_line: Line) -> None: + def maybe_append_string_operators(new_line: Line) -> None: """ Side Effects: - If @line starts with a plus and this is the first line we are - constructing, this function appends a PLUS leaf to @new_line - and replaces the old PLUS leaf in the node structure. Otherwise - this function does nothing. + If @line starts with a string operator and this is the first + line we are constructing, this function appends the string + operator to @new_line and replaces the old string operator leaf + in the node structure. Otherwise this function does nothing. """ - if line_needs_plus(): - plus_leaf = Leaf(token.PLUS, "+") - replace_child(LL[0], plus_leaf) - new_line.append(plus_leaf) + maybe_prefix_leaves = string_op_leaves if first_string_line else [] + for i, prefix_leaf in enumerate(maybe_prefix_leaves): + replace_child(LL[i], prefix_leaf) + new_line.append(prefix_leaf) ends_with_comma = ( is_valid_index(string_idx + 1) and LL[string_idx + 1].type == token.COMMA @@ -1054,7 +1079,7 @@ def max_last_string() -> int: result = self.line_length result -= line.depth * 4 result -= 1 if ends_with_comma else 0 - result -= 2 if line_needs_plus() else 0 + result -= string_op_leaves_length return result # --- Calculate Max Break Index (for string value) @@ -1103,7 +1128,7 @@ def more_splits_should_be_made() -> bool: break_idx = csplit.break_idx else: # Algorithmic Split (automatic) - max_bidx = max_break_idx - 2 if line_needs_plus() else max_break_idx + max_bidx = max_break_idx - string_op_leaves_length maybe_break_idx = self._get_break_idx(rest_value, max_bidx) if maybe_break_idx is None: # If we are unable to algorithmically determine a good split @@ -1148,7 +1173,7 @@ def more_splits_should_be_made() -> bool: # --- Construct `next_line` next_line = line.clone() - maybe_append_plus(next_line) + maybe_append_string_operators(next_line) next_line.append(next_leaf) string_line_results.append(Ok(next_line)) @@ -1169,7 +1194,7 @@ def more_splits_should_be_made() -> bool: self._maybe_normalize_string_quotes(rest_leaf) last_line = line.clone() - maybe_append_plus(last_line) + maybe_append_string_operators(last_line) # If there are any leaves to the right of the target string... if is_valid_index(string_idx + 1): @@ -1345,6 +1370,17 @@ def _normalize_f_string(self, string: str, prefix: str) -> str: else: return string + def _get_string_operator_leaves(self, leaves: Iterable[Leaf]) -> List[Leaf]: + LL = list(leaves) + + string_op_leaves = [] + i = 0 + while LL[i].type in self.STRING_OPERATORS + [token.NAME]: + prefix_leaf = Leaf(LL[i].type, str(LL[i]).strip()) + string_op_leaves.append(prefix_leaf) + i += 1 + return string_op_leaves + class StringParenWrapper(CustomSplitMapMixin, BaseStringSplitter): """ diff --git a/tests/data/long_strings__regression.py b/tests/data/long_strings__regression.py index 231d88651b6..bd7b6351321 100644 --- a/tests/data/long_strings__regression.py +++ b/tests/data/long_strings__regression.py @@ -406,6 +406,40 @@ def _legacy_listen_examples(): } ) + +assert str(suffix_arr) == ( + "['$', 'angaroo$', 'angrykangaroo$', 'aroo$', 'garoo$', " + "'grykangaroo$', 'kangaroo$', 'ngaroo$', 'ngrykangaroo$', " + "'o$', 'oo$', 'roo$', 'rykangaroo$', 'ykangaroo$']" +) +assert str(suffix_arr) != ( + "['$', 'angaroo$', 'angrykangaroo$', 'aroo$', 'garoo$', " + "'grykangaroo$', 'kangaroo$', 'ngaroo$', 'ngrykangaroo$', " + "'o$', 'oo$', 'roo$', 'rykangaroo$', 'ykangaroo$']" +) +assert str(suffix_arr) <= ( + "['$', 'angaroo$', 'angrykangaroo$', 'aroo$', 'garoo$', " + "'grykangaroo$', 'kangaroo$', 'ngaroo$', 'ngrykangaroo$', " + "'o$', 'oo$', 'roo$', 'rykangaroo$', 'ykangaroo$']" +) +assert str(suffix_arr) >= ( + "['$', 'angaroo$', 'angrykangaroo$', 'aroo$', 'garoo$', " + "'grykangaroo$', 'kangaroo$', 'ngaroo$', 'ngrykangaroo$', " + "'o$', 'oo$', 'roo$', 'rykangaroo$', 'ykangaroo$']" +) +assert str(suffix_arr) < ( + "['$', 'angaroo$', 'angrykangaroo$', 'aroo$', 'garoo$', " + "'grykangaroo$', 'kangaroo$', 'ngaroo$', 'ngrykangaroo$', " + "'o$', 'oo$', 'roo$', 'rykangaroo$', 'ykangaroo$']" +) +assert str(suffix_arr) > ( + "['$', 'angaroo$', 'angrykangaroo$', 'aroo$', 'garoo$', " + "'grykangaroo$', 'kangaroo$', 'ngaroo$', 'ngrykangaroo$', " + "'o$', 'oo$', 'roo$', 'rykangaroo$', 'ykangaroo$']" +) +assert str(suffix_arr) in "['$', 'angaroo$', 'angrykangaroo$', 'aroo$', 'garoo$', 'grykangaroo$', 'kangaroo$', 'ngaroo$', 'ngrykangaroo$', 'o$', 'oo$', 'roo$', 'rykangaroo$', 'ykangaroo$']" +assert str(suffix_arr) not in "['$', 'angaroo$', 'angrykangaroo$', 'aroo$', 'garoo$', 'grykangaroo$', 'kangaroo$', 'ngaroo$', 'ngrykangaroo$', 'o$', 'oo$', 'roo$', 'rykangaroo$', 'ykangaroo$']" + # output @@ -906,3 +940,53 @@ def _legacy_listen_examples(): "since": since, } ) + + +assert ( + str(suffix_arr) + == "['$', 'angaroo$', 'angrykangaroo$', 'aroo$', 'garoo$', " + "'grykangaroo$', 'kangaroo$', 'ngaroo$', 'ngrykangaroo$', " + "'o$', 'oo$', 'roo$', 'rykangaroo$', 'ykangaroo$']" +) +assert ( + str(suffix_arr) + != "['$', 'angaroo$', 'angrykangaroo$', 'aroo$', 'garoo$', " + "'grykangaroo$', 'kangaroo$', 'ngaroo$', 'ngrykangaroo$', " + "'o$', 'oo$', 'roo$', 'rykangaroo$', 'ykangaroo$']" +) +assert ( + str(suffix_arr) + <= "['$', 'angaroo$', 'angrykangaroo$', 'aroo$', 'garoo$', " + "'grykangaroo$', 'kangaroo$', 'ngaroo$', 'ngrykangaroo$', " + "'o$', 'oo$', 'roo$', 'rykangaroo$', 'ykangaroo$']" +) +assert ( + str(suffix_arr) + >= "['$', 'angaroo$', 'angrykangaroo$', 'aroo$', 'garoo$', " + "'grykangaroo$', 'kangaroo$', 'ngaroo$', 'ngrykangaroo$', " + "'o$', 'oo$', 'roo$', 'rykangaroo$', 'ykangaroo$']" +) +assert ( + str(suffix_arr) + < "['$', 'angaroo$', 'angrykangaroo$', 'aroo$', 'garoo$', " + "'grykangaroo$', 'kangaroo$', 'ngaroo$', 'ngrykangaroo$', " + "'o$', 'oo$', 'roo$', 'rykangaroo$', 'ykangaroo$']" +) +assert ( + str(suffix_arr) + > "['$', 'angaroo$', 'angrykangaroo$', 'aroo$', 'garoo$', " + "'grykangaroo$', 'kangaroo$', 'ngaroo$', 'ngrykangaroo$', " + "'o$', 'oo$', 'roo$', 'rykangaroo$', 'ykangaroo$']" +) +assert ( + str(suffix_arr) + in "['$', 'angaroo$', 'angrykangaroo$', 'aroo$', 'garoo$', 'grykangaroo$'," + " 'kangaroo$', 'ngaroo$', 'ngrykangaroo$', 'o$', 'oo$', 'roo$', 'rykangaroo$'," + " 'ykangaroo$']" +) +assert ( + str(suffix_arr) + not in "['$', 'angaroo$', 'angrykangaroo$', 'aroo$', 'garoo$', 'grykangaroo$'," + " 'kangaroo$', 'ngaroo$', 'ngrykangaroo$', 'o$', 'oo$', 'roo$'," + " 'rykangaroo$', 'ykangaroo$']" +) From 199e3eb76b74d9d2cada527403a3989287c4e8b3 Mon Sep 17 00:00:00 2001 From: Bryan Bugyi Date: Sun, 30 May 2021 18:34:33 -0400 Subject: [PATCH 3/6] Fix regular expression that black uses to identify f-expressions (#2287) Fixes #1469 --- CHANGES.md | 1 + src/black/trans.py | 4 ++-- tests/data/long_strings__regression.py | 12 ++++++++++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index c761d14624a..39256982215 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -12,6 +12,7 @@ - Add extra uvloop install + import support if in python env (#2258) - Fix --experimental-string-processing crash when matching parens are not found (#2283) - Make sure to split lines that start with a string operator (#2286) +- Fix regular expression that black uses to identify f-expressions (#2287) ### _Blackd_ diff --git a/src/black/trans.py b/src/black/trans.py index 80e88a2d2fb..fd0de727620 100644 --- a/src/black/trans.py +++ b/src/black/trans.py @@ -971,8 +971,8 @@ class StringSplitter(CustomSplitMapMixin, BaseStringSplitter): | \{\{ | \}\} | (?R) - )+? - (? TMatchResult: diff --git a/tests/data/long_strings__regression.py b/tests/data/long_strings__regression.py index bd7b6351321..cd8053f9eb5 100644 --- a/tests/data/long_strings__regression.py +++ b/tests/data/long_strings__regression.py @@ -407,6 +407,12 @@ def _legacy_listen_examples(): ) +temp_msg = ( + f"{f'{humanize_number(pos)}.': <{pound_len+2}} " + f"{balance: <{bal_len + 5}} " + f"<<{author.display_name}>>\n" +) + assert str(suffix_arr) == ( "['$', 'angaroo$', 'angrykangaroo$', 'aroo$', 'garoo$', " "'grykangaroo$', 'kangaroo$', 'ngaroo$', 'ngrykangaroo$', " @@ -942,6 +948,12 @@ def _legacy_listen_examples(): ) +temp_msg = ( + f"{f'{humanize_number(pos)}.': <{pound_len+2}} " + f"{balance: <{bal_len + 5}} " + f"<<{author.display_name}>>\n" +) + assert ( str(suffix_arr) == "['$', 'angaroo$', 'angrykangaroo$', 'aroo$', 'garoo$', " From cf75673e1a2c993025a2113ce194d5c65f311c85 Mon Sep 17 00:00:00 2001 From: Cooper Lees Date: Mon, 31 May 2021 07:25:54 -0700 Subject: [PATCH 4/6] Update CHANGES.md for 21.5b2 release (#2290) * Update CHANGES.md for 21.5b2 release --- CHANGES.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 39256982215..4bf08275cf8 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,6 +1,6 @@ # Change Log -## Unreleased +## 21.5b2 ### _Black_ @@ -21,7 +21,8 @@ ### _Packaging_ -- Release self-contained macOS binaries as part of the GitHub release pipeline (#2198) +- Release self-contained x86_64 MacOS binaries as part of the GitHub release pipeline + (#2198) - Always build binaries with the latest available Python (#2260) ### Documentation From a4e35b314977baae2e930abd24fa1013c7235e39 Mon Sep 17 00:00:00 2001 From: Bryan Bugyi Date: Mon, 31 May 2021 20:57:23 -0400 Subject: [PATCH 5/6] Correct max string length calculation when there are string operators (#2292) PR #2286 did not fix the edge-cases (e.g. when the string is just long enough to cause a line to be 89 characters long). This PR corrects that mistake. --- CHANGES.md | 6 ++++++ src/black/trans.py | 28 ++++++++++++++------------- tests/data/long_strings__edge_case.py | 19 ++++++++++++++++++ 3 files changed, 40 insertions(+), 13 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 4bf08275cf8..de67943d29f 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,11 @@ # Change Log +## Unreleased + +### _Black_ + +- Correct max string length calculation when there are string operators (#2292) + ## 21.5b2 ### _Black_ diff --git a/src/black/trans.py b/src/black/trans.py index fd0de727620..bc6e93b01b4 100644 --- a/src/black/trans.py +++ b/src/black/trans.py @@ -738,6 +738,18 @@ class BaseStringSplitter(StringTransformer): * The target string is not a multiline (i.e. triple-quote) string. """ + STRING_OPERATORS = [ + token.EQEQUAL, + token.GREATER, + token.GREATEREQUAL, + token.LESS, + token.LESSEQUAL, + token.NOTEQUAL, + token.PERCENT, + token.PLUS, + token.STAR, + ] + @abstractmethod def do_splitter_match(self, line: Line) -> TMatchResult: """ @@ -847,9 +859,9 @@ def _get_max_string_length(self, line: Line, string_idx: int) -> int: p_idx -= 1 P = LL[p_idx] - if P.type == token.PLUS: - # WMA4 a space and a '+' character (e.g. `+ STRING`). - offset += 2 + if P.type in self.STRING_OPERATORS: + # WMA4 a space and a string operator (e.g. `+ STRING` or `== STRING`). + offset += len(str(P)) + 1 if P.type == token.COMMA: # WMA4 a space, a comma, and a closing bracket [e.g. `), STRING`]. @@ -952,16 +964,6 @@ class StringSplitter(CustomSplitMapMixin, BaseStringSplitter): CustomSplit objects and add them to the custom split map. """ - STRING_OPERATORS = [ - token.PLUS, - token.STAR, - token.EQEQUAL, - token.NOTEQUAL, - token.LESS, - token.LESSEQUAL, - token.GREATER, - token.GREATEREQUAL, - ] MIN_SUBSTR_SIZE = 6 # Matches an "f-expression" (e.g. {var}) that might be found in an f-string. RE_FEXPR = r""" diff --git a/tests/data/long_strings__edge_case.py b/tests/data/long_strings__edge_case.py index 6919db5a80b..07c27537191 100644 --- a/tests/data/long_strings__edge_case.py +++ b/tests/data/long_strings__edge_case.py @@ -29,6 +29,9 @@ ) return f'{x}/b/c/d/d/d/dadfjsadjsaidoaisjdsfjaofjdfijaidfjaodfjaoifjodjafojdoajaaaaaaaaaaa' return f'{x}/b/c/d/d/d/dadfjsadjsaidoaisjdsfjaofjdfijaidfjaodfjaoifjodjafojdoajaaaaaaaaaaaa' +assert str(result) == "This long string should be split at some point right close to or around hereeeeeee" +assert str(result) < "This long string should be split at some point right close to or around hereeeeee" +assert "A format string: %s" % "This long string should be split at some point right close to or around hereeeeeee" != result # output @@ -108,3 +111,19 @@ f"{x}/b/c/d/d/d/dadfjsadjsaidoaisjdsfjaofjdfijaidfjaodfjaoifjodjafojdoajaaaaaaaaaaa" ) return f"{x}/b/c/d/d/d/dadfjsadjsaidoaisjdsfjaofjdfijaidfjaodfjaoifjodjafojdoajaaaaaaaaaaaa" +assert ( + str(result) + == "This long string should be split at some point right close to or around" + " hereeeeeee" +) +assert ( + str(result) + < "This long string should be split at some point right close to or around" + " hereeeeee" +) +assert ( + "A format string: %s" + % "This long string should be split at some point right close to or around" + " hereeeeeee" + != result +) From 4005246f86d1459e1123ac399721054182e9fef6 Mon Sep 17 00:00:00 2001 From: Stefan Foulis Date: Tue, 1 Jun 2021 03:45:50 +0200 Subject: [PATCH 6/6] Add `version` to github action (and rewrite the whole thing while at it) (#1940) Commit history before merge: * Add black_version to github action * Merge upstream/main into this branch * Add version support for the Black action pt.2 Since we're moving to a composite based action, quite a few changes were made. 1) Support was added for all OSes (Windows was painful). 2) Isolation from the rest of the workflow had to be done manually with a virtual environment. Other noteworthy changes: - Rewrote basically all of the logic and put it in a Python script for easy testing (not doing it here tho cause I'm lazy and I can't think of a reasonable way of testing it). - Renamed `black_version` to `version` to better fit the existing input naming scheme. - Added support for log groups, this makes our action's output a bit more fancy (I may or may have not added some debug output too). * Add more to and sorta rewrite the Action's docs Reflect compatability and gotchas. * Add CHANGELOG entry * Merge main into this branch * Remove debug; address typos; clean up action.yml Co-authored-by: Richard Si <63936253+ichard26@users.noreply.github.com> --- CHANGES.md | 7 ++++- action.yml | 43 ++++++++++++++++++++++++++--- action/Dockerfile | 10 ------- action/entrypoint.sh | 9 ------ action/main.py | 39 ++++++++++++++++++++++++++ docs/integrations/github_actions.md | 23 ++++++++++++--- 6 files changed, 103 insertions(+), 28 deletions(-) delete mode 100644 action/Dockerfile delete mode 100755 action/entrypoint.sh create mode 100644 action/main.py diff --git a/CHANGES.md b/CHANGES.md index de67943d29f..81a6d9f6668 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -25,7 +25,12 @@ - Add a lower bound for the `aiohttp-cors` dependency. Only 0.4.0 or higher is supported. (#2231) -### _Packaging_ +### Integrations + +- The official Black action now supports choosing what version to use, and supports the + major 3 OSes. (#1940) + +### Packaging - Release self-contained x86_64 MacOS binaries as part of the GitHub release pipeline (#2198) diff --git a/action.yml b/action.yml index 827e971801b..ddf07933a3e 100644 --- a/action.yml +++ b/action.yml @@ -4,21 +4,56 @@ author: "Łukasz Langa and contributors to Black" inputs: options: description: - "Options passed to black. Use `black --help` to see available options. Default: + "Options passed to Black. Use `black --help` to see available options. Default: '--check'" required: false default: "--check --diff" src: - description: "Source to run black. Default: '.'" + description: "Source to run Black. Default: '.'" required: false default: "." black_args: description: "[DEPRECATED] Black input arguments." required: false default: "" + deprecationMessage: + "Input `with.black_args` is deprecated. Use `with.options` and `with.src` instead." + version: + description: 'Python Version specifier (PEP440) - e.g. "21.5b1"' + required: false + default: "" branding: color: "black" icon: "check-circle" runs: - using: "docker" - image: "action/Dockerfile" + using: composite + steps: + - run: | + # Exists since using github.action_path + path to main script doesn't work because bash + # interprets the backslashes in github.action_path (which are used when the runner OS + # is Windows) destroying the path to the target file. + # + # Also semicolons are necessary because I can't get the newlines to work + entrypoint="import sys; + import subprocess; + from pathlib import Path; + + MAIN_SCRIPT = Path(r'${{ github.action_path }}') / 'action' / 'main.py'; + + proc = subprocess.run([sys.executable, str(MAIN_SCRIPT)]); + sys.exit(proc.returncode) + " + + if [ "$RUNNER_OS" == "Windows" ]; then + echo $entrypoint | python + else + echo $entrypoint | python3 + fi + env: + # TODO: Remove once https://github.com/actions/runner/issues/665 is fixed. + INPUT_OPTIONS: ${{ inputs.options }} + INPUT_SRC: ${{ inputs.src }} + INPUT_BLACK_ARGS: ${{ inputs.black_args }} + INPUT_VERSION: ${{ inputs.version }} + pythonioencoding: utf-8 + shell: bash diff --git a/action/Dockerfile b/action/Dockerfile deleted file mode 100644 index eb2209940db..00000000000 --- a/action/Dockerfile +++ /dev/null @@ -1,10 +0,0 @@ -FROM python:3 - -ENV PYTHONDONTWRITEBYTECODE 1 -ENV PYTHONUNBUFFERED 1 - -RUN pip install --upgrade --no-cache-dir black - -COPY entrypoint.sh /entrypoint.sh - -ENTRYPOINT ["/entrypoint.sh"] diff --git a/action/entrypoint.sh b/action/entrypoint.sh deleted file mode 100755 index 30bf4eb688f..00000000000 --- a/action/entrypoint.sh +++ /dev/null @@ -1,9 +0,0 @@ -#!/bin/bash -e - -if [ -n "$INPUT_BLACK_ARGS" ]; then - echo '::warning::Input `with.black_args` is deprecated. Use `with.options` and `with.src` instead.' - black $INPUT_BLACK_ARGS - exit $? -fi - -black $INPUT_OPTIONS $INPUT_SRC diff --git a/action/main.py b/action/main.py new file mode 100644 index 00000000000..fde312553bf --- /dev/null +++ b/action/main.py @@ -0,0 +1,39 @@ +import os +import shlex +import sys +from pathlib import Path +from subprocess import run, PIPE, STDOUT + +ACTION_PATH = Path(os.environ["GITHUB_ACTION_PATH"]) +ENV_PATH = ACTION_PATH / ".black-env" +ENV_BIN = ENV_PATH / ("Scripts" if sys.platform == "win32" else "bin") +OPTIONS = os.getenv("INPUT_OPTIONS", default="") +SRC = os.getenv("INPUT_SRC", default="") +BLACK_ARGS = os.getenv("INPUT_BLACK_ARGS", default="") +VERSION = os.getenv("INPUT_VERSION", default="") + +run([sys.executable, "-m", "venv", str(ENV_PATH)], check=True) + +req = "black[colorama,python2]" +if VERSION: + req += f"=={VERSION}" +pip_proc = run( + [str(ENV_BIN / "python"), "-m", "pip", "install", req], + stdout=PIPE, + stderr=STDOUT, + encoding="utf-8", +) +if pip_proc.returncode: + print(pip_proc.stdout) + print("::error::Failed to install Black.", flush=True) + sys.exit(pip_proc.returncode) + + +base_cmd = [str(ENV_BIN / "black")] +if BLACK_ARGS: + # TODO: remove after a while since this is deprecated in favour of SRC + OPTIONS. + proc = run([*base_cmd, *shlex.split(BLACK_ARGS)]) +else: + proc = run([*base_cmd, *shlex.split(OPTIONS), *shlex.split(SRC)]) + +sys.exit(proc.returncode) diff --git a/docs/integrations/github_actions.md b/docs/integrations/github_actions.md index 9e8cf436453..d293c40dadf 100644 --- a/docs/integrations/github_actions.md +++ b/docs/integrations/github_actions.md @@ -3,6 +3,14 @@ You can use _Black_ within a GitHub Actions workflow without setting your own Python environment. Great for enforcing that your code matches the _Black_ code style. +## Compatiblity + +This action is known to support all GitHub-hosted runner OSes. In addition, only +published versions of _Black_ are supported (i.e. whatever is available on PyPI). + +Finally, this action installs _Black_ with both the `colorama` and `python2` extras so +the `--color` flag and formatting Python 2 code are supported. + ## Usage Create a file named `.github/workflows/black.yml` inside your repository with: @@ -17,19 +25,26 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 - - uses: actions/setup-python@v2 - uses: psf/black@stable ``` We recommend the use of the `@stable` tag, but per version tags also exist if you prefer -that. +that. Note that the action's version you select is independent of the version of _Black_ +the action will use. + +The version of _Black_ the action will use can be configured via `version`. The action +defaults to the latest release available on PyPI. Only versions available from PyPI are +supported, so no commit SHAs or branch names. + +You can also configure the arguments passed to _Black_ via `options` (defaults to +`'--check --diff'`) and `src` (default is `'.'`) -You may use `options` (Default is `'--check --diff'`) and `src` (Default is `'.'`) as -follows: +Here's an example configuration: ```yaml - uses: psf/black@stable with: options: "--check --verbose" src: "./src" + version: "21.5b1" ```