Skip to content

Improve folding ranges if editor only supports line folding #1385

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 26 additions & 2 deletions Sources/SourceKitLSP/Swift/FoldingRange.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import LSPLogging
import LanguageServerProtocol
import SKSupport
import SwiftSyntax

fileprivate final class FoldingRangeFinder: SyntaxAnyVisitor {
Expand Down Expand Up @@ -210,9 +211,21 @@ fileprivate final class FoldingRangeFinder: SyntaxAnyVisitor {
let end = snapshot.positionOf(utf8Offset: end.utf8Offset)
let range: FoldingRange
if lineFoldingOnly {
// If the folding range doesn't end at the end of the last line, exclude that line from the folding range since
// the end line gets folded away. This means if we reported `end.line`, we would eg. fold away the `}` that
// matches a `{`, which looks surprising.
// If the folding range does end at the end of the line we are in cases that don't have a closing indicator (like
// comments), so we can fold the last line as well.
let endLine: Int
if snapshot.lineTable.isAtEndOfLine(end) {
endLine = end.line
} else {
endLine = end.line - 1
}

// Since the client cannot fold less than a single line, if the
// fold would span 1 line there's no point in reporting it.
guard end.line > start.line else {
guard endLine > start.line else {
return .visitChildren
}

Expand All @@ -221,7 +234,7 @@ fileprivate final class FoldingRangeFinder: SyntaxAnyVisitor {
range = FoldingRange(
startLine: start.line,
startUTF16Index: nil,
endLine: end.line,
endLine: endLine,
endUTF16Index: nil,
kind: kind
)
Expand Down Expand Up @@ -264,3 +277,14 @@ extension SwiftLanguageService {
return ranges.sorted()
}
}

fileprivate extension LineTable {
func isAtEndOfLine(_ position: Position) -> Bool {
guard position.line >= 0, position.line < self.count else {
return false
}
let line = self[position.line]
let suffixAfterPositionColumn = line[line.utf16.index(line.startIndex, offsetBy: position.utf16index)...]
return suffixAfterPositionColumn.allSatisfy(\.isNewline)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we care about trailing whitespace too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t think it matters in practice because this is really only for comment folding, which would include the whitespace in the comment’s trivia and thus point to the position right before the newline.

Let’s leave it for now because I don’t have a clean name for this function if we also consider whitespaces.

}
}
58 changes: 52 additions & 6 deletions Tests/SourceKitLSPTests/FoldingRangeTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -96,17 +96,63 @@ final class FoldingRangeTests: XCTestCase {
try await assertFoldingRanges(
markedSource: """
1️⃣func foo() {

2️⃣}
"""
,
2️⃣
}
""",
expectedRanges: [
FoldingRangeSpec(from: "1️⃣", to: "2️⃣")
],
lineFoldingOnly: true
)
}

func testLineFoldingOfFunctionWithMultiLineParameters() async throws {
try await assertFoldingRanges(
markedSource: """
1️⃣func foo(
2️⃣ param: Int
3️⃣) {
print(param)
4️⃣
}
""",
expectedRanges: [
FoldingRangeSpec(from: "1️⃣", to: "2️⃣"),
FoldingRangeSpec(from: "3️⃣", to: "4️⃣"),
],
lineFoldingOnly: true
)
}

func testLineFoldingOfComment() async throws {
try await assertFoldingRanges(
markedSource: """
1️⃣// abc
// def
2️⃣// ghi

""",
expectedRanges: [
FoldingRangeSpec(from: "1️⃣", to: "2️⃣", kind: .comment)
],
lineFoldingOnly: true
)
}

func testLineFoldingOfCommentAtEndOfFile() async throws {
try await assertFoldingRanges(
markedSource: """
1️⃣// abc
// def
2️⃣// ghi
""",
expectedRanges: [
FoldingRangeSpec(from: "1️⃣", to: "2️⃣", kind: .comment)
],
lineFoldingOnly: true
)
}

func testLineFoldingDoesntReportSingleLine() async throws {
try await assertFoldingRanges(
markedSource: """
Expand Down Expand Up @@ -272,8 +318,8 @@ final class FoldingRangeTests: XCTestCase {
try await assertFoldingRanges(
markedSource: """
let x = [1️⃣
1: "one",
2: "two",
1: "one",
2: "two",
3: "three"
2️⃣]
""",
Expand Down