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.

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

gh-123853: Update locale.windows_locale #123901

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Sep 10, 2024

Update the table of Windows language code identifiers (LCIDs) to protocol version 16.0 (4/23/2024).

Update the table of Windows language code identifiers (LCIDs) to
protocol version 16.0 (4/23/2024).
@@ -0,0 +1,3 @@
Update the table of Windows language code identifiers (LCIDs) used by
:func:`locale.getdefaultlocale()` on Windows to protocol version 16.0
(4/23/2024).
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
(4/23/2024).
(2024-04-23).

Copy link
Member Author

Choose a reason for hiding this comment

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

This is how it is indicated in the original document.

Copy link
Member

Choose a reason for hiding this comment

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

I understand but I dislike this date format, it's misleading. I prefer ISO 8601 date ;-)

@@ -1467,10 +1467,15 @@ def getpreferredencoding(do_setlocale=True):
#

windows_locale = {
Copy link
Member

Choose a reason for hiding this comment

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

Would it be possible to call GetLocaleInfo() instead of using a hardcoded dictionary?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is called. The dictionary is used if GetLocaleInfo fails to provide such information (on Windows 95, etc). That is, almost never.

And if we use GetLocaleInfo, windows_locale is not needed.

There is also better modern API than GetLocaleInfo, but since _getdefaultlocale() is deprecated anyway...

Copy link
Member

Choose a reason for hiding this comment

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

It is called. The dictionary is used if GetLocaleInfo fails to provide such information (on Windows 95, etc)

We don't support Windows 95 anymore. Do you mean that the dict is no longer needed?

And if we use GetLocaleInfo, windows_locale is not needed.

I don't understand. getdefaultlocale() calls GetLocaleInfo() on Windows.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I think that it is no longer needed. GetLocaleInfoA supports LOCALE_SISO639LANGNAME and LOCALE_SISO3166CTRYNAME on all modern Windows. It only returns a LCID if:

    /* If we end up here, this windows version didn't know about
       ISO639/ISO3166 names (it's probably Windows 95).  Return the
       Windows language identifier instead (a hexadecimal number) */

And only in this case the dict is used.

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

I'm not convinced that this change is needed / useful, but it looks correct to me.

I suggest to not backport this change, since it can change the behavior on programs relying the exact values.

# http://msdn.microsoft.com/library/default.asp?url=/library/en-us/intl/nls_238z.asp
# to include every locale up to Windows Vista.
# https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-lcid/70feba9f-294e-491e-b6eb-56532684c37f
# to include every locale up to protocol revision 16.0 (4/23/2024).
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
# to include every locale up to protocol revision 16.0 (4/23/2024).
# to include every locale up to protocol revision 16.0 (2024-04-23).

@serhiy-storchaka
Copy link
Member Author

I only created this PR because you suggested to do this. This is actually a simplified version, it does not include names for reserved codes and ignores all parts of the name besides language and country.

And it is completely useless.

@serhiy-storchaka serhiy-storchaka changed the title gh-123853: Fix locale.getdefaultlocale() on Windows gh-123853: Update locale.windows_locale Sep 11, 2024
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.

2 participants