Skip to content

[iOS][non-icu] HybridGlobalization implement calendar data #90004

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
Merged
mkhamoyan merged 10 commits into from
Aug 8, 2023
Merged

[iOS][non-icu] HybridGlobalization implement calendar data #90004

mkhamoyan merged 10 commits into from
Aug 8, 2023

Conversation

@mkhamoyan
Copy link
Contributor

@mkhamoyan mkhamoyan commented Aug 4, 2023 •
edited
Loading

Implement calendar data for hybrid mode on Apple platforms

Public APIs affected by the PR:

  • DateTimeFormatInfo.AbbreviatedDayNames
  • DateTimeFormatInfo.AbbreviatedMonthGenitiveNames
  • DateTimeFormatInfo.AbbreviatedMonthNames
  • DateTimeFormatInfo.CalendarWeekRule
  • DateTimeFormatInfo.DayNames
  • DateTimeFormatInfo.FirstDayOfWeek
  • DateTimeFormatInfo.FullDateTimePattern
  • DateTimeFormatInfo.LongDatePattern
  • DateTimeFormatInfo.LongTimePattern
  • DateTimeFormatInfo.MonthDayPattern
  • DateTimeFormatInfo.MonthGenitiveNames
  • DateTimeFormatInfo.MonthNames
  • DateTimeFormatInfo.NativeCalendarName
  • DateTimeFormatInfo.ShortDatePattern
  • DateTimeFormatInfo.ShortestDayNames
  • DateTimeFormatInfo.ShortTimePattern
  • DateTimeFormatInfo.YearMonthPattern

Contributes to #80689

cc @SamMonoRT

@ghost ghost assigned mkhamoyan Aug 4, 2023
@ghost
Copy link

ghost commented Aug 4, 2023

Tagging subscribers to this area: @dotnet/area-system-globalization
See info in area-owners.md if you want to be subscribed.

Issue Details

null

Author: mkhamoyan
Assignees: -
Labels:

area-System.Globalization
Milestone: -

@mkhamoyan mkhamoyan changed the title Implement calendar info for hybrid mode [iOS][non-icu] HybridGlobalization implement calendar data Aug 4, 2023
@mkhamoyan
Copy link
Contributor Author

mkhamoyan commented Aug 4, 2023

/azp run runtime-ioslike

@azure-pipelines
Copy link

azure-pipelines bot commented Aug 4, 2023

Azure Pipelines successfully started running 1 pipeline(s).

@mkhamoyan
Copy link
Contributor Author

mkhamoyan commented Aug 4, 2023

/azp run runtime-ioslike

@azure-pipelines
Copy link

azure-pipelines bot commented Aug 4, 2023

Azure Pipelines successfully started running 1 pipeline(s).

@mkhamoyan mkhamoyan requested a review from ilonatommy August 4, 2023 15:09
@mkhamoyan mkhamoyan marked this pull request as ready for review August 4, 2023 15:10
@tarekgh tarekgh added the os-ios Apple iOS label Aug 4, 2023
@ghost
Copy link

ghost commented Aug 4, 2023

Tagging subscribers to 'os-ios': @steveisok, @akoeplinger, @kotlarmilos
See info in area-owners.md if you want to be subscribed.

Issue Details

Implement calendar data for hybrid mode on Apple platforms

Public APIs affected by the PR:

  • DateTimeFormatInfo.AbbreviatedDayNames
  • DateTimeFormatInfo.AbbreviatedMonthGenitiveNames
  • DateTimeFormatInfo.AbbreviatedMonthNames
  • DateTimeFormatInfo.CalendarWeekRule
  • DateTimeFormatInfo.DayNames
  • DateTimeFormatInfo.FirstDayOfWeek
  • DateTimeFormatInfo.FullDateTimePattern
  • DateTimeFormatInfo.LongDatePattern
  • DateTimeFormatInfo.LongTimePattern
  • DateTimeFormatInfo.MonthDayPattern
  • DateTimeFormatInfo.MonthGenitiveNames
  • DateTimeFormatInfo.MonthNames
  • DateTimeFormatInfo.NativeCalendarName
  • DateTimeFormatInfo.ShortDatePattern
  • DateTimeFormatInfo.ShortestDayNames
  • DateTimeFormatInfo.ShortTimePattern
  • DateTimeFormatInfo.YearMonthPattern

Contributes to #80689

cc @SamMonoRT

Author: mkhamoyan
Assignees: mkhamoyan
Labels:

area-System.Globalization, os-ios
Milestone: -

This was referenced Aug 4, 2023
Closed
Closed
akoeplinger
akoeplinger approved these changes Aug 7, 2023
View reviewed changes
mkhamoyan and others added 2 commits August 7, 2023 20:46
@mkhamoyan @akoeplinger
Update src/libraries/System.Globalization.Calendars/tests/Hybrid/Syst…
e7f33f7 
…em.Globalization.Calendars.IOS.Tests.csproj

Co-authored-by: Alexander Köplinger <[email protected]>
@mkhamoyan @akoeplinger
Update docs/design/features/globalization-hybrid-mode.md
81dc180 
Co-authored-by: Alexander Köplinger <[email protected]>
mdh1418
mdh1418 reviewed Aug 7, 2023
View reviewed changes
src/libraries/System.Private.CoreLib/src/System/Globalization/CalendarData.iOS.cs Outdated
Comment on lines 18 to 55
this.sNativeName = GetCalendarInfoNative(localeName, calendarId, CalendarDataType.NativeName);
this.sMonthDay = GetCalendarInfoNative(localeName, calendarId, CalendarDataType.MonthDay);
this.saShortDates = GetCalendarInfoNative(localeName, calendarId, CalendarDataType.ShortDates).Split("||");
this.saLongDates = GetCalendarInfoNative(localeName, calendarId, CalendarDataType.LongDates).Split("||");
this.saYearMonths = GetCalendarInfoNative(localeName, calendarId, CalendarDataType.YearMonths).Split("||");
this.saDayNames = GetCalendarInfoNative(localeName, calendarId, CalendarDataType.DayNames).Split("||");
this.saAbbrevDayNames = GetCalendarInfoNative(localeName, calendarId, CalendarDataType.AbbrevDayNames).Split("||");
this.saSuperShortDayNames = GetCalendarInfoNative(localeName, calendarId, CalendarDataType.SuperShortDayNames).Split("||");

string? leapHebrewMonthName = null;
this.saMonthNames = NormalizeMonthArray(GetCalendarInfoNative(localeName, calendarId, CalendarDataType.MonthNames).Split("||"), calendarId, ref leapHebrewMonthName);
if (leapHebrewMonthName != null)
{
Debug.Assert(this.saMonthNames != null);

// In Hebrew calendar, get the leap month name Adar II and override the non-leap month 7
Debug.Assert(calendarId == CalendarId.HEBREW && saMonthNames.Length == 13);
saLeapYearMonthNames = (string[]) saMonthNames.Clone();
saLeapYearMonthNames[6] = leapHebrewMonthName;

// The returned data has 6th month name as 'Adar I' and 7th month name as 'Adar'
// We need to adjust that in the list used with non-leap year to have 6th month as 'Adar' and 7th month as 'Adar II'
// note that when formatting non-leap year dates, 7th month shouldn't get used at all.
saMonthNames[5] = saMonthNames[6];
saMonthNames[6] = leapHebrewMonthName;

}
this.saAbbrevMonthNames = NormalizeMonthArray(GetCalendarInfoNative(localeName, calendarId, CalendarDataType.AbbrevMonthNames).Split("||"), calendarId, ref leapHebrewMonthName);
this.saMonthGenitiveNames = NormalizeMonthArray(GetCalendarInfoNative(localeName, calendarId, CalendarDataType.MonthGenitiveNames).Split("||"), calendarId, ref leapHebrewMonthName);
this.saAbbrevMonthGenitiveNames = NormalizeMonthArray(GetCalendarInfoNative(localeName, calendarId, CalendarDataType.AbbrevMonthGenitiveNames).Split("||"), calendarId, ref leapHebrewMonthName);

this.saEraNames = NormalizeEraNames(calendarId, GetCalendarInfoNative(localeName, calendarId, CalendarDataType.EraNames).Split("||"));
this.saAbbrevEraNames = Array.Empty<string>();//NormalizeEraNames(calendarId, GetCalendarInfoNative(localeName, calendarId, CalendarDataType.AbbrevEraNames).Split("||"));

return this.sNativeName != null && this.saShortDates != null && this.saLongDates != null && this.saYearMonths != null &&
this.saDayNames != null && this.saAbbrevDayNames != null && this.saSuperShortDayNames != null && this.saMonthNames != null &&
this.saAbbrevMonthNames != null && this.saMonthGenitiveNames != null && this.saAbbrevMonthGenitiveNames != null &&
this.saEraNames != null && this.saAbbrevEraNames != null;
Copy link
Member

@mdh1418 mdh1418 Aug 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this. necessary? https://github.com/dotnet/runtime/blob/main/docs/coding-guidelines/coding-style.md

Copy link
Contributor Author

@mkhamoyan mkhamoyan Aug 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was following same style as in https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.CoreLib/src/System/Globalization/CalendarData.cs#L118 . Will remove as it is not necessary.

@mkhamoyan
Copy link
Contributor Author

mkhamoyan commented Aug 8, 2023

/azp run runtime-ioslike

@mkhamoyan mkhamoyan requested a review from mdh1418 August 8, 2023 12:08
@azure-pipelines
Copy link

azure-pipelines bot commented Aug 8, 2023

Azure Pipelines successfully started running 1 pipeline(s).

@mkhamoyan
Copy link
Contributor Author

mkhamoyan commented Aug 8, 2023

/azp run runtime-ioslike

@azure-pipelines
Copy link

azure-pipelines bot commented Aug 8, 2023

Azure Pipelines successfully started running 1 pipeline(s).

mdh1418
mdh1418 approved these changes Aug 8, 2023
View reviewed changes
Copy link
Member

@mdh1418 mdh1418 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Thanks

@mkhamoyan
Copy link
Contributor Author

mkhamoyan commented Aug 8, 2023

Failures are not related.

@mkhamoyan mkhamoyan merged commit c83324d into dotnet:main Aug 8, 2023
@mkhamoyan mkhamoyan deleted the hybrid_calendar_data branch August 8, 2023 15:53
This was referenced Aug 8, 2023
Closed
Open
Closed
@mkhamoyan mkhamoyan mentioned this pull request Aug 9, 2023
Closed
23 tasks
@ghost ghost locked as resolved and limited conversation to collaborators Sep 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@akoeplinger akoeplinger akoeplinger approved these changes

@mdh1418 mdh1418 mdh1418 approved these changes

@steveisok steveisok Awaiting requested review from steveisok steveisok is a code owner

@ilonatommy ilonatommy Awaiting requested review from ilonatommy

@vargaz vargaz Awaiting requested review from vargaz

@lambdageek lambdageek Awaiting requested review from lambdageek

@SamMonoRT SamMonoRT Awaiting requested review from SamMonoRT

Assignees

@mkhamoyan mkhamoyan

Labels

area-System.Globalization os-ios Apple iOS

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mkhamoyan @akoeplinger @mdh1418 @tarekgh