Closed gin-ahirsch closed 6 months ago
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
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 {}
.
What about disallowing {}
on unit variants? Currently that will format "as-is", i.e. an on_string="{}"
is like format!("{{}}")
on a unit variant.
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.
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.
I'm thinking it would also make sense to emit an error, or at least a warning, if
{}
-brackets are found in theto_string
-literal on unit variants. I'd be happy to add that.