Skip to content

Commit 8455858

Browse files
Address code review feedback
- Remove unnecessary goToMarker call in VerifyFormatSelection - Add consistent null result handling to all three formatting methods - Optimize sort function by caching position conversions Co-authored-by: DanielRosenwasser <972891+DanielRosenwasser@users.noreply.github.com>
1 parent a77e23b commit 8455858

File tree

1 file changed

+33
-12
lines changed

1 file changed

+33
-12
lines changed

internal/fourslash/fourslash.go

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2244,7 +2244,13 @@ func (f *FourslashTest) VerifyFormatDocument(t *testing.T, options *lsproto.Form
22442244
t.Fatal("Nil response received for document formatting request")
22452245
}
22462246
if !resultOk {
2247-
t.Fatalf("Unexpected response type for document formatting request: %T", resMsg.AsResponse().Result)
2247+
// Check if result is nil - this is valid, just means no formatting needed
2248+
resp := resMsg.AsResponse()
2249+
if resp.Result == nil {
2250+
f.addFormattingResultToBaseline(t, "formatDocument", nil)
2251+
return
2252+
}
2253+
t.Fatalf("Unexpected response type for document formatting request: %T", resp.Result)
22482254
}
22492255

22502256
f.addFormattingResultToBaseline(t, "formatDocument", result.TextEdits)
@@ -2261,7 +2267,7 @@ func (f *FourslashTest) VerifyFormatSelection(t *testing.T, rangeMarker *RangeMa
22612267
}
22622268
}
22632269

2264-
f.goToMarker(t, rangeMarker)
2270+
f.ensureActiveFile(t, rangeMarker.FileName())
22652271
formatRange := rangeMarker.LSRange
22662272

22672273
params := &lsproto.DocumentRangeFormattingParams{
@@ -2277,7 +2283,13 @@ func (f *FourslashTest) VerifyFormatSelection(t *testing.T, rangeMarker *RangeMa
22772283
t.Fatal("Nil response received for range formatting request")
22782284
}
22792285
if !resultOk {
2280-
t.Fatalf("Unexpected response type for range formatting request: %T", resMsg.AsResponse().Result)
2286+
// Check if result is nil - this is valid, just means no formatting needed
2287+
resp := resMsg.AsResponse()
2288+
if resp.Result == nil {
2289+
f.addFormattingResultToBaseline(t, "formatSelection", nil)
2290+
return
2291+
}
2292+
t.Fatalf("Unexpected response type for range formatting request: %T", resp.Result)
22812293
}
22822294

22832295
f.addFormattingResultToBaseline(t, "formatSelection", result.TextEdits)
@@ -2359,18 +2371,27 @@ func (f *FourslashTest) applyEditsToString(content string, edits []*lsproto.Text
23592371
})
23602372

23612373
// Sort edits in reverse order to avoid affecting positions
2362-
sortedEdits := slices.Clone(edits)
2363-
slices.SortFunc(sortedEdits, func(a, b *lsproto.TextEdit) int {
2364-
aStart := converters.LineAndCharacterToPosition(script, a.Range.Start)
2365-
bStart := converters.LineAndCharacterToPosition(script, b.Range.Start)
2366-
return int(bStart) - int(aStart)
2374+
// Create a slice with cached position conversions for efficient sorting
2375+
type editWithPosition struct {
2376+
edit *lsproto.TextEdit
2377+
pos core.TextPos
2378+
}
2379+
editsWithPositions := make([]editWithPosition, len(edits))
2380+
for i, edit := range edits {
2381+
editsWithPositions[i] = editWithPosition{
2382+
edit: edit,
2383+
pos: converters.LineAndCharacterToPosition(script, edit.Range.Start),
2384+
}
2385+
}
2386+
slices.SortFunc(editsWithPositions, func(a, b editWithPosition) int {
2387+
return int(b.pos) - int(a.pos)
23672388
})
23682389

23692390
result := content
2370-
for _, edit := range sortedEdits {
2371-
start := int(converters.LineAndCharacterToPosition(script, edit.Range.Start))
2372-
end := int(converters.LineAndCharacterToPosition(script, edit.Range.End))
2373-
result = result[:start] + edit.NewText + result[end:]
2391+
for _, editWithPos := range editsWithPositions {
2392+
start := int(editWithPos.pos)
2393+
end := int(converters.LineAndCharacterToPosition(script, editWithPos.edit.Range.End))
2394+
result = result[:start] + editWithPos.edit.NewText + result[end:]
23742395
}
23752396

23762397
return result

0 commit comments

Comments
 (0)