Merge pull request #238 from akaihola/isort-if-imports-edited

isort only if imports at the top of the module were edited

CHANGES.rst

@@ -8,7 +8,9 @@ Added
 - The ``--workers``/``-W`` option now specifies how many Darker jobs are used to
  process files in parallel to complete reformatting/linting faster.
 - Linters can now be installed and run in the GitHub Action using the ``lint:`` option.
-
+- Sort imports only if the range of modified lines overlaps with changes resulting from
+ sorting the imports.
+
 Fixed
 -----
 - Avoid memory leak from using ``@lru_cache`` on a method.

src/darker/__main__.py

@@ -20,15 +20,15 @@
 from darker.command_line import parse_command_line
 from darker.concurrency import get_executor
 from darker.config import OutputMode, dump_config
-from darker.diff import diff_and_get_opcodes, opcodes_to_chunks
+from darker.diff import diff_chunks
 from darker.exceptions import DependencyError, MissingPackageError
 from darker.git import (
  PRE_COMMIT_FROM_TO_REFS,
  WORKTREE,
  EditedLinenumsDiffer,
  RevisionRange,
  get_missing_at_revision,
- get_rev1_path,
+ get_path_in_repo,
  git_get_content_at_revision,
  git_get_modified_python_files,
  git_is_repository,
@@ -80,63 +80,77 @@ def format_edited_parts(
  with get_executor(max_workers=workers) as executor:
  # pylint: disable=unsubscriptable-object
  futures: List[concurrent.futures.Future[ProcessedDocument]] = []
- for path_in_repo in sorted(changed_files):
+ edited_linenums_differ = EditedLinenumsDiffer(root, revrange)
+ for relative_path_in_rev2 in sorted(changed_files):
  future = executor.submit(
  _isort_and_blacken_single_file,
  root,
- path_in_repo,
+ relative_path_in_rev2,
+ edited_linenums_differ,
+ black_exclude,
  revrange,
- black_config,
  enable_isort,
- enable_black=path_in_repo not in black_exclude,
+ black_config,
  )
  futures.append(future)
 
  for future in concurrent.futures.as_completed(futures):
- src, rev2_content, content_after_reformatting = future.result()
+ (
+ absolute_path_in_rev2,
+ rev2_content,
+ content_after_reformatting,
+ ) = future.result()
  if report_unmodified or content_after_reformatting != rev2_content:
- yield (src, rev2_content, content_after_reformatting)
+ yield (absolute_path_in_rev2, rev2_content, content_after_reformatting)
 
 
 def _isort_and_blacken_single_file( # pylint: disable=too-many-arguments
  root: Path,
- relative_path: Path,
+ relative_path_in_rev2: Path,
+ edited_linenums_differ: EditedLinenumsDiffer,
+ black_exclude: Collection[Path], # pylint: disable=unsubscriptable-object
  revrange: RevisionRange,
- black_config: BlackConfig,
  enable_isort: bool,
- enable_black: bool,
+ black_config: BlackConfig,
 ) -> ProcessedDocument:
  """Black and/or isort formatting for modified chunks in a single file
 
  :param root: Root directory for the relative path
- :param relative_path: Relative path to a Python source code file
+ :param relative_path_in_rev2: Relative path to a Python source code file
+ :param black_exclude: Python files to not reformat using Black, according to Black
+ configuration
  :param revrange: The Git revisions to compare
- :param black_config: Configuration to use for running Black
  :param enable_isort: ``True`` to run ``isort`` first on the file contents
- :param enable_black: ``True`` to also run ``black`` on the file contents
+ :param black_config: Configuration to use for running Black
  :return: Details about changes for the file
 
  """
- src = root / relative_path
- rev2_content = git_get_content_at_revision(relative_path, revrange.rev2, root)
+ # With VSCode, `relative_path_in_rev2` may be a `.py.<HASH>.tmp` file in the
+ # working tree instead of a `.py` file.
+ absolute_path_in_rev2 = root / relative_path_in_rev2
+ rev2_content = git_get_content_at_revision(
+ relative_path_in_rev2, revrange.rev2, root
+ )
  # 1. run isort
  if enable_isort:
  rev2_isorted = apply_isort(
  rev2_content,
- src,
+ relative_path_in_rev2,
+ edited_linenums_differ,
  black_config.get("config"),
  black_config.get("line_length"),
  )
  else:
  rev2_isorted = rev2_content
- if enable_black:
+ if relative_path_in_rev2 not in black_exclude:
  # 9. A re-formatted Python file which produces an identical AST was
  # created successfully - write an updated file or print the diff if
  # there were any changes to the original
  content_after_reformatting = _blacken_single_file(
  root,
- relative_path,
- revrange,
+ relative_path_in_rev2,
+ get_path_in_repo(relative_path_in_rev2),
+ edited_linenums_differ,
  rev2_content,
  rev2_isorted,
  enable_isort,
@@ -145,23 +159,27 @@ def _isort_and_blacken_single_file( # pylint: disable=too-many-arguments
  else:
  # File was excluded by Black configuration, don't reformat
  content_after_reformatting = rev2_isorted
- return src, rev2_content, content_after_reformatting
+ return absolute_path_in_rev2, rev2_content, content_after_reformatting
 
 
 def _blacken_single_file( # pylint: disable=too-many-arguments,too-many-locals
  root: Path,
- relative_path: Path,
- revrange: RevisionRange,
+ relative_path_in_rev2: Path,
+ relative_path_in_repo: Path,
+ edited_linenums_differ: EditedLinenumsDiffer,
  rev2_content: TextDocument,
  rev2_isorted: TextDocument,
  enable_isort: bool,
  black_config: BlackConfig,
 ) -> TextDocument:
  """In a Python file, reformat chunks with edits since the last commit using Black
 
- :param root: Root directory for the relative path
- :param relative_path: Relative path to a Python source code file
- :param revrange: The Git revisions to compare
+ :param root: The common root of all files to reformat
+ :param relative_path_in_rev2: Relative path to a Python source code file. Possibly a
+ VSCode ``.py.<HASH>.tmp`` file in the working tree.
+ :param relative_path_in_repo: Relative path to source in the Git repository. Same as
+ ``relative_path_in_rev2`` save for VSCode temp files.
+ :param edited_linenums_differ: Helper for finding out which lines were edited
  :param rev2_content: Contents of the file at ``revrange.rev2``
  :param rev2_isorted: Contents of the file after optional import sorting
  :param enable_isort: ``True`` if ``isort`` was already run for the file
@@ -170,20 +188,20 @@ def _blacken_single_file( # pylint: disable=too-many-arguments,too-many-locals
  :raise: NotEquivalentError
 
  """
- src = root / relative_path
- rev1_relative_path = get_rev1_path(relative_path)
- edited_linenums_differ = EditedLinenumsDiffer(root, revrange)
+ absolute_path_in_rev2 = root / relative_path_in_rev2
 
  # 4. run black
  formatted = run_black(rev2_isorted, black_config)
- logger.debug("Read %s lines from edited file %s", len(rev2_isorted.lines), src)
+ logger.debug(
+ "Read %s lines from edited file %s",
+ len(rev2_isorted.lines),
+ absolute_path_in_rev2,
+ )
  logger.debug("Black reformat resulted in %s lines", len(formatted.lines))
 
  # 5. get the diff between the edited and reformatted file
- opcodes = diff_and_get_opcodes(rev2_isorted, formatted)
-
  # 6. convert the diff into chunks
- black_chunks = list(opcodes_to_chunks(opcodes, rev2_isorted, formatted))
+ black_chunks = diff_chunks(rev2_isorted, formatted)
 
  # Exit early if nothing to do
  if not black_chunks:
@@ -201,15 +219,15 @@ def _blacken_single_file( # pylint: disable=too-many-arguments,too-many-locals
  logger.debug(
  "Trying with %s lines of context for `git diff -U %s`",
  context_lines,
- src,
+ absolute_path_in_rev2,
  )
  # 2. diff the given revision and worktree for the file
  # 3. extract line numbers in the edited to-file for changed lines
  edited_linenums = edited_linenums_differ.revision_vs_lines(
- rev1_relative_path, rev2_isorted, context_lines
+ relative_path_in_repo, rev2_isorted, context_lines
  )
  if enable_isort and not edited_linenums and rev2_isorted == rev2_content:
- logger.debug("No changes in %s after isort", src)
+ logger.debug("No changes in %s after isort", absolute_path_in_rev2)
  last_successful_reformat = rev2_isorted
  break
 
@@ -232,15 +250,15 @@ def _blacken_single_file( # pylint: disable=too-many-arguments,too-many-locals
  debug_dump(black_chunks, edited_linenums)
  logger.debug(
  "AST verification of %s with %s lines of context failed",
- src,
+ absolute_path_in_rev2,
  context_lines,
  )
  minimum_context_lines.respond(False)
  else:
  minimum_context_lines.respond(True)
  last_successful_reformat = chosen
  if not last_successful_reformat:
- raise NotEquivalentError(relative_path)
+ raise NotEquivalentError(relative_path_in_rev2)
  return last_successful_reformat
 
 

src/darker/diff.py

@@ -177,3 +177,29 @@ def opcodes_to_chunks(
  _validate_opcodes(opcodes)
  for _tag, src_start, src_end, dst_start, dst_end in opcodes:
  yield src_start + 1, src.lines[src_start:src_end], dst.lines[dst_start:dst_end]
+
+
+def diff_chunks(src: TextDocument, dst: TextDocument) -> List[DiffChunk]:
+ """Diff two documents and return the list of chunks in the diff
+
+ Each chunk is a 3-tuple::
+
+ (
+ linenum: int,
+ old_lines: List[str],
+ new_lines: List[str],
+ )
+
+ ``old_lines`` and ``new_lines`` may be
+
+ - identical to indicate a chunk with no changes,
+ - of the same length but different items to indicate some modified lines, or
+ - of different lengths to indicate removed or inserted lines.
+
+ For the return value ``retval``, the following always holds::
+
+ retval[n + 1][0] == retval[n][0] + len(retval[n][old_lines])
+
+ """
+ opcodes = diff_and_get_opcodes(src, dst)
+ return list(opcodes_to_chunks(opcodes, src, dst))

src/darker/git.py

@@ -170,7 +170,7 @@ def _with_common_ancestor(cls, rev1: str, rev2: str, cwd: Path) -> "RevisionRang
  return cls(rev1 if common_ancestor == rev1_hash else common_ancestor, rev2)
 
 
-def get_rev1_path(path: Path) -> Path:
+def get_path_in_repo(path: Path) -> Path:
  """Return the relative path to the file in the old revision
 
  This is usually the same as the relative path on the command line. But in the
@@ -188,7 +188,7 @@ def get_rev1_path(path: Path) -> Path:
 
 
 def should_reformat_file(path: Path) -> bool:
- return path.exists() and get_rev1_path(path).suffix == ".py"
+ return path.exists() and get_path_in_repo(path).suffix == ".py"
 
 
 @lru_cache(maxsize=1)