Compare to correct common ancestor with <rev>...

src/darker/__main__.py

@@ -11,7 +11,7 @@
 from darker.command_line import ISORT_INSTRUCTION, parse_command_line
 from darker.config import dump_config
 from darker.diff import diff_and_get_opcodes, opcodes_to_chunks
-from darker.git import EditedLinenumsDiffer, git_get_modified_files
+from darker.git import EditedLinenumsDiffer, RevisionRange, git_get_modified_files
 from darker.import_sorting import apply_isort, isort
 from darker.linting import run_linter
 from darker.utils import get_common_root, joinlines
@@ -22,7 +22,7 @@
 
 def format_edited_parts(
     srcs: Iterable[Path],
-    revision: str,
+    revrange: RevisionRange,
     enable_isort: bool,
     linter_cmdlines: List[str],
     black_args: BlackArgs,
@@ -51,7 +51,7 @@ def format_edited_parts(
     14. print only linter error lines which fall on changed lines
 
     :param srcs: Directories and files to re-format
-    :param revision: The Git revision against which to compare the working tree
+    :param revrange: The Git revision against which to compare the working tree
     :param enable_isort: ``True`` to also run ``isort`` first on each changed file
     :param linter_cmdlines: The command line(s) for running linters on the changed
                             files.
@@ -61,8 +61,8 @@ def format_edited_parts(
 
     """
     git_root = get_common_root(srcs)
-    changed_files = git_get_modified_files(srcs, revision, git_root)
-    edited_linenums_differ = EditedLinenumsDiffer(git_root, revision)
+    changed_files = git_get_modified_files(srcs, revrange, git_root)
+    edited_linenums_differ = EditedLinenumsDiffer(git_root, revrange)
 
     for path_in_repo in sorted(changed_files):
         src = git_root / path_in_repo
@@ -151,7 +151,7 @@ def format_edited_parts(
     # 13. extract line numbers in each file reported by a linter for changed lines
     # 14. print only linter error lines which fall on changed lines
     for linter_cmdline in linter_cmdlines:
-        run_linter(linter_cmdline, git_root, changed_files, revision)
+        run_linter(linter_cmdline, git_root, changed_files, revrange)
 
 
 def modify_file(path: Path, new_content: str) -> None:
@@ -226,8 +226,9 @@ def main(argv: List[str] = None) -> int:
     # `new_content` is just `new_lines` concatenated with newlines.
     # We need both forms when showing diffs or modifying files.
     # Pass them both on to avoid back-and-forth conversion.
+    revrange = RevisionRange.parse(args.revision)
     for path, old_content, new_content, new_lines in format_edited_parts(
-        paths, args.revision, args.isort, args.lint, black_args
+        paths, revrange, args.isort, args.lint, black_args
     ):
         some_files_changed = True
         if args.diff:

src/darker/git.py

@@ -14,7 +14,6 @@
 logger = logging.getLogger(__name__)
 
 
-COMMIT_RANGE_SPLIT_RE = re.compile(r"\.{2,3}")
 # Split a revision range into the "from" and "to" revisions and the dots in between.
 # Handles these cases:
 # <rev>..   <rev>..<rev>   ..<rev>
@@ -27,18 +26,18 @@
 WORKTREE = ":WORKTREE:"
 
 
-def git_get_unmodified_content(path: Path, revision: str, cwd: Path) -> List[str]:
+def git_get_content_at_revision(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. This can also
-                     be a range of commits like ``master..HEAD`` or ``master...HEAD``,
-                     in which case the leftmost end of the range is used.
+    :param revision: The Git revision for which to get the file content, or ``WORKTREE``
+                     to get what's on disk right now.
     :param cwd: The root of the Git repository
 
     """
-    commit = COMMIT_RANGE_SPLIT_RE.split(revision, 1)[0]
-    cmd = ["git", "show", f"{commit}:./{path}"]
+    if revision == WORKTREE:
+        return (cwd / path).read_text("utf-8").splitlines()
+    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()
@@ -120,7 +119,7 @@ def _git_check_output_lines(cmd: List[str], cwd: Path) -> List[str]:
 
 
 def git_get_modified_files(
-    paths: Iterable[Path], revision: str, cwd: Path
+    paths: Iterable[Path], revrange: RevisionRange, cwd: Path
 ) -> Set[Path]:
     """Ask Git for modified and untracked files
 
@@ -129,49 +128,59 @@ def git_get_modified_files(
 
     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
+    :param paths: Paths to the files to diff
+    :param revrange: Git revision range to compare
+    :param cwd: The Git repository root
 
     """
     relative_paths = {p.resolve().relative_to(cwd) for p in paths}
     str_paths = [str(path) for path in relative_paths]
+    if revrange.use_common_ancestor:
+        rev2 = "HEAD" if revrange.rev2 == WORKTREE else revrange.rev2
+        merge_base_cmd = ["git", "merge-base", revrange.rev1, rev2]
+        rev1 = _git_check_output_lines(merge_base_cmd, cwd)[0]
+    else:
+        rev1 = revrange.rev1
     diff_cmd = [
         "git",
         "diff",
         "--name-only",
         "--relative",
-        # `revision` is inserted here if non-empty
+        rev1,
+        # revrange.rev2 is inserted here if not WORKTREE
         "--",
         *str_paths,
     ]
-    if revision:
-        diff_cmd.insert(diff_cmd.index("--"), revision)
+    if revrange.rev2 != WORKTREE:
+        diff_cmd.insert(diff_cmd.index("--"), revrange.rev2)
     lines = _git_check_output_lines(diff_cmd, cwd)
-    ls_files_cmd = [
-        "git",
-        "ls-files",
-        "--others",
-        "--exclude-standard",
-        "--",
-        *str_paths,
-    ]
-    lines.extend(_git_check_output_lines(ls_files_cmd, cwd))
+    if revrange.rev2 == WORKTREE:
+        ls_files_cmd = [
+            "git",
+            "ls-files",
+            "--others",
+            "--exclude-standard",
+            "--",
+            *str_paths,
+        ]
+        lines.extend(_git_check_output_lines(ls_files_cmd, cwd))
     changed_paths = (Path(line) for line in lines)
     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 a given Git revision"""
+    """Find out changed lines for a file between given Git revisions"""
 
     git_root: Path
-    revision: str
+    revrange: RevisionRange
 
     @lru_cache(maxsize=1)
-    def revision_vs_worktree(self, path_in_repo: Path, context_lines: int) -> List[int]:
+    def compare_revisions(self, path_in_repo: Path, context_lines: int) -> List[int]:
         """Return numbers of lines changed between a given revision and the worktree"""
-        lines = (self.git_root / path_in_repo).read_text("utf-8").splitlines()
+        lines = git_get_content_at_revision(
+            path_in_repo, self.revrange.rev2, self.git_root
+        )
         return self.revision_vs_lines(path_in_repo, lines, context_lines)
 
     def revision_vs_lines(
@@ -181,11 +190,12 @@ def revision_vs_lines(
 
         :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
+        :param context_lines: The number of lines to include before and after a change
         :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
+        revision_lines = git_get_content_at_revision(
+            path_in_repo, self.revrange.rev1, self.git_root
         )
         edited_opcodes = diff_and_get_opcodes(revision_lines, lines)
         return list(opcodes_to_edit_linenums(edited_opcodes, context_lines))

src/darker/linting.py

@@ -24,7 +24,7 @@
 from subprocess import PIPE, Popen
 from typing import Set, Tuple, Union
 
-from darker.git import EditedLinenumsDiffer
+from darker.git import WORKTREE, EditedLinenumsDiffer, RevisionRange
 
 logger = logging.getLogger(__name__)
 
@@ -58,30 +58,37 @@ def _parse_linter_line(
     return path_in_repo, linenum
 
 
-def run_linter(cmdline: str, git_root: Path, paths: Set[Path], revision: str) -> None:
+def run_linter(
+    cmdline: str, git_root: Path, paths: Set[Path], revrange: RevisionRange
+) -> None:
     """Run the given linter and print linting errors falling on changed lines
 
     :param cmdline: The command line for running the linter
     :param git_root: The repository root for the changed files
     :param paths: Paths of files to check, relative to ``git_root``
-    :param revision: The Git revision against which to compare the working tree
+    :param revrange: The Git revision rango to compare
 
     """
     if not paths:
         return
+    if revrange.rev2 is not WORKTREE:
+        raise NotImplementedError(
+            "Linting arbitrary commits is not supported. "
+            "Please use -r {<rev>|<rev>..|<rev>...} instead."
+        )
     linter_process = Popen(
         cmdline.split() + [str(git_root / path) for path in sorted(paths)],
         stdout=PIPE,
         encoding="utf-8",
     )
     # assert needed for MyPy (see https://stackoverflow.com/q/57350490/15770)
     assert linter_process.stdout is not None
-    edited_linenums_differ = EditedLinenumsDiffer(git_root, revision)
+    edited_linenums_differ = EditedLinenumsDiffer(git_root, revrange)
     for line in linter_process.stdout:
         path_in_repo, linter_error_linenum = _parse_linter_line(line, git_root)
         if path_in_repo is None:
             continue
-        edited_linenums = edited_linenums_differ.revision_vs_worktree(
+        edited_linenums = edited_linenums_differ.compare_revisions(
             path_in_repo, context_lines=0
         )
         if linter_error_linenum in edited_linenums:

src/darker/tests/test_command_line.py

@@ -9,6 +9,7 @@
 from darker import black_diff
 from darker.__main__ import main
 from darker.command_line import make_argument_parser, parse_command_line
+from darker.git import RevisionRange
 from darker.tests.helpers import filter_dict, raises_if_exception
 from darker.utils import joinlines
 
@@ -339,21 +340,27 @@ 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")}, RevisionRange("HEAD"), False, [], {})),
+        (["--isort", "a.py"], ({Path("a.py")}, RevisionRange("HEAD"), True, [], {})),
         (
             ["--config", "my.cfg", "a.py"],
-            ({Path("a.py")}, "HEAD", False, [], {"config": "my.cfg"}),
+            ({Path("a.py")}, RevisionRange("HEAD"), False, [], {"config": "my.cfg"}),
         ),
         (
             ["--line-length", "90", "a.py"],
-            ({Path("a.py")}, "HEAD", False, [], {"line_length": 90}),
+            ({Path("a.py")}, RevisionRange("HEAD"), False, [], {"line_length": 90}),
         ),
         (
             ["--skip-string-normalization", "a.py"],
-            ({Path("a.py")}, "HEAD", False, [], {"skip_string_normalization": True}),
+            (
+                {Path("a.py")},
+                RevisionRange("HEAD"),
+                False,
+                [],
+                {"skip_string_normalization": True},
+            ),
         ),
-        (["--diff", "a.py"], ({Path("a.py")}, "HEAD", False, [], {})),
+        (["--diff", "a.py"], ({Path("a.py")}, RevisionRange("HEAD"), False, [], {})),
     ],
 )
 def test_options(tmpdir, monkeypatch, options, expect):