Merge lp:~widelands-dev/widelands/bug-1798297-locale-C into lp:widelands

Proposed by GunChleoc
Status: Merged
Merged at revision: 8935
Proposed branch: lp:~widelands-dev/widelands/bug-1798297-locale-C
Merge into: lp:widelands
Diff against target: 35 lines (+16/-5)
1 file modified
src/base/i18n.cc (+16/-5)
To merge this branch: bzr merge lp:~widelands-dev/widelands/bug-1798297-locale-C
Reviewer Review Type Date Requested Status
Widelands Developers Pending
Review via email: mp+358364@code.launchpad.net

Commit message

Fix fetching of translations when user locale is set to C. If the desired Widelands locale does not exist, try to fall back to en_US.utf8 to make libintl happy.

To post a comment you must log in.
Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4193. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/452673990.
Appveyor build 3989. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1798297_locale_C-3989.

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4235. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/455848130.
Appveyor build 4031. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1798297_locale_C-4031.

Revision history for this message
Arty (artydent) wrote :

Code LGTM, just some minor indentation irregularities (see comments below).

Not tested because I don't have a working Linux system atm.

Revision history for this message
GunChleoc (gunchleoc) wrote :

I tested this pretty thoroughly on Linux, so it should be fine.

Bunnybot will take care of the indentation.

Thanks for the review! :)

@bunnybot merge

Revision history for this message
bunnybot (widelandsofficial) wrote :

Refusing to merge, since Travis is not green. Use @bunnybot merge force for merging anyways.

Travis build 4235. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/455848130.

Revision history for this message
GunChleoc (gunchleoc) wrote :

Transient failure on Travis

@bunnybot merge force

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/base/i18n.cc'
2--- src/base/i18n.cc 2018-08-12 06:47:11 +0000
3+++ src/base/i18n.cc 2018-11-16 06:30:53 +0000
4@@ -290,15 +290,26 @@
5 found = alt_str.find(',', 0);
6 }
7 if (leave_while) {
8- setenv("LANG", locale.c_str(), 1);
9+ setenv("LC_ALL", locale.c_str(), 1);
10+ setenv("LANG", locale.c_str(), 1);
11 setenv("LANGUAGE", locale.c_str(), 1);
12 } else {
13- log("No corresponding locale found - trying to set it via LANGUAGE=%s, LANG=%s\n",
14- lang.c_str(), lang.c_str());
15+ log("No corresponding locale found\n");
16+ log(" - Set LANGUAGE, LANG and LC_ALL to '%s'\n",
17+ lang.c_str());
18+
19 setenv("LANGUAGE", lang.c_str(), 1);
20 setenv("LANG", lang.c_str(), 1);
21- SETLOCALE(LC_MESSAGES, ""); // set locale according to the env. variables
22- // --> see $ man 3 setlocale
23+ setenv("LC_ALL", lang.c_str(), 1);
24+
25+ try {
26+ SETLOCALE(LC_MESSAGES, "en_US.utf8"); // set locale according to the env. variables
27+ // --> see $ man 3 setlocale
28+ log(" - Set system locale to 'en_US.utf8' to make '%s' accessible to libintl\n", lang.c_str());
29+ } catch (std::exception&) {
30+ SETLOCALE(LC_MESSAGES, ""); // set locale according to the env. variables
31+ // --> see $ man 3 setlocale
32+ }
33 // assume that it worked
34 // maybe, do another check with the return value (?)
35 locale = lang;

Subscribers

People subscribed via source and target branches

to status/vote changes: