Problem/Motivation

#2289999: Add an easy way to create HTML on the fly without having to create a theme function / template introduced not one but two ways to put inline TWIG templates in Drupal code. Either may be used to add translatable text and would need to somehow parse them. Not yet sure how, we need to rely on the TWIG parser to parse the inline text AFAIS...

Proposed resolution

1. Figure out what are the supported forms.
2. Build support.
3. Add tests.

Remaining tasks

Do it.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Gábor Hojtsy’s picture

Looks like both the inline_template and the renderInline() would need specific support in potx. We are not looking at PHP files for Twig translatable text of course. So we need reliable ways to identify these template fragments and then parse them as Twig. The change notice explains two forms: https://www.drupal.org/node/2311123, can we rely on such specific things as an array defined all at once with type key before the template key? Ie. if you define it the other way around it will not be translatable? Consider we need to statically parse this PHP code to then statically parse the Twig code.

Fabianx’s picture

Fabianx’s picture

Once #2308215: Create a script to compile all twig template files and inlines is solved, this one can re-use that work as the compiled template can be parsed for t() as usual. Consider co-ordinating that effort with chx.

Gábor Hojtsy’s picture

@Fabianx: potx does not currently compile the template, we essentially do this:

  $twig_lexer = new Twig_Lexer();
  $stream = $twig_lexer->tokenize($code, $file);

  while (!$stream->isEOF()) {
    $token = $stream->next();
    // Capture strings translated with the t or trans filter.
  }

Probably much quicker than making TWIG do the lexing, code generation and then read back the file from somewhere temp. We could theoretically use a copy of part of the logic from #2308215: Create a script to compile all twig template files and inlines.

herom’s picture

just noting that we should be careful not to skip too much stuff while parsing here. t() calls might come in-between the render array items.
example from #2309215: HTML double-escaping in revision messages:

+            'data' => array(
+              '#type' => 'inline_template',
+              '#template' => '{{ revision_info }} {% if revision_log %} <p class="revision-log">{{ revision_log }}</p> {% endif %}',
+              '#context' => array(
+                'revision_info' => $this->t('!date by !username', array('!date' => $this->l($this->dateFormatter->format($revision->revision_timestamp->value, 'short'), 'node.view', array('node' => $node->id())), '!username' => drupal_render($username))),
+                'revision_log' => Xss::filter($revision->revision_log->value),
+              ),
+            ),
Gábor Hojtsy’s picture

Right, we already find those t()s I believe. Those should not be an issue or need any more special treatment. Maybe we want to add some tests for them, but I'm not sure it is necessary even.

herom’s picture

Status: Active » Needs review
FileSize
3.6 KB
1.28 KB

Here is a patch.
I have implemented this in the simplest form: it searches for the '#template' => '...' pattern, and feeds the template string to the potx twig parser.

Running this inline template parser on Drupal 8 core generated 6 string, which I have also attached here.

herom’s picture

Adding support for ['#template'] = '...' was also simple to do. I think that should be enough to do. There is also tests for both formats.

Fabianx’s picture

I would RTBC that, but I did not test, but only reviewed the code.

So just a RTBC + 1 from me.

Gábor Hojtsy’s picture

Looks good except:

+++ b/potx.inc
@@ -235,7 +235,8 @@ function _potx_process_file($file_path, $strip_prefix = 0, $save_callback = '_po
+             || ($token[0] == T_CONSTANT_ENCAPSED_STRING && $token[1] == "'#template'")) {

@@ -2491,6 +2496,40 @@ function _potx_find_constraint_messages($file_name, $save_callback) {
+  foreach ($_potx_lookup["'#template'"] as $key => $ti) {

I suspect this would not work with "#template" (double quotes as opposed to single quotes). It is true our code style suggest single quotes, but should we not recognise the double quote variant at all?

herom’s picture

Yeah, we should support "#template" too, but I had missed it. Here's a new patch.

Thanks for the reviews.

Gábor Hojtsy’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me :) Except:

+++ b/potx.inc
@@ -235,10 +235,15 @@ function _potx_process_file($file_path, $strip_prefix = 0, $save_callback = '_po
            // Give doc comments a specific key because their content varies.
            $key = ($token[0] == T_DOC_COMMENT) ? 'T_DOC_COMMENT' : ($constraint_match ? 'T_POTX_CONSTRAINT' : $token[1]);
+           if ($key == '"#template"') {
+             $key = "'#template'";
+           }

Let's add a comment something like "Normalise template key token to support both quotes". Or something along those lines.

Marking RTBC, feel free to commit with that fix to both branches :)

  • herom committed ae1f86f on 6.x-3.x
    Issue #2315329 by herom | Gábor Hojtsy: Support Drupal 8 inline twig...

  • herom committed 2a838b7 on
    Issue #2315329 by herom | Gábor Hojtsy: Support Drupal 8 inline twig...
herom’s picture

Status: Reviewed & tested by the community » Fixed

Added this one-line comment suggested by #12, and committed.

diff --git a/potx.inc b/potx.inc
index 687beef..5224cc7 100644
--- a/potx.inc
+++ b/potx.inc
@@ -240,6 +240,7 @@ function _potx_process_file($file_path, $strip_prefix = 0, $save_callback = '_po
 
            // Give doc comments a specific key because their content varies.
            $key = ($token[0] == T_DOC_COMMENT) ? 'T_DOC_COMMENT' : ($constraint_match ? 'T_POTX_CONSTRAINT' : $token[1]);
+           // Normalise "#template" token to support both single-quoted and double-quoted #template keys.
            if ($key == '"#template"') {
              $key = "'#template'";
            }

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.