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

feat(rome_service): allow workspace clients to provide a language hint #3140

Merged
merged 5 commits into from
Sep 1, 2022

Conversation

leops
Copy link
Contributor

@leops leops commented Aug 31, 2022

Summary

Fixes #3006

This PR adds a new language_hint field to the parameters of the open_file Workspace method. It lets clients specify a language ID as a hint for the file type resolver, for instance for files that do not have a file extension yet in the editor. To support this I've merged the EditorLanguage enum in rome_lsp into the Language enum in rome_service, and refactored error handling across the code base to make the lack of a file extension a non-fatal error in file type determination.

Test Plan

I've added an additional test for the LSP to verify we can correctly format files without an extension

@leops leops requested a review from ematipico as a code owner August 31, 2022 10:22
@leops leops requested a review from a team August 31, 2022 10:22
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Aug 31, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 2d8194f
Status: ✅  Deploy successful!
Preview URL: https://bd12ca46.tools-8rn.pages.dev
Branch Preview URL: https://feature-workspace-language-h.tools-8rn.pages.dev

View logs

},
)
.await?
.context("formatting returned None")?;
Copy link
Contributor

Choose a reason for hiding this comment

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

What's this .context call? Is it something like expect?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The context (and with_context) methods come from anyhow and wrap an existing Result (or in this case Option) with a new error object that contains the provided "context" as an the error message.
It's primarily intended to easily let functions propagate inner error results to the outer context while attaching additional information about the error (that's how the error handling in crates/rome_bin/src/service/mod.rs works for instance), and is a concept I'm think we could integrate in the diagnostics refactor if we start using diagnostics as a generic tool for error reporting across the codebase.
But in this case I'm using it as a simple way to turn an Option<T> into a Result<T, Error> with a custom message, so it's very similar to an expect method that returns an error instead of panicking

crates/rome_service/src/workspace/server.rs Outdated Show resolved Hide resolved
Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
Base automatically changed from feature/vscode-status-bar to main August 31, 2022 14:11
@netlify
Copy link

netlify bot commented Aug 31, 2022

Deploy Preview for rometools canceled.

Name Link
🔨 Latest commit 2d8194f
🔍 Latest deploy log https://app.netlify.com/sites/rometools/deploys/6310631ba7055c000939283c

@leops leops temporarily deployed to aws August 31, 2022 14:14 Inactive
@github-actions
Copy link

Parser conformance results on ubuntu-latest

js/262

Test result main count This PR count Difference
Total 45879 45879 0
Passed 44939 44939 0
Failed 940 940 0
Panics 0 0 0
Coverage 97.95% 97.95% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 39 39 0
Passed 36 36 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.31% 92.31% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 5946 5946 0
Passed 1621 1621 0
Failed 4325 4325 0
Panics 0 0 0
Coverage 27.26% 27.26% 0.00%

ts/babel

Test result main count This PR count Difference
Total 588 588 0
Passed 519 519 0
Failed 69 69 0
Panics 0 0 0
Coverage 88.27% 88.27% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 16257 16257 0
Passed 12397 12397 0
Failed 3860 3860 0
Panics 0 0 0
Coverage 76.26% 76.26% 0.00%

@leops leops temporarily deployed to aws September 1, 2022 07:45 Inactive
@github-actions
Copy link

github-actions bot commented Sep 1, 2022

@leops leops merged commit 1bf83c5 into main Sep 1, 2022
@leops leops deleted the feature/workspace-language-hint branch September 1, 2022 08:01
ematipico added a commit that referenced this pull request Sep 5, 2022
#3140)

* feat(vscode): add a status bar item for the language server

* feat(rome_service): allow workspace clients to provide a language ID as a hint for paths without a file extension

* Update crates/rome_service/src/workspace/server.rs

Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>

* fix bindings codegen

Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
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.

🐛 Extension doesn't work for New Files
2 participants