feat: add app.getPreferredSystemLanguages() API #36035

rzhao271 · 2022-10-14T22:46:38Z

Description of Change

This PR provides an API that retrieves the user's preferred system languages.

It turns out that retrieving the user's locale often ends up retrieving "actual locale" data, which isn't what I originally wanted with PR #35697. However, upon discussing with @deepak1556, returning the "actual locale" data makes sense for a method named app.getSystemLocale, so this PR adds a new method, app.getPreferredSytemLanguages, that returns GetPreferredLanguages().

Downstream: microsoft/vscode#159958.

Checklist

PR description included and stakeholders cc'd
npm test passes
tests are added (https://github.com/electron/electron/blob/main/docs/development/testing.md)
relevant documentation is changed or added
PR release notes describe the change in a way relevant to app developers, and are capitalized, punctuated, and past tense.
Manually test the change on Windows with the region set to a different value than the most preferred system language
Manually test the change on macOS with the region set to a different value than the most preferred system language

Release Notes

Notes: Added an app.getPreferredSystemLanguages() API to return the user's system languages.

rzhao271 · 2022-10-14T22:49:24Z

Windows 11 screenshots

A screenshot showing that the strings returned by app.getSystemLanguage(), app.getSystemLocale(), and app.getLocale() can all be different. Only the last one is affected by the --lang flag.

A screenshot showing that French (Canada) is the preferred language, whereas Chinese (Simplified, China) is the region. app.getSystemLocale() returns zh-CN whereas app.getSystemLanguage() returns fr-CA.

rzhao271 · 2022-10-15T00:01:01Z

macOS Monterey screenshots

A screenshot showing that the strings returned by app.getLocale(), app.getSystemLocale(), and app.getSystemLanguage() are different.

Canadian French (fr-CA) is the preferred language, whereas Finland (FI) is the region, and Swiss German (de-CH) is the language passed in through the --lang flag. app.getLocale() returns en-US, app.getSystemLocale() returns fr-FI, and app.getSystemLanguage() returns fr-CA.

I didn't expect app.getLocale() to return en-US, but I guess de-CH isn't a registered locale. If I pass in just German (de) through the --lang flag, then app.getLocale() returns de.

rzhao271 · 2022-10-17T16:29:15Z

Sample results on Windows 11

app.getPreferredSystemLanguages() fr-CA,en-US,fa,zh-Hans-CN,ko,ja,pt-PT,pt-BR
app.getSystemLocale() zh-CN
app.getLocale() fr

deepak1556 · 2022-10-18T00:25:12Z

I didn't expect app.getLocale() to return en-US, but I guess de-CH isn't a registered locale. If I pass in just German (de) through the --lang flag, then app.getLocale() returns de.

Yes this is expected, it depends on what Cocoa locale the application supports. It is the list of *.lproj files shipped with the application.

docs/api/app.md

deepak1556

Linux implementation can return the list from g_get_language_names instead of empty result. NOTE: l10n_util::GetApplicationLocaleInternal also uses it but we cannot call into this base API because it eventually returns the locale for which translatable strings are available in the application.

docs/api/app.md

shell/common/language_util_linux.cc

docs/api/app.md

rzhao271 · 2022-10-20T17:22:43Z

I wasn't able to add tables as examples due to the error message

× npm run lint:docs:
-
- Checking argv
× Generating API in directory: "C:\Users\raymondzhao\work\electron\src\electron"
An error occurred while processing: "C:\Users\raymondzhao\work\electron\src\electron\docs\api\app.md"  
AssertionError: We only support plain text, links, softbreaks, inline code, strong tags and paragraphs inside joinable tokens: expected 'table_open' to be one of [ Array(22) ]
    at Object.exports.safelyJoinTokens (C:\Users\raymondzhao\work\electron\src\electron\node_modules\@electron\docs-parser\dist\markdown-helpers.js:386:48)
    at Object.exports.extractReturnType (C:\Users\raymondzhao\work\electron\src\electron\node_modules\@electron\docs-parser\dist\markdown-helpers.js:329:36)
    at Object.exports._headingToMethodBlock (C:\Users\raymondzhao\work\electron\src\electron\node_modules\@electron\docs-parser\dist\block-parsers.js:81:72)
> electron-docs-parser --dir=./

I have added nested lists for now.

deepak1556

Getting close! Implementation LGTM 👍

spec/api-app-spec.ts

zcbenz

API LGTM

shell/browser/api/electron_api_app.cc

zcbenz · 2022-10-27T07:21:15Z

We need a few more API LGTMs from @electron/wg-api.

codebytere

(sorry x3)

codebytere

x4

codebytere · 2022-11-08T23:05:53Z

x5

codebytere

x6

codebytere

x7

codebytere

x8

codebytere

x9

codebytere

x10

release-clerk · 2022-11-09T15:50:47Z

Release Notes Persisted

Added an app.getPreferredSystemLanguages() API to return the user's system languages.

* feat: add app.getSystemLanguage() API * Change the API to getPreferredSystemLanguages * Fix test * Clarify docs and add Linux impl * Remove USE_GLIB * Don't add C to list * Remove examples since there's a lot of edge cases * Fix lint * Add examples * Fix compile error * Apply PR feedback * Update the example Co-authored-by: Raymond Zhao <7199958+rzhao271@users.noreply.github.com>

trop · 2022-11-09T15:51:20Z

I have automatically backported this PR to "21-x-y", please check out #36290

trop · 2022-11-09T15:51:56Z

I have automatically backported this PR to "22-x-y", please check out #36291

feat: add app.getPreferredSystemLanguages() API (#36035) * feat: add app.getSystemLanguage() API * Change the API to getPreferredSystemLanguages * Fix test * Clarify docs and add Linux impl * Remove USE_GLIB * Don't add C to list * Remove examples since there's a lot of edge cases * Fix lint * Add examples * Fix compile error * Apply PR feedback * Update the example Co-authored-by: Raymond Zhao <7199958+rzhao271@users.noreply.github.com> Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: Raymond Zhao <7199958+rzhao271@users.noreply.github.com>

* feat: add app.getSystemLanguage() API * Change the API to getPreferredSystemLanguages * Fix test * Clarify docs and add Linux impl * Remove USE_GLIB * Don't add C to list * Remove examples since there's a lot of edge cases * Fix lint * Add examples * Fix compile error * Apply PR feedback * Update the example

electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Oct 14, 2022

rzhao271 changed the title ~~feat: add app.getSystemLanguage() API~~ feat: add app.getPreferredSystemLanguages() API Oct 17, 2022

rzhao271 marked this pull request as ready for review October 18, 2022 00:06

deepak1556 reviewed Oct 18, 2022

View reviewed changes

docs/api/app.md Outdated Show resolved Hide resolved

deepak1556 requested changes Oct 18, 2022

View reviewed changes

deepak1556 added api-review/requested 🗳 target/21-x-y PR should also be added to the "21-x-y" branch. target/22-x-y PR should also be added to the "22-x-y" branch. semver/minor backwards-compatible functionality labels Oct 18, 2022

zcbenz reviewed Oct 18, 2022

View reviewed changes

docs/api/app.md Outdated Show resolved Hide resolved

deepak1556 reviewed Oct 19, 2022

View reviewed changes

shell/common/language_util_linux.cc Outdated Show resolved Hide resolved

docs/api/app.md Outdated Show resolved Hide resolved

docs/api/app.md Outdated Show resolved Hide resolved

docs/api/app.md Outdated Show resolved Hide resolved

deepak1556 reviewed Oct 24, 2022

View reviewed changes

spec/api-app-spec.ts Outdated Show resolved Hide resolved

zcbenz approved these changes Oct 24, 2022

View reviewed changes

shell/browser/api/electron_api_app.cc Outdated Show resolved Hide resolved

deepak1556 approved these changes Oct 25, 2022

View reviewed changes

rzhao271 added 10 commits October 27, 2022 16:21

feat: add app.getSystemLanguage() API

f8aad8e

Change the API to getPreferredSystemLanguages

9ec31bc

Fix test

c1fd204

Clarify docs and add Linux impl

f8b2c2c

Remove USE_GLIB

eeb64f0

Don't add C to list

95efefa

Remove examples since there's a lot of edge cases

cf1f280

Fix lint

63e558b

Add examples

c850100

Fix compile error

68ebd1e

codebytere reviewed Nov 8, 2022

View reviewed changes

electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Nov 9, 2022

codebytere reviewed Nov 9, 2022

View reviewed changes

electron-cation bot added api-review/approved ✅ and removed api-review/requested 🗳 labels Nov 9, 2022

jkleinsc merged commit 5fc3ed9 into electron:main Nov 9, 2022

trop bot mentioned this pull request Nov 9, 2022

feat: add app.getPreferredSystemLanguages() API #36290

Merged

trop bot added in-flight/21-x-y and removed target/21-x-y PR should also be added to the "21-x-y" branch. labels Nov 9, 2022

trop bot mentioned this pull request Nov 9, 2022

feat: add app.getPreferredSystemLanguages() API #36291

Merged

trop bot added in-flight/22-x-y and removed target/22-x-y PR should also be added to the "22-x-y" branch. labels Nov 9, 2022

rzhao271 deleted the rzhao271/system-languages branch November 9, 2022 16:03

trop bot added merged/22-x-y PR was merged to the "22-x-y" branch. merged/21-x-y PR was merged to the "21-x-y" branch. and removed in-flight/22-x-y labels Nov 9, 2022

trop bot removed the in-flight/21-x-y label Nov 9, 2022

commit-lint bot mentioned this pull request Feb 23, 2023

[pull] main from electron:main Blaquez-home/electron#3

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add app.getPreferredSystemLanguages() API #36035

feat: add app.getPreferredSystemLanguages() API #36035

feat: add app.getPreferredSystemLanguages() API #36035

feat: add app.getPreferredSystemLanguages() API #36035

Conversation

Description of Change

Checklist

Release Notes

Windows 11 screenshots

macOS Monterey screenshots

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment