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

Treat functions/classes in blocks as if they're nested #2472

Merged
merged 6 commits into from Dec 1, 2021

Conversation

ichard26
Copy link
Collaborator

@ichard26 ichard26 commented Sep 2, 2021

Description

One curveball is that we still want two preceding newlines before blocks
that are probably logically disconnected. In other words:

if condition:

    def foo():
        return "hi"
                         # <- aside: this is the goal of this commit
else:

    def foo():
        return "cya"
                         # <- the two newlines spacing here should stay
                         #    since this probably isn't related
with open("db.json", encoding="utf-8") as f:
    data = f.read()

Unfortunately that means we have to special case specific clause types
instead of just being able to just for a colon leaf. The hack used here
is to check whether we're adding preceding newlines for a standalone or
dependent clause. "Standalone" being a clause that doesn't need another
clause to be valid (eg. if) and vice versa.

Checklist - did you ...

  • Add a CHANGELOG entry if necessary?
  • Add / update tests if necessary?
  • Add new / update outdated documentation? -> probably n/a?

Fixes GH-647

I understand if this is way too hacky to be acceptable but I spent too much time to not at least give it a shot :)

One curveball is that we still want two preceding newlines before blocks
that are probably logically disconnected. In other words:

    if condition:

        def foo():
            return "hi"
                             # <- aside: this is the goal of this commit
    else:

        def foo():
            return "cya"
                             # <- the two newlines spacing here should stay
                             #    since this probably isn't related
    with open("db.json", encoding="utf-8") as f:
        data = f.read()

Unfortunately that means we have to special case specific clause types
instead of just being able to just for a colon leaf. The hack used here
is to check whether we're adding preceding newlines for a standalone or
dependent clause. "Standalone" being a clause that doesn't need another
clause to be valid (eg. if) and vice versa.
@ichard26 ichard26 added the F: empty lines Wasting vertical space efficiently. label Sep 2, 2021
Copy link
Collaborator

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Looks good with a couple of suggestions for wording

src/black/lines.py Outdated Show resolved Hide resolved
src/black/lines.py Outdated Show resolved Hide resolved
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
@ichard26
Copy link
Collaborator Author

ichard26 commented Dec 1, 2021

Diff-shades result:

--- a/attrs:src/attr/_compat.py
+++ b/attrs:src/attr/_compat.py
@@ -109,11 +109,10 @@
     def just_warn(*args, **kw):  # pragma: no cover
         """
         We only warn on Python 3 because we are not aware of any concrete
         consequences of not setting the cell on Python 2.
         """
-
 
 else:  # Python 3 and later.
     from collections.abc import Mapping, Sequence  # noqa
 
     def just_warn(*args, **kw):

--- a/attrs:src/attr/_make.py
+++ b/attrs:src/attr/_make.py
@@ -624,11 +624,10 @@
         ):
             BaseException.__setattr__(self, name, value)
             return
 
         raise FrozenInstanceError()
-
 
 else:
 
     def _frozen_setattrs(self, name, value):
         """
@@ -1624,11 +1623,10 @@
             getattr(cls.__setattr__, "__module__", None)
             == _frozen_setattrs.__module__
             and cls.__setattr__.__name__ == _frozen_setattrs.__name__
         )
 
-
 else:
 
     def _has_frozen_base_class(cls):
         """
         Check whether *cls* has a frozen ancestor by looking at its
@@ -1927,11 +1925,10 @@
         lines.append("    working_set.remove(id(self))")
 
         return _make_method(
             "__repr__", "\n".join(lines), unique_filename, globs=globs
         )
-
 
 else:
 
     def _make_repr(attrs, ns, _):
         """

--- a/django:django/core/files/locks.py
+++ b/django:django/core/files/locks.py
@@ -90,11 +90,10 @@
         hfile = msvcrt.get_osfhandle(_fd(f))
         overlapped = OVERLAPPED()
         ret = UnlockFileEx(hfile, 0, 0, 0xFFFF0000, byref(overlapped))
         return bool(ret)
 
-
 else:
     try:
         import fcntl
 
         LOCK_SH = fcntl.LOCK_SH  # shared lock

--- a/django:tests/dispatch/tests.py
+++ b/django:tests/dispatch/tests.py
@@ -11,11 +11,10 @@
 
     def garbage_collect():
         # Collecting weakreferences can take two collections on PyPy.
         gc.collect()
         gc.collect()
-
 
 else:
 
     def garbage_collect():
         gc.collect()

--- a/django:tests/gis_tests/gis_migrations/migrations/0001_setup_extensions.py
+++ b/django:tests/gis_tests/gis_migrations/migrations/0001_setup_extensions.py
@@ -12,10 +12,9 @@
                 CreateExtension('postgis_raster'),
             ]
         else:
             operations = []
 
-
 else:
 
     class Migration(migrations.Migration):
         operations = []

--- a/django:tests/gis_tests/rasterapp/migrations/0001_setup_extensions.py
+++ b/django:tests/gis_tests/rasterapp/migrations/0001_setup_extensions.py
@@ -12,10 +12,9 @@
                 CreateExtension('postgis_raster'),
             ]
         else:
             operations = []
 
-
 else:
 
     class Migration(migrations.Migration):
         operations = []

--- a/django:tests/model_forms/models.py
+++ b/django:tests/model_forms/models.py
@@ -216,11 +216,10 @@
         image = models.ImageField(storage=temp_storage, upload_to=upload_to)
 
         def __str__(self):
             return self.description
 
-
 except ImportError:
     test_images = False
 
 
 class Homepage(models.Model):

--- a/django:tests/utils_tests/test_module_loading.py
+++ b/django:tests/utils_tests/test_module_loading.py
@@ -212,11 +212,10 @@
             self.importer = zipimporter(*args, **kwargs)
 
         def find_spec(self, path, target=None):
             return self.importer.find_spec(path, target)
 
-
 else:
 
     class TestFinder:
         def __init__(self, *args, **kwargs):
             self.importer = zipimporter(*args, **kwargs)

--- a/hypothesis:hypothesis-python/src/hypothesis/entry_points.py
+++ b/hypothesis:hypothesis-python/src/hypothesis/entry_points.py
@@ -36,11 +36,10 @@
             # so we'll retain this fallback logic for some time to come.  See also
             # https://importlib-metadata.readthedocs.io/en/latest/using.html
             eps = importlib_metadata.entry_points().get("hypothesis", [])
         yield from eps
 
-
 except ImportError:
     # But if we're not on Python >= 3.8 and the importlib_metadata backport
     # is not installed, we fall back to pkg_resources anyway.
     try:
         import pkg_resources

--- a/hypothesis:hypothesis-python/src/hypothesis/extra/cli.py
+++ b/hypothesis:hypothesis-python/src/hypothesis/extra/cli.py
@@ -67,11 +67,10 @@
 
     def main():
         """If `click` is not installed, tell the user to install it then exit."""
         sys.stderr.write(MESSAGE.format("click"))
         sys.exit(1)
-
 
 else:
     # Ensure that Python scripts in the current working directory are importable,
     # on the principle that Ghostwriter should 'just work' for novice users.  Note
     # that we append rather than prepend to the module search path, so this will

--- a/hypothesis:hypothesis-python/src/hypothesis/internal/coverage.py
+++ b/hypothesis:hypothesis-python/src/hypothesis/internal/coverage.py
@@ -102,11 +102,10 @@
             with check_block(f.__name__, 2):
                 return f(*args, **kwargs)
 
         return accept
 
-
 else:  # pragma: no cover
 
     def check_function(f: Func) -> Func:
         return f
 

--- a/hypothesis:hypothesis-python/tests/cover/test_pretty.py
+++ b/hypothesis:hypothesis-python/tests/cover/test_pretty.py
@@ -304,11 +304,10 @@
         assert "SA" in output
 
         sb = SB()
         output = pretty.pretty(super(SA, sb))
         assert "SA" in output
-
 
 except AttributeError:
 
     def test_super_repr():
         pretty.pretty(super(SA))

--- a/pillow:Tests/helper.py
+++ b/pillow:Tests/helper.py
@@ -27,11 +27,10 @@
     class test_image_results:
         @staticmethod
         def upload(a, b):
             a.show()
             b.show()
-
 
 elif "GITHUB_ACTIONS" in os.environ:
     HAS_UPLOADER = True
 
     class test_image_results:
@@ -41,11 +40,10 @@
             os.makedirs(dir_errors, exist_ok=True)
             tmpdir = tempfile.mkdtemp(dir=dir_errors)
             a.save(os.path.join(tmpdir, "a.png"))
             b.save(os.path.join(tmpdir, "b.png"))
             return tmpdir
-
 
 else:
     try:
         import test_image_results
 

--- a/pillow:src/PIL/Image.py
+++ b/pillow:src/PIL/Image.py
@@ -73,11 +73,10 @@
                     DeprecationWarning,
                     stacklevel=2,
                 )
                 return categories[name]
         raise AttributeError(f"module '{__name__}' has no attribute '{name}'")
-
 
 else:
 
     from . import PILLOW_VERSION
 

--- a/pillow:src/PIL/__init__.py
+++ b/pillow:src/PIL/__init__.py
@@ -38,11 +38,10 @@
     def __getattr__(name):
         if name == "PILLOW_VERSION":
             _raise_version_warning()
             return __version__
         raise AttributeError(f"module '{__name__}' has no attribute '{name}'")
-
 
 else:
 
     class _Deprecated_Version(str):
         def __str__(self):

--- a/pytest:src/_pytest/_argcomplete.py
+++ b/pytest:src/_pytest/_argcomplete.py
@@ -106,11 +106,10 @@
     filescompleter: Optional[FastFilesCompleter] = FastFilesCompleter()
 
     def try_argcomplete(parser: argparse.ArgumentParser) -> None:
         argcomplete.autocomplete(parser, always_complete_options=False)
 
-
 else:
 
     def try_argcomplete(parser: argparse.ArgumentParser) -> None:
         pass
 

--- a/pytest:src/_pytest/assertion/rewrite.py
+++ b/pytest:src/_pytest/assertion/rewrite.py
@@ -321,11 +321,10 @@
             # we ignore any failure to write the cache file
             # there are many reasons, permission-denied, pycache dir being a
             # file etc.
             return False
         return True
-
 
 else:
 
     def _write_pyc(
         state: "AssertionState",

--- a/pytest:src/_pytest/compat.py
+++ b/pytest:src/_pytest/compat.py
@@ -191,11 +191,10 @@
 
     @contextmanager
     def nullcontext():
         yield
 
-
 else:
     from contextlib import nullcontext as nullcontext  # noqa: F401
 
 
 def get_default_arg_names(function: Callable[..., Any]) -> Tuple[str, ...]:

--- a/pytest:src/_pytest/pathlib.py
+++ b/pytest:src/_pytest/pathlib.py
@@ -560,11 +560,10 @@
 if sys.platform.startswith("win"):
 
     def _is_same(f1: str, f2: str) -> bool:
         return Path(f1) == Path(f2) or os.path.samefile(f1, f2)
 
-
 else:
 
     def _is_same(f1: str, f2: str) -> bool:
         return os.path.samefile(f1, f2)
 

--- a/sqlalchemy:lib/sqlalchemy/engine/row.py
+++ b/sqlalchemy:lib/sqlalchemy/engine/row.py
@@ -27,11 +27,10 @@
     # The extra function embedding is needed so that the
     # reconstructor function has the same signature whether or not
     # the extension is present.
     def rowproxy_reconstructor(cls, state):
         return safe_rowproxy_reconstructor(cls, state)
-
 
 except ImportError:
 
     def rowproxy_reconstructor(cls, state):
         obj = cls.__new__(cls)

--- a/sqlalchemy:lib/sqlalchemy/processors.py
+++ b/sqlalchemy:lib/sqlalchemy/processors.py
@@ -168,8 +168,7 @@
         # For example, the Python implementation might return
         # Decimal('5.00000') whereas the C implementation will
         # return Decimal('5'). These are equivalent of course.
         return DecimalResultProcessor(target_class, "%%.%df" % scale).process
 
-
 except ImportError:
     globals().update(py_fallback())

--- a/sqlalchemy:lib/sqlalchemy/testing/util.py
+++ b/sqlalchemy:lib/sqlalchemy/testing/util.py
@@ -70,11 +70,10 @@
     def random_choices(population, k=1):
         pop = list(population)
         # lame but works :)
         random.shuffle(pop)
         return pop[0:k]
-
 
 else:
 
     def random_choices(population, k=1):
         return random.choices(population, k=k)

--- a/sqlalchemy:lib/sqlalchemy/util/compat.py
+++ b/sqlalchemy:lib/sqlalchemy/util/compat.py
@@ -224,11 +224,10 @@
 
     from abc import ABC
 
     def _qualname(fn):
         return fn.__qualname__
-
 
 else:
     import base64
     import ConfigParser as configparser  # noqa
     import itertools
@@ -429,11 +428,10 @@
         result = "(" + ", ".join(specs) + ")"
         if "return" in annotations:
             result += formatreturns(formatannotation(annotations["return"]))
         return result
 
-
 else:
     from inspect import formatargspec as _inspect_formatargspec
 
     def inspect_formatargspec(*spec, **kw):
         # convert for a potential FullArgSpec from compat.getfullargspec()
@@ -471,11 +469,10 @@
             return [
                 f for f in dataclasses.fields(cls) if f not in super_fields
             ]
         else:
             return []
-
 
 else:
 
     def dataclass_fields(cls):
         return []

--- a/virtualenv:src/virtualenv/util/path/_sync.py
+++ b/virtualenv:src/virtualenv/util/path/_sync.py
@@ -12,11 +12,10 @@
 
 if PY2 and IS_CPYTHON and IS_WIN:  # CPython2 on Windows supports unicode paths if passed as unicode
 
     def norm(src):
         return ensure_text(str(src))
-
 
 else:
     norm = str
 
 

--- a/virtualenv:tasks/__main__zipapp.py
+++ b/virtualenv:tasks/__main__zipapp.py
@@ -128,11 +128,10 @@
                 return spec
 
         def module_repr(self, module):
             raise NotImplementedError
 
-
 else:
     # noinspection PyDeprecation
     from imp import new_module
 
     class VersionedFindLoad(VersionPlatformSelect):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F: empty lines Wasting vertical space efficiently.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistent spacing around top level functions
3 participants