Uninstaller

When you say "guys", you mostly mean "Doug" but also now "Stella". And as I have about 20 modules, these things easily fall off the radar. I do think that this should be in hook_uninstall and not hook_disable, though.

This is on the list to get done. But a patch would help it along. This would also be a nice GHOP task: Write uninstall's for all my modules :)

I agree, variable_del should be in hook_uninstall.

See also #210665

Here's the whole thing - I don't know how to make diff add a new file.

diff -u originalfile newfile > patchfile

Two patches against current 5.x and 6.x.

I was thinking about this after I uploaded my patch. Cache_menu should also be cleared (and, BTW, Doug, you don't use "if ($may_cache)" in your hook_menu). We should also include cache_clear_all('coder:', 'cache', true).

function coder_uninstall() {
  global $user;

  variable_del('coder_cache');
  variable_del('coder_reviews');
  variable_del('coder_severity');
  variable_del('coder_active_modules');
  variable_del('coder_core');
  variable_del('coder_includes');
  variable_del('coder_modules');
  variable_del('coder_themes');

  cache_clear_all('coder:', 'cache', true);
  cache_clear_all('*', 'cache_menu', true);

  drupal_set_message(t('The Coder module has been uninstalled.'), 'notice');
  watchdog('Coder', 'Coder module uninstalled by user/'. $user->uid .'.', WATCHDOG_NOTICE);
}

@sun: I always like to know who deleted a module, that's why the watchdog entry.

@nancyw:
(i) drupal_uninstall_module() executes hook_uninstall() in front of removing a module's menu entries. Clearing the cache in hook_uninstall() does not make sense. (btw: This would be a serious Drupal core issue)
(ii) drupal_set_message() does not have a 'notice' type. See http://api.drupal.org/api/function/drupal_set_message/5
(iii) I don't know any other module that inserts a watchdog entry upon module uninstall. I don't think, that should be in Coder, either. However, I think that would be a valuable feature request for D7. Until then, you might propose it as a feature request for http://drupal.org/project/journal

As for adding files, see Creating patches, the section on Adding/Deleting files.

Yes we should fix that $may_cache bug. I've noticed that before, and just never fixed it.

I kinda agree with @sun, that we don't need to break new ground on hook_uninstall messages. It should be a core feature.

Re: $may_cache for menu items, see http://drupal.org/node/210828

@sun: i) Okay. But I have seen menu entries remaining.
ii) Hmm, I wonder where I got that from. Guess I need to change lots of modules. BTW, it works though.
iii) I've seen other modules that do it, and certainly mine do. I believe I submitted that feature before D6, but I will go through my issue list and check.

@doug: Yes, I noticed the patches above were done with -N, so I looked it up. Now I know.

Revised patch. Checked for any missed variables.

Ran clean and I had no orphan variables afterwards.

Looks good to me! (reviewed)

committed, Thanks!