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.

Sign up for GitHub

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

Jump to bottom

[HybridGlobalization] Pass non-breaking space / narrow non-breaking space characters #103226

Merged
merged 6 commits into from
Jun 18, 2024
Merged

[HybridGlobalization] Pass non-breaking space / narrow non-breaking space characters #103226

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
cff1dd2
convert narrow no-break spaces to spaces too
mkhamoyan Jun 10, 2024
49335c2
Enable test cases only for ICU and hybrid mode
mkhamoyan Jun 10, 2024
f107812
Pass non-breaking, narrow non-breaking space as they are
mkhamoyan Jun 11, 2024
112b217
Merge branch 'main' into fix_spaces_hybrid
mkhamoyan Jun 11, 2024
535a931
Apply feedback
mkhamoyan Jun 13, 2024
be6111c
Merge branch 'main' into fix_spaces_hybrid
mkhamoyan Jun 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 25 additions & 10 deletions src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Icu.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,18 +291,34 @@ private unsafe string IcuGetTimeFormatString(bool shortFormat)
Debug.Assert(!GlobalizationMode.UseNls);
Debug.Assert(_sWindowsName != null, "[CultureData.GetTimeFormatString(bool shortFormat)] Expected _sWindowsName to be populated already");

char* buffer = stackalloc char[ICU_ULOC_KEYWORD_AND_VALUES_CAPACITY];

bool result = Interop.Globalization.GetLocaleTimeFormat(_sWindowsName, shortFormat, buffer, ICU_ULOC_KEYWORD_AND_VALUES_CAPACITY);
if (!result)
ReadOnlySpan<char> span;
#if TARGET_MACCATALYST || TARGET_IOS || TARGET_TVOS
if (GlobalizationMode.Hybrid)
{
// Failed, just use empty string
Debug.Fail("[CultureData.GetTimeFormatString(bool shortFormat)] Failed");
return string.Empty;
string res = Interop.Globalization.GetLocaleTimeFormatNative(_sWindowsName, shortFormat);
mkhamoyan marked this conversation as resolved.
Show resolved Hide resolved
if (string.IsNullOrEmpty(res))
{
Debug.Fail("[CultureData.GetTimeFormatString(bool shortFormat)] Failed");
return string.Empty;
}
span = res.AsSpan();
}
else
#endif
{
char* buffer = stackalloc char[ICU_ULOC_KEYWORD_AND_VALUES_CAPACITY];
bool result = Interop.Globalization.GetLocaleTimeFormat(_sWindowsName, shortFormat, buffer, ICU_ULOC_KEYWORD_AND_VALUES_CAPACITY);
if (!result)
{
// Failed, just use empty string
Debug.Fail("[CultureData.GetTimeFormatString(bool shortFormat)] Failed");
return string.Empty;
}
span = new ReadOnlySpan<char>(buffer, ICU_ULOC_KEYWORD_AND_VALUES_CAPACITY);
span = span.Slice(0, span.IndexOf('\0'));
}

var span = new ReadOnlySpan<char>(buffer, ICU_ULOC_KEYWORD_AND_VALUES_CAPACITY);
return ConvertIcuTimeFormatString(span.Slice(0, span.IndexOf('\0')));
return ConvertIcuTimeFormatString(span);
}

// no support to lookup by region name, other than the hard-coded list in CultureData
Expand Down Expand Up @@ -373,7 +389,6 @@ private static string ConvertIcuTimeFormatString(ReadOnlySpan<char> icuFormatStr
case '\u202F': // narrow no-break space
result[resultPos++] = current;
break;

case 'a': // AM/PM
if (!amPmAdded)
{
Expand Down
4 changes: 0 additions & 4 deletions src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Unix.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,7 @@ internal sealed partial class CultureData

private string[]? GetTimeFormatsCore(bool shortFormat)
{
#if TARGET_MACCATALYST || TARGET_IOS || TARGET_TVOS
string format = GlobalizationMode.Hybrid ? GetTimeFormatStringNative(shortFormat) : IcuGetTimeFormatString(shortFormat);
#else
string format = IcuGetTimeFormatString(shortFormat);
#endif
return new string[] { format };
}

Expand Down
4 changes: 0 additions & 4 deletions src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1978,11 +1978,7 @@ internal string TimeSeparator
}
else
{
#if TARGET_MACCATALYST || TARGET_IOS || TARGET_TVOS
string? longTimeFormat = GlobalizationMode.Hybrid ? GetTimeFormatStringNative() : IcuGetTimeFormatString();
#else
string? longTimeFormat = ShouldUseUserOverrideNlsData ? NlsGetTimeFormatString() : IcuGetTimeFormatString();
#endif
if (string.IsNullOrEmpty(longTimeFormat))
{
longTimeFormat = LongTimes[0];
Expand Down
68 changes: 0 additions & 68 deletions src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.iOS.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ namespace System.Globalization
{
internal sealed partial class CultureData
{
private const int LOC_FULLNAME_CAPACITY = 157; // max size of locale name

internal static string GetLocaleNameNative(string localeName)
{
return Interop.Globalization.GetLocaleNameNative(localeName);
Expand Down Expand Up @@ -64,71 +62,5 @@ private int[] GetLocaleInfoNative(LocaleGroupingData type)

return new int[] { primaryGroupingSize, secondaryGroupingSize };
}

private string GetTimeFormatStringNative() => GetTimeFormatStringNative(shortFormat: false);

private string GetTimeFormatStringNative(bool shortFormat)
{
Debug.Assert(_sWindowsName != null, "[CultureData.GetTimeFormatStringNative(bool shortFormat)] Expected _sWindowsName to be populated already");

string result = Interop.Globalization.GetLocaleTimeFormatNative(_sWindowsName, shortFormat);

return ConvertNativeTimeFormatString(result);
}

private static string ConvertNativeTimeFormatString(string nativeFormatString)
{
Span<char> result = stackalloc char[LOC_FULLNAME_CAPACITY];

bool amPmAdded = false;
int resultPos = 0;

for (int i = 0; i < nativeFormatString.Length; i++)
{
switch (nativeFormatString[i])
{
case '\'':
result[resultPos++] = nativeFormatString[i++];
while (i < nativeFormatString.Length)
{
char current = nativeFormatString[i];
result[resultPos++] = current;
if (current == '\'')
{
break;
}
i++;
}
break;

case ':':
case '.':
case 'H':
case 'h':
case 'm':
case 's':
result[resultPos++] = nativeFormatString[i];
break;

case ' ':
case '\u00A0':
// Convert nonbreaking spaces into regular spaces
result[resultPos++] = ' ';
break;

case 'a': // AM/PM
if (!amPmAdded)
{
amPmAdded = true;
result[resultPos++] = 't';
result[resultPos++] = 't';
}
break;

}
}

return result.Slice(0, resultPos).ToString();
}
}
}
14 changes: 14 additions & 0 deletions .../tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongTimePattern.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,5 +283,19 @@ public void LongTimePattern_CheckReadingTimeFormatWithSingleQuotes_ICU()
}
}
}

[Fact]
public void LongTimePattern_CheckTimeFormatWithSpaces()
{
var date = DateTime.Today + TimeSpan.FromHours(15) + TimeSpan.FromMinutes(15);
var culture = new CultureInfo("en-US");
string formattedDate = date.ToString("t", culture);
bool containsSpace = formattedDate.Contains(' ');
bool containsNoBreakSpace = formattedDate.Contains('\u00A0');
bool containsNarrowNoBreakSpace = formattedDate.Contains('\u202F');

Assert.True(containsSpace || containsNoBreakSpace || containsNarrowNoBreakSpace,
$"Formatted date string '{formattedDate}' does not contain any of the specified spaces.");
}
}
}
14 changes: 14 additions & 0 deletions ...tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoShortTimePattern.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,5 +254,19 @@ public void ShortTimePattern_SetReadOnly_ThrowsInvalidOperationException()
{
Assert.Throws<InvalidOperationException>(() => DateTimeFormatInfo.InvariantInfo.ShortTimePattern = "HH:mm");
}

[Fact]
public void ShortTimePattern_CheckTimeFormatWithSpaces()
{
var date = DateTime.Today + TimeSpan.FromHours(15) + TimeSpan.FromMinutes(15);
var culture = new CultureInfo("en-US");
string formattedDate = date.ToString("t", culture);
bool containsSpace = formattedDate.Contains(' ');
bool containsNoBreakSpace = formattedDate.Contains('\u00A0');
bool containsNarrowNoBreakSpace = formattedDate.Contains('\u202F');

Assert.True(containsSpace || containsNoBreakSpace || containsNarrowNoBreakSpace,
$"Formatted date string '{formattedDate}' does not contain any of the specified spaces.");
}
}
}
6 changes: 2 additions & 4 deletions src/mono/browser/runtime/hybrid-globalization/calendar.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import { VoidPtrNull } from "../types/internal";
import { runtimeHelpers } from "./module-exports";
import { Int32Ptr, VoidPtr } from "../types/emscripten";
import { INNER_SEPARATOR, OUTER_SEPARATOR, normalizeSpaces } from "./helpers";
import { INNER_SEPARATOR, OUTER_SEPARATOR } from "./helpers";

const MONTH_CODE = "MMMM";
const YEAR_CODE = "yyyy";
Expand Down Expand Up @@ -96,7 +96,6 @@ function getMonthYearPattern (locale: string | undefined, date: Date): string {
pattern = pattern.replace("999", YEAR_CODE);
// sometimes the number is localized and the above does not have an effect
const yearStr = date.toLocaleDateString(locale, { year: "numeric" });
pattern = normalizeSpaces(pattern);
return pattern.replace(yearStr, YEAR_CODE);
}

Expand Down Expand Up @@ -165,7 +164,7 @@ function getShortDatePattern (locale: string | undefined): string {
const localizedDayCode = dayStr.length == 1 ? "d" : "dd";
pattern = pattern.replace(dayStr, localizedDayCode);
}
return normalizeSpaces(pattern);
return pattern;
}

function getLongDatePattern (locale: string | undefined, date: Date): string {
Expand Down Expand Up @@ -196,7 +195,6 @@ function getLongDatePattern (locale: string | undefined, date: Date): string {
pattern = pattern.replace(replacedWeekday, "dddd");
pattern = pattern.replace("22", DAY_CODE);
const dayStr = date.toLocaleDateString(locale, { day: "numeric" }); // should we replace it for localized digits?
pattern = normalizeSpaces(pattern);
return pattern.replace(dayStr, DAY_CODE);
}

Expand Down
4 changes: 2 additions & 2 deletions src/mono/browser/runtime/hybrid-globalization/culture-info.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import { VoidPtrNull } from "../types/internal";
import { runtimeHelpers } from "./module-exports";
import { Int32Ptr, VoidPtr } from "../types/emscripten";
import { OUTER_SEPARATOR, normalizeLocale, normalizeSpaces } from "./helpers";
import { OUTER_SEPARATOR, normalizeLocale } from "./helpers";

export function mono_wasm_get_culture_info (culture: number, cultureLength: number, dst: number, dstMaxLength: number, dstLength: Int32Ptr): VoidPtr {
try {
Expand Down Expand Up @@ -91,7 +91,7 @@ function getLongTimePattern (locale: string | undefined, designators: any): stri
hourPattern = hasPrefix ? "hh" : "h";
pattern = pattern.replace(hasPrefix ? hour12WithPrefix : localizedHour12, hourPattern);
}
return normalizeSpaces(pattern);
return pattern;
}

function getShortTimePattern (pattern: string): string {
Expand Down
9 changes: 0 additions & 9 deletions src/mono/browser/runtime/hybrid-globalization/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,6 @@ export function normalizeLocale (locale: string | null) {
}
}

export function normalizeSpaces (pattern: string) {
if (!pattern.includes("\u202F"))
return pattern;

// if U+202F present, replace them with spaces
return pattern.replace("\u202F", "\u0020");
}


export function isSurrogate (str: string, startIdx: number): boolean {
return SURROGATE_HIGHER_START <= str[startIdx] &&
str[startIdx] <= SURROGATE_HIGHER_END &&
Expand Down
Loading