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

Ignore non-file workspace URL #12725

Merged
merged 2 commits into from
Aug 7, 2024
Merged

Conversation

dhruvmanila
Copy link
Member

Summary

This PR updates the server to ignore non-file workspace URL.

This is to avoid crashing the server if the URL scheme is not "file". We'd still raise an error if the URL to file path conversion fails.

Also, as per the docs of to_file_path:

Note: This does not actually check the URL’s scheme, and may give nonsensical results for other schemes. It is the user’s responsibility to check the URL’s scheme before calling this.

resolves: #12660

Test Plan

I'm not sure how to test this locally but the change is small enough to validate on its own.

Copy link
Contributor

github-actions bot commented Aug 7, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@dhruvmanila dhruvmanila force-pushed the dhruv/ignore-non-workspace-url branch from bed704f to 5cbbbf6 Compare August 7, 2024 05:29
crates/ruff_server/src/session/index.rs Outdated Show resolved Hide resolved
@dhruvmanila dhruvmanila enabled auto-merge (squash) August 7, 2024 09:12
@dhruvmanila dhruvmanila merged commit 7fcfedd into main Aug 7, 2024
17 checks passed
@dhruvmanila dhruvmanila deleted the dhruv/ignore-non-workspace-url branch August 7, 2024 09:15
dhruvmanila added a commit that referenced this pull request Aug 7, 2024
## Summary

Follow-up from #12725, this is
just a small refactor to use a wrapper struct instead of type alias for
workspace settings index. This avoids the need to have the
`register_workspace_settings` as a static method on `Index` and instead
is a method on the new struct itself.
dylwil3 pushed a commit to dylwil3/ruff that referenced this pull request Aug 7, 2024
## Summary

This PR updates the server to ignore non-file workspace URL.

This is to avoid crashing the server if the URL scheme is not "file".
We'd still raise an error if the URL to file path conversion fails.

Also, as per the docs of
[`to_file_path`](https://docs.rs/url/2.5.2/url/struct.Url.html#method.to_file_path):

> Note: This does not actually check the URL’s scheme, and may give
nonsensical results for other schemes. It is the user’s responsibility
to check the URL’s scheme before calling this.

resolves: astral-sh#12660

## Test Plan

I'm not sure how to test this locally but the change is small enough to
validate on its own.
dylwil3 pushed a commit to dylwil3/ruff that referenced this pull request Aug 7, 2024
…-sh#12726)

## Summary

Follow-up from astral-sh#12725, this is
just a small refactor to use a wrapper struct instead of type alias for
workspace settings index. This avoids the need to have the
`register_workspace_settings` as a static method on `Index` and instead
is a method on the new struct itself.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
server Related to the LSP server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ruff crashed workspace URL was not a file path!
2 participants