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

Handle multibyte characters in RubyDocument #2669

Merged

Conversation

NotFounds
Copy link
Contributor

Motivation

Close #1251

In Shopify/ruby-lsp#2619, we modified the Index creation to handle multibyte characters. However, when processing requests from the LSP to calculate the target node, the character positions are calculated based on the PrismResult rather than the Index, so adjustments are needed in this part of the code as well.

Implementation

This Pull Request updates the locate function in RubyDocument to handle multibyte characters .

Automated Tests

I added simple test cases for it and the definition request.

Manual Tests

I have tested this on multiple projects (including those with Japanese) and confirmed that the definition jump and hover functions work as intended.

@NotFounds NotFounds requested a review from a team as a code owner October 4, 2024 15:36
@NotFounds NotFounds requested review from st0012 and vinistock October 4, 2024 15:36
@vinistock vinistock added bugfix This PR will fix an existing bug server This pull request should be included in the server gem's release notes labels Oct 4, 2024
Copy link
Member

@vinistock vinistock left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution! And thanks for ensuring that multibyte support is working end to end, I appreciate it!

@vinistock vinistock merged commit eca9613 into Shopify:main Oct 4, 2024
20 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This PR will fix an existing bug server This pull request should be included in the server gem's release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better handle multibyte character locations
2 participants