Skip to content

KLocalizedString: add in-place manipulation API for chained changes

Friedrich W. H. Kossebau requested to merge work/kossebau/avoiddeepcopies into master

Enables typical use-cases to avoid repeated deep copies.

The current API is designed around a use-case referenced in the API dox:

subs methods do not update the KLocalizedString instance on which they are invoked, but return a copy of it with one argument slot filled. This allows to use KLocalizedString instances as a templates for constructing final texts, by supplying different arguments.

By own experience a lot of places only resolve to one string though. And when doing that are getting a full deep copy on each modification call.

A good example is the implementation code of all the i18n* macros. Depending on the number of arguments, they are doing multiple modifications to a KLocalzedString, using chained calls via the return values. Just, each time a full deep copy is created, without any need:

 template<typename A1>
 inline QString i18n(const char *text, const A1 &a1)
 {
    // ki18n first instance, subs() creates second by deep copy
    return ki18n(text).subs(a1).toString();
 }

And respective worse with lots of arguments, e.g.

    // kxi18ndp first instance, all subs() create eight more by deep copies
    return kxi18ndp(domain, singular, plural).subs(a1).subs(a2).subs(a3).subs(a4).subs(a5).subs(a6).subs(a7).subs(a8).toString();

Or see the implementation of KuitTag::format(...) where lots of deep copies are created for the aggText variable, where instead the original instance itself could be changed every time directly.

While those copies did not show up in serious numbers in some simple valgrind checks, they still might be a papercut issue.

So I would propose to extend the API by some in-place modification methods, to enable chained calls but without the price of deep copies. Following the example of the QString, QRect and similar, where methods doFoo modify the instance itself, while fooed means returning a copy with the modification instead.

Initial uploaded version is a working proof-of-concept, to give some more foundation to the discussion.

@ilic Do you remember any related discussion about having separate API for modify-by-copy and modify-in-place?

@vkrause @aacid @ltoscano

Current state and util methods:

  • bool isEmpty() const
  • QByteArray untranslatedText() const
  • QString toString() const
  • QString toString(const QStringList &languages) const
  • QString toString(const char *domain) const
  • QString toString(Kuit::VisualFormat format) const

Current modify-copy methods:

  • KLocalizedString withLanguages(const QStringList &languages) const
  • KLocalizedString withDomain(const char *domain) const
  • KLocalizedString withFormat(Kuit::VisualFormat format) const
  • KLocalizedString subs(...) const family
  • KLocalizedString inContext(const QString &key, const QString &value) const
  • KLocalizedString relaxSubs() const
  • KLocalizedString ignoreMarkup() const

Proposed new modify-in-place methods:

  • KLocalizedString &setLanguages(const QStringList &languages)
  • KLocalizedString &setDomain(const char *domain)
  • KLocalizedString &setFormat(Kuit::VisualFormat format)
  • KLocalizedString &substitute(...) family
  • KLocalizedString &setContextEntry(const QString &key, const QString &value) or setContext()/insertContext()/insertContextEntry() ?
  • KLocalizedString &setSubstitutionsRelaxed() or relaxSubstitutions()?
  • KLocalizedString &setMarkupIgnored()

Sadly the new method KLocalizedString &setLanguages(const QStringList &languages) clashes with an existing static method of this method family for controlling the language to use:

  • static void setLanguages(const QStringList &languages)
  • static QStringList languages()
  • static void clearLanguages()

The proposal here is to replace those methods with new ones having additional the term "Application", matching the naming pattern of other static methods in the class namespace:

  • static void setApplicationDomain(const char *domain)
  • static QByteArray applicationDomain()
  • static bool isApplicationTranslatedInto(const QString &language)
  • static QSet<QString> availableApplicationTranslations()
  • static QSet<QString> availableDomainTranslations(const QByteArray &domain)
  • static void addDomainLocaleDir(const QByteArray &domain, const QString &path)

So the new ones would be this, also changing "clear" to "reset" while at it:

  • static void setApplicationLanguages(const QStringList &languages)
  • static QStringList applicationLanguages()
  • static void resetApplicationLanguages()

An other option might be to move all these static util methods into a namespace of its own, perhaps KLocalization?

Edited by Friedrich W. H. Kossebau

Merge request reports