Add --line-length parameter to format subcommand for cf policy files

src/cfengine_cli/commands.py

@@ -47,46 +47,46 @@ def deploy() -> int:
     return r
 
 
-def _format_filename(filename):
+def _format_filename(filename, line_length):
     if filename.startswith("./."):
         return
     if filename.endswith(".json"):
         format_json_file(filename)
         return
     if filename.endswith(".cf"):
-        format_policy_file(filename)
+        format_policy_file(filename, line_length)
         return
     raise UserError(f"Unrecognized file format: {filename}")
 
 
-def _format_dirname(directory):
+def _format_dirname(directory, line_length):
     for filename in find(directory, extension=".json"):
-        _format_filename(filename)
+        _format_filename(filename, line_length)
     for filename in find(directory, extension=".cf"):
-        _format_filename(filename)
+        _format_filename(filename, line_length)
 
 
-def format(args) -> int:
-    if not args:
-        _format_dirname(".")
+def format(names, line_length) -> int:
+    if not names:
+        _format_dirname(".", line_length)
         return 0
-    if len(args) == 1 and args[0] == "-":
+    if len(names) == 1 and names[0] == "-":
         # Special case, format policy file from stdin to stdout
-        format_policy_fin_fout(sys.stdin, sys.stdout)
+        format_policy_fin_fout(sys.stdin, sys.stdout, line_length)
         return 0
 
-    for arg in args:
-        if arg == "-":
+    for name in names:
+        if name == "-":
             raise UserError(
                 "The - argument has a special meaning and cannot be combined with other paths"
             )
-        if not os.path.exists(arg):
-            raise UserError(f"{arg} does not exist")
-        if os.path.isfile(arg):
-            _format_filename(arg)
+        if not os.path.exists(name):
+            raise UserError(f"{name} does not exist")
+        if os.path.isfile(name):
+            _format_filename(name, line_length)
             continue
-        if os.path.isdir(arg):
-            _format_dirname(arg)
+        if os.path.isdir(name):
+            _format_dirname(name, line_length)
             continue
     return 0
 

src/cfengine_cli/format.py

@@ -80,14 +80,14 @@ def split_generic_value(node, indent):
     return [stringify_single_line(node)]
 
 
-def split_generic_list(middle, indent):
+def split_generic_list(middle, indent, line_length):
     elements = []
     for element in middle:
         if elements and element.type == ",":
             elements[-1] = elements[-1] + ","
             continue
         line = " " * indent + stringify_single_line(element)
-        if len(line) < 80:
+        if len(line) < line_length:
             elements.append(line)
         else:
             lines = split_generic_value(element, indent)
@@ -96,50 +96,50 @@ def split_generic_list(middle, indent):
     return elements
 
 
-def maybe_split_generic_list(nodes, indent):
+def maybe_split_generic_list(nodes, indent, line_length):
     string = " " * indent + stringify_children(nodes)
-    if len(string) < 80:
+    if len(string) < line_length:
         return [string]
-    return split_generic_list(nodes, indent)
+    return split_generic_list(nodes, indent, line_length)
 
 
-def split_rval_list(node, indent):
+def split_rval_list(node, indent, line_length):
     assert node.type == "list"
     assert node.children[0].type == "{"
     first = text(node.children[0])
     last = " " * indent + text(node.children[-1])
     middle = node.children[1:-1]
-    elements = maybe_split_generic_list(middle, indent + 2)
+    elements = maybe_split_generic_list(middle, indent + 2, line_length)
     return [first, *elements, last]
 
 
-def split_rval_call(node, indent):
+def split_rval_call(node, indent, line_length):
     assert node.type == "call"
     assert node.children[0].type == "calling_identifier"
     assert node.children[1].type == "("
     first = text(node.children[0]) + "("
     last = " " * indent + text(node.children[-1])
     middle = node.children[2:-1]
-    elements = maybe_split_generic_list(middle, indent + 2)
+    elements = maybe_split_generic_list(middle, indent + 2, line_length)
     return [first, *elements, last]
 
 
-def split_rval(node, indent):
+def split_rval(node, indent, line_length):
     if node.type == "list":
-        return split_rval_list(node, indent)
+        return split_rval_list(node, indent, line_length)
     if node.type == "call":
-        return split_rval_call(node, indent)
+        return split_rval_call(node, indent, line_length)
     return [stringify_single_line(node)]
 
 
-def maybe_split_rval(node, indent, offset):
+def maybe_split_rval(node, indent, offset, line_length):
     line = stringify_single_line(node)
-    if len(line) + offset < 80:
+    if len(line) + offset < line_length:
         return [line]
-    return split_rval(node, indent)
+    return split_rval(node, indent, line_length)
 
 
-def attempt_split_attribute(node, indent):
+def attempt_split_attribute(node, indent, line_length):
     assert len(node.children) == 3
     lval = node.children[0]
     arrow = node.children[1]
@@ -148,22 +148,22 @@ def attempt_split_attribute(node, indent):
     if rval.type == "list" or rval.type == "call":
         prefix = " " * indent + text(lval) + " " + text(arrow) + " "
         offset = len(prefix)
-        lines = maybe_split_rval(rval, indent, offset)
+        lines = maybe_split_rval(rval, indent, offset, line_length)
         lines[0] = prefix + lines[0]
         return lines
     return [stringify_single_line(node)]
 
 
-def stringify(node, indent):
+def stringify(node, indent, line_length):
     single_line = " " * indent + stringify_single_line(node)
-    if len(single_line) < 80:
+    if len(single_line) < line_length:
         return [single_line]
     if node.type == "attribute":
-        return attempt_split_attribute(node, indent)
+        return attempt_split_attribute(node, indent, line_length)
     return [single_line]
 
 
-def autoformat(node, fmt, macro_indent, indent=0):
+def autoformat(node, fmt, line_length, macro_indent, indent=0):
     previous = fmt.update_previous(node)
     if previous and previous.type == "macro" and text(previous).startswith("@else"):
         indent = macro_indent
@@ -190,20 +190,20 @@ def autoformat(node, fmt, macro_indent, indent=0):
     ]:
         indent += 2
     if node.type == "attribute":
-        lines = stringify(node, indent)
+        lines = stringify(node, indent, line_length)
         fmt.print_lines(lines, indent=0)
         return
     if children:
         for child in children:
-            autoformat(child, fmt, macro_indent, indent)
+            autoformat(child, fmt, line_length, macro_indent, indent)
         return
     if node.type in [",", ";"]:
         fmt.print_same_line(node)
         return
     fmt.print(node, indent)
 
 
-def format_policy_file(filename):
+def format_policy_file(filename, line_length):
     assert filename.endswith(".cf")
     PY_LANGUAGE = Language(tscfengine.language())
     parser = Parser(PY_LANGUAGE)
@@ -216,7 +216,7 @@ def format_policy_file(filename):
 
     root_node = tree.root_node
     assert root_node.type == "source_file"
-    autoformat(root_node, fmt, macro_indent)
+    autoformat(root_node, fmt, line_length, macro_indent)
 
     new_data = fmt.buffer + "\n"
     if new_data != original_data.decode("utf-8"):
@@ -225,7 +225,7 @@ def format_policy_file(filename):
         print(f"Policy file '{filename}' was reformatted")
 
 
-def format_policy_fin_fout(fin, fout):
+def format_policy_fin_fout(fin, fout, line_length):
     PY_LANGUAGE = Language(tscfengine.language())
     parser = Parser(PY_LANGUAGE)
 
@@ -236,7 +236,7 @@ def format_policy_fin_fout(fin, fout):
 
     root_node = tree.root_node
     assert root_node.type == "source_file"
-    autoformat(root_node, fmt, macro_indent)
+    autoformat(root_node, fmt, line_length, macro_indent)
 
     new_data = fmt.buffer + "\n"
     fout.write(new_data)

src/cfengine_cli/main.py

@@ -43,6 +43,7 @@ def _get_arg_parser():
     subp.add_parser("deploy", help="Deploy a built policy set")
     fmt = subp.add_parser("format", help="Autoformat .json and .cf files")
     fmt.add_argument("files", nargs="*", help="Files to format")
+    fmt.add_argument("--line-length", default=80, type=int, help="Maximum line length")
     subp.add_parser(
         "lint",
         help="Look for syntax errors and other simple mistakes",
@@ -86,7 +87,7 @@ def run_command_with_args(args) -> int:
     if args.command == "deploy":
         return commands.deploy()
     if args.command == "format":
-        return commands.format(args.files)
+        return commands.format(args.files, args.line_length)
     if args.command == "lint":
         return commands.lint()
     if args.command == "report":

tests/shell/003-format.sh

@@ -0,0 +1,37 @@
+#!/bin/bash
+
+set -e
+set -x
+
+# Verify that the line length parameter shows up in help
+cfengine format -h | grep -- "--line-length"
+
+# Test three common line length
+for ll in 80 100 120; do
+	# Format the following HEREDOC and let wc count the longest line
+	l=$(cfengine format --line-length $ll - <<-EOF |
+	bundle agent slists
+	{
+	vars:
+	"variable_name"
+	slist => { "one", "two", "three", "four", "five", "six" };
+	"variable_name"
+	slist => { "one", "two", "three", "four", "five", "six", "seven" };
+	"variable_name"
+	slist => {
+	"one", "two", "three", "four", "five", "six", "seven", "eight"
+	};
+	"variable_name"
+	slist => {
+	"one","two","three","four","five","six","seven","eight","nine","ten","eleven","twelve","thirteen",
+	};
+	"variable_name"
+	slist => {
+	"one","two","three","four","five","six","seven","eight","nine","ten","eleven","twelve","thirteen","fourteen","fifteen","sixteen","seventeen"
+	};
+	}
+	EOF
+	wc -L)
+	# Verify that the actual longest line has less or equal characters
+	[[ $l -le $ll ]]
+done