Problem/Motivation

For git coding standards all files need an ending blank newline character.
This becomes problematic when it makes twig files add a newline character that may break markup or the tests for the markup.

A workaround that has been used to add a {% spaceless %}...{% endspaceless %} control to solve the problem but that removes spaces from inside as well. The reason spaceless control works is because it is a twig template tag and from Twig docs all template tags remove the proceeding newline character. @see http://twig.sensiolabs.org/doc/templates.html#whitespace-control

Therefore some other solutions were to append a comment to the end of the last HTML line.

</div>{# This comment removes the newline that follows #}

Proposed resolution

We'd like to automatically remove the last newline of the twig files.

For performance reasons Fabian suggests injecting a comment control before the last newline while the twig template is loaded and compiled. We could also possibly do a regex (if it's not too slow) to remove the extra newline character.

Caveat: We don't want to punish themers by removing the newline they intentionally put in.

Remaining tasks

Create a patch along these lines: except from Google Hangouts (Sept 5, 2013)

source = $this->loader->getSource($name);
// Check last character and remove ...
source = $this->loader->getSource($name);

$compiled_source = $this->compileSource($source, $name);
in TwigTemplate::updateCompiledTemplate

$source = $this->loader->getSource($name);
// Check last character and remove ...
And thats it.
to remove the new lines
core/lib/Drupal/Core/Template/TwigEnvironment.php

User interface changes

N/A

API changes

N/A

#1757550: [Meta] Convert core theme functions to Twig templates.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jenlampton’s picture

Issue summary: View changes

grammar

joelpittet’s picture

Issue summary: View changes

added notes

joelpittet’s picture

Here's the patch. It removes the last newline character of the file once loaded. Then the file is compiled to php and saved to the filesystem.

Attached is the latest patch from theme_image conversion without the {# Comment hack #}

To show fail and pass tests.

joelpittet’s picture

So one question I have, what about intentional newlines? Should/can we add two newlines?

Status: Needs review » Needs work

The last submitted patch, 2082845-3-twig-eof-newline-remove.patch, failed testing.

joelpittet’s picture

Curious what do about this ending new line... sometimes it seems nice to have, most of the time not.

block.html.twig seems to be a good case to have a newline (though two newlines at the end get's marked red by git too)

image.html.twig on the other hand is a nice place to not have a newline...

For reference, only two tests check for newlines that needed fixing. BlockStorageUnitTest.php and ThemeTest.php

I'm leaning towards if we need to remove it, add the comment hack and put that in Twig Coding Standards.
Comments?

joelpittet’s picture

Status: Needs work » Closed (works as designed)

Ok so having a newline in the markup is more good than bad.

I'm closing this as works as designed.

If we did this we'd have to have special cases the other direction if we wanted a newline like \n\n or something...

Recommendations:

  • Change the test if it needs to pass.
  • Or Add a twig template tag to the end.

Adding notes on the subject to Twig Coding Standards
https://drupal.org/node/1823416

joelpittet’s picture

Issue summary: View changes

added caveat