Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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'];
}
Comment | File | Size | Author |
---|---|---|---|
#26 | 2607116-26.patch | 862 bytes | andypost |
Comments
Comment #3
mayurjadhav CreditAttribution: mayurjadhav commentedHi rakesh.gectcr,
I think you miss out else condition.
Uploading patch to remove else condition.
Comment #5
rajeshwari10 CreditAttribution: rajeshwari10 as a volunteer and at Blisstering Solutions commentedremoved the unused variable.
Thanks!!
Comment #6
markdorisonComment #7
alexpottThis 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.Comment #8
rajeshwari10 CreditAttribution: rajeshwari10 as a volunteer and at Blisstering Solutions commentedAdding patch.
Please review.
Thanks!!
Comment #9
alexpottI just don't think these changes should be made at all - you want to pass the langcode on to format_date.
Comment #10
rajeshwari10 CreditAttribution: rajeshwari10 as a volunteer and at Blisstering Solutions commented@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!!
Comment #12
ZeiP CreditAttribution: ZeiP as a volunteer and at Citrus Solutions Oy commentedIMO 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.
Comment #13
ZeiP CreditAttribution: ZeiP as a volunteer and at Citrus Solutions Oy commentedThe patch in #3 passed testing and looks good to me.
Comment #14
amit.drupal CreditAttribution: amit.drupal as a volunteer and at gai Technologies Pvt Ltd for gai Technologies Pvt Ltd commentedpatch #3 is looking good stage and it working fine.
@mayurjadhav Thanks for patch.
Comment #15
catch@rajeshwari10 @alexpott means we shouldn't remove that hunk of code, but instead use the $langcode that's set as the argument.
Comment #16
Anonymous (not verified) CreditAttribution: Anonymous commentedUnused 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)
Comment #17
ZeiP CreditAttribution: ZeiP as a volunteer and at Citrus Solutions Oy commented@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.
Comment #18
xjmWhen 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:
D8 hook example for the same lines:
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:
$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: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!
Comment #19
xjmOh, and just to confirm, the
hook_tokens()
example does still use the langcode, so that documentation example should not be changed. Only thehook_tokens_alter()
example should be changed.Comment #26
andypostAs #19 said
hook_tokens_alter()
the only needs fixHere'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"
Comment #27
ranjith_kumar_k_u CreditAttribution: ranjith_kumar_k_u at Zyxware Technologies commentedThe above patch works fine on 9.2.x dev ,it removes the unused variables in the hook_tokens_alter().RTBC
Comment #28
ranjith_kumar_k_u CreditAttribution: ranjith_kumar_k_u at Zyxware Technologies commentedComment #29
alexpottCommitted 3977710 and pushed to 9.2.x. Thanks!
Comment #31
andypostPatch applies to 9.1.x
Comment #32
alexpott