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

Allow variables inheritance in block #264

Merged

Conversation

GuillaumeGomez
Copy link
Contributor

Fixes #246.

@GuillaumeGomez GuillaumeGomez force-pushed the allow-variables-inheritance-in-block branch 2 times, most recently from a873960 to b3f8110 Compare November 22, 2024 17:42
@GuillaumeGomez
Copy link
Contributor Author

The original discussion was in djc/askama#989. I don't really see a reason why we didn't allow variables to be inherited. Well, fixed now.

@GuillaumeGomez GuillaumeGomez force-pushed the allow-variables-inheritance-in-block branch from b3f8110 to 07e6216 Compare November 22, 2024 17:44
// `<'b, 'b, ...>`. Except... it doesn't work because `self` still doesn't live long
// enough here for some reason...
MapChain::with_parent(unsafe {
mem::transmute::<
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm still very unhappy about this. I was able to improve the situation a lot but still get this last error:

error: lifetime may not live long enough
    --> rinja_derive/src/generator.rs:1166:13
     |
84   | impl<'a, 'b> Generator<'a, 'b> {
     |      --  -- lifetime `'b` defined here
     |      |
     |      lifetime `'a` defined here
...
1166 |             MapChain::with_parent(&self.locals),
     |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ argument requires that `'b` must outlive `'a`
     |
     = help: consider adding the following bound: `'b: 'a`

I pushed my changes here.

I'll try to continue to understand what's wrong. If anyone wants to give it a try though...

@Kijewski
Copy link
Collaborator

Kijewski commented Nov 23, 2024

Please have a look at my change whether it's okay for you.

I won't admit that it took me like 6 hours in total until I came up with a solution I liked.

@Kijewski Kijewski force-pushed the allow-variables-inheritance-in-block branch from 4c1a67b to 5604db8 Compare November 23, 2024 22:13
Remove inheritance from `MapChain`, and simply open a new scope.

Add lifetime to `Generator` and `Heritage` to reference its root
context.
@Kijewski Kijewski force-pushed the allow-variables-inheritance-in-block branch from 5604db8 to e182022 Compare November 23, 2024 22:55
@GuillaumeGomez
Copy link
Contributor Author

So Heritage was the issue! It was one of my next targets for finding out what went wrong. Less generics is a very good idea!

Book build is failing for some reason though? Seems unrelated to this PR.

@GuillaumeGomez
Copy link
Contributor Author

(also I cannot approve my own PR haha)

Copy link
Collaborator

@Kijewski Kijewski left a comment

Choose a reason for hiding this comment

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

👍

@Kijewski Kijewski merged commit 8709364 into rinja-rs:master Nov 23, 2024
36 checks passed
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.

Variables defined in base template, fail to be accessed in partials included by extended templates
2 participants