Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Yii:t() variants #251

Closed
bwoester opened this issue May 13, 2013 · 40 comments
Closed

Yii:t() variants #251

bwoester opened this issue May 13, 2013 · 40 comments
Assignees
Milestone

Comments

@bwoester
Copy link
Contributor

Related to http://www.yiiframework.com/forum/index.php/topic/43147-backward-compatibility-questions/

The thread suggests a couple of different variants about how to avoid the string mangling approach used in the current implementation of YiiBase::t() and L18N::translate().

Basic goals:

  • avoid the need of passing the same parameters to Yii::t() on every invokation (for example passing 'app' as category on each call)
  • simple to use, clean interface
  • message extraction shouldn't be complicated too much
@samdark
Copy link
Member

samdark commented May 13, 2013

I'm for the following variant:

// default category
\Yii::t('eat {stuff}', array(
    '{stuff}' => 'meat',
));

// default category, 5.4
\Yii::t('eat {stuff}', [
    '{stuff}' => 'meat'
]);

// non-default category
\Yii::t(array('category', 'eat {stuff}'), array(
    '{stuff}' => 'meat'
));

// non-default category, 5.4
\Yii::t(['category', 'eat {stuff}'], [
    '{stuff}' => 'meat'
]);

@bwoester
Copy link
Contributor Author

I gave it a try implementing a variadic version of the YiiBase::t() method. Can be reviewed in my variadic-yii-t branch. While it allows very flexible invocation of the method, I think it lacks on the 'clean interface' requirement. Those parameter names are weird...

@bwoester
Copy link
Contributor Author

Moving category and language into the params array might be another approach:

\Yii::t('I like {stuff}', array(
  '{stuff}' => 'meat',
  'category' => 'myExt',
  'language' => 'de_DE',
));

But I guess that makes message extraction hard to do (category needs to be extracted as well)...

@andersonamuller
Copy link
Contributor

@samdark I'm with you, just don't know about the message extraction tool. Do you think it would be feasible?

@bwoester
Copy link
Contributor Author

Current solution:

\Yii::t('message', $params); // default category
\Yii::t('category|message', $params); // non-default category

@samdark
Copy link
Member

samdark commented May 13, 2013

Array variant doesn't make it too hard to implement message extraction, I think.

@samdark
Copy link
Member

samdark commented May 13, 2013

Moving to params isn't a reliable way when considering extraction tool.

@tonydspaniard
Copy link
Contributor

@bwoester
Copy link
Contributor Author

@tonydspaniard lol, typo and c&p. Should have been $default. 😃

@alanwillms
Copy link
Contributor

If \Yii::t('category|message'); becomes the pattern, I think you should allow us to choose which character will be the separator. | is nice, but . seems easier to read and it's easier to type.

It also would be cool if Yii could detect a namespace from current app controller or command, so |message would point to controller_resource|message.

Do you have any plans for date & time formats? Something like this?

@samdark
Copy link
Member

samdark commented May 13, 2013

@alanwillms the problem is that dots are common in messages.

@Ragazzo
Copy link
Contributor

Ragazzo commented May 13, 2013

i think idea with separator is really bad. i vote for @samdark first case. Also need to consider modules or extensions translations, so it must be possible to set full path in the category, for example:

Yii:t(array(
    'category' => 'my/own/path/to/the/cat/MainCategory',
    'translation message',
))

i dont like to set basePath every time, so this situation really need to be considered, in Yii1.1 it is made as "by design" so need this one in Yii2 too.

@resurtm
Copy link
Contributor

resurtm commented May 13, 2013

What do you think about following variant:

\Yii::tapp('message', $params); // app category
\Yii::t_app('message', $params); // perhaps this
\Yii::t('message', $params); // default category

@alanwillms
Copy link
Contributor

@samdark You're right, I forgot. But that's another problem, if I have the following message:

\Yii::t('category|My beautiful welcome message!');

And then I decide to change it:

\Yii::t('category|My new welcome message!');

I would have to update all translation files or create a new key for the default language. I think people should be encouraged to use message ID's instead of the proper sentence:

\Yii::t('category|welcome_message');

@Ragazzo
Copy link
Contributor

Ragazzo commented May 13, 2013

@resurtm create little-different methods for same purpose, hm, i think not good.

@resurtm
Copy link
Contributor

resurtm commented May 13, 2013

@Ragazzo, i meant __callStatic. :-)

@Ragazzo
Copy link
Contributor

Ragazzo commented May 13, 2013

ah, ok, got it. But anyway i think that samdark case is much better and explicit)

@samdark
Copy link
Member

samdark commented May 13, 2013

@alanwillms if it's different message the key should be different. Else you can end up with outdated translations.

@alanwillms
Copy link
Contributor

@samdark or unused translations. Well, in the end this is left to the programmer to decide. I vote for your first usage example 😄

@qiangxue
Copy link
Member

I don't quite like the array approach as it requires too much typing (8 more characters).

The variadic solution is better, but could easily cause typos.

I prefer to using two functions: t($category, $message, $params), and tt($message, $params), where tt() is equivalent to t('app', $message, $params).

@Ragazzo
Copy link
Contributor

Ragazzo commented May 13, 2013

@qiangxue but it is not very useful, it is only useful (1 param less) for the framework with default category app.

@bwoester
Copy link
Contributor Author

Would it be possible to override tt() in extension scope?
Maybe like we did it with mocking PHP's time() function in the DbCacheTest?

@mdomba
Copy link
Member

mdomba commented May 13, 2013

What about having the $category as the last (3rd) parameter?

The only con is if we want to specify the category but don't have params so the call would be like t($message,null,$category)

@Ragazzo
Copy link
Contributor

Ragazzo commented May 13, 2013

@mdomba what about $lang param?

@qiangxue
Copy link
Member

Yii::t() is mainly provided as a shortcut function. So the requirement is that it should be very easy to type and use. Otherwise we lose the purpose of having it.

@Ragazzo It is useful in your main application code when your message category is "app", which should be the case most of the time (if not, we won't take time discussing about this.)

@bwoester You can certainly override it or use your own shortcut function. Anyway, it is just a shortcut function. You can define whatever you want. The only concern is to let the message extraction tool recognize it.

@mdomba Yes, this is a con. And because the category is set as the last parameter, it could be easily omitted, especially after you finish writing a complex $params. Another possibility to put $category as the second parameter, but it also has similar cons.

@resurtm
Copy link
Contributor

resurtm commented May 13, 2013

There is one more option:

public static function t($message, $category = 'app', $params = array())
{
    if (is_array($category)) {
        $params = $category;
        $category = 'app';
    }

    // ... do main stuff here
}

Usage:

Yii::t('test', array('param1' => 'value1'));
Yii::t('test', 'app', array('param1' => 'value1'));
Yii::t('test', 'app');

@qiangxue
Copy link
Member

This is the similar variadic approach. The drawback with variadic approaches is that it is difficult to document and can easily cause usage problems. For this particular approach you described, it conflicts with the choice format where the second parameter could be a scalar used to choose which plural form to use.

@resurtm
Copy link
Contributor

resurtm commented May 13, 2013

Ok. Personally for me leaving everything as it was in 1.1 is well (Yii::t('app', 'message', array());). I could define new shortcut in my IDE (google for Live templates if you're using JetBrains IDE). I'd define t<TAB> for Yii::t('app', string and T<TAB> for Yii::t(' string in PHP context.

@bwoester
Copy link
Contributor Author

Mhm. Just another idea... Basically if I do this:

namespace MyExtension;

class Yii extends yii\YiiBase
{
  static function t( $message, $params=array(), $language=null ) {
    return yii\YiiBase::t2_or_tt_or_tr( 'myExtension', $message, $params, $language );
  }
}

class MyExtension {
  // ...
}

I could call Yii::t( 'message' ) from everywhere inside my extension and it would be forwarded to YiiBase::t2_or_tt_or_tr() using the context of my extension. Also, when I extract the messages from my extension, all the messages would be found. The message extraction command needed a parameter to define the category the messages should be placed in, but that shouldn't be hard.

// EDIT:
Maybe this might be a good idea in general. Having extension define their own Yii class. Isn't that the reason why there is YiiBase and Yii? For being able to customize it to the application's needs when developing an application? When developing an extension, we should customize it for the extension's needs...

@psihius
Copy link

psihius commented May 13, 2013

I have to ask, how many of commenters had written a project with a desent amount of categories and text? I can say for sure - I have. Just one of my projects has 20 categories and 107 KB of text in single language translation file - and I have 7 languages.
I use default category rarely, because I group my stuff - a single category file would be unmanageble. Although it's easier for small to medium apps with little text to translate use a method with default category just being the default - I have to say that having like it is in Yii 1.1 actually saves me from mistakes of forgetting to write the message category.
Also automated message extraction and management is very important and in 1.1 it's some what lacking in fuctionality (you can have strings translated that are not in source code and you just can't make the "yiic message" to not mark those messages as having no translation (it marks them @@string@@) or other stuff.

The message keyword idea is nice, but if you have to deal with the amount of text I have to deal (and I have to say that half the text is translated throught view static files system that Yii has), you understand at first glance, that without some special tools it's a pita. Right now I just give the generated file with 'source text' => '' and they translate the second part to the language required. How should I do that with message keywords or message ID - it's an open question.

My 0.02$

@qiangxue
Copy link
Member

@psihius Thank you for sharing your experience. It's true in serious projects, message categories are very important and the default app category is probably not used much. However, in most projects the app category is probably the mostly used, and that's why this issue is opened. Regarding the message ID approach, the current solution is compatible with that. You just need to define the "source translation" for the message IDs.

I think we are close to the conclusion. Most likely we will keep the 1.1 version of t(). We may consider adding an additional method tt (or some other name) specifically to handle the default app case.

@psihius
Copy link

psihius commented May 13, 2013

Some additional thoughts.

Actually I had to deal with the issue when source message changes and I have to update it in translation. While thinking about that and keeping in mind, that I really don't wana deal with some kind of message id, a wild thought crossed my mind.
What if the message system is designed to work in 2 modes, configured by the param in the config file for "yiic message": A strict "message id" mode where you write a message id and then add the actual text in the translation file, basicaly the mode we have now - copy text key as written in the source code. Second mode will use actual text in the source, but when scanning the files - it can automatically add "message id" into the source code, something like Yii::t('category', 'Hello world') => Yii::t('category#25', 'Hello world') and add to the translation file something like this: '#25' => 'Hello world' or 'Hello world' => array(25, 'Hello world'),. That way if the source text changes in the future - you can track the changes and mark them as changed so the relevant person can take care of it.
Just a wild late night thought.

@qiangxue thank you for considering my thoughts :)

@HnH
Copy link

HnH commented May 14, 2013

I think that a translation into any available language is a must, so as an easy category change and params array for translated message. The most flexible option I see is a Yii::t() to return an object:

// Category — app, language — russian
Yii::t('app', 'ru')->do('message.in.category', array(
    '{stuff}' => 'meat'
));

// Or all goes default
Yii::t()->do('message.in.category');

// The real benefit is felt when translating a set of strings
$t = Yii::t('app', 'ru');
$t->do('message.in.category');
$t->do('second.message.with.params', array(
    '{stuff}' => 'meat'
));

// Or even like that
foreach (array('en', 'ru') as &$lang)
{
    $t->setLanguage($lang)->do('message.in.category');
}

Sure it's not the shortest option available, but I think it's the most flexible and functionally rich one.

@slavcodev
Copy link
Contributor

+1 to

// is shortcut to Yii::$app->message component
Yii::t($message, array $params, $category = MessageSource::$defaultCategory); 

// is shortcut to Yii::$app->coreMessages component to translate core messages
Yii::tt($message, array $params); 

@cebe
Copy link
Member

cebe commented May 14, 2013

I think we are close to the conclusion. Most likely we will keep the 1.1 version of t(). We may consider adding an additional method tt (or some other name) specifically to handle the default app case.

I am also for keeping Yii::t as it was in 1.1, if your app has more than 4 or 5 controllers that work in slightly different use cases you really need to use categories to keep track of what is going on with your translations, so having category as the first param is really nice. also for code readability:

'label1' => Yii::t('models', 'My cool label 1'),
'label2' => Yii::t('models', 'Another wild translation'),

vs.

'label1' => Yii::t('My cool label 1', 'models'),
'label2' => Yii::t('Another wild translation', 'models'),

Moving category between $message and $params is not cool, because you want the params connected with the message you use them in. After params will be annoying when you really want to use categories.
So we need two methods which will be these 2 imo:

Yii::t($category, $message, $params = array(), $language = null); 
Yii::tt($message, $params = array(), $language = null); // category is 'app' by default 

The name Yii::tt is okay as it does not say more or less than Yii::t does but suggerates that they are related.

Just a wild late night thought.

@psihius sounds like a cool thing but I think this is a bit too much to put in the framework. Might be good to create an extension to MessageCommand that allows that. The tagging might also be done with PHP comments so it does not affect the code:

Yii::t('app', 'Translate this!'/*#25*/)

@slavcodev I'm afraid you are mixing something up here. discussion is not about 2 different methods for coreMessages and messages. And btw: there is no coreMessages vs. messages component in yii2

@slavcodev
Copy link
Contributor

right, I mixed up with yii1.1, but on the syntax is correct, that's what I like the most.
Moving $category in third param with default app (for core is yii right?) will be usefull to small application which use only one $category. For big applications will be just as easy to change the category.

@slavcodev
Copy link
Contributor

or add the feature to use a single file for all categories

@cebe
Copy link
Member

cebe commented May 15, 2013

For big applications will be just as easy to change the category.

It is not, because you would have to specify $params to change the category which is not cool.

or add the feature to use a single file for all categories

What for? You add categories to keep overview. Putting it all in one file after that you could have used just one category.

@bwoester
Copy link
Contributor Author

What for? You add categories to keep overview. Putting it all in one file after that you could have used just one category.

Sorry, disagree on this. Category's primary use case is not keeping messages in separate files for overview purposes. There are tools that can help you with this problem, regardless how or where the data is saved.

Category's primary use case is about having one and the same message twice in your source language, which needs to be translated differently, depending on its context. At least that's the way I see it. 😉

@psihius
Copy link

psihius commented May 15, 2013

@cebe That's actually a great though about using comments for that. Yep, maybe an extension is in order - should be pretty straight forward. Such tagging will deal with the only annoying issue the yii 1.1 has with translations :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests