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

Support for multilingual DisplayAttribute #1145

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Support for multilingual DisplayAttribute #1145

wants to merge 1 commit into from

Conversation

FarhadGhE
Copy link

@FarhadGhE FarhadGhE commented Mar 12, 2020

I needed NJsonSchema in a multilingual project, where each language was added in .resx files. NJsonSchema didn't respect ResourceType argument in DisplayAttribute. So I added support for this scenario. Also it doesn't matter if the resource class has internal or public accessors.
Since Net standard 1.0 doesn't have BindingFlags, I kept the past behavior if target framework is Net standard 1.0

… resource via reflection. This is important in multilingual projects.
@@ -20,6 +20,9 @@
<PropertyGroup Condition="'$(TargetFramework)' == 'net40'">
<DefineConstants>LEGACY</DefineConstants>
</PropertyGroup>
<PropertyGroup Condition="'$(TargetFramework)' == 'netstandard1.0'">
Copy link
Owner

Choose a reason for hiding this comment

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

I think there is already a const for this (not needed here)

@RicoSuter
Copy link
Owner

Thanks for the PR. Please also add some unit tests.
Doesnt this introduce a breaking change for some people?

@wazzamatazz
Copy link
Contributor

wazzamatazz commented Feb 12, 2021

I've just run into this exact problem - getting values from resources would be an incredibly useful addition. I don't think it's a breaking change, because the behaviour when you specify [Display] but don't specify a ResourceType is the same as the current behaviour i.e. use the value of the Name property directly instead of looking up a resource.

That said, it would be easier to just let the attribute do the heavy lifting for you, and use the GetName and GetDescription methods to get the name and description, as these return either the literal value or a resource depending on how the attribute has been declared. Both are available since .NET Framework 4.0.

@RicoSuter
Copy link
Owner

If there are GetName() and GetDescription() especially for that we should use that and not "reimplement" their logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants