-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Description
Background:
- Black should enforce zero empty leading lines in function body #902
Original issue, with some follow-up comments post merge - Remove newline after code block open #3035
Original implementation - Allow a single newline after opening blocks, possibly via an option setting #3551
Follow-up issue - Allow empty line after block open before a comment or compound statement #3967
Recent preview change partially walking things back - One space between class definition and the first inner method #619 (comment)
Discussion about PEP 8
(there's discussion elsewhere too, but I think these are the most important links)
tldr; the 2023 stable style strips newlines after code block opens. This has proven controversial (and was most of the change from 22 to 23 by volume). Recently, we walked this back a little bit in preview style, but we should try to get it right for 24.
The main argument against is it can hurt readability, see #902 (comment) . I'm quite sympathetic to this. For instance, functions with a complicated return type and first statement can get quite dense. Similarly, there are examples with comments in a new code block where things can get too dense.
An auxiliary argument against it is that stripping new lines between class and first method is in violation of a close reading and changelog analysis of PEP 8, see #619 (comment). I don't care much for PEP 8 absolutism (and certainly not at the expense of churning established behaviour), but I consider it a nice-to-have to not reformat away from it.
Recently in #3967, Henri contributed a nice change that walks this back in some cases, involving comments or compound statements. However, I'm not sure this is enough. Here are two examples from the first few pages of Black 22 -> 23 diff in my work codebase where I feel readability really suffers (scrolling through I see many others). Henri's change won't save us here. And fundamentally this code is quite simple, it just has slightly long type and variable names, i.e. it's not the kind of code where there should have been a comment or something.
Jelle nudged me to think about this, and I now feel we should just walk this change back entirely. We should allow users to have either 0 or 1 blank newlines. This is also simpler and more explainable than the partially walked back logic.
I am fine with continuing to strip blank newlines between a function declaration and its docstring (and preview does this for classes as well). We currently enforce one blank newline after class and module docstrings. We don't have an opinion on blank newlines after function docstrings (see #2370). I think this is probably fine as-is and not having an opinion would be consistent with the proposed no docstring case. (I'll grant that it feels easier to come up with a good opinion for the post docstring blank line in functions. At the very least, we certainly should not resolve that issue by always removing blank lines).
This will not create any churn going from 23 -> 24 (in fact, most of this change technically even meets our stability policy (although any change here should certainly only be made in preview style)).
As a meta thing, as a relatively new maintainer of Black, I'm still unsure of my decision making framework. I feel the original change had at most marginal benefit, especially given its churn (I've never browsed code and been like ugh one extra blank line, nor has any code reviewer or linter ever yelled at me for it. These are the things Black usually saves me from. All the motivating changes I found have multiple blank newlines or are toy code). On the other hand, it's very costly to revert changes in stable style. The only thing that makes it possible in this case is that both unreverted and reverted formatting will be valid and inconsistency in this specific case is not costly.