Merge pull request #455 from akaihola/shlex-split-join-cmdlines

.github/workflows/python-package.yml

@@ -119,9 +119,17 @@ jobs:
         if: matrix.constraints == '--constraint constraints-oldest.txt'
         run: |
           sed -i 's/py311/py39/' pyproject.toml
-      - name: Run Pytest
+      - name: Run Pytest with the Darker plugin on recent Black versions
+        if: matrix.constraints != '--constraint constraints-oldest.txt'
         run: |
           pytest --darker
+      - name: Run Pytest without the Darker plugin on oldest Black version
+        # The reformatting rules used to be a bit different. We don't need to
+        # test reformatting Darker's own code base with old Black versions.
+        # Interoperability is ensured by unit tests.
+        if: matrix.constraints == '--constraint constraints-oldest.txt'
+        run: |
+          pytest
 
   build-sdist-validate-dists:
     runs-on: ubuntu-latest

CHANGES.rst

@@ -21,6 +21,8 @@ Fixed
 - Pass Git errors to stderr correctly both in raw and encoded subprocess output mode.
 - Add a work-around for cleaning up temporary directories. Needed for Python 3.7 on
   Windows.
+- Split and join command lines using ``shlex`` from the Python standard library. This
+  deals with quoting correctly.
 
 
 1.6.1_ - 2022-12-28

README.rst

@@ -762,12 +762,16 @@ Most notably, the following linters/checkers have been verified to work with Dar
 
 *New in version 1.2.0:* Support for test coverage output using `cov_to_lint.py`_.
 
-To run a linter, use the ``--lint`` / ``-L`` command line option:
+To run a linter, use the ``--lint`` / ``-L`` command line option with the linter
+command or a full command line to pass to a linter. Some examples:
 
-  - ``-L mypy``: do static type checking using Mypy_
-  - ``-L pylint``: analyze code using Pylint_
-  - ``-L flake8``: enforce the Python style guide using Flake8_
-  - ``-L cov_to_lint.py``: read ``.coverage`` and list non-covered modified lines
+- ``-L flake8``: enforce the Python style guide using Flake8_
+- ``-L "mypy --strict"``: do static type checking using Mypy_
+- ``--lint="pylint --ignore='setup.py'""``: analyze code using Pylint_
+- ``-L cov_to_lint.py``: read ``.coverage`` and list non-covered modified lines
+
+**Note:** Full command lines aren't fully tested on Windows. See issue `#456`_ for a
+possible bug.
 
 Darker also groups linter output into blocks of consecutive lines
 separated by blank lines.
@@ -795,6 +799,7 @@ Here's an example of `cov_to_lint.py`_ output::
 .. _Pylint: https://pypi.org/project/pylint
 .. _Flake8: https://pypi.org/project/flake8
 .. _cov_to_lint.py: https://gist.github.com/akaihola/2511fe7d2f29f219cb995649afd3d8d2
+.. _#456: https://github.com/akaihola/darker/issues/456
 
 
 Syntax highlighting

src/darker/git.py

@@ -3,6 +3,7 @@
 import logging
 import os
 import re
+import shlex
 import sys
 from dataclasses import dataclass
 from datetime import datetime
@@ -26,6 +27,20 @@
 from darker.multiline_strings import get_multiline_string_ranges
 from darker.utils import GIT_DATEFORMAT, TextDocument
 
+if sys.version_info < (3, 8):
+
+    def shlex_join(split_command: Iterable[str]) -> str:
+        """Backport `shlex.join` for Python 3.7
+
+        :param split_command: The elements on the command line
+        :return: The command line as one string, with appropriate quoting
+
+        """
+        return " ".join(shlex.quote(arg) for arg in split_command)
+
+else:
+    shlex_join = shlex.join
+
 logger = logging.getLogger(__name__)
 
 
@@ -277,7 +292,7 @@ def _git_check_output(
     encoding: Optional[str] = None,
 ) -> Union[str, bytes]:
     """Log command line, run Git, return stdout, exit with 123 on error"""
-    logger.debug("[%s]$ git %s", cwd, " ".join(cmd))
+    logger.debug("[%s]$ git %s", cwd, shlex_join(cmd))
     try:
         return check_output(  # nosec
             ["git"] + cmd,

src/darker/linting.py

@@ -20,13 +20,15 @@
 """
 
 import logging
+import shlex
 from contextlib import contextmanager
 from pathlib import Path
 from subprocess import PIPE, Popen  # nosec
 from typing import IO, Generator, List, Set, Tuple
 
-from darker.git import WORKTREE, EditedLinenumsDiffer, RevisionRange
+from darker.git import WORKTREE, EditedLinenumsDiffer, RevisionRange, shlex_join
 from darker.highlighting import colorize
+from darker.utils import WINDOWS
 
 logger = logging.getLogger(__name__)
 
@@ -143,8 +145,10 @@ def _check_linter_output(
     :return: The standard output stream of the linter subprocess
 
     """
-    cmdline_and_paths = cmdline.split() + [str(root / path) for path in sorted(paths)]
-    logger.debug("[%s]$ %s", Path.cwd(), " ".join(cmdline_and_paths))
+    cmdline_and_paths = shlex.split(cmdline, posix=not WINDOWS) + [
+        str(root / path) for path in sorted(paths)
+    ]
+    logger.debug("[%s]$ %s", Path.cwd(), shlex_join(cmdline_and_paths))
     with Popen(  # nosec
         cmdline_and_paths,
         stdout=PIPE,

src/darker/tests/test_linting.py

@@ -104,15 +104,37 @@ def test_require_rev2_worktree(rev2, expect):
         linting._require_rev2_worktree(rev2)
 
 
-def test_check_linter_output():
+@pytest.mark.kwparametrize(
+    dict(cmdline="echo", expect=["first.py the  2nd.py\n"]),
+    dict(cmdline="echo words before", expect=["words before first.py the  2nd.py\n"]),
+    dict(
+        cmdline='echo "two  spaces"',
+        expect=["two  spaces first.py the  2nd.py\n"],
+        marks=[
+            pytest.mark.xfail(
+                reason=(
+                    "Quotes not removed on Windows."
+                    " See https://github.com/akaihola/darker/issues/456"
+                )
+            )
+        ]
+        if WINDOWS
+        else [],
+    ),
+    dict(cmdline="echo eat  spaces", expect=["eat spaces first.py the  2nd.py\n"]),
+)
+def test_check_linter_output(cmdline, expect):
     """``_check_linter_output()`` runs linter and returns the stdout stream"""
     with linting._check_linter_output(
-        "echo", Path("root/of/repo"), {Path("first.py"), Path("second.py")}
+        cmdline, Path("root/of/repo"), {Path("first.py"), Path("the  2nd.py")}
     ) as stdout:
         lines = list(stdout)
 
     assert lines == [
-        f"{Path('root/of/repo/first.py')} {Path('root/of/repo/second.py')}\n"
+        line.replace("first.py", str(Path("root/of/repo/first.py"))).replace(
+            "the  2nd.py", str(Path("root/of/repo/the  2nd.py"))
+        )
+        for line in expect
     ]