Skip to content
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

Fix an off-by-one error in the LSP formatter #1605

Merged
merged 2 commits into from
Sep 14, 2023

Conversation

vkleen
Copy link
Contributor

@vkleen vkleen commented Sep 14, 2023

According to the LSP specification the end of a text range in the protocol is exclusive. When executing a formatting request in the language server we want to replace the entire document with the formatted result, including any trailing newlines. This means we need to specify the beginning of the line after the very last line in the document as the end of the range of text to be replaced.

This also fixes an annoying behaviour when formatting with NLS in Helix. Previously, Helix would insist on selecting (what looked like) the entire document on every formatting request. With this change, Helix refrains from doing this.

According to [the LSP specification](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#range)
the end of a text range in the protocol is exclusive. When executing
a formatting request in the language server we want to replace the
entire document with the formatted result, including any trailing
newlines. This means we need to specify the beginning of the line after
the very last line in the document as the end of the range of text to
be replaced.

This also fixes an annoying behaviour when formatting with NLS in Helix.
Previously, Helix would insist on selecting (what looked like) the
entire document on every formatting request. With this change, Helix
refrains from doing this.
@github-actions github-actions bot temporarily deployed to pull request September 14, 2023 13:26 Inactive
@github-actions github-actions bot temporarily deployed to pull request September 14, 2023 13:58 Inactive
@vkleen vkleen added this pull request to the merge queue Sep 14, 2023
Merged via the queue into master with commit ee6369a Sep 14, 2023
4 checks passed
@vkleen vkleen deleted the fix/lsp-formatting-off-by-one branch September 14, 2023 14:39
suimong pushed a commit to suimong/nickel that referenced this pull request Sep 17, 2023
* Fix an off-by-one error in the LSP formatter

According to [the LSP specification](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#range)
the end of a text range in the protocol is exclusive. When executing
a formatting request in the language server we want to replace the
entire document with the formatted result, including any trailing
newlines. This means we need to specify the beginning of the line after
the very last line in the document as the end of the range of text to
be replaced.

This also fixes an annoying behaviour when formatting with NLS in Helix.
Previously, Helix would insist on selecting (what looked like) the
entire document on every formatting request. With this change, Helix
refrains from doing this.

* Update LSP snapshot test
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.

2 participants