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

[WIP] Implement enum variants #255

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

Conversation

Kijewski
Copy link
Collaborator

Resolves #223.

rinja_derive/src/integration.rs Fixed Show fixed Hide fixed
rinja_derive/src/integration.rs Fixed Show fixed Hide fixed
@Kijewski Kijewski force-pushed the pr-enum branch 4 times, most recently from faae42b to b9f1a5f Compare November 24, 2024 03:27
@Kijewski Kijewski force-pushed the pr-enum branch 3 times, most recently from efdb6fc to 06290b4 Compare November 24, 2024 04:40
rinja_derive/src/input.rs Dismissed Show resolved Hide resolved
@Kijewski Kijewski force-pushed the pr-enum branch 6 times, most recently from a2e9198 to 33afb4c Compare November 24, 2024 23:13
@Kijewski Kijewski force-pushed the pr-enum branch 6 times, most recently from a948a9d to aa66f84 Compare November 26, 2024 00:53
@@ -271,6 +271,69 @@ impl TemplateInput<'_> {
}
}

pub(crate) enum AnyTemplateArgs {
Item(TemplateArgs),
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be better to name it Struct since we don't support unions currently.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed.

T: Clone,
A: FnMut(&mut S) -> &mut Option<T>,
{
if let dest @ None = access(dest) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It's super weird. What about:

Suggested change
if let dest @ None = access(dest) {
if access(dest).is_none() {

?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's the same as let dest = access(dest); if dest.is_none() { in one line. I found it nifty. :) But changed.

@GuillaumeGomez
Copy link
Contributor

So if I understand correctly, when a variant has a #[template()] attribute, we generate a struct representing it then call render/render_into() on this struct, right?

Very hypothetical (and in case I understood everything correctly up to this point): isn't it possible to instead handle the variant like we handle our structs. Trying to explain in code:

#[template(src = "boo.txt")]
enum Foo {
    A,
    #[template(src = "I-am-b.txt")]
    B(u32, u32),
    #[template(src = "not-boo.txt")]
    C { a: u32 },
}

impl Render for Foo {
    fn render(&self) -> Result<String> {
        match self {
            // First the variants with the template
            B(_0, _1) => {
                // this one needs a struct binding :'(
            }
            C { a } => {
                // We can re-use the code for structs where we can say that this is a struct with a field named `a` and tada.
            }
        }
    }
}

Not sure if it's a good idea though... That would allow to skip a type creation for one case. But then self.a wouldn't work. So I think overall I just had a terrible idea. ^^'

@Kijewski Kijewski force-pushed the pr-enum branch 2 times, most recently from bf0e2ac to b5c642d Compare November 28, 2024 05:41
@Kijewski
Copy link
Collaborator Author

Yep, I implemented it this way so that self.x still works. I'd let the compile/optimizer decide if it's good idea to inline everything into one big function or if the implementations for every variant should stay.

@Kijewski Kijewski force-pushed the pr-enum branch 4 times, most recently from 5424b18 to 92cb0bd Compare November 28, 2024 06:18
@GuillaumeGomez
Copy link
Contributor

Please tell me when it's ready for final review. ;)

(also git conflict)

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.

Add enum support
2 participants