psf / black

The uncompromising Python code formatter
https://black.readthedocs.io/en/stable/
MIT License
38.62k stars 2.43k forks source link

Support for `--config` in pyproject.toml #1826

Open grazhopper opened 3 years ago

grazhopper commented 3 years ago

I am often dealing with projects that produce multiple libraries from the same repository, which looks a bit like:

src/lib1/pyproject.toml
src/lib1/setup.py
src/lib2/pyproject.toml
src/lib2/setup.py

All libs need to follow the same formatting rules, which means that the black configuration has to be duplicated in each pyproject.toml file.

A possible way to avoid the duplication could be to teach black to handle the already existing --config option when it is provided inside the normal pyproject.toml. Basically something like:

src/lib1/pyproject.toml
src/lib1/setup.py
src/lib2/pyproject.toml
src/lib2/setup.py
src/black.toml

with each pyproject.toml containing:

[tool.black]
config = '../black.toml'

A relative path would be interpreted relative to the folder containing the pyproject.toml file.

Thoughts? Would this be a welcome contribution?

felix-hilden commented 3 years ago

This seems like a reasonable idea for a solid use case 👌

ichard26 commented 3 years ago

I'm happy to support this use case, but I got some questions around the implementation here:

  1. Should we only support POSIX directory separators (/)? - we do this for the regex based options so it would be consistent and not surprising (I'm also not 100% sure how to take a Windows path and convert to pathlib.Path correctly)
  2. Should chaining be supported?
  3. Should user level configuration support this feature? - my guess is that the main reason why someone would use this feature in user config is to change Black's default at looking at pyproject.toml, but for that to work well, changes would have to be made to the project root detection AND this feature's implementation would have to special case this case to work from CWD
  4. Should absolute paths be supported?

I lean towards: 1-yes, 2-maybe, 3-no, and 4-yes.

Also, we might have to add cycle detection to avoid runaway recursion or infinite looping.

cooperlees commented 3 years ago
  1. Should we only support POSIX directory separators (/)? - we do this for the regex based options so it would be consistent and not surprising (I'm also not 100% sure how to take a Windows path and convert to pathlib.Path correctly)

This is exactly what pathlib makes easy. if you use the Path() object everywhere in code pathlib will workout to make a POSIX or Windows based path object. if the string in the config is c:\foo\config.conf on windows it will just work. The '/' overload is treated universally and when you str() or use a method on the respective platform it will "Do the right thing™️". If someone uses c:\config on POSIX, I'm sure an exception will be thrown.

To make pathing work, always do:

config_path = Path(config_dir) / "file"

NOT

config_path = Path(f"{config_dir}/file")

And you should be good. Having Unittests on Ubuntu / Windows will confirm. I did this all for bandersnatch ... and it all works in Winblows.

hauntsaninja commented 7 months ago

Note with https://github.com/psf/black/pull/4204 we now no longer look at pyproject.toml that is missing a [tool.black] section (we continue to always stop at VCS root). For some layouts, this gives you the ability to effectively inherit configuration from a pyproject.toml in the root of your monorepo.

I'm not sure we should implement this feature without biting the bullet and doing true hierarchical config, as requested in #3952. Given that we now have #4204, how many folks here have a reason to want just this without also #3952?