Skip to content

Commit d89e2b8

Browse files
olehermanseclaude
andcommitted
Refactored formatting code
From Claude: 1. Extracted named constants for repeated node type sets: INDENTED_TYPES, CLASS_GUARD_TYPES, BLOCK_TYPES 2. Extracted format_block_header() — the bundle/body header formatting logic that was inline in autoformat 3. Extracted needs_blank_line_before() — replaces the big if/elif chain for blank line insertion between children 4. Extracted get_comment_indent() — the comment indentation logic 5. Extracted is_empty_comment() — the bare # comment detection 6. Simplified the single-line promise path in autoformat by reusing can_single_line_promise() instead of duplicating its logic inline Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Ole Herman Schumacher Elgesem <ole.elgesem@northern.tech>
1 parent 66f300a commit d89e2b8

1 file changed

Lines changed: 128 additions & 141 deletions

File tree

src/cfengine_cli/format.py

Lines changed: 128 additions & 141 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,25 @@ def stringify(node, indent, line_length):
207207
return [single_line]
208208

209209

210+
INDENTED_TYPES = {
211+
"bundle_section",
212+
"class_guarded_promises",
213+
"class_guarded_body_attributes",
214+
"class_guarded_promise_block_attributes",
215+
"promise",
216+
"half_promise",
217+
"attribute",
218+
}
219+
220+
CLASS_GUARD_TYPES = {
221+
"class_guarded_promises",
222+
"class_guarded_body_attributes",
223+
"class_guarded_promise_block_attributes",
224+
}
225+
226+
BLOCK_TYPES = {"bundle_block", "promise_block", "body_block"}
227+
228+
210229
def can_single_line_promise(node, indent, line_length):
211230
"""Check if a promise node can be formatted on a single line."""
212231
if node.type != "promise":
@@ -226,6 +245,100 @@ def can_single_line_promise(node, indent, line_length):
226245
return indent + len(line) <= line_length
227246

228247

248+
def format_block_header(node, fmt):
249+
"""Format the header of a bundle/body block and return its body children."""
250+
header_parts = []
251+
header_comments = []
252+
for x in node.children[0:-1]:
253+
if x.type == "comment":
254+
header_comments.append(text(x))
255+
elif x.type == "parameter_list":
256+
parts = []
257+
for p in x.children:
258+
if p.type == "comment":
259+
header_comments.append(text(p))
260+
else:
261+
parts.append(text(p))
262+
header_parts[-1] = header_parts[-1] + stringify_parameter_list(parts)
263+
else:
264+
header_parts.append(text(x))
265+
line = " ".join(header_parts)
266+
if not fmt.empty:
267+
prev_sib = node.prev_named_sibling
268+
if not (prev_sib and prev_sib.type == "comment"):
269+
fmt.print("", 0)
270+
fmt.print(line, 0)
271+
for i, comment in enumerate(header_comments):
272+
if comment.strip() == "#":
273+
prev_is_comment = i > 0 and header_comments[i - 1].strip() != "#"
274+
next_is_comment = (
275+
i + 1 < len(header_comments) and header_comments[i + 1].strip() != "#"
276+
)
277+
if not (prev_is_comment and next_is_comment):
278+
continue
279+
fmt.print(comment, 0)
280+
return node.children[-1].children
281+
282+
283+
def needs_blank_line_before(child, indent, line_length):
284+
"""Check if a blank line should be inserted before this child node."""
285+
prev = child.prev_named_sibling
286+
if not prev:
287+
return False
288+
289+
if child.type == "bundle_section":
290+
return prev.type == "bundle_section"
291+
292+
if child.type == "promise" and prev.type in {"promise", "half_promise"}:
293+
promise_indent = indent + 2
294+
both_single = (
295+
prev.type == "promise"
296+
and can_single_line_promise(prev, promise_indent, line_length)
297+
and can_single_line_promise(child, promise_indent, line_length)
298+
)
299+
return not both_single
300+
301+
if child.type in CLASS_GUARD_TYPES:
302+
return prev.type in {"promise", "half_promise", "class_guarded_promises"}
303+
304+
if child.type == "comment":
305+
if prev.type in {"promise", "half_promise"} | CLASS_GUARD_TYPES:
306+
parent = child.parent
307+
return parent and parent.type in {"bundle_section"} | {
308+
"class_guarded_promises"
309+
}
310+
return False
311+
312+
return False
313+
314+
315+
def get_comment_indent(node, indent):
316+
"""Determine the indentation level for a comment node."""
317+
next_sib = node.next_named_sibling
318+
while next_sib and next_sib.type == "comment":
319+
next_sib = next_sib.next_named_sibling
320+
321+
if next_sib is None:
322+
prev_sib = node.prev_named_sibling
323+
while prev_sib and prev_sib.type == "comment":
324+
prev_sib = prev_sib.prev_named_sibling
325+
if prev_sib and prev_sib.type in INDENTED_TYPES:
326+
return indent + 2
327+
elif next_sib.type in INDENTED_TYPES:
328+
return indent + 2
329+
330+
return indent
331+
332+
333+
def is_empty_comment(node):
334+
"""Check if a comment is just '#' with no content."""
335+
if text(node).strip() != "#":
336+
return False
337+
prev = node.prev_named_sibling
338+
nxt = node.next_named_sibling
339+
return not (prev and prev.type == "comment" and nxt and nxt.type == "comment")
340+
341+
229342
def autoformat(node, fmt, line_length, macro_indent, indent=0):
230343
previous = fmt.update_previous(node)
231344
if previous and previous.type == "macro" and text(previous).startswith("@else"):
@@ -238,159 +351,33 @@ def autoformat(node, fmt, line_length, macro_indent, indent=0):
238351
indent = macro_indent
239352
return
240353
children = node.children
241-
if node.type in ["bundle_block", "promise_block", "body_block"]:
242-
header_parts = []
243-
header_comments = []
244-
for x in node.children[0:-1]:
245-
if x.type == "comment":
246-
header_comments.append(text(x))
247-
elif x.type == "parameter_list":
248-
parts = []
249-
for p in x.children:
250-
if p.type == "comment":
251-
header_comments.append(text(p))
252-
else:
253-
parts.append(text(p))
254-
# Append directly to previous part (no space before parens)
255-
header_parts[-1] = header_parts[-1] + stringify_parameter_list(parts)
256-
else:
257-
header_parts.append(text(x))
258-
line = " ".join(header_parts)
259-
if not fmt.empty:
260-
prev_sib = node.prev_named_sibling
261-
if not (prev_sib and prev_sib.type == "comment"):
262-
fmt.print("", 0)
263-
fmt.print(line, 0)
264-
for i, comment in enumerate(header_comments):
265-
if comment.strip() == "#":
266-
prev_is_comment = i > 0 and header_comments[i - 1].strip() != "#"
267-
next_is_comment = (
268-
i + 1 < len(header_comments)
269-
and header_comments[i + 1].strip() != "#"
270-
)
271-
if not (prev_is_comment and next_is_comment):
272-
continue
273-
fmt.print(comment, 0)
274-
children = node.children[-1].children
275-
if node.type in [
276-
"bundle_section",
277-
"class_guarded_promises",
278-
"class_guarded_body_attributes",
279-
"class_guarded_promise_block_attributes",
280-
"promise",
281-
"half_promise",
282-
"attribute",
283-
]:
354+
if node.type in BLOCK_TYPES:
355+
children = format_block_header(node, fmt)
356+
if node.type in INDENTED_TYPES:
284357
indent += 2
285358
if node.type == "attribute":
286359
lines = stringify(node, indent, line_length)
287360
fmt.print_lines(lines, indent=0)
288361
return
289-
if node.type == "promise":
290-
# Single-line promise: if exactly 1 attribute, no half_promise continuation,
291-
# not inside a class guard, and the whole line fits in line_length
292-
attr_children = [c for c in children if c.type == "attribute"]
293-
next_sib = node.next_named_sibling
294-
has_continuation = next_sib and next_sib.type == "half_promise"
295-
if len(attr_children) == 1 and not has_continuation:
296-
promiser_node = next((c for c in children if c.type == "promiser"), None)
297-
if promiser_node:
298-
line = (
299-
text(promiser_node)
300-
+ " "
301-
+ stringify_single_line_node(attr_children[0])
302-
+ ";"
303-
)
304-
if indent + len(line) <= line_length:
305-
fmt.print(line, indent)
306-
return
362+
if node.type == "promise" and can_single_line_promise(node, indent, line_length):
363+
promiser_node = next(c for c in children if c.type == "promiser")
364+
attr_node = next(c for c in children if c.type == "attribute")
365+
line = text(promiser_node) + " " + stringify_single_line_node(attr_node) + ";"
366+
fmt.print(line, indent)
367+
return
307368
if children:
308369
for child in children:
309-
# Blank line between bundle sections
310-
if child.type == "bundle_section":
311-
prev = child.prev_named_sibling
312-
if prev and prev.type == "bundle_section":
313-
fmt.print("", 0)
314-
# Blank line between promises in a section
315-
elif child.type == "promise":
316-
prev = child.prev_named_sibling
317-
if prev and prev.type in ["promise", "half_promise"]:
318-
# Skip blank line between consecutive single-line promises
319-
promise_indent = indent + 2
320-
both_single = (
321-
prev.type == "promise"
322-
and can_single_line_promise(prev, promise_indent, line_length)
323-
and can_single_line_promise(child, promise_indent, line_length)
324-
)
325-
if not both_single:
326-
fmt.print("", 0)
327-
elif child.type in [
328-
"class_guarded_promises",
329-
"class_guarded_body_attributes",
330-
"class_guarded_promise_block_attributes",
331-
]:
332-
prev = child.prev_named_sibling
333-
if prev and prev.type in [
334-
"promise",
335-
"half_promise",
336-
"class_guarded_promises",
337-
]:
338-
fmt.print("", 0)
339-
elif child.type == "comment":
340-
prev = child.prev_named_sibling
341-
if prev and prev.type in [
342-
"promise",
343-
"half_promise",
344-
"class_guarded_promises",
345-
"class_guarded_body_attributes",
346-
"class_guarded_promise_block_attributes",
347-
]:
348-
parent = child.parent
349-
if parent and parent.type in [
350-
"bundle_section",
351-
"class_guarded_promises",
352-
]:
353-
fmt.print("", 0)
370+
if needs_blank_line_before(child, indent, line_length):
371+
fmt.print("", 0)
354372
autoformat(child, fmt, line_length, macro_indent, indent)
355373
return
356-
if node.type in [",", ";"]:
374+
if node.type in {",", ";"}:
357375
fmt.print_same_line(node)
358376
return
359377
if node.type == "comment":
360-
if text(node).strip() == "#":
361-
prev = node.prev_named_sibling
362-
nxt = node.next_named_sibling
363-
if not (prev and prev.type == "comment" and nxt and nxt.type == "comment"):
364-
return
365-
comment_indent = indent
366-
next_sib = node.next_named_sibling
367-
while next_sib and next_sib.type == "comment":
368-
next_sib = next_sib.next_named_sibling
369-
if next_sib is None:
370-
prev_sib = node.prev_named_sibling
371-
while prev_sib and prev_sib.type == "comment":
372-
prev_sib = prev_sib.prev_named_sibling
373-
if prev_sib and prev_sib.type in [
374-
"bundle_section",
375-
"class_guarded_promises",
376-
"class_guarded_body_attributes",
377-
"class_guarded_promise_block_attributes",
378-
"promise",
379-
"half_promise",
380-
"attribute",
381-
]:
382-
comment_indent = indent + 2
383-
elif next_sib.type in [
384-
"bundle_section",
385-
"class_guarded_promises",
386-
"class_guarded_body_attributes",
387-
"class_guarded_promise_block_attributes",
388-
"promise",
389-
"half_promise",
390-
"attribute",
391-
]:
392-
comment_indent = indent + 2
393-
fmt.print(node, comment_indent)
378+
if is_empty_comment(node):
379+
return
380+
fmt.print(node, get_comment_indent(node, indent))
394381
return
395382
fmt.print(node, indent)
396383

0 commit comments

Comments
 (0)