kalessil / yii2inspections

MIT License
31 stars 3 forks source link

[dev] Update Translation action #4

Closed kalessil closed 7 years ago

kalessil commented 7 years ago
kalessil commented 7 years ago

@brandonkelly, can you share some details regarding this:

brandonkelly commented 7 years ago

The {% includeTranslations %} tag and the |t filter are Craft CMS-specific Twig extensions. Not things this plugin should worry about.

Although it does bring up a good question: how should the plugin account for dynamic calls to Yii::t()? For example that {% includeTranslations %} tag maps to a function that does this (simplified):

public function registerTranslations($category, $messages)
{
    foreach ($messages as $message) {
        $translation = Yii::t($category, $message);
        // store the translation for later use ...
    }
}

So to resolve $category and $message in this case, the plugin would need to be smart enough to realize that Yii::t() is being called within that foreach-loop, and find any calls to that includeTranslations() function, and analyze the $messages array being passed in.

I'm assuming that would not be easily done though. So as much as I was hoping to keep this plugin application-agnostic, maybe we need to add one little Craft-specific feature: when indexing calls to Yii::t(), also index any calls to craft\web\View::registerTranslations(), and index each of the values in the array being passed to its $messages argument. Is that feasible?

kalessil commented 7 years ago

This case is not feasible for plugin v1 (not a rocket science, but needs time).

For twig I already have to extract translations: https://github.com/kalessil/yii2inspections/blob/master/src/main/com/kalessil/phpStorm/yii2inspections/actors/upadateTranslations/ProjectTranslationTwigCallsFinder.java#L34. I think that in this context we can skip processing includeTranslations. Am I right, or?

brandonkelly commented 7 years ago

If we are going to have any Craft-specific support in the plugin, it should be for Craft 3.0, which is hitting Beta in the coming weeks. I just gave you access to the repo (email coming soon on how to get it installed), but here are the relevant changes:

I think that in this context we can skip processing includeTranslations. Am I right, or?

There are places in PHP that are calling View::registerTranslations() as well, so it would be helpful to specifically look for places that are calling that in PHP.

kalessil commented 7 years ago

Thank you for giving the access @brandonkelly, what caught my eye is that translations structure is as Yii uses - simplifies my code a lot. Will follow up on this with specific answers.

EDIT:

brandonkelly commented 7 years ago

Ah yes, Craft::t() is identical to Yii::t() in 3.0, unlike 2.x.

kalessil commented 7 years ago

Main part is done, #7, #8 are follow up tickets for consideration.