From 7230de1e9b221d723efc12a0bc28f83afea76846 Mon Sep 17 00:00:00 2001 From: Antti Kaihola <13725+akaihola@users.noreply.github.com> Date: Wed, 2 Oct 2024 17:51:37 +0300 Subject: [PATCH 1/6] test: failing test for isort's skip_glob setting --- src/darker/tests/test_main_isort.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/darker/tests/test_main_isort.py b/src/darker/tests/test_main_isort.py index 710381f8b..cdda101eb 100644 --- a/src/darker/tests/test_main_isort.py +++ b/src/darker/tests/test_main_isort.py @@ -2,6 +2,7 @@ # pylint: disable=no-member,redefined-outer-name,unused-argument,use-dict-literal +from textwrap import dedent from types import SimpleNamespace from unittest.mock import patch @@ -93,3 +94,26 @@ def test_isort_option_with_isort_calls_sortimports(run_isort, isort_args): settings_path=str(run_isort.root), **isort_args, ) + + +def test_isort_respects_skip_glob(tmp_path): + """Test that Darker respects isort's skip_glob setting.""" + # Create a pyproject.toml file with isort skip_glob configuration + configuration = dedent( + """ + [tool.isort] + skip_glob = ['*/conf/settings/*'] + filter_files = true + """ + ) + (tmp_path / "pyproject.toml").write_text(configuration) + # Create a file that should be skipped + settings_dir = tmp_path / "conf" / "settings" + settings_dir.mkdir(parents=True) + backoffice_py = settings_dir / "backoffice.py" + backoffice_py.write_text("import sys\nimport os\n") + + # Run darker with --isort + darker.__main__.main(["--isort", str(settings_dir / "backoffice.py")]) + + assert backoffice_py.read_text() == "import sys\nimport os\n" From 6750fb8771859c94eadec39803335f58b06aa78d Mon Sep 17 00:00:00 2001 From: Antti Kaihola <13725+akaihola@users.noreply.github.com> Date: Wed, 2 Oct 2024 19:47:14 +0300 Subject: [PATCH 2/6] fix: pass the path of the file to format to isort --- src/darker/__main__.py | 1 + src/darker/import_sorting.py | 27 ++++++++++++++++----------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/darker/__main__.py b/src/darker/__main__.py index 3689a0e82..19c9f23ed 100644 --- a/src/darker/__main__.py +++ b/src/darker/__main__.py @@ -145,6 +145,7 @@ def _modify_and_reformat_single_file( # noqa: PLR0913 rev2_isorted = apply_isort( rev2_content, relative_path_in_rev2, + root, exclude.isort, edited_linenums_differ, formatter.get_config_path(), diff --git a/src/darker/import_sorting.py b/src/darker/import_sorting.py index bcf284a60..2c46237f7 100644 --- a/src/darker/import_sorting.py +++ b/src/darker/import_sorting.py @@ -1,6 +1,7 @@ """Helpers for invoking ``isort`` and acting on its output""" import logging +from contextlib import suppress from pathlib import Path from typing import Any, Collection, List, Optional, TypedDict @@ -51,9 +52,10 @@ class IsortArgs(TypedDict, total=False): settings_path: str -def apply_isort( # pylint: disable=too-many-arguments +def apply_isort( # pylint: disable=too-many-arguments # noqa: PLR0913 content: TextDocument, src: Path, + root: Path, exclude: Collection[str], edited_linenums_differ: EditedLinenumsDiffer, config: Optional[str] = None, @@ -62,9 +64,10 @@ def apply_isort( # pylint: disable=too-many-arguments """Run isort on the given Python source file content :param content: The contents of the Python source code file to sort imports in - :param src: The relative path to the file. This must be the actual path in the - repository, which may differ from the path given on the command line in + :param src: The path to the file relative to ``root``. Note that this may differ + from the path given on the command line in case of VSCode temporary files. + :param root: The root directory based on which ``src`` is resolved. :param exclude: The file path patterns to exclude from import sorting :param edited_linenums_differ: Helper for finding out which lines were edited :param config: Path to configuration file @@ -82,7 +85,7 @@ def apply_isort( # pylint: disable=too-many-arguments if not edited_linenums: return content isort_args = _build_isort_args(src, config, line_length) - rev2_isorted = _call_isort_code(content, isort_args) + rev2_isorted = _call_isort_code(content, root / src, isort_args) # Get the chunks in the diff between the edited and import-sorted file isort_chunks = diff_chunks(content, rev2_isorted) if not isort_chunks: @@ -104,8 +107,9 @@ def _build_isort_args( ) -> IsortArgs: """Build ``isort.code()`` keyword arguments - :param src: The relative path to the file. This must be the actual path in the - repository, which may differ from the path given on the command line in + :param src: The path to the file. This must be either an absolute path or a relative + path from the current working directory. Note that this may differ from + the path given on the command line in case of VSCode temporary files. :param config: Path to configuration file :param line_length: Maximum line length to use @@ -121,10 +125,13 @@ def _build_isort_args( return isort_args -def _call_isort_code(content: TextDocument, isort_args: IsortArgs) -> TextDocument: +def _call_isort_code( + content: TextDocument, src: Path, isort_args: IsortArgs +) -> TextDocument: """Call ``isort.code()`` and return the result as a `TextDocument` object :param content: The contents of the Python source code file to sort imports in + :param src: The path to the file with the given content :param isort_args: Keyword arguments for ``isort.code()`` """ @@ -133,10 +140,8 @@ def _call_isort_code(content: TextDocument, isort_args: IsortArgs) -> TextDocume "isort.code(code=..., %s)", ", ".join(f"{k}={v!r}" for k, v in isort_args.items()), ) - try: - code = isort_code(code=code, **isort_args) - except isort.exceptions.FileSkipComment: - pass + with suppress(isort.exceptions.FileSkipped): + code = isort_code(code=code, file_path=src, **isort_args) return TextDocument.from_str( code, encoding=content.encoding, From aa1e68b3097e9c28d7d7b7714454c9022c86910e Mon Sep 17 00:00:00 2001 From: Antti Kaihola <13725+akaihola@users.noreply.github.com> Date: Wed, 2 Oct 2024 19:49:02 +0300 Subject: [PATCH 3/6] fix: resolve isort settings path based on full .py file path --- src/darker/import_sorting.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/darker/import_sorting.py b/src/darker/import_sorting.py index 2c46237f7..67d4d85e5 100644 --- a/src/darker/import_sorting.py +++ b/src/darker/import_sorting.py @@ -84,7 +84,7 @@ def apply_isort( # pylint: disable=too-many-arguments # noqa: PLR0913 ) # pylint: disable=duplicate-code if not edited_linenums: return content - isort_args = _build_isort_args(src, config, line_length) + isort_args = _build_isort_args(root / src, config, line_length) rev2_isorted = _call_isort_code(content, root / src, isort_args) # Get the chunks in the diff between the edited and import-sorted file isort_chunks = diff_chunks(content, rev2_isorted) From 0297acbc1f6f000b0e51d67534694e7f39c5f5f0 Mon Sep 17 00:00:00 2001 From: Antti Kaihola <13725+akaihola@users.noreply.github.com> Date: Wed, 2 Oct 2024 19:49:37 +0300 Subject: [PATCH 4/6] fix: update tests to match repaired isort path behavior --- src/darker/tests/test_import_sorting.py | 24 ++++++++++++++++++------ src/darker/tests/test_main_isort.py | 2 ++ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/darker/tests/test_import_sorting.py b/src/darker/tests/test_import_sorting.py index 19d02d4aa..0d5baa349 100644 --- a/src/darker/tests/test_import_sorting.py +++ b/src/darker/tests/test_import_sorting.py @@ -71,7 +71,11 @@ def test_apply_isort(apply_isort_repo_root, encoding, newline, content, expect): content_ = TextDocument.from_lines(content, encoding=encoding, newline=newline) result = darker.import_sorting.apply_isort( - content_, src, exclude=[], edited_linenums_differ=edited_linenums_differ + content_, + src, + git_repo.root, + exclude=[], + edited_linenums_differ=edited_linenums_differ, ) assert result.lines == expect @@ -106,7 +110,11 @@ def test_apply_isort_exclude( content_ = TextDocument.from_lines(content, encoding=encoding, newline=newline) result = darker.import_sorting.apply_isort( - content_, src, exclude=exclude, edited_linenums_differ=edited_linenums_differ + content_, + src, + git_repo.root, + exclude=exclude, + edited_linenums_differ=edited_linenums_differ, ) assert result.lines == expect @@ -148,10 +156,10 @@ def test_apply_isort_exclude( ), ), ) -def test_isort_config(monkeypatch, tmpdir, line_length, settings_file, expect): +def test_isort_config(monkeypatch, tmp_path, line_length, settings_file, expect): """``apply_isort()`` parses ``pyproject.toml``correctly""" - monkeypatch.chdir(tmpdir) - (tmpdir / "pyproject.toml").write( + monkeypatch.chdir(tmp_path) + (tmp_path / "pyproject.toml").write_text( dedent( f"""\ [tool.isort] @@ -161,11 +169,14 @@ def test_isort_config(monkeypatch, tmpdir, line_length, settings_file, expect): ) content = "from module import ab, cd, ef, gh, ij, kl, mn, op, qr, st, uv, wx, yz" - config = str(tmpdir / settings_file) if settings_file else None + # isort doesn't read the file but skips formatting if the file doesn't exist: + (tmp_path / "test1.py").write_text(content) + config = str(tmp_path / settings_file) if settings_file else None actual = darker.import_sorting.apply_isort( TextDocument.from_str(content), Path("test1.py"), + tmp_path, set(), EditedLinenumsDiffer(Path("."), RevisionRange("master", "HEAD")), config, @@ -206,6 +217,7 @@ def test_isort_file_skip_comment(): actual = darker.import_sorting.apply_isort( TextDocument.from_str(content), Path("test1.py"), + Path(), set(), EditedLinenumsDiffer(Path("."), RevisionRange("master", "HEAD")), ) diff --git a/src/darker/tests/test_main_isort.py b/src/darker/tests/test_main_isort.py index cdda101eb..1d1a58d88 100644 --- a/src/darker/tests/test_main_isort.py +++ b/src/darker/tests/test_main_isort.py @@ -2,6 +2,7 @@ # pylint: disable=no-member,redefined-outer-name,unused-argument,use-dict-literal +from pathlib import Path from textwrap import dedent from types import SimpleNamespace from unittest.mock import patch @@ -91,6 +92,7 @@ def test_isort_option_with_isort_calls_sortimports(run_isort, isort_args): """Relevant config options are passed from command line to ``isort``.""" run_isort.isort_code.assert_called_once_with( code="changed", + file_path=Path(run_isort.root / "test1.py"), settings_path=str(run_isort.root), **isort_args, ) From af499683e8c9ca9eb584f724c2cb511b72b3e8fb Mon Sep 17 00:00:00 2001 From: Antti Kaihola <13725+akaihola@users.noreply.github.com> Date: Wed, 2 Oct 2024 20:13:56 +0300 Subject: [PATCH 5/6] style: clarify .py file path argument names - `relative_path`: path relative to a `root` directory - `path_from_cwd`: either relative to working directory or an absolute path --- src/darker/import_sorting.py | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/src/darker/import_sorting.py b/src/darker/import_sorting.py index 67d4d85e5..b4698dc59 100644 --- a/src/darker/import_sorting.py +++ b/src/darker/import_sorting.py @@ -54,7 +54,7 @@ class IsortArgs(TypedDict, total=False): def apply_isort( # pylint: disable=too-many-arguments # noqa: PLR0913 content: TextDocument, - src: Path, + relative_path: Path, root: Path, exclude: Collection[str], edited_linenums_differ: EditedLinenumsDiffer, @@ -64,9 +64,9 @@ def apply_isort( # pylint: disable=too-many-arguments # noqa: PLR0913 """Run isort on the given Python source file content :param content: The contents of the Python source code file to sort imports in - :param src: The path to the file relative to ``root``. Note that this may differ - from the path given on the command line in - case of VSCode temporary files. + :param relative_path: The path to the file relative to ``root``. Note that this may + differ from the path given on the command line in case of + VSCode temporary files. :param root: The root directory based on which ``src`` is resolved. :param exclude: The file path patterns to exclude from import sorting :param edited_linenums_differ: Helper for finding out which lines were edited @@ -75,17 +75,17 @@ def apply_isort( # pylint: disable=too-many-arguments # noqa: PLR0913 :return: Original Python source file contents with imports sorted """ - if glob_any(src, exclude): + if glob_any(relative_path, exclude): return content edited_linenums = edited_linenums_differ.revision_vs_lines( - src, + relative_path, content, context_lines=0, ) # pylint: disable=duplicate-code if not edited_linenums: return content - isort_args = _build_isort_args(root / src, config, line_length) - rev2_isorted = _call_isort_code(content, root / src, isort_args) + isort_args = _build_isort_args(root / relative_path, config, line_length) + rev2_isorted = _call_isort_code(content, root / relative_path, isort_args) # Get the chunks in the diff between the edited and import-sorted file isort_chunks = diff_chunks(content, rev2_isorted) if not isort_chunks: @@ -101,16 +101,16 @@ def apply_isort( # pylint: disable=too-many-arguments # noqa: PLR0913 def _build_isort_args( - src: Path, + path_from_cwd: Path, config: Optional[str] = None, line_length: Optional[int] = None, ) -> IsortArgs: """Build ``isort.code()`` keyword arguments - :param src: The path to the file. This must be either an absolute path or a relative - path from the current working directory. Note that this may differ from - the path given on the command line in - case of VSCode temporary files. + :param path_from_cwd: The path to the file. This must be either an absolute path or + a relative path from the current working directory. Note that + this may differ from the path given on the command line in + case of VSCode temporary files. :param config: Path to configuration file :param line_length: Maximum line length to use @@ -119,19 +119,20 @@ def _build_isort_args( if config: isort_args["settings_file"] = config else: - isort_args["settings_path"] = str(find_project_root((str(src),))) + isort_args["settings_path"] = str(find_project_root((str(path_from_cwd),))) if line_length: isort_args["line_length"] = line_length return isort_args def _call_isort_code( - content: TextDocument, src: Path, isort_args: IsortArgs + content: TextDocument, path_from_cwd: Path, isort_args: IsortArgs ) -> TextDocument: """Call ``isort.code()`` and return the result as a `TextDocument` object :param content: The contents of the Python source code file to sort imports in - :param src: The path to the file with the given content + :param path_from_cwd: The path to the file with the given content, either relative + to the current working directory or an absolute path. :param isort_args: Keyword arguments for ``isort.code()`` """ @@ -141,7 +142,7 @@ def _call_isort_code( ", ".join(f"{k}={v!r}" for k, v in isort_args.items()), ) with suppress(isort.exceptions.FileSkipped): - code = isort_code(code=code, file_path=src, **isort_args) + code = isort_code(code=code, file_path=path_from_cwd, **isort_args) return TextDocument.from_str( code, encoding=content.encoding, From fee25b7e35067d94de822e840253a76f95e9a174 Mon Sep 17 00:00:00 2001 From: Antti Kaihola <13725+akaihola@users.noreply.github.com> Date: Wed, 2 Oct 2024 20:09:30 +0300 Subject: [PATCH 6/6] chore: amend .gitignore and flake8 excludes --- .gitignore | 1 + setup.cfg | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/.gitignore b/.gitignore index c452eb6d3..51a09e5f6 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,4 @@ __pycache__/ /.vscode/ /build /venv +/.venv diff --git a/setup.cfg b/setup.cfg index 0016a1517..e7e3ea54c 100644 --- a/setup.cfg +++ b/setup.cfg @@ -94,6 +94,10 @@ release = [flake8] # Line length according to Black rules max-line-length = 88 +# Skip some directories +exclude = + .* + build # Ignore rules which conflict with Black ignore = # C408 Unnecessary dict call - rewrite as a literal.