Subject: Re: bug#5293: 23.1; unload-feature on buffer-local hooks
From: Juanma Barranquero <[email-masked]>
To: Kevin Ryde <[email-masked]>
Cc: [email-masked]
Date: Wed, 13 Jul 2011 22:28:02 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

> Evalling the code in try-foo.el below loads then unloads foo.el.  It
> gets an error
>
>    void-function foo-message
>
> where I hoped unload-feature might have purged that `foo-message' from
> `after-change-functions'.
>
> I suppose looking in all buffers is more work for unload-feature, but
> would be a good protection against bad things happening later.  I expect
> some of the standard hooks like `after-change-functions' are used
> buffer-local most of the time.

You're right that some hooks are used buffer-locally most of the time,
but it's also true that in many cases they are used from a major mode,
i.e., in your case, if foo.el defined a foo-mode and set
`after-change-functions' locally in foo-mode buffers, unload-feature
would do OK (with the current trunk, not any released Emacsen).

As it is, foo.el is doing something non-standard, and unload-feature
cannot try to revert by itself every non-standard thing packages do.
That's why FEATURE-unload-function exists. So, in this case, the right
fix would be adding this function to foo.el:

(defun foo-unload-function ()
  "Unload foo.el."
  (ignore-errors
    (with-current-buffer (get-buffer "foo-buffer")
      (remove-hook 'after-change-functions 'foo-message t)))
  ;; continue standard unloading
  nil)

That said, there are improvements that could be made to unload-feature
(for example, trying to automatically deactivate minor modes being
undefined), but I'm not sure that looking in every buffer-local
variable of every live buffer is a sensible thing to do. I'm closing
this one because it is not really a bug. We can open a wishlist bug
for unload-feature is you want.

    Juanma