From fccbdc8ec51670e0c4bce3920c937c0b5d474d75 Mon Sep 17 00:00:00 2001 From: Antti Kaihola <13725+akaihola@users.noreply.github.com> Date: Sat, 8 Aug 2020 01:26:22 +0300 Subject: [PATCH 1/8] -r/--revision argument - what to compare to (#42) --- CHANGES.rst | 2 ++ README.rst | 7 +++- setup.cfg | 1 + src/darker/__main__.py | 11 +++--- src/darker/command_line.py | 6 ++++ src/darker/git.py | 50 ++++++++++++++++++++------- src/darker/tests/test_command_line.py | 12 +++---- src/darker/tests/test_git.py | 11 +++--- src/darker/tests/test_main.py | 8 +++-- 9 files changed, 77 insertions(+), 31 deletions(-) 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..e5c9a8fd0 100644 --- a/README.rst +++ b/README.rst @@ -112,6 +112,9 @@ 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 + --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 +140,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..03f157f03 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,8 +47,8 @@ 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: src = git_root / path_in_repo @@ -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..369328e3a 100644 --- a/src/darker/command_line.py +++ b/src/darker/command_line.py @@ -30,6 +30,12 @@ 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", + ) 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..45db1e7a9 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/test_command_line.py b/src/darker/tests/test_command_line.py index d54c3341f..650f1900c 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'}), + ({Path('a.py')}, "HEAD", False, {'config': 'my.cfg'}), ), ( ['--line-length', '90', 'a.py'], - ({Path('a.py')}, False, {'line_length': 90}), + ({Path('a.py')}, "HEAD", False, {'line_length': 90}), ), ( ['--skip-string-normalization', 'a.py'], - ({Path('a.py')}, False, {'skip_string_normalization': True}), + ({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..439071f5a 100644 --- a/src/darker/tests/test_git.py +++ b/src/darker/tests/test_git.py @@ -14,7 +14,9 @@ def test_get_unmodified_content(git_repo): paths = git_repo.add({'my.txt': 'original 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"), "HEAD", cwd=git_repo.root + ) assert original_content == ['original content'] @@ -69,7 +71,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 +80,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 +100,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..95170ea19 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 == [] From 53744ff3d4f733648553ce1aab65a93ab6f6c2a6 Mon Sep 17 00:00:00 2001 From: Antti Kaihola <13725+akaihola@users.noreply.github.com> Date: Sat, 8 Aug 2020 14:01:34 +0300 Subject: [PATCH 2/8] Output diffs for files in alphabetic order Makes testing easier. --- src/darker/__main__.py | 2 +- src/darker/tests/test_main_revision.py | 0 2 files changed, 1 insertion(+), 1 deletion(-) create mode 100644 src/darker/tests/test_main_revision.py diff --git a/src/darker/__main__.py b/src/darker/__main__.py index 03f157f03..e632186f3 100644 --- a/src/darker/__main__.py +++ b/src/darker/__main__.py @@ -50,7 +50,7 @@ def format_edited_parts( 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() diff --git a/src/darker/tests/test_main_revision.py b/src/darker/tests/test_main_revision.py new file mode 100644 index 000000000..e69de29bb From 81be51547fe29520702f2db9f60cc24cdacefc8b Mon Sep 17 00:00:00 2001 From: Antti Kaihola <13725+akaihola@users.noreply.github.com> Date: Sat, 8 Aug 2020 14:03:20 +0300 Subject: [PATCH 3/8] Basic unit test for `--revision` In unit tests, take advantage of `contextlib.suppress()` so separate code paths with and without a context manager are not needed. See https://stackoverflow.com/a/45187287/15770 --- src/darker/tests/test_main_revision.py | 111 +++++++++++++++++++++++++ 1 file changed, 111 insertions(+) diff --git a/src/darker/tests/test_main_revision.py b/src/darker/tests/test_main_revision.py index e69de29bb..d01026727 100644 --- a/src/darker/tests/test_main_revision.py +++ 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 + + +@pytest.mark.parametrize( + 'revision, expect', + [ + ( + "", + [ + "appear", + "appear_and_modify", + "delete", + "modify", + "modify_and_delete", + "never_change", + ], + ), + ( + "HEAD", + [ + "appear", + "appear_and_modify", + "delete", + "modify", + "modify_and_delete", + "never_change", + ], + ), + ( + "HEAD^", + [ + "appear", + "appear_and_modify", + "delete", + "modify", + "modify_and_delete", + "never_change", + ], + ), + ( + "HEAD~2", + [ + "appear", + "appear_and_modify", + "delete", + "modify", + "modify_and_delete", + "never_change", + ], + ), + ("HEAD~3", SystemExit), + ], +) +def test_revision(git_repo, monkeypatch, capsys, revision, expect): + monkeypatch.chdir(git_repo.root) + paths = git_repo.add( + { + 'never_change.py': 'ORIGINAL = 1\n', + 'modify.py': 'ORIGINAL = 1\n', + 'delete.py': 'ORIGINAL = 1\n', + 'modify_and_delete.py': 'ORIGINAL = 1\n', + }, + commit='First commit', + ) + paths.update( + git_repo.add( + { + 'modify.py': 'MODIFIED = 1\n', + 'appear.py': 'ADDED = 1\n', + 'appear_and_modify.py': 'ORIGINAL = 1\n', + 'delete.py': None, + 'modify_and_delete.py': 'MODIFIED = 1\n', + }, + commit='Second commit', + ) + ) + git_repo.add( + {'appear_and_modify.py': 'MODIFIED = 1\n', 'modify_and_delete.py': None}, + commit='Third commit', + ) + for path in paths.values(): + path.write('USER_MODIFICATION=1\n') + arguments = ["--diff", "--revision", revision, '.'] + should_raise = expect is SystemExit + + with pytest.raises(SystemExit) if should_raise else nullcontext(): + main(arguments) + + if not should_raise: + expect_diff_output = [ + line + for name in expect + for line in [ + f'--- {name}.py', + f'+++ {name}.py', + '@@ -1 +1 @@', + '', + '-USER_MODIFICATION=1', + '+USER_MODIFICATION = 1', + ] + ] + assert capsys.readouterr().out.splitlines() == expect_diff_output From c19afbbb0964ea5239ef7e39c6e551ff1d46c028 Mon Sep 17 00:00:00 2001 From: Antti Kaihola <13725+akaihola@users.noreply.github.com> Date: Sat, 8 Aug 2020 18:10:11 +0300 Subject: [PATCH 4/8] The `git_repo` fixture now supports deleting files --- src/darker/tests/conftest.py | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) 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 From 78f51ad40b1215d761fed763134ac2763ae73fce Mon Sep 17 00:00:00 2001 From: Antti Kaihola <13725+akaihola@users.noreply.github.com> Date: Sat, 8 Aug 2020 18:10:37 +0300 Subject: [PATCH 5/8] Functional test for the `--revision` option --- src/darker/tests/test_main_revision.py | 148 ++++++++++++------------- 1 file changed, 74 insertions(+), 74 deletions(-) diff --git a/src/darker/tests/test_main_revision.py b/src/darker/tests/test_main_revision.py index d01026727..94fe53b28 100644 --- a/src/darker/tests/test_main_revision.py +++ b/src/darker/tests/test_main_revision.py @@ -9,103 +9,103 @@ 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, expect', + "revision, worktree_content, expect", [ - ( - "", - [ - "appear", - "appear_and_modify", - "delete", - "modify", - "modify_and_delete", - "never_change", - ], - ), - ( - "HEAD", - [ - "appear", - "appear_and_modify", - "delete", - "modify", - "modify_and_delete", - "never_change", - ], - ), - ( - "HEAD^", - [ - "appear", - "appear_and_modify", - "delete", - "modify", - "modify_and_delete", - "never_change", - ], - ), - ( - "HEAD~2", - [ - "appear", - "appear_and_modify", - "delete", - "modify", - "modify_and_delete", - "never_change", - ], - ), - ("HEAD~3", SystemExit), + ("", "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, expect): +def test_revision(git_repo, monkeypatch, capsys, revision, worktree_content, expect): monkeypatch.chdir(git_repo.root) + # 2: HEAD~2: paths = git_repo.add( { - 'never_change.py': 'ORIGINAL = 1\n', - 'modify.py': 'ORIGINAL = 1\n', - 'delete.py': 'ORIGINAL = 1\n', - 'modify_and_delete.py': 'ORIGINAL = 1\n', + "+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', + commit="First commit", ) + # 1: HEAD~1 i.e. HEAD^ paths.update( git_repo.add( { - 'modify.py': 'MODIFIED = 1\n', - 'appear.py': 'ADDED = 1\n', - 'appear_and_modify.py': 'ORIGINAL = 1\n', - 'delete.py': None, - 'modify_and_delete.py': 'MODIFIED = 1\n', + "+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', + commit="Second commit", ) ) + # 0: HEAD~0 i.e. HEAD: git_repo.add( - {'appear_and_modify.py': 'MODIFIED = 1\n', 'modify_and_delete.py': None}, - commit='Third commit', + {"+1M0.py": "MODIFIED=1\n", "+2M1-0.py": None}, commit="Third commit", ) + # Working tree: for path in paths.values(): - path.write('USER_MODIFICATION=1\n') - arguments = ["--diff", "--revision", revision, '.'] + 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: - expect_diff_output = [ - line - for name in expect - for line in [ - f'--- {name}.py', - f'+++ {name}.py', - '@@ -1 +1 @@', - '', - '-USER_MODIFICATION=1', - '+USER_MODIFICATION = 1', - ] + modified_files = [ + line[4:-3] + for line in capsys.readouterr().out.splitlines() + if line.startswith("+++ ") ] - assert capsys.readouterr().out.splitlines() == expect_diff_output + assert modified_files == expect From 199fc7df6fe2cf46d030a41b64938c650925c528 Mon Sep 17 00:00:00 2001 From: Antti Kaihola <13725+akaihola@users.noreply.github.com> Date: Sat, 8 Aug 2020 18:50:45 +0300 Subject: [PATCH 6/8] Test for `get_unmodified_content` with revisions --- src/darker/tests/test_git.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/darker/tests/test_git.py b/src/darker/tests/test_git.py index 439071f5a..01ccf4e78 100644 --- a/src/darker/tests/test_git.py +++ b/src/darker/tests/test_git.py @@ -10,15 +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"), "HEAD", cwd=git_repo.root + Path("my.txt"), revision, cwd=git_repo.root ) - assert original_content == ['original content'] + assert original_content == expect @pytest.mark.parametrize( From bc240d1764f1c5b1ed2f47d9c935623918236223 Mon Sep 17 00:00:00 2001 From: Antti Kaihola <13725+akaihola@users.noreply.github.com> Date: Sat, 8 Aug 2020 23:31:02 +0300 Subject: [PATCH 7/8] Mention examples of revision expressions --- README.rst | 4 +++- src/darker/command_line.py | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/README.rst b/README.rst index e5c9a8fd0..febf73556 100644 --- a/README.rst +++ b/README.rst @@ -113,7 +113,9 @@ 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 + 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 diff --git a/src/darker/command_line.py b/src/darker/command_line.py index 369328e3a..d22bbaa52 100644 --- a/src/darker/command_line.py +++ b/src/darker/command_line.py @@ -34,7 +34,10 @@ def parse_command_line(argv: List[str]) -> Namespace: "-r", "--revision", default="HEAD", - help="Git revision against which to compare the working tree", + 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: From ad2b403eb775515ca3b63cdcab9fef98165c0a7e Mon Sep 17 00:00:00 2001 From: Antti Kaihola <13725+akaihola@users.noreply.github.com> Date: Sun, 9 Aug 2020 18:51:23 +0300 Subject: [PATCH 8/8] Normalize strings on modified lines of this branch This is actually my first real-life test of the `--revision` feature. What I did was - set `skip-string-normalization = false` for Black in `pyproject.toml` - run `darker src -r master` in the `revision-argument` branch - revert `pyproject.toml` - commit the changes --- src/darker/git.py | 2 +- src/darker/tests/test_command_line.py | 18 +++++++++--------- src/darker/tests/test_git.py | 2 +- src/darker/tests/test_main.py | 4 ++-- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/darker/git.py b/src/darker/git.py index 45db1e7a9..64ffca4b0 100644 --- a/src/darker/git.py +++ b/src/darker/git.py @@ -23,7 +23,7 @@ def git_get_unmodified_content(path: Path, revision: str, cwd: Path) -> List[str cmd = ["git", "show", f"{revision}:./{path}"] logger.debug("[%s]$ %s", cwd, " ".join(cmd)) try: - return check_output(cmd, cwd=str(cwd), encoding='utf-8').splitlines() + 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 diff --git a/src/darker/tests/test_command_line.py b/src/darker/tests/test_command_line.py index 650f1900c..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')}, "HEAD", False, {})), - (['--isort', 'a.py'], ({Path('a.py')}, "HEAD", True, {})), + (["a.py"], ({Path("a.py")}, "HEAD", False, {})), + (["--isort", "a.py"], ({Path("a.py")}, "HEAD", True, {})), ( - ['--config', 'my.cfg', 'a.py'], - ({Path('a.py')}, "HEAD", 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')}, "HEAD", 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')}, "HEAD", 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')}, "HEAD", 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 01ccf4e78..227352298 100644 --- a/src/darker/tests/test_git.py +++ b/src/darker/tests/test_git.py @@ -11,7 +11,7 @@ @pytest.mark.parametrize( - 'revision, expect', + "revision, expect", [("HEAD", ["modified content"]), ("HEAD^", ["original content"]), ("HEAD~2", []),], ) def test_get_unmodified_content(git_repo, revision, expect): diff --git a/src/darker/tests/test_main.py b/src/darker/tests/test_main.py index 95170ea19..c1d3b40c2 100644 --- a/src/darker/tests/test_main.py +++ b/src/darker/tests/test_main.py @@ -119,7 +119,7 @@ def test_format_edited_parts(git_repo, monkeypatch, enable_isort, black_args, ex changes = list( darker.__main__.format_edited_parts( - [Path('a.py')], "HEAD", enable_isort, black_args + [Path("a.py")], "HEAD", enable_isort, black_args ) ) @@ -134,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')], "HEAD", True, {})) + result = list(darker.__main__.format_edited_parts([Path("a.py")], "HEAD", True, {})) assert result == []