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

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented May 31, 2024

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.

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.
@ahoppen ahoppen requested review from bnbarham and hamishknight May 31, 2024 23:41
@ahoppen ahoppen requested a review from benlangmuir as a code owner May 31, 2024 23:41
@ahoppen
Copy link
Member Author

ahoppen commented May 31, 2024

@swift-ci Please test

Copy link
Contributor

@xedin xedin left a comment

Choose a reason for hiding this comment

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

Works great, thank you!

@ahoppen ahoppen merged commit 045024d into swiftlang:main Jun 1, 2024
3 checks passed
@ahoppen ahoppen deleted the improve-line-folding branch June 1, 2024 04:24
}
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants