-
Notifications
You must be signed in to change notification settings - Fork 5
V9.0.0/housekeeping for extensions globalization #90
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -59,15 +59,26 @@ public static IEnumerable<CultureInfo> UseNationalLanguageSupport(this IEnumerab | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| var surrogate = typeof(CultureInfoExtensions).GetEmbeddedResources($"{culture.Name}.bin", ManifestResourceMatch.ContainsName).SingleOrDefault(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| var ms = new MemoryStream(surrogate.Value.DecompressGZip().ToByteArray()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| var suggogateCulture = YamlFormatter.DeserializeObject<CultureInfoSurrogate>(ms, o => | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| var surrogateCulture = YamlFormatter.DeserializeObject<CultureInfoSurrogate>(ms, o => | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| o.Settings.NamingConvention = NullNamingConvention.Instance; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| o.Settings.ReflectionRules = new MemberReflection(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| o.Settings.IndentSequences = false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Enrich(culture, suggogateCulture); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| EnrichedCultureInfos.Add(culture); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| enrichedCultures.Add(culture); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (culture.IsReadOnly) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| var cultureClone = culture.Clone() as CultureInfo; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Enrich(cultureClone, surrogateCulture); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| EnrichedCultureInfos.Add(cultureClone); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| enrichedCultures.Add(cultureClone); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Enrich(culture, surrogateCulture); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| EnrichedCultureInfos.Add(culture); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| enrichedCultures.Add(culture); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+69
to
+81
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Prevent Duplicate Entries in There's a potential for duplicate entries in Modify the code to check for existing entries before adding: if (culture.IsReadOnly)
{
var cultureClone = culture.Clone() as CultureInfo;
Enrich(cultureClone, surrogateCulture);
- EnrichedCultureInfos.Add(cultureClone);
+ if (!EnrichedCultureInfos.Any(ci => ci.Name.Equals(cultureClone.Name, StringComparison.Ordinal)))
+ {
+ EnrichedCultureInfos.Add(cultureClone);
+ }
enrichedCultures.Add(cultureClone);
}
else
{
Enrich(culture, surrogateCulture);
- EnrichedCultureInfos.Add(culture);
+ if (!EnrichedCultureInfos.Any(ci => ci.Name.Equals(culture.Name, StringComparison.Ordinal)))
+ {
+ EnrichedCultureInfos.Add(culture);
+ }
enrichedCultures.Add(culture);
}📝 Committable suggestion
Suggested change
🛠️ Refactor suggestion Optional: Preserve Read-Only State After Enrichment If it's important to maintain the read-only state of the original Apply this change to preserve the read-only state: var cultureClone = culture.Clone() as CultureInfo;
Enrich(cultureClone, surrogateCulture);
+var readOnlyCultureClone = CultureInfo.ReadOnly(cultureClone);
-EnrichedCultureInfos.Add(cultureClone);
+EnrichedCultureInfos.Add(readOnlyCultureClone);
-enrichedCultures.Add(cultureClone);
+enrichedCultures.Add(readOnlyCultureClone);📝 Committable suggestion
Suggested change
Ensure Thread Safety When Modifying Shared Collections The static list One way to address this is by replacing -private static readonly List<CultureInfo> EnrichedCultureInfos = new();
+private static readonly ConcurrentBag<CultureInfo> EnrichedCultureInfos = new();
...
if (enrichedCulture != null)
{
enrichedCultures.Add(enrichedCulture);
}
else
{
// Existing code
- EnrichedCultureInfos.Add(cultureClone);
+ EnrichedCultureInfos.Add(cultureClone);
enrichedCultures.Add(cultureClone);
}Alternatively, implement a locking mechanism: +private static readonly object EnrichedCultureInfosLock = new();
...
if (enrichedCulture != null)
{
enrichedCultures.Add(enrichedCulture);
}
else
{
+ lock (EnrichedCultureInfosLock)
+ {
// Existing code
EnrichedCultureInfos.Add(cultureClone);
+ }
enrichedCultures.Add(cultureClone);
}
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return enrichedCultures; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dispose
MemoryStreamto Prevent Memory LeaksThe
MemoryStreaminstancemsis not being disposed, which can lead to memory leaks. Consider wrapping it in ausingstatement to ensure it is properly disposed after use.Apply this diff to address the issue: