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 support for .. in let pattern matching for structs (alternative take) #36

Merged
merged 7 commits into from
Jun 28, 2024

Conversation

Kijewski
Copy link
Collaborator

This is an alternative solution to implement rest patters .. for struct or tuple targets. I think this implementation is a little bit easier to read and understand. I hope it's OK that I opened a new PR!

Closes #35.

|
= note: this error originates in the derive macro `Template` (in Nightly builds, run with -Z macro-backtrace for more info)

error: failed to parse template source at row 2, column 17 near:
Copy link
Contributor

Choose a reason for hiding this comment

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

This error message got worse, should precise that a comma is missing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Absolutely right. I added a new helper function collect_targets that has better error messages likely errors such as missing a comma.

@GuillaumeGomez
Copy link
Contributor

Yes no problem. Your code is SO MUCH better, thanks a lot! Just some nits and suggestions for better error messages but it's really details at this point.

@Kijewski
Copy link
Collaborator Author

Your code is SO MUCH better, thanks a lot!

Thank you! I loved compiler construction during my studies, and I'm always happy when I find an excuse to still dabble into that field nowadays.

Just some nits and suggestions for better error messages but it's really details at this point.

Yes, I think good error messages make all the difference. It's a meme that rust tells you with subpixel accuracy where your mistakes are and how to fix them, and we should embrace it. :)


#[derive(Template)]
#[template(source = "
{%- if let X { a, 1 } = x -%}hello {{ a }}{%- endif -%}
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh nice idea!

#[template(source = "
{%- if let X { a, 1 } = x -%}hello {{ a }}{%- endif -%}
", ext = "html")]
struct T3 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just thought about another case:

let X { a, .., b } = x

and also:

let X { .., a } = x

I suppose the parsing will fail but better ensure the errors are good.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I expanded the error message a bit to give better information how to use the .. in named structs.

@GuillaumeGomez
Copy link
Contributor

Your code is SO MUCH better, thanks a lot!

Thank you! I loved compiler construction during my studies, and I'm always happy when I find an excuse to still dabble into that field nowadays.

Glad you're enjoying it! 😄

Just some nits and suggestions for better error messages but it's really details at this point.

Yes, I think good error messages make all the difference. It's a meme that rust tells you with subpixel accuracy where your mistakes are and how to fix them, and we should embrace it. :)

Definitely! askama's great, but error messages were terrible. I'm glad things improved to this point already. We can still do better and I think we will!

@GuillaumeGomez
Copy link
Contributor

Great work, thanks a lot!

@GuillaumeGomez GuillaumeGomez merged commit eb41904 into rinja-rs:master Jun 28, 2024
17 checks passed
@Kijewski Kijewski deleted the rest-pattern-struct#2 branch June 28, 2024 22:04
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.

2 participants