-
Notifications
You must be signed in to change notification settings - Fork 57
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
align the enum name handling with the behavior of system.text.json #53
Comments
I don't see this feature supported as at 5.0.100-rc.1.20452.10... The PR is mostly fine, but I have a couple of concerns:
|
Looking through some of the PRs to donet/runtime & dotnet/corefx for the linked issue, I came across this one https://github.com/dotnet/corefx/pull/41648/files which uses reflection with the string names of types and attributes. If we applied the same process within this library we could support the Attribute enumMemberAttribute = field.GetCustomAttributes()?.FirstOrDefault(ca => ca.GetType().FullName == "System.Runtime.Serialization.EnumMemberAttribute");
if (enumMemberAttribute != null)
{
transformedName = GetEnumMemberValue(enumMemberAttribute) ?? name;
}
...
private static string GetEnumMemberValue(Attribute enumMemberAttribute)
{
var enumMemberAttributeValuePropertyInfo = enumMemberAttribute
.GetType()
.GetProperty("Value", BindingFlags.Public | BindingFlags.Instance);
return (string)enumMemberAttributeValuePropertyInfo.GetValue(enumMemberAttribute);
} This would also solve #48 without needing a separate package. |
I think the latter approach makes more sense for the |
align the enum handling with the behavior of
System.Text.Json
:JsonStringEnumConverter
is specifiedother than that, use the
EnumMemberAttribute
if the internalEnumMemberConverter
is specifiedas this behavior is subject to be changed in .NET5 (dotnet/runtime#31081) I would propose to add support for the
JsonStringEnumMemberConverter
(part ofMacross.Json.Extensions
) by applying the same logic as with the internalEnumMemberConverter
.The text was updated successfully, but these errors were encountered: