← Back to team overview

widelands-dev team mailing list archive

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

 

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

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

Diff comments:

> === modified file 'src/base/i18n.cc'
> --- src/base/i18n.cc	2018-08-12 06:47:11 +0000
> +++ src/base/i18n.cc	2018-11-16 06:30:53 +0000
> @@ -290,15 +290,26 @@
>  		found = alt_str.find(',', 0);
>  	}
>  	if (leave_while) {
> -		setenv("LANG", locale.c_str(), 1);
> +		setenv("LC_ALL", locale.c_str(), 1);
> +        setenv("LANG", locale.c_str(), 1);

indent should be tabs instead of spaces

>  		setenv("LANGUAGE", locale.c_str(), 1);
>  	} else {
> -		log("No corresponding locale found - trying to set it via LANGUAGE=%s, LANG=%s\n",
> -		    lang.c_str(), lang.c_str());
> +		log("No corresponding locale found\n");
> +        log(" - Set LANGUAGE, LANG and LC_ALL to '%s'\n",

indent again

> +		    lang.c_str());
> +
>  		setenv("LANGUAGE", lang.c_str(), 1);
>  		setenv("LANG", lang.c_str(), 1);
> -		SETLOCALE(LC_MESSAGES, "");  // set locale according to the env. variables
> -		                             // --> see  $ man 3 setlocale
> +        setenv("LC_ALL", lang.c_str(), 1);
> +
> +        try {
> +            SETLOCALE(LC_MESSAGES, "en_US.utf8");  // set locale according to the env. variables
> +                                         // --> see  $ man 3 setlocale
> +            log(" - Set system locale to 'en_US.utf8' to make '%s' accessible to libintl\n", lang.c_str());
> +        } catch (std::exception&) {
> +            SETLOCALE(LC_MESSAGES, "");  // set locale according to the env. variables
> +                                         // --> see  $ man 3 setlocale
> +        }
>  		// assume that it worked

indent again (in all 10 lines above)

>  		// maybe, do another check with the return value (?)
>  		locale = lang;


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1798297-locale-C/+merge/358364
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1798297-locale-C into lp:widelands.


References