Fix context for QDateTime::toString() translations
Needs ReviewPublic

Authored by elvisangelaccio on Tue, Feb 5, 9:10 PM.

Details

Reviewers
lueck
aacid
cfeck
Summary

The documentation of QDateTime::toString() says that:

Any sequence of characters that are enclosed in single
quotes will be treated as text and not be used as an expression.

This means that translators cannot replace single quotes with other
characters such as «...». This is now described in the context of the
affected strings.

We also check that the translated string contains exactly 2 single
quotes, and we use the untraslated string otherwise.

BUG: 401382

Diff Detail

Repository
R318 Dolphin
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 8453
Build 8471: arc lint + arc unit
elvisangelaccio created this revision.Tue, Feb 5, 9:10 PM
Restricted Application added a project: Dolphin. · View Herald TranscriptTue, Feb 5, 9:10 PM
Restricted Application added a subscriber: kfm-devel. · View Herald Transcript
elvisangelaccio requested review of this revision.Tue, Feb 5, 9:10 PM
aacid added a subscriber: ltoscano.Tue, Feb 5, 9:57 PM

One thing we could do is not pass the string directly to toString and check if there's any ' character (there should be at least/exactly 2) if not ignore the translation and go with untranslated, that will be better than "random stuff", but i guess this is probably enough.

@ltoscano what do you think?

If it's not too difficult to pre-filter the string, maybe it's worth doing.

  • Use untraslated string as fallback
elvisangelaccio edited the summary of this revision. (Show Details)Sun, Feb 10, 9:42 PM

Can you add a qWarning in the else branches saying something "the translation is wrong, file a bug report"?

  • Added warnings

I didn't realize you moved the text out of the i18n call.

If you do that gettext won't extract it and thus it'll be untranslatable.

I see that you did that so you can reuse it untranslated later.

Maybe this would work (haven't tried it)?

KLocalizedString s = ki18nc(context, text);
const QString translatedFormat = s.toString();
if (translatedFormat.count(QLatin1Char('\'')) == 2) { /* good */ }
else { qCWarning();      newGroupValue = s.toString({"en"}); }

i think that would mean "no" translation in the else since it is asking for an en translation.

I didn't realize you moved the text out of the i18n call.

If you do that gettext won't extract it and thus it'll be untranslatable.

I see that you did that so you can reuse it untranslated later.

Maybe this would work (haven't tried it)?

KLocalizedString s = ki18nc(context, text);
const QString translatedFormat = s.toString();
if (translatedFormat.count(QLatin1Char('\'')) == 2) { /* good */ }
else { qCWarning();      newGroupValue = s.toString({"en"}); }

i think that would mean "no" translation in the else since it is asking for an en translation.

Ah sorry, didn't know that. I'll try.

  • Use KLocalizedString
aacid added inline comments.Sun, Feb 17, 6:01 PM
src/kitemviews/kfileitemmodel.cpp
2059

Sorry for being the most nitpicky person ever, should we include translatedFormat here (and the rest of the warning messages)?

Seems like it could be useful

Improved warnings

Code looks good to me, do you want me to actually test it or are you confident enough that it works?

@aacid If the french translation is still broken, maybe would be good to test this patch using a french locale?

Hmmmm, it doesn't seem to totally work

I get this https://i.imgur.com/bM3IPeu.png

I guess the MMMM and yyyy should have been replaced by actual text, right?