Problem/Motivation

Drupal's Twig implementation does not print anything for the following:
{% set var = 0 %}{{ var }}

This should print '0'.

Proposed resolution

Track down the bug and squash it. @Fabianx said that there is a check for (!$var) somewhere that should be refactored, and possibly we can check is_numeric().

Remaining tasks

TBD

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

alexrayu’s picture

Assigned: Unassigned » alexrayu
alexrayu’s picture

FileSize
452 bytes

The issue seems to reside in non-strict '==' in twig.engine@102. Making it strict '===' outputs var = 0.

alexrayu’s picture

Status: Active » Needs review
FileSize
452 bytes

Needs review.

Status: Needs review » Needs work

The last submitted patch, Twig_null_vars-1970960-2.patch, failed testing.

Fabianx’s picture

+++ core/themes/engines/twig/twig.engine	(revision )
@@ -99,7 +99,7 @@
   // == is true also for empty arrays
-  if ($arg == NULL) {
+  if ($arg === NULL) {

Unfortunately that is not the right fix.

The right fix is to add an if statement before to check for === 0.

if ($arg === 0) {
  return 0;
}

// == is true also for empty arrays
if ($arg == NULL) {
...
Fabianx’s picture

Issue tags: +Twig

tagging

alexrayu’s picture

Status: Needs work » Needs review
FileSize
535 bytes

Yes, thank you!

Status: Needs review » Needs work

The last submitted patch, Twig_null_vars-1970960-6.patch, failed testing.

alexrayu’s picture

FileSize
417 bytes

Resubmitting

alexrayu’s picture

Status: Needs work » Needs review
Fabianx’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

Looks good to me.

Can we have some tests for this, please?

alexrayu’s picture

Status: Needs work » Needs review
FileSize
1.69 KB

1. Rewrote the code, removed test for zero, but moved up the test for scalar values instead, so NULL-test does not intercept it.
2. Wrote a little test to test for NULL and scalar values. Test goes to twig_render() test.

Fabianx’s picture

Status: Needs review » Needs work
Issue tags: -Needs tests

Thanks for the tests.

> 1. Rewrote the code, removed test for zero, but moved up the test for scalar values instead, so NULL-test does not intercept it.

Uhm, the NULL check is there first for a reason.

A function call is very expensive in PHP and this is called very many times, so checking for 0 and then NULL is much faster, than calling is_scalar every time - even if the output is NULL or empty array.

So I would like to keep the behavior of #9

Thanks!

alexrayu’s picture

Status: Needs work » Needs review
FileSize
1.37 KB

Reverted to initial approach ;)

Fabianx’s picture

Status: Needs review » Reviewed & tested by the community

Passes tests, is a simple fix, does not make things slower, fixes a bug, has test coverage

=> RTBC

chx’s picture

Unrelated followup: if ($arg == NULL) is just if (!$arg).

thedavidmeister’s picture

Status: Reviewed & tested by the community » Closed (duplicate)
thedavidmeister’s picture

Issue summary: View changes

Code tags