Implement support for "mul" locale in LocalizedText Class & ResourceManager #3076
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3076 +/- ##
==========================================
+ Coverage 57.36% 57.47% +0.10%
==========================================
Files 357 357
Lines 69015 69113 +98
Branches 14164 14201 +37
==========================================
+ Hits 39593 39721 +128
+ Misses 25180 25147 -33
- Partials 4242 4245 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…les provieded to Resource Manager (return only requested locales)
romanett
changed the title
mrsuciu
left a comment
mrsuciu
left a comment
There was a problem hiding this comment.
I do not know exactly if {"t":[["en-US","Hello"],["de-DE","Hallo"]]} should be considered the same as {"t":[["de-DE","Hallo"],["en-US","Hello"]]}. Semantically I would say they represent the same translations. If this is true than LocalizedText.Equals and LocalizedText.GetHashCode methods should be changed to take this into account and operate on the translations not the string.
| t = string.Format(table.Locale, t, info.Args); | ||
| } | ||
| catch | ||
| { } |
There was a problem hiding this comment.
Shouldn't it be at least logged ?
There was a problem hiding this comment.
in the old implementation it was ignored as well and just the old text used formattedText = translatedText;
| translation = string.Format(culture, translation, info.Args); | ||
| } | ||
| catch | ||
| { } |
There was a problem hiding this comment.
Shouldn't it be at least logged ?
There was a problem hiding this comment.
in the old implementation it was ignored as well and just the old text used formattedText = translatedText;
| } | ||
| catch | ||
| { | ||
| formattedText = translatedText; |
There was a problem hiding this comment.
Shouldn't it be at least logged ?
There was a problem hiding this comment.
in the old implementation it was ignored as well
| } | ||
| catch | ||
| { | ||
| culture = CultureInfo.InvariantCulture; |
There was a problem hiding this comment.
Shouldn't it be at least logged ?
There was a problem hiding this comment.
in the old implementation it was not. I think there a cases where it is okay that the locale is not known to the dotnet culture info, so it would spam the logging with unnecessary information
| // else return the first available locale | ||
| else | ||
| { | ||
| KeyValuePair<string, string> defaultKVP = Translations.First(); |
There was a problem hiding this comment.
Add check if not empty since calling First() on empty Translations might throw exception.
There was a problem hiding this comment.
m_locale always has a value there so the dictionary is never empty
I disagree, the order of translations matters as it could represent the priority of the received translations for the client. so these two texts should not be considered eqal. |
I did not find a clear distinction criteria between two such localizedtext definitions in the spec other than the definition which indeed points to your conclusion since the locale and text pairs are put into an array (where order matters). |
2 participants
Proposed changes
The OPC UA Spec Part 3 (https://reference.opcfoundation.org/Core/Part3/v105/docs/8.5) describes the Support for a special 'mul' locale. This PR adds support for this 'mul' locale.
If the LocalizedText class is now initialized with a 'mul' local the new Property
IsMulitiLanguageis set to true and the new PropertyTranslationscontains a Dictionary of strings with the different locales / texts extracted from the JSON in theTextProperty.Also a new Constructor was added which takes a dictionary of strings to initalize a new 'mul' locale text. This constructor automatically creates the needed json.
The
TranslationsProperty can also be set and a 'mul' locale with sufficient text is created.Also the translation manager is adapted to return only requested locales if a 'mul' locale string is provided and also support 'mul' locae requests by the client, as described in https://reference.opcfoundation.org/Core/Part4/v105/docs/5.4.
Types of changes
Checklist