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

feat(rome_lsp): add a replace_range method to Document #4135

Merged
merged 2 commits into from
Jan 5, 2023

Conversation

leops
Copy link
Contributor

@leops leops commented Jan 3, 2023

Summary

This PR add a new replace_range method to the Document and LineIndex data structures used internally in the Language Server to track the state of an open document. This new method allows the text of an existing document and its associated line index to be updated in place, instead of having to be recreated from scratch on every change. I also took the opportunity to remove the duplicate storage of the text in both Document and LineIndex, the string is now only stored in the innermost structure (the line index).

Test Plan

I've added a few manual test case for the new replace_range method on LineIndex, as well as an automated "property test" that checks the function works correctly over a diverse range of values using the proptest crate.

Documentation

  • The PR requires documentation
  • I will create a new PR to update the documentation

@leops leops requested review from ematipico and a team as code owners January 3, 2023 16:40
@netlify
Copy link

netlify bot commented Jan 3, 2023

Deploy Preview for docs-rometools ready!

Name Link
🔨 Latest commit 50d2422
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/63b5b1eb73a29000084fbbda
😎 Deploy Preview https://deploy-preview-4135--docs-rometools.netlify.app/playground
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@ematipico
Copy link
Contributor

@leops does this PR fix #4094?

crates/rome_lsp/src/line_index.rs Show resolved Hide resolved
crates/rome_lsp/src/line_index.rs Outdated Show resolved Hide resolved
crates/rome_lsp/src/line_index.rs Outdated Show resolved Hide resolved
@ematipico ematipico merged commit 15cee37 into main Jan 5, 2023
@ematipico ematipico deleted the feature/document-replace-range branch January 5, 2023 11:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants