Merge pull request #69 from olehermanse/trailing

cfengine format: Removed trailing commas where they're not supported

Author: olehermanse

CLAUDE.md

@@ -0,0 +1,40 @@
+# CFEngine CLI information for LLMs
+
+## Fix the implementation
+
+In general, when the prompter asks you to fix the implementation, this means that they have adjusted the tests already and they want you to fix the implementation.
+Typically you should not touch the tests in this case, unless there is something obviously wrong in them, like a typo.
+The first step to identify what is necessary should be to run the tests and see which ones are failing.
+
+## Running tests
+
+In general, the main command to run for testing is:
+
+```bash
+make check
+```
+
+This will run all the test suites.
+
+## Running python tools
+
+This project uses `uv`.
+That means that you should not run `python`, `python3`, `pip`, `pip3` directly.
+Instead, run the appropriate uv command to ensure we're using the right python and the right dependencies.
+
+## Pointers for the source code
+
+When fixing issues, these are usually the files to look at:
+
+- The implementation of `cfengine format` is in `src/cfengine_cli/format.py`.
+- The implementation of `cfengine lint` is in `src/cfengine_cli/lint.py`.
+
+## Test suites
+
+As mentioned above, the `make check` command runs all the tests.
+We have different suites:
+
+- Unit tests in `tests/unit` test individual python functions.
+- Formatting tests in `tests/format` test the formatter (`cfengine format`).
+- Linting tests in `tests/lint` test the linter.
+- Shell tests in `tests/shell` tests various subcommands and the tool as a whole in an end-to-end fashion.

src/cfengine_cli/format.py

@@ -182,7 +182,9 @@ def split_generic_value(node: Node, indent: int, line_length: int) -> list[str]:
     return [stringify_single_line_node(node)]
 
 
-def split_generic_list(middle: list[Node], indent: int, line_length: int) -> list[str]:
+def split_generic_list(
+    middle: list[Node], indent: int, line_length: int, trailing_comma: bool = True
+) -> list[str]:
     """Split list elements into one-per-line strings, each pre-indented."""
     elements: list[str] = []
     for element in middle:
@@ -196,25 +198,27 @@ def split_generic_list(middle: list[Node], indent: int, line_length: int) -> lis
             lines = split_generic_value(element, indent, line_length)
             elements.append(" " * indent + lines[0])
             elements.extend(lines[1:])
-    # Always add a trailing comma on multi-line lists, on the last
+    # Ensure trailing comma state matches the desired setting, on the last
     # non-comment element (so it doesn't end up after a trailing comment).
     for i in range(len(elements) - 1, -1, -1):
         if elements[i].lstrip().startswith("#"):
             continue
-        if not elements[i].endswith(","):
+        if trailing_comma and not elements[i].endswith(","):
             elements[i] = elements[i] + ","
+        elif not trailing_comma and elements[i].endswith(","):
+            elements[i] = elements[i][:-1]
         break
     return elements
 
 
 def maybe_split_generic_list(
-    nodes: list[Node], indent: int, line_length: int
+    nodes: list[Node], indent: int, line_length: int, trailing_comma: bool = True
 ) -> list[str]:
     """Try a single-line rendering; fall back to split_generic_list if too long."""
     string = " " * indent + stringify_single_line_nodes(nodes)
     if len(string) < line_length:
         return [string]
-    return split_generic_list(nodes, indent, line_length)
+    return split_generic_list(nodes, indent, line_length, trailing_comma)
 
 
 def split_rval_list(node: Node, indent: int, line_length: int) -> list[str]:
@@ -236,7 +240,9 @@ def split_rval_call(node: Node, indent: int, line_length: int) -> list[str]:
     first = text(node.children[0]) + "("
     last = " " * indent + text(node.children[-1])
     middle = node.children[2:-1]
-    elements = maybe_split_generic_list(middle, indent + 2, line_length)
+    elements = maybe_split_generic_list(
+        middle, indent + 2, line_length, trailing_comma=False
+    )
     return [first, *elements, last]
 
 

tests/format/003_wrapping.expected.cf

@@ -70,7 +70,7 @@ bundle agent function_calls
         "seven",
         "eight",
         "nine",
-        "ten",
+        "ten"
       );
 }
 
@@ -90,15 +90,15 @@ bundle agent nested_function_calls
     "variable_name"
       string => concat(
         "one",
-        concat("two", "three", "four", "five", "six", "seven", "eight", ""),
+        concat("two", "three", "four", "five", "six", "seven", "eight", "")
       );
 
     "variable_name"
       string => concat(
         "one",
         concat(
           "two", "three", "four", "five", "six", "seven", "eight", "nine", ""
-        ),
+        )
       );
 
     "variable_name"
@@ -113,24 +113,24 @@ bundle agent nested_function_calls
           "seven",
           "eight",
           "nine",
-          "ten",
-        ),
+          "ten"
+        )
       );
 
     "variable_name"
       string => concat(
         "one",
         "two",
         "three",
-        concat("two", "three", "four", "five", "six", "seven", "eight"),
+        concat("two", "three", "four", "five", "six", "seven", "eight")
       );
 
     "variable_name"
       string => concat(
         "one",
         "two",
         "three",
-        concat("two", "three", "four", "five", "six", "seven", "eight"),
+        concat("two", "three", "four", "five", "six", "seven", "eight")
       );
 
     "variable_name"
@@ -139,7 +139,7 @@ bundle agent nested_function_calls
         "two",
         "three",
         # comment a
-        concat("two", "three", "four", "five", "six", "seven", "eight"),
+        concat("two", "three", "four", "five", "six", "seven", "eight")
         # comment b
       );
 }