Object translation option #5: Wrapper function for contrib modules

I'm a little perplexed about this - you're not declaring a hook in the sense that there is a single named function that may exist.

It seems like you need a litttle more complete system for telling Drupal which library you are using - like the cache system?

For example: http://drupal.org/project/memcache

following up the analogy to the cache system, you'd have drupal ship with an include with empty functions like:

function dt($type , $param, $dt_language = NULL) {

  return $param;
}

let's assume it's called /includes/dynamic.locale.inc

as we have this line in bootstrap.inc:

includes/bootstrap.inc:933:  require_once variable_get('cache_inc', './includes/cache.inc');

You'd put in a line like:

require_once variable_get('dynamic_locale_inc', './includes/dynamic.locale.inc');

probably in function drupal_init_language() in bootstrap.inc.

If a contrib module has an alternate library, you could copy that new .inc file to /includes and change settings.php like:

 $conf = array(
   'dynamic_locale_inc' => './includes/jose-dynamic.locale.inc',
);

Jose, I hope others will give feedback on this because I may be off base. However, I really think what you want is more appropriately described in the context of a library, as I described in detail above. The node type hooks are not similar because *every* module can declare as many node types as they wish via hook_node_info. This is more similar to the case where one of the database.*.inc files is included to provide the appropriate definition of functions called throughout core.

What if two modules declare function dt()?

Jose, I understand exactly the functionality you propose, and I agree it would work (though perhaps degrade site performance). However, I'm suggesting that your solution falls outside the pattern of the rest of Drupal core for how hooks are used. Correct me if I'm wrong. That's why I'm suggesting the library/.inc approach which is already used for substituting different libraries for a single set of named functions.

A switchable include file is great for implenting a fixed set of functions, as e.g. the cache system does. Here, however, we have a variable amount of functions - one for each dynamically translateble object type (contrib might even provide more types, I guess). I can't argue right now wether this approach (one function for each dynamic type) is actually better than just one function, custom_dt(), that then deals with all types of dynamically translateble objects. The baseline is, though, that just _one_ module can implement a dynamic translation solution, everything else wouldn't work.

Basically I'd say it really doesn't matter wether it's an .inc-file or a custom function that one module can declare (AFAIR we already have something like this in core with custom_url_rewrite) - it does both serve the purpose equally well.

@pwolanin: I don't think that this solution is far outside of the Drupal pattern - the locale system works (and was designed) in exactly this way, you simply need to implement a module that has a function named locale() and turn it on (and leave the core locale module off) and everything will use your module instead. This is exactly the same as what Jose is suggesting - which is at least leaves the behavior or dt() consistent with that of t().

Now this doesn't necessarily mean that this setup is better, just that it is following an already established pattern. I can see the advantages of the include(variable_get(...)) approach in terms of clarity of interface (i.e. it is obvious where the 'switch' occurs). However, I don't think that it is that much more more robust - at the end of the day, if you enable 2 modules that are attempting to override the same core functionality, something will break (either you get errors of already declared function names, or the one that sets the variable last will 'win').

A couple of other notes - the dt() function has this code:

 if (function_exists($function = 'custom_dt_'.$type)) {
    return call_user_func($param, $dt_language ? $dt_language : $language);

I think there is a bug here, because the function name $function (which was just checked for) is not actually being called, instead the function name (the first parameter of call_user_func) is coming from $param (which will obviously fail, since this is supposed to be a string, not an object).

In addition, I am not sure I support the schama of functions named 'custom_dt_'.$type.
This seems to suggest that each module (e.g. aggregator) should implement the translation itself (in which case dt() is a pointless wrapper - since they could simply use their translation function themselves), or that the dynamic translation module should know in advance each of the $type's that it is expected to translate. The problem with the latter is that it means that this system becomes effectively useless for contrib modules, since the translation module would have to know about each one beforehand for it to work.

Instead I would rather see a single function (e.g. custom_dt(), or object_locale()) that all object translation is passed to. This will make it much easier to focus energy on a single, fast engine to handle the actual translation lookup and caching. There is no need to differentiate functions on the lookup site - an object is an object is an object - we don't care what type :)

This engine would provide a set and delete API for translations, that additional modules or includes could use to provide friendly admin interfaces to add/update translations - either in the source module (using the functions directly) or through form_alter (for core modules).

A decision has to be made here, mainly whether this can still get in even if we're post-freeze or if it's postponed (which would of course reduce the i18n possibilities of d6). I still think that Joses approach is completely acceptable and possibly the best we can get as an interim solution for d6. It will allow us to see how dynamic translation can happen (or how not). Not committing a patch similar to this one for D6 will remove that possibility (at least a clean way for it).

@Frando - I'm concerned that that approach requires potentially dozens (or hundreds) of extra calls to function_exists() per page even if no translation module is enabled. The approach I suggested does not. I support getting some version of this in, but it must be done in a way that's not going to be a big performance drag for single-language site.

See, for example: http://drupal.org/node/108750 on the performance drag of function_exists().

Hmmm, actually if I read the issue I just referenced in more detail, perhpas function_exists() is not as bad as I thought.

Still, it would be nice if dt() could be totally redefined in a drop-in library.

This would be a potentially huge change affecting all core modules (and contrib for that matter). There is plenty of stuff to brake on the way, and some stuff needs serious thinking: like when you edit an object, it needs to be presented in the language you saved it in, but what about admin listing pages and what about user listing pages? Anyway, lots of open questions still, and lots of changes required to Drupal core. It is very unfortunate that our previous attempts at solving this problem were not taken seriously by those who wish complete multilanguage support in Drupal 6, but this is how it happened. This is now targeted at Drupal 7.

Clearly this would fail, but it does fail, so setting to CNW.

$ patch -p0 < dt_abstract_wrapper_01.patch
patching file modules/aggregator/aggregator.module
Hunk #1 succeeded at 301 (offset 34 lines).
Hunk #2 succeeded at 349 (offset 34 lines).
Hunk #3 FAILED at 1043.
Hunk #4 FAILED at 1098.
Hunk #5 FAILED at 1109.
Hunk #6 FAILED at 1126.
Hunk #7 FAILED at 1253.
Hunk #8 FAILED at 1265.
Hunk #9 FAILED at 1327.
7 out of 9 hunks FAILED -- saving rejects to file modules/aggregator/aggregator.module.rej
patching file modules/node/node.module
Hunk #1 FAILED at 414.
1 out of 1 hunk FAILED -- saving rejects to file modules/node/node.module.rej
patching file includes/common.inc
Hunk #1 succeeded at 798 (offset 60 lines).

Correct me if I am wrong but this should have been replaced by #593746: Prepare Drupal core for dynamic data translation.