Email text posting in the user's language

Agreed.

However, this is a localization issue, so moving it to that module.

new features go into devel version.

Changing version and title. This is still an issue.

This patch, together with #143249 (locale improvement: multilingual requests) will allow sending emails to users in the user language. There may be more than one email sent to different users in the same request, like when the user registers and the administrator gets a notification.

The patch adds a new 'user_mail' function that is a wrapper for 'drupal_mail'. It takes basically the same parameters, but instead of a 'to' email address it uses an user account. Also, it automatically handles user emails for known ids so subject and body parameters are optional and also variables for string substitution, which greatly simplifies the logic for all user emails in user.module.

- This function will also take care of body and subject for known user emails, doing the localization in the user language instead of the current language.
- The e-mail id is now a user email type for 'user_mail_text', and will be prepended with 'user-' to get the email id for drupal_mail function.

The patch needs some testing and the 'user email ids' may need some rework, but I'd like to receive feedback on the idea and the approach.

The good:

- centralized variables are cool!
- there is lots of duplicated code removed, which is very good :)

The nuances:

- what is exactly sent in to hook_user() if $user is not set? why would not it be set on a user editing page?
- "Selecting a different locale will change the interface and mail language and of the site." is bogus at the end
- there is some context related cruft at the end of locale_user()
- comment optional parameters to drupal_mail() by prefixing their description with "Optional"
- start the description of the return value on a new line, just like @param descriptions
- $key.'_subject' and the like should be $key .'_subject' (note the space after $key)
- _user_mail_text() has $langcode default to an empty string... IMHO we should match the t() API here and use NULL

Question:

What if we send an email to the site address? There is no $account in that case... You use user_mail('approval-admin', user_load(1), $variables), but this is not equivalent to sending to the site email address (which might very well be different from the user 1 email address). Previously this mail was sent to the site email address. One option here would be to allow passing a string in place of $account. If it is a string, it is regarded as an email address, and the language from language_default() is used to send the email.

As far as I see, this has a clear way to get in once the above issues are fixed.

Or even better, to avoid variable_get()-ing the site email in multiple places, if $account is NULL, or in other words !isset($account), use the site email address and the site default language to send the mail.

My major gripe with this patch is that it doesn't provide a good solution for contrib modules. They too want to send localized e-mails. Looking up the preferred language should probably be an API function.

Ideally, our API would enforce good practices; i.e. (i) making sure that e-mails are easy to localize and (ii) making it difficult to forget that e-mails might need to be localized. Both aspects are missing here, although, I'm not sure there is a good solution for this. Just thinking up loud in an attempt to trigger your brain cells to help me think this through.

The better the API, the less likely people will have to submit bug reports to tell developers that mail A and B can't be translated to the user's preferred language. If this system is going to be used for many years to come, it might pay of to think this through a bit more. :)

Here's an improved patch. Quite extended functionality.

I've fixed the issues Gabor pointed out and, thinking about Dries' comments, I've added a new feature that will allow composing texts once (emails) and run them through localization multiple times.

The idea is to build messages as arrays of simple texts, texts that need localization, and a set of arguments that will be replaced after localization.

As a sample implementation of this, I've done the contact module. Note how we compose the text once, and then we send it to both users (the sender's copy if requested) and for each of them it will be localized to the right language.

Now think of composing a notification once, and sending it to 1000 users localized to different languages. Also, as it is sent with user_mail, we have some user dependent variables there, like !username, that could be used to personalize each email .

The important code is here, for easier review. This would be in common.inc

/**
 * Text replacement with deferred localization
 *   
 * It can be used for the same base text to be localized to multiple languages
 * or when the language is not yet known when composing the text.
 * 
 * Example
 * - This will print the same message in all available languages
 * 
 * @code  
 *   // This first line will be localized with the username parameter
 *   $text[] = array('Hello !username,', array('!username' => $user->name)); 
 *   // This will be also localized, but the !content_type string has to be replaced later
 *   $text[] = array('A new !content_type has been posted to !site_name:');
 *   // Simple string, won't be localized
 *   $text[] = $node->title 
 * 
 *   // Variables for replacement
 *   $args['!site_name'] => variable_get('site_name', 'Drupal'); // Simple text, won't be localized
 *   $args['!content_type'] => array($node->type); // Array, will be localized
 * 
 *   // We have composed the base text, now it will be localized and printed for each language
 *   foreach (language_list() as $language) {
 *     $output .= '<h3>'. $language->name.'</h3>';
 *     $output .= tt($text, $args, $language->language);
 *    }
 * @endcode
 *
 * @param $text
 *    Plain text or array of text lines
 * @param $args
 *    Optional associative array of replacements to make after translation.
 *    * Array keys are placeholders, like in t() function
 *    * Array values may be simple strings or an array with string and arguments when it must be localized
 * @param $langcode
 *   Language code for localization
 * @param $wrap
 *   Optional, TRUE for wrapping each text line
 * @param $endline
 *   Optional string for end of each line
 * @return
 *   Text localized and formatted with variable replacement
 */
function tt($text, $args = array(), $langcode = NULL, $wrap = FALSE, $endline = "\n") {
  // If plain text, convert to array
  $text = is_array($text) ? $text : array($text);
  
  // Prepare arguments for replacement
  // This replacement also supports localization
  foreach ($args as $key => $value) {
      // If value is an array, it means it must go through localization
      $value = is_array($value) ? t(array_shift($value), array_shift($value), $langcode) : $value;
      switch ($key[0]) {
        // Escaped only
        case '@':
          $args[$key] = check_plain($value);
        break;
        // Escaped and placeholder
        case '%':
        default:
          $args[$key] = theme('placeholder', $value);
          break;
        // Pass-through
        case '!':
          $args[$key] = $value;
      }    
  }
  // Arguments have been parsed and localized
  // Now we go with the text.
  // Each line can be localized with its own parameters
  foreach ($text as $key => $value) {
    // It is an array, run through localization with optional parameters
    $text[$key] = is_array($value) ? t(array_shift($value), array_shift($value), $langcode) : $value;
    if ($wrap) {
      $text[$key] = wordwrap($text[$key]);
    }
  }
  // Put the text together and final variable replacement
  return strtr(implode($endline, $text), $args);
}

Are you aware of the progress at? http://drupal.org/node/144859 A similar change is made there to centralize email notifications. So these two patches are conflicting now (or to phrase alternatively, block each other).

Anyway, to comment on your latest patch, why is tt() duplicating much of the code of t() with handling placeholders? How would you envision the extraction of the strings in the arrays? How can extractor.php catch those? Also, there is an empty _user_mail_localize() in the patch. I am not sure we need this complication, just to be able to send hundreds of mails with some prepared text values.

I think Jose's patch is a step in the right direction. By enforcing this API, all e-mail will be translatable and developers will have a hard time ignoring that. *excellent* :-)

However, as Gabor pointed out, it is a little rough around the edges, and conflicts with Derek's patch. I'd prefer to see Derek's patch go in first, and then to integrate the language specific bits that will come out of this patch. Thus, I'd focus on the function to make it easy to localize the message.

Maybe Goba can help supervise this patch?

dww's patch got committed, so that should greatly simplify this patch!

Updated and improved the patch. Main changes:
- Updated for dww's user notification patch. Now _user_mail_notify() is a wrapper for user_mail(). The logic for predefined notifications is now in _user_mail_notify().
- Extended the array = callback idea. Now the first element is the function name. This will allow to extract strings for localization, as all of these arrays look like:

  array('t', 'string to translate'......)

It will also allow to use other functions as a callback, like what we'll need if we ever get some object translation into Drupal (http://drupal.org/node/141461)
- Renamed 'tt' function to 'drupal_text'. There's no reason to keep this name that short and the new one is more descriptive.
- Moved the placeholder logic from 't' function into a new one 'drupal_replace'. And one extra, this placeholder replacement will now be recursive, allowing also callback arrays. One example use of this is:

t('This !node_type is very interesting', array('!node_type => array('t', $node->type)));

This was not needed before because we weren't dealing with 'deferred localization'. Well, we need it now.

Btw, I've also fixed some issues with user_mail_text that looked like some bugs. I've had to change all the strings now to return callbacks, like

case 'register_no_approval_required_subject':
  return array('t', 'Account details for !username at !site_name', $variables);

I've changed some variable names in user_mail_tokens (site -> site_name) because if we are going to use them at a bigger scale, they need to be somehow more descriptive and avoid clashes with other variables that may be passed.

First reaction: incredibly big patch. Looked into it, and it includes additional files you probably not intended to include.

Done a code review. A TODO list:

- remove the user.module copy from the end of the patch!
- fix coding style issues in the phpdoc comments. drupal_text()'s comments contain invalid indendting, improper spacing, a colon at the end of $text's doc, but no explanation there
- $variables = array_shift($value); will throw an error if there is no third array argument... you could also use call_user_func_array() to simplify that code
- $langcode is not documented on drupal_replace()
- why is $args[$key] = $value; in drupal_replace()?
- you did not modify the site-wide contact form (I don't know if there is any other email sending code in Drupal which should be looked at)
- [!site_name] @subject does not seem to be right as an email subject (why HTML encode the subject there?)
- you modified contact mail Ids, although they have a hierarchical structure which should be followed
- 'user-'.$key is a coding style error
- you removed the password argument from register_pending_approval()
- user_mail_tokens() phpdoc includes copied code cruft
- user_load(1) does not result in the site email address, so it should not be used to send mail to the site email address

These are smaller issues, the clever use of this array structure is a brave move, but could turn out quite well. It is extractor friendly too, at least the extractor can be updated to work with such structures. Great work! Keep it up!

Fixed following Gabor's comments, and some improvements:

- For 'user_mail' the second parameter may be now a user account or an email address. If it is an email address, we search for a matching user account to get language. If none found, we use site default language. This is needed for sending emails from the site contact form, and to send emails to the site email address, which is not a predefined user
- New drupal_callback function to encapsulate callback logic expand possibilities with default values for callbacks -langcode is the one used accross this patch but there may be others
- Reworked some parameters to use this new callback function. I.e. for drupal_replace.
- Applied to site contact form. Which languages are used there for each email may need further discussion though, they're commented in the code.
- Added a security note about user input and text replacement. It is advised not to use user input as base text for replacement. Just preventing possible future issues. Notes on the code
- Split user_mail_tokens() variables in two groups, to return also some tokens when we don't have a user account.

About drupal_callback function

/**
 * Executes a callback array
 * 
 * A callback array is an array containing a function name and ordered parameters
 * The second argument allows to define default parameters for known functions
 * 
 * - Example:
 * 
 * @code
 *   // This defines the function to be called and its parameters
 *   $callback = array('t', 'Welcome to !site_name', variable_get('site_name', 'Drupal'));
 *   // This defines aditional default parameters for t() function
 *   $defaults = array('t' => array(NULL, 0, $langcode));
 *   // The result will be the same as
 *   t('Welcome to !site_name', variable_get('site_name', 'Drupal'), $langcode);
 * @endcode
 * 
 * @param $callback
 *   Array with function name and parameters
 * @param $defaults
 *   Optional default parameters for known functions. The array keys are the
 *   function names while the values are arrays of default values for that functions.
 */
function drupal_callback($callback, $defaults = NULL) {
  $function = array_shift($callback);
  // Build the parameters for the callback
  $params = $callback;
  if ($defaults && isset($defaults[$function])) {
    foreach ($defaults[$function] as $key => $value) {
      $params[$key] = isset($params[$key]) ? $params[$key] : $value;
    }
  }
  // Call function, or just pass on the first parameter if function doesn't exist
  return function_exists($function) ? call_user_func_array($function, $params) : $params[0];
}

Some comments:
Gabor,

- why is $args[$key] = $value; in drupal_replace()?

Because the value may have needed some processing if it is a callback

- [!site_name] @subject does not seem to be right as an email subject (why HTML encode the subject there?)

- you modified contact mail Ids, although they have a hierarchical structure which should be followed

I don't think I did... I just removed the prefix 'user-' because that will be added automatically in 'user_mail' function

- you removed the password argument from register_pending_approval()

That password is passed now in the $variables for replacement where it is needed

And yes, I think I could use some help to format all that help text in the comments... I'll go through it again when the patch is in final form, because now we are changing functions and parameters once and again...

I forgot the patch :-(

Well, I have gone through the code, because it is required to understand what is happening, and was confused by some documentation here and there. So I fixed phpdoc, variable names and other coding style problems. I did not modify any logic or test the code though.

Major things modified:
- internalized 'correct' and 'incorrect' comments in examples
- underlined that we are dealing with email text so HTML escaping should not be there (otherwise the examples are buggy, so I also reworked the text example to not output HTML)
- renamed drupal_text() to drupal_format_text()... the lack of a word here did confuse me in what this does actually
- fixed the missing password parameter I noted but you did miss, and did not fix in the previous patch

Outstanding issues:
- you did not actually use the $destination on user_mail() as an email address at the end, so the use case of that is quite unjustified, why not let the caller load the user if he wants to... you did solve the mail sending issues in contact module where an $account was not involved just fine without user_mail()
- needs to be tested

I played with the idea of somehow being more clever here with lambda functions. The problem with t() in this case is that language turns out later then the text and some placeholder values are also computed later. So we know of the text, and some of the placeholder values and need to add in the language and other placeholder values to the parameter list later, before execution. A working example I tested:

function t($string, $vars, $langcode) {
  var_dump(func_get_args());
}
function t_later($string, $default_vars) {
  return create_function('$more_vars,$langcode', 't('. var_export($string, TRUE) .', array_merge('. var_export($default_vars, TRUE) . ', $more_vars), $langcode);');
}
$text = t_later('Thanks for reading, !name', array('!name' => 'Drupaler'));
$text(array('!var' => 'value'), 'hu');

So t_later() basically returns a partially evaluated function call as a callable function, which we can pass the missing values later on. That would avoid this array madness :) BUT we need to identify the callable function in the text[] array and separate it from simple strings. PHP is clunky there that it returns a function name string with a zero byte at the start. So these are equivalent to call $func:

$func = create_function('$f', 'echo $f;');
$func('aaa');
$name = chr(0) .'lambda_1';
$name('bbb');

We should also be clever by passing on the right arguments as required by our lamba function. That might become complicated if we allow any kind of lambda there, because we have no information about what are the required arguments there...

So this would make the text definition code a lot more cleaner, but could possibly blow up the handling code. Anyway, food for thought.

Gabor: can you drive this patch home? Feel free to commit it when you consider it to be ready.

A couple of comments of my own:

1. "Can be used to localize the same base text to multiple languages or when the language is not yet known when composing the text." -- I'm not sure I understand this. In the PHPdoc, please explain _why_ this is valuable and _why_ the language might not yet be known?

2. Do we really have to support multiple/difficult callbacks in one array? What is the use case? If there is no clear use-case, I'd go for a global callback function and a single message string. I could always do multiple calls to drupal_format_text() if I need different callbacks. This would also make the $glue parameter redundant. Maybe I'm overlooking an important use-case, but if this is something that can be simplified, I'd like to see that happen. It looks like this function is trying to do too much.

3. Could other parts of Drupal reuse drupal_replace()?

subscribing

I put a lot more thought to this issue last night (found it hard to sleep). The partial evaluation callback mechanism introduced in this patch is quite powerful, but my gripe was: why do we complicate things this much? I thought we should focus on the problem at hand. We would like to send emails in the users' language.

There are a lot of ways to solve this:

1. Let the programmer think. Let him query user_language() and call t() with that language on the strings. => Dries said we should make it hard not to do the wrong thing, so this is not attractive

2. Let the mail handling system internalize the localization. This is what Jose proposes in the patch. As far as I see, this gets quite a bit more complicated "just" to get the emails in the user language. You need to master these new never seen special arrays with Drupal 6 to get an email sent. A possible simplification was what I explored above, with lamba functions. It can also be done by passing an object instance around, on which we can invoke an evaluate() method to execute the evaluation (now we need t(), format_plural() and possibly soon variable_get() support deferred evaluation).

3. Let the mail handling system call back the sender module to get the data. We did not explore this yet, but *this* is what we have previous practice with. If you look at drupal_get_form(), you pass on a form Id, it finds the callback function, gets the form array and builds the form. Now we can easily modify drupal_mail() (yes, the core mail function), to only allow passing of a mail id and variables, no subject, no body. The callback function will be named after the mail id, just as with drupal_get_form(), the subject and body will be requested from there. That callback will get the variables, so if the module developer wants to use text depending on certain values, it could be made available. This would enforce developers to segregate their mail text generation to a callback function. Now this gets us to many mail functions per module if more types of mails are sent, so this gets ugly in user module. It is not hard to use the prefix of the mail id instead and tack on a hook name, ie a user-register-approval mail would get to call user_mail_text() to get the mail, and the mail id will be passed as well as the variables and languages. This could allow modules to translate mails to the requested language always and keep the simplicity of translation as well as use an analogy to forms.

I will look into quickly doing a proof of concept for (3). Any comments?

Gabor,

Yes, agree, the previous callback idea was quite difficult to understand. The lambda functions are not easy though, and worse, you end up with a function which is actually a string, and you're going to use it mixed with text substitution, that is somehow quite hard to manage.

So I've reworked the drupal_callback idea to be somewhere in-between and I think it can be quite reusable. This takes us to this new 'drupal callback' concept I'd like to introduce which:
- It is an object with meaningful properties so it won't me mixed up with strings nor any other data.
- It also can take optional aditional parameters, $langcode in this case
- It is evaluated *always* through 'drupal_callback' which will automatically identify callback objects and will just return the value in any other case, thus drupal_callback($parameter) can be used just to run parameters through it.
- As the whole 'callback' is now structured data, It will allow some further processing, like smart parameter replacement if needed.

So, in the code, which uses #17 as the base so all Gabor's improvements are there, there's this new 'drupal_callback' and the 't_later' function which just produces a callback object.
The rest is just updating functions for this new callbacks and removing some array mess that is not needed anymore

/**
 * Executes a Drupal callback or just returns the first parameter if it's not a callback.
 * 
 * A callback is an special Drupal object that is produced by certain functions
 * like t_later(). It allows to pass arguments that are actually executable functions.
 * 
 * Example:
 * 
 * @code
 *   // The t_later() function will produce a callback object.
 *   $callback = t_later('Welcome to !site_name', variable_get('site_name', 'Drupal'));
 *   drupal_callback($callback, $langcode);
 *   
 *   // The result will be the same as
 *   t('Welcome to !site_name', variable_get('site_name', 'Drupal'), $langcode);
 * @endcode
 * 
 * @param $callback
 *   Callback object.
 * @param $param1, $param2...
 *   Optional aditional parameters for the callback function.
 */
function drupal_callback() {
  $params = func_get_args();
  $callback = array_shift($params);
  if (is_object($callback) && isset($callback->type) && $callback->type == 'callback' && is_callable($callback->execute)) {
    return call_user_func_array($callback->execute, array_merge($callback->params, $params));
  } else {
    return $callback;
  }
}

/**
 * Produces a callback for t() function, to be executed later with optional $langcode argument 
 */
function t_later($string, $args = 0) {
  $callback = new StdClass();
  $callback->type = 'callback';
  $callback->execute = 't';
  $callback->params = array($string, $args);
  return $callback;
}

About the user_mail $destination parameter -which btw, yes, it is used from _user_mail_notify- I still think it can be useful. Thinking even of extending it so it can also take user names as arguments. This will be useful for these features in which you have to provide a list of emails...

1. Please provide a few examples of the value of this callback system out of generating user mails. Unless there are strong contenders, I think we have simpler ways to achieve what we need (see #20).
2. $destination: the *caller* knows if he has a user name or a user email address or anything else. Why build complex logic into the mail function to identify a username or an email address, when the caller can simply to the user_load() himself?

Make this simple for the developer!

As requested, more hipothetical use cases for this callback system, which I think is quite powerful. However I see that this patch has somehow grown out of hand for the single use case of user emails, but now we've got to this point, I think these 'generic callbacks' can be useful for a lot of things.

Btw, will it help to note that these callbacks and texts with callbacks are full language-independent storable/cacheable data?

1. If the object translation finally hits core, this will need different callbacks, for 'dt' function to translate objects. See http://drupal.org/node/141461

2. The watchdog function use could be extended using callbacks. Also it could be simplified with this mechanism

// It could be reworked using callbacks, instead of 'string' + arguments
watchdog(t_later('message to localize', $variables)))

// And anyway, it will need callbacks to produce well localized watchdogs. Sample from node_submit
// Note that we are saving and presenting the message, with the raw node->type value

watchdog('content', '@type: updated %title.', array('@type' => $node->type, '%title' => $node->title), WATCHDOG_NOTICE, l(t('view'), 'node/'. $node->nid));

3. More powerful watchdog that can save and display full objects

// Example of log with extra information that will be rendered in display time
// So we can create *full* logs saving all the information sent back and forth
// This will saves current data, so when viewing the logs, you are looking at the actual data, not at the current user or node that may have been modified
$callback1->execute = node_view;
$callback2->params = array($node);

watchdog('Someone posted this !node', array('!node' => $callback1))

$callback1->execute = user_view;
$callback2->params = array($user);

watchdog('This user just registered: !user', array('!user' => $callback2));

4. More powerful object caching

// How to cache a page so it can be rendered for any user and language
// The permissions and localization can be worked out at display time
// We can think of extending the caching to logged in users!!

$callback->execute = node_view;
$callback->params = array($node);

// Now we cache a serialized callback for this path (/node/1), that won't need to be updated unless we edit that node
......

Ok, this are not inmediate needs, but I think it can give an idea of how powerful having some callback mechanism may be.
What we need IMHO, is also an api function to create callback objects, so the full object structure of these callbacks is completely hidden out of that functions.

function drupal_create_callback($function, $params...){
...
}

Well, the problem with this patch is that Dries indicated he would like to have a *simple* solution which *enforces* translated mails, and this is neither simple nor does it enforce sending of translated mails. If we introduce some complex callback system *only* used for this in Drupal 6, people will not care. They will not be familiar with this, and easily bypass with using drupal_mail() the old way. With or without this patch committed, drupal_mail() can still be used the old way to send emails just right, so there is "no business" in learning this new system as long as the old works just fine, and this is a lot more complex.

We would at least need a kind of t_later(), format_plural_later(), variable_get_later(), dt_later() as the current situation looks with other patches, so we can get translated site names and possibly other Drupal object properties into emails. We might need even more of these for more complex mails. This patch clearly outgrown the goals set out, and while the implemented system has several advantages, it does not solve the problem effectively. I took several hours with this patch through the last days, and it was not easy to grok what happens even for an i18n aware guy, like me. I tried to put a lot of thought into how we can make it simply and effective *for the problem at hand*, not trying to save the world.

Let's save this callback system for Drupal 7 or a nice contrib module, so other contribs can use it, and implement a simple and easy solution for this. What about cleaning up the drupal_mail() API and enforcing localization at the same time. A quick 'code mockup':

// new drupal_mail() enforces language, does not allow subject
// and body setting without considering language
function drupal_mail($recipient, $mail) {
  
  // Choose language for email and merge in common tokens.
  if (!isset($mail['#language'])) {
    if (is_object($recipient)) {
      $mail['#language'] = user_language($recipient);
      $mail['#variables'] = array_merge($mail['#variables'], user_mail_tokens($recipient));
    }
    else {
      $default = language_default();
      $mail['#language'] = $default->language;
      $mail['#variables'] = array_merge($mail['#variables'], user_mail_tokens());
    }
  }
  
  // Get module to invoke hook_mail_text() on
  $module = explode('-', $mail_array['#mail_id'], 1);
  // This will return #subject and #body values for mail,
  // replaced with variables passed on in $mail in the
  // language passed in $mail.
  $mail += module_invoke($module, 'mail_text', $mail);
  
  // headers, mail altering, smtp lib and mail() sending just
  // as before, but use the $mail array
}

// hook_mail_text() implementation, just a little bit more then
// what we have in user_mail_text() already in Drupal core
function user_mail_text($mail) {
  switch($mail['#mail_id']) {
    case 'user-...':
      return array(
        '#subject' => t('Blabla...', array('!var'=> variable_get('var', 1, $mail['#language'])), $mail['#language']),
        '#body' => t(...)
      );
      break;
  }
}

Note that I am not entirely a fan of arbitrarily using # keys (we don't need it as a distinguishing mark for arrays), but hook_mail_alter() already uses this format, so it is logical to use that format all around.

I think this approach is better because it does not enforce developers to learn a new concept, we don't need to rush in a set of confusing _later() functions, we generalize the mail text generation and altering to the same array structure and we disrupt drupal_mail() enough that everybody needs to think about properly generating mail subjects and text, otherwise they old modules will not work. Rather then inventing a new callback system, we get the looks of mail API closer to form API, and employ a similar callback system for defining mail contents.

Note that this is actually a *back to the roots* effort. If you look at your original patch, you did just this: pass on the language to user_mail_text() and get the resulting text from there. Dries pointed out that it does not solve contrib mails. You took a different direction, but going this way of generalizing the mail_text hook results in a simpler and more fitting approach as far as I see.

One more try with callbacks.

Now I've created a special predefined constant, UNKNOWN_LANGUAGE that when passed to localization functions will produce a callback instead of a string value. This should greatly simpllfy the API, and also we have the parameter marked for later replacement. Also, each function creates its own callbacks so it should 'know' about parameters.

function t($string, $args = 0, $langcode = NULL) {
  global $language;
  static $custom_strings;

  if ($langcode === UNKNOWN_LANGUAGE){
    return drupal_callback('create', 't', $string, $args, UNKNOWN_LANGUAGE);
  }
........
}

Other changes:
- Added this callback support to format_plural, format_interval, url, etc...
- Encapsulated the callback functionality in a drupal_callback function.
- Moved the $langcode parameter in url() function from the options array to it's own parameter, to be used with callbacks
- I've included into this patch an aditional $langcode parameter for some localization functions that were missing it. This could be moved to a different patch though

Some comments:
- Yes, the drupal_mail() function can be improved, but I don't think just breaking backwards compatibility on purpose is the best solution.
- The mail generation outlined in #24 looks like forms api approach but it actually spreads the mail composition into several functions -not like forms API which let's you create the form in a single function- and IMHO this makes more difficult to read and follow the code.

Some possible improvements, for the future, better in a different patch:
- Extend this callback support to drupal_render(), and then using this function to render texts, emails, etc.. This may allow including forms in e-mails.
- Rework drupal_mail api to support directly this new array text format with callbacks, passing the email to alter functions before all these callbacks have been executed and the text has been put together. This will break backwards compatibility btw, which sems to be 'good practices' here ;-)
- Add some page level caching for these emails so the same email can be sent to a few thousand users without too much processing.

Jose, in short I still don't see how this generic callback mechanism can (and will in Drupal 6) be reused elsewhere. If it is only used here, it is an overkill to have this generic, hard to understand mechanism. You only "UNKNOWN_LANGUAGE enabled" a small set of functions, so using this approach, we would only allow those small set of functions to be used for email text composition, blocking out most of the larger Drupal API.

I've included into this patch an aditional $langcode parameter for some localization functions that were missing it. This could be moved to a different patch though

Yes, it would be nice to include a correct patch for format_plural(), format_interval(), format_date() to $langcode enable them in another patch. I looked at this patch, and it does not really do this. It adds a $langcode parameter, but then does not pass that langcode on to *any* of the t() calls in them. So the purpose of the $langcode parameter is not fulfilled.

Yes, the drupal_mail() function can be improved, but I don't think just breaking backwards compatibility on purpose is the best solution.

Well, to not break BC with drupal_mail() you introduce user_mail() and a whole set of new conventions to generate an email, so people need to learn a lot more to adapt to the new system. I won't do it, if I were a lazy contrib maintainer.

Extend this callback support to drupal_render(), and then using this function to render texts, emails, etc.. This may allow including forms in e-mails.

I called up the drupal_render() API doc page to see how it works. It uses nice big form API arrays, unlike this patch, which uses callback arrays (again, instead of callback objects). I don't see how this can be useful for drupal_render(). Care to elaborate? I also don't understand the "forms in emails" claim. Drupal has no support for HTML emails, so having forms in emails seems to be a very far stretch to me to support in core.

Rework drupal_mail api to support directly this new array text format with callbacks, passing the email to alter functions before all these callbacks have been executed and the text has been put together. This will break backwards compatibility btw, which sems to be 'good practices' here ;-)

Indeed, we might need two levels of alters. Now the alter function can work on the final text, which has a value in itself (like wrapping the text in a shiny HTML page). Having an alter phase before the text is assembled with vars and placeholders is another option. I am not sure one or the other alone is enough. Anyway, this is not connected to this patch, it can be done either way.

Add some page level caching for these emails so the same email can be sent to a few thousand users without too much processing.

The problem with this patch is exactly this focus on mail enable everything, on the price of other parts of Drupal. Just to let url(), t() and other often called functions be used in this mail composition, you put a performance hit on what Drupal does 99% of the time: generate web pages. I don't think disrupting many of the core functions with selecting a few privileged ones to support later evaluation is justified to optimize sending thousands of mails a bit more quickly.

I also think that the callback mechanism is too complex. I was hoping we could come up with a _simple_ solution that is _easy_ to understand. Let's make something that is accessible to developers.

Ok, I give up with callbacks. I'm working out some forms-api like way of composing texts...
But I'd like to make progress before with the 'New variable defaults patch', which also has some impact on this one -we may not need the _user_mail_text function anymore, so I'll be back.

One more try. Let's create texts Forms API style.

• drupal__render_text() for rendering these 'text arrays'
• drupal_render_mail() for rendering emails with 'body', 'subject' and 'variables'
• text() and text_t() for convenience, and for marking texts for locale extractor, though they are not really needed.

This code pretty much illustrates the whole thing. Everything else is documented in the code. Callbacks, yes, but completely hidden into 'drupal_render_text'. This 'hidden callbacks' support all locale functions with language parameter, and they're quite extendable.

Other possible uses for this: Help texts, which are a mess of html and t() functions right now.

  // Format the subject
  $message['subject'] = text_t('[!site_name] !subject');
  // Compose the body
  $message['body'][] = text("!name-to,");
  $message['body'][] = text_t("!name-from (!name-from-url) has sent you a message via your contact form (!form-url) at !site_name.");
  $message['body'][] = text_t("If you don't want to receive such e-mails, you can change your settings at !url.");
  $message['body'][] = text_t('Message:');
  $message['body'][] = $form_values['message'];

  // Prepare variables for replacement. User mail tokens will be
  // added automatically in user_mail, so they're not needed here.
  $message['variables'] = array(
    '!name-from'     => $user->name,
    '!name-to'       => $account->name,
    '!name-from-url' => array('#type' => 'url', 'url' => "user/$user->uid", 'options' => array('absolute' => TRUE)),
    '!form-url'      => array('#type' => 'url', 'url' => $_GET['q'], 'options' => array('absolute' => TRUE)),
    '!url'           => array('#type' => 'url', 'url' => "user/$account->uid", 'options' => array('absolute' => TRUE)),
    '!subject'       => $form_values['subject'],
  );
  
  // Send the e-mail
  $message['#mail_id'] = 'user-contact-mail';
  $message['#from'] = $user->mail;
  user_mail($account, $message);

Just to make it clear, this patch does all these callback arrays and render functions, to avoid doing two things:

1. decide on a language
2. pass the language code around to the function calls

You go to great lengths to introduce meta-meta level stuff to avoid these two things... Like this default parameters descriptors:

+  static $callback_parameters = array( // Parameters for known callbacks
+    'url' => array('url' => NULL, 'options' => array()),
+    'l' => array('text' => NULL, 'path' => NULL, 'options' => array()),
+    'variable_get' => array('name' => NULL, 'default' => NULL),
+    'format_date' => array('timestamp' => NULL, 'type' => 'medium', 'format' => '', 'timezone' => NULL, 'langcode' => array('#type' => 'langcode')),
+    'format_interval' => array('timestamp' => NULL, 'granularity' => 2, 'langcode' => array('#type' => 'langcode')),
+    'format_plural' => array('count' => NULL, 'singular' => NULL, 'plural' => NULL, 'args' => array(), 'langcode' => array('#type' => 'langcode')),  
+  );

Seems like you had url() calls in the code without language codes, which have their results returned right away, so the links will not include language codes, or on multidomain sites will not results in proper links. Seems like we need to do all function calls differently when composing emails just to not decide on and specify a language code up front.

I made this simple example before to show what might need to be customized based on languages:

// Let's say these users all have different languages
$uids = array(5, 6, 23, 6);

foreach ($uids as $uid) {
  $account = user_load($uid);

  // Get the language code somehow (see more below the code)
  $langcode = user_language($account);

  $subject = format_plural($count, '1 new post', '@count new posts', $langcode);
  $body = t('Hi! You set up this alert to get emails when new posts come up on the site', array(), $langcode);
  $body .= t('Now alerts are sent you every !interval', array('!interval' => format_interval(..., $langcode)));
  $body .= format_plural($count, 'There is 1 new post overall', 'There are @count new posts overall', $langcode);

  $posts = get_new_posts_by_language($langcode);
  $body .= format_plural($posts, 'There is 1 new post in your language', 'There are @count new posts in your language', $langcode);

  drupal_mail($user->email, $subject, $body...);
}

// And we send mail to some email address
$langcode = language_default('language');
$subject = t(...., $langcode);
$body = some_function(...., $langcode);
drupal_mail('joe@example.com', $subject, $body);

It is not that much, but it can be diverse. Even URLs or some data might be added based on user language. Sometimes we might be smart by setting the language. When a user registers for example, he should get the mail in the page language and not the site default (the user has no language at that time, he might even get the page language set as a default a good choice).

Just to make it clear, this patch does all these callback arrays and render functions, to avoid doing two things:
1. decide on a language
2. pass the language code around to the function calls

Not really, the idea is to allow:
- Composing complex e-mails independently of a user/language
- Allowing mail/text altering passing semantic data, not a plain text glued together.
- Hide e-mail composing details, like the line endings or text wrapping into the mail API.

Besides that, the text rendering function was aiming at supporting text composition in device agnostic way, so it can be rendered later for any device: html, wml, email... Kind of: "do you want to receive notifications as: text email/html email/sms...". And the callbacks here were just a device to save a few dozen switch/case statements.

While these two use cases -user emails and notifications- are the only ones in Drupal core, and they're quite simple, we don't need to make anything complex for that, I was thinking more of making an useful api for any other aplication, like a notification module sending the same message to some thousand users.

- Composing complex e-mails independently of a user/language

That is what I said: avoid deciding on a language up front and passing that language around to function parameters.

- Allowing mail/text altering passing semantic data, not a plain text glued together.

This seems to be the new possibility here, so you can alter the text and parameters before passed on to localization. Altering before formatting but after localization is pretty simple to do by a few lines of modification in the current system, so you don't need the callback system for that.

- Hide e-mail composing details, like the line endings or text wrapping into the mail API.

That was so in the old system, wrapping and line endings were handled in drupal_mail().

Besides that, the text rendering function was aiming at supporting text composition in device agnostic way, so it can be rendered later for any device: html, wml, email... Kind of: "do you want to receive notifications as: text email/html email/sms...". And the callbacks here were just a device to save a few dozen switch/case statements.

We have pretty good text to HTML renderers called node content filters in core. You bypass this and introduce a whole callback system, for which every custom function needs to be explicitly mapped to be usable, and developers need to learn these array syntaxes or special function names just used for 'device agnostic text composition'. There is already an effort for Drupal 6 to allow different output renderings (XML, JSON, WML, whatever) for pages/content: http://drupal.org/node/145551

I don't think that as we have content filters to turn text to HTML and we have a push to support device agnostic output in a generic and less developer-disturbing way for Drupal 6, we really need another completely different way used for emails.

So far the above suggest me that the new possibility this patch really brings in is that I can alter mail text and arguments before localized and the replacements are already done. As I pointed out before, I don't think that this complication is worth the gain.

New version. Forget about callbacks. And thanks, btw for the thorough reviews on my previous experiments ;-)

• Compose messages using simplified text rendering function
• Pass messages as built array to user_mail(), which just adds user data and language, or drupal_mail().
• Use hook_mail_alter() to add any language dependent stuff, or for anything else
• Mail variables can now be reused by all modules so we need to standardize them a little bit

Notes:
- We could just send emails using drupal_mail($mail_id), if there's another module composing the e-mail with hook_alter().
- We can also use #text and #header as plain text, or text arrays for rendering. It will work either way because drupal_text_render works also for plain texts.
- As variables for text substitution are provided now with a hook, they need some renaming, kind of prefixing with module name to avoid clashes, not completed yet for all...
- Yes, the rendering functions support complex nesting. One use of that is for user_mail_ variables though we could also add the text in hook_mail_alter(). But the code is quite simple, with recursive functions...
- Of course, all this doesn't really help that much for the over simplified 4 use cases of mails sent from Drupal core, but we want to provide some useful api for other possibilities, don't we?

Err.. are we getting closer? (Don't bother commenting on coding style and comments, I'll clean it up if it looks like we are close to something....)

Looked though the patch. I am still not conviced that this whole later evaluation engine, the custom function names to learn and all are worth sending mails in different languages. We started from a simple patch. Dries voiced some concerns, but pointed out that it might be too complicated to implement to be worth it. I think we are exactly at that point.

To be a bit more constructive: we are not far away from returning the actual subject and body in the callback invoked by the mailer function. Now you add language specific variables there, but we can just as well add the language specific strings there. That way we could reuse the existing functions already in Drupal (t, format_plural and so on), reduce complexity a whole lot and support easy, enforced language specific emails.

I was trying to advocate using a mail id and a callback to invoke based on the mail ID to get the subject and body (and language specific variables) way back before this point. I think that enforces that developers think about languages and allows us to reuse the existing APIs without building whole big layers of specialized stuff on top of them.

I think it is important to share the established APIs and reuse as much as possible (so much that I sticked to supporting Drupal.formatPlural in the JS localization patch too with this exact name, not with a special calling convention of Drupal.t()). I think we should not have domain specific stuff for the established APIs all around.

Greatly simplified version. Switched the approach to get something simple and working:
- Helper functions like _contact_mail() to build e-mails easily for different languages.
- Adding newly created password into $contact object to simplify parameter passing.
- Added language parameters everywhere for _user_mail_text()
- Moved call to user_mail_tokens() -with extra language parameter- into _user_mail_text()

We are running out of time for Drupal 6 new features, so I've added just the bare minimum to get the emails delivered in the right language, while reusing the new mail api recently committed.

So I think we should forget about an specific language aware mail api for now...

This is getting to be super-simple :)

Some comments:

- I was again horrified by the patch size, before notifying that a "Copy of user.module" sits at the end of the patch. This huge size made it a lot less attractive to review.

- drupal_mail() should have information about the language used! Amnon works to RTL-enable emails (now in the html email module, or whatever it is called, I cannot remember), which needs the $language information passed along to see if the mail is actually sent in an RTL language.

- what about making this a bit more super-simple? :) Now we always pass on a "mailkey" as the first argument to drupal_mail() but we also collect the subject and body based on a kind of "mailkey". These keys are not the same, but we need to use them manually anyway. What about adding a *little* bit of automation to the mix? Both mail sending modules have the subject and body generating functions now in core with this patch and they can act as callbacks. drupal_mail() can simply look at the first word in the mailkey and assume it is the function name and as the hook_mail_text() or something to provide a subject and a body for the given mailkey. This hook could get the "$op, $values = array(), $language = NULL" parameters as it is now with both of them.

IMHO this simplification makes the mail API easier to use and automates the callback mechanism, ensures people think about depending on languages with their mails.

I would imagine: drupal_mail('contact-user-mail', $to, $values, $language, $from); so the body and subject will only be defined once hook_mail_text() is called back with the mailkey ('contact-user-mail' in this case). That hook could also return the subject and body in an array, so initializing the language and tokens twice as in the user mail text function currently implemented in this patch would not happen. It makes the email composition a little bit more performant.

Thanks for keeping up the work on this patch! I think we are getting close to a solution to commit.

Again :-(, sorry about files that slipped in.

So, I've changed it to a forms api like structure....

- New drupal_get_mail(...
- Changed drual_mail(, now it takes the whole mail array as parameter
- Introducted hook_mail_text(
- Most of the mail composing and default values moved to drupal_get_mail(),
- Wrapping is done automatically in drupal_get_mail()
- Mail body can be composed as an array, that will be glued together after _mail_alter().

The reason I've split the functionality in two functions is to support 'composing the email once/send many times' feature, which should be useful for mass mailing performance.

Not extensively tested, hope to get some feedback on the implementation before...

Looked at the code:

- I don't like a function named drupal_get_mail() to be used to send email directly. The name says I *get* the email, not *send. As this is supposed to be the actually used function, we can name this to drupal_mail() and the sender function to drupal_mail_send() or something along these lines. I would also default $send to TRUE, as that is the default use case.
- Steven noted earlier in the mail sending patches, that the hash (#) indexes are not required, and look ugly as there is no functional need for them. In the form API, they distinguish between the form item nesting and properties, # provide the attributes, the normal keys provide the nesting. Here there is no such requirement at all. Let's make this simple as Steven wished ;)
- contact_mail_text() looks cool :)
- $message['headers'] could be directly used in place of $defaults, so the miselading $defaults variable name can be avoided
- I don't think all modules should be called to return message details for a certain message... Only the module signified by the first word of the $mail_id should be called... other modules will have the mail alter hook to chime in with modifications
- The code comments above drupal_mail() {to be renamed drupal_mail_send()} in the code should have the array index doc part indented
- "If user ir being created, so we set user language to the current page one" should be "If user is being created, we set the user language to the page language"
- user_mail_text() and _user_mail_text() should be merged
- Dots are missing at end of @param comments and in function comments like at user_language()
- "Optional anguage to use for the notification" typo

None of these are fundamental problems, I agree with the direction this patch took, so it is on the way to get committed.

Gabor, addressing all your issues:
- I don't like a function named drupal_get_mail() to be used to send email directly. The name says I *get* the email, not *send. As this is supposed to be the actually used function, we can name this to drupal_mail() and the sender function to drupal_mail_send() or something along these lines. I would also default $send to TRUE, as that is the default use case.
+ I didn't like it either. Done
+ Added subject, body, headers paramter so it can be used also without a callback for composing the e-mail. Much similar to the current drupal_mail().

- Steven noted earlier in the mail sending patches, that the hash (#) indexes are not required, and look ugly as there is no functional need for them. In the form API, they distinguish between the form item nesting and properties, # provide the attributes, the normal keys provide the nesting. Here there is no such requirement at all. Let's make this simple as Steven wished ;)
+ Agree. Removed #, I had just used it because it was already there

- contact_mail_text() looks cool :)
+ Yes, it does :-) Renamed to 'contact_mail' though. Changed parameter name for consistency and fixed some minor issues there.

- $message['headers'] could be directly used in place of $defaults, so the miselading $defaults variable name can be avoided
+ Yes, but if you look at the code there, having $defaults allows some more clean and shorter code. Not changed

- I don't think all modules should be called to return message details for a certain message... Only the module signified by the first word of the $mail_id should be called... other modules will have the mail alter hook to chime in with modifications
+ Ok, but the first word may not be the module (there are module names with underscore) so I've added an aditional $type there, useful to build the callback and to allow _mail_alter based on _mail_type

- The code comments above drupal_mail() {to be renamed drupal_mail_send()} in the code should have the array index doc part indented
+ Cleaned it up. Indented and changed some explanations that didn't really applied there.

- "If user ir being created, so we set user language to the current page one" should be "If user is being created, we set the user language to the page language"
+ Done

- user_mail_text() and _user_mail_text() should be merged
+ That's not that straight forward, because of that variable_get(). I think it's cleaner how it is now.

- Dots are missing at end of @param comments and in function comments like at user_language()
+ Added dots. Also updated some parameter descriptions.
- "Optional anguage to use for the notification" typo
+ Fixed

So, summarizing, I've cleaned up most of the issues, renamed functions, and added old parameters do drupal_mail so it allows composing and sending emails with that single function without callback, like the old one, while we have also drupal_mail_send, which is a lower level funcion that can be used to send an email many times without going through all the composing and rewriting.

One thing I'm not sure about is whether is make sense to automatically wrap the body *always* or we may need one more parameter to wrap it or not...

- I don't think drupal_mail() should be directly usable with the subject and body. If people want to specify that, use drupal_mail(), and compose/process their mails properly.

- Renaming hook_mail_text() to hook_mail() makes sense, it can provide any of the mail values, not just the text... But this is still a hook_mail(), and the function comments should reflect that. Think about the module defined node type hooks, like forum_delete(), forum_insert() and so on... These are only called for the given node type, so they are very similar to hook_mail() in how they work.

- I feel strong about the need to integrate user_mail() and _user_mail_text(), I don't understand how the variables can make it any complex.

Otherwise I think this patch looks very good.

Ok, so
- "Implementation of hook_mail()" comment
- Goodbye to _user_mail_text()
- Removed body and subject parameter
+ Made 'from' parameter optional, for which I've had to change the order. Defaults to site_mail, as before.

Well, I went through the patch.

- Corrected many coding style issues (mostly '} else {' stuff, and missing dots at end of sentences)
- Added more documentation, made existing docs more verbose.
- Made some small simplifications, like renamed $type to $module as it is a module name and 'mail_id' to 'id', as we are in a message array anyway.
- Also simplified code in drupal_mail_send().
- Simplified language on user form to not mention locale, but keep with language, fixed form group title.

Tested contact form mails, and found that the language_list() function does not work properly if there is no languages table yet... Obviously we need to check whether we have more then one language there, and only disturb the database if we do have, in which case we have that table.

Seems like this is out for some final testing and to see what Dries says.

Gabor, your changes look ok to me. I've just found:
- a typo 'e-amil' in user_mail comments,
- some not needed $result = NULL in _user_mail_notify()
- inconsistent use of 'e-mail', 'email' through the comments

This is not a new revision of the patch but a test script. After 12 versions of this patch, I know how tedious it can be to test the user emails part, and specially now that this latest version doesn't work with devel module.

So I have this test script, that will test and output (var_dump) all user e-mails. Just place it in the root folder, rename to 'test_user_mail.php' and try http://yoursite/test_user_mail.php...

And enjoy !:-)

(But please don't forget to do some 'real' tests too)

Dries asked for clarification on why we need hook_mail(). Let's look at this use case:

- we have user a contact form, where another user enters some text
- she asks for a copy of the mail

Now we need to send two emails. There is a common part of the two mails, the information provided on the contact form. But there is also a mail template used to send the mail, which we should send to each user in their own language. What we need to do:

1. Grab values from the form.

2. Decide on the recipient user language.
3. Compute email subject and body based on the mail template, translated to the recipient language, with replacement values from the form.
4. Send the mail.

5. Decide on the copy user language. (That is the language of the user we need to send the copy to).
6. Compute email subject and body based on the mail template, translated to this "new" language, with replacement values from the form.
7. Send the email.

What the patch does is that it abstracts the (3) and (6) steps out to a hook_mail() so that when we need to send emails with the same template but with different replacement values, we can call the hook to grab the processed template with the replacements. The mail template could include site URLs or any other type of content which should be different based on language, it is not just a matter of t()-ing the subject and body template strings and doing a placeholder replacement.

So hook_mail() helps the reuse of mail templates, when we need to send multiple emails with the same template but with differrent languages and/or placeholder replacement values. User module already did this abstraction naturally with _user_mail_text(), the patch makes this generic. All-in-all this was a natural fit to the Drupal core use cases, and I believe that it enforces developers to think about their emails as to-be-localized text.

Gabor, thanks for the explanation/use-case. I think that information is crucial, and it should be part of the PHPdoc.

Right now, the documentation starts of with "Compose and optionally send an e-mail message, using Drupal variables and default settings." and then moves on to explain some technical mumbo jumbo. We need to explain long and clear how this works, and why.

We should make the PHPdoc a lot more _practical_ for people that are no Drupal experts. Let's not be afraid to add a couple more paragraphs of documentation for people to read/use. Don't be afraid to add examples either.

Shouldn't that big switch-case in user.module have breaks?

I'd rename user_language() to user_preferred_language(). In the PHPdoc of user_language(), I'd add one sentence to mention how the the preferred language is determined. As a developer, I'd like to have a clue as where this value comes from, or how it has been derived from different sources. How is it defined for anonymous users? How is it defined for authenticated users? Does it also look at the HTTP headers? Does it look at a configuration setting on the profile page? Let's be a bit more explicit about this in the documentation.

The code itself looks good. Add some more documentation with a focus on practical value, and this should be ready.

Great, here is an updated patch. Code changes:

- removed the $result = NULL noticed by Jose,
- renamed user_language() to user_preferred_language() as Dries suggested
- added break-s into the user_mail() switch, which were required, noted by Dries

Documentation changes:

- *lots* of explanations about how all this works on top of drupal_mail(), now that we have a distilled picture about it :) Also added an example, explained language decisions in great detail, to make this easier for developers.
- explained how user_preferred_language() works for different types of users, and where the language comes from

I hope to get this tested by at least somebody else additionally to me, so we can RTBC.

I looked at the code and documentation and it looks good. The design looks sounds. I haven't tested the patch though.

Well, while preparing to test the patch, meba pointed out that _user_mail_text() is used in all the settings forms, so we should not fold that in to the user_mail() hook. Let me apologize to Jose, for pushing for that, it makes code reuse harder.

Here is an updated patch, which makes _user_mail_text() usable for settings forms again.

The patch applies succesfully. I used slightly modified test_user_mail.php and seems to work OK.

Got a note from meba, that the settings pages gave notices about undefined objects. Fixed that in this update, retested the code, and committed.

The update guide should be updated with changes introduced here.

Sorry to enter so late into this thread. I was sent here via CVS annotate on drupal_mail() in HEAD right now, trying to port update.module to the new way to send emails (http://drupal.org/node/94154). I've tried to read this whole thread and process it as well as I could.

I must admit, I don't fully understand all the nuances of the translation problems you're talking about here. But, as a developer, this new approach is *very* complicated, and actually makes sending email quite a challenge in D6. :( I don't think the goal of "make it simple, but force developers to think about language" has been achieved. Here's a low-level, technical example of the problem I'm facing with the new design...

When update.module finds your site out of date, there are 2 possible message texts. Roughly:

1) "New updates are available. Please update as soon as possible."
2) "Security updates are available, UPDATE NOW YOU FOOL!!!". ;) (well, not quite, but you get the idea).

Each of these 2 statements could be true for either Drupal core or "one or more of your installed modules or themes" (aka contrib). Plus, either core or contrib could be up to date. So, effectively, there are 3 choices (everything's cool, something's out of date, something's insecure) for each of 2 things (core vs. contrib), which gives you 9 possible combinations, 8 of which generate an email (no email is sent if both core and contrib are all cool).

I honestly have no idea how to make this work in the new system, where the email body is generated by an isolated callback, totally separated from the code deciding who to send the emails to and what to send them.

In the old system, this was trivial. I just programatically built up a $body variable, appending the appropriate strings, and called drupal_mail().

In the new system, it seems like the only viable solution is to have 8 possible email keys, and have a huge switch statement in my hook_mail() implementation, to return the right text based on which key was passed in. :(

Do I just misunderstand the new approach? Or, was it written to fundamentally assume a small number of hard-coded templates, with only variable substitution as the means to customize? Should I just handle this problem as if these are variable substitutions? For example:

function update_mail($key, &$message, $params) {
  $language = something;
  $message['body'][] = t('@core @contrib', array('@core' => $params['core_msg'], '@contrib' => $params['contrib_msg']));
  ...
}

?? That seems like quite a hack to me. :( The alternative seems worse:

function update_mail($key, &$message, $params) {
  switch ($key) {
    case 'core_security-contrib_uptodate':
      $message['body'][] = t('Core is missing security update.');
      break;
    case 'core_security-contrib_stale':
      $message['body'][] = t('Core is missing security update.  Contrib is out of date.');
      break;
    ...
  }
}

Sorry if I'm being dense, but this new API is making my head spin, and it appears to be vastly harder to send programatically generated emails. :( Not sure what to do about this. Honestly, I wasn't aware of this issue until Gabor pointed it out in the update.module thread. I'm all for internationalization, but this seems like quite a step backwards in terms of developer clarity.

Can I get some help over at http://drupal.org/node/94154? Can someone explain the right way to solve this problem? Is there anything we can do to support this kind of use case in a more sane way given the drupal_mail() and hook_mail() design?

Again, sorry I didn't see this coming sooner, but my hands have been incredibly full. :(

Thanks for any input, help, advice, "you dumb American, it's easy, just do XXX" scolding, etc. ;)

Cheers,
-Derek

Hi Derek,

I agree it may be more complicated, though not that much. It just tries to enforce language selection -for which you'll need to know the receiver in advance- before composing the e-mail. And in addition to that, it lets the _mail_alter() modules know about $language and parameters for this e-mail.

So, you can think of it as a theme function, to which you pass a $params array data, or you can move all the logic into update_mail(). As update_mail can add any part of the e-mail, you can build the whole thing there.

Alternatively, if none of the previous approaches works for you, you can build an empty e-mail, then fill it in, then send it:

$language = language_default();
// Passing FALSE as the last parameter won't actually send the mail, but return an structured array populated with default parameters for headers, etc....

$mail = drupal_mail('update', 'status', $to, $language, ..... FALSE);
// Implementation of the 'update_mail()' callback is not mandatory.
$mail['subject'] = ...
$mail['body'] = ...
drupal_mail_send($mail);

Derek, it would be great to get input from others about this issue, unfortunately only Dries was around to review. If this is truly too complicated for those not into language handling, then we might need to rethink it, as email sending is a crucial part of any web application. While getting mail sending ready for multilanguage operations, we ended up with the following workflow:

- You decide on the language the mail will be sent, as it is unique. You know what language the recipient understands possibly, so you use user_preferred_language($account) if you have a user, or global $language, if there was a form completed which initiated the mail (so you can expect the human who completed the form to understand that language), or you can fall back on language_default().
- You compose the email with the given language in mind.
- You send the composed email to the recipient.

It is crucial to decide on the language used. In your examples, you used t() without the language parameter, which would always work with global $language, and is not correct oftentimes. You also used $params['core_msg'] and $params['contrib_msg'] to build the message in your other example, which assumes that these strings are already localized into the language you would like to send the mail with.

We ended up with the drupal_mail()/hook_mail() design because we found that there are common things done in email composition. There are common tokens used in the emails sent out by a module, which should be set up, some parts of emails are shared, although the emails are sent from different places in the module code. These were all abstracted in the core modules already, and it made sense to automate this to have a common pattern.

As Jose pointed out, you can really think of hook_mail() as a kind of template/theme function. You pass on $params which are used for replacement in prewritten email templates. Your hook_mail() implementation ensures that the email templates are translated to the language required.

Because the language you choose for sending an email highly depends on the actual task you solve with the code, it cannot be automated. We have only been able to abstract the mail template functionality.

Derek, although none of your examples showed that you actually taken the language into account, your alternative example could be reworked quite easily to concatenate messages from t()-ed sentences, if you think this looks better. Of course the smaller parts you concatenate the big message from, it gets harder and harder to translate, as the larger context is not there. This is why core and many contrib modules use standalone mail templates, even if things like the "site signature" repeat in them. I don't see here, how much would you repeat in each mail, so what to do there is your decision.

Let me repeat that it would be great if more people would see this design and either cry out loud, if it should be redone ASAP, or acknowledge that the mail sending code became a bit harder to understand, but not pointlessly.

Thanks for the replies. I was trying to be brief in my examples, but apparently I was too brief. Of course the strings have to be translatable at some point. ;) (even a dumb american like me knows that much). ;)

In terms of the language choice -- this is just a raw list of email addresses we get from an admin setting. The only way I could find a better language than language_default() is to iterate over the list, query the {users} table for each email, and if I find a match, it corresponds to a Drupal user and I can use their preferred language. Otherwise, language_default() really seems like the only viable option (this email is triggered by hook_cron(), not a form, etc).

In my case, I'm just re-using the translatable strings from hook_requirements(), since it's the identical message I want to display at admin/logs/status as should be the body of the email. So, for example, in update_requirements(), we do stuff like this:

 $requirements['update_core']['description'] = t('There are updates available for your version of Drupal. To ensure the proper functioning of your site, you should update as soon as possible.') 

So, I was imagining that at the callsite where I'm trying to invoke drupal_mail(), it'd be something like this:

$status = update_requirements();
$params = array();
$params['core'] = $status['update_core']['description'];
$params['contrib'] = $status['update_contrib']['description'];
drupal_mail('update', 'status_notify', $target, language_default(), $params);

And then, hook_mail() would just be something like this:

function update_mail($key, &$message, $params) {
  $message['body'] = $params['core'] .' '. $params['contrib'];
  ...
}

Does that make more sense? Is that the recommended way to handle this situation?

I really don't understand the translation side of this very well. It's not clear why a "mail builder" callback hook_mail() makes this easier to translate, but I'll definitely trust y'all on that. Jose even suggests I could just generate an empty email and then populate it right before sending it, bypassing hook_mail() entirely. If that's allowed, does it make matters worse for translators? If it's no harder to translate in that case, then it seems hook_mail() is added complication that doesn't make things better. If bypassing hook_mail() makes life difficult in some way, it'd be helpful to understand why, so that module developers have a sense of what this extra hook is allowing and why they should cooperate with it.

Again, I don't at all mean to question the hard work that went into this issue, and I'm genuinely interested in helping. I'm just looking at it with a fresh pair of eyes and trying to make sense of it. Hopefully the troubles I'm having here, the solutions we come up with, and the resulting improvements to the docs will make it easier for others to follow. ;)

Thanks,
-Derek

OK, let's look at this code:

$status = update_requirements();
$params = array();
$params['core'] = $status['update_core']['description'];
$params['contrib'] = $status['update_contrib']['description'];
drupal_mail('update', 'status_notify', $target, language_default(), $params);

If it is ensured that this only runs on cron, that is no page view will initiate this code, then (because cron runs with the anonymous user AFAIR and the anonymous user has the site default language set), all will be right.

But your module has a manual check option, which can be initiated from the web. Now image that your administrators speak different languages, and they prefer different ones. Now when a German administrator initiates a manual check, she should get the status report in German (the page langauge used because of the user preference of the current user), but you don't want to send emails in German, as other admins might not understand that, it is not the common default language of the site, which every admin understands.

Reusing update_requirements(), which uses t() calls without language parameters, which thus translates to the current page language only makes sense, if the current page language is always understandable for those who you send the email to. If this code only runs on cron, and never will be run on a page requested by a human, then you can reuse update_requirements(), otherwise you need to let the system use a different language for the page and the emails, which this reuse does not allow.

Basically there are two situations when you need to think about using a different language for the page and the email(s) being sent:
- when the page view was initiated by a user, or an alternate language domain/path was used even for an anonymous user, like /de/contact, /it/contact etc.
- when at least one of the emails sent are to be sent to a user
When neither of this is true, you can send email in the same language as the page.

Drupal 6 has a domain per language feature. You can have italian.example.com, german.example.com and example.com, the last possibly being in the default language (say Danish). If italian.example.com/cron.php is invoked, because cron.php does a full boostrap, the Italian language will be the page request language, which the admins might not understand.

So in your case, if you send only from cron *and* the cron is invoked on the default language domain *and* you only send to non-users, you can reuse the update_requirements() output, which generates text in the page language, which will be the default language in this case, which we assume all admins understand.

If you look at the update.module code (update.fetch.inc, to be precise), yes, this email notification only happens during cron. It's in a function called _update_cron_notify() in fact, to be all self-documenting and everything. ;) Checking manually doesn't trigger this function, so that's not an issue. This is by design (details are more relevent in the other issue, if we want to discuss this further).

So, assuming they're invoking cron.php in a sane way (I honestly can't see how I'm supposed to defend against that in the code), it seems like my proposed code snippets are the right ones. Again, these emails might not be going to other Drupal users at all, but pagers, other monitoring systems, whatever.

Understanding how to set $language is helpful, but that still doesn't answer my question about what hook_mail() buys us. If it can just be left out of the workflow, and nothing is worse for the translators, why have it in the workflow at all? It seems (based on my limited understanding) to assume that Drupal code has a single, static message template that is invoked multiple times at different call sites. Maybe you have a small # of different templates with different keys. But, basically, it's just there to enforce that the template code is shared. Is that really what it's for, and it has nothing to do with translation per se? If so, it should definitely say that in the phpdoc, and I shouldn't be using it here at all. But, those of us who don't live and breath internationalization issues don't know this. The only docs about hook_mail() are in drupal_mail(), and the bulk of those comments are along the lines of "getting the languge right is important"...

Thanks for your patience in working with me through my attempts to understand the new API. I *promise* I'll post a patch against the phpdoc to make it more clear, once I really know what's going on. ;)

Well, you could defend against people calling cron from other language subsites by actually enforcing language_default() for the composition of the email text, not only expecting that because of how the code is structured *now*, it will be the language used anyway. This is nitpicking though.

Sending e-mail from default language cron calls to non-users is a minority of what I imagine Drupal modules send mail at. Most email sending involves a user either seeing the page or receiving the email. hook_mail() has the following roles:

- it centralizes all the mail templates you use
- it assumes you use the decided mail language to translate the templates (think of hook_mail() as the place where you *should* compose stuff with *not* the page language most of the time)
- it offers you the possibility to centralize common tokens in your mails (!username, !timestamp, whatever), set common mail headers, etc.
- it offers a common, well known place to retrieve the mail from if sending the "same" mail to different destinations with different languages/replacement values

It is not an effective tool if you expect to send mails in the page language always, and you don't have multiple mail templates and you don't have replacement values in the mail templates. This is the actual situation with update module. hook_mail() seems to be overkill there, for sure.

Otherwise, hook_mail() was good for contact module, because we needed to reuse the same mail to send to different destinations with different languages (think about the request a copy feature), and we centralized tokens. User module also centralized tokens and the user mails are customizable, so handling of the customized values is also abstracted (although with the variables patch not landed, it is not as nice as it could have been ;)

So we have seen that hook_mail() is good for core modules as we would do:

$language = ...
$variables = array_merge(our variables, common tokens);
$subject = our_custom_subject_abstraction($key, $variables, $language);
$body = our_custom_body_abstraction($key, $variables, $language);
drupal_mail($key, ... $subject, $body, $language);

Now we do:

$language = ...
$variables = array(our variables);
drupal_mail($module, $key, ... $variables, $language);

my_mail($key, $message, $variables) {
  ...
}

And it looks up what we would do manually. So we basically abstracted out the array merge, and the custom lookups (used in case of mail text reused more then once in a module, or mail text customizable on the admin interface). At the same time we put all "you need to think about a different language, not the page language" stuff into hook_mail().

Thanks for all the input. As a result of this, I've posted a new patch over at http://drupal.org/node/94154#comment-269438 (comment #87) for update.module. Please give it a look.

Sometime in the next few days, I hope to have a patch against the phpdoc for all the language-related email stuff to help clarify things, based on my (i hope) new-found understanding of all the pieces and gotchas. I'll post that here when I have a chance to write it.