Tokens should use $options['langcode'] and not need a language object

Langcode is more lightweight, I think we used 'language' at places where we already knew we had the language loaded and we did not want to do it again, but it backfired with inconsistency and worsened developer experience. I agree we should standardize on one method and langcode is better. Also, we don't have a a load function to load one language for a langcode, which makes it cumbersome to get that language object. We started to attempt to solve that at #1260510: Introduce a language_load($langcode) but probably shoot at too big a goal, maybe we can get improvements in once we scale our goal back there.

BTW langcode might not be the final name for language codes in Drupal 8, but we don't have anything better at the moment. Eg. if that langcode comes from a node language code then it would be 'langcode' => $node->language which looks odd, right? (all schema columns use language to signify a langcode, uh) This is being endlessly debated in #1216094: We call too many things 'language', clean that up and we did not seem to be close to reaching a conclusion on that.

Point #2 in this comment should help you understand the background and current D8 status even more: https://drupal.org/node/1216094#comment-5101648

Yes, let's go with 'langcode' for now.

However, I do wonder though -- if we pass in a simple language identifier string instead of an object, then either individual token replacement callback functions need to convert that into language objects as needed (and also, individually, have to answer the question for how to deal with an invalid language identifier) - or - we make token_replace() do that string to object conversion on behalf of the caller, and only once for all token callback functions.

I'd prefer the latter, and would even dare to say that this might be backported.

@sun: ok I think that makes a lot of sense. Should be a good way to go. Also tagging for the base language system.

As well as the langcode spread tag :)

Dave! Would love to see if you could work on this. Thanks!

Working on this...

Here's a first attempt.

Do we need tests for the langcode/language parameter synchronisation?

Done for today..

Need to remove the language argument, not just make it optional by supporting langcode :)

Hm, like this? ;-) Let's see if this still passes tests...

oops double post =)

Thanks for the patch. The changes look good to me, but I'm wondering about the usage of both $langcode and $language_code variables. Would it be good to standardize on one variable name for consistency throughout the codebase? I've called out the usage below. If it should be standardized, $language_code is used more than $langcode so it would be easiest to use that.

+++ b/core/modules/comment/comment.tokens.inc
@@ -104,9 +104,9 @@ function comment_token_info() {
+    $langcode = $options['langcode'];

Uses $langcode.

+++ b/core/modules/node/node.tokens.inc
@@ -91,9 +91,9 @@ function node_token_info() {
+    $language_code = $options['langcode'];

Uses $language_code.

+++ b/core/modules/poll/poll.tokens.inc
@@ -40,9 +40,9 @@ function poll_token_info() {
+    $language_code = $options['langcode'];

Uses $language_code.

+++ b/core/modules/system/system.api.php
@@ -3752,9 +3752,9 @@ function hook_url_outbound_alter(&$path, &$options, $original_path) {
+    $language_code = $options['langcode'];

Uses $language_code.

+++ b/core/modules/system/system.api.php
@@ -3825,9 +3825,9 @@ function hook_tokens($type, $tokens, array $data = array(), array $options = arr
+    $language_code = $options['langcode'];

Uses $language_code.

+++ b/core/modules/system/system.tokens.inc
@@ -130,9 +130,9 @@ function system_token_info() {
+    $language_code = $options['langcode'];

Uses $language_code.

+++ b/core/modules/user/user.tokens.inc
@@ -63,9 +63,9 @@ function user_token_info() {
+    $langcode = $options['langcode'];

Uses $langcode.

Updated patch fixing #15. I decided to go with $langcode, as that seems to be the standard variable name throughout the code-base ($language_code was actually only used in test classes). This now includes one change which is not related to tokens in the TranslationTest class.

$langcode should be good!

The document need to be updated in in "mail.inc" where $option['langcode'] to be update for langcode. (But currently it is not affecting any thing). Also, user_mail_tokens is also a callback function from user.module.

* $options['language'] = $message['language'];
* user_mail_tokens($variables, $data, $options);

That sounds like NW ;-)

Well, that is an artifact of the hook_mail() semantics as far as I see, no? That should be a different issue, let's not overload this IMHO. Any other problems?

Fine by me.

+++ b/core/modules/user/user.moduleundefined
@@ -2297,7 +2297,7 @@ Your account on [site:name] has been canceled.
     // We do not sanitize the token replacement, since the output of this
     // replacement is intended for an e-mail message, not a web browser.
-    return token_replace($text, $variables, array('language' => $language, 'callback' => 'user_mail_tokens', 'sanitize' => FALSE, 'clear' => TRUE));
+    return token_replace($text, $variables, array('langcode' => $langcode, 'callback' => 'user_mail_tokens', 'sanitize' => FALSE, 'clear' => TRUE));
   }
 

Ok, so there indeed seems to be an issue here, since user_mail_tokens is not updated, BUT that does not use the langcode at all: http://api.drupal.org/api/drupal/core%21modules%21user%21user.module/fun...

So this seems like an unrelated bug here that the reset and cancel links are not generated for the email's or the user's language.

@webflo is opening an issue about it.

As for passing on the langcode here, since user_mail_tokens() would rely on the passed language object, we should pass the language object here for now IMHO, and then fix it later when/if hook_mail() and friends get converted to langcode instead of $language.

adapted the documentation

So the user mail tokens line is taken care of by #1754162: Implement language aware tokens for one time login link and cancel link and would be bugos to change here. It should be removed from this patch.

#23: tokens-langcode-option-1305378-19.patch queued for re-testing.

Reviewed this again with @webflo and @Schnitzel, and we figured out user_mail_tokens does not language currently either way and @webflo is fixing that in #1754162: Implement language aware tokens for one time login link and cancel link. So it is good either way for now, and the two patches can be independently worked on.

Also, this inspired me to open #1754208: Convert hook_mail() and hook_mail_alter() to langcode that should further improve consistency.

The testing issue was a glitch with the bot. So as said above, this should be good to go, and remaining language calls have other issues.

Shoot. Based on other stuff committed, this no longer applies. However, looks like a straight-forward DX improvement (and possibly performance improvement since we're passing around strings rather than objects) so I'm happy to commit once that's done.

Quick reroll.

Re-Bumping to RTBC since that's where it was before so I don't lose track of it.

Rerolled by hand, so waiting for tests results and a second check would be useful.

Forgot one chunk.

diff --git a/core/modules/system/system.api.php b/core/modules/system/system.api.php
index baef287..de8a2ae 100644
--- a/core/modules/system/system.api.php
+++ b/core/modules/system/system.api.php
@@ -3832,7 +3832,7 @@ function hook_tokens_alter(array &$replacements, array $context) {
     $langcode = $options['langcode'];
   }
   else {
-    $langcode= NULL;
+    $langcode = NULL;
   }
   $sanitize = !empty($options['sanitize']);

I broke code style on last attempt. Comparing patches, now looks good. The conflict was because a true was changed to TRUE.

#1688144-19: tnid does not updated to zero after node delete maybe related?

#34: tokens-langcode-option-1305378-34.patch queued for re-testing.

#30: tokens-langcode-option-1305378-30.patch queued for re-testing.

Ok, committed and pushed #30 to 8.x, since the remainder were necessary (and failing) due to #1688144: tnid does not updated to zero after node delete, which was rolled back.

Thanks!!

Added a change notice for this at http://drupal.org/node/1776688 - I don't think a CHANGELOG.txt entry is needed. Removing off of sprint as a result.