Clarify MatchAbbreviatedMonthName comment #92405
Conversation
ghost
assigned 
|
Tagging subscribers to this area: @dotnet/area-system-globalization Issue DetailsContextWhen admiring the code of
DetailsGetting all the cultures (from my system) that have conflicting prefixes of abberavited month names: foreach (var cultureInfo in CultureInfo.GetCultures(CultureTypes.AllCultures))
{
DateTimeFormatInfo dtfi = cultureInfo.DateTimeFormat;
int monthsInYear = (dtfi.GetAbbreviatedMonthName(13).Length == 0 ? 12 : 13);
List<string> monthsAbbreviations =
Enumerable.Range(1, monthsInYear)
.Select(dtfi.GetAbbreviatedMonthName)
.ToList();
List<string> conflictingTuples =
// month indexes combinations
Enumerable.Range(0, monthsInYear)
.SelectMany(m1 => Enumerable.Range(m1 + 1, monthsInYear - m1 - 1).Select(m2 => (m1, m2)))
// that are conflicting
.Where(tpl =>
monthsAbbreviations[tpl.m1].StartsWith(monthsAbbreviations[tpl.m2], true, cultureInfo)
||
monthsAbbreviations[tpl.m2].StartsWith(monthsAbbreviations[tpl.m1], true, cultureInfo)
)
.Select(tpl => monthsAbbreviations[tpl.m1] + " : " + monthsAbbreviations[tpl.m2])
.ToList();
if (conflictingTuples.Any())
{
Console.WriteLine($"{cultureInfo.DisplayName} ({cultureInfo.Name}) : " + string.Join(", ", conflictingTuples));
}
else
{
// Console.WriteLine(cultureInfo.DisplayName + ": OK");
}
}Conclusion: Conflicts exist, so the code is correct - it cannot short circuit exit matching after first match identified. However the culture mentioned in the comment is not one of the cases. It originaly confused me - I thought the code was copy-pasted from
|
|
@JanKrivanek thanks for helping with this. Could you please try to add some test for such a change? Maybe testing with |
src/libraries/System.Globalization.Calendars/tests/Misc/MiscCalendars.cs
Outdated
Show resolved
Hide resolved
|
/azp run |
|
You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list. |
|
/azp run runtime-nativeaot-outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@JanKrivanek I am trying to follow up to see what is wrong with CI legs which are not finishing. I know this is unrelated to your change, but we need to resolve it before we merge. |
|
Appreciated @tarekgh! |
|
@JanKrivanek it doesn't matter if the change is related or not. The CI leg shouldn't hang too long anyway. This is what we need to resolve. @agocke could you please help look at these hanging CI legs? |
Context
When admiring the code of
MatchAbbreviatedMonthName(credit to @cincuranet for pointing such gem) I found 2 nits:GetMonthName(probably copied fromMatchMonthName?), while following code callsGetAbbreviatedMonthName. MaterializingmonthNamesis unnecessary here.Details
Getting all the cultures (from my system) that have conflicting prefixes of abberavited month names:
Conclusion: Conflicts exist, so the code is correct - it cannot short circuit exit matching after first match identified. However the culture mentioned in the comment is not one of the cases. It originaly confused me - I thought the code was copy-pasted from
MatchMonthName- where cs-CZ realy hase conflicting prefixes of full month names ("červen", "červenec").