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

Error detecting locale with an empty LC_ALL #28

Closed
pasabanov opened this issue Aug 25, 2024 · 7 comments · Fixed by #29
Closed

Error detecting locale with an empty LC_ALL #28

pasabanov opened this issue Aug 25, 2024 · 7 comments · Fixed by #29

Comments

@pasabanov
Copy link
Contributor

Current behavior

If LC_ALL is empty, the function sys_locale::get_locale() returns a locale from an empty string. This occurs because the _get function does not check whether LC_ALL is empty or not:

fn _get(env: &impl EnvAccess) -> Option<String> {
    let code = env
        .get(LC_ALL)
        .or_else(|| env.get(LC_CTYPE))
        .or_else(|| env.get(LANG))?;

    parse_locale_code(&code)
}

Expected behavior

I expect the get_locale function to continue evaluating the locale using other variables like LANG and LANGUAGE if LC_ALL is empty.

@complexspaces
Copy link
Collaborator

Hey there, thank you for the bug report. I've fixed the oversight in #29. Would you mind trying the latest commit on main in your application and confirm the issue is gone? If so, I can make a new patch release soon.

@pasabanov
Copy link
Contributor Author

@complexspaces Thank you for the fix! I’ve tested the latest commit on the main branch and the issue is resolved.

Also, could you clarify why the LANGUAGE variable is not used as one of the fallback options?

@pasabanov
Copy link
Contributor Author

Also, the LANGUAGE variable contains a list of locales in descending order of preference, so it would be reasonable to use it in the get_locales() function, which currently returns only one locale.

@complexspaces
Copy link
Collaborator

IIUC LANGUAGE seems to be something that gettext handles but not something glibc looks for when determining what the current locale is? I don't believe there's a strong reason to not support it, its just not something we seem to have run into yet.

@complexspaces
Copy link
Collaborator

Also, the LANGUAGE variable contains a list of locales in descending order of preference

Do you happen to know if any desktop environments support setting this variable for spawned apps? When I last looked KDE seemed to be the only desktop environment that supported multiple, ordered locales but they were just in a config file.

@pasabanov
Copy link
Contributor Author

I’m not sure if any desktop environments set the LANGUAGE variable for spawned applications. However, I found some relevant sources that might help clarify the role of the LANGUAGE variable:

  1. Debian mailing list discussion where it is mentioned that LANGUAGE primarily affects the locale for message translation and takes precedence over other variables, even LC_ALL.
  2. Baeldung article on Linux locale environment variables, which also states that LANGUAGE has priority over other locale-related variables. But it might not be entirely accurate, as this should only be true in the context of message localization, which is what the article discusses.

Ideally, it would be useful to implement separate functions for determining the general locale and the message locale (and, ideally, for each specific LC_*** category). However, if we want to keep the locale determination process simple, one approach could be to give LANGUAGE priority over LC_ALL when determining the overall locale, because the locale is mainly used for message translation.

Additionally, if we aim to provide the user with a list of locales in descending order of preference (with the get_locales() function), LANGUAGE is well-suited for this purpose, as it literally stores such a list. If we want to go further, we could append the locale from LC_ALL, followed by those from LANG, to the list from LANGUAGE. We might consider excluding LC_CTYPE since it is more focused on specifying character encoding rather than language.

@pasabanov
Copy link
Contributor Author

pasabanov commented Aug 26, 2024

Also, as in electron/shell/common/language_util_linux.cc and in chromium/src/l10n/l10n_util.cc (links from this issue), I think we should use the LC_MESSAGES variable in locale evaluation: the precedence order: LANGUAGE, LC_ALL, LC_MESSAGES and LANG.

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 a pull request may close this issue.

2 participants