From 8e215c504475843d5c6f9dba23cf6e043b20d576 Mon Sep 17 00:00:00 2001 From: Moritz Schlarb Date: Fri, 15 Aug 2025 15:01:54 +0200 Subject: [PATCH 1/2] Add --line-length parameter to format subcommand --- src/cfengine_cli/commands.py | 36 ++++++++++++------------ src/cfengine_cli/format.py | 54 ++++++++++++++++++------------------ src/cfengine_cli/main.py | 5 +++- 3 files changed, 49 insertions(+), 46 deletions(-) diff --git a/src/cfengine_cli/commands.py b/src/cfengine_cli/commands.py index c19dc0e..a308185 100644 --- a/src/cfengine_cli/commands.py +++ b/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 diff --git a/src/cfengine_cli/format.py b/src/cfengine_cli/format.py index 734a9bb..ee9a672 100644 --- a/src/cfengine_cli/format.py +++ b/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,12 +190,12 @@ 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) @@ -203,7 +203,7 @@ def autoformat(node, fmt, macro_indent, indent=0): 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) diff --git a/src/cfengine_cli/main.py b/src/cfengine_cli/main.py index 31b6fd4..e8923bd 100644 --- a/src/cfengine_cli/main.py +++ b/src/cfengine_cli/main.py @@ -43,6 +43,9 @@ 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 +89,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": From e74c980fe570148af38e7b06e1ec6c5e04836802 Mon Sep 17 00:00:00 2001 From: Moritz Schlarb Date: Tue, 26 Aug 2025 14:39:36 +0200 Subject: [PATCH 2/2] Add shell test for format --line-length subcommand --- src/cfengine_cli/main.py | 4 +--- tests/shell/003-format.sh | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) create mode 100755 tests/shell/003-format.sh diff --git a/src/cfengine_cli/main.py b/src/cfengine_cli/main.py index e8923bd..5a22357 100644 --- a/src/cfengine_cli/main.py +++ b/src/cfengine_cli/main.py @@ -43,9 +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" - ) + 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", diff --git a/tests/shell/003-format.sh b/tests/shell/003-format.sh new file mode 100755 index 0000000..90c1e79 --- /dev/null +++ b/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