Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

fix(rome_lsp): improve the handling of UTF-8 and overflow errors in LineIndex #3745

Merged
merged 1 commit into from
Nov 16, 2022

Conversation

leops
Copy link
Contributor

@leops leops commented Nov 15, 2022

Summary

Fixes #3733

This PR ensures the LineIndex utility (used to convert between the LineCol and TextSize representations of text positions within the language server) correctly accounts for UTF-8 character boundaries by properly converting between column numbers and byte offsets instead of directly reinterpreting each value kind as the other. I've also introduced additional error checking (especially in the line_col method that's not failible just like offset) to ensure that invalid text positions properly return None rather than an invalid offset (or panic)

Test Plan

I've added new test cases for LineIndex to check the result of both offset and line_col with various kinds of valid or invalid input

@netlify
Copy link

netlify bot commented Nov 15, 2022

Deploy Preview for docs-rometools canceled.

Name Link
🔨 Latest commit ae8c509
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/63739b5dd80bdf00084cfdfb

@ematipico ematipico added the A-LSP Area: language server protocol label Nov 15, 2022
@leops leops merged commit 6193042 into main Nov 16, 2022
@leops leops deleted the fix/lsp-line-index-unicode branch November 16, 2022 08:38
@leops leops mentioned this pull request Nov 17, 2022
1 task
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-LSP Area: language server protocol
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 LSP/parser get in a buggy state when encountering some strings
2 participants