Cache info from hook_filter_info() and allow to be altered

+++ modules/filter/filter.test	21 Aug 2009 21:55:21 -0000
@@ -74,10 +74,10 @@
-    	debug($format);
+      debug($format);

Not sure how this has been added, but the line needs to be removed.

+++ modules/filter/filter.api.php	21 Aug 2009 21:55:10 -0000
@@ -104,5 +104,20 @@
+  // Replace the PHP evaluator process callback with an improved PHP evaluator provided
+  // by a module.

Exceeds 80 chars.

+++ modules/filter/filter.api.php	21 Aug 2009 21:55:10 -0000
@@ -104,5 +104,20 @@
+  $info['php_code'] += array(
+      'process callback' => 'my_module_php_evaluator',
+  );

Wrong indentation.

+++ modules/filter/filter.module	21 Aug 2009 21:55:17 -0000
@@ -325,34 +325,32 @@
+  $filter_info = &drupal_static(__FUNCTION__, array());

Please keep the variable name "$filters".

+++ modules/filter/filter.module	21 Aug 2009 21:55:17 -0000
@@ -325,34 +325,32 @@
+  
...
+    

Trailing white-space.

+++ modules/filter/filter.module	21 Aug 2009 21:55:17 -0000
@@ -325,34 +325,32 @@
+    foreach (module_implements('filter_info') as $module) {
+      $info = module_invoke($module, 'filter_info');
+      if (isset($info) && is_array($info)) {
+        $filter_info = array_merge($filter_info, $info);
       }
     }

I don't agree with this change. There is no guarantee that two or more of our 4,000+ contributed modules try to register a filter having the same name.

To overcome this issue, filters should be registered keyed by module.

+++ modules/filter/filter.admin.inc	21 Aug 2009 21:55:09 -0000
@@ -188,7 +188,7 @@
     $name = trim($form_state['values']['name']);
     $result = db_query("SELECT format FROM {filter_format} WHERE name = :name", array(':name' => $name))->fetchField();
     if ($result) {
-      form_set_error('name', t('Text format names must be unique. A format named %name already exists.', array('%name' => $format_name)));
+      form_set_error('name', t('Text format names must be unique. A format named %name already exists.', array('%name' => $name)));
@@ -199,17 +199,17 @@
-  $name = trim($form_state['values']['name']);
+  $format_name = trim($form_state['values']['name']);

It would be good if we would use $format_name consistently - so instead of fixing the wrong variable name in form_set_error(), we should fix the variable declaration.

+++ modules/filter/filter.admin.inc	21 Aug 2009 21:55:09 -0000
@@ -217,20 +217,16 @@
-  $query = db_insert('filter')->fields(array('format', 'module', 'name', 'weight'));
-  foreach ($form_state['values']['filters'] as $id => $checked) {
+  $query = db_insert('filter')->fields(array('format', 'name', 'weight'));
+  foreach ($form_state['values']['filters'] as $name => $checked) {
     if ($checked) {
-      list($module, $filter_name) = explode('/', $id);
       // Add new filters to the bottom.
-      $weight = isset($current[$id]->weight) ? $current[$id]->weight : 10;
+      $weight = isset($current[$name]->weight) ? $current[$name]->weight : 10;
       $query->values(array(
         'format' => $format,
-        'module' => $module,
-        'name'  => $filter_name,
+        'name'  => $name,

We still need the 'module' information (especially for some other issues in the queue).

+++ modules/filter/filter.admin.inc	21 Aug 2009 21:55:09 -0000
@@ -217,20 +217,16 @@
-      // Check if there are any 'no cache' filters.
-      $cache &= !module_invoke($module, 'filter', 'no cache', $filter_name);

uhm, what happened to this?

Beer-o-mania starts in 9 days! Don't drink and patch.

I acknowledge that most of these registry hooks use cache_set() to persist for a long time. But thats becuase they do expensive stuff like build theme registry and menu tree. filter info should run fast enough that i do not mind running it on every page. caching brings along baggage about remembering to clear the cache when the data changes.

an alter hook sounds like a good idea.

Re-rolled for removed registry.

Fixed a couple of comments, coding-style issues, and some instances of $filter->name that should have been $filter->title.

Ready to fly.

Sorry, wrong patch/issue.

@sun is this RTBC from #11?

yes, #11 it is. I mistakenly attached a patch from another issue to this issue. It seems the testbot catched it up although I unpublished that comment immediately.

My comments in #10 have not been addressed. I'm not going to be a hard ass about it, but I think this is one unnecessary cache.

@moshe: Could you clarify #10, please? The patch only introduces a static cache, and your comment sounds like you would agree that a static cache is sufficient and we shouldn't use a db-cache here (at least I would agree with that).

Oh thanks for clarifying. I misunderstood badly. Reset status.

Not sure why it's still green, but it obviously needs a re-roll.

Looks good. Committed to CVS HEAD.