Skip to content

GH-12943 ext/intl accept C as acceptable locale argument. #12955

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

Closed
wants to merge 1 commit into from

Conversation

devnexen
Copy link
Member

No description provided.

@SakiTakamachi
Copy link
Member

I don't have approval authority, but it looks good.

Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me,not sure if we should add for a backport to 8.1 considering that this bug was introduced late in the release cycle, @ramsey opinions?

Comment on lines +8 to +12
$fmt = new IntlDateFormatter(
'C',
IntlDateFormatter::FULL,
IntlDateFormatter::FULL,
);
Copy link
Member

Choose a reason for hiding this comment

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

Nit: CS

Suggested change
$fmt = new IntlDateFormatter(
'C',
IntlDateFormatter::FULL,
IntlDateFormatter::FULL,
);
$fmt = new IntlDateFormatter(
'C',
IntlDateFormatter::FULL,
IntlDateFormatter::FULL,
);

@@ -112,7 +112,7 @@ static zend_result datefmt_ctor(INTERNAL_FUNCTION_PARAMETERS, zend_error_handlin
}
locale = Locale::createFromName(locale_str);
/* get*Name accessors being set does not preclude being bogus */
if (locale.isBogus() || strlen(locale.getISO3Language()) == 0) {
if (locale.isBogus() || (strcmp("C", locale_str) && strlen(locale.getISO3Language()) == 0)) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (locale.isBogus() || (strcmp("C", locale_str) && strlen(locale.getISO3Language()) == 0)) {
if (locale.isBogus() || (locale_str == 1 && locale_str[0] == 'C') && strlen(locale.getISO3Language()) == 0)) {

The call to strcmp() is not needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess you meant locale_len == 1

Copy link
Member

Choose a reason for hiding this comment

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

Yes indeed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants