Merge pull request #43 from akaihola/revision-argument

WIP: Add -r / --revision argument for specifying which Git revision to compare to

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 <rev>`` / ``--revision <rev>`` 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
 

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

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]

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,17 +39,18 @@ 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
              be reformatted, and skips unchanged files.
 
     """
     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:

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.")

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 -- <path(s)>``
+    - ``git diff --name-only --relative <rev> -- <path(s)>``
     - ``git ls-files --others --exclude-standard -- <path(s)>``
 
     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))

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

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):

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,15 +76,16 @@ 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
         if content is None:
             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

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 == []