diff --git a/CHANGES.rst b/CHANGES.rst index e862e5164..476a1f1cc 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -9,6 +9,8 @@ Added - ``--check`` returns 1 from the process but leaves files untouched if any file would require reformatting - Untracked i.e. freshly created Python files are now also reformatted +- ``-r `` / ``--revision `` can be used to specify the Git revision to compare + against when finding out modified lines. Defaults to ``HEAD`` as before. - ``--no-skip-string-normalization`` flag to override ``skip_string_normalization = true`` from a configuration file diff --git a/README.rst b/README.rst index cd94d05ac..febf73556 100644 --- a/README.rst +++ b/README.rst @@ -112,6 +112,11 @@ The following `command line arguments`_ can also be used to modify the defaults: .. code-block:: shell + -r REVISION, --revision REVISION + Git revision against which to compare the working + tree. Tags, branch names, commit hashes, and other + expressions like HEAD~5 work here. + --diff Don't write the files back, just output a diff for each file on stdout. Highlight syntax on screen if the `pygments` package is available. @@ -137,10 +142,12 @@ The following `command line arguments`_ can also be used to modify the defaults: *New in version 1.0.0:* isort_ is configured with ``-c`` and ``-l``, too. -*New in version 1.1.0:* The ``--check`` command line option. +*New in version 1.1.0:* The ``-r`` / ``--revision`` command line option. *New in version 1.1.0:* The ``--diff`` command line option. +*New in version 1.1.0:* The ``--check`` command line option. + *New in version 1.1.0:* The ``--no-skip-string-normalization`` command line option. .. _Black documentation about pyproject.toml: https://black.readthedocs.io/en/stable/pyproject_toml.html diff --git a/setup.cfg b/setup.cfg index cc478b4fd..1d70dbb2d 100644 --- a/setup.cfg +++ b/setup.cfg @@ -26,6 +26,7 @@ packages = find: install_requires = black typing-extensions ; python_version < "3.8" + dataclasses ; python_version < "3.7" python_requires = >=3.6 [options.packages.find] diff --git a/src/darker/__main__.py b/src/darker/__main__.py index 825761f35..e632186f3 100644 --- a/src/darker/__main__.py +++ b/src/darker/__main__.py @@ -19,7 +19,7 @@ def format_edited_parts( - srcs: Iterable[Path], enable_isort: bool, black_args: BlackArgs + srcs: Iterable[Path], revision: str, enable_isort: bool, black_args: BlackArgs ) -> Generator[Tuple[Path, str, str, List[str]], None, None]: """Black (and optional isort) formatting for chunks with edits since the last commit @@ -39,6 +39,7 @@ def format_edited_parts( 10. write the reformatted source back to the original file :param srcs: Directories and files to re-format + :param revision: The Git revision against which to compare the working tree :param enable_isort: ``True`` to also run ``isort`` first on each changed file :param black_args: Command-line arguments to send to ``black.FileMode`` :return: A generator which yields details about changes for each file which should @@ -46,10 +47,10 @@ def format_edited_parts( """ git_root = get_common_root(srcs) - changed_files = git_get_modified_files(srcs, git_root) - edited_linenums_differ = EditedLinenumsDiffer(git_root) + changed_files = git_get_modified_files(srcs, revision, git_root) + edited_linenums_differ = EditedLinenumsDiffer(git_root, revision) - for path_in_repo in changed_files: + for path_in_repo in sorted(changed_files): src = git_root / path_in_repo worktree_content = src.read_text() @@ -68,7 +69,7 @@ def format_edited_parts( for context_lines in range(max_context_lines + 1): # 2. diff HEAD and worktree for the file # 3. extract line numbers in each edited to-file for changed lines - edited_linenums = edited_linenums_differ.head_vs_lines( + edited_linenums = edited_linenums_differ.revision_vs_lines( path_in_repo, edited_lines, context_lines ) if ( @@ -199,7 +200,7 @@ def main(argv: List[str] = None) -> int: # We need both forms when showing diffs or modifying files. # Pass them both on to avoid back-and-forth conversion. for path, old_content, new_content, new_lines in format_edited_parts( - paths, args.isort, black_args + paths, args.revision, args.isort, black_args ): some_files_changed = True if args.diff: diff --git a/src/darker/command_line.py b/src/darker/command_line.py index 22eebee02..d22bbaa52 100644 --- a/src/darker/command_line.py +++ b/src/darker/command_line.py @@ -30,6 +30,15 @@ def parse_command_line(argv: List[str]) -> Namespace: help="Path(s) to the Python source file(s) to reformat", metavar="PATH", ) + parser.add_argument( + "-r", + "--revision", + default="HEAD", + help=( + "Git revision against which to compare the working tree. Tags, branch" + " names, commit hashes, and other expressions like HEAD~5 work here." + ), + ) isort_help = ["Also sort imports using the `isort` package"] if not isort: isort_help.append(f". {ISORT_INSTRUCTION} to enable usage of this option.") diff --git a/src/darker/git.py b/src/darker/git.py index db8af3ce5..64ffca4b0 100644 --- a/src/darker/git.py +++ b/src/darker/git.py @@ -2,6 +2,7 @@ import logging import sys +from dataclasses import dataclass from pathlib import Path from subprocess import CalledProcessError, check_output from typing import Iterable, List, Set @@ -11,16 +12,25 @@ logger = logging.getLogger(__name__) -def git_get_unmodified_content(path: Path, cwd: Path) -> List[str]: - """Get unmodified text lines of a file at Git HEAD +def git_get_unmodified_content(path: Path, revision: str, cwd: Path) -> List[str]: + """Get unmodified text lines of a file at a Git revision :param path: The relative path of the file in the Git repository + :param revision: The Git revision for which to get the file content :param cwd: The root of the Git repository """ - cmd = ["git", "show", f":./{path}"] + cmd = ["git", "show", f"{revision}:./{path}"] logger.debug("[%s]$ %s", cwd, " ".join(cmd)) - return check_output(cmd, cwd=str(cwd), encoding='utf-8').splitlines() + try: + return check_output(cmd, cwd=str(cwd), encoding="utf-8").splitlines() + except CalledProcessError as exc_info: + if exc_info.returncode == 128: + # The file didn't exist at the given revision. Act as if it was an empty + # file, so all current lines appear as edited. + return [] + else: + raise def should_reformat_file(path: Path) -> bool: @@ -40,15 +50,18 @@ def _git_check_output_lines(cmd: List[str], cwd: Path) -> List[str]: raise -def git_get_modified_files(paths: Iterable[Path], cwd: Path) -> Set[Path]: +def git_get_modified_files( + paths: Iterable[Path], revision: str, cwd: Path +) -> Set[Path]: """Ask Git for modified and untracked files - - ``git diff --name-only --relative HEAD -- `` + - ``git diff --name-only --relative -- `` - ``git ls-files --others --exclude-standard -- `` Return file names relative to the Git repository root. :paths: Paths to the files to diff + :param revision: Git revision to compare current working tree against :cwd: The Git repository root """ @@ -59,9 +72,12 @@ def git_get_modified_files(paths: Iterable[Path], cwd: Path) -> Set[Path]: "diff", "--name-only", "--relative", + # `revision` is inserted here if non-empty "--", *str_paths, ] + if revision: + diff_cmd.insert(diff_cmd.index("--"), revision) lines = _git_check_output_lines(diff_cmd, cwd) ls_files_cmd = [ "git", @@ -76,15 +92,25 @@ def git_get_modified_files(paths: Iterable[Path], cwd: Path) -> Set[Path]: return {path for path in changed_paths if should_reformat_file(cwd / path)} +@dataclass(frozen=True) class EditedLinenumsDiffer: - """Find out changed lines for a file compared to Git HEAD""" + """Find out changed lines for a file compared to a given Git revision""" - def __init__(self, git_root: Path): - self._git_root = git_root + git_root: Path + revision: str = "HEAD" - def head_vs_lines( + def revision_vs_lines( self, path_in_repo: Path, lines: List[str], context_lines: int ) -> List[int]: - head_lines = git_get_unmodified_content(path_in_repo, self._git_root) - edited_opcodes = diff_and_get_opcodes(head_lines, lines) + """For file `path_in_repo`, return changed line numbers from given revision + + :param path_in_repo: Path of the file to compare, relative to repository root + :param lines: The contents to compare to, e.g. from current working tree + :return: Line numbers of lines changed between the revision and given content + + """ + revision_lines = git_get_unmodified_content( + path_in_repo, self.revision, self.git_root + ) + edited_opcodes = diff_and_get_opcodes(revision_lines, lines) return list(opcodes_to_edit_linenums(edited_opcodes, context_lines)) diff --git a/src/darker/tests/conftest.py b/src/darker/tests/conftest.py index 37015fa49..8d1a336e0 100644 --- a/src/darker/tests/conftest.py +++ b/src/darker/tests/conftest.py @@ -1,7 +1,7 @@ import sys import types from subprocess import check_call -from typing import Dict +from typing import Dict, Optional from unittest.mock import patch import pytest @@ -25,15 +25,28 @@ def __init__(self, root: LocalPath): self.root = root def add( - self, paths_and_contents: Dict[str, str], commit: str = None + self, paths_and_contents: Dict[str, Optional[str]], commit: str = None ) -> Dict[str, LocalPath]: + """Add/remove/modify files and optionally commit the changes + + :param paths_and_contents: Paths of the files relative to repository root, and + new contents for the files as strings. ``None`` can + be specified as the contents in order to remove a + file. + :param commit: The message for the commit, or ``None`` to skip making a commit. + + """ absolute_paths = { relative_path: self.root / relative_path for relative_path in paths_and_contents } for relative_path, content in paths_and_contents.items(): - absolute_paths[relative_path].write(content, ensure=True) - check_call(["git", "add", relative_path], cwd=self.root) + path = absolute_paths[relative_path] + if content is None: + check_call(["git", "rm", "--", relative_path], cwd=self.root) + else: + path.write(content, ensure=True) + check_call(["git", "add", "--", relative_path], cwd=self.root) if commit: check_call(["git", "commit", "-m", commit], cwd=self.root) return absolute_paths diff --git a/src/darker/tests/test_command_line.py b/src/darker/tests/test_command_line.py index d54c3341f..fdb1aff70 100644 --- a/src/darker/tests/test_command_line.py +++ b/src/darker/tests/test_command_line.py @@ -132,21 +132,21 @@ def test_black_options_skip_string_normalization(git_repo, config, options, expe @pytest.mark.parametrize( 'options, expect', [ - (['a.py'], ({Path('a.py')}, False, {})), - (['--isort', 'a.py'], ({Path('a.py')}, True, {})), + (["a.py"], ({Path("a.py")}, "HEAD", False, {})), + (["--isort", "a.py"], ({Path("a.py")}, "HEAD", True, {})), ( - ['--config', 'my.cfg', 'a.py'], - ({Path('a.py')}, False, {'config': 'my.cfg'}), + ["--config", "my.cfg", "a.py"], + ({Path("a.py")}, "HEAD", False, {"config": "my.cfg"}), ), ( - ['--line-length', '90', 'a.py'], - ({Path('a.py')}, False, {'line_length': 90}), + ["--line-length", "90", "a.py"], + ({Path("a.py")}, "HEAD", False, {"line_length": 90}), ), ( - ['--skip-string-normalization', 'a.py'], - ({Path('a.py')}, False, {'skip_string_normalization': True}), + ["--skip-string-normalization", "a.py"], + ({Path("a.py")}, "HEAD", False, {"skip_string_normalization": True}), ), - (['--diff', 'a.py'], ({Path('a.py')}, False, {})), + (["--diff", "a.py"], ({Path("a.py")}, "HEAD", False, {})), ], ) def test_options(options, expect): diff --git a/src/darker/tests/test_git.py b/src/darker/tests/test_git.py index d2314707c..227352298 100644 --- a/src/darker/tests/test_git.py +++ b/src/darker/tests/test_git.py @@ -10,13 +10,20 @@ ) -def test_get_unmodified_content(git_repo): - paths = git_repo.add({'my.txt': 'original content'}, commit='Initial commit') +@pytest.mark.parametrize( + "revision, expect", + [("HEAD", ["modified content"]), ("HEAD^", ["original content"]), ("HEAD~2", []),], +) +def test_get_unmodified_content(git_repo, revision, expect): + git_repo.add({"my.txt": "original content"}, commit="Initial commit") + paths = git_repo.add({"my.txt": "modified content"}, commit="Initial commit") paths['my.txt'].write('new content') - original_content = git_get_unmodified_content(Path('my.txt'), cwd=git_repo.root) + original_content = git_get_unmodified_content( + Path("my.txt"), revision, cwd=git_repo.root + ) - assert original_content == ['original content'] + assert original_content == expect @pytest.mark.parametrize( @@ -69,7 +76,8 @@ def test_git_get_modified_files(git_repo, modify_paths, paths, expect): 'c/d.py': 'original', 'c/e.js': 'original', 'd/f/g.py': 'original', - } + }, + commit="Initial commit", ) for path, content in modify_paths.items(): absolute_path = git_repo.root / path @@ -77,7 +85,7 @@ def test_git_get_modified_files(git_repo, modify_paths, paths, expect): absolute_path.remove() else: absolute_path.write(content, ensure=True) - result = git_get_modified_files({root / p for p in paths}, cwd=root) + result = git_get_modified_files({root / p for p in paths}, "HEAD", cwd=root) assert {str(p) for p in result} == set(expect) @@ -97,5 +105,5 @@ def test_edited_linenums_differ_head_vs_lines(git_repo, context_lines, expect): git_repo.add({'a.py': '1\n2\n3\n4\n5\n6\n7\n8\n'}, commit='Initial commit') lines = ['1', '2', 'three', '4', '5', '6', 'seven', '8'] differ = EditedLinenumsDiffer(git_repo.root) - result = differ.head_vs_lines(Path('a.py'), lines, context_lines) + result = differ.revision_vs_lines(Path("a.py"), lines, context_lines) assert result == expect diff --git a/src/darker/tests/test_main.py b/src/darker/tests/test_main.py index 9a11bd3ce..c1d3b40c2 100644 --- a/src/darker/tests/test_main.py +++ b/src/darker/tests/test_main.py @@ -57,7 +57,7 @@ def test_isort_option_with_isort_calls_sortimports(tmpdir, run_isort, isort_args def test_format_edited_parts_empty(): with pytest.raises(ValueError): - list(darker.__main__.format_edited_parts([], False, {})) + list(darker.__main__.format_edited_parts([], "HEAD", False, {})) A_PY = ['import sys', 'import os', "print( '42')", ''] @@ -118,7 +118,9 @@ def test_format_edited_parts(git_repo, monkeypatch, enable_isort, black_args, ex paths['b.py'].write('print(42 )\n') changes = list( - darker.__main__.format_edited_parts([Path('a.py')], enable_isort, black_args) + darker.__main__.format_edited_parts( + [Path("a.py")], "HEAD", enable_isort, black_args + ) ) expect_changes = [(paths['a.py'], '\n'.join(A_PY), '\n'.join(expect), expect[:-1])] @@ -132,7 +134,7 @@ def test_format_edited_parts_all_unchanged(git_repo, monkeypatch): paths['a.py'].write('"properly"\n"formatted"\n') paths['b.py'].write('"not"\n"checked"\n') - result = list(darker.__main__.format_edited_parts([Path('a.py')], True, {})) + result = list(darker.__main__.format_edited_parts([Path("a.py")], "HEAD", True, {})) assert result == [] diff --git a/src/darker/tests/test_main_revision.py b/src/darker/tests/test_main_revision.py new file mode 100644 index 000000000..94fe53b28 --- /dev/null +++ b/src/darker/tests/test_main_revision.py @@ -0,0 +1,111 @@ +import sys + +if sys.version_info >= (3, 7): + from contextlib import nullcontext +else: + from contextlib import suppress as nullcontext + +import pytest + +from darker.__main__ import main + +# The following test is a bit dense, so some explanation is due. +# +# A Git repository with 3 commits is created. Python files with one line of code +# ("ORIGINAL=1") are added, modified or deleted in each commit. An abbreviated number is +# used for each commit in the test code: +# - 2: HEAD~2 add 4 files +# - 1: HEAD~1 (HEAD^) add 2 files, delete 1 file, keep 1 file, modify 2 files +# - 0: HEAD~0 (HEAD) delete 1 file, modify 1 file, keep 4 files +# +# In each test case, all of the 6 files are overwritten in the working tree with given +# content. +# +# `darker --diff --revision ` is called and the diff captured from stdout. +# +# Each case is gets as parameters: +# - the `` argument – which Git revision to compare to +# - `worktree_content` – the content to write to all the 6 known Python files +# - `expect` – a list of files expected to appear in `darker --diff` +# +# The Python file names tell the history of the file with a list of actions: +# +2 - added in HEAD~2 +# +1 - added in HEAD^ +# -1 - deleted in HEAD^ +# M1 - modified in HEAD^ +# -0 - deleted in HEAD +# M0 - modified in HEAD +# +# So e.g. the test case `HEAD~2`, `ORIGINAL=1` tells us that when `ORIGINAL=1` is +# written to all Python files and `darker --diff --revision HEAD~2` is called, the diff +# indicates reformatting in +# - +1.py – the file which was added in HEAD^ +# - +1M0.py – the file added in HEAD^ and overwritten in HEAD (with `MODIFIED=1`) +# All other files are missing from the diff since their content was the same as now +# (`ORIGINAL=1`) at HEAD~2. + + +@pytest.mark.parametrize( + "revision, worktree_content, expect", + [ + ("", "USERMOD=1\n", ["+1", "+1M0", "+2-1", "+2", "+2M1-0", "+2M1"]), + ("", "ORIGINAL=1\n", ["+1M0", "+2-1", "+2M1-0", "+2M1"]), + ("", "MODIFIED=1\n", ["+1", "+2-1", "+2", "+2M1-0"]), + ("HEAD", "USERMOD=1\n", ["+1", "+1M0", "+2-1", "+2", "+2M1-0", "+2M1"]), + ("HEAD", "ORIGINAL=1\n", ["+1M0", "+2-1", "+2M1-0", "+2M1"],), + ("HEAD", "MODIFIED=1\n", ["+1", "+2-1", "+2", "+2M1-0"]), + ("HEAD^", "USERMOD=1\n", ["+1", "+1M0", "+2-1", "+2", "+2M1-0", "+2M1"]), + ("HEAD^", "USERMOD=1\n", ["+1", "+1M0", "+2-1", "+2", "+2M1-0", "+2M1"]), + ("HEAD^", "ORIGINAL=1\n", ["+2-1", "+2M1-0", "+2M1"]), + ("HEAD^", "MODIFIED=1\n", ["+1", "+1M0", "+2-1", "+2"]), + ("HEAD~2", "USERMOD=1\n", ["+1", "+1M0", "+2-1", "+2", "+2M1-0", "+2M1"]), + ("HEAD~2", "ORIGINAL=1\n", ["+1", "+1M0"]), + ("HEAD~2", "MODIFIED=1\n", ["+1", "+1M0", "+2-1", "+2", "+2M1-0", "+2M1"]), + ("HEAD~3", "USERMOD=1\n", SystemExit), + ], +) +def test_revision(git_repo, monkeypatch, capsys, revision, worktree_content, expect): + monkeypatch.chdir(git_repo.root) + # 2: HEAD~2: + paths = git_repo.add( + { + "+2.py": "ORIGINAL=1\n", + "+2M1.py": "ORIGINAL=1\n", + "+2-1.py": "ORIGINAL=1\n", + "+2M1-0.py": "ORIGINAL=1\n", + }, + commit="First commit", + ) + # 1: HEAD~1 i.e. HEAD^ + paths.update( + git_repo.add( + { + "+2M1.py": "MODIFIED=1\n", + "+1.py": "ORIGINAL=1\n", + "+1M0.py": "ORIGINAL=1\n", + "+2-1.py": None, + "+2M1-0.py": "MODIFIED=1\n", + }, + commit="Second commit", + ) + ) + # 0: HEAD~0 i.e. HEAD: + git_repo.add( + {"+1M0.py": "MODIFIED=1\n", "+2M1-0.py": None}, commit="Third commit", + ) + # Working tree: + for path in paths.values(): + path.write(worktree_content) + arguments = ["--diff", "--revision", revision, "."] + should_raise = expect is SystemExit + + with pytest.raises(SystemExit) if should_raise else nullcontext(): + main(arguments) + + if not should_raise: + modified_files = [ + line[4:-3] + for line in capsys.readouterr().out.splitlines() + if line.startswith("+++ ") + ] + assert modified_files == expect