Lightweight API for localization of images, icons, etc.
faure at kde.org
Tue Jan 15 16:43:37 GMT 2008
On Tuesday 15 January 2008, Chusslove Illich wrote:
> > [: David Faure :]
> > OK, but the real question is: are we sure that this code couldn't use
> > KStandardDirs instead? :-) I am not convinced about adding redundant API
> > just because application code doesn't do things right, you see...
> It's the matter of, so to say, "what is older". I expect the need for
> localized resources to pop up rather rarely, and on language-by-language
> basis. I imagine that it would then be burdensome for a translator to
> coordinate with the programmer rewriting segments of code, just to
> accomodate one particular language needing one particular file localized.
> With a standalone method, it's just the matter of locating one line of
> interest and adding the wrap, with no further modifications. And to someone
> refactoring the code later, the intent is made obvious, so as not to mess it
> up in the process.
I said nonsense, sorry; I had missed the fact that your patch actually makes KStandardDirs
call i18nFilePath and not the other way round.
My concern becomes quite different then: performance. Every call to KStandardDirs::findResource()
(i.e. every call to locate()) is going to try and file the file in N language directories.... this seems like
a rather important slow down of things. The API might be "lightweight", but the implementation is definitely not :-)
With this in mind, I would much rather see a solution where applications need to ask
explicitely for a localizable file. Not every call to findResource()/locate() needs to look into
all language dirs, far from it.... Grepping a bit I found a few examples:
io/kdebug.cpp:244: QString filename(KStandardDirs::locate("config", QLatin1String("kdebug.areas")));
kernel/kautostart.cpp:127: return QFile::exists(KStandardDirs::locate("autostart", entryName + ".desktop"));
tests/kstandarddirstest.cpp:171: KGlobal::dirs()->locate( "exe", "kdeinit4" ) );
tests/kstandarddirstest.cpp:173: KGlobal::dirs()->locate( "exe", "lnusertemp" ) );
It doesn't make sense for any of those. I think it's dependent on the resource type; we'll never have
per-language executables or libraries. I think it's mostly something for the "data" resource, since the
one case I found where it would make sense was: f = KStandardDirs::locate("data", "LICENSES/LGPL_V2");
In fact I think the best solution would be a flag passed to locate(), to ask it to look in per-lang directories.
The i18nFilePath solution could be used in combination with KStandardDirs (first use KStandardDirs to
find the non-localized version of the file and then use i18nFilePath to look for locale-specific versions),
but this isn't as nice as having it inside KStandardDirs so that locale-specific versions can also be
found in more specific directories than the global one, e.g. $KDEDIR/share/apps/kfoo/myfile and
~/.kde/share/apps/kfoo/l10n/fr/myfile. Which I would find with
KStandardDirs::locate("data", "kfoo/myfile", KStandardDirs::CheckForLocalizedVersion) or
KStandardDirs::locate("appdata", "myfile", KStandardDirs::CheckForLocalizedVersion);
BTW the code in KLocale::localizedFilePath could start by checking that the l10n dir exists,
to skip the whole loop if it doesn't (common case); this will at least help when d->languageList
has more than one language.
Same thing for the iconloader, I am very afraid of the performance hit there. And I don't think
that <full-path-to-dir-of-icon>/l10n/fr/iconname is even compatible with the icon spec?
Language-specific icons should be done at a level higher IMHO, like in UserIcon() maybe,
if changing the application code is ok. Otherwise, hmm, we better make triple sure that
this additional directory lookup is properly cached, the iconloader already checks a very
large number of directories for each icon (but the icon cache helps with that).
David Faure, faure at kde.org, sponsored by Trolltech to work on KDE,
Konqueror (http://www.konqueror.org), and KOffice (http://www.koffice.org).
More information about the kde-core-devel