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
@@ -60,6 +60,7 @@
#endif

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

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

std::string App::GetLocaleCountryCode() {
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);
This conversation was marked as resolved by zarubond

This comment has been minimized.

Copy link
@miniak

miniak Nov 4, 2018

Contributor

even though not mandatory, please put the body in {} block, it's a bit confusing now considering the condition is 6 lines of code

#elif defined(OS_MACOSX)
CFLocaleRef locale = CFLocaleCopyCurrent();
CFStringRef value = CFStringRef(
static_cast<CFTypeRef>(CFLocaleGetValue(locale, kCFLocaleCountryCode)));
const CFIndex kCStringSize = 128;
char temporaryCString[kCStringSize] = {0};
CFStringGetCString(value, temporaryCString, kCStringSize,
kCFStringEncodingUTF8);
region = temporaryCString;
#else
const char* locale_ptr = setlocale(LC_TIME, NULL);
if (locale_ptr == NULL)
This conversation was marked as resolved by zarubond

This comment has been minimized.

Copy link
@miniak

miniak Nov 4, 2018

Contributor

if (!locale_ptr)

locale_ptr = setlocale(LC_NUMERIC, NULL);
if (locale_ptr) {
std::string locale = locale_ptr;
std::string::size_type rpos = locale.find('.');
if (rpos != std::string::npos)
locale = locale.substr(0, rpos);
rpos = locale.find('_');
if (rpos != std::string::npos && rpos + 1 < locale.size())
region = locale.substr(rpos + 1);
}
#endif
return region.size() == 2 ? region : std::string();
}

void App::OnSecondInstance(const base::CommandLine::StringVector& cmd,
const base::FilePath& cwd) {
Emit("second-instance", cmd, cwd);
@@ -1299,7 +1338,8 @@ void App::BuildPrototype(v8::Isolate* isolate,
.SetMethod("setPath", &App::SetPath)
.SetMethod("getPath", &App::GetPath)
.SetMethod("setDesktopName", &App::SetDesktopName)
.SetMethod("getLocale", &App::GetLocale)
.SetMethod("getRegion", &App::GetRegion)

This comment has been minimized.

Copy link
@codebytere

codebytere Oct 12, 2018

Member

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

.SetMethod("getLocaleCountryCode", &App::GetLocaleCountryCode)
#if defined(USE_NSS_CERTS)
.SetMethod("importCertificate", &App::ImportCertificate)
#endif
@@ -182,6 +182,7 @@ class App : public AtomBrowserClient::Delegate,

void SetDesktopName(const std::string& desktop_name);
std::string GetLocale();
std::string GetLocaleCountryCode();
void OnSecondInstance(const base::CommandLine::StringVector& cmd,
const base::FilePath& cwd);
bool HasSingleInstanceLock() const;
@@ -580,6 +580,11 @@ 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.GetLocaleCountryCode()`

This comment has been minimized.

Copy link
@HashimotoYT

HashimotoYT Oct 16, 2018

Member

app.GetLocaleCountryCode() => app.getLocaleCountryCode()

Returns `String` - User operating system´s region in ISO3166 [here](https://www.iso.org/iso-3166-country-codes.html). The value is taken from native OS APIs.

This comment has been minimized.

Copy link
@codebytere

codebytere Oct 12, 2018

Member

region => locale country code


**Note:** When unable to detect region, it returns empty string.

This comment has been minimized.

Copy link
@codebytere

codebytere Oct 12, 2018

Member

same as above ☝️


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

* `path` String
@@ -122,6 +122,12 @@ describe('app module', () => {
})
})

describe('app.getLocaleCountryCode()', () => {
it('should be empty or have length of two', () => {
expect(app.getLocaleCountryCode()).to.have.lengthOf.oneOf([0, 2])
})
})

describe('app.isPackaged', () => {
it('should be false durings tests', () => {
expect(app.isPackaged).to.be.false()
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.