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

helix-lsp-types: Replace url::Url type with String wrapper #11889

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

the-mikedavis
Copy link
Member

This replaces the use of the url::Url type with a newtype wrapper around a String. Changing URL type was the original motivation of vendoring/forking the lsp_types crate in #11355. The url crate is based on the WHATWG spec for URLs which conflicts with the spec that LSP requires - RFC3986. This change uses the RFC3986 reserved characters for encoding paths which should fix some issues like #11888. The parsing and interpretation of the URL as a path is then covered by Url::try_from which we use ubiquitously since #11486.

Fixes #11888

Internally the LSP client should hold workspace folders as paths. Using
URLs for this type is inconvenient (since we compare it to paths) and
might cause mismatches because of URLs not being normalized. The URLs
must be paths anyways so we can convert these types lazily when we need
to send them to a server.
This is a cosmetic change to replace all direct `use`s of the `url::Url`
type in the `helix-lsp-types` crate with `use crate::Url;`.

It's the same type currently but will diverge in the child commits and
this refactor makes that change simpler.
This seems to be a historical artifact in `lsp_types` - we can use a
regular `use` statement to pull in the `bitflags!` macro rather than
an external crate definition. This fixes rust-analyzer's ability to find
the macro at least on rust-analyzer 2024-02-26.
@the-mikedavis the-mikedavis added C-enhancement Category: Improvements A-language-server Area: Language server client S-waiting-on-review Status: Awaiting review from a maintainer. labels Oct 14, 2024
These functions return `Result<Self, ()>` in the `url` crate but the
result is unnecessary since the functions never return the error branch.
We can eliminate the Result to remove some `expect`s in the calling
code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-language-server Area: Language server client C-enhancement Category: Improvements S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
1 participant