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

Interpolate unnamed enum variant fields in to_string attribute #345

Merged

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.

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
.enumerate()
.map(|(index, field)| {
assert!(field.ident.is_none());
let ident = syn::parse_str::<Ident>(format!("field{}", index).as_str()).unwrap();
Copy link

Choose a reason for hiding this comment

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

I think this use of format!() broke the no_std builds. We see error: cannot find macro format in this scope. Should this perhaps be alloc::fmt::format!?

Copy link

Choose a reason for hiding this comment

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

that format is fine as it is run at build time.
The problem is the format in the next chunk which is under a quote!

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.

4 participants