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 enum support #223

Open
GuillaumeGomez opened this issue Nov 5, 2024 · 6 comments · May be fixed by #255
Open

Add enum support #223

GuillaumeGomez opened this issue Nov 5, 2024 · 6 comments · May be fixed by #255
Assignees

Comments

@GuillaumeGomez
Copy link
Contributor

It would allow to do:

#[derive(Template)]
#[derive(Serialize, Deserialize)]
pub enum QuestionData {
	#[template(path = "./templates/question/editor/multiple-text.html")]
	MultipleChoiceText {
		numbering: Numbering,
		text: Vec<String>,
	},

	#[template(path = "./templates/question/editor/multiple-image.html")]
	MultipleChoiceImage {
		numbering: Numbering,
		images: Vec<i32>,
	},
}

And so calling render on the enum would be on its variant instead. It was suggested in djc/askama#1104 and I kinda like the idea.

@darrenmothersele
Copy link

I agree, this would be useful. Something like this, perhaps?

@GuillaumeGomez
Copy link
Contributor Author

With a default template? Hum why not!

@Kijewski
Copy link
Collaborator

Kijewski commented Nov 9, 2024

I think it would a good feature to have!

The main problem I have are the constants in Template. For the SIZE_HINT we could simply use the median, I guess. But EXTENSION and MIME_TYPE might be off, e.g. if one enum variant should generate a plaintext error message.

I think, in a first iteration we could require that the ext parameter is the same for all variants. Later on, we could explore how to lift this requirement. If you too think that's a good idea, then I'll have a look how best to salvage askama-enum. I think most of the code can be copied.

@GuillaumeGomez
Copy link
Contributor Author

Not sure if median is the best idea here. Maybe taking the biggest value would be better?

@Kijewski Kijewski self-assigned this Nov 10, 2024
@Kijewski Kijewski linked a pull request Nov 19, 2024 that will close this issue
@chrisp60
Copy link
Contributor

As a user, I would expect the HINT_SIZE to derived from the largest variant. This would mimic the rust sizing of enums. Just an opinion.

Maybe an attribute can be accepted when deriving enums that lets you specify an escape hatch. Unsure if it is worth the maintenance cost for how often it would actually get used. I imagine most users would just take the default and not worry about it.

#[derive(Template)]
#[derive(Serialize, Deserialize)]
#[template(size_hint = "median")]
// #[template(size_hint = "largest")] default
// #[template(size_hint = Self::MultipleChoiceText)] a "hot path" variant
pub enum QuestionData {
	#[template(path = "./templates/question/editor/multiple-text.html")]
	MultipleChoiceText {
		numbering: Numbering,
		text: Vec<String>,
	},

	#[template(path = "./templates/question/editor/multiple-image.html")]
	MultipleChoiceImage {
		numbering: Numbering,
		images: Vec<i32>,
	},
}

FWIW - I have many cases where I use a single field struct to wrap and display an enum like this, so this feature would be a great add. Especially Result like templates for handling form submissions would be 👍

If work is needed on implementation, I may be able to help a bit. Still getting used to the codebase and it seems substantial progress has already been made. Maybe if documentation or examples are needed?

@GuillaumeGomez
Copy link
Contributor Author

As a user, I would expect the HINT_SIZE to derived from the largest variant. This would mimic the rust sizing of enums.

It's also what I think.

Maybe an attribute can be accepted when deriving enums that lets you specify an escape hatch. Unsure if it is worth the maintenance cost for how often it would actually get used. I imagine most users would just take the default and not worry about it.

That seems a bit too much in my opinion.

If work is needed on implementation, I may be able to help a bit. Still getting used to the codebase and it seems substantial progress has already been made. Maybe if documentation or examples are needed?

Work is already in progress in #255. But if you have some tests in mind, don't hesitate to post them there.

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 a pull request may close this issue.

4 participants