Extract the trans function in separate method

[kachar]

No description provided.

[SpacePossum]

maybe remove the ! and flip the results, save an OP without draw back

[kachar]

I've kept it as it was in the previous version. Will update it now

[kachar]

It's weird but the build fails with the reversed check.

[fabpot]

What about removing the default value?

[fabpot]

Thank you @kachar.

[jaimeperez]

Hi,

Thanks a lot for merging this, it's a step in the right direction. However, it's still far from usable, for three reasons:

• It doesn't allow you to configure what functions to use. That means you need to extend this extension so that you can use a new, extended trans node that reimplements the getTransFunction() method so that it returns different translation functions. Not very friendly if what you want is to be able to switch your translation implementation easily.
• It doesn't allow callables, so you are forced to use a function and cannot use static methods in some class.
• It's "fixing" the issue for the trans node, but not for the trans filter, which is still calling the native gettext() function. So if you have both {% trans %} blocks and {{ variable|trans }} filters in your templates, one will use your custom function, while the other will still use gettext(). Of course you could also reimplement the getFilters() method of the extension to register the right functions, but again I think that's far from usable.

So basically I don't think #152, #160 and #166 can be considered resolved yet...

[jaimeperez]

Please, take a look at this for what I think would be a usable solution (from the perspective of the theme developers and users of this library). The extension itself is a horrible, horrible hack and I'd like to get rid of it as soon as possible, but unfortunately we need it until this is resolved here, since we can't rely on the native gettext implementation.

[stof]

@jaimeperez you should be able to use a static method: return 'Acme\TranslationUtil::gettext';

[stof]

Btw, regarding your repo, you should put configuration in the extension constructor instead of extending the Twig environment to get the config from the environment

[jaimeperez]

I could live with that instead of callables, especially because callables can specify an object instead of a class, and that's not something that you could use in the compiled template. However, the other two reasons remain valid.

Regarding how to configure the extension, whether to use the environment or the constructor of the extension itself, using the constructor was my initial idea. The problem is that trans nodes are not related to the extension directly as far as I know, so there would be no way to get the configuration if it's not in the environment itself. I'd gladly fix that if there's a way, but I would actually prefer to get rid of my repo because the original problem was fixed here, where it should be.

[jaimeperez]

@fabpot, any chance to get this fixed?