[RTM] Translator #1072

ausi · 2017-09-12T17:04:40Z

Can be used as following:

->trans('MSC.view', [], 'contao_default');
->trans('ERR.minlength', ['My Field', 1234], 'contao_default');
->trans('tl_settings.websiteTitle.0', [], 'contao_tl_settings');
->trans('MSC.key\.mit\.punkt\.und\\\\backslash', 'contao_default');

Twig:

{% trans_default_domain 'contao_default' %}
{{ 'MSC.view'|trans }}
{{ 'ERR.minlength'|trans(['My Field', 1234]) }}
{{ 'tl_settings.websiteTitle.0'|trans({}, 'contao_tl_settings') }}
{{ 'MSC.key\\.mit\\.punkt\\.und\\\\backslash'|trans }}

leofeyer · 2017-09-12T17:18:40Z

I like a lot!

As discussed, we also want to use https://symfony.com/doc/current/service_container/service_decoration.html

aschempp · 2017-09-13T10:18:24Z

src/Translation/Translator.php

+            return $this->translator->trans($id, $parameters, $domain, $locale);
+        }
+
+        if (!$this->framework->isInitialized()) {


Should we initialize the framework here?

aschempp · 2017-09-13T10:29:41Z

src/Translation/Translator.php

+    private function loadLanguageFile(string $name)
+    {
+        /** @var \Contao\System */
+        $system = $this->framework->getAdapter('System');


You must always use the FQCN, or just use System::class and import the class from Contao\System.

…used

aschempp · 2017-09-13T19:32:01Z

src/Translation/Translator.php

+        $item = &$GLOBALS['TL_LANG'];
+
+        foreach ($parts as $part) {
+            if (!isset($item[$part])) {


what if $item is not an array here? Will we get an error message trying to access a key of a scalar variable?

No, isset() doesn’t generate a warning for such cases. Tested in aea72e3.

DanielSchwiperich

main request changes regarding use of php 7 features

DanielSchwiperich · 2017-09-14T16:02:46Z

src/Translation/Translator.php

@@ -0,0 +1,138 @@
+<?php


since this PR is targeted for 4.5.0 (same as #1077) and is already using php 7 features like the string parameter type hints it should fully make use of it.

right here declare strict declaration is missing

DanielSchwiperich · 2017-09-14T16:04:05Z

src/Translation/Translator.php

+    /**
+     * {@inheritdoc}
+     */
+    public function trans($id, array $parameters = [], $domain = null, $locale = null)


the symfony interface is php5, it is ok to use parameter hints in the implementing method, also return type declaration is missing

same for following methods

No, parameter hints are not allowed for implemented methods in PHP 7.1, only in 7.2

you are absolutely right on this. Thought I had done this successfully in a project but was wrong

DanielSchwiperich · 2017-09-14T16:07:39Z

src/Translation/Translator.php

+     *
+     * @return string|null
+     */
+    private function getFromGlobals(string $id, string $domain)


I saw leo using nullable return type, so I assume we have php 7.1 features available and we can set a return type : ?string

DanielSchwiperich · 2017-09-14T16:21:50Z

src/Translation/Translator.php

+ *
+ * @author Martin Auswöger <martin@auswoeger.com>
+ */
+class Translator implements TranslatorInterface


Since this Translator is only for usage with $Globals['TL_LANG'] and hopefully replaced someday by a solution which doesn't need to use a superglobal I would recommend a name like GlobalsTranslator or so

Where this translator gets the messages from may change in the future. In the optimal case this class gets removed in the future and the default translater service can be used.

DanielSchwiperich · 2017-09-14T16:25:25Z

src/Translation/Translator.php

+    /**
+     * {@inheritdoc}
+     */
+    public function setLocale($locale)


I'd recommend to give info in the doc block that this method and the following do nothing in regard of contao.

Done in bbf0b94.

ausi · 2017-09-14T17:02:12Z

@DanielSchwiperich Adding PHP 7 features is not part of this pull request and should be done in #1077 IMO

DanielSchwiperich · 2017-09-14T18:01:20Z

ah ok in that case you would need to remove the string parameter type hints because they are only allowed from php 7 onwards. And then ignore most of my other comments 😄

aschempp · 2017-09-15T13:05:09Z

Just found the LoggingTranslatorPass class. Does that affect us?

ausi · 2017-09-15T15:54:11Z

The logging still works for the Symfony translator, Contao translations don’t get logged.

leofeyer · 2017-09-18T11:55:03Z

Thank you @ausi.

DanielSchwiperich · 2017-09-18T11:57:03Z

@leofeyer you have now merged code which requires php7, you know that?

Toflar · 2017-09-18T11:57:32Z

The develop branch is php 7.1 only.

DanielSchwiperich · 2017-09-18T12:01:13Z

then I don't understand why this PR was not made using all the php 7.1 features (see my comments).
Either do it php 7(.1) way or not at all. imho

ausi · 2017-09-20T09:38:52Z

@DanielSchwiperich the PHP 7.1 features for this PR got added in https://github.com/contao/core-bundle/pull/1077/files#diff-144b2e2beae5529dc5a75f36c717cbb7

Description ----------- Backports the changes from #1067 and #1071. Commits ------- 8da1a5a2 Backport the CS fixer changes (see #1071)

Add Translator

b77e009

leofeyer assigned ausi Sep 12, 2017

leofeyer added the feature label Sep 12, 2017

leofeyer added this to the 4.5.0 milestone Sep 12, 2017

ausi added 5 commits September 12, 2017 21:19

Decorate the translator service

82a1089

Support translation parameters

a1266c9

Use a contao prefix for the message domain

2f82cac

Add unit test

fd09847

Test the translator service

380c7e8

aschempp mentioned this pull request Sep 13, 2017

[RFC] Use KnpMenu for the main backend menu #1066

Merged

4 tasks

aschempp suggested changes Sep 13, 2017

View reviewed changes

ausi added 4 commits September 13, 2017 14:59

Always initialize the Contao framework if a Contao message domain is …

a1c9583

…used

Use the System class from the Contao namespace

8b6e892

Coding style

7391900

Only escape dots and backslashes

5eba242

aschempp reviewed Sep 13, 2017

View reviewed changes

aschempp approved these changes Sep 13, 2017

View reviewed changes

Test missing sub item of translation key

aea72e3

ausi changed the title ~~[WIP] Translator~~ [RTM] Translator Sep 14, 2017

DanielSchwiperich suggested changes Sep 14, 2017

View reviewed changes

ausi added 2 commits September 14, 2017 20:38

Improve method documentation

bbf0b94

Fix var annotation

267c715

Toflar approved these changes Sep 15, 2017

View reviewed changes

leofeyer added 2 commits September 18, 2017 13:21

Fix the coding style.

58dfc76

Fix the coding style.

d64ba6e

leofeyer merged commit e275d66 into contao:develop Sep 18, 2017

richardhj mentioned this pull request Jul 24, 2018

Change contao/core-bundle version to 4.4 as the minimum requirement richardhj/contao-recaptcha#2

Closed

leofeyer added a commit that referenced this pull request Dec 9, 2019

Backport the CS fixer changes (see #1072)

c1b7035

Description ----------- Backports the changes from #1067 and #1071. Commits ------- 8da1a5a2 Backport the CS fixer changes (see #1071)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RTM] Translator #1072

[RTM] Translator #1072

ausi commented Sep 12, 2017 •

edited by leofeyer

Loading

leofeyer commented Sep 12, 2017 •

edited

Loading

aschempp Sep 13, 2017

ausi Sep 13, 2017

aschempp Sep 13, 2017

ausi Sep 13, 2017

aschempp Sep 13, 2017

ausi Sep 14, 2017

DanielSchwiperich left a comment

DanielSchwiperich Sep 14, 2017

DanielSchwiperich Sep 14, 2017

DanielSchwiperich Sep 14, 2017

ausi Sep 14, 2017

DanielSchwiperich Sep 14, 2017

DanielSchwiperich Sep 14, 2017

DanielSchwiperich Sep 14, 2017

ausi Sep 14, 2017

DanielSchwiperich Sep 14, 2017

ausi Sep 14, 2017

ausi commented Sep 14, 2017

DanielSchwiperich commented Sep 14, 2017

aschempp commented Sep 15, 2017

ausi commented Sep 15, 2017

leofeyer commented Sep 18, 2017

DanielSchwiperich commented Sep 18, 2017

Toflar commented Sep 18, 2017

DanielSchwiperich commented Sep 18, 2017

ausi commented Sep 20, 2017

[RTM] Translator #1072

[RTM] Translator #1072

Conversation

ausi commented Sep 12, 2017 • edited by leofeyer Loading

leofeyer commented Sep 12, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanielSchwiperich left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ausi commented Sep 14, 2017

DanielSchwiperich commented Sep 14, 2017

aschempp commented Sep 15, 2017

ausi commented Sep 15, 2017

leofeyer commented Sep 18, 2017

DanielSchwiperich commented Sep 18, 2017

Toflar commented Sep 18, 2017

DanielSchwiperich commented Sep 18, 2017

ausi commented Sep 20, 2017

ausi commented Sep 12, 2017 •

edited by leofeyer

Loading

leofeyer commented Sep 12, 2017 •

edited

Loading