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

Migrate from askama to rinja #155

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Migrate from askama to rinja #155

wants to merge 1 commit into from

Conversation

Johennes
Copy link
Collaborator

@Johennes Johennes commented Nov 6, 2024

Fixes: #150

I started working on this before realizing it'll first require a new release of uniffi-rs. So leaving this here as a draft to be picked up again later.

@@ -237,19 +237,19 @@
{%- endif %}
{%- endmacro %}

{%- macro async(func) %}
{%- macro is_async(func) %}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This renaming and the ones below are required because rinja doesn't allow Rust keywords as macro names. I'm not sure if the names are particularly great but this follows what uniffi-rs itself does.

@jhugman
Copy link
Owner

jhugman commented Nov 6, 2024

I started working on this before realizing it'll first require a new release of uniffi-rs. So leaving this here as a draft to be picked up again later.

I don't think this relies on uniffi-rs, but there maybe some more self:: and borrows to add to the templates. Would @GuillaumeGomez be able to help here?

@Johennes
Copy link
Collaborator Author

Johennes commented Nov 6, 2024

I haven't dug deeply but this error looked as if it was still pulling in askama (even though its not a direct dependency anymore) and the only crate that appears to do it was from uniffi-rs.

115 | #[derive(Template)]
    |                 ^ the trait `From<askama::error::Error>` is not implemented for `bindings::_::rinja::Error`, which is required by `Result<(), bindings::_::rinja::Error>: FromResidual<Result<Infallible, askama::error::Error>>`

Maybe that error is just a surrogate though?

@GuillaumeGomez
Copy link

That's strange. Unless you still have askama in your dependency tree, you should never see askama anywhere as rinja doesn't depend on it (directly or indirectly).

@Johennes
Copy link
Collaborator Author

Johennes commented Nov 7, 2024

Poking at it a little, the build fails on a number of statements in the templates, for instance:

{%- let obj = ci|get_object_definition(name) %}

If I'm reading things correctly, that function bubbles up into lookup_error which results in askama::Error in the versions of the uniffi-rs crates that we're using and was changed to rinja::Error in the merged yet unreleased pull request that migrated uniffi-rs to rinja.

@GuillaumeGomez
Copy link

Ah then until you use the latest version, I suppose you'll have the issue.

@jhugman jhugman added the waiting on uniffi-rs Waiting on a release from uniffi label Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting on uniffi-rs Waiting on a release from uniffi
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Switch from askama to rinja
3 participants