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

Consider not using url crate for directory specifiers? #261

Closed
dsherret opened this issue Mar 23, 2023 · 2 comments
Closed

Consider not using url crate for directory specifiers? #261

dsherret opened this issue Mar 23, 2023 · 2 comments

Comments

@dsherret
Copy link

dsherret commented Mar 23, 2023

Consider a workspace folder sent from a client that sends a folder url like "uri": "file:///dev/my-project" (such as what VSCode does).

pub struct WorkspaceFolder {
/// The associated URI for this workspace folder.
pub uri: Url,
/// The name of the workspace folder. Defaults to the uri's basename.
pub name: String,
}

Given the JSON of that request, this will be deserialize to a url::Url. The problem though is the trailing slash is significant to the Url crate, so the representation in Url is now incorrect because it now considers this a file url rather than a directory. For example, doing .join will not work properly (https://docs.rs/url/2.3.1/url/struct.Url.html#method.join -- "Note: a trailing slash is significant."). A consumer of this crate must then take the url out as a string and normalize it to have a trailing slash (then I believe the consumer must also maintain a mapping back to the original unnormalized value when using this in any responses).

Perhaps it would be better if these values were not deserialized to the url crate's Url representation?

@gibbz00
Copy link
Contributor

gibbz00 commented Oct 6, 2023

uriparse-rs might be a better alternative. rust-url implements https://url.spec.whatwg.org/, not RFC 3986, the latter is the one being used by the LSP Spec:

The URI’s format is defined in https://tools.ietf.org/html/rfc3986

@gibbz00
Copy link
Contributor

gibbz00 commented May 19, 2024

@Marwes

This should be fixed now that e3d0ed2 exists in master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants