Empty table cells never hidden if twig debug is true

I just made a small commit that solves for me the issue mentioned in #28.

I also made a merge request.

You can download the patch as a diff directly from my commit. I'm also attaching a patch file here.

Tried to fix CCF for #31.

32 incorrectly introduces a space in the regex.

Noticed a problem with the patch. The following error is raised, likely due to PHP8 changes.

Deprecated function: preg_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated in Drupal\views\Plugin\views\field\FieldPluginBase->isValueEmpty() (line 1226 of core/modules/views/src/Plugin/views/field/FieldPluginBase.php).

We should ensure we don't enter the if (\Drupal::service('twig')->isDebug()) { condition if value is NULL.

Adding a new patch addressing points from #34 and #35, and also fixing some phpcs errors. please review.

The last submitted patch, 36: 2950758-36.patch, failed testing. View results →

1. +++ b/core/modules/views/src/Plugin/views/field/FieldPluginBase.php
   @@ -1236,6 +1240,15 @@ public function isValueEmpty($value, $empty_zero, $no_skip_empty = TRUE) {
   +    if ($twig_debug_on == TRUE && !is_null($twig_debug_on)) {
   

   the previous way of doing it was fine. no need to assign and check TRUE

   this is also checking the wrong value for whether it is NULL. you should be checking $value. Not the result of isDebug()

   $value !== NULL will be fine for the null check.

2. +++ b/core/modules/views/src/Plugin/views/field/FieldPluginBase.php
   @@ -1236,6 +1240,15 @@ public function isValueEmpty($value, $empty_zero, $no_skip_empty = TRUE) {
   +      $value = preg_replace('/<!--.*?-->/s', '', (string) $value);
   

   this correctly removed the leading space

@TanujJain-TJ there are many changes in the new patch compared to the previous without explanation, in addition to an overly large interdiff? Have you brought in irrelevant changes?

Tests continue to fail.

NW

@dpi sorry for that null check i interpreted it wrong at first updating the patch to correct all the changes required, also these out of scope changes are just PHPCS fixes i got while running this, did it so the patch doesn't fail Custom commands check. adding new interdiff with #32.
phpcs --standard="Drupal,DrupalPractice" --extensions="php,module,inc,install,test,profile,theme,css,info,txt,md,yml"

Please address the CS changes relevant to the lines being modified only.

updated according to #40, please review.

The last submitted patch, 41: 2950758-41.patch, failed testing. View results →

@Tanuj ... for tests to pass i think you also need to check the twig service exists

So, maybe you could update patch (or create new MR) with something like:

    // If twig_debug is enabled we strip out the
    // html comments before running the empty check.
    if ($value !== NULL) {
      if (\Drupal::hasService('twig') && \Drupal::service('twig')->isDebug()) {
        $value = preg_replace('/<!--.*?-->/s', '', (string) $value);
        $value = trim($value);
      }
    }

@Tanuj: Patch #41 works, but the core media library form widget does not work anymore. You cannot select items there.

@Tanuj: I can confirm that after applying patch #41, the core media library widget (both grid and table views) is not functioning correctly. The checkboxes are not being rendered when twig debugging is enabled.

I am not sure whether this solves all the problems people face here, but I have a completely different approach that solves it for me with views at least and does not introduce any additional logic.

When I check for empty fields, the problem is not the added XML-commented debug output per se, but the added whitespace/line breaks that are not inside the comments and therefore lead to twig believing there is content indeed.

I first thought that removing the line breaks would mess up the generated markup (but I wanted to live with it, as long as it would solve the initial problem of seeing different rendered things with and without debug mode enabled).
But as it turns out, both firefox and chromium take care of adding linebreaks themselves when using developer tools in the browser and the markup looks just the same as with the patch. And it makes checks in twig whether fields are empty truely true :-)

I am not a developer so please improve as you see fit. Just posting here as it fixes it for me and seems less intrusive.
Applies against 10.1.x and 10.2.x.

I have created a new patch over #41 after observing the patch issues in #46. This patch fixes the issue of checkboxes not showing in media library picker when twig debug is on.