Fix handling of comments in multiline cmd tasks (#225)

Fixes #203

---------

Co-authored-by: Nat Noordanus <n@noordan.us>

poethepoet/helpers/command/__init__.py

@@ -1,6 +1,17 @@
 import re
 from glob import escape
-from typing import TYPE_CHECKING, Iterator, List, Mapping, Optional, Tuple, cast
+from typing import (
+    TYPE_CHECKING,
+    Iterable,
+    Iterator,
+    List,
+    Mapping,
+    Optional,
+    Tuple,
+    cast,
+)
+
+from .ast import Comment
 
 if TYPE_CHECKING:
     from .ast import Line, ParseConfig
@@ -19,7 +30,7 @@ def parse_poe_cmd(source: str, config: Optional["ParseConfig"] = None):
 
 
 def resolve_command_tokens(
-    line: "Line",
+    lines: Iterable["Line"],
     env: Mapping[str, str],
     config: Optional["ParseConfig"] = None,
 ) -> Iterator[Tuple[str, bool]]:
@@ -53,48 +64,54 @@ def finalize_token(token_parts):
         token_parts.clear()
         return (token, includes_glob)
 
-    for word in line:
-        # For each token part indicate whether it is a glob
-        token_parts: List[Tuple[str, bool]] = []
-        for segment in word:
-            for element in segment:
-                if isinstance(element, ParamExpansion):
-                    param_value = env.get(element.param_name, "")
-                    if not param_value:
-                        continue
-                    if segment.is_quoted:
-                        token_parts.append((env.get(element.param_name, ""), False))
-                    else:
-                        # If the the param expansion it not quoted then:
-                        # - Whitespace inside a substituted param value results in
-                        #  a word break, regardless of quotes or backslashes
-                        # - glob patterns should be evaluated
+    for line in lines:
+        # Ignore line breaks, assuming they're only due to comments
+        for word in line:
+            if isinstance(word, Comment):
+                # strip out comments
+                continue
+
+            # For each token part indicate whether it is a glob
+            token_parts: List[Tuple[str, bool]] = []
+            for segment in word:
+                for element in segment:
+                    if isinstance(element, ParamExpansion):
+                        param_value = env.get(element.param_name, "")
+                        if not param_value:
+                            continue
+                        if segment.is_quoted:
+                            token_parts.append((env.get(element.param_name, ""), False))
+                        else:
+                            # If the the param expansion it not quoted then:
+                            # - Whitespace inside a substituted param value results in
+                            #  a word break, regardless of quotes or backslashes
+                            # - glob patterns should be evaluated
+
+                            if param_value[0].isspace() and token_parts:
+                                # param_value starts with a word break
+                                yield finalize_token(token_parts)
 
-                        if param_value[0].isspace() and token_parts:
-                            # param_value starts with a word break
-                            yield finalize_token(token_parts)
+                            param_words = (
+                                (word, bool(glob_pattern.search(word)))
+                                for word in param_value.split()
+                            )
 
-                        param_words = (
-                            (word, bool(glob_pattern.search(word)))
-                            for word in param_value.split()
-                        )
+                            token_parts.append(next(param_words))
 
-                        token_parts.append(next(param_words))
+                            for param_word in param_words:
+                                if token_parts:
+                                    yield finalize_token(token_parts)
+                                token_parts.append(param_word)
 
-                        for param_word in param_words:
-                            if token_parts:
+                            if param_value[-1].isspace() and token_parts:
+                                # param_value ends with a word break
                                 yield finalize_token(token_parts)
-                            token_parts.append(param_word)
 
-                        if param_value[-1].isspace() and token_parts:
-                            # param_value ends with a word break
-                            yield finalize_token(token_parts)
+                    elif isinstance(element, Glob):
+                        token_parts.append((element.content, True))
 
-                elif isinstance(element, Glob):
-                    token_parts.append((element.content, True))
-
-                else:
-                    token_parts.append((element.content, False))
+                    else:
+                        token_parts.append((element.content, False))
 
-        if token_parts:
-            yield finalize_token(token_parts)
+            if token_parts:
+                yield finalize_token(token_parts)

poethepoet/helpers/command/ast.py

@@ -375,6 +375,8 @@ def _parse(self, chars: ParseCursor):
 
 
 class Line(SyntaxNode[Union[Word, Comment]]):
+    _terminator: str
+
     @property
     def words(self) -> Tuple[Word, ...]:
         if self._children and isinstance(self._children[-1], Comment):
@@ -394,13 +396,15 @@ def _parse(self, chars: ParseCursor):
         self._children = []
         for char in chars:
             if char in self.config.line_separators:
+                self._terminator = char
                 break
 
             elif char.isspace():
                 continue
 
             elif char == "#":
                 self._children.append(CommentCls(chars, self.config))
+                self._terminator = char
                 return
 
             else:

poethepoet/task/cmd.py

@@ -76,15 +76,16 @@ def _resolve_commandline(self, context: "RunContext", env: "EnvVarsManager"):
             raise PoeException(
                 f"Invalid cmd task {self.name!r} does not include any command lines"
             )
-        if len(command_lines) > 1:
+        if any(line._terminator == ";" for line in command_lines[:-1]):
+            # lines terminated by a line break or comment are implicitly joined
             raise PoeException(
                 f"Invalid cmd task {self.name!r} includes multiple command lines"
             )
 
         working_dir = self.get_working_dir(env)
 
         result = []
-        for cmd_token, has_glob in resolve_command_tokens(command_lines[0], env):
+        for cmd_token, has_glob in resolve_command_tokens(command_lines, env):
             if has_glob:
                 # Resolve glob pattern from the working directory
                 result.extend([str(match) for match in working_dir.glob(cmd_token)])

tests/fixtures/cmds_project/pyproject.toml

@@ -2,6 +2,20 @@ tool.poe.tasks.show_env = "poe_test_env"
 
 tool.poe.tasks.ls_color = "poe_test_echo --color='always' \"a\"' b 'c"
 
+tool.poe.tasks.multiline_no_comments = """
+poe_test_echo   first_arg
+                second_arg
+"""
+tool.poe.tasks.multiline_with_single_last_line_comment = """
+poe_test_echo   first_arg
+                second_arg    # second arg
+"""
+
+tool.poe.tasks.multiline_with_many_comments = """
+poe_test_echo   first_arg     # first arg
+                second_arg    # second arg
+"""
+
 [tool.poe.tasks.echo]
 cmd = "poe_test_echo POE_ROOT:$POE_ROOT ${BEST_PASSWORD}, task_args:"
 help = "It says what you say"

tests/helpers/command/test_command_parsing.py

@@ -8,13 +8,13 @@ def test_resolve_command_tokens():
         """
     )[0]
 
-    assert list(resolve_command_tokens(line, {"thing2": ""})) == [
+    assert list(resolve_command_tokens([line], {"thing2": ""})) == [
         ("abcdef", False),
         ("*?", True),
     ]
 
     assert list(
-        resolve_command_tokens(line, {"thing1": " space ", "thing2": "s p a c e"})
+        resolve_command_tokens([line], {"thing1": " space ", "thing2": "s p a c e"})
     ) == [
         ("abc", False),
         ("space", False),
@@ -27,7 +27,7 @@ def test_resolve_command_tokens():
     ]
 
     assert list(
-        resolve_command_tokens(line, {"thing1": " space ", "thing2": "s p a c e"})
+        resolve_command_tokens([line], {"thing1": " space ", "thing2": "s p a c e"})
     ) == [
         ("abc", False),
         ("space", False),
@@ -40,7 +40,7 @@ def test_resolve_command_tokens():
     ]
 
     assert list(
-        resolve_command_tokens(line, {"thing1": "x'[!] ]'y", "thing2": "z [foo ? "})
+        resolve_command_tokens([line], {"thing1": "x'[!] ]'y", "thing2": "z [foo ? "})
     ) == [
         ("abcx'[!]", True),
         ("]'ydef", False),
@@ -56,8 +56,24 @@ def test_resolve_command_tokens():
         """
     )[0]
 
-    assert list(resolve_command_tokens(line, {"thing1": r" *\o/", "thing2": ""})) == [
+    assert list(resolve_command_tokens([line], {"thing1": r" *\o/", "thing2": ""})) == [
         (r"ab *\o/* and ? ' *\o/'", False),
         ("${thing1}", False),
         ("", False),
     ]
+
+    lines = parse_poe_cmd(
+        """
+        # comment
+        one # comment
+        two # comment
+        three # comment
+        # comment
+        """
+    )
+
+    assert list(resolve_command_tokens(lines, {})) == [
+        ("one", False),
+        ("two", False),
+        ("three", False),
+    ]

tests/test_cmd_tasks.py

@@ -1,5 +1,7 @@
 from pathlib import Path
 
+import pytest
+
 
 def test_call_echo_task(run_poe_subproc, projects, esc_prefix, is_windows):
     result = run_poe_subproc("echo", "foo", "!", project="cmds")
@@ -192,3 +194,18 @@ def test_cmd_with_capture_stdout(run_poe_subproc, projects, poe_project_path):
             assert output_file.read() == "I'm Mr. Meeseeks! Look at me!\n"
     finally:
         output_path.unlink()
+
+
+@pytest.mark.parametrize(
+    "testcase",
+    [
+        "multiline_no_comments",
+        "multiline_with_single_last_line_comment",
+        "multiline_with_many_comments",
+    ],
+)
+def test_cmd_multiline(run_poe_subproc, testcase):
+    result = run_poe_subproc(testcase, project="cmds")
+    assert result.capture == "Poe => poe_test_echo first_arg second_arg\n"
+    assert result.stdout == "first_arg second_arg\n"
+    assert result.stderr == ""