in file : /core/lib/Drupal/Core/Utility/token.api.php, in function hook_tokens_alter

@@ -153,7 +153,6 @@ function hook_tokens_alter(array &$replacements, array $context, \Drupal\Core\Re
 
   if (isset($options['langcode'])) {
     $url_options['language'] = \Drupal::languageManager()->getLanguage($options['langcode']);
-    $langcode = $options['langcode'];
   }
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rakesh.gectcr created an issue. See original summary.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

mayurjadhav’s picture

Hi rakesh.gectcr,

I think you miss out else condition.

if (isset($options['langcode'])) {
    $url_options['language'] = \Drupal::languageManager()->getLanguage($options['langcode']);
  -  $langcode = $options['langcode'];
  }
  - else {
   - $langcode = NULL;
  - }

Uploading patch to remove else condition.

Status: Needs review » Needs work

The last submitted patch, 3: unused-variable-token-api-2607116-3.patch, failed testing.

rajeshwari10’s picture

Assigned: Unassigned » rajeshwari10
Status: Needs work » Needs review
FileSize
1.38 KB

removed the unused variable.

Thanks!!

markdorison’s picture

Assigned: rajeshwari10 » Unassigned
Status: Needs review » Reviewed & tested by the community
alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/lib/Drupal/Core/Utility/token.api.php
@@ -77,10 +77,6 @@ function hook_tokens($type, $tokens, array $data, array $options, \Drupal\Core\R
-    $langcode = $options['langcode'];
-  }
-  else {
-    $langcode = NULL;

@@ -111,7 +107,7 @@ function hook_tokens($type, $tokens, array $data, array $options, \Drupal\Core\R
-          $replacements[$original] = format_date($node->getCreatedTime(), 'medium', '', NULL, $langcode);
+          $replacements[$original] = format_date($node->getCreatedTime(), 'medium', '', NULL, $options['langcode']);

This is a change in logic I don't think we should be making here - if $options['langcode'] is not set it'll issue a PHP warning with these changes.

rajeshwari10’s picture

Status: Needs work » Needs review
FileSize
1.37 KB

Adding patch.

Please review.

Thanks!!

alexpott’s picture

Status: Needs review » Needs work
+++ b/core/lib/Drupal/Core/Utility/token.api.php
@@ -77,10 +77,6 @@ function hook_tokens($type, $tokens, array $data, array $options, \Drupal\Core\R
-    $langcode = $options['langcode'];
-  }
-  else {
-    $langcode = NULL;

@@ -111,7 +107,7 @@ function hook_tokens($type, $tokens, array $data, array $options, \Drupal\Core\R
-          $replacements[$original] = format_date($node->getCreatedTime(), 'medium', '', NULL, $langcode);
+          $replacements[$original] = format_date($node->getCreatedTime(), 'medium', '', NULL, NULL);

I just don't think these changes should be made at all - you want to pass the langcode on to format_date.

rajeshwari10’s picture

@alexpott

as we are removing the piece of code
$langcode = $options['langcode'];
}
else {
$langcode = NULL;

So, i have doubt how the $langcode will get the value.? in this line

$replacements[$original] = format_date($node->getCreatedTime(), 'medium', '', NULL, $langcode);

please guide.

thanks!!

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

ZeiP’s picture

Status: Needs work » Needs review

IMO the logic in hook_tokens() is fine as-is, so the patch in #3 is sufficient for this issue. Added a test for 8.2.x for the patch.

ZeiP’s picture

Status: Needs review » Reviewed & tested by the community

The patch in #3 passed testing and looks good to me.

amit.drupal’s picture

patch #3 is looking good stage and it working fine.
@mayurjadhav Thanks for patch.

catch’s picture

Status: Reviewed & tested by the community » Needs work

@rajeshwari10 @alexpott means we shouldn't remove that hunk of code, but instead use the $langcode that's set as the argument.

Anonymous’s picture

Unused variable in hook_tokens_alter() it is just copy/paste from hook_tokens when we need this variable. We can bit change for save it. Because it is hook, and variables can be useful as an example for own hooks. Also we can create one more variable "$language" for similar reasons)

ZeiP’s picture

Status: Needs work » Reviewed & tested by the community

@catch, the necessary change was already done in #3; IMO the comment by @rajeshwari10 was a misunderstanding about needing to change the other instance also, which works fine.

I think this issue should be closed with the patch in #3. The patch in #16 doesn't seem correct to me, since hook_tokens_alter() doesn't require langcode at all – adding it in another way seems counter-productive towards the goal of removing unused variables.

Re-setting to RTBC, revert to Needs work if there is more to be done on this besides the patch in #3.

xjm’s picture

Component: other » documentation
Status: Reviewed & tested by the community » Needs review

When we remove an unused variable, we should always ask ourselves how the variable got added in the first place.

In this case, the langcode is there because it was used in the hook example in Drupal 7:
https://api.drupal.org/api/drupal/modules%21system%21system.api.php/func...

Compare. D7 hook example:

    // Alter the [node:title] token, and replace it with the rendered content
    // of a field (field_title).
    if (isset($context['tokens']['title'])) {
      $title = field_view_field('node', $node, 'field_title', 'default', $language_code);
      $replacements[$context['tokens']['title']] = drupal_render($title);
    }

D8 hook example for the same lines:

    // Alter the [node:title] token, and replace it with the rendered content
    // of a field (field_title).
    if (isset($context['tokens']['title'])) {
      $title = $node->field_title->view('default');
      $replacements[$context['tokens']['title']] = drupal_render($title);

So the reason the langcode was needed in D7 was because field_view_field() required it as an argument. In D8, it's no longer necessary, because entity and field objects know their langcodes now. Removing langcode arguments was a big Entity API DX improvement in Drupal 8. So, the patch from #3 was actually correct.

However, looking at the conditional in D8:

  if (isset($options['langcode'])) {
    $url_options['language'] = \Drupal::languageManager()->getLanguage($options['langcode']);
    $langcode = $options['langcode'];
  }
  else {
    $langcode = NULL;
  }

$url_options['langcode'] is also never used that I can see. So we might be able to remove this entire misleading, useless, confusing conditional. What we need to do is git blame the hook documentation in D7, and see what issue introduced the use of $url_options, to determine whether it was ever used or whether it was always just cruft. Then, based on the outcome of that and others' review, we could remove the whole conditional, so the hook documentation just becomes:

function hook_tokens_alter(array &$replacements, array $context, \Drupal\Core\Render\BubbleableMetadata $bubbleable_metadata) {
  if ($context['type'] == 'node' && !empty($context['data']['node'])) {
    $node = $context['data']['node'];

    // Alter the [node:title] token, and replace it with the rendered content
    // of a field (field_title).
    if (isset($context['tokens']['title'])) {
      $title = $node->field_title->view('default');
      $replacements[$context['tokens']['title']] = drupal_render($title);
    }
  }
}

Whenever you remove an unused variable, please do the research to document why the variable was there in the first place. That way, we will catch bugs that the unused variables might be connected to. Thanks!

xjm’s picture

Oh, and just to confirm, the hook_tokens() example does still use the langcode, so that documentation example should not be changed. Only the hook_tokens_alter() example should be changed.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

andypost’s picture

As #19 said hook_tokens_alter() the only needs fix

Here's a patch for scope of the issue

But it needs follow-up in context of related to rework multilingual support of tokens (I recall few old issues with token's metadata as well)

For example fix broken "nid"

--- a/core/lib/Drupal/Core/Utility/token.api.php
+++ b/core/lib/Drupal/Core/Utility/token.api.php
@@ -92,7 +92,7 @@ function hook_tokens($type, $tokens, array $data, array $options, \Drupal\Core\R
       switch ($name) {
         // Simple key values on the node.
         case 'nid':
-          $replacements[$original] = $node->nid;
+          $replacements[$original] = $node->id();
           break;
 
ranjith_kumar_k_u’s picture

The above patch works fine on 9.2.x dev ,it removes the unused variables in the hook_tokens_alter().RTBC

ranjith_kumar_k_u’s picture

Status: Needs review » Reviewed & tested by the community
alexpott’s picture

Title: Unused variable in token.api.php » [backport] Unused variable in token.api.php
Version: 8.9.x-dev » 9.1.x-dev

Committed 3977710 and pushed to 9.2.x. Thanks!

  • alexpott committed 3977710 on 9.2.x
    Issue #2607116 by rajeshwari10, rakesh.gectcr, andypost, mayurjadhav,...
andypost’s picture

Patch applies to 9.1.x

alexpott’s picture

Title: [backport] Unused variable in token.api.php » Unused variable in token.api.php
Status: Reviewed & tested by the community » Fixed

  • alexpott committed c1135d6 on 9.1.x
    Issue #2607116 by rajeshwari10, rakesh.gectcr, andypost, mayurjadhav,...

Status: Fixed » Closed (fixed)

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