Pass language information to filters

We cache filtered string by md5(text). do we need to add language to the cache key?

Here's an updated patch that includes the language string in the cache key.

Cheers,
Stella

Here's an updated patch which uses $langcode instead. I've also attached a patch to update the hook_filter() api documentation.

It would be good if you could describe in the phpdoc (i) the format of the language code and more importantly (ii) why the language code matters and what it could/would be used for.

I'm also wondering if this affects any of our tests -- it should.

I'm using the latest version of Drupal from HEAD. I ran all of the simple tests before I made the change and again afterwards. No additional simple tests failed because of the application of the patch.

Will take a look at improving the php documentation.

Cheers,
Stella

Updated patch with improved function documentation.

I was wondering though, if no language code is provided to check_markup(), should check_markup() provide the code of the current language to hook_filter() or should it be left to the hook_filter() to determine the current language code if it requires it? Currently, the patch assumes the latter.

Cheers,
Stella

I know that I got us started down this road, but I really was asking if the langcode is needed as a cache key. If the md5($text) is a perfect match, can't we be sure that the cached text is good and we need not run filters?

The documentation improvements look good. I'm still curious what language-specific filter you're working on.

Let's wait for the reply to Moshe's question in #10 too.

wow - good answer. i'll support a langcode in the key.

+1 for this change.

First thought:

I'd like to propose (ugh, slightly scope creep) to revert to "language", but don't do:

check_markup($block->body, $block->format, '', FALSE);
check_markup($comment->comment, $comment->format, '', FALSE);
check_markup($node->body, $node->format, $node->language, FALSE);

Do this instead:

check_markup($block, FALSE);
check_markup($comment, FALSE); // Change $comment->comment into $comment->body!
check_markup($node, FALSE);

@nedjo: thanks for the clarification -- that is really helpful. It seems like a fairly special case, but I do support the change nonetheless. It is a valid use case after all.

I've run the tests, and this looks like it is ready to be committed. I committed it to CVS HEAD. Two things left to do before this issue can be marked fixed:

* We have to commit the documentation update.
* We have to update the module upgrade instructions in the handbook.

Thanks!

I've committed the API documentation update.

Suggestion for module upgrade instructions:

Parameters to hook_filter() have changed
hook_filter() now accepts an additional parameter, $langcode, which allows filters to be language aware. This means modules can now implement language specific text replacements.

Drupal 6.x:

<?php
function my_module_filter($op, $delta = 0, $format = -1, $text = '', $cache_id = 0) {
  // Apply filter...
}
?>

Drupal 7.x:

<?php
function my_module_filter($op, $delta = 0, $format = -1, $text = '', $langcode = '', $cache_id = 0) {
  // Apply filter...
}
?>

Parameters to check_markup() have changed
check_markup() now accepts an additional parameter, $langcode. This is needed for filters which implement language specific text replacements.

Drupal 6.x:

$node->body = check_markup($node->body, $node->format);

Drupal 7.x:

$node->body = check_markup($node->body, $node->format, $node->language);

Module upgrade instructions updated on http://drupal.org/node/224333

Cheers,
Stella