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 end position of diagnostic for LSP #635

Merged
merged 1 commit into from
Jun 23, 2024

Conversation

koic
Copy link
Contributor

@koic koic commented Jun 23, 2024

Follow up rubocop/rubocop#12932.

This is a porting because it can be said that even Standard.rb, which serves as the base for RuboCop's built-in LSP, does the same thing.


This is a quote from rubocop/rubocop#12932.

character of Position is zero-based
https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#position

Character offset on a line in a document (zero-based).

And, Parser::Source::Range is also zero-based
https://github.com/whitequark/parser/blob/3e260d2e37bcb3de8705489d1c2799c26c7a2215/lib/parser/source/range.rb#L104

zero-based column number of the end of this range.

Therefore, -1 is not necessary.

Follow up rubocop/rubocop#12932.

This is a porting because it can be said that even Standard.rb, which serves as the base for RuboCop's built-in LSP,
does the same thing.

---

This is a quote from rubocop/rubocop#12932.

`character` of `Position` is zero-based
https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#position
> Character offset on a line in a document (zero-based).

And, `Parser::Source::Range` is also zero-based
https://github.com/whitequark/parser/blob/3e260d2e37bcb3de8705489d1c2799c26c7a2215/lib/parser/source/range.rb#L104
> zero-based column number of the end of this range.

Therefore, `-1` is not necessary.
@searls
Copy link
Contributor

searls commented Jun 23, 2024

Thanks for back/forward/down porting this for us -- I would've missed it otherwise. Incidentally, I was looking at the exact same line of code in my spike of a Ruby LSP addon today, so I'll fix this there too.

@searls searls merged commit de139d7 into standardrb:main Jun 23, 2024
4 checks passed
@koic koic deleted the lsp_diagnostic_range branch June 23, 2024 13:36
searls added a commit that referenced this pull request Jun 23, 2024
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.

None yet

2 participants