Skip to content

Commit

Permalink
Fix the ICU time format conversion logic (dotnet#103681)
Browse files Browse the repository at this point in the history
* Fix the ICU time format conversion logic

Revise the ICU time format conversion logic to support all unquoted literal texts and the `B` and `b` pattern symbols.

Fix dotnet#103592

* Clarify literal texts in the conversion logic

* Add tests for verifying time patterns

Add tests verifying that all the short and long time patterns either use
a 24-hour clock or have an AM/PM designator.

* Fix literal single quote and literal backslash conversion

* Refactor the literal quote conversion logic

* Revise the test logic to ignore literal texts and check pattern redundancy

Modify the test logic so that it recognizes literal texts correctly, and
fails if 12-hour and 24-hour clocks are used at the same time.

* Revise the test logic to ensure all cultures are tested

* Add comments to clarify the backslash conversion

* Refactor the conversion logic

Simplify some logic and improve readability.

* Exclude bad ICU patterns from the tests

* Exclude the VerifyTimePatterns tests from hybrid globalization on browser

* Add missing usings

* Improve readability of the for-loops
  • Loading branch information
PopSlime authored and rzikm committed Jun 24, 2024
1 parent 4220bae commit ecf7165
Show file tree
Hide file tree
Showing 4 changed files with 128 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -364,32 +364,59 @@ private static string ConvertIcuTimeFormatString(ReadOnlySpan<char> icuFormatStr
char current = icuFormatString[i];
switch (current)
{
case '\\':
// The ICU format does not use backslashes to escape while the .NET format does
result[resultPos++] = '\\';
result[resultPos++] = '\\';
break;
case '\'':
result[resultPos++] = icuFormatString[i++];
while (i < icuFormatString.Length)
static bool HandleQuoteLiteral(ReadOnlySpan<char> icuFormatString, ref int i, Span<char> result, ref int resultPos)
{
if (i + 1 < icuFormatString.Length && icuFormatString[i + 1] == '\'')
{
result[resultPos++] = '\\';
result[resultPos++] = '\'';
i++;
return true;
}
result[resultPos++] = '\'';
return false;
}

if (HandleQuoteLiteral(icuFormatString, ref i, result, ref resultPos))
{
break;
}
i++;
for (; i < icuFormatString.Length; i++)
{
current = icuFormatString[i];
result[resultPos++] = current;
if (current == '\'')
{
if (HandleQuoteLiteral(icuFormatString, ref i, result, ref resultPos))
{
continue;
}
break;
}
i++;
if (current == '\\')
{
// The same backslash escaping mentioned above
result[resultPos++] = '\\';
}
result[resultPos++] = current;
}
break;

case ':':
case '.':
case 'H':
case 'h':
case 'm':
case 's':
case ' ':
case '\u00A0': // no-break space
case '\u202F': // narrow no-break space
result[resultPos++] = current;
break;
case 'a': // AM/PM
case 'b': // am, pm, noon, midnight
case 'B': // flexible day periods
if (!amPmAdded)
{
amPmAdded = true;
Expand All @@ -398,6 +425,13 @@ private static string ConvertIcuTimeFormatString(ReadOnlySpan<char> icuFormatStr
}
break;

default:
// Characters that are not ASCII letters are literal texts
if (!char.IsAsciiLetter(current))
{
result[resultPos++] = current;
}
break;
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.Runtime.InteropServices;
using Xunit;

Expand Down Expand Up @@ -58,5 +59,19 @@ public static Exception GetCultureNotSupportedException(CultureInfo cultureInfo)
cultureInfo.Name,
cultureInfo.Calendar.GetType().Name));
}

// These cultures have bad ICU time patterns below the corresponding versions
// They are excluded from the VerifyTimePatterns tests
public static readonly Dictionary<string, Version> _badIcuTimePatterns = new Dictionary<string, Version>()
{
{ "mi", new Version(65, 0) },
{ "mi-NZ", new Version(65, 0) },
};
public static bool HasBadIcuTimePatterns(CultureInfo culture)
{
return PlatformDetection.IsIcuGlobalizationAndNotHybridOnBrowser
&& _badIcuTimePatterns.TryGetValue(culture.Name, out var version)
&& PlatformDetection.ICUVersion < version;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,41 @@ public void LongTimePattern_CheckReadingTimeFormatWithSingleQuotes_ICU()
}
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
public void LongTimePattern_VerifyTimePatterns()
{
Assert.All(CultureInfo.GetCultures(CultureTypes.AllCultures), culture => {
if (DateTimeFormatInfoData.HasBadIcuTimePatterns(culture))
{
return;
}
var pattern = culture.DateTimeFormat.LongTimePattern;
bool use24Hour = false;
bool use12Hour = false;
bool useAMPM = false;
for (var i = 0; i < pattern.Length; i++)
{
switch (pattern[i])
{
case 'H': use24Hour = true; break;
case 'h': use12Hour = true; break;
case 't': useAMPM = true; break;
case '\\': i++; break;
case '\'':
i++;
for (; i < pattern.Length; i++)
{
var c = pattern[i];
if (c == '\'') break;
if (c == '\\') i++;
}
break;
}
}
Assert.True((use24Hour || useAMPM) && (use12Hour ^ use24Hour), $"Bad long time pattern for culture {culture.Name}: '{pattern}'");
});
}

[Fact]
public void LongTimePattern_CheckTimeFormatWithSpaces()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,41 @@ public void ShortTimePattern_SetReadOnly_ThrowsInvalidOperationException()
Assert.Throws<InvalidOperationException>(() => DateTimeFormatInfo.InvariantInfo.ShortTimePattern = "HH:mm");
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotHybridGlobalizationOnBrowser))]
public void ShortTimePattern_VerifyTimePatterns()
{
Assert.All(CultureInfo.GetCultures(CultureTypes.AllCultures), culture => {
if (DateTimeFormatInfoData.HasBadIcuTimePatterns(culture))
{
return;
}
var pattern = culture.DateTimeFormat.ShortTimePattern;
bool use24Hour = false;
bool use12Hour = false;
bool useAMPM = false;
for (var i = 0; i < pattern.Length; i++)
{
switch (pattern[i])
{
case 'H': use24Hour = true; break;
case 'h': use12Hour = true; break;
case 't': useAMPM = true; break;
case '\\': i++; break;
case '\'':
i++;
for (; i < pattern.Length; i++)
{
var c = pattern[i];
if (c == '\'') break;
if (c == '\\') i++;
}
break;
}
}
Assert.True((use24Hour || useAMPM) && (use12Hour ^ use24Hour), $"Bad short time pattern for culture {culture.Name}: '{pattern}'");
});
}

[Fact]
public void ShortTimePattern_CheckTimeFormatWithSpaces()
{
Expand Down

0 comments on commit ecf7165

Please sign in to comment.