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

fix(rome_lsp): fix the parsing of the Workspace root URI on Windows #3185

Merged
merged 1 commit into from
Sep 8, 2022

Conversation

leops
Copy link
Contributor

@leops leops commented Sep 8, 2022

Summary

Fixes #3182

In the Language Server Protocol, the path to the workspace root is sent from the editor to the language server as a URI. In order to load the configuration file from the disk we need to parse this URI as a filesystem path, previously this was done by extracting the path segment of the URI which works on Unix systems (the URI file:///home/user/workspace becomes the path /home/user/workspace) but is not sufficient on Windows where the drive ID needs to be decoded: the URI file:///c%3A/users/user/workspace should be parsed as c:/users/user/workspace but was previously interpreted as /c%3A/users/user/workspace

Test Plan

Unfortunately this relies on OS-specific code (the implementation of the Uri::to_file_path differs depending on the target platform) so it's hard to test automatically as we do not run tests on Windows. Instead I just built and run the language server locally and verified it could parse the workspace path and load the configuration file correctly on Windows

@leops leops added S-Bug: confirmed Status: report has been confirmed as a valid bug A-LSP Area: language server protocol E-VScode Editors: VSCode P-Windows Platform: Windows labels Sep 8, 2022
@netlify
Copy link

netlify bot commented Sep 8, 2022

Deploy Preview for rometools canceled.

Name Link
🔨 Latest commit eaa4e13
🔍 Latest deploy log https://app.netlify.com/sites/rometools/deploys/6319a0cff6840500084c275b

@leops leops temporarily deployed to netlify-playground September 8, 2022 07:59 Inactive
@github-actions
Copy link

github-actions bot commented Sep 8, 2022

@leops leops merged commit a5a0277 into main Sep 8, 2022
@leops leops deleted the fix/lsp-workspace-root-windows branch September 8, 2022 08:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-LSP Area: language server protocol E-VScode Editors: VSCode P-Windows Platform: Windows S-Bug: confirmed Status: report has been confirmed as a valid bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 Rome Extension ignores rome.json configuration on Windows
2 participants