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 Debug supertype to iri parameter in get function of Loader trait #169

Open
GordianDziwis opened this issue Jun 6, 2024 · 3 comments

Comments

@GordianDziwis
Copy link

impl Loader for ResourceLoader {
    fn get<T: Borrow<str>>(
        &self,
        iri: Iri<T>,
    ) -> std::result::Result<(Vec<u8>, String), LoaderError> {
    iri.borrow_term() // This is not possible because T does not implment Debug
    iri.clone() // This is also not possible, because T does not implement Clone
...
}
@pchampin
Copy link
Owner

pchampin commented Jun 6, 2024

Just to clarify: your suggestion is to change the declaration of Loader::get from

fn get<T: Borrow<str>>(
        &self,
        iri: Iri<T>
    ) -> Result<(Vec<u8>, String), LoaderError>;

to

fn get<T: Borrow<str> + Debug>(  // <-- added "+ Debug" here
        &self,
        iri: Iri<T>
    ) -> Result<(Vec<u8>, String), LoaderError>;

right?

I'm guessing that you are encountering issues when generating a log or error message, because something like format!("{:?}", iri) does not work?

Given that Iri<T> can easily be converted to strings, I think you can actually do without the Debug trait: format!("<{}>", iri.as_str() would give an even better message (in terms of user-friendliness, at least).

@GordianDziwis
Copy link
Author

Addung either Debug or Clone is fine.

I needed just two owned values of iri.

@pchampin
Copy link
Owner

pchampin commented Jun 9, 2024

Sorry, I think I initially read the body of your issue too quickly.

The immediate solution to your problem is to use Iri::as_ref, which gives you a "copy" of the IRI, borrowing data from the original.

I understand that it was tempting to use Term::borrow_term instead, and indeed, it is surprising that Iri<T> does not implement Term in this case (even I was surprised 😛 ). I just opened #170 to remove this surprising exception.

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

No branches or pull requests

2 participants