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

Honor locale time settings in lock screen. #47

Closed
wants to merge 1 commit into from

Conversation

bk2204
Copy link
Contributor

@bk2204 bk2204 commented May 30, 2014

The time display in the lock screen should honor the LC_TIME variable,
including the locale-specific settings for whether to display a 12-hour or
24-hour time. Sending the time and date strings through gettext results in
a value which is specific to LC_MESSAGES, which can be different from
LC_TIME. Use the %X value for a time, which is guaranteed to be appropriate
for the locale in question.

In my particular case, I use UTC for my system time even though I live in the United States, so a 12-hour time is not very useful to me. I set LC_TIME to POSIX and LC_MESSAGES (and all the rest of the locale variables) to en_US.UTF-8, so passing the locale strings through gettext breaks things for me. Assuming people set their locales correctly, everything should just work.

@stefano-k
Copy link
Collaborator

But please keep it translatable (and keep comments for translators)

@bk2204
Copy link
Contributor Author

bk2204 commented Jun 2, 2014

I don't think you quite understand. Keeping it translatable means that the translation string is looked up using LC_MESSAGES. So if the French translator uses something other than "%X" for the time, then the code doesn't honor LC_TIME anymore when LC_MESSAGES=fr_FR.UTF-8, and it's broken.

There's never a reason for the LC_MESSAGES setting or any translator to modify this, so it shouldn't be translatable. The code should always honor LC_TIME.

@stefano-k
Copy link
Collaborator

So you have to change "%A, %B %e" to "%x"

The time display in the lock screen should honor the LC_TIME variable,
including the locale-specific settings for whether to display a 12-hour or
24-hour time.  Sending the time and date strings through gettext results in
a value which is specific to LC_MESSAGES, which can be different from
LC_TIME.  Use the %X and %x values for a time and date, which are guaranteed
to be appropriate for the locale in question.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
@sunweaver
Copy link
Member

Hi Stefano,

any progress on the LC_TIME issue? I agree with Brian that the logic is in strftime() and the locale should handle the correct TIME and DATE layout. The translator teams should not be bugged with the correct translation of time/date strings.

And using the translations here indeed fails for LC_TIME=POSIX.

Mike

@sunweaver
Copy link
Member

Hi all,

I have just uploaded mate-screensaver 1.8.0-5 to Debian unstable including this patch.

I have tested it and it works as expected. Thanks Brian for providing this.

@stefano-k: I highly recommend accepting this pull request.

Mike

@infirit infirit added this to the 1.10 milestone Jun 11, 2014
@infirit
Copy link
Contributor

infirit commented Jun 11, 2014

Pushed with debbeac with small modification to use the previous date string. Using %x for the date looks awful imho.

@infirit infirit closed this Jun 11, 2014
@sunweaver
Copy link
Member

Hi Sander,

thanks for (partly) accepting this pull request. IMHO opinion, only using %X for that date, but not using %x is a half solution to this reported issue.

For the Debian package of mate-screensaver I will keep %x and %X to fulfill the requirements of a fully locale compliant Linux distro.

@infirit infirit reopened this Jun 12, 2014
@infirit
Copy link
Contributor

infirit commented Jun 12, 2014

@sunweaver using "%A, %B %e" will work perfectly fine with whatever locale setting you have, even C.

I don't like to use %x for the date as it looks horrible. This morning I realized why @stefano-k send this through gettext and reverted the commit and re-opened this PR for more discussion.

Using %X for time is ok and I see no need to send this through gettext but the date is now up for discussion.

@stefano-k stefano-k closed this in fc4dfbc Jun 25, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants