From 9451c57d1c60d70ddd55e21b44382ca96637398e Mon Sep 17 00:00:00 2001 From: Harish Rajagopal Date: Thu, 1 Apr 2021 18:39:18 +0200 Subject: [PATCH] Support for top-level user configuration (#1899) * Added support for top-level user configuration At the user level, a TOML config can be specified in the following locations: * Windows: ~\.black * Unix-like: $XDG_CONFIG_HOME/black (~/.config/black fallback) Instead of changing env vars for the entire black-primer process, they are now changed only for the black subprocess, using a tmpdir. --- README.md | 14 ++++++++++++ docs/pyproject_toml.md | 14 ++++++++++++ src/black/__init__.py | 22 ++++++++++++++++++- src/black_primer/lib.py | 47 ++++++++++++++++++++++++----------------- tests/test_black.py | 36 +++++++++++++++++++++++++++++-- 5 files changed, 111 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index 0be356e..b19e6e5 100644 --- a/README.md +++ b/README.md @@ -293,6 +293,20 @@ parent directories. It stops looking when it finds the file, or a `.git` directo If you're formatting standard input, _Black_ will look for configuration starting from the current working directory. +You can use a "global" configuration, stored in a specific location in your home +directory. This will be used as a fallback configuration, that is, it will be used if +and only if _Black_ doesn't find any configuration as mentioned above. Depending on your +operating system, this configuration file should be stored as: + +- Windows: `~\.black` +- Unix-like (Linux, MacOS, etc.): `$XDG_CONFIG_HOME/black` (`~/.config/black` if the + `XDG_CONFIG_HOME` environment variable is not set) + +Note that these are paths to the TOML file itself (meaning that they shouldn't be named +as `pyproject.toml`), not directories where you store the configuration. Here, `~` +refers to the path to your home directory. On Windows, this will be something like +`C:\\Users\UserName`. + You can also explicitly specify the path to a particular file that you want with `--config`. In this situation _Black_ will not look for any other file. diff --git a/docs/pyproject_toml.md b/docs/pyproject_toml.md index 9acc4c0..77c12b7 100644 --- a/docs/pyproject_toml.md +++ b/docs/pyproject_toml.md @@ -28,6 +28,20 @@ parent directories. It stops looking when it finds the file, or a `.git` directo If you're formatting standard input, _Black_ will look for configuration starting from the current working directory. +You can use a "global" configuration, stored in a specific location in your home +directory. This will be used as a fallback configuration, that is, it will be used if +and only if _Black_ doesn't find any configuration as mentioned above. Depending on your +operating system, this configuration file should be stored as: + +- Windows: `~\.black` +- Unix-like (Linux, MacOS, etc.): `$XDG_CONFIG_HOME/black` (`~/.config/black` if the + `XDG_CONFIG_HOME` environment variable is not set) + +Note that these are paths to the TOML file itself (meaning that they shouldn't be named +as `pyproject.toml`), not directories where you store the configuration. Here, `~` +refers to the path to your home directory. On Windows, this will be something like +`C:\\Users\UserName`. + You can also explicitly specify the path to a particular file that you want with `--config`. In this situation _Black_ will not look for any other file. diff --git a/src/black/__init__.py b/src/black/__init__.py index 52a5769..e85ffb3 100644 --- a/src/black/__init__.py +++ b/src/black/__init__.py @@ -306,7 +306,11 @@ def find_pyproject_toml(path_search_start: Iterable[str]) -> Optional[str]: """Find the absolute filepath to a pyproject.toml if it exists""" path_project_root = find_project_root(path_search_start) path_pyproject_toml = path_project_root / "pyproject.toml" - return str(path_pyproject_toml) if path_pyproject_toml.is_file() else None + if path_pyproject_toml.is_file(): + return str(path_pyproject_toml) + + path_user_pyproject_toml = find_user_pyproject_toml() + return str(path_user_pyproject_toml) if path_user_pyproject_toml.is_file() else None def parse_pyproject_toml(path_config: str) -> Dict[str, Any]: @@ -6248,6 +6252,22 @@ def find_project_root(srcs: Iterable[str]) -> Path: return directory +@lru_cache() +def find_user_pyproject_toml() -> Path: + r"""Return the path to the top-level user configuration for black. + + This looks for ~\.black on Windows and ~/.config/black on Linux and other + Unix systems. + """ + if sys.platform == "win32": + # Windows + user_config_path = Path.home() / ".black" + else: + config_root = os.environ.get("XDG_CONFIG_HOME", "~/.config") + user_config_path = Path(config_root).expanduser() / "black" + return user_config_path.resolve() + + @dataclass class Report: """Provides a reformatting counter. Can be rendered with `str(report)`.""" diff --git a/src/black_primer/lib.py b/src/black_primer/lib.py index 39ae93b..7999f6a 100644 --- a/src/black_primer/lib.py +++ b/src/black_primer/lib.py @@ -13,6 +13,7 @@ from platform import system from shutil import rmtree, which from subprocess import CalledProcessError from sys import version_info +from tempfile import TemporaryDirectory from typing import Any, Callable, Dict, NamedTuple, Optional, Sequence, Tuple from urllib.parse import urlparse @@ -121,28 +122,36 @@ async def black_run( cmd.extend(*project_config["cli_arguments"]) cmd.extend(["--check", "--diff", "."]) - try: - _stdout, _stderr = await _gen_check_output(cmd, cwd=repo_path) - except asyncio.TimeoutError: - results.stats["failed"] += 1 - LOG.error(f"Running black for {repo_path} timed out ({cmd})") - except CalledProcessError as cpe: - # TODO: Tune for smarter for higher signal - # If any other return value than 1 we raise - can disable project in config - if cpe.returncode == 1: - if not project_config["expect_formatting_changes"]: + with TemporaryDirectory() as tmp_path: + # Prevent reading top-level user configs by manipulating envionment variables + env = { + **os.environ, + "XDG_CONFIG_HOME": tmp_path, # Unix-like + "USERPROFILE": tmp_path, # Windows (changes `Path.home()` output) + } + + try: + _stdout, _stderr = await _gen_check_output(cmd, cwd=repo_path, env=env) + except asyncio.TimeoutError: + results.stats["failed"] += 1 + LOG.error(f"Running black for {repo_path} timed out ({cmd})") + except CalledProcessError as cpe: + # TODO: Tune for smarter for higher signal + # If any other return value than 1 we raise - can disable project in config + if cpe.returncode == 1: + if not project_config["expect_formatting_changes"]: + results.stats["failed"] += 1 + results.failed_projects[repo_path.name] = cpe + else: + results.stats["success"] += 1 + return + elif cpe.returncode > 1: results.stats["failed"] += 1 results.failed_projects[repo_path.name] = cpe - else: - results.stats["success"] += 1 - return - elif cpe.returncode > 1: - results.stats["failed"] += 1 - results.failed_projects[repo_path.name] = cpe - return + return - LOG.error(f"Unknown error with {repo_path}") - raise + LOG.error(f"Unknown error with {repo_path}") + raise # If we get here and expect formatting changes something is up if project_config["expect_formatting_changes"]: diff --git a/tests/test_black.py b/tests/test_black.py index 72e16a3..c603233 100644 --- a/tests/test_black.py +++ b/tests/test_black.py @@ -296,6 +296,7 @@ class BlackTestCase(BlackBaseTestCase): def test_expression_diff(self) -> None: source, _ = read_data("expression.py") + config = THIS_DIR / "data" / "empty_pyproject.toml" expected, _ = read_data("expression.diff") tmp_file = Path(black.dump_to_file(source)) diff_header = re.compile( @@ -303,7 +304,9 @@ class BlackTestCase(BlackBaseTestCase): r"\d\d:\d\d:\d\d\.\d\d\d\d\d\d \+\d\d\d\d" ) try: - result = BlackRunner().invoke(black.main, ["--diff", str(tmp_file)]) + result = BlackRunner().invoke( + black.main, ["--diff", str(tmp_file), f"--config={config}"] + ) self.assertEqual(result.exit_code, 0) finally: os.unlink(tmp_file) @@ -320,11 +323,12 @@ class BlackTestCase(BlackBaseTestCase): def test_expression_diff_with_color(self) -> None: source, _ = read_data("expression.py") + config = THIS_DIR / "data" / "empty_pyproject.toml" expected, _ = read_data("expression.diff") tmp_file = Path(black.dump_to_file(source)) try: result = BlackRunner().invoke( - black.main, ["--diff", "--color", str(tmp_file)] + black.main, ["--diff", "--color", str(tmp_file), f"--config={config}"] ) finally: os.unlink(tmp_file) @@ -1842,6 +1846,34 @@ class BlackTestCase(BlackBaseTestCase): self.assertEqual(black.find_project_root((src_dir,)), src_dir.resolve()) self.assertEqual(black.find_project_root((src_python,)), src_dir.resolve()) + @patch("black.find_user_pyproject_toml", black.find_user_pyproject_toml.__wrapped__) + def test_find_user_pyproject_toml_linux(self) -> None: + if system() == "Windows": + return + + # Test if XDG_CONFIG_HOME is checked + with TemporaryDirectory() as workspace: + tmp_user_config = Path(workspace) / "black" + with patch.dict("os.environ", {"XDG_CONFIG_HOME": workspace}): + self.assertEqual( + black.find_user_pyproject_toml(), tmp_user_config.resolve() + ) + + # Test fallback for XDG_CONFIG_HOME + with patch.dict("os.environ"): + os.environ.pop("XDG_CONFIG_HOME", None) + fallback_user_config = Path("~/.config").expanduser() / "black" + self.assertEqual( + black.find_user_pyproject_toml(), fallback_user_config.resolve() + ) + + def test_find_user_pyproject_toml_windows(self) -> None: + if system() != "Windows": + return + + user_config_path = Path.home() / ".black" + self.assertEqual(black.find_user_pyproject_toml(), user_config_path.resolve()) + def test_bpo_33660_workaround(self) -> None: if system() == "Windows": return -- 2.39.5