From efea86b6a81e63af3cc299288e41866346070fb7 Mon Sep 17 00:00:00 2001
From: Jeffrey Faer <jfaer@google.com>
Date: Mon, 22 Dec 2025 14:05:52 -0700
Subject: [PATCH] refactor: Clarify the logic in line_group's grouping
 function.

I think this is somewhat easier to understand, at least at face value.

It's a little unfortunate that you still need to dive into the
implementation of `shouldAddToGroup` to understand the interplay with
`numUnmatchedStartDirectives`, but I think this is still a net
positive.
---
 keepsorted/line_group.go | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/keepsorted/line_group.go b/keepsorted/line_group.go
index 00c29b4..01c61e5 100644
--- a/keepsorted/line_group.go
+++ b/keepsorted/line_group.go
@@ -91,6 +91,19 @@ func groupLines(lines []string, metadata blockMetadata) []*lineGroup {
 		indents = calculateIndents(lines)
 	}
 
+	// Determines whether the current block is still accepting additional lines.
+	shouldAddToBlock := func() bool {
+		return metadata.opts.Block && !lineRange.empty() && block.expectsContinuation()
+	}
+	// Determines whether the current group should accept the next line.
+	shouldAddToGroup := func(i int, l string) bool {
+		if !metadata.opts.Group {
+			return false
+		}
+
+		increasedIndent := !lineRange.empty() && initialIndent != nil && indents[i] > *initialIndent
+		return increasedIndent || numUnmatchedStartDirectives > 0 || metadata.opts.hasGroupPrefix(l)
+	}
 	countStartDirectives := func(l string) {
 		if strings.Contains(l, metadata.startDirective) {
 			numUnmatchedStartDirectives++
@@ -98,7 +111,6 @@ func groupLines(lines []string, metadata blockMetadata) []*lineGroup {
 			numUnmatchedStartDirectives--
 		}
 	}
-
 	// append a line to both lineRange, and block, if necessary.
 	appendLine := func(i int, l string) {
 		lineRange.append(i)
@@ -126,26 +138,24 @@ func groupLines(lines []string, metadata blockMetadata) []*lineGroup {
 		block = codeBlock{}
 	}
 	for i, l := range lines {
-		if metadata.opts.Block && !lineRange.empty() && block.expectsContinuation() {
-			appendLine(i, l)
-		} else if metadata.opts.Group && (!lineRange.empty() && initialIndent != nil && indents[i] > *initialIndent || numUnmatchedStartDirectives > 0) {
-			appendLine(i, l)
-		} else if metadata.opts.Group && metadata.opts.hasGroupPrefix(l) {
+		if shouldAddToBlock() || shouldAddToGroup(i, l) {
 			appendLine(i, l)
 		} else if metadata.opts.hasStickyPrefix(l) {
+			// Top-level comments break the current block/group.
 			if !lineRange.empty() {
 				finishGroup()
 			}
 
 			commentRange.append(i)
 			if metadata.opts.Group {
-				// Note: This will not count end directives. If this call ever finds a
-				// start directive, it will set numUnmatchedStartDirectives > 0 and then
-				// we will enter the branch above where we'll count end directives via
-				// its appendLine call.
+				// Note: This line will not count end directives. If this call ever
+				// finds a start directive, it will set numUnmatchedStartDirectives > 0
+				// and then we will enter the shouldAddToGroup branch above where we'll
+				// count end directives via its appendLine call.
 				countStartDirectives(l)
 			}
 		} else {
+			// Begin a new block or group.
 			if !lineRange.empty() {
 				finishGroup()
 			}