D26366: [Kuit] Port QRegExp to QRegularExpression, third pass

[Chusslove Illich]

ilic added inline comments.

INLINE COMMENTS

> dfaure wrote in kuitmarkup.cpp:90
> Interesting idea, this simplifies the code.
> 
> However the name "wsRx" no longer matches what this regexp does.
> 
> Also, the old trimming is missing here, isn't it?
> I guess \s*:?\s* is needed, and \s*/?\s*
> 
> Can you look into adding unittests for this stuff? Although I'm not sure if 
> that's possible.

I recall I (or on someone's suggestion) didn't want to use regexes too much for 
performance reasons. In the original code, other than this whitespace regex, 
all other regexes appear in places where things went wrong and some heuristics 
was needed to reduce garbage in output.

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D26366

To: ahmadsamir, #frameworks, ilic, dfaure, mlaurent, aacid
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26408: ki18n: add unittest for KUitSetup::setFormatForMarker

[Chusslove Illich]

ilic accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R249 KI18n

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D26408

To: dfaure, ilic
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26413: Hide Kuit::setupForDomain and KuitSetup from the public API

[Chusslove Illich]

ilic added a comment.


  In D26413#587390 , @dfaure wrote:
  
  > Alternative approach: export it and unittest it, D26408 
.
  >
  > @ilic your input would be very welcome on which approach should be taken. I 
don't care much personally, it's just really inconsistent right now (looks like 
public API, but is NOT public API)
  
  
  I did intended it as public API, its usage is even explained in the 
Programmers Guide. So for the moment I would go with D26408 
.
  
  For KF6 I'd leave decision to others: should a piece of public API that 
seemed like it could have been useful but no-one ever tried to use in more than 
seven years, still remain public API?

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D26413

To: dfaure, ilic
Cc: ahmadsamir, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24898: Add KLocalizedString::untranslatedText

[Chusslove Illich]

ilic accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R249 KI18n

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D24898

To: aacid, ilic
Cc: apol, ilic, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24898: Add KLocalizedString::untranslatedText

[Chusslove Illich]

ilic added a comment.


  An apparent need for this functionality was popping up from time to time, but 
somehow never got too serious. Probably because each time it would have been 
just a convenience, since the original text could have been transported to 
where it is needed next to the KLocalizedString objects themselves. Is it then 
worthy of adding more API? I don't know.
  
  However, if it is added, then there should exist also methods for retrieving 
the context and original plural strings.

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D24898

To: aacid
Cc: apol, ilic, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D24884: I18N_NOOP2 was deprecated but we can't replace by I18NC_NOOP as it expends it as 2 elements (context + text)

[Chusslove Illich]

ilic added a comment.


  The idea was indeed to deprecate stripping of context, and not only the macro 
name, for the reason Alber provided.
  
  The apparent counterexample in kuitmarkup.cpp is seen only due to 
macro-within-macro call and the macro expansion order, which is a situation 
that does not (or did not at the time of writing) occur anywhere else through 
KDE projects. But it too can be reformulated easily to the cleaner variant.
  
  However, as I recall, even though KLocalizedString objects do deferred 
translation, I18N* macros were not deprecated alltogether in order to still 
allow for well-defined static initializers.

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D24884

To: mlaurent, dfaure, ilic
Cc: aacid, vkrause, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns

D22698: Support passing target to ki18n_wrap_ui macro

[Chusslove Illich]

ilic accepted this revision.
This revision is now accepted and ready to land.

REVISION DETAIL
  https://phabricator.kde.org/D22698

To: daandemeyer, ilic
Cc: ltoscano, alexmerry, turbov, cgiboudeaux, kde-frameworks-devel, LeGast00n, 
sbergeron, michaelh, ngraham, bruns

D22698: Support passing target to ki18n_wrap_ui macro

[Chusslove Illich]

ilic added a comment.


  (As a maintainer) I can just say, that I'm not qualified to judge if the 
added behavior and implementation fits the modern CMake practices. If someone 
else confirms this is indeed the case, I'm happy to accept it.

REVISION DETAIL
  https://phabricator.kde.org/D22698

To: daandemeyer, ilic
Cc: ltoscano, alexmerry, turbov, cgiboudeaux, kde-frameworks-devel, LeGast00n, 
sbergeron, michaelh, ngraham, bruns

D22069: Localize long number strings

[Chusslove Illich]

ilic added a comment.


  The way forward is to find out why making that one dot-to-comma change and 
having fr_FR.utf8 glibc locale installed is not sufficient to get the test to 
pass. To my understanding, an explicit QLocale::setDefault() call either 1) 
should not be necessary, or 2) it should be somewhere in the source and not in 
the test.

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D22069

To: ngraham, #localization, #frameworks, broulik
Cc: safaalfulaij, mikeroyal, aspotashev, ilic, kde-frameworks-devel, broulik, 
LeGast00n, michaelh, ngraham, bruns

D22069: Localize long number strings

[Chusslove Illich]

ilic added a comment.


  I'm not sure about what happens with global state, but at any rate, what I 
expected is that in klocalizedstringtest.cpp exactly one character is modified, 
replacing dot with comma in string " 4.20". Do you have fr_FR.utf8 glibc locale 
installed on your system?

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D22069

To: ngraham, #localization, #frameworks, broulik
Cc: mikeroyal, aspotashev, ilic, kde-frameworks-devel, broulik, LeGast00n, 
michaelh, ngraham, bruns

D22069: Localize long number strings

[Chusslove Illich]

ilic added inline comments.

INLINE COMMENTS

> klocalizedstringtest.cpp:222
> +
> +QLocale::setDefault(QLocale(QLocale::French));
> +QCOMPARE(ki18n("%1").subs(4.2, 5, 'f', 2).toString(),

The French locale is already being set in line 45 using setlocale, for which 
the system has to have fr_FR.utf8 glibc locale installed. Any further setting 
up of locale details, if needed, should happen in lines 70-76, which are 
currently commented out with "until locale system is ready" (and if not needed, 
this whole commented out part should be deleted).

> klocalizedstringtest.cpp:226
> +
> +QLocale::setDefault(QLocale(QLocale::English, QLocale::UnitedStates));
>  QCOMPARE(ki18n("%1").subs(4.2, 5, 'f', 2).toString(),

The tests work with French locale, there is no reason introduce English here 
only. It also clobbers the globaly set locale elements in initTestCase.

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D22069

To: ngraham, #localization, #frameworks, broulik
Cc: mikeroyal, aspotashev, ilic, kde-frameworks-devel, broulik, LeGast00n, 
michaelh, ngraham, bruns

D22069: Localize long number strings

[Chusslove Illich]

ilic added a comment.


  Still lacks the change in unit test in autotests/klocalizedstringtest.cpp, 
from " 4.20" to " 4,20".

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D22069

To: ngraham, #localization, #frameworks, broulik
Cc: ilic, kde-frameworks-devel, broulik, LeGast00n, michaelh, ngraham, bruns

D14686: API docs: fix *xi18n* calls in example code to use correct variants

[Chusslove Illich]

ilic accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R249 KI18n

BRANCH
  fixxi18ncallexamples

REVISION DETAIL
  https://phabricator.kde.org/D14686

To: kossebau, ilic
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D14473: Fix KCatalog::translate when translation is same as original text

[Chusslove Illich]

ilic accepted this revision.
ilic added a comment.
This revision is now accepted and ready to land.


  I distinctly remember having had the same quandry once, and then someone from 
the gettext side said to compare pointers. Even the comment to dngettext call 
in the plural version of KCatalog::translate mentions comparison of pointers to 
this effect. So, "someone" simply screwed up on porting from kdelibs4, when 
replacing char*'s with QByteArray's...

REPOSITORY
  R249 KI18n

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D14473

To: aacid, ilic
Cc: ilic, kde-frameworks-devel, michaelh, ngraham, bruns

D12355: [API dox] New UI marker @info:placeholder

[Chusslove Illich]

ilic added a comment.


  In D12355#255342 , @ilic wrote:
  
  > The diff is a bit weird, as if it is for the vallusuffix from D12353 
. But looks good in the repository as 
committed.
  
  
  ...or, my browser did something wrong. Or PEBKAC.

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D12355

To: kossebau, #localization, ilic
Cc: #frameworks, michaelh, bruns

D12355: [API dox] New UI marker @info:placeholder

[Chusslove Illich]

ilic added a comment.


  The diff is a bit weird, as if it is for the vallusuffix from D12353 
. But looks good in the repository as 
committed.

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D12355

To: kossebau, #localization, ilic
Cc: #frameworks, michaelh, bruns

D12353: [API dox] New UI marker @item:valuesuffix

[Chusslove Illich]

ilic accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R249 KI18n

BRANCH
  additemvaluesuffixuimaker

REVISION DETAIL
  https://phabricator.kde.org/D12353

To: kossebau, #localization, ilic
Cc: #frameworks, michaelh, bruns

D11149: Create a constructor for KLocalizedStringPrivate

[Chusslove Illich]

ilic accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D11149

To: jtamate, #frameworks, ilic
Cc: ilic, michaelh

D11149: Create a constructor for KLocalizedStringPrivate

[Chusslove Illich]

ilic added inline comments.

INLINE COMMENTS

> klocalizedstring.cpp:202-204
> +   :format()
> +   ,numberSet(false)
> +   ,markupAware(false)

Should have space after colon/comma?

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D11149

To: jtamate, #frameworks
Cc: ilic, michaelh

D10758: Use QLocale instead of QLocale::system

[Chusslove Illich]

ilic accepted this revision.

REPOSITORY
  R249 KI18n

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D10758

To: aacid, apol, ilic
Cc: lueck, #frameworks, michaelh

D8351: API dox: add note about calling setApplicationDomain after QApp creation

[Chusslove Illich]

ilic added a comment.


  Yes, I guess in the end it's the simplest way to look at it.
  
  One thing though: I'd add at least another sentence claryfing that unlike an 
i18n* call, a ki18n* can happen at any time, and only its toString method 
should be called after Q*App instance creation (because that's when translation 
happens and locale is queried).

REPOSITORY
  R249 KI18n

BRANCH
  addNoteAboutCallingSetAppDomainAfterQApp

REVISION DETAIL
  https://phabricator.kde.org/D8351

To: kossebau, #frameworks, ilic, ltoscano, dfaure
Cc: dfaure, michaelh, ngraham

D9585: Fix finding libintl when "cross"-compiling native Yocto packages

[Chusslove Illich]

ilic accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R249 KI18n

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D9585

To: vkrause, #frameworks, ilic

D9295: do not treat ts-pmap-compile as exectuable

[Chusslove Illich]

ilic accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R249 KI18n

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D9295

To: sitter, ilic
Cc: #frameworks

D9162: KI18n: fix a number of double lookups

[Chusslove Illich]

ilic accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R249 KI18n

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D9162

To: dfaure, ilic
Cc: #frameworks

D8296: Use Ctrl+Shift+. as the standard shortcut for "Configure "

[Chusslove Illich]

ilic added a comment.


  `KStandardShortcut::Preferences` label is being overridden in 
KConfigWidgets's `src/kstandardaction_p.h` to `"&Configure %1..."` and %1 
substituted with current application name.

REPOSITORY
  R237 KConfig

REVISION DETAIL
  https://phabricator.kde.org/D8296

To: ngraham, #frameworks, #vdg, broulik, rkflx
Cc: ilic, abetts, elvisangelaccio, aacid, argonel, marten, graesslin, broulik, 
#frameworks

D8296: Use Ctrl+Shift+. as the standard shortcut for "Configure "

[Chusslove Illich]

ilic added a comment.


  "Configure Application" does sound better than "Preferences", but that's just 
my opinion as a random translator, no link to my i18n plumber's hat :)

REPOSITORY
  R237 KConfig

REVISION DETAIL
  https://phabricator.kde.org/D8296

To: ngraham, #frameworks, #vdg, broulik, rkflx
Cc: ilic, abetts, elvisangelaccio, aacid, argonel, kfunk, marten, graesslin, 
broulik, #frameworks

D8351: API dox: add note about calling setApplicationDomain after QApp creation

[Chusslove Illich]

ilic added a comment.


  Well... it's a tough situation. It is not by design that `i18n` calls should 
in any way depend on creation of `QApplication`, and also any library may place 
an `i18n` call before the main program creates `QApplication`. The only 
solution I see is that environment is rechecked at every `i18n` call. This 
would be easy to do (just replacing every `s->languages` with a newly 
implemented `s->getLanguages()`), but I've no idea what would be the 
performance hit of that.

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D8351

To: kossebau, #frameworks, ilic, ltoscano

D7033: Port ki18n from QtScript to QtQml

[Chusslove Illich]

ilic added a comment.


  I can't say about QtScript to QtQML conversion, but wrt. those FIXMEs, 
removing variable argument lists obviously breaks compatibility, and especially 
the acall function makes no sense without it.

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D7033

To: carewolf, ilic
Cc: dhaumann, ltoscano, kfunk, #frameworks

D7523: Use the application domain when doing i18n of actions

[Chusslove Illich]

ilic accepted this revision.
ilic added a comment.
This revision is now accepted and ready to land.


  I'd say the patch is fine as it is. If this code is invoked through an 
intermediate library, that library's definition of TRANSLATION_DOMAIN will have 
no effect, instead kross' TRANSLATION_DOMAIN will be used. Therefore if 
applicationDomain is empty, then there is no place to fetch translation from, 
and untranslated string should be returned (as it will happen with this patch).
  
  Alternatively, in case of empty applicationDomain some sort of warning could 
be produced.

REPOSITORY
  R317 Kross

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D7523

To: aacid, ltoscano, ilic, aspotashev
Cc: #frameworks

D5208: Allow loading i18n catalogs from arbitrary locations

[Chusslove Illich]

ilic accepted this revision.
ilic added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> davidedmundson wrote in kcatalog.cpp:124
> I think I still need it.
> 
> QHash::value() isn't marked as being atomic so we still need to make sure 
> we're not reading at the same time as someone else is modifying the hash 
> table.

Hm, right... That makes me wonder where else there should be a lock and it's 
missing :) But that's for another story.

REPOSITORY
  R249 KI18n

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D5208

To: davidedmundson, #frameworks, ilic
Cc: ilic

D5208: Allow loading i18n catalogs from arbitrary locations

[Chusslove Illich]

ilic added inline comments.

INLINE COMMENTS

> davidedmundson wrote in klocalizedstringtest.cpp:59
> Pretty sure it's fine. kactivities and kjs use them with no issue.

Well, since no-one else complained either, guess it's ok. Easy to fix later if 
someone does complain.

> ilic wrote in kcatalog.cpp:124
> Only read access is needed here, user QHash::value instead of 
> QHash::operator[].

Also the lock should be removed, now that only read access is used.

> ilic wrote in kcatalog.cpp:151
> Also only read access needed.

Also the lock should be removed.

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D5208

To: davidedmundson, #frameworks, ilic
Cc: ilic

D6339: Remove custom doxygen config files

[Chusslove Illich]

ilic accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R249 KI18n

BRANCH
  removecustomdoxygencfg

REVISION DETAIL
  https://phabricator.kde.org/D6339

To: kossebau, #frameworks, ilic

D6338: DOXYGEN_PREPROC -> DOXYGEN_SHOULD_SKIP_THIS, to standardize within KF5

[Chusslove Illich]

ilic accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R249 KI18n

BRANCH
  standardizedoxygenppmacro

REVISION DETAIL
  https://phabricator.kde.org/D6338

To: kossebau, #frameworks, ilic

D5455: Extend Programmer's Guide with notes about influence of setlocale()

[Chusslove Illich]

ilic accepted this revision.
ilic added a comment.


  All good to me.

REPOSITORY
  R249 KI18n

BRANCH
  addNoteAboutGettextLocaleIssuse

REVISION DETAIL
  https://phabricator.kde.org/D5455

To: kossebau, #frameworks, ilic, aacid
Cc: aacid

D5455: Extend Programmer's Guide with notes about influence of setlocale()

[Chusslove Illich]

ilic added a comment.


  I guess not bad to mention this stuff, but I would make following adaptations:
  
  1. Move the section as subsection of "Controlling Language Of Catalog to 
Use", and give it a more focused ( title, e.g. "Placing i18n calls before 
creation of QCoreApplication".
  
  2. State up front that in general one should avoid making i18n calls before 
QCoreApplication is crated, in one of two ways: either using statically 
initialized character arrays wrapped in I18N_* macros for extraction, and later 
making actual i18n* calls; or, using ki18n* calls to make KLocalizedString 
instances, that do not perform immediate catalog lookup, and later fetch actual 
translation throug their toString() methods.
  
  3. Finally the part about setlocale etc, for the case that all of the above 
is for some reason not applicable. I'm not 100% percent sure that everything 
will work as expected with this. For example, before QCoreApplication is 
initialized, what happens with ki18n's internal search for catalogs through 
standard directory paths...

REPOSITORY
  R249 KI18n

BRANCH
  addNoteAboutGettextLocaleIssuse

REVISION DETAIL
  https://phabricator.kde.org/D5455

To: kossebau, #frameworks, ilic, aacid
Cc: aacid

D5208: Allow loading i18n catalogs from arbitrary locations

[Chusslove Illich]

ilic added inline comments.

INLINE COMMENTS

> klocalizedstring.h:558
>  
> +static void addDomainLocaleDir(const QByteArray &domain, const QString 
> &path);
> +

Misses docstring. It should be made clear that this method is for "special 
purposes" (e.g. plugins and whatnot), and that normal programs should not force 
locale directories but rely on standard data directory lookup hierarchy.

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D5208

To: davidedmundson, #frameworks, ilic
Cc: ilic

D5208: Allow loading i18n catalogs from arbitrary locations

[Chusslove Illich]

ilic requested changes to this revision.
ilic added a comment.
This revision now requires changes to proceed.


  I think it's fine to add this capability, especially given that Gettext 
(which Ki18n is an extension of) not only has it, but uses it exclusively (it 
does not search through any environment-derived directories).
  
  However, there are a few questions of semantics.
  
  So far it was always possible for a user to override the locale directory 
through environment variables, e.g. to test translation without building the 
full program. Should we still enable this, i.e. provide environment overriding 
also in case program sets the locale directory? But I guess we can postpone 
this question, since as you mention in KDE 4 Plasma the locale directory was 
being forced by the program anyway by other means.
  
  On the other side, if the locale directory for the domain is set, should (as 
in currently proposed implementation) environment-derived directories still be 
looked through if catalog is not found in the set directory?
  
  I think better name for the method is setDomainLocaleDir, because if it is 
called again for the same domain, it will override the previous setting and not 
add another directory for it.

INLINE COMMENTS

> klocalizedstringtest.cpp:59
>  if (m_hasFrench) {
> -m_hasFrench = compileCatalogs(dataDir);
> +m_hasFrench = compileCatalogs({QFINDTESTDATA("po/fr/ki18n-test.po"), 
> QFINDTESTDATA("po/fr/ki18n-test-qt.po")}, dataDir);
>  }

I think we're not allowed to use initializer lists yet, due to MSVC11 support.

> klocalizedstringtest.cpp:523
> +compileCatalogs({QFINDTESTDATA("po/fr/ki18n-test2.po")}, dir.path());
> +KLocalizedString::setApplicationDomain("ki18n-test2");
> +KLocalizedString::addDomainLocaleDir("ki18n-test2", dir.path() + 
> "/locale");

setApplicationDomain should not be called twice, use i18nd method instead, like 
i18nd("ki18n-test2", "Cheese").

> kcatalog.cpp:124
> +{
> +QMutexLocker lock(&catalogStaticData->mutex);
> +const QString customLocaleDir = 
> catalogStaticData->customCatalogDirs[domain];

Only read access is needed here, user QHash::value instead of QHash::operator[].

> kcatalog.cpp:126
> +const QString customLocaleDir = 
> catalogStaticData->customCatalogDirs[domain];
> +if (!customLocaleDir.isEmpty() && !QFileInfo::exists(customLocaleDir 
> + relpath)) {
> +return customLocaleDir;

Maybe I'm seeing something wrongly here, but... customLocaleDir + relpath may 
miss path separator? Really !QFileInfo::exists and not QFileInfo::exists?

> kcatalog.cpp:151
> +{
> +QMutexLocker lock(&catalogStaticData->mutex);
> +if (catalogStaticData->customCatalogDirs.contains(domain_)) {

Also only read access needed.

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D5208

To: davidedmundson, #frameworks, ilic
Cc: ilic

D5195: don't add the autotests dir when test building is disabled

[Chusslove Illich]

ilic accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R249 KI18n

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D5195

To: sitter, ilic
Cc: #frameworks

Re: Review Request 127800: Format number in KLocalizedString::subs

[Chusslove Illich]

> On Мај 1, 2016, 6:40 по п., Chusslove Illich wrote:
> > To have localized numbers is the expected behavior (and documented as 
> > such). But it was "temporarily" disabled between kdelibs and KF5, until 
> > replacements for KLocale were decided. I don't remember why I didn't switch 
> > to %L1 at that moment, since it was available too... Well, I guess best to 
> > commit this and see later if there is any trouble.
> > 
> > Did you run unit tests? Could be some of them needs adapting.
> 
> Kai Uwe Broulik wrote:
> The unit tests fail (2 out of 4) locally both with and without this patch.
> 
> Chusslove Illich wrote:
> Hm, and which are these failing tests? When I run them, they all pass 
> (including expected fails).
> 
> With the patch, I had to change in autotests/klocalizedstringtest.cpp one 
> comparison literal from " 4.20" to " 4,20".
> 
> Kai Uwe Broulik wrote:
> Ah, they pass, but only if I run them with LANG=en_US
> 
> ```
> FAIL!  : KI18nDeclarativeTest::testLocalizedContext(plural translation 
> with domain) Compared values are not the same
>Actual   
> (object->property(propertyName.toUtf8().constData()).toString()): "In 3 
> Sekunden"
>Expected (value)   
>   : "in 3 seconds"
>Loc: 
> [/home/kaiuwe/Projekte/kf5/src/frameworks/ki18n/autotests/ki18ndeclarativetest.cpp(60)]
>
> FAIL!  : KLocalizedStringTest::semanticTags() Compared values are not the 
> same
>Actual   (xi18nc("@info", "Apply color scheme 
> %1?", "XXX")): "Apply color scheme 
> \u201EXXX\u201C?"
>    Expected (QString("Apply color scheme “XXX”?"))   
>  : "Apply color scheme \u201CXXX\u201D?"
>Loc: 
> [/home/kaiuwe/Projekte/kf5/src/frameworks/ki18n/autotests/klocalizedstringtest.cpp(351)]
> ```
> 
> Chusslove Illich wrote:
> Ok, there's something strange with those two failing tests, that has 
> nothing to do with the current patch.
> 
> But then what is also strange is that with patched version you don't get 
> a failure on that number format test. Maybe you don't have locale fr_FR.UTF-8 
> generated on the system, and then it falls back to C locale.
> 
> Anyway, then just make that change in literals (" 4.20" to " 4,20"), and 
> we call it good. (Even if then you should get failure on that test :)

Hm, wasn't the only thing missing here the modification of the unit test?


- Chusslove


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127800/#review95056
---


On Феб. 8, 2017, 3:10 по п., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127800/
> ---
> 
> (Updated Феб. 8, 2017, 3:10 по п.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> Use %L1 instead of %1 which tells it it's a number.
> 
> This way I get decimal indicator and thousands separators according to my 
> locale in eg. KUnitConversion
> 
> 
> Diffs
> -
> 
>   src/klocalizedstring.cpp fc80135 
> 
> Diff: https://git.reviewboard.kde.org/r/127800/diff/
> 
> 
> Testing
> ---
> 
> 0,00094697205065430554 Meilen and 1.524 Millimeter
> instead of
> 0.00094697205065430554 Meilen and 1524 Millimeter
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

Re: Review Request 129455: Add FreeBSD to metainfo.yaml.

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129455/#review101088
---


Ship it!




Ship It!

- Chusslove Illich


On Нов. 20, 2016, 4:39 по п., Tobias Berner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129455/
> ---
> 
> (Updated Нов. 20, 2016, 4:39 по п.)
> 
> 
> Review request for KDE Frameworks and Chusslove Illich.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> Add FreeBSD to metainfo.yaml.
> 
> 
> Diffs
> -
> 
>   metainfo.yaml 7f8beac45ec17f0074352bb9b23065737964c994 
> 
> Diff: https://git.reviewboard.kde.org/r/129455/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tobias Berner
> 
>

Re: Review Request 128243: Fix parsing of env vars in KLocalizedStringPrivateStatics::initializeLocaleLanguages()

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128243/#review96878
---


Ship it!




Ship It!

- Chusslove Illich


On Јун 18, 2016, 10:36 по п., Friedrich W. H. Kossebau wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128243/
> ---
> 
> (Updated Јун 18, 2016, 10:36 по п.)
> 
> 
> Review request for KDE Frameworks and Chusslove Illich.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> This patch fixes two things:
> 
> a) `splitLocale(...)` had the wrong order of splitting off modifier and 
> codeset: the old code was first splitting off anything behind "." as charset 
> (which would include the modifier though if present) and only then seeing to 
> split off anything behind "@" as modifier. So with both charset and modifier 
> present this would fail.
> 
> b) The locales listed in "LANGUAGE" would be, other than those in "LC_ALL", 
> "LC_MESSAGES" and "LANG", only added as they are to the list of 
> `localeLanguages`, without generating their variants. That seems unbalanced 
> to me, as it would mean KCatalog not properly detecting mo files e.g. in 
> "/usr/share/locale/ru/LC_MESSAGES/" subfolder with "LANGUAGE=ru_RU.UTF-8", 
> "LANG=", "LC_ALL=", "LC_MESSAGES=". Or is that on purpose?
> 
> 
> Diffs
> -
> 
>   src/klocalizedstring.cpp fc80135 
> 
> Diff: https://git.reviewboard.kde.org/r/128243/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Friedrich W. H. Kossebau
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128243: Fix parsing of env vars in KLocalizedStringPrivateStatics::initializeLocaleLanguages()

[Chusslove Illich]

> On Јун 19, 2016, 1:11 пре п., Chusslove Illich wrote:
> > a) Right, old code is obviously wrong (sigh).
> > 
> > b) According to Gettext convention, LANGUAGE is a fine-tuning variable and 
> > it (each of its colon-separated parts) is to be taken as-is, without the 
> > system trying to be smart.
> 
> Friedrich W. H. Kossebau wrote:
> a) possibly stayed undetected as there might be no locales in real world 
> which have both modifier and charset set. Still is better to have the code 
> straight, less confusing for anyone who might come to read it (like me :) ).
> 
> b) My (recently gained) theoretic knowledge about the LANGUAGE env var is 
> mainly from ABOUT-NLS, and there it says "'LL_CC' combinations can be 
> abbreviated as 'LL' to
> denote the language's main dialect.". From which I had guessed that e.g. 
> both "LANGUAGE=ru_RU:foo:bar" and "LANGUAGE=ru:foo:bar" would result in the 
> LOCALEPREFIX/ru/LC_MESSAGES/" catalogs being picked up (at least when there 
> are no ru_RU of course). Being a newbie on this field, I miss real-world 
> experience surely and just can talk from what I understood so far and see on 
> my system.
> 
> Now, I have not yet got to understanding the code in the gettext lib, but 
> from testing on commandline "LANGUAGE=ru_RU:de:en" will result in the 
> catalogs from the ru folder being used e.g. for the coreutils tool "ls":
> ```
> LANG=de_DE.UTF-8 LC_MESSAGES= LC_ALL= LANGUAGE=ru_RU.UTF-8:de:en ls --help
> ```
> (yes, even using charset here, to show that even that is dealt with when 
> set, though perhaps just ignored)
> 
> Doing the same with a ki18n app does not work:
> ```
> LANG=de_DE.UTF-8 LC_MESSAGES= LC_ALL= LANGUAGE=ru_RU.UTF-8:de:en okteta
> 
> ```
> will with the current code not result in russian translations in "ru" 
> folder being used for ki18n-based translations, but the "de_DE" on.
> More, other than ki18n, the Qt system locale seems to pick up the 
> LANGUAGE setting and goes fur russian, at least QLocale::system(), as used by 
> kf5 code for picking the language to use with QTranslator, results in russian 
> catalogs ("*_ru.qm") being loaded and used.
> Can be seen e.g. in the menu string "Show Toolbar" (in "Settings" menu, 
> from kconfig5_qt.po) or the print dialog, where most string are from Qt lib 
> catalog.
> Which results in a language mix in the UI.
> 
> Only when using just the language code "ru", matching exactly the folder 
> name with the catalog, this will give me also russian translations for 
> anything based on ki18n:
> ```
> LANG=de_DE.UTF-8 LC_MESSAGES= LC_ALL= LANGUAGE=ru:de:en okteta
> ```
> 
> So at least on my openSUSE Tumbleweed system both the gettext lib and the 
> Qt locale code seem to have a different treatment of the LANGUAGE env var 
> than what the current ki18n code does.
> No idea/experience if that is a problem in the real world though with 
> real world usage of values for the LANGUAGE var. Just hit this issues during 
> naive/clueless translation handling testing and playing around with all the 
> vars. Still, with my current knowledge I would feel better to align the 
> behaviour of ki18n more with the others.
> 
> Friedrich W. H. Kossebau wrote:
> Possibly things need even more fixing. So values in the list set for 
> "LANGUAGE" seem to be not only taken as they are but instead are also tried 
> in stripped variants (so "ru" catalogs are taken for "LANGUAGE=ru_RU") from 
> what I see. But that comment from gettext's ABOUT-NLS I cited earlier 
> ("'LL_CC' combinations can be abbreviated as 'LL' to denote the language's 
> main dialect.") might also mean ki18n needs to check for a catalog with the 
> language's main dialect, in case there is none for for just the language 
> itself?
> 
> Question which currently prevents me from understanding more: how to know 
> the language's main dialect? Is that "ll_LL", so the same letters as used for 
> the country as used for the language? (I learned that LANG has to have the 
> country always set in the given code, so it is not a problem there)
> 
> So with "LANGUAGE=ll" if there is no "ll/LC_MESSAGES/foo.mo" would we 
> also need to check for "ll_LL/LC_MESSAGES/foo.mo"?
> 
> Friedrich W. H. Kossebau wrote:
> The code at least of the gnu implementation seems to not make a different 
> handling of the values for LANGUAGE ov

Re: Review Request 128243: Fix parsing of env vars in KLocalizedStringPrivateStatics::initializeLocaleLanguages()

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128243/#review96702
---



a) Right, old code is obviously wrong (sigh).

b) According to Gettext convention, LANGUAGE is a fine-tuning variable and it 
(each of its colon-separated parts) is to be taken as-is, without the system 
trying to be smart.

- Chusslove Illich


On Јун 18, 2016, 10:36 по п., Friedrich W. H. Kossebau wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128243/
> ---
> 
> (Updated Јун 18, 2016, 10:36 по п.)
> 
> 
> Review request for KDE Frameworks and Chusslove Illich.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> This patch fixes two things:
> 
> a) `splitLocale(...)` had the wrong order of splitting off modifier and 
> codeset: the old code was first splitting off anything behind "." as charset 
> (which would include the modifier though if present) and only then seeing to 
> split off anything behind "@" as modifier. So with both charset and modifier 
> present this would fail.
> 
> b) The locales listed in "LANGUAGE" would be, other than those in "LC_ALL", 
> "LC_MESSAGES" and "LANG", only added as they are to the list of 
> `localeLanguages`, without generating their variants. That seems unbalanced 
> to me, as it would mean KCatalog not properly detecting mo files e.g. in 
> "/usr/share/locale/ru/LC_MESSAGES/" subfolder with "LANGUAGE=ru_RU.UTF-8", 
> "LANG=", "LC_ALL=", "LC_MESSAGES=". Or is that on purpose?
> 
> 
> Diffs
> -
> 
>   src/klocalizedstring.cpp fc80135 
> 
> Diff: https://git.reviewboard.kde.org/r/128243/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Friedrich W. H. Kossebau
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127834: ki18n: Fix theoretically possible use-after-free in gettext when using strict ANSI compilers

[Chusslove Illich]

> On Мај 7, 2016, 2:26 по п., Christoph Feck wrote:
> > This commit causes compilation failure:
> > 
> > ki18n/src/gettext.h:130:9: error: 'translation_found' was not declared in 
> > this scope
> > ki18n/src/gettext.h:180:9: error: 'translation_found' was not declared in 
> > this scope

Moved the declaration now outside of #ifdef block, should work for you.


- Chusslove


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127834/#review95259
---


On Мај 7, 2016, 6:12 пре п., Michael Pyne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127834/
> ---
> 
> (Updated Мај 7, 2016, 6:12 пре п.)
> 
> 
> Review request for KDE Frameworks, Localization and Translation (l10n) and 
> Chusslove Illich.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> Coverity noted that some of code for message catalog lookup uses some pointer 
> values after they are freed. Even though the use in question is a simple 
> equality comparison against a different (valid) pointer, it is still 
> undefined behavior according to the C (and C++) language specs and is 
> therefore liable to cause miscompilation and who knows what other kinds of 
> problems.
> 
> This code is not normally enabled, normally a code path that supports 
> variable-length arrays is active, which is not susceptible to this bug.
> 
> Since VLAs are not supported even in current C++ versions, making VLA support 
> mandatory is not feasible, so instead I opted to move the pointer comparisons 
> to a point in the code where the comparison is valid, and then use the saved 
> result later.
> 
> I have also reported the bug to GNU Gettext, since upstream still has the 
> error. It is GNU Gettext bug 47847.
> 
> 
> Diffs
> -
> 
>   src/gettext.h b06fc90 
> 
> Diff: https://git.reviewboard.kde.org/r/127834/diff/
> 
> 
> Testing
> ---
> 
> Code compiles and KDE applications still seem to work fine. I also tested 
> with the changed code path forcibly enabled by disabling VLA support, and 
> things still seemed to work.
> 
> There's not a lot of autotests to choose from, but the KLocalizedString test 
> still passed.
> 
> 
> Thanks,
> 
> Michael Pyne
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127834: ki18n: Fix theoretically possible use-after-free in gettext when using strict ANSI compilers

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127834/#review95232
---


Ship it!




Ship It!

- Chusslove Illich


On Мај 5, 2016, 3:07 пре п., Michael Pyne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127834/
> ---
> 
> (Updated Мај 5, 2016, 3:07 пре п.)
> 
> 
> Review request for KDE Frameworks, Localization and Translation (l10n) and 
> Chusslove Illich.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> Coverity noted that some of code for message catalog lookup uses some pointer 
> values after they are freed. Even though the use in question is a simple 
> equality comparison against a different (valid) pointer, it is still 
> undefined behavior according to the C (and C++) language specs and is 
> therefore liable to cause miscompilation and who knows what other kinds of 
> problems.
> 
> This code is not normally enabled, normally a code path that supports 
> variable-length arrays is active, which is not susceptible to this bug.
> 
> Since VLAs are not supported even in current C++ versions, making VLA support 
> mandatory is not feasible, so instead I opted to move the pointer comparisons 
> to a point in the code where the comparison is valid, and then use the saved 
> result later.
> 
> I have also reported the bug to GNU Gettext, since upstream still has the 
> error. It is GNU Gettext bug 47847.
> 
> 
> Diffs
> -
> 
>   src/gettext.h b06fc90 
> 
> Diff: https://git.reviewboard.kde.org/r/127834/diff/
> 
> 
> Testing
> ---
> 
> Code compiles and KDE applications still seem to work fine. I also tested 
> with the changed code path forcibly enabled by disabling VLA support, and 
> things still seemed to work.
> 
> There's not a lot of autotests to choose from, but the KLocalizedString test 
> still passed.
> 
> 
> Thanks,
> 
> Michael Pyne
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127800: Format number in KLocalizedString::subs

[Chusslove Illich]

> On Мај 1, 2016, 6:40 по п., Chusslove Illich wrote:
> > To have localized numbers is the expected behavior (and documented as 
> > such). But it was "temporarily" disabled between kdelibs and KF5, until 
> > replacements for KLocale were decided. I don't remember why I didn't switch 
> > to %L1 at that moment, since it was available too... Well, I guess best to 
> > commit this and see later if there is any trouble.
> > 
> > Did you run unit tests? Could be some of them needs adapting.
> 
> Kai Uwe Broulik wrote:
> The unit tests fail (2 out of 4) locally both with and without this patch.
> 
> Chusslove Illich wrote:
> Hm, and which are these failing tests? When I run them, they all pass 
> (including expected fails).
> 
> With the patch, I had to change in autotests/klocalizedstringtest.cpp one 
> comparison literal from " 4.20" to " 4,20".
> 
> Kai Uwe Broulik wrote:
> Ah, they pass, but only if I run them with LANG=en_US
> 
> ```
> FAIL!  : KI18nDeclarativeTest::testLocalizedContext(plural translation 
> with domain) Compared values are not the same
>Actual   
> (object->property(propertyName.toUtf8().constData()).toString()): "In 3 
> Sekunden"
>Expected (value)   
>   : "in 3 seconds"
>Loc: 
> [/home/kaiuwe/Projekte/kf5/src/frameworks/ki18n/autotests/ki18ndeclarativetest.cpp(60)]
>
> FAIL!  : KLocalizedStringTest::semanticTags() Compared values are not the 
> same
>Actual   (xi18nc("@info", "Apply color scheme 
> %1?", "XXX")): "Apply color scheme 
> \u201EXXX\u201C?"
>Expected (QString("Apply color scheme “XXX”?"))   
>  : "Apply color scheme \u201CXXX\u201D?"
>Loc: 
> [/home/kaiuwe/Projekte/kf5/src/frameworks/ki18n/autotests/klocalizedstringtest.cpp(351)]
> ```

Ok, there's something strange with those two failing tests, that has nothing to 
do with the current patch.

But then what is also strange is that with patched version you don't get a 
failure on that number format test. Maybe you don't have locale fr_FR.UTF-8 
generated on the system, and then it falls back to C locale.

Anyway, then just make that change in literals (" 4.20" to " 4,20"), and we 
call it good. (Even if then you should get failure on that test :)


- Chusslove


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127800/#review95056
---


On Мај 1, 2016, 12:47 пре п., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127800/
> ---
> 
> (Updated Мај 1, 2016, 12:47 пре п.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> Use %L1 instead of %1 which tells it it's a number.
> 
> This way I get decimal indicator and thousands separators according to my 
> locale in eg. KUnitConversion
> 
> 
> Diffs
> -
> 
>   src/klocalizedstring.cpp fc80135 
> 
> Diff: https://git.reviewboard.kde.org/r/127800/diff/
> 
> 
> Testing
> ---
> 
> 0,00094697205065430554 Meilen and 1.524 Millimeter
> instead of
> 0.00094697205065430554 Meilen and 1524 Millimeter
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127800: Format number in KLocalizedString::subs

[Chusslove Illich]

> On Мај 1, 2016, 6:40 по п., Chusslove Illich wrote:
> > To have localized numbers is the expected behavior (and documented as 
> > such). But it was "temporarily" disabled between kdelibs and KF5, until 
> > replacements for KLocale were decided. I don't remember why I didn't switch 
> > to %L1 at that moment, since it was available too... Well, I guess best to 
> > commit this and see later if there is any trouble.
> > 
> > Did you run unit tests? Could be some of them needs adapting.
> 
> Kai Uwe Broulik wrote:
> The unit tests fail (2 out of 4) locally both with and without this patch.

Hm, and which are these failing tests? When I run them, they all pass 
(including expected fails).

With the patch, I had to change in autotests/klocalizedstringtest.cpp one 
comparison literal from " 4.20" to " 4,20".


- Chusslove


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127800/#review95056
---


On Мај 1, 2016, 12:47 пре п., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127800/
> ---
> 
> (Updated Мај 1, 2016, 12:47 пре п.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> Use %L1 instead of %1 which tells it it's a number.
> 
> This way I get decimal indicator and thousands separators according to my 
> locale in eg. KUnitConversion
> 
> 
> Diffs
> -
> 
>   src/klocalizedstring.cpp fc80135 
> 
> Diff: https://git.reviewboard.kde.org/r/127800/diff/
> 
> 
> Testing
> ---
> 
> 0,00094697205065430554 Meilen and 1.524 Millimeter
> instead of
> 0.00094697205065430554 Meilen and 1524 Millimeter
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127800: Format number in KLocalizedString::subs

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127800/#review95056
---



To have localized numbers is the expected behavior (and documented as such). 
But it was "temporarily" disabled between kdelibs and KF5, until replacements 
for KLocale were decided. I don't remember why I didn't switch to %L1 at that 
moment, since it was available too... Well, I guess best to commit this and see 
later if there is any trouble.

Did you run unit tests? Could be some of them needs adapting.

- Chusslove Illich


On Мај 1, 2016, 12:47 пре п., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127800/
> ---
> 
> (Updated Мај 1, 2016, 12:47 пре п.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> Use %L1 instead of %1 which tells it it's a number.
> 
> This way I get decimal indicator and thousands separators according to my 
> locale in eg. KUnitConversion
> 
> 
> Diffs
> -
> 
>   src/klocalizedstring.cpp fc80135 
> 
> Diff: https://git.reviewboard.kde.org/r/127800/diff/
> 
> 
> Testing
> ---
> 
> 0,00094697205065430554 Meilen and 1.524 Millimeter
> instead of
> 0.00094697205065430554 Meilen and 1524 Millimeter
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127784: Add a small note to the documentation on how tips are i18n'ed

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127784/#review94998
---


Ship it!




Ship It!

- Chusslove Illich


On Април 28, 2016, 10:47 по п., Albert Astals Cid wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127784/
> ---
> 
> (Updated Април 28, 2016, 10:47 по п.)
> 
> 
> Review request for KDE Frameworks and Chusslove Illich.
> 
> 
> Repository: kconfigwidgets
> 
> 
> Description
> ---
> 
> See Summary.
> 
> 
> Diffs
> -
> 
>   src/ktipdialog.h b18ef96 
> 
> Diff: https://git.reviewboard.kde.org/r/127784/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Albert Astals Cid
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127777: Make KTipDialog use the domain of the application when translating the tips

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/12/#review94931
---


Ship it!




I wouldn't add domain parameter to showXTip, until someone asks for it. To me 
KTipDialog looks like something that is really going to be used directly by 
applications, and if someone does find a reason to wrap it into another 
library, can also ask for adding the domain parameter.

- Chusslove Illich


On Април 28, 2016, 12:48 пре п., Albert Astals Cid wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/12/
> ---
> 
> (Updated Април 28, 2016, 12:48 пре п.)
> 
> 
> Review request for KDE Frameworks and Chusslove Illich.
> 
> 
> Repository: kconfigwidgets
> 
> 
> Description
> ---
> 
> Makes no sense to use ::tr() here, we need to use i18n that is what the rest 
> of the framework (and hence the app) uses.
> 
> The biggest question is if we want to allow passing a domain as parameter to 
> showXTip in case someone has more esoteric use cases.
> 
> 
> Diffs
> -
> 
>   src/ktipdialog.cpp e6a70af 
> 
> Diff: https://git.reviewboard.kde.org/r/12/diff/
> 
> 
> Testing
> ---
> 
> kig tips now show translated.
> 
> 
> Thanks,
> 
> Albert Astals Cid
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Finding local translations?

[Chusslove Illich]

>> [: Boudewijn Rempt :]
>> However, KCatalog::catalogLocaleDir is run before the main runs, and uses
>> QStandardPaths to find the location of the translations. That's before
>> the XDG_DATA_DIRS environment variable is set.
>
> [: Alexander Potashev :]
> You can use a wrapper application that runs Krita with XDG_DATA_DIRS set
> to whatever appropriate. Of course this is dirty, and we should instead
> support custom paths to .mo files in KI18n.

I don't think there should be specific support in KI18n for setting custom
paths. What makes KI18n special compared to other code that uses
QStandardPaths?

Regarding KCatalog::catalogLocaleDir being called before main runs in Krita,
I don't know why that is happening. KCatalog::catalogLocaleDir should first
execute when the first translation call happens (or first
availableDomainTranslations/availableApplicationTranslations call). I
checked it now with KWrite, and that is what is happening. I could happily
munge XDG_DATA_DIRS before QApplication in KWrite is created, and KCatalog
would heed it.

-- 
Chusslove Illich (Часлав Илић)


signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127275: Ki18n: Fallback to QLocale::system uiLanguages in language initalisation

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127275/#review93265
---


Ship it!




Ship It!

- Chusslove Illich


On Март 7, 2016, 1:32 по п., Andre Heinecke wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127275/
> ---
> 
> (Updated Март 7, 2016, 1:32 по п.)
> 
> 
> Review request for KDE Frameworks and Localization and Translation (l10n).
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The intention for this patch is to fix the inital Language selection for 
> Windows where the environment variables used in Ki18n are not set.
> This is not a fix for a regression in Ki18n, afaik this never worked on 
> Windows, we had some hacks in Gpg4win to write the language into kdeglobals 
> during installation in kde4 times.
> 
> I don't think this needs to be ifdefed because it only appends so previous 
> language selection is not affected.
> 
> 
> Diffs
> -
> 
>   src/klocalizedstring.cpp b24fe9b 
> 
> Diff: https://git.reviewboard.kde.org/r/127275/diff/
> 
> 
> Testing
> ---
> 
> Tested on a german Windows system and got a "de" localized application.
> 
> 
> Thanks,
> 
> Andre Heinecke
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127275: Ki18n: Fallback to QLocale::system uiLanguages in language initalisation

[Chusslove Illich]

> On Март 4, 2016, 4:01 по п., Chusslove Illich wrote:
> > src/klocalizedstring.cpp, line 368
> > <https://git.reviewboard.kde.org/r/127275/diff/1/?file=447946#file447946line368>
> >
> > I would nevertheless ifdef it. It may be that some strange thing is 
> > intentionally done with locale variables (e.g. to test something or kill 
> > localization), and then the behavior would deviate from that documented for 
> > Gettext.
> 
> Andre Heinecke wrote:
> I'm assuming that QLocale::system() is also changed accodingly in that 
> case so I don't think its a problem. The code looks like it's using the same 
> environment variables: 
> http://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/tools/qlocale_unix.cpp#n104
> We still have to look at the environment variables in ki18n first because 
> of modifier / charset handling, imo otherwise we could just use QLocale here.
> 
> For ifdef I would probably do an #infdef O_OS_UNIX instead of a Q_OS_WIN 
> ? I'm not sure but I think this probably also makes sense on other platforms 
> like android?

Regardless of modifier/charset handling, we cannot use QLocale, because in 
QLocale that is an implementation detail (Qt does not use Gettext), and in 
ki18n that is a behavioral guarantee (conforming to Gettext).

I'm also not sure what kind of ifdef would be the best, although #infdef 
O_OS_UNIX does sound better than #ifdef Q_OS_WIN to me as well.


- Chusslove


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127275/#review93158
---


On Март 4, 2016, 3:53 по п., Andre Heinecke wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127275/
> ---
> 
> (Updated Март 4, 2016, 3:53 по п.)
> 
> 
> Review request for KDE Frameworks and Localization and Translation (l10n).
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The intention for this patch is to fix the inital Language selection for 
> Windows where the environment variables used in Ki18n are not set.
> This is not a fix for a regression in Ki18n, afaik this never worked on 
> Windows, we had some hacks in Gpg4win to write the language into kdeglobals 
> during installation in kde4 times.
> 
> I don't think this needs to be ifdefed because it only appends so previous 
> language selection is not affected.
> 
> 
> Diffs
> -
> 
>   src/klocalizedstring.cpp b24fe9b 
> 
> Diff: https://git.reviewboard.kde.org/r/127275/diff/
> 
> 
> Testing
> ---
> 
> Tested on a german Windows system and got a "de" localized application.
> 
> 
> Thanks,
> 
> Andre Heinecke
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127275: Ki18n: Fallback to QLocale::system uiLanguages in language initalisation

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127275/#review93158
---




src/klocalizedstring.cpp (line 349)
<https://git.reviewboard.kde.org/r/127275/#comment63522>

I would nevertheless ifdef it. It may be that some strange thing is 
intentionally done with locale variables (e.g. to test something or kill 
localization), and then the behavior would deviate from that documented for 
Gettext.


- Chusslove Illich


On Март 4, 2016, 3:53 по п., Andre Heinecke wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127275/
> ---
> 
> (Updated Март 4, 2016, 3:53 по п.)
> 
> 
> Review request for KDE Frameworks and Localization and Translation (l10n).
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The intention for this patch is to fix the inital Language selection for 
> Windows where the environment variables used in Ki18n are not set.
> This is not a fix for a regression in Ki18n, afaik this never worked on 
> Windows, we had some hacks in Gpg4win to write the language into kdeglobals 
> during installation in kde4 times.
> 
> I don't think this needs to be ifdefed because it only appends so previous 
> language selection is not affected.
> 
> 
> Diffs
> -
> 
>   src/klocalizedstring.cpp b24fe9b 
> 
> Diff: https://git.reviewboard.kde.org/r/127275/diff/
> 
> 
> Testing
> ---
> 
> Tested on a german Windows system and got a "de" localized application.
> 
> 
> Thanks,
> 
> Andre Heinecke
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127233: Skip looking up . and .. to find the translations for an application

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127233/#review92971
---


Ship it!




270 directories does seem a bit excessive to me... how do you count them, by 
strace or?

- Chusslove Illich


On Март 1, 2016, 3:01 пре п., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127233/
> ---
> 
> (Updated Март 1, 2016, 3:01 пре п.)
> 
> 
> Review request for KDE Frameworks and Chusslove Illich.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> Make sure we're looking where we need to.
> 
> 
> Diffs
> -
> 
>   src/kcatalog.cpp 62db19c 
> 
> Diff: https://git.reviewboard.kde.org/r/127233/diff/
> 
> 
> Testing
> ---
> 
> Builds, tests pass, applications don't access these on load. (but do access 
> another 270 directories on my system, btw)
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127048: Restrict _nl_msg_cat_cntr use to GNU gettext implentations.

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127048/#review92271
---


Ship it!




Ship It!

- Chusslove Illich


On Феб. 11, 2016, 8:11 по п., Andreas Cord-Landwehr wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127048/
> ---
> 
> (Updated Феб. 11, 2016, 8:11 по п.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Chusslove Illich.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> On Android, we do not have libintl but only libintl-lite, which does
> not provide _nl_msg_cat_cntr. Actually, _nl_msg_cat_cntr is a GNU
> gettext specific tool to prevent wrong localization after runtime language
> changes. Thus, it is tied to the specific GNU gettext implementation.
> 
> 
> Diffs
> -
> 
>   src/kcatalog.cpp 095a6beec721729537a89214a1fbdb661fbfb548 
> 
> Diff: https://git.reviewboard.kde.org/r/127048/diff/
> 
> 
> Testing
> ---
> 
> Tested compilation on Android and Linux.
> 
> 
> Thanks,
> 
> Andreas Cord-Landwehr
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127002: Add KLocalizedString::getLanguages()

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127002/#review92121
---


Ship it!




Ship It!

- Chusslove Illich


On Феб. 7, 2016, 9:28 пре п., Pino Toscano wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127002/
> ---
> 
> (Updated Феб. 7, 2016, 9:28 пре п.)
> 
> 
> Review request for KDE Frameworks and Chusslove Illich.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> Easy way to get the ordered list of languages where translations are looked 
> into.
> 
> This makes it possible to lookup other kind of translated resources using the 
> same list of priorities of languages.
> 
> (Note: I'm not sure about which name fits better, i.e. `languages()` or 
> `getLanguages()`; I chose the latter as the former could be mistaken as 
> non-static method of `KLocalizedString`.)
> 
> 
> Diffs
> -
> 
>   src/klocalizedstring.h 87030a443d36c3e9ca57e1e61ba1cba13f39bbee 
>   src/klocalizedstring.cpp 20b2bd46d6748b540e4a0bc9b3c3168e55cf47f3 
> 
> Diff: https://git.reviewboard.kde.org/r/127002/diff/
> 
> 
> Testing
> ---
> 
> Builds, tests pass.
> 
> 
> Thanks,
> 
> Pino Toscano
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126934: Reduce use of gettext API.

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126934/#review91899
---


Ship it!




Ship It!

- Chusslove Illich


On Феб. 1, 2016, 10:51 по п., Andreas Cord-Landwehr wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126934/
> ---
> 
> (Updated Феб. 1, 2016, 10:51 по п.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Chusslove Illich.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> This change removes usage of dcnpgettext and npgettext and by this
> allows building of ki18n against libintl-lite, which provides a
> viable libintl alternative on e.g. Android systems (which does not
> provide libintl). This change does not do any functional changes.
> The changed gettext.h header is only used internally.
> 
> 
> Diffs
> -
> 
>   cmake/FindLibIntl.cmake 887dbc7 
>   src/gettext.h 608f38a 
> 
> Diff: https://git.reviewboard.kde.org/r/126934/diff/
> 
> 
> Testing
> ---
> 
> Unit tests still pass.
> 
> 
> Thanks,
> 
> Andreas Cord-Landwehr
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126934: Reduce use of gettext API.

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126934/#review91826
---



I'm fine with this, I'm just thinking that the file as whole would look 
somewhat "disbalanced" if someone inspects it later. Could you maybe remove all 
uses of *cgettext API, so that the changes are symmetric for both dnpgettext* 
and dpgettext* functions/macros?

- Chusslove Illich


On Јан. 31, 2016, 9:53 пре п., Andreas Cord-Landwehr wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126934/
> ---
> 
> (Updated Јан. 31, 2016, 9:53 пре п.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Chusslove Illich.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> This change removes usage of dcnpgettext and npgettext and by this
> allows building of ki18n against libintl-lite, which provides a
> viable libintl alternative on e.g. Android systems (which does not
> provide libintl). This change does not do any functional changes.
> The changed gettext.h header is only used internally.
> 
> 
> Diffs
> -
> 
>   cmake/FindLibIntl.cmake 887dbc71354c856e7bf39147e5634bd39f09b326 
>   src/gettext.h 608f38aaf85d464b5314368d5af84948348823a8 
> 
> Diff: https://git.reviewboard.kde.org/r/126934/diff/
> 
> 
> Testing
> ---
> 
> Unit tests still pass.
> 
> 
> Thanks,
> 
> Andreas Cord-Landwehr
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126489: Reduce unwanted type changes

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126489/#review90005
---

Ship it!


Ship It!

- Chusslove Illich


On Дец. 23, 2015, 5:07 по п., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126489/
> ---
> 
> (Updated Дец. 23, 2015, 5:07 по п.)
> 
> 
> Review request for KDE Frameworks and Chusslove Illich.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> Makes sure we don't cast string types on some occasions.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt a4bd8d9 
>   src/klocalizedstring.cpp ac14bd5 
>   src/kuitmarkup.h bb2f65d 
>   src/kuitmarkup.cpp 64db024 
> 
> Diff: https://git.reviewboard.kde.org/r/126489/diff/
> 
> 
> Testing
> ---
> 
> Tests still pass.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126476: Make it possible to use doubles as index for i18np*() calls

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126476/#review90002
---

Ship it!


Ship It!

- Chusslove Illich


On Дец. 23, 2015, 4:03 по п., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126476/
> ---
> 
> (Updated Дец. 23, 2015, 4:03 по п.)
> 
> 
> Review request for KDE Frameworks and Chusslove Illich.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> Also considers double values to check for plurals.
> 
> In QML it's hard to ensure a value is an integer and we'd get unreliable 
> results, e.g. `i18nd("%1 thing", "%1 things", 3-i)`.
> 
> Also it creates a separate function to reuse some code.
> 
> 
> Diffs
> -
> 
>   autotests/ki18ndeclarativetest.cpp cf8e778 
>   autotests/test.qml 314b516 
>   src/klocalizedcontext.h c322e33 
>   src/klocalizedcontext.cpp 3bc42dd 
>   src/klocalizedstring.cpp 69950d2 
> 
> Diff: https://git.reviewboard.kde.org/r/126476/diff/
> 
> 
> Testing
> ---
> 
> Added new tests, which now pass.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126476: Make it possible to use doubles as index for i18np*() calls

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126476/#review8
---



src/klocalizedstring.cpp (line 1128)
<https://git.reviewboard.kde.org/r/126476/#comment61680>

Missing std::abs.


- Chusslove Illich


On Дец. 23, 2015, 3:41 по п., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126476/
> ---
> 
> (Updated Дец. 23, 2015, 3:41 по п.)
> 
> 
> Review request for KDE Frameworks and Chusslove Illich.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> Also considers double values to check for plurals.
> 
> In QML it's hard to ensure a value is an integer and we'd get unreliable 
> results, e.g. `i18nd("%1 thing", "%1 things", 3-i)`.
> 
> Also it creates a separate function to reuse some code.
> 
> 
> Diffs
> -
> 
>   autotests/ki18ndeclarativetest.cpp cf8e778 
>   autotests/test.qml 314b516 
>   src/klocalizedcontext.h c322e33 
>   src/klocalizedcontext.cpp 3bc42dd 
>   src/klocalizedstring.cpp 69950d2 
> 
> Diff: https://git.reviewboard.kde.org/r/126476/diff/
> 
> 
> Testing
> ---
> 
> Added new tests, which now pass.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126476: Make it possible to use doubles as index for i18np*() calls

[Chusslove Illich]

> On Дец. 23, 2015, 8:24 пре п., Alexander Potashev wrote:
> > Test question: should it be "1.5 other windows" or "1.5 other window"?
> > 
> > The code in this patch seems to truncate the double to an integer and then 
> > use the appropriate plural form for the integer. But, for example, in 
> > Russian "5.4 billion" is read as "five and four tens of a billion" where 
> > instead of "of a billion" we actually put "billion" in genitive case. Thus 
> > for fractions the proper plural form will always be the same, independent 
> > of their values.

The way I understood, the problem here is that an argument that was intended to 
be an int can be converted "unexpectedly" to double in QML, where "unexpected" 
is in the sense of less predictably/differently/unavoidably compared to C++. 
E.g. instead of an intended value of 42 the methods get 42.0. This is the case 
that the patch is trying to handle, and not to introduce plurals for 
intentional doubles.

But, of course, if a programmer misunderstands the intended usage of plurals, 
this patch would also make intentional doubles be used for plurals. So I look 
at it as the lesser of two evils, enabling wrong use of plurals vs. not being 
able to use plurals at all.


- Chusslove


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126476/#review89968
---


On Дец. 23, 2015, 1:53 по п., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126476/
> -------
> 
> (Updated Дец. 23, 2015, 1:53 по п.)
> 
> 
> Review request for KDE Frameworks and Chusslove Illich.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> Also considers double values to check for plurals.
> 
> In QML it's hard to ensure a value is an integer and we'd get unreliable 
> results, e.g. `i18nd("%1 thing", "%1 things", 3-i)`.
> 
> Also it creates a separate function to reuse some code.
> 
> 
> Diffs
> -
> 
>   autotests/ki18ndeclarativetest.cpp cf8e778 
>   autotests/test.qml 314b516 
>   src/klocalizedcontext.cpp 3bc42dd 
>   src/klocalizedstring.cpp 69950d2 
> 
> Diff: https://git.reviewboard.kde.org/r/126476/diff/
> 
> 
> Testing
> ---
> 
> Added new tests, which now pass.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126476: Make it possible to use doubles as index for i18np*() calls

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126476/#review89981
---



src/klocalizedstring.cpp (line 231)
<https://git.reviewboard.kde.org/r/126476/#comment61673>

Why is kls being passed here? The method is supposed to act on its own 
members.


The second version is much better, since it does not change the behavior of the 
non-QML plural handling.

It should be stated somewhere in documentation that when using plural calls in 
QML, the first argument must be the plural-deciding number. Best in the part 
where it currently mentions that the first integer-typed argument is 
plural-deciding.

- Chusslove Illich


On Дец. 23, 2015, 1:53 по п., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126476/
> ---
> 
> (Updated Дец. 23, 2015, 1:53 по п.)
> 
> 
> Review request for KDE Frameworks and Chusslove Illich.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> Also considers double values to check for plurals.
> 
> In QML it's hard to ensure a value is an integer and we'd get unreliable 
> results, e.g. `i18nd("%1 thing", "%1 things", 3-i)`.
> 
> Also it creates a separate function to reuse some code.
> 
> 
> Diffs
> -
> 
>   autotests/ki18ndeclarativetest.cpp cf8e778 
>   autotests/test.qml 314b516 
>   src/klocalizedcontext.cpp 3bc42dd 
>   src/klocalizedstring.cpp 69950d2 
> 
> Diff: https://git.reviewboard.kde.org/r/126476/diff/
> 
> 
> Testing
> ---
> 
> Added new tests, which now pass.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: about ki18n/locales: installing only a subset?

[Chusslove Illich]

> [: René J.V. Bertin :]
> [...] AFAIK it's always been a habit among distributions to package
> languages separately.

Which distributions? Not the ones I use, Debian and Suse. I know only of
Ubuntu doing it.

>> [: Chusslove Illich :]
>> Furthermore, if one thinks of stripping the translation files from
>> release packages, or fetching them manually from the repo, that will not
>> work properly.
>
> [: René J.V. Bertin :]
> It worked just fine with KDE4, esp. with the fallback feature to a
> language that isn't automatically the developer's language (usually
> en_US).

I meant whoever thinks doing that on his own. In KDE4 it worked because the
language packages were officially provided.

And it only worked in the sense that all files were accounted for. In
practice I had only frustration with language packages, having to pay
attention to install them as well when provided separately. The disk space
overhead is preferable to having to think about it, and highly preferable to
bothering any admins about it.

-- 
Chusslove Illich (Часлав Илић)


signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: about ki18n/locales: installing only a subset?

[Chusslove Illich]

>> [: Chusslove Illich :]
>> It has a use in every situation where multiple-language users work in a
>> centrally administered network. Like in... an office.
>
> [: René J.V. Bertin :]
> How many offices do you know that require all known languages to be
> installed? The only organisations I can think of where that situation
> might be approached are the UN and (to a much lesser extent) EU
> administrations.

I'm not talking about offices requiring it. I'm talking about coming to a
random office in the world and finding my language already available,
without trifling anyone about it.

>> To put it in perspective, a single contemporary high-budget game will
>> occupy much more disk blocks than all Gettext translations on the system
>> combined.
>
> You haven't read (or understood) my argument, which is about how blocks
> are used, not how many.

I read it, but apparently I didn't understand it. Would you explain it
further, or point me to an explanation?

-- 
Chusslove Illich (Часлав Илић)


signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: about ki18n/locales: installing only a subset?

[Chusslove Illich]

> [: René J.V. Bertin :]
> It seems that with KF5 we have gotten back in the situation where you get
> every possible language installed. Now that may be nice for the occasional
> office prank [...] something that has absolutely no use for the vast
> majority of users.

It has a use in every situation where multiple-language users work in a
centrally administered network. Like in... an office. My personal experience
in this regard is that whenever I sat at some machine of that type, I get
translated all software *but* the KDE software. And the unacceptable
overhead in that case is asking IT to do something about it.

> [...] but it'll end up amounting to a significant disk overhead (the one
> that comes with lots of small files) [...]

I don't think the disk overhead is significant. There are no loud complains
about other Gettext-using software, which normally comes with all
translations. To put it in perspective, a single contemporary high-budget
game will occupy much more disk blocks than all Gettext translations on the
system combined.

Furthermore, if one thinks of stripping the translation files from release
packages, or fetching them manually from the repo, that will not work
properly. There will be versioning mismatches, and some files will be
omitted, as PO files are not the only translation-related files. So this
should be done only with a much stronger reason than disk usage.

-- 
Chusslove Illich (Часлав Илић)


signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126230: Make python, gettext and Qt5::QML optional for ki18n

[Chusslove Illich]

> On Дец. 4, 2015, 10:35 пре п., Chusslove Illich wrote:
> > The KI18N_INSTALL macro is also used by KI18n itself, to install its own 
> > translations. And so do other higher-tier frameworks. So I'm not sure when 
> > this no-Gettext/no-Python build is supposed to be useful. When one wants to 
> > omit translations?
> 
> Boudewijn Rempt wrote:
> I _never_ install translations. Where would I install them from? Running 
> "make install" in ki18n doesn't install translations, as far as I can see? I 
> thought translations get installed by the distribution, or I package them 
> manually for Windows and OSX. 
> 
> So, when I build Krita on OSX or Windows, I first need to build the 
> dependencies, and ki18n is one of them, but because it mixes up translation 
> management with providing the translation framework an application uses. So 
> why should I build Python and Gettext and QtQML for parts of this framework 
> that never get executed?
> 
> Chusslove Illich wrote:
> For me "make" does build translations using msgfmt, and builds some other 
> translation-related files using a Python script, and "make install" installs 
> those built files.
> 
> Something is definitely going wrong if QtQML is seen as dependency. What 
> is needed is QtScript, and that is a real dependency, used at runtime by 
> translations. It can be disabled, but that will cause degradation in quality 
> of some translations.
> 
> Chusslove Illich wrote:
> Hm, where do you fetch the frameworks to build from? If from the Git 
> repositories, there are no translations there. But the release tarballs do 
> contain translations.
> 
> Only Applications are released with separate translation packages. (Which 
> I think is not a good thing, but hell...)
> 
> Boudewijn Rempt wrote:
> Always from git.
> 
> Chusslove Illich wrote:
> If you are fetching and installing translations manually by taking only 
> PO files, that will leave any scripted translations not working. Nothing will 
> crash, but lower quality fallbacks will be used. If you want to support also 
> scripted translations, you should fetch and install any modules from script/ 
> subdirectories of translations in the repository.
> 
> Regarding this patch, I don't see it as appropriate, since it would 
> basically allow for broken treatment of translations when building release 
> tarballs. Better just add it to be applied in your build setup. I know, not 
> nice when something changes and the patch needs to be updated, but it's the 
> less bad alternative.
> 
> Aleix Pol Gonzalez wrote:
> @Chusslove, that might make it harder for people to port applications to 
> Windows though. (Or Android for that matter)
> 
> Maybe we can find an in-between compromise solution? In the end building 
> translations is something that only happens when you're about to release.
> 
> Kåre Särs wrote:
> I did a compile Kate on Windows exercise this summer and fall (hoping I 
> will get to the installer soon :) I was expecting _some_ hurdles but I was 
> shocked about how hard it was to compile KDE Frameworks 5 on Windows without 
> using emerge. This basically means that big parts of frameworks just was not 
> a realistic alternative for third-party Windows applications. With changes 
> that have flown in during this fall the building of frameworks on Windows has 
> become _much_ easier and is soon a real alternative for Windows Qt projects.
> 
> The gettext and python dependencies are probably the biggest hurdles. My 
> build script uses a trick inherited from emerge that just downloads a 
> pre-built binary package for the gettext tools and I had to rename some of 
> the include directories found in the Python installation because they clashed 
> with the rest of the build. Requiring python and downloading random packages 
> from SF is probably not something that any of my colleges at work would like 
> to add to their Qt projects.
> 
> Translations are really important and needs to be taken care of. I would 
> like it to be possible to split out the utilities somehow so that I could 
> compile the application without installing the utilities needed for the 
> compilation. The parts requiring Python/Gettext would be like CMake a 
> separate tool and not like Autotools a part of the package.
> 
> @Chusslove: This patch only disables the tools. What would it take to 
> split the tools out into a separate build?
> 
> Chusslove Illich wrote:
> If you don't have Gettext tools and Python, then you cannot properly 
> build translation files, of KI18n and other packages using it, no m

Re: Review Request 126230: Make python, gettext and Qt5::QML optional for ki18n

[Chusslove Illich]

> On Дец. 4, 2015, 10:35 пре п., Chusslove Illich wrote:
> > The KI18N_INSTALL macro is also used by KI18n itself, to install its own 
> > translations. And so do other higher-tier frameworks. So I'm not sure when 
> > this no-Gettext/no-Python build is supposed to be useful. When one wants to 
> > omit translations?
> 
> Boudewijn Rempt wrote:
> I _never_ install translations. Where would I install them from? Running 
> "make install" in ki18n doesn't install translations, as far as I can see? I 
> thought translations get installed by the distribution, or I package them 
> manually for Windows and OSX. 
> 
> So, when I build Krita on OSX or Windows, I first need to build the 
> dependencies, and ki18n is one of them, but because it mixes up translation 
> management with providing the translation framework an application uses. So 
> why should I build Python and Gettext and QtQML for parts of this framework 
> that never get executed?
> 
> Chusslove Illich wrote:
> For me "make" does build translations using msgfmt, and builds some other 
> translation-related files using a Python script, and "make install" installs 
> those built files.
> 
> Something is definitely going wrong if QtQML is seen as dependency. What 
> is needed is QtScript, and that is a real dependency, used at runtime by 
> translations. It can be disabled, but that will cause degradation in quality 
> of some translations.
> 
> Chusslove Illich wrote:
> Hm, where do you fetch the frameworks to build from? If from the Git 
> repositories, there are no translations there. But the release tarballs do 
> contain translations.
> 
> Only Applications are released with separate translation packages. (Which 
> I think is not a good thing, but hell...)
> 
> Boudewijn Rempt wrote:
> Always from git.
> 
> Chusslove Illich wrote:
> If you are fetching and installing translations manually by taking only 
> PO files, that will leave any scripted translations not working. Nothing will 
> crash, but lower quality fallbacks will be used. If you want to support also 
> scripted translations, you should fetch and install any modules from script/ 
> subdirectories of translations in the repository.
> 
> Regarding this patch, I don't see it as appropriate, since it would 
> basically allow for broken treatment of translations when building release 
> tarballs. Better just add it to be applied in your build setup. I know, not 
> nice when something changes and the patch needs to be updated, but it's the 
> less bad alternative.
> 
> Aleix Pol Gonzalez wrote:
> @Chusslove, that might make it harder for people to port applications to 
> Windows though. (Or Android for that matter)
> 
> Maybe we can find an in-between compromise solution? In the end building 
> translations is something that only happens when you're about to release.
> 
> Kåre Särs wrote:
> I did a compile Kate on Windows exercise this summer and fall (hoping I 
> will get to the installer soon :) I was expecting _some_ hurdles but I was 
> shocked about how hard it was to compile KDE Frameworks 5 on Windows without 
> using emerge. This basically means that big parts of frameworks just was not 
> a realistic alternative for third-party Windows applications. With changes 
> that have flown in during this fall the building of frameworks on Windows has 
> become _much_ easier and is soon a real alternative for Windows Qt projects.
> 
> The gettext and python dependencies are probably the biggest hurdles. My 
> build script uses a trick inherited from emerge that just downloads a 
> pre-built binary package for the gettext tools and I had to rename some of 
> the include directories found in the Python installation because they clashed 
> with the rest of the build. Requiring python and downloading random packages 
> from SF is probably not something that any of my colleges at work would like 
> to add to their Qt projects.
> 
> Translations are really important and needs to be taken care of. I would 
> like it to be possible to split out the utilities somehow so that I could 
> compile the application without installing the utilities needed for the 
> compilation. The parts requiring Python/Gettext would be like CMake a 
> separate tool and not like Autotools a part of the package.
> 
> @Chusslove: This patch only disables the tools. What would it take to 
> split the tools out into a separate build?
> 
> Chusslove Illich wrote:
> If you don't have Gettext tools and Python, then you cannot properly 
> build translation files, of KI18n and other packages using it, no m

Re: Review Request 126230: Make python, gettext and Qt5::QML optional for ki18n

[Chusslove Illich]

> On Дец. 4, 2015, 10:35 пре п., Chusslove Illich wrote:
> > The KI18N_INSTALL macro is also used by KI18n itself, to install its own 
> > translations. And so do other higher-tier frameworks. So I'm not sure when 
> > this no-Gettext/no-Python build is supposed to be useful. When one wants to 
> > omit translations?
> 
> Boudewijn Rempt wrote:
> I _never_ install translations. Where would I install them from? Running 
> "make install" in ki18n doesn't install translations, as far as I can see? I 
> thought translations get installed by the distribution, or I package them 
> manually for Windows and OSX. 
> 
> So, when I build Krita on OSX or Windows, I first need to build the 
> dependencies, and ki18n is one of them, but because it mixes up translation 
> management with providing the translation framework an application uses. So 
> why should I build Python and Gettext and QtQML for parts of this framework 
> that never get executed?
> 
> Chusslove Illich wrote:
> For me "make" does build translations using msgfmt, and builds some other 
> translation-related files using a Python script, and "make install" installs 
> those built files.
> 
> Something is definitely going wrong if QtQML is seen as dependency. What 
> is needed is QtScript, and that is a real dependency, used at runtime by 
> translations. It can be disabled, but that will cause degradation in quality 
> of some translations.
> 
> Chusslove Illich wrote:
> Hm, where do you fetch the frameworks to build from? If from the Git 
> repositories, there are no translations there. But the release tarballs do 
> contain translations.
> 
> Only Applications are released with separate translation packages. (Which 
> I think is not a good thing, but hell...)
> 
> Boudewijn Rempt wrote:
> Always from git.
> 
> Chusslove Illich wrote:
> If you are fetching and installing translations manually by taking only 
> PO files, that will leave any scripted translations not working. Nothing will 
> crash, but lower quality fallbacks will be used. If you want to support also 
> scripted translations, you should fetch and install any modules from script/ 
> subdirectories of translations in the repository.
> 
> Regarding this patch, I don't see it as appropriate, since it would 
> basically allow for broken treatment of translations when building release 
> tarballs. Better just add it to be applied in your build setup. I know, not 
> nice when something changes and the patch needs to be updated, but it's the 
> less bad alternative.
> 
> Aleix Pol Gonzalez wrote:
> @Chusslove, that might make it harder for people to port applications to 
> Windows though. (Or Android for that matter)
> 
> Maybe we can find an in-between compromise solution? In the end building 
> translations is something that only happens when you're about to release.
> 
> Kåre Särs wrote:
> I did a compile Kate on Windows exercise this summer and fall (hoping I 
> will get to the installer soon :) I was expecting _some_ hurdles but I was 
> shocked about how hard it was to compile KDE Frameworks 5 on Windows without 
> using emerge. This basically means that big parts of frameworks just was not 
> a realistic alternative for third-party Windows applications. With changes 
> that have flown in during this fall the building of frameworks on Windows has 
> become _much_ easier and is soon a real alternative for Windows Qt projects.
> 
> The gettext and python dependencies are probably the biggest hurdles. My 
> build script uses a trick inherited from emerge that just downloads a 
> pre-built binary package for the gettext tools and I had to rename some of 
> the include directories found in the Python installation because they clashed 
> with the rest of the build. Requiring python and downloading random packages 
> from SF is probably not something that any of my colleges at work would like 
> to add to their Qt projects.
> 
> Translations are really important and needs to be taken care of. I would 
> like it to be possible to split out the utilities somehow so that I could 
> compile the application without installing the utilities needed for the 
> compilation. The parts requiring Python/Gettext would be like CMake a 
> separate tool and not like Autotools a part of the package.
> 
> @Chusslove: This patch only disables the tools. What would it take to 
> split the tools out into a separate build?

If you don't have Gettext tools and Python, then you cannot properly build 
translation files, of KI18n and other packages using it, no matter how you do 
it.

But I don

Re: Review Request 126230: Make python, gettext and Qt5::QML optional for ki18n

[Chusslove Illich]

> On Дец. 4, 2015, 10:35 пре п., Chusslove Illich wrote:
> > The KI18N_INSTALL macro is also used by KI18n itself, to install its own 
> > translations. And so do other higher-tier frameworks. So I'm not sure when 
> > this no-Gettext/no-Python build is supposed to be useful. When one wants to 
> > omit translations?
> 
> Boudewijn Rempt wrote:
> I _never_ install translations. Where would I install them from? Running 
> "make install" in ki18n doesn't install translations, as far as I can see? I 
> thought translations get installed by the distribution, or I package them 
> manually for Windows and OSX. 
> 
> So, when I build Krita on OSX or Windows, I first need to build the 
> dependencies, and ki18n is one of them, but because it mixes up translation 
> management with providing the translation framework an application uses. So 
> why should I build Python and Gettext and QtQML for parts of this framework 
> that never get executed?
> 
> Chusslove Illich wrote:
> For me "make" does build translations using msgfmt, and builds some other 
> translation-related files using a Python script, and "make install" installs 
> those built files.
> 
> Something is definitely going wrong if QtQML is seen as dependency. What 
> is needed is QtScript, and that is a real dependency, used at runtime by 
> translations. It can be disabled, but that will cause degradation in quality 
> of some translations.
> 
> Chusslove Illich wrote:
> Hm, where do you fetch the frameworks to build from? If from the Git 
> repositories, there are no translations there. But the release tarballs do 
> contain translations.
> 
> Only Applications are released with separate translation packages. (Which 
> I think is not a good thing, but hell...)
> 
> Boudewijn Rempt wrote:
> Always from git.

If you are fetching and installing translations manually by taking only PO 
files, that will leave any scripted translations not working. Nothing will 
crash, but lower quality fallbacks will be used. If you want to support also 
scripted translations, you should fetch and install any modules from script/ 
subdirectories of translations in the repository.

Regarding this patch, I don't see it as appropriate, since it would basically 
allow for broken treatment of translations when building release tarballs. 
Better just add it to be applied in your build setup. I know, not nice when 
something changes and the patch needs to be updated, but it's the less bad 
alternative.


- Chusslove


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126230/#review89109
---


On Дец. 4, 2015, 1:17 пре п., Boudewijn Rempt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126230/
> ---
> 
> (Updated Дец. 4, 2015, 1:17 пре п.)
> 
> 
> Review request for Build System, KDE Frameworks, Aleix Pol Gonzalez, 
> Chusslove Illich, and Luigi Toscano.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> When building ki18n as a dependency framework for shipping with an 
> application, the tools to actually create and manage translations are 
> superfluous. These tools have some big dependencies that are a pain to have 
> around, especially gettext on Windows. This patch makes the requirement for 
> Python and Gettext optional.
> 
> This patch checks the BUILD_TESTING variable to see if the autotests should 
> be built, because when we just need the library to build an application we 
> shouldn't waste electicity building the tests (and the Qt5::QML dependency).
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 59917fa 
>   cmake/KF5I18NMacros.cmake 53ba812 
> 
> Diff: https://git.reviewboard.kde.org/r/126230/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Boudewijn Rempt
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126230: Make python, gettext and Qt5::QML optional for ki18n

[Chusslove Illich]

> On Дец. 4, 2015, 10:35 пре п., Chusslove Illich wrote:
> > The KI18N_INSTALL macro is also used by KI18n itself, to install its own 
> > translations. And so do other higher-tier frameworks. So I'm not sure when 
> > this no-Gettext/no-Python build is supposed to be useful. When one wants to 
> > omit translations?
> 
> Boudewijn Rempt wrote:
> I _never_ install translations. Where would I install them from? Running 
> "make install" in ki18n doesn't install translations, as far as I can see? I 
> thought translations get installed by the distribution, or I package them 
> manually for Windows and OSX. 
> 
> So, when I build Krita on OSX or Windows, I first need to build the 
> dependencies, and ki18n is one of them, but because it mixes up translation 
> management with providing the translation framework an application uses. So 
> why should I build Python and Gettext and QtQML for parts of this framework 
> that never get executed?
> 
> Chusslove Illich wrote:
> For me "make" does build translations using msgfmt, and builds some other 
> translation-related files using a Python script, and "make install" installs 
> those built files.
> 
> Something is definitely going wrong if QtQML is seen as dependency. What 
> is needed is QtScript, and that is a real dependency, used at runtime by 
> translations. It can be disabled, but that will cause degradation in quality 
> of some translations.

Hm, where do you fetch the frameworks to build from? If from the Git 
repositories, there are no translations there. But the release tarballs do 
contain translations.

Only Applications are released with separate translation packages. (Which I 
think is not a good thing, but hell...)


- Chusslove


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126230/#review89109
---


On Дец. 4, 2015, 1:17 пре п., Boudewijn Rempt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126230/
> -------
> 
> (Updated Дец. 4, 2015, 1:17 пре п.)
> 
> 
> Review request for Build System, KDE Frameworks, Aleix Pol Gonzalez, 
> Chusslove Illich, and Luigi Toscano.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> When building ki18n as a dependency framework for shipping with an 
> application, the tools to actually create and manage translations are 
> superfluous. These tools have some big dependencies that are a pain to have 
> around, especially gettext on Windows. This patch makes the requirement for 
> Python and Gettext optional.
> 
> This patch checks the BUILD_TESTING variable to see if the autotests should 
> be built, because when we just need the library to build an application we 
> shouldn't waste electicity building the tests (and the Qt5::QML dependency).
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 59917fa 
>   cmake/KF5I18NMacros.cmake 53ba812 
> 
> Diff: https://git.reviewboard.kde.org/r/126230/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Boudewijn Rempt
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126230: Make python, gettext and Qt5::QML optional for ki18n

[Chusslove Illich]

> On Дец. 4, 2015, 10:35 пре п., Chusslove Illich wrote:
> > The KI18N_INSTALL macro is also used by KI18n itself, to install its own 
> > translations. And so do other higher-tier frameworks. So I'm not sure when 
> > this no-Gettext/no-Python build is supposed to be useful. When one wants to 
> > omit translations?
> 
> Boudewijn Rempt wrote:
> I _never_ install translations. Where would I install them from? Running 
> "make install" in ki18n doesn't install translations, as far as I can see? I 
> thought translations get installed by the distribution, or I package them 
> manually for Windows and OSX. 
> 
> So, when I build Krita on OSX or Windows, I first need to build the 
> dependencies, and ki18n is one of them, but because it mixes up translation 
> management with providing the translation framework an application uses. So 
> why should I build Python and Gettext and QtQML for parts of this framework 
> that never get executed?

For me "make" does build translations using msgfmt, and builds some other 
translation-related files using a Python script, and "make install" installs 
those built files.

Something is definitely going wrong if QtQML is seen as dependency. What is 
needed is QtScript, and that is a real dependency, used at runtime by 
translations. It can be disabled, but that will cause degradation in quality of 
some translations.


- Chusslove


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126230/#review89109
---


On Дец. 4, 2015, 1:17 пре п., Boudewijn Rempt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126230/
> ---
> 
> (Updated Дец. 4, 2015, 1:17 пре п.)
> 
> 
> Review request for Build System, KDE Frameworks, Aleix Pol Gonzalez, 
> Chusslove Illich, and Luigi Toscano.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> When building ki18n as a dependency framework for shipping with an 
> application, the tools to actually create and manage translations are 
> superfluous. These tools have some big dependencies that are a pain to have 
> around, especially gettext on Windows. This patch makes the requirement for 
> Python and Gettext optional.
> 
> This patch checks the BUILD_TESTING variable to see if the autotests should 
> be built, because when we just need the library to build an application we 
> shouldn't waste electicity building the tests (and the Qt5::QML dependency).
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 59917fa 
>   cmake/KF5I18NMacros.cmake 53ba812 
> 
> Diff: https://git.reviewboard.kde.org/r/126230/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Boudewijn Rempt
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126230: Make python, gettext and Qt5::QML optional for ki18n

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126230/#review89109
---


The KI18N_INSTALL macro is also used by KI18n itself, to install its own 
translations. And so do other higher-tier frameworks. So I'm not sure when this 
no-Gettext/no-Python build is supposed to be useful. When one wants to omit 
translations?

- Chusslove Illich


On Дец. 4, 2015, 1:17 пре п., Boudewijn Rempt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126230/
> ---
> 
> (Updated Дец. 4, 2015, 1:17 пре п.)
> 
> 
> Review request for Build System, KDE Frameworks, Aleix Pol Gonzalez, 
> Chusslove Illich, and Luigi Toscano.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> When building ki18n as a dependency framework for shipping with an 
> application, the tools to actually create and manage translations are 
> superfluous. These tools have some big dependencies that are a pain to have 
> around, especially gettext on Windows. This patch makes the requirement for 
> Python and Gettext optional.
> 
> This patch checks the BUILD_TESTING variable to see if the autotests should 
> be built, because when we just need the library to build an application we 
> shouldn't waste electicity building the tests (and the Qt5::QML dependency).
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 59917fa 
>   cmake/KF5I18NMacros.cmake 53ba812 
> 
> Diff: https://git.reviewboard.kde.org/r/126230/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Boudewijn Rempt
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126152: Fly away from QString::isNull() in favor of isEmpty()

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126152/#review88755
---

Ship it!


Ship It!

- Chusslove Illich


On Нов. 24, 2015, 12:57 пре п., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126152/
> ---
> 
> (Updated Нов. 24, 2015, 12:57 пре п.)
> 
> 
> Review request for KDE Frameworks and Chusslove Illich.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> They don't work and it's a pointless distinction really.
> 
> 
> Diffs
> -
> 
>   src/klocalizedcontext.cpp 2b6bcf6 
> 
> Diff: https://git.reviewboard.kde.org/r/126152/diff/
> 
> 
> Testing
> ---
> 
> Fixes the test. Actually I started getting the same crash as build.kde.org as 
> soon as I started adding qDebugs...
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126087: Move the i18n context from KDeclarative

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126087/#review88721
---

Ship it!


Ship It!

- Chusslove Illich


On Нов. 23, 2015, 1:52 по п., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Нов. 23, 2015, 1:52 по п.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 1cf0f7a 
>   src/klocalizedcontext.h PRE-CREATION 
>   src/CMakeLists.txt 818595e 
>   docs/programmers-guide.md 13a5f9d 
>   autotests/ki18ndeclarativetest.cpp PRE-CREATION 
>   autotests/test.qml PRE-CREATION 
>   src/klocalizedcontext.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126087: Move the i18n context from KDeclarative

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126087/#review88717
---



src/klocalizedcontext.cpp (line 76)
<https://git.reviewboard.kde.org/r/126087/#comment60832>

Style wise, shouldn't this conditional also use braces?



src/klocalizedcontext.cpp (line 79)
<https://git.reviewboard.kde.org/r/126087/#comment60831>

If this happens, all subsequent substitutions will use placeholders shifted 
back by one, so the message will be messed up more than necessary. Better 
substitute something, e.g. "???".


- Chusslove Illich


On Нов. 23, 2015, 12:54 по п., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Нов. 23, 2015, 12:54 по п.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 1cf0f7a 
>   autotests/ki18ndeclarativetest.cpp PRE-CREATION 
>   autotests/test.qml PRE-CREATION 
>   docs/programmers-guide.md 13a5f9d 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
>   src/klocalizedcontext.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126087: Move the i18n context from KDeclarative

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126087/#review88648
---



src/klocalizedcontext.h (line 28)
<https://git.reviewboard.kde.org/r/126087/#comment60765>

...of KI18n framework in QML?



src/klocalizedcontext.cpp (line 87)
<https://git.reviewboard.kde.org/r/126087/#comment60766>

How are numbers converted to strings, before being passed here? Because 
when it receives actual numeric types, KI18n will take care to represent them 
in locale-specific way. Also it will let translators (through "translation 
scripting" feature) operate on numbers, e.g. to handle multi-plural cases.

Is it possible to get back to actual numbers, or the originating data type 
is irrevocably lost? If not, at least we should document this mungling 
somewhere.



src/klocalizedcontext.cpp (line 145)
<https://git.reviewboard.kde.org/r/126087/#comment60767>

KI18n plural semantics is to take as plural deciding the first 
integer-valued argument, which needs not be the first argument. So this should 
be a loop over all arguments, until first successful conversion. (Of course, 
better if the originating numeric types can be recovered.)


- Chusslove Illich


On Нов. 19, 2015, 5 по п., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Нов. 19, 2015, 5 по п.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   autotests/CMakeLists.txt 1cf0f7a 
>   autotests/ki18ndeclarativetest.cpp PRE-CREATION 
>   autotests/test.qml PRE-CREATION 
>   docs/programmers-guide.md 13a5f9d 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
>   src/klocalizedcontext.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126087: Move the i18n context from KDeclarative

[Chusslove Illich]

> On Нов. 17, 2015, 9:43 пре п., Chusslove Illich wrote:
> > Could you also document the usage in docs/programmers-guide.md (section 
> > #link_cat)? I'm not much into QML, so it would help me understand the 
> > implications of the usage.
> > 
> > It seems to me other three series of calls (xi18n*, ki18n*, kxi18n*) should 
> > be made available as well.
> 
> Aleix Pol Gonzalez wrote:
> I don't think we want the `k*i18n*` version of the calls, as they return 
> KLocalizedString and we probably don't need to deal with it from QML.
> 
> Regarding `xi18n*`, I see that we have counter-parts for every of the 
> `i18n*` calls. Could you explain a bit what does it do differently? Is it to 
> support the xml-like markup?

KLocalizedString exposes the lower-level interface, and more possible argument 
combinations, so in principle k*i18n* can be useful even in one-liners ending 
with .toString(). Is there any particular reason not to have it?

Yes, xi18n* are for ki18n's own XML markup. So if someone likes to use xi18n* 
generally in the whole program, he should want to use it in QML files as well.


- Chusslove


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126087/#review88459
---


On Нов. 16, 2015, 3:55 по п., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> -------
> 
> (Updated Нов. 16, 2015, 3:55 по п.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   src/klocalizedcontext.cpp PRE-CREATION 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126087: Move the i18n context from KDeclarative

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126087/#review88459
---


Could you also document the usage in docs/programmers-guide.md (section 
#link_cat)? I'm not much into QML, so it would help me understand the 
implications of the usage.

It seems to me other three series of calls (xi18n*, ki18n*, kxi18n*) should be 
made available as well.

- Chusslove Illich


On Нов. 16, 2015, 3:55 по п., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126087/
> ---
> 
> (Updated Нов. 16, 2015, 3:55 по п.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Marco Martin.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> The only way to use `i18n*()` so far in QML is by depending on all 
> KDeclarative. This patch moves the necessary bits there so it can be adopted 
> by an application or framework just by offering the needed bits within KI18n.
> This is done by offering an object with methods that map to the `i18n*()` C++ 
> counter-parts.
> 
> 
> Diffs
> -
> 
>   src/klocalizedcontext.cpp PRE-CREATION 
>   src/CMakeLists.txt 818595e 
>   src/klocalizedcontext.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126087/diff/
> 
> 
> Testing
> ---
> 
> Ported KDeclarative, everything still seems to work.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 125695: Make KAboutData::translators/setTranslators simple

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125695/#review87086
---

Ship it!


Technically I see no other solution. Because tier 1, it should not depend on 
anything Gettext related. And because there is no such special message 
convention in Qt, makes little sense to use Qt::tr().

- Chusslove Illich


On Окт. 19, 2015, 12:38 пре п., Albert Astals Cid wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125695/
> ---
> 
> (Updated Окт. 19, 2015, 12:38 пре п.)
> 
> 
> Review request for KDE Frameworks, Chusslove Illich and Michael Pyne.
> 
> 
> Repository: kcoreaddons
> 
> 
> Description
> ---
> 
> Move the responsability to the user.
> 
> Document how to implement them either using ki18n and document that 
> KMainWindow does it for you.
> 
> 
> Diffs
> -
> 
>   src/lib/kaboutdata.h a94e2c5 
>   src/lib/kaboutdata.cpp af10fc6 
> 
> Diff: https://git.reviewboard.kde.org/r/125695/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Albert Astals Cid
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 125682: Pre-fill translator information for KAboutApplicationDialog

[Chusslove Illich]

> On Окт. 18, 2015, 10:44 пре п., Albert Astals Cid wrote:
> > src/kmainwindow.cpp, line 209
> > 
> >
> > This is not going to work, you need to undefine TRANSLATION_DOMAIN 
> > include klocalizedstring again and then call this i18nc so it gets it from 
> > the "global" (i.e. the app) catalog and not from the kxmlgui5 catalog

Should work if instead the domain-explicit call with NULL domain is used, as 
that means to look in the application catalog. Like i18ndc(NULL, "NAME OF 
TRANSLATORS", "Your names").


- Chusslove


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125682/#review86998
---


On Окт. 18, 2015, 4:13 пре п., Michael Pyne wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125682/
> ---
> 
> (Updated Окт. 18, 2015, 4:13 пре п.)
> 
> 
> Review request for KDE Frameworks, Localization and Translation (l10n) and 
> Albert Astals Cid.
> 
> 
> Bugs: 345320
> https://bugs.kde.org/show_bug.cgi?id=345320
> 
> 
> Repository: kxmlgui
> 
> 
> Description
> ---
> 
> KAboutData has a placeholder for information regarding who translated the 
> running KDE-based application (KAboutData::translators()). However it relies 
> on the application developer to call KAboutData::setTranslator() since 
> KCoreAddons (a Tier 1 framework) cannot use KI18n directly. Instead the Qt 
> translation infrastructure is used where translations are unavoidable, but 
> that is not compatible with KF5's i18n. This (IIUC) gives different behavior 
> than KDE4, where KAboutData could (and did!) directly pull the translated 
> list of translators, so application authors didn't have to do anything 
> special for their about dialog to have the list of translators.
> 
> For the majority of our applications we can make the ::setTranslator() call 
> on the application developer's behalf, as recommended by Albert on 
> kdeframeworks-devel, and by doing so from KMainWindow (a relatively central 
> location for KF5-using applications) we can use KI18n and get the 
> accurately-translated message. Feeding the already-translated information 
> into KAboutData should work to form the list of translators for later use by 
> KAboutApplicationDialog, or other accessors of that information.
> 
> This patch implements all that. I avoided using a global startup method as 
> suggested by Albert (since the Qt docs suggest that this startup would happen 
> *before* the GUI starts up, and I want to make sure KI18n is available); 
> KMainWindow seems the next best option, and even non-KXmlGuiWindow users 
> might use this. There are probably other good alternatives too (maybe even 
> the platform plugin?).
> 
> 
> Diffs
> -
> 
>   src/kmainwindow.cpp 7c86841 
> 
> Diff: https://git.reviewboard.kde.org/r/125682/diff/
> 
> 
> Testing
> ---
> 
> Compiled, apps all still work.
> 
> I find it hard to test whether the translators actually show up or not 
> though, as I do not use translated KF5 apps.
> 
> 
> Thanks,
> 
> Michael Pyne
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 123872: Add TranslationDomain attribute to kconfig_compiler

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123872/
---

(Updated June 22, 2015, 1:13 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, Alexander Potashev and Matthew Dawson.


Changes
---

Submitted with commit 2fff683110385047bd20fb5f0b68a008b48320dd by Chusslove 
Illich (?? ) to branch master.


Repository: kconfig


Description
---

When using Ki18n as the translation system, library .kcfg files also need to 
specify the translation domain. This is analogous to the TRANSLATION_DOMAIN 
define in C++ code, and translationDomain attribute in .rc files.


Diffs
-

  autotests/kconfig_compiler/CMakeLists.txt f73aec0 
  autotests/kconfig_compiler/kconfigcompiler_test.cpp 77a31a3 
  autotests/kconfig_compiler/klocalizedstring.h e69de29 
  autotests/kconfig_compiler/test_translation.kcfg e69de29 
  autotests/kconfig_compiler/test_translation_kde.cpp.ref e69de29 
  autotests/kconfig_compiler/test_translation_kde.h.ref e69de29 
  autotests/kconfig_compiler/test_translation_kde.kcfgc e69de29 
  autotests/kconfig_compiler/test_translation_kde_domain.cpp.ref e69de29 
  autotests/kconfig_compiler/test_translation_kde_domain.h.ref e69de29 
  autotests/kconfig_compiler/test_translation_kde_domain.kcfgc e69de29 
  autotests/kconfig_compiler/test_translation_kde_domain_main.cpp e69de29 
  autotests/kconfig_compiler/test_translation_kde_main.cpp e69de29 
  autotests/kconfig_compiler/test_translation_qt.cpp.ref e69de29 
  autotests/kconfig_compiler/test_translation_qt.h.ref e69de29 
  autotests/kconfig_compiler/test_translation_qt.kcfgc e69de29 
  autotests/kconfig_compiler/test_translation_qt_main.cpp e69de29 
  src/kconfig_compiler/kconfig_compiler.cpp 7160bb5 

Diff: https://git.reviewboard.kde.org/r/123872/diff/


Testing
---

Compiles.


Thanks,

Chusslove Illich

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 123872: Add TranslationDomain attribute to kconfig_compiler

[Chusslove Illich]

> On Јун 6, 2015, 10:21 пре п., David Faure wrote:
> > A test doesn't really need ki18n, it could just define its own i18n() 
> > function.
> > 
> > Alternatively, you can put the test in a higher level framework, like 
> > frameworkintegration.

I took the first approach, by adding a fake klocalizedstring.h in the test 
directory.


- Chusslove


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123872/#review81259
---


On Јун 21, 2015, 10:47 по п., Chusslove Illich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123872/
> ---
> 
> (Updated Јун 21, 2015, 10:47 по п.)
> 
> 
> Review request for KDE Frameworks, Alexander Potashev and Matthew Dawson.
> 
> 
> Repository: kconfig
> 
> 
> Description
> ---
> 
> When using Ki18n as the translation system, library .kcfg files also need to 
> specify the translation domain. This is analogous to the TRANSLATION_DOMAIN 
> define in C++ code, and translationDomain attribute in .rc files.
> 
> 
> Diffs
> -
> 
>   autotests/kconfig_compiler/CMakeLists.txt f73aec0 
>   autotests/kconfig_compiler/kconfigcompiler_test.cpp 77a31a3 
>   autotests/kconfig_compiler/klocalizedstring.h e69de29 
>   autotests/kconfig_compiler/test_translation.kcfg e69de29 
>   autotests/kconfig_compiler/test_translation_kde.cpp.ref e69de29 
>   autotests/kconfig_compiler/test_translation_kde.h.ref e69de29 
>   autotests/kconfig_compiler/test_translation_kde.kcfgc e69de29 
>   autotests/kconfig_compiler/test_translation_kde_domain.cpp.ref e69de29 
>   autotests/kconfig_compiler/test_translation_kde_domain.h.ref e69de29 
>   autotests/kconfig_compiler/test_translation_kde_domain.kcfgc e69de29 
>   autotests/kconfig_compiler/test_translation_kde_domain_main.cpp e69de29 
>   autotests/kconfig_compiler/test_translation_kde_main.cpp e69de29 
>   autotests/kconfig_compiler/test_translation_qt.cpp.ref e69de29 
>   autotests/kconfig_compiler/test_translation_qt.h.ref e69de29 
>   autotests/kconfig_compiler/test_translation_qt.kcfgc e69de29 
>   autotests/kconfig_compiler/test_translation_qt_main.cpp e69de29 
>   src/kconfig_compiler/kconfig_compiler.cpp 7160bb5 
> 
> Diff: https://git.reviewboard.kde.org/r/123872/diff/
> 
> 
> Testing
> ---
> 
> Compiles.
> 
> 
> Thanks,
> 
> Chusslove Illich
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 123872: Add TranslationDomain attribute to kconfig_compiler

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123872/
---

(Updated Јун 21, 2015, 10:47 по п.)


Review request for KDE Frameworks, Alexander Potashev and Matthew Dawson.


Changes
---

Added unit tests.


Repository: kconfig


Description
---

When using Ki18n as the translation system, library .kcfg files also need to 
specify the translation domain. This is analogous to the TRANSLATION_DOMAIN 
define in C++ code, and translationDomain attribute in .rc files.


Diffs (updated)
-

  autotests/kconfig_compiler/CMakeLists.txt f73aec0 
  autotests/kconfig_compiler/kconfigcompiler_test.cpp 77a31a3 
  autotests/kconfig_compiler/klocalizedstring.h e69de29 
  autotests/kconfig_compiler/test_translation.kcfg e69de29 
  autotests/kconfig_compiler/test_translation_kde.cpp.ref e69de29 
  autotests/kconfig_compiler/test_translation_kde.h.ref e69de29 
  autotests/kconfig_compiler/test_translation_kde.kcfgc e69de29 
  autotests/kconfig_compiler/test_translation_kde_domain.cpp.ref e69de29 
  autotests/kconfig_compiler/test_translation_kde_domain.h.ref e69de29 
  autotests/kconfig_compiler/test_translation_kde_domain.kcfgc e69de29 
  autotests/kconfig_compiler/test_translation_kde_domain_main.cpp e69de29 
  autotests/kconfig_compiler/test_translation_kde_main.cpp e69de29 
  autotests/kconfig_compiler/test_translation_qt.cpp.ref e69de29 
  autotests/kconfig_compiler/test_translation_qt.h.ref e69de29 
  autotests/kconfig_compiler/test_translation_qt.kcfgc e69de29 
  autotests/kconfig_compiler/test_translation_qt_main.cpp e69de29 
  src/kconfig_compiler/kconfig_compiler.cpp 7160bb5 

Diff: https://git.reviewboard.kde.org/r/123872/diff/


Testing
---

Compiles.


Thanks,

Chusslove Illich

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 123872: Add TranslationDomain attribute to kconfig_compiler

[Chusslove Illich]

> On Мај 21, 2015, 5:29 по п., Matthew Dawson wrote:
> > LGTM in general, but could you please add/update a unit test to test all 
> > the possible cominbations of the translation?

That would make the test depend on the Ki18n framework, which is not acceptable 
for tier 1 frameworks.

Alternatively, one could add fake klocalizedstring.h header, to shadow any 
system header, and fill it with fake translation calls. But such test would 
only check if generated translation call names are as expected, which is 
trivially obvious from the code.


- Chusslove


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123872/#review80698
---


On Мај 21, 2015, 5:21 по п., Chusslove Illich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123872/
> ---
> 
> (Updated Мај 21, 2015, 5:21 по п.)
> 
> 
> Review request for KDE Frameworks, Alexander Potashev and Matthew Dawson.
> 
> 
> Repository: kconfig
> 
> 
> Description
> ---
> 
> When using Ki18n as the translation system, library .kcfg files also need to 
> specify the translation domain. This is analogous to the TRANSLATION_DOMAIN 
> define in C++ code, and translationDomain attribute in .rc files.
> 
> 
> Diffs
> -
> 
>   src/kconfig_compiler/kconfig_compiler.cpp 7160bb5 
> 
> Diff: https://git.reviewboard.kde.org/r/123872/diff/
> 
> 
> Testing
> ---
> 
> Compiles.
> 
> 
> Thanks,
> 
> Chusslove Illich
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 123872: Add TranslationDomain attribute to kconfig_compiler

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123872/
---

Review request for KDE Frameworks, Alexander Potashev and Matthew Dawson.


Repository: kconfig


Description
---

When using Ki18n as the translation system, library .kcfg files also need to 
specify the translation domain. This is analogous to the TRANSLATION_DOMAIN 
define in C++ code, and translationDomain attribute in .rc files.


Diffs
-

  src/kconfig_compiler/kconfig_compiler.cpp 7160bb5 

Diff: https://git.reviewboard.kde.org/r/123872/diff/


Testing
---

Compiles.


Thanks,

Chusslove Illich

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 123453: ki18n: Remove mention of language skipping

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123453/#review79331
---

Ship it!


Ship It!

- Chusslove Illich


On Април 21, 2015, 9:25 по п., Lasse Liehu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123453/
> ---
> 
> (Updated Април 21, 2015, 9:25 по п.)
> 
> 
> Review request for KDE Frameworks and Chusslove Illich.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> This removes mention of a feature that no longer exists.
> Since commit 9f0e0e428dbf0626660a911fdb76d7544ea8beac in April 2014 Ki18n has 
> not skipped a language if application domain is not translated for that 
> language.
> 
> 
> Diffs
> -
> 
>   docs/programmers-guide.md 66629dc 
> 
> Diff: https://git.reviewboard.kde.org/r/123453/diff/
> 
> 
> Testing
> ---
> 
> None.
> 
> 
> Thanks,
> 
> Lasse Liehu
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 122937: Preserve translation domain when merging KXmlGui files

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122937/
---

(Updated March 18, 2015, 12:30 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, David Faure and Lasse Liehu.


Changes
---

Submitted with commit 2920e946c24d78eb0b6b3038a9cc38a8897ce4ac by Chusslove 
Illich (?? ) to branch master.


Repository: kxmlgui


Description
---

When two .rc documents that specify different translation domains are merged, 
only one of the two domains will appear in the merged document. This breaks 
translation of text elements under the dropped domain. To fix it, the top 
domain attribute (or the application domain) should be propagated to each text 
element before merging. Then, at the moment of translation, the local domain 
attribute should take priority over the top attribute (or the application 
domain).

This fix also works if the merged document is written out and later loaded. 
Hence the local translation domain attribute is added to schemas as well.

Reference bug report: https://bugs.kde.org/show_bug.cgi?id=342976


Diffs
-

  src/kedittoolbar.cpp 40c8bd6 
  src/kpartgui.dtd 587cd8e 
  src/ktoolbar.cpp f79a149 
  src/kxmlgui.xsd bca02f1 
  src/kxmlguibuilder.cpp d4cfa7a 
  src/kxmlguiclient.cpp 12d3f44 

Diff: https://git.reviewboard.kde.org/r/122937/diff/


Testing
---

Tested an application that previously had an untranslated menu title due to 
this issue, now it is translated.


Thanks,

Chusslove Illich

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 122937: Preserve translation domain when merging KXmlGui files

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122937/
---

Review request for KDE Frameworks, David Faure and Lasse Liehu.


Repository: kxmlgui


Description
---

When two .rc documents that specify different translation domains are merged, 
only one of the two domains will appear in the merged document. This breaks 
translation of text elements under the dropped domain. To fix it, the top 
domain attribute (or the application domain) should be propagated to each text 
element before merging. Then, at the moment of translation, the local domain 
attribute should take priority over the top attribute (or the application 
domain).

This fix also works if the merged document is written out and later loaded. 
Hence the local translation domain attribute is added to schemas as well.

Reference bug report: https://bugs.kde.org/show_bug.cgi?id=342976


Diffs
-

  src/kedittoolbar.cpp 40c8bd6 
  src/kpartgui.dtd 587cd8e 
  src/ktoolbar.cpp f79a149 
  src/kxmlgui.xsd bca02f1 
  src/kxmlguibuilder.cpp d4cfa7a 
  src/kxmlguiclient.cpp 12d3f44 

Diff: https://git.reviewboard.kde.org/r/122937/diff/


Testing
---

Tested an application that previously had an untranslated menu title due to 
this issue, now it is translated.


Thanks,

Chusslove Illich

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 122754: Remove use of KI18N_MODULE_DIR.

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122754/#review76794
---

Ship it!


Checked also that _ki18n_pmap_compile_script still works.

- Chusslove Illich


On Феб. 28, 2015, 2:52 по п., Alex Merry wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122754/
> ---
> 
> (Updated Феб. 28, 2015, 2:52 по п.)
> 
> 
> Review request for KDE Frameworks and Chusslove Illich.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> This variable (and the corresponding if/else in KF5I18NMacros) is
> unnecessary, as the scripts needed by KF5I18NMacros.cmake are always in
> the same directory as KF5I18NMacros.cmake.
> 
> We keep the variable, albeit set in a simpler way, in case anything else
> is using it (by the CMake naming convention, a variable with no leading
> underscore is public).
> 
> 
> Diffs
> -
> 
>   KF5I18nConfig.cmake.in 64dc5009e4821f9e1bb3631c596b56e66c578c3c 
>   cmake/KF5I18NMacros.cmake a55f472bdfdebd2e353064f102c1f1372fdf0918 
> 
> Diff: https://git.reviewboard.kde.org/r/122754/diff/
> 
> 
> Testing
> ---
> 
> Built, run `make test`. Built and installed ktexteditor (uses ki18n_wrap_ui).
> 
> 
> Thanks,
> 
> Alex Merry
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 122204: Mark results as required.

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122204/#review74632
---

Ship it!


Ship It!

- Chusslove Illich


On Јан. 22, 2015, 7:34 по п., Milian Wolff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122204/
> ---
> 
> (Updated Јан. 22, 2015, 7:34 по п.)
> 
> 
> Review request for KDE Frameworks and Chusslove Illich.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> This prevents API misuage, similar to how QString::arg is doing it.
> 
> See also:
> http://quickgit.kde.org/?p=kdevplatform.git&a=commit&h=078f1cd584e2de75ad19c46282b47dd1e0feff8f
> 
> 
> Diffs
> -
> 
>   src/klocalizedstring.h 8e8e84926b444150e00c80b3b77766ba16e03e25 
> 
> Diff: https://git.reviewboard.kde.org/r/122204/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Milian Wolff
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 122039: ki18n: add BUILD_WITH_QTSCRIPT option.

[Chusslove Illich]

> On Јан. 13, 2015, 10:30 по п., Albert Astals Cid wrote:
> > would it be possible to show some warning if compiled without qt script but 
> > the translation asks for it? or that is architerually hard?

The Transcript plugin is internally loaded lazily, when the first scripted 
translation is seen. If the plugin was not compiled, this warning is shown:

  qWarning() << QString::fromLatin1("Cannot find Transcript plugin.");

But it too could be compiled-away.


- Chusslove


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122039/#review73969
---


On Јан. 13, 2015, 4:08 по п., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122039/
> ---
> 
> (Updated Јан. 13, 2015, 4:08 по п.)
> 
> 
> Review request for KDE Frameworks and Chusslove Illich.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> Use case: to use KIO core in an embedded device, having to embed
> a javascript engine just for translations is overkill.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 2fbed024f392fefcb96dc8d3f6e421463280536c 
>   autotests/CMakeLists.txt f57af342217a2481cfa4dde1fd312b9c9d67ba48 
>   src/CMakeLists.txt ed5c789952372c2fcd3a8ca1b78ccb4ec2672840 
> 
> Diff: https://git.reviewboard.kde.org/r/122039/diff/
> 
> 
> Testing
> ---
> 
> Building with and without qtscript support works; ktranscript and its 
> unittests are not built with the option is OFF.
> 
> 
> Thanks,
> 
> David Faure
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 122039: ki18n: add BUILD_WITH_QTSCRIPT option.

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122039/#review73955
---

Ship it!


Ship It!

- Chusslove Illich


On Јан. 13, 2015, 4:08 по п., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122039/
> ---
> 
> (Updated Јан. 13, 2015, 4:08 по п.)
> 
> 
> Review request for KDE Frameworks and Chusslove Illich.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> Use case: to use KIO core in an embedded device, having to embed
> a javascript engine just for translations is overkill.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 2fbed024f392fefcb96dc8d3f6e421463280536c 
>   autotests/CMakeLists.txt f57af342217a2481cfa4dde1fd312b9c9d67ba48 
>   src/CMakeLists.txt ed5c789952372c2fcd3a8ca1b78ccb4ec2672840 
> 
> Diff: https://git.reviewboard.kde.org/r/122039/diff/
> 
> 
> Testing
> ---
> 
> Building with and without qtscript support works; ktranscript and its 
> unittests are not built with the option is OFF.
> 
> 
> Thanks,
> 
> David Faure
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Translations in frameworks released tarballs

[Chusslove Illich]

> [: Christoph Feck :]
> with the 5.2 release, the bug that unneeded translations are packaged into
> each tarball is back in a different form. Now, a "scripts" folder is
> inside each po/ subfolder, with redundant translations.
>
> Does not happen on every archive, but on many of them.

Happens on packages that have foo5_* catalog name. My bad, fix attached.

-- 
Chusslove Illich (Часлав Илић)
From c7e9a79f337cf8eb65a46fe66cfb189bf1bba376 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Chusslove=20Illich=20=28=D0=A7=D0=B0=D1=81=D0=BB=D0=B0?=
 =?UTF-8?q?=D0=B2=20=D0=98=D0=BB=D0=B8=D1=9B=29?= 
Date: Sun, 14 Sep 2014 17:57:00 +0200
Subject: [PATCH] Fix wrong reference to PO dir instead of scripts dir

This was causing erroneous packaging of PO file into scripts for
foo5_* named translation domains.
---
 make_rc_tag.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/make_rc_tag.sh b/make_rc_tag.sh
index 96c8b09..5fe2760 100755
--- a/make_rc_tag.sh
+++ b/make_rc_tag.sh
@@ -43,7 +43,7 @@ function grabTranslations()
 local scriptdir=$subdir/scripts/$l10n_module
 rm -rf $destdir/scripts
 mkdir $destdir/scripts
-cp -rf $scriptdir/${repo}5 $destdir/scripts/ 2>/dev/null || cp -rf $podir/${repo}5_* $destdir/scripts/ 2>/dev/null || rmdir $destdir/scripts
+cp -rf $scriptdir/${repo}5 $destdir/scripts/ 2>/dev/null || cp -rf $scriptdir/${repo}5_* $destdir/scripts/ 2>/dev/null || rmdir $destdir/scripts
 rm -rf $destdir/scripts/${repo}5/.svn
 elif [ $hasdestdir -eq 0 ]; then
 rm -r $destdir
-- 
2.0.0



signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Translations are broken in KF5 apps

[Chusslove Illich]

>> [: Chusslove Illich :]
>> To note additionally, packaging of translations for Frameworks is still
>> not resolved (to my knowledge?), so translations are actually not
>> released right now.
>
> [: Albert Astals Cid :]
> I see the translations in the tarballs. Why do you say they are not
> released?

My knowledge was lacking.

-- 
Chusslove Illich (Часлав Илић)


signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Translations are broken in KF5 apps

[Chusslove Illich]

> [: Alexander Potashev :]
> What a non-English user can see is, many menu items still appear in
> English while they have translations in *.mo files installed [...]

One thing I know is still not being translated, because I didn't implement
it yet, is menu names coming from *.rc files. This also holds for menu items
which are submenu names. This should cover some of the cases you have
observed. (Another such thing is stuff coming from *.kcfg files, though this
is certainly not related to menus.) I'll see to get this done, ehm, soon.

To note additionally, packaging of translations for Frameworks is still not
resolved (to my knowledge?), so translations are actually not released right
now.

-- 
Chusslove Illich (Часлав Илић)


signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 118692: Fix reading of entries for language/country combinations

[Chusslove Illich]

> On June 12, 2014, 1:58 p.m., David Faure wrote:
> > Looks good; but I'm surprised that it works :) the code for using fallbacks 
> > must be in there somewhere already then? (if de_DE not present then use 
> > de). 
> > This patch is only about not skipping these two entries (de and de_CH), but 
> > how will it then pick the right one? Could it be that it works by chance, 
> > because the first one wins or something? (where first could mean "in the 
> > file" or worse "in some data structure"...)
> >
> 
> Martin Gräßlin wrote:
> I think it works because it's the order in the file. I do not know 
> whether there are rules on how the order has to be in the file, I simply 
> assumed common sense which would list "de" before "de_DE" or "de_CH".
> 
> From reading the code I didn't find anything which looked like a 
> fallback. It looked to me like a found value is overwritten when a translated 
> comes in. But I might have missed something.
> 
> David Faure wrote:
> I don't trust common sense, but "scripty" orders them indeed. Just 
> possibly not people writing desktop files by hand (e.g. distros?)... I guess 
> the "desktop entry spec" doesn't specify ordering.
> 
> Martin Gräßlin wrote:
> the spec only specifies the parsing, but not the writing. In fact the 
> only example is the other way around:
> 
>  Name=Foo
>  Name[sr_YU]=...
>  Name[sr@Latn]=...
>  Name[sr]=...
> 
> Chusslove Illich wrote:
> So... support for handling the language codes listed by David is simply 
> gone in KF5, as it stands? More precisely, if the user has set glibc locale 
> to sr_RS@latin, and a .desktop file contains Name[sr]= and Name[sr@latin]= 
> (in any order), what will be read by the current implementation?
>
> 
> Martin Gräßlin wrote:
> @latin gets removed by QLocale, so it would pick Name[sr]

This means that localization for the said locales will be very obviously 
broken, given the prominence of strings from .desktop files.


- Chusslove


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118692/#review59869
---


On June 16, 2014, 10:26 a.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118692/
> ---
> 
> (Updated June 16, 2014, 10:26 a.m.)
> 
> 
> Review request for KDE Frameworks, David Faure, John Layt, and Oswald 
> Buddenhagen.
> 
> 
> Repository: kconfig
> 
> 
> Description
> ---
> 
> Fix reading of entries for language/country combinations
> 
> This fixes a regression introduced in
> 988f09bb051dca0437ecec431ee44ed5b4a560d8.
> 
> The mentioned commit ensures that if the locale is e.g. "de_DE" the
> entry "de" will be used. But this breaks if there is a translation
> for another country. E.g. for "de_CH" it would also pick the "de"
> entry.
> 
> This change now operates on both just the language code and the locale.
> If an entry with the language code is present it will be picked. If
> another entry with the exact locale is found it will be overwritten.
> 
> 
> Diffs
> -
> 
>   autotests/kdesktopfiletest.cpp 6c3af4c2cd55b9140c0cd48526947431ceb7e798 
>   src/core/kconfig.cpp a2598f8e8fce91a8de3f34b272e15a6c280a50db 
>   src/core/kconfigdata.h fdec85dc90467560bd312b72266ef0b3f243d076 
>   src/core/kconfigdata.cpp 109063d65e97bcb7ba08cf6e5a6f3acb885fe845 
>   src/core/kconfigini.cpp a882ee31150658f3e5cfb036362ff0583f71cbd9 
> 
> Diff: https://git.reviewboard.kde.org/r/118692/diff/
> 
> 
> Testing
> ---
> 
> unit tests still pass, but my knowledge about KConfig and locales is not 
> sufficient to be sure that this is now 100 % correct.
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 118692: Fix reading of entries for language/country combinations

[Chusslove Illich]

> On June 12, 2014, 1:58 p.m., David Faure wrote:
> > Looks good; but I'm surprised that it works :) the code for using fallbacks 
> > must be in there somewhere already then? (if de_DE not present then use 
> > de). 
> > This patch is only about not skipping these two entries (de and de_CH), but 
> > how will it then pick the right one? Could it be that it works by chance, 
> > because the first one wins or something? (where first could mean "in the 
> > file" or worse "in some data structure"...)
> >
> 
> Martin Gräßlin wrote:
> I think it works because it's the order in the file. I do not know 
> whether there are rules on how the order has to be in the file, I simply 
> assumed common sense which would list "de" before "de_DE" or "de_CH".
> 
> From reading the code I didn't find anything which looked like a 
> fallback. It looked to me like a found value is overwritten when a translated 
> comes in. But I might have missed something.
> 
> David Faure wrote:
> I don't trust common sense, but "scripty" orders them indeed. Just 
> possibly not people writing desktop files by hand (e.g. distros?)... I guess 
> the "desktop entry spec" doesn't specify ordering.
> 
> Martin Gräßlin wrote:
> the spec only specifies the parsing, but not the writing. In fact the 
> only example is the other way around:
> 
>  Name=Foo
>  Name[sr_YU]=...
>  Name[sr@Latn]=...
>  Name[sr]=...

So... support for handling the language codes listed by David is simply gone in 
KF5, as it stands? More precisely, if the user has set glibc locale to 
sr_RS@latin, and a .desktop file contains Name[sr]= and Name[sr@latin]= (in any 
order), what will be read by the current implementation?


- Chusslove


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118692/#review59869
---


On June 16, 2014, 10:26 a.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118692/
> ---
> 
> (Updated June 16, 2014, 10:26 a.m.)
> 
> 
> Review request for KDE Frameworks, David Faure, John Layt, and Oswald 
> Buddenhagen.
> 
> 
> Repository: kconfig
> 
> 
> Description
> ---
> 
> Fix reading of entries for language/country combinations
> 
> This fixes a regression introduced in
> 988f09bb051dca0437ecec431ee44ed5b4a560d8.
> 
> The mentioned commit ensures that if the locale is e.g. "de_DE" the
> entry "de" will be used. But this breaks if there is a translation
> for another country. E.g. for "de_CH" it would also pick the "de"
> entry.
> 
> This change now operates on both just the language code and the locale.
> If an entry with the language code is present it will be picked. If
> another entry with the exact locale is found it will be overwritten.
> 
> 
> Diffs
> -
> 
>   autotests/kdesktopfiletest.cpp 6c3af4c2cd55b9140c0cd48526947431ceb7e798 
>   src/core/kconfig.cpp a2598f8e8fce91a8de3f34b272e15a6c280a50db 
>   src/core/kconfigdata.h fdec85dc90467560bd312b72266ef0b3f243d076 
>   src/core/kconfigdata.cpp 109063d65e97bcb7ba08cf6e5a6f3acb885fe845 
>   src/core/kconfigini.cpp a882ee31150658f3e5cfb036362ff0583f71cbd9 
> 
> Diff: https://git.reviewboard.kde.org/r/118692/diff/
> 
> 
> Testing
> ---
> 
> unit tests still pass, but my knowledge about KConfig and locales is not 
> sufficient to be sure that this is now 100 % correct.
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 118690: Introduce a KLocalizedTranslator

[Chusslove Illich]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118690/#review60357
---

Ship it!


Ship It!

- Chusslove Illich


On June 12, 2014, 10:55 a.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118690/
> ---
> 
> (Updated June 12, 2014, 10:55 a.m.)
> 
> 
> Review request for KDE Frameworks, Aurélien Gâteau and Chusslove Illich.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> Introduce a KLocalizedTranslator
> 
> The KLocalizedTranslator is a QTranslator subclass which can delegate
> to ki18n for translations. This class allows to translate text which
> can only use Qt's translation system with ki18n.
> 
> 
> Diffs
> -
> 
>   autotests/klocalizedstringtest.h 52b6248270e7d117855cb96c5d3d8582d89a99b0 
>   autotests/klocalizedstringtest.cpp e4e97ce92224f8886705c39091a5f07ea3a209d1 
>   src/CMakeLists.txt 09490e54ecd2126873482b187b17642c57ca7d28 
>   src/klocalizedtranslator.h PRE-CREATION 
>   src/klocalizedtranslator.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/118690/diff/
> 
> 
> Testing
> ---
> 
> see added unit test and class already used in (and implemented for) KWin.
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel