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

days of the week not localized #1709

Closed
sevu opened this issue May 19, 2017 · 41 comments
Closed

days of the week not localized #1709

sevu opened this issue May 19, 2017 · 41 comments
Assignees
Labels
Bug Issues involving unexpected behavior. Confirmed Issues that have been successfully reproduced by at least one developer. Regression Issues that were not present in previous releases. Translations Issues with translations, translation tooling, the translations engine, or code that uses it.

Comments

@sevu
Copy link
Member

sevu commented May 19, 2017

When you want to load a saved game, there is on the right the date shown.
If it is a more or less recent save, then the date is shown with the weekday.
They aren't translated any more since 1.13.7.

I think @Vultraz has some more information about it.

Edit: same goes for months

@CelticMinstrel CelticMinstrel added Bug Issues involving unexpected behavior. Translations Issues with translations, translation tooling, the translations engine, or code that uses it. labels May 19, 2017
@gfgtdf
Copy link
Contributor

gfgtdf commented May 19, 2017

i cannot reproduce this one in 1.13.7

@sevu
Copy link
Member Author

sevu commented May 19, 2017

make a new save and wait two days

@gfgtdf
Copy link
Contributor

gfgtdf commented May 19, 2017

make a new save and wait two days

that's what i did (well i took an old save and adjusted the system time to be 2 days after it.)

screenshot in german language:

grafik

@Wedge009
Copy link
Member

Seems okay to me with French and 1.13.8+dev. Which language are you using?

@sevu
Copy link
Member Author

sevu commented May 20, 2017

I do have this problem with 1.13.7 and 1.13.8+dev.
And like gfgtdt I'm using German, but on Archlinux, built with scons and gcc.
The translations for the days aren't in the po-files, they must come from elsewhere. Maybe I miss the dependency for it?

@gfgtdf
Copy link
Contributor

gfgtdf commented May 20, 2017

The translations for the days aren't in the po-files, they must come from elsewhere. Maybe I miss the dependency for it?

wesnoth current uses boost::locale to translate these strings. boost::locale might use differnt beckend depending on how it it compiled. (you can for example compile boost with icu backend). the 1.13.7 release whch was using above seems to be using a winapi backend. if you start wesnoth with '--log-debug=general' it'll show you what backend it uses.

@sevu
Copy link
Member Author

sevu commented May 20, 2017

it shows me this:

20170520 23:52:00 info general: Found boost locale backend: 'icu'
20170520 23:52:00 info general: Found boost locale backend: 'posix'
20170520 23:52:00 info general: Found boost locale backend: 'std'

full log: https://pastebin.com/zQa69Y76
doesn't really differ from 1.12 https://pastebin.com/NZzUNekJ

@gfgtdf
Copy link
Contributor

gfgtdf commented May 20, 2017

oh it might also depend on whther this code https://github.com/wesnoth/wesnoth/blob/1.13.8/src/gettext_boost.cpp#L440 decides to use std::put_time. You coudl try to replace that #if HAVE_PUT_TIME with a #if 0 or #if 1 and see whethe rit works then.

@sevu
Copy link
Member Author

sevu commented May 20, 2017

Setting it to 0 works

@gfgtdf
Copy link
Contributor

gfgtdf commented May 20, 2017

I did some invetigating, and loonycyborg said the wesnoth build im using uses gcc 5.1 on which HAVE_PUT_TIME would be true, so there migth be something else involved that makes it not work with HAVE_PUT_TIME .

@sevu
Copy link
Member Author

sevu commented May 21, 2017

I tried gcc 5.4, 6.3 and clang, can look if cmake makes a difference tomorrow.
Who knows, maybe I miss sth. on the system on which put_time relies, what could that be?
Looking at the logfile above, at some places locale C appears - is that fine?

Did someone already try a linux build?

@CelticMinstrel
Copy link
Member

The presence of the C locale in that log strikes me as weird. I'm not sure if it could be the cause though; I'd need to look around to see where all these log messages are generated.

@Wedge009
Copy link
Member

I had been using a utility to set dates on files and retesting today, I find some puzzling results:

Windows build using MSVC 2017 and boost 1.64:

  • French: dd mmm, with month in French.
  • English GB: mmm dd, with month in English - but I would consider this incorrectly using American date format.
  • English US: mmm dd, with month in English.

Linux build using scons and boost 1.62:

  • French: dd mmm, with month in English.
  • English GB: mmm dd, with month in English.
  • English US: mmm dd, with month in English.

Both builds were recent (within last day or two) master revisions. For some reason, I cannot get the day of the week to show in the load game screen today, in either Windows or Linux.

For Linux, I also get this in stderr:
warning general: setlocale() failed for 'fr_FR'

@sevu sevu added the Regression Issues that were not present in previous releases. label Jan 24, 2018
@Vultraz
Copy link
Member

Vultraz commented Feb 21, 2018

Is this still an issue? While testing I've noticed the date translated correctly in French on Windows with an MSVC 2017 build.

@Wedge009
Copy link
Member

I can see both day-of-week and month translated in my Windows build, but not in my Linux build (bf8c1b9). I have boost 1.66 in Windows, but still only 1.62 in Linux. I don't know if the differentiating factor is the OS, the boost version, or something else entirely.

@Vultraz
Copy link
Member

Vultraz commented Feb 21, 2018

Do you have translations built for the Linux build? And if not, can you try after renaming your po/ folder to translations/?

@Wedge009
Copy link
Member

Yes, translations are built - Linux SCons script builds translations first. I can see everything else translated, but looking at the Load Game dialogue, the dates are still in English.

I can't really rename my po directory to translations because there's already a translations directory present. The translations directory has the mo files.

@CelticMinstrel
Copy link
Member

CelticMinstrel commented Feb 22, 2018

I wouldn't expect this issue to have anything to do with whether translations are built? The translated month and day names are more likely built into the date formatter somehow.

@Wedge009
Copy link
Member

Official Windows 1.14.7 release with boost 1.66:

  • Today's files: HH:MM
  • Less than one week old: ddd HH:MM with day translated
  • One week or older, within current year: dd mmm for French with month translated, mmm dd for English even for GB
  • Before current year: dd mmm yy for French with month translated, mmm dd yy for English even for GB

Linux 1.14.8+dev with boost 1.67:

  • Today's files: HH:MM
  • Less than one week old: ddd HH:MM with day in English even when French selected
  • One week or older, within current year: dd mmm for French with month translated, mmm dd for English even for GB
  • Before current year: dd mmm yy for French with month translated, mmm dd yy for English even for GB

So it looks like the issue is still around...

@sevu sevu added the Confirmed Issues that have been successfully reproduced by at least one developer. label Aug 23, 2019
@gfgtdf
Copy link
Contributor

gfgtdf commented Aug 23, 2019

can someone test with wesnoth 1.15.1 on windows?

@Wedge009
Copy link
Member

Wedge009 commented Aug 23, 2019

Behaviour in 1.15.1+dev is the same for both Linux and Windows. I mean same as what I described above, not same as each other.

@gfgtdf
Copy link
Contributor

gfgtdf commented Aug 23, 2019

and i assume replacing this line

#if HAVE_PUT_TIME

with an #if 0 still fixes it on linux 1.14 ?

@sevu
Copy link
Member Author

sevu commented Aug 24, 2019

yes, this fixes it.

@Wedge009
Copy link
Member

Interesting. So std::put_time is always used in 1.15, while it's activated by a #define in 1.14 which is only set to 1 (as best as I can tell) when #defining MSVC, clang, or gcc 5 or later.

Out of curiosity I set it to 0 (using boost instead of put_time) in Windows to see what happens and there seems to be no difference - the translations are correct as before.

I also confirm using boost version in Linux behaves properly for non-English translations. put_time compiles okay but doesn't behave the same as boost's formatting, for some reason.

So I wonder why put_time doesn't work as intended at the moment, and whether or not that's going to be an issue for 1.15 where it seems to be the only function call for strftime there.

@Wedge009
Copy link
Member

Wedge009 commented Sep 13, 2019

If it's dependent on boost vs std implementation, does that make it an upstream issue? Or still Wesnoth issue because Wesnoth code decides which implementation to use? (Are they even supposed to be equivalent functions?)

Edit: Actually, since master uses only put_time now I think that's going to be a problem. What's wrong with using boost? Or is it the aim to try to replace boost dependencies where possible?

@gfgtdf
Copy link
Contributor

gfgtdf commented Sep 13, 2019

Edit: Actually, since master uses only put_time now I think that's going to be a problem. What's wrong with using boost?

I don't know and if it fixes this issue I support changing it back to the boost version (unless it breaks something else)

@Wedge009
Copy link
Member

@sevu As original reporter, can you check the PRs when you have time, please?

@Wedge009
Copy link
Member

Regarding the day-month/month-day ordering. I'm guessing the internals of the put_time formatting is correctly handles locale-appropriate format for non-English languages since I see this displayed as day-month in French. But for English it is stuck as always being month-day ordering because of the coding:

format_string = _("%b %d %Y");

put_time leaves it as it stands - en_US formatted - even if en_GB is selected.

@CelticMinstrel
Copy link
Member

  1. Those look like they should probably have TRANSLATORS notes...
  2. I'm confused, those are clearly marked translatable, so can't you override them in en_GB?

@sevu
Copy link
Member Author

sevu commented Sep 16, 2019

both PRs work

@Wedge009
Copy link
Member

@CelticMinstrel I wasn't aware these were configurable in translations! (I'm aware of the underscore notation but they don't usually register in my mind.) Thanks for the tip - I'll fix this for en_GB.

@sevu Thanks for confirmation. Now it's just a question of what Vultraz wants to do with regards to boost vs std.

Wedge009 added a commit that referenced this issue Sep 16, 2019
@Wedge009
Copy link
Member

Noting for the record that @jostephd could get std::put_time() to translate the dates if the prior imbue() call used a hard-coded std::locale object instead of the one returned by get_locale(). Which indicates that it's probably not an issue with glibc implementation - I added a comment against master to include this new information.

@jostephd
Copy link
Member

For the record, I used dummy.imbue(std::locale("de_DE.UTF-8")).

@CelticMinstrel
Copy link
Member

We're using Boost for locales, aren't we? Could this be a bug in Boost?

@Wedge009
Copy link
Member

I'm not sure how it all works, but I noticed this comment which implies that our locale handling relies mostly on boost:

wesnoth/src/gettext.cpp

Lines 232 to 234 in b1e8196

// We cannot have current_locale_ be a non boost-generated locale since it might not supply
// the bl::info facet. As soon as we add message paths, update_locale_internal might fail,
// for example because of invalid .mo files. So make sure we call it at least once before adding paths/domains

@jostephd I've also been trying to use the hard-coded dummy.imbue(std::locale("de_DE.UTF-8")) line to see what happens but I can't get it to work in either Linux or Windows (compiles okay but get crash when attempting to open load dialogue). Windows log simply says 'bad locale name' while Linux catches a 'St13runtime_error exception: locale::facet::_S_create_c_locale name not valid'.

@jostephd
Copy link
Member

@Wedge009 Locale names are platform-dependent. I don't know whether that string is a valid locale name on windows. On Linux, it's a valid locale name but locales have to be generated: see https://wiki.archlinux.org/index.php/Locale#Generating_locales (your distro may vary). You've mentioned testing in French, so fr_FR.UTF-8 might work for you. You can test with LC_ALL=fr_FR.UTF-8 ls -l.

@Wedge009
Copy link
Member

I first checked what locale was being used before trying to force my own. On start-up it uses en_AU.UTF-8 (OS locale), then switches to fr_FR.UTF-8 as per Wesnoth preferences. It's the same on both Windows and Linux (at least according to the logs).

@Wedge009
Copy link
Member

Hmm, it's the same when I hard-code to en_AU.UTF-8. I must be doing something wrong.

@jostephd
Copy link
Member

@Wedge009 Can you try compiling and running this program?

#include <locale>
int main(){ const auto& foo = std::locale("de_DE.UTF-8"); }

For me, the program compiles and runs cleanly, but if I change the string to a locale name that's not printed by locale -a, it terminates with locale::facet::_S_create_c_locale name not valid and SIGABRT.

@Wedge009
Copy link
Member

Ah, that's interesting. My locale -a list is made up only of en variants (makes sense I suppose, since I only have English installed on the OS). Furthermore, the locale names have a different format to what appears to be used inside Wesnoth: eg en_AU.utf8 instead of en_AU.UTF-8.

When I tried using that hard-coded in gettext_boost.cpp the load dialogue only shows what appears to be UNIX time-stamps instead of date strings. But at least I can see the hard-coding 'working' even if the translation itself is not.

Funnily enough my LANG variable is en_AU.UTF-8...

@jostephd
Copy link
Member

Furthermore, the locale names have a different format to what appears to be used inside Wesnoth: eg en_AU.utf8 instead of en_AU.UTF-8.

These two names are equivalent (they are aliases).

When I tried using that hard-coded in gettext_boost.cpp the load dialogue only shows what appears to be UNIX time-stamps instead of date strings

Odd, I see that too. The format string is %H:%M so this really shouldn't happen...it doesn't happen with clean master, though, so not urgent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Issues involving unexpected behavior. Confirmed Issues that have been successfully reproduced by at least one developer. Regression Issues that were not present in previous releases. Translations Issues with translations, translation tooling, the translations engine, or code that uses it.
Projects
None yet
Development

No branches or pull requests

6 participants