getkirby-v2 / toolkit

This is the deprecated toolkit for Kirby v2.
http://getkirby.com
81 stars 50 forks source link

Translation Feature #97

Closed timkaechele closed 8 years ago

timkaechele commented 9 years ago

I added a translation feature that allows to conveniently keep track of different translations with a c::set()/c::get() like syntax.

lukasbestle commented 9 years ago

Have you seen the L class? It actually does exactly that.

It doesn't have capabilities to import strings for multiple languages at once though, but Kirby uses it already by loading only one language file.

timkaechele commented 9 years ago

I saw the L class but I found it a bit limited. It didn't provide more functionality than the c class. It was just a renamed Silo.

With my T class you can write:

T::set("en.user.username", "Username");
T::set("de.user.username", "Benutzername");

T::setLanguage("en");
T::get("user.username"); // => "Username"

T::setLanguage("de");
T::get("user.username"); // => "Benutzername"

But yeah, whatever.

lukasbestle commented 9 years ago

I know. Your approach is much more flexible, but having two classes do nearly the same can be really confusing. Maybe you could implement the extra functionality in a backwards-compatible way in the L class?

Also, I think it should be possible to set an array for each language (like T::set("en", array("user.username" => "Username"));).

timkaechele commented 9 years ago

Maybe I will look into that later on, currently I just tried to solve my particular problem with translations while using the toolkit. Thought it could replace the L class. Just by looking at both classes I find it hard to think of a solution that doesn't obscure the code tremendously just to provide backwards compatibility.

I am not sure if the toolkit is using semantic versioning, but maybe it could be included in the next major release.

lukasbestle commented 9 years ago

Yeah, it's quite difficult to get right. But having two separate classes is not a viable solution IMHO. Simply replacing it doesn't work either, as the CMS relies on the L class. What do you think, @bastianallgeier?

timkaechele commented 9 years ago

Ok I just added the array set method. Now you can write

T::set("en", array(
  "user.username" => "Username",
  "user.password" => "Password"
));
bastianallgeier commented 9 years ago

I'm late to the party, but I agree that there shouldn't be another class for this. I also don't really see the use case to switch the language on runtime, but maybe I'm just missing something.

timkaechele commented 9 years ago

Personally I found it useful. It is a cleaner way to implement multi language support and it adds the ability to hold multiple translations at runtime, which I found especially useful in a context where you would load the user language settings after the setup of the translation tables.

Take for example an application that allows every user to specify its own language. You would store that language setting in a database. Usually you would have a stack that looks like this

  1. load all config data
  2. identify user, based on the session cookie
  3. access database to get her/his language setting
  4. you already loaded the translations with the config data or you are forced to mixin a reload of the translation data, which I find quite nasty.

In my class I wanted a strict distinction between providing static data that is set up at the beginning of the request lifecycle and dynamic settings like the language of the user that usually take place later in the request.

Anyways I used the class in KirbyZoo and found it quite helpful to provide i18n.

bastianallgeier commented 8 years ago

I'm going to make L better in one of the next releases. So I'm going to close this for now.