From 2f522ebb37b771f1e345c531bf7326101052b3ec Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Mon, 8 Jun 2026 21:31:47 +0200 Subject: [PATCH 1/5] cfengine format: Consistently separate class guarded attributes in bodies, and fixed comment indentation Co-authored-by: Claude Opus 4.7 (1M context) Signed-off-by: Ole Herman Schumacher Elgesem --- src/cfengine_cli/format.py | 79 +++++++++++++++----- tests/format/002_basics.expected.cf | 2 + tests/format/005_bundle_comments.expected.cf | 21 ++++++ tests/format/005_bundle_comments.input.cf | 21 ++++++ tests/format/011_promises.expected.cf | 14 ++++ 5 files changed, 119 insertions(+), 18 deletions(-) diff --git a/src/cfengine_cli/format.py b/src/cfengine_cli/format.py index 845a09f..94e9fcf 100644 --- a/src/cfengine_cli/format.py +++ b/src/cfengine_cli/format.py @@ -536,6 +536,9 @@ def _can_single_line_promise(node: Node, indent: int, line_length: int) -> bool: return False if _contains_list_with_comment(children): return False + if any(c.type == "comment" for c in children): + # Comments between promiser and attribute force multi-line layout + return False attrs = [c for c in children if c.type == "attribute"] next_sib = node.next_named_sibling while next_sib and next_sib.type == "macro": @@ -647,9 +650,21 @@ def _format_remaining_children( line_length: int, ) -> None: """Format promise children, skipping promiser/arrow/stakeholder parts.""" + # When the promise was multi-lined because of comments between the + # promiser and the attribute, preserve the attribute's original spacing + # rather than running it through the spacing-normalizing stringifier. + # Only applies when the comments and attribute aren't separated by a + # macro — half-promise patterns still use the normal renderer. + verbatim_attr = any(c.type == "comment" for c in children) and not any( + c.type == "macro" for c in children + ) for child in children: if child.type in PROMISER_PARTS: continue + if verbatim_attr and child.type == "attribute": + fmt.print(text(child), indent + 2) + fmt.update_previous(child) + continue _autoformat(child, fmt, line_length, indent) @@ -736,27 +751,39 @@ def _needs_blank_line_before(child: Node, indent: int, line_length: int) -> bool if child.type == "class_guarded_promise_block_attributes": return prev.type in {"attribute", "class_guarded_promise_block_attributes"} + if child.type == "class_guarded_body_attributes": + return prev.type in {"attribute", "class_guarded_body_attributes"} + if child.type in CLASS_GUARD_TYPES: return prev.type in {"promise", "half_promise", "class_guarded_promises"} if child.type == "comment": - if prev.type not in {"promise", "half_promise"} | CLASS_GUARD_TYPES: + if _is_empty_comment(child): return False + # Top-level comment after a complete block — visually separates them + if prev.type in BLOCK_TYPES: + return True parent = child.parent - if parent and parent.type in {"bundle_section", "class_guarded_promises"}: + # Trailing comment at the end of a bundle body lands deeply + # indented (aligned with the deepest attribute); insert a blank + # line so it doesn't run into the preceding section. + if ( + prev.type == "bundle_section" + and parent + and parent.type == "bundle_block_body" + and _skip_comments(child.next_named_sibling, "next") is None + ): return True - # Inside a body/promise block, a comment between two class guards - # only gets a blank-line separator when the preceding class guard - # already has interior comments (i.e. the visual block is rich - # enough that running it together with the next would look dense). + if parent and parent.type in {"bundle_section", "class_guarded_promises"}: + return prev.type in {"promise", "half_promise"} | CLASS_GUARD_TYPES if parent and parent.type in {"body_block_body", "promise_block_body"}: - if _skip_comments(child.next_named_sibling, "next") is None: + next_sib = _skip_comments(child.next_named_sibling, "next") + if next_sib is None: return False - if prev.type in CLASS_GUARD_TYPES and any( - c.type == "comment" for c in prev.children - ): - return True - return False + # Leading comment for a class-guarded section preceded by + # content above it. + if next_sib.type in CLASS_GUARD_TYPES: + return prev.type in CLASS_GUARD_TYPES | {"attribute"} return False return False @@ -790,13 +817,14 @@ def _skip_comments(sibling: Node | None, direction: str = "next") -> Node | None def _comment_indent(node: Node, indent: int) -> int: """Compute indentation for a leaf comment based on its nearest non-comment neighbor.""" nearest = _skip_comments(node.next_named_sibling, "next") - # A trailing comment whose previous sibling is a class-guarded group - # lines up with that group's contents (one extra indent level), as if - # it were the last comment inside the class guard. + # A trailing comment with no next sibling aligns with the deepest + # content at the end of the previous sibling subtree, so that comments + # appended after a class-guarded block visually belong to that block. if nearest is None: - nearest = _skip_comments(node.prev_named_sibling, "prev") - if nearest and nearest.type in CLASS_GUARD_TYPES: - return indent + 4 + prev = _skip_comments(node.prev_named_sibling, "prev") + if prev and prev.type in INDENTED_TYPES: + return _trailing_comment_indent(prev, indent) + nearest = prev if nearest and nearest.type in INDENTED_TYPES: return indent + 2 # No indented sibling found — if we're directly inside a block body, @@ -806,6 +834,21 @@ def _comment_indent(node: Node, indent: int) -> int: return indent +def _trailing_comment_indent(prev: Node, indent: int) -> int: + """Walk down the end of a sibling subtree to compute the deepest indent.""" + while prev.type in INDENTED_TYPES: + indent += 2 + deeper = None + for child in reversed(prev.children): + if child.type in INDENTED_TYPES: + deeper = child + break + if deeper is None: + break + prev = deeper + return indent + + # --------------------------------------------------------------------------- # Main recursive formatter # --------------------------------------------------------------------------- diff --git a/tests/format/002_basics.expected.cf b/tests/format/002_basics.expected.cf index 138e065..04bb88a 100644 --- a/tests/format/002_basics.expected.cf +++ b/tests/format/002_basics.expected.cf @@ -3,8 +3,10 @@ body common control { inputs => { "/var/cfengine/inputs/some_file.cf" }; + linux:: inputs => { "/var/cfengine/inputs/other_file.cf" }; + ubuntu:: inputs => {}; } diff --git a/tests/format/005_bundle_comments.expected.cf b/tests/format/005_bundle_comments.expected.cf index 519d7a9..9a31916 100644 --- a/tests/format/005_bundle_comments.expected.cf +++ b/tests/format/005_bundle_comments.expected.cf @@ -52,3 +52,24 @@ bundle agent g { # comment } + +# For the library +bundle edit_line fixapache +{ + # Comment before promise type + field_edits: + # Comment before promiser + "APACHE_MODULES=.*" + # Comment can be here + # and multiple lines + edit_field => quotedvar("$(add_modules)","append"); + + # Comment before class guard: + linux:: + "APACHE_MODULES=.*" + # Comment can be here + # and multiple lines + edit_field => quotedvar("$(del_modules)","delete"); + + # Trailing comments are fun! +} diff --git a/tests/format/005_bundle_comments.input.cf b/tests/format/005_bundle_comments.input.cf index c08366f..8548dd6 100644 --- a/tests/format/005_bundle_comments.input.cf +++ b/tests/format/005_bundle_comments.input.cf @@ -50,3 +50,24 @@ bundle agent g { # comment } + +# For the library +bundle edit_line fixapache +{ +# Comment before promise type +field_edits: +# Comment before promiser +"APACHE_MODULES=.*" +# Comment can be here +# and multiple lines +edit_field => quotedvar("$(add_modules)","append"); + +# Comment before class guard: +linux:: +"APACHE_MODULES=.*" +# Comment can be here +# and multiple lines +edit_field => quotedvar("$(del_modules)","delete"); + +# Trailing comments are fun! +} diff --git a/tests/format/011_promises.expected.cf b/tests/format/011_promises.expected.cf index e389bc5..2ef6d1b 100644 --- a/tests/format/011_promises.expected.cf +++ b/tests/format/011_promises.expected.cf @@ -67,6 +67,7 @@ body common control "services/main.cf", }; version => "CFEngine Promises.cf 3.27.0"; + # From 3.7 onwards there is a new package promise implementation using package # modules in which you MUST provide package modules used to generate # software inventory reports. You can also provide global default package module @@ -76,6 +77,7 @@ body common control $(package_module_knowledge.platform_default), @(default:package_module_knowledge.additional_inventory), }; + # We only define package_inventory on redhat like systems that have a # python version that works with the package module. (redhat|centos|suse|sles|opensuse|amazon_linux).cfe_python_for_package_modules_supported.!disable_inventory_package_refresh:: @@ -83,43 +85,55 @@ body common control $(package_module_knowledge.platform_default), @(default:package_module_knowledge.additional_inventory), }; + aix.!disable_inventory_package_refresh:: package_inventory => { $(package_module_knowledge.platform_default), @(default:package_module_knowledge.additional_inventory), }; + freebsd.!disable_inventory_package_refresh:: package_inventory => { $(package_module_knowledge.platform_default), @(default:package_module_knowledge.additional_inventory), }; + aix:: package_module => $(package_module_knowledge.platform_default); + (debian|redhat|suse|sles|opensuse|amazon_linux|freebsd):: package_module => $(package_module_knowledge.platform_default); + windows:: package_inventory => { $(package_module_knowledge.platform_default), @(default:package_module_knowledge.additional_inventory), }; package_module => $(package_module_knowledge.platform_default); + termux:: package_module => $(package_module_knowledge.platform_default); + alpinelinux:: package_module => $(package_module_knowledge.platform_default); + any:: ignore_missing_bundles => "$(def.control_common_ignore_missing_bundles)"; ignore_missing_inputs => "$(def.control_common_ignore_missing_inputs)"; # The number of minutes after which last-seen entries are purged from cf_lastseen.lmdb lastseenexpireafter => "$(def.control_common_lastseenexpireafter)"; + control_common_tls_min_version_defined:: tls_min_version => "$(default:def.control_common_tls_min_version)"; + # See also: allowtlsversion in body server control control_common_tls_ciphers_defined:: tls_ciphers => "$(default:def.control_common_tls_ciphers)"; + # See also: allowciphers in body server control control_common_system_log_level_defined:: system_log_level => "$(default:def.control_common_system_log_level)"; + control_common_protocol_version_defined:: protocol_version => "$(default:def.control_common_protocol_version)"; } From 2a98cdc497658280177b67faeffe6b90420eaa84 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Fri, 12 Jun 2026 20:41:43 +0200 Subject: [PATCH 2/5] cfengine format: Tightly surrounded macros Co-authored-by: Claude Fable 5 Signed-off-by: Ole Herman Schumacher Elgesem --- src/cfengine_cli/format.py | 31 ++++++++---------------- tests/format/011_macros.expected.cf | 4 --- tests/lint/015_macro_multi_def_bundle.cf | 1 - 3 files changed, 10 insertions(+), 26 deletions(-) diff --git a/src/cfengine_cli/format.py b/src/cfengine_cli/format.py index 94e9fcf..63f789d 100644 --- a/src/cfengine_cli/format.py +++ b/src/cfengine_cli/format.py @@ -696,12 +696,9 @@ def _format_block_header(node: Node, fmt: Formatter) -> list[Node]: # Skip over preceding empty comments since they will be removed while prev_sib and prev_sib.type == "comment" and _is_empty_comment(prev_sib): prev_sib = prev_sib.prev_named_sibling - is_macro_wrapped = ( - prev_sib - and prev_sib.type == "macro" - and text(prev_sib).startswith(("@if", "@else")) - ) - if not (prev_sib and prev_sib.type == "comment") and not is_macro_wrapped: + # Macros are always tightly surrounded — never a blank line after one + follows_macro = prev_sib is not None and prev_sib.type == "macro" + if not (prev_sib and prev_sib.type == "comment") and not follows_macro: fmt.blank_line() fmt.print(line, 0) for i, comment in enumerate(header_comments): @@ -727,23 +724,19 @@ def _needs_blank_line_before(child: Node, indent: int, line_length: int) -> bool if not prev: return False + # Macros are always tightly surrounded — never a blank line after one + if prev.type == "macro": + return False + if child.type == "bundle_section": return prev.type == "bundle_section" if child.type == "promise": - # An @if macro already provides visual separation - if prev.type == "macro" and text(prev).startswith("@if"): - return False - # Skip past macros to find the content-bearing previous sibling, - # so we detect promise groups separated by macro-split halves. - prev_content = prev - while prev_content and prev_content.type == "macro": - prev_content = prev_content.prev_named_sibling - if prev_content and prev_content.type in {"promise", "half_promise"}: + if prev.type in {"promise", "half_promise"}: promise_indent = indent + 2 both_single = ( - prev_content.type == "promise" - and _can_single_line_promise(prev_content, promise_indent, line_length) + prev.type == "promise" + and _can_single_line_promise(prev, promise_indent, line_length) and _can_single_line_promise(child, promise_indent, line_length) ) return not both_single @@ -869,10 +862,6 @@ def _autoformat( indent = fmt.macro_indent if node.type == "macro": if text(node).startswith("@if"): - # Add blank line before @if at top level if preceded by a block - prev_sib = node.prev_named_sibling - if prev_sib and prev_sib.type in BLOCK_TYPES and not fmt.empty: - fmt.blank_line() fmt.macro_indent = indent fmt.print(node, 0) return diff --git a/tests/format/011_macros.expected.cf b/tests/format/011_macros.expected.cf index 20ac1b1..54c59b3 100644 --- a/tests/format/011_macros.expected.cf +++ b/tests/format/011_macros.expected.cf @@ -22,7 +22,6 @@ bundle agent other "v" string => "old_value"; @endif } - @if minimum_version(3.18) bundle agent new_bundle { @@ -30,7 +29,6 @@ bundle agent new_bundle "Only available in 3.18+"; } @endif - bundle agent half_promises { vars: @@ -40,7 +38,6 @@ bundle agent half_promises @else string => "old_value"; @endif - "another" @if minimum_version(3.18) string => "new"; @@ -190,7 +187,6 @@ bundle agent bundle_e @endif }; } - @if minimum_version(3.24) bundle agent test(a, b) { diff --git a/tests/lint/015_macro_multi_def_bundle.cf b/tests/lint/015_macro_multi_def_bundle.cf index 52f2a21..33d8e68 100644 --- a/tests/lint/015_macro_multi_def_bundle.cf +++ b/tests/lint/015_macro_multi_def_bundle.cf @@ -11,7 +11,6 @@ bundle agent test(a) "$(a)"; } @endif - bundle agent main { methods: From d7e499363b5e6cf3cb97b0f8e06811e59d5807ce Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Fri, 12 Jun 2026 20:53:47 +0200 Subject: [PATCH 3/5] cfengine format: Consistently added space after comma in args Co-authored-by: Claude Fable 5 Signed-off-by: Ole Herman Schumacher Elgesem --- src/cfengine_cli/format.py | 12 ------------ tests/format/005_bundle_comments.expected.cf | 4 ++-- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/src/cfengine_cli/format.py b/src/cfengine_cli/format.py index 63f789d..8709bbe 100644 --- a/src/cfengine_cli/format.py +++ b/src/cfengine_cli/format.py @@ -650,21 +650,9 @@ def _format_remaining_children( line_length: int, ) -> None: """Format promise children, skipping promiser/arrow/stakeholder parts.""" - # When the promise was multi-lined because of comments between the - # promiser and the attribute, preserve the attribute's original spacing - # rather than running it through the spacing-normalizing stringifier. - # Only applies when the comments and attribute aren't separated by a - # macro — half-promise patterns still use the normal renderer. - verbatim_attr = any(c.type == "comment" for c in children) and not any( - c.type == "macro" for c in children - ) for child in children: if child.type in PROMISER_PARTS: continue - if verbatim_attr and child.type == "attribute": - fmt.print(text(child), indent + 2) - fmt.update_previous(child) - continue _autoformat(child, fmt, line_length, indent) diff --git a/tests/format/005_bundle_comments.expected.cf b/tests/format/005_bundle_comments.expected.cf index 9a31916..bed9442 100644 --- a/tests/format/005_bundle_comments.expected.cf +++ b/tests/format/005_bundle_comments.expected.cf @@ -62,14 +62,14 @@ bundle edit_line fixapache "APACHE_MODULES=.*" # Comment can be here # and multiple lines - edit_field => quotedvar("$(add_modules)","append"); + edit_field => quotedvar("$(add_modules)", "append"); # Comment before class guard: linux:: "APACHE_MODULES=.*" # Comment can be here # and multiple lines - edit_field => quotedvar("$(del_modules)","delete"); + edit_field => quotedvar("$(del_modules)", "delete"); # Trailing comments are fun! } From 0e73539f40aadc095f16e68799abfcd3e1969935 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Fri, 12 Jun 2026 22:05:07 +0200 Subject: [PATCH 4/5] cfengine format: Consistently handle line length limit as less than or equal Co-authored-by: Claude Fable 5 Signed-off-by: Ole Herman Schumacher Elgesem --- src/cfengine_cli/format.py | 8 +++++--- tests/format/003_wrapping.expected.cf | 2 +- tests/format/003_wrapping.input.cf | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/cfengine_cli/format.py b/src/cfengine_cli/format.py index 8709bbe..298e1e4 100644 --- a/src/cfengine_cli/format.py +++ b/src/cfengine_cli/format.py @@ -241,6 +241,7 @@ def split_generic_list( elements.append(text(element)) continue line = " " * indent + stringify_single_line_node(element) + # Strict < reserves 1 char for the comma appended after this check if len(line) < line_length: elements.append(line) else: @@ -269,7 +270,7 @@ def maybe_split_generic_list( has_comment = any(n.type == "comment" for n in nodes) if not _contains_macro(nodes) and not has_comment: string = " " * indent + stringify_single_line_nodes(nodes) - if len(string) < line_length: + if len(string) <= line_length: return [string] return split_generic_list(nodes, indent, line_length, trailing_comma) @@ -315,7 +316,7 @@ def maybe_split_rval( if _contains_macro(node) or _contains_list_with_comment(node): return split_rval(node, indent, line_length) line = stringify_single_line_node(node) - if len(line) + offset < line_length: + if len(line) + offset <= line_length: return [line] return split_rval(node, indent, line_length) @@ -393,7 +394,7 @@ def _stringify(node: Node, indent: int, line_length: int) -> list[str]: single_line = " " * indent + stringify_single_line_node(node) # Reserve 1 char for trailing ; or , after attributes effective_length = line_length - 1 if node.type == "attribute" else line_length - if len(single_line) < effective_length: + if len(single_line) <= effective_length: return [single_line] if node.type == "attribute": return _attempt_split_attribute(node, indent, line_length - 1) @@ -498,6 +499,7 @@ def _format_stakeholder_elements( elements.append(" " * indent + text(node)) else: line = " " * indent + stringify_single_line_node(node) + # Strict < reserves 1 char for the comma appended after this check if len(line) < line_length: elements.append(line) else: diff --git a/tests/format/003_wrapping.expected.cf b/tests/format/003_wrapping.expected.cf index 82cc4f3..202c3ee 100644 --- a/tests/format/003_wrapping.expected.cf +++ b/tests/format/003_wrapping.expected.cf @@ -2,7 +2,7 @@ bundle agent strings { vars: # Single-line promises are allowed, as long as there is only 1 attribute, - # and the whole promise fits in less than 80 chars. + # and the whole promise fits in 80 chars or less. "some_variable_name" string => "some_long_variable_value_but_not_past_80"; # Split attribute to separate line if we go over 80: diff --git a/tests/format/003_wrapping.input.cf b/tests/format/003_wrapping.input.cf index 51d89ce..5bc7522 100644 --- a/tests/format/003_wrapping.input.cf +++ b/tests/format/003_wrapping.input.cf @@ -2,7 +2,7 @@ bundle agent strings { vars: # Single-line promises are allowed, as long as there is only 1 attribute, -# and the whole promise fits in less than 80 chars. +# and the whole promise fits in 80 chars or less. "some_variable_name" string => "some_long_variable_value_but_not_past_80"; # Split attribute to separate line if we go over 80: "some_variable_name" string => "some_other_variable_value_which_would go_past_80"; From c16e7892c718d616e925b999da6f2b59f1d71537 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Fri, 12 Jun 2026 22:24:35 +0200 Subject: [PATCH 5/5] Added test cases for the various options for trailing comments in bundles Signed-off-by: Ole Herman Schumacher Elgesem --- tests/format/005_bundle_comments.expected.cf | 31 ++++++++++++++++++++ tests/format/005_bundle_comments.input.cf | 30 +++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/tests/format/005_bundle_comments.expected.cf b/tests/format/005_bundle_comments.expected.cf index bed9442..55e1d88 100644 --- a/tests/format/005_bundle_comments.expected.cf +++ b/tests/format/005_bundle_comments.expected.cf @@ -73,3 +73,34 @@ bundle edit_line fixapache # Trailing comments are fun! } + +bundle agent trailing_example_1 +{ + reports: + + # Trailing +} + +bundle agent trailing_example_2 +{ + reports: + "Hello, world!"; + + # Trailing +} + +bundle agent trailing_example_3 +{ + vars: + foobar:: + "foobar" + if => "something", + string => "value"; + + classes: + "foo" + comment => "blah", + if => "bar"; + + # Trailing +} diff --git a/tests/format/005_bundle_comments.input.cf b/tests/format/005_bundle_comments.input.cf index 8548dd6..08c4e5b 100644 --- a/tests/format/005_bundle_comments.input.cf +++ b/tests/format/005_bundle_comments.input.cf @@ -71,3 +71,33 @@ edit_field => quotedvar("$(del_modules)","delete"); # Trailing comments are fun! } + +bundle agent trailing_example_1 +{ +reports: +# Trailing +} + +bundle agent trailing_example_2 +{ +reports: +"Hello, world!"; + +# Trailing +} + +bundle agent trailing_example_3 +{ +vars: +foobar:: +"foobar" +if => "something", +string => "value"; + +classes: +"foo" +comment => "blah", +if => "bar"; + +# Trailing +}