Skip to content

Conversation

@ramsey
Copy link
Member

@ramsey ramsey commented Nov 11, 2021

This documentation change supports the php-src pull request at php/php-src#7644

This documentation change supports the php-src pull request at
php/php-src#7644
@cmb69 cmb69 added this to the PHP 8.2 milestone Nov 11, 2021
Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! Besides some minor nits (see below) this looks good to me, but the new method/function also needs entries in reference/intl/versions.xml; otherwise:

Screenshot 2022-12-09 153759

<methodparam><type>string</type><parameter>locale</parameter></methodparam>
</methodsynopsis>
<para>
Gets the language for the input locale
Copy link
Member

Choose a reason for hiding this comment

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

Even though this has been copied, let's stick with basic English punctuation rules :)

Suggested change
Gets the language for the input locale
Gets the language for the input locale.

<term><parameter>locale</parameter></term>
<listitem>
<para>
The locale to extract the language code from
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
The locale to extract the language code from
The locale to extract the language code from.

</methodsynopsis>
<para>
Gets the primary language for the input locale
Gets the language for the input locale
Copy link
Member

Choose a reason for hiding this comment

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

Since this line is modified anyway:

Suggested change
Gets the language for the input locale
Gets the language for the input locale.

</para>
<note>
<para>
<methodname>Locale::getPrimaryLanguage</methodname> is an alias of <methodname>Locale::getLanguage</methodname>.
Copy link
Member

Choose a reason for hiding this comment

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

This is only the case as of PHP 8.2.0, so:

Suggested change
<methodname>Locale::getPrimaryLanguage</methodname> is an alias of <methodname>Locale::getLanguage</methodname>.
As of PHP 8.2.0, <methodname>Locale::getPrimaryLanguage</methodname>
is an alias of <methodname>Locale::getLanguage</methodname>.

<listitem>
<para>
The locale to extract the primary language code from
The locale to extract the language code from
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
The locale to extract the language code from
The locale to extract the language code from.

<para>
&style.oop;
</para>
<methodsynopsis role="oop">
Copy link
Member

Choose a reason for hiding this comment

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

@kocsismate, do we want to fix the role right away?

Copy link
Member

Choose a reason for hiding this comment

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

Yes :)

Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

Ugh, it seems php/php-src#7644 has never been merged.

@afilina
Copy link
Contributor

afilina commented Dec 23, 2022

@ramsey If the php-src PR was closed without merging, is this documentation PR still relevant?

@ramsey
Copy link
Member Author

ramsey commented Dec 23, 2022

This PR can be closed, since the other was closed without merging. Thanks!

@ramsey ramsey closed this Dec 23, 2022
@ramsey ramsey deleted the locale-getLanguage branch December 23, 2022 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants