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

add loader for file: URLs #65

Open
wants to merge 25 commits into
base: main
Choose a base branch
from

Conversation

pchampin
Copy link
Contributor

No description provided.

Copy link
Owner

@timothee-haudebourg timothee-haudebourg left a comment

Choose a reason for hiding this comment

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

That sounds great thanks! My only caveat is the use of the url crate where the iref crate already handles URLs through the Uri type. URLs and URIs are equivalents.

Also maybe add a description to the PR next time please 😃

@@ -43,6 +44,7 @@ pretty_dtoa = "0.3"
mime = "0.3"
reqwest = { version = "^0.11", optional = true }
bytes = { version = "^1.3", optional = true }
url = { version = "2.4.1", optional = true }

Choose a reason for hiding this comment

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

The iref crate already handles URLs through the Uri/UriBuf type.

if !url_str.starts_with("file:") {
return Err(Error::NotFileUrl(url_str.into()));
}
let url_parsed = Url::parse(url_str).map_err(Error::InvalidUrl)?;

Choose a reason for hiding this comment

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

You can use Iri::as_uri for that.

{
async move {
let url_str = vocabulary.iri(&url).unwrap().as_str();
if !url_str.starts_with("file:") {

Choose a reason for hiding this comment

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

You can use:

let url = vocabulary.iri(&url).unwrap();
if url.scheme() != "file" {
  // ...
}

@pchampin
Copy link
Contributor Author

pchampin commented Nov 9, 2023

That sounds great thanks! My only caveat is the use of the url crate where the iref crate already handles URLs through the Uri type. URLs and URIs are equivalents.

I agree that this is redundant, but the reason I used the url crate was the to_file_path method, which I don't think iref provides. And this conversion is not trivial (especially with Windows).

Also maybe add a description to the PR next time please 😃

oops, sorry about that! 😨

@timothee-haudebourg
Copy link
Owner

timothee-haudebourg commented Apr 9, 2024

I used the url crate was the to_file_path method, which I don't think iref provides.

You're right, I should add a similar function into iref.

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 this pull request may close these issues.

2 participants