Review comments

Author: ssmifi

server/folding_range_provider.go

@@ -32,6 +32,8 @@ type FoldingRangeFilter interface {
 	IncludeLastFoldingLineForComment() bool
 }
 
+// DefaultFoldingRangeFilter provides the standard folding behavior:
+// Processes all nodes, excludes last line for nodes ending with closing brackets, and includes last line for comments.
 type DefaultFoldingRangeFilter struct{}
 
 func (f *DefaultFoldingRangeFilter) ShouldProcess(node core.AstNode) bool {
@@ -127,20 +129,24 @@ func (p *DefaultFoldingRangeProvider) toFoldingRange(segment *core.TextSegment,
 		return nil
 	}
 
-	foldRange := segment.Range
-	if !includeLastLine {
-		foldRange = core.TextRange{
-			Start: segment.Range.Start,
-			End:   core.TextLocation{Line: segment.Range.End.Line - 1, Column: segment.Range.End.Column},
-		}
-	}
+	lspRange := segment.Range.LspRange()
+	endLine := lspRange.End.Line
+	endChar := lspRange.End.Character
 
-	lspRange := foldRange.LspRange()
-	return &lsp.FoldingRange{
+	folding := &lsp.FoldingRange{
 		StartLine:      &lspRange.Start.Line,
-		EndLine:        &lspRange.End.Line,
 		StartCharacter: &lspRange.Start.Character,
-		EndCharacter:   &lspRange.End.Character,
+		EndLine:        &endLine,
 		Kind:           kind,
 	}
+
+	if !includeLastLine {
+		endLine--
+	} else {
+		// Only set EndCharacter when including the last line.
+		// When not set, LSP defaults to end of line.
+		folding.EndCharacter = &endChar
+	}
+
+	return folding
 }

server/folding_range_provider_test.go

@@ -8,32 +8,13 @@ import (
 	"context"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	core "typefox.dev/fastbelt"
 	"typefox.dev/fastbelt/internal/grammar"
 	"typefox.dev/fastbelt/test"
 	"typefox.dev/fastbelt/util/service"
 	"typefox.dev/lsp"
 )
 
-func assertHasFoldingRange(t *testing.T, result []lsp.FoldingRange, markerRange core.TextRange, label string) {
-	t.Helper()
-	for _, fr := range result {
-		if fr.StartLine == nil || fr.EndLine == nil {
-			continue
-		}
-
-		if *fr.StartLine == uint32(markerRange.Start.Line) && *fr.EndLine == uint32(markerRange.End.Line) {
-			if label == "comment" {
-				assert.Equal(t, "comment", fr.Kind, "Comment folding should have kind='comment'")
-			}
-			return
-		}
-	}
-	t.Errorf("Should have folding range for marker '%s' (lines %d-%d)", label, markerRange.Start.Line, markerRange.End.Line)
-}
-
 func TestFoldingRangeIntegration(t *testing.T) {
 	sc := service.NewContainer()
 	grammar.SetupServices(sc)
@@ -79,13 +60,5 @@ func TestFoldingRangeIntegration(t *testing.T) {
 	require.NoError(t, err)
 	require.NotNil(t, result)
 
-	// We expect folding ranges for the marked regions plus the grammar root
-	expectedLabels := []string{"first", "second", "comment", "third"}
-	assert.Equal(t, len(result), len(expectedLabels)+1, "Should have at least %d folding ranges", len(expectedLabels))
-
-	for _, label := range expectedLabels {
-		markerRange, ok := doc.MarkerRange(label)
-		require.True(t, ok, "Marker '%s' should exist", label)
-		assertHasFoldingRange(t, result, markerRange, label)
-	}
+	doc.AssertFoldingRanges(result, "first", "second", "comment", "third")
 }

test/doc_fixture.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 
 	core "typefox.dev/fastbelt"
+	"typefox.dev/lsp"
 )
 
 // Doc wraps a built [core.Document] with assertion methods and marker positions.
@@ -17,6 +18,7 @@ type Doc struct {
 	Indices  []IndexMarker
 	ctx      context.Context
 	t        testing.TB
+	fixture  *Fixture
 }
 
 // Ctx returns the context for this document. Pass it to [core.Reference.Ref].
@@ -424,3 +426,38 @@ func MustFindReference[T core.AstNode](d *Doc, label string) *core.Reference[T]
 	}
 	return r
 }
+
+// AssertFoldingRanges verifies that the given folding ranges contain entries for all specified marker labels.
+// Returns the Doc for chaining.
+func (d *Doc) AssertFoldingRanges(result []lsp.FoldingRange, labels ...string) *Doc {
+	d.fixture.t.Helper()
+
+	for _, label := range labels {
+		expectedRange, ok := d.MarkerRange(label)
+		if !ok {
+			d.fixture.t.Fatalf("fbtest: no marker with label %q", label)
+		}
+
+		found := false
+		for _, fr := range result {
+			if fr.StartLine != nil && fr.EndLine != nil {
+				if *fr.StartLine == uint32(expectedRange.Start.Line) &&
+					*fr.EndLine == uint32(expectedRange.End.Line) {
+					// For comment ranges, also verify the kind
+					if label == "comment" && fr.Kind != "comment" {
+						continue
+					}
+					found = true
+					break
+				}
+			}
+		}
+
+		if !found {
+			d.fixture.t.Errorf("fbtest: expected folding range at label %q (lines %d-%d) not found",
+				label, expectedRange.Start.Line, expectedRange.End.Line)
+		}
+	}
+
+	return d
+}

test/fixture.go

@@ -158,7 +158,7 @@ func (f *Fixture) ParseAll(uriContentPairs ...string) []*Doc {
 }
 
 func (f *Fixture) newDoc(doc *core.Document, ranges []RangeMarker, indices []IndexMarker) *Doc {
-	return &Doc{Document: doc, Ranges: ranges, Indices: indices, ctx: f.ctx, t: f.t}
+	return &Doc{Document: doc, Ranges: ranges, Indices: indices, ctx: f.ctx, t: f.t, fixture: f}
 }
 
 // extractMarkers scans content for embedded position markers, removes them, and