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

feat: Add app.getLocaleCountryCode() method for region detection #15035

Merged
merged 18 commits into from Nov 20, 2018
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
46 changes: 46 additions & 0 deletions atom/browser/api/atom_api_app.cc
Expand Up @@ -60,6 +60,7 @@
#endif

#if defined(OS_MACOSX)
#include <CoreFoundation/CoreFoundation.h>
#include "atom/browser/ui/cocoa/atom_bundle_mover.h"
#endif

Expand Down Expand Up @@ -874,6 +875,50 @@ std::string App::GetLocale() {
return g_browser_process->GetApplicationLocale();
}

std::string App::GetRegion() {
std::string region;
#if defined(OS_WIN)
WCHAR locale_name[LOCALE_NAME_MAX_LENGTH] = {0};

if (GetLocaleInfoEx(LOCALE_NAME_USER_DEFAULT, LOCALE_SISO3166CTRYNAME,
(LPWSTR)&locale_name,
sizeof(locale_name) / sizeof(WCHAR)) ||
GetLocaleInfoEx(LOCALE_NAME_SYSTEM_DEFAULT, LOCALE_SISO3166CTRYNAME,
(LPWSTR)&locale_name,
sizeof(locale_name) / sizeof(WCHAR)))
base::WideToUTF8(locale_name, wcslen(locale_name), &region);
zarubond marked this conversation as resolved.
Show resolved Hide resolved
#elif defined(OS_MACOSX)
CFLocaleRef locale = CFLocaleCopyCurrent();
CFStringRef value = CFStringRef(
static_cast<CFTypeRef>(CFLocaleGetValue(locale, kCFLocaleCountryCode)));
const CFIndex kCStringSize = 128;
char temporaryCString[kCStringSize];
bzero(temporaryCString, kCStringSize);
Copy link
Member

Choose a reason for hiding this comment

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

bzero() is legacy; prefer memset()

Copy link
Contributor

Choose a reason for hiding this comment

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

or just char temporaryCString[kCStringSize] = {}

CFStringGetCString(value, temporaryCString, kCStringSize,
kCFStringEncodingUTF8);
region = temporaryCString;
#else
const char* locale_ptr = setlocale(LC_TIME, NULL);
if (locale_ptr == NULL)
zarubond marked this conversation as resolved.
Show resolved Hide resolved
locale_ptr = setlocale(LC_NUMERIC, NULL);

if (locale_ptr) {
std::string locale = locale_ptr;
std::string::size_type rpos = locale.rfind('.');
if (rpos != std::string::npos)
locale = locale.substr(0, rpos);

rpos = locale.rfind('_');
if (rpos != std::string::npos && rpos + 1 < locale.size())
region = locale.substr(rpos + 1, 2);
}
#endif
if (region.size() == 2)
Copy link
Member

Choose a reason for hiding this comment

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

nit: this is probably cleaner as a ternary:
return region.size() == 2 ? region : std::string();

return region;

return std::string();
}

void App::OnSecondInstance(const base::CommandLine::StringVector& cmd,
const base::FilePath& cwd) {
Emit("second-instance", cmd, cwd);
Expand Down Expand Up @@ -1300,6 +1345,7 @@ void App::BuildPrototype(v8::Isolate* isolate,
.SetMethod("getPath", &App::GetPath)
.SetMethod("setDesktopName", &App::SetDesktopName)
.SetMethod("getLocale", &App::GetLocale)
.SetMethod("getRegion", &App::GetRegion)
Copy link
Member

Choose a reason for hiding this comment

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

getRegion should be removed now i believe since you changed the name

#if defined(USE_NSS_CERTS)
.SetMethod("importCertificate", &App::ImportCertificate)
#endif
Expand Down
1 change: 1 addition & 0 deletions atom/browser/api/atom_api_app.h
Expand Up @@ -182,6 +182,7 @@ class App : public AtomBrowserClient::Delegate,

void SetDesktopName(const std::string& desktop_name);
std::string GetLocale();
std::string GetRegion();
void OnSecondInstance(const base::CommandLine::StringVector& cmd,
const base::FilePath& cwd);
bool HasSingleInstanceLock() const;
Expand Down
4 changes: 4 additions & 0 deletions docs/api/app.md
Expand Up @@ -580,6 +580,10 @@ To set the locale, you'll want to use a command line switch at app startup, whic

**Note:** On Windows you have to call it after the `ready` events gets emitted.

### `app.getRegion()` _macOS_ _Linux_ _Windows_
Copy link
Member

Choose a reason for hiding this comment

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

For functions that work across platforms, no need to specify all of _macOS_ _Linux_ _Windows_

Returns `String` - User operating system region in ISO3166 [here](https://www.iso.org/iso-3166-country-codes.html). The value is taken from OS apis.
**Note:** When unable to detect region, it returns empty string.
Copy link
Member

Choose a reason for hiding this comment

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

same as above ☝️


### `app.addRecentDocument(path)` _macOS_ _Windows_

* `path` String
Expand Down