Load Wysiwyg profiles as entities

Thank you! Mostly looks good, only a few remarks:

+++ wysiwyg.admin.inc	20 Jan 2011 21:38:29 -0000
@@ -333,8 +333,14 @@ function wysiwyg_profile_form_submit($fo
+  db_merge('wysiwyg')
+  ->key(array('format' => $format))
+  ->fields(array(
+    'editor' => $editor,
+    'settings' => serialize($values),
+  ))
+  ->execute();

Wrong indentation for the chained methods; see http://drupal.org/node/310075 or any core module for correct coding style examples.

+++ wysiwyg.admin.inc	20 Jan 2011 21:38:29 -0000
@@ -333,8 +333,14 @@ function wysiwyg_profile_form_submit($fo
+  entity_get_controller('wysiwyg_profile')->resetCache();

@@ -518,7 +524,7 @@ function wysiwyg_profile_overview_submit
-  cache_clear_all('wysiwyg_profiles', 'cache');
+  entity_get_controller('wysiwyg_profile')->resetCache();

@@ -544,7 +550,7 @@ function wysiwyg_profile_delete_confirm(
-  cache_clear_all('wysiwyg_profiles', 'cache');
+  entity_get_controller('wysiwyg_profile')->resetCache();

We need to reset both the db cache and the static cache, so both lines are required in all instances.

+++ wysiwyg.module	20 Jan 2011 21:38:29 -0000
@@ -590,20 +590,7 @@ function wysiwyg_get_css() {
 function wysiwyg_profile_load($format) {
...
-  if ($cached = cache_get('wysiwyg_profiles')) {
-    $profiles = $cached->data;
-  }
-  else {
...
-    cache_set('wysiwyg_profiles', $profiles);
-  }
-
+  $profiles = entity_load('wysiwyg_profile', array($format));
   return (isset($profiles[$format]) ? $profiles[$format] : FALSE);

We cannot remove the database caching, since Drupal core's entity system does not abstract that part.

However, let's move the database caching into _load_all() and call _load_all() from _load().

+++ wysiwyg.module	20 Jan 2011 21:38:29 -0000
@@ -611,18 +598,7 @@ function wysiwyg_profile_load($format) {
+  return entity_load('wysiwyg_profile');

Missing second parameter, needs to be FALSE to load all records.

+++ wysiwyg.module	20 Jan 2011 21:38:29 -0000
@@ -1020,5 +996,38 @@ function _wysiwyg_process_include($modul
+ * Implementation of hook_entity_info().

Documentation standards for hooks have changed; it's Implements hook_...(). now.

+++ wysiwyg.module	20 Jan 2011 21:38:29 -0000
@@ -1020,5 +996,38 @@ function _wysiwyg_process_include($modul
+  $entity_info['wysiwyg_profile'] = array(

Let's rename the variable to $types.

+++ wysiwyg.module	20 Jan 2011 21:38:29 -0000
@@ -1020,5 +996,38 @@ function _wysiwyg_process_include($modul
+    'base table' => 'wysiwyg',
+    'controller class' => 'WysiwygProfileController',
+    'fieldable' => FALSE,

Let's add

'static cache' => TRUE,

here, so the removal of statics elsewhere in this patch is actually valid. ;)

+++ wysiwyg.module	20 Jan 2011 21:38:29 -0000
@@ -1020,5 +996,38 @@ function _wysiwyg_process_include($modul
+  function attachLoad(&$queried_wysiwyg_profiles, $revision_id = FALSE) {

1) Missing phpDoc; should be "Overrides ..."; see http://drupal.org/node/1354 for details.

2) Let's rename the variable to $queried_entities.

Powered by Dreditor.

I thought it does, from my reading of http://api.drupal.org/api/drupal/includes--entity.inc/function/DrupalDef.... Am I misinterpreting something?

If you step further, then you can see that ->cacheGet() is about the static cache only.

FALSE is the default parameter in the function declaration

Not unwise, but a bit sparse. That default value is likely going to change for D8, so let's be explicit.

'static cache' [...] defaults to TRUE.

I see. However, let's also be explicit here, since we actively rely on the static caching for performance reasons.

+++ wysiwyg.module	21 Jan 2011 18:25:50 -0000
@@ -611,14 +598,15 @@ function wysiwyg_profile_load($format) {
 function wysiwyg_profile_load_all() {
-  static $profiles;
+  $profiles = &drupal_static(__FUNCTION__, array());
...
-  if (!isset($profiles)) {
...
+  if(empty($profiles)) {

1) The static should default to NULL, because there can be no profiles yet. Just remove the second argument.

2) Missing space after if

+++ wysiwyg.module	21 Jan 2011 18:25:50 -0000
@@ -1020,5 +1008,47 @@ function _wysiwyg_process_include($modul
+function wysiwyg_entity_info() {
...
+class WysiwygProfileController extends DrupalDefaultEntityController {

Let's move these to the top of the .module file, please.

Powered by Dreditor.

I suspect if(empty...) is wrong.

Yup. ;)

Thanks for reporting, reviewing, and testing! Committed attached patch to HEAD. Would be good if you'd have a look at the final tweaks I applied.

A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.

Say what?? I'm gone for a few days and what happens... profiles are entities? That's......... awesome! XD