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.

Sign up for GitHub

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

Jump to bottom

Interpolate unnamed enum variant fields in to_string attribute #345

Merged
merged 3 commits into from May 5, 2024
Merged

Interpolate unnamed enum variant fields in to_string attribute #345

merged 3 commits into from May 5, 2024
+93 −36

Conversation

gin-ahirsch
Copy link
Contributor

I'm thinking it would also make sense to emit an error, or at least a warning, if {}-brackets are found in the to_string-literal on unit variants. I'd be happy to add that.

@Peternator7
Copy link
Owner

Looks great! Only change is let's error on the empy {}. In the example from the PR: "hue is {1}, saturation is {}", mixing them feels bad to me so let's just not allow it by disallowing the empty braces

@gin-ahirsch
Copy link
Contributor Author

I was going for an equivalency to how print!() works. I agree mixing them looks bad, in the example I just wanted to show either syntax works.
I would think very commonly unnamed enum fields only have a single member anyways.
But if you're against it on principle I'll remove support for empty {}.

@gin-ahirsch
Copy link
Contributor Author

What about disallowing {} on unit variants? Currently that will format "as-is", i.e. an on_string="{}" is like format!("{{}}") on a unit variant.

@Peternator7
Copy link
Owner

Hey, sorry for the delay here. I don't feel too strongly either way. I'm inclined to reject it for consistency with other types of enums, but could be persuaded the other way.

@gin-ahirsch
Reject "{}" in unit-variants in enums
b2ba013 
Strings for unit-variants in enums were taken verbatim and not passed to
format!() since there's no associated values that *could* be formatted.
For consistency with non-unit variants these now have to use the escaped
form "{{}}", otherwise an error is produced for the format string.
@gin-ahirsch
Copy link
Contributor Author

I lean the same way, so I changed it to now invoke format!() as well, which will produce an error if a {} is present in the format-string since no additional values are passed to fill it in.

@Peternator7 Peternator7 merged commit 186d29f into Peternator7:master May 5, 2024
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Peternator7 Peternator7 Peternator7 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@gin-ahirsch @Peternator7