bug#5294: 23.1; unload-feature disable minor-mode

bug#5294: 23.1; unload-feature disable minor-mode

[Kevin Ryde]

[-- Attachment #1: Type: text/plain, Size: 748 bytes --]

As an idea for unload-feature, when unloading a buffer-local minor mode
it could helpfully find buffers where the mode is enabled and disable it
before unloading.

An example foo.el mode below.  Eval the code in try-foo.el and it leaves
the buffer boldened, where disabling the mode could have undone it.

Of course foo.el can do something like the commented-out
`foo-unload-function' itself, but I think almost all minor modes would
benefit from this and that `unload-feature' might therefore handle it.

Identifying a minor mode function would be as easy as looking in
`minor-mode-list' would it?  Otherwise I expect define-minor-mode could
chuck some code in `foo-unload-hook' - if it presumes the feature symbol
will match the load filename.



[-- Attachment #2: foo.el --]
[-- Type: application/emacs-lisp, Size: 528 bytes --]

[-- Attachment #3: try-foo.el --]
[-- Type: application/emacs-lisp, Size: 174 bytes --]

[-- Attachment #4: Type: text/plain, Size: 732 bytes --]



In GNU Emacs 23.1.1 (i486-pc-linux-gnu, GTK+ Version 2.16.5)
 of 2009-09-14 on raven, modified by Debian
configured using `configure  '--build=i486-linux-gnu' '--host=i486-linux-gnu' '--prefix=/usr' '--sharedstatedir=/var/lib' '--libexecdir=/usr/lib' '--localstatedir=/var/lib' '--infodir=/usr/share/info' '--mandir=/usr/share/man' '--with-pop=yes' '--enable-locallisppath=/etc/emacs23:/etc/emacs:/usr/local/share/emacs/23.1/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/23.1/site-lisp:/usr/share/emacs/site-lisp:/usr/share/emacs/23.1/leim' '--with-x=yes' '--with-x-toolkit=gtk' '--with-toolkit-scroll-bars' 'build_alias=i486-linux-gnu' 'host_alias=i486-linux-gnu' 'CFLAGS=-DDEBIAN -g -O2' 'LDFLAGS=-g' 'CPPFLAGS=''

bug#5294: 23.1; unload-feature disable minor-mode

[Juanma Barranquero]

On Sat, Jan 2, 2010 at 22:09, Kevin Ryde <user42@zip.com.au> wrote:

> As an idea for unload-feature, when unloading a buffer-local minor mode
> it could helpfully find buffers where the mode is enabled and disable it
> before unloading.
>
> An example foo.el mode below.  Eval the code in try-foo.el and it leaves
> the buffer boldened, where disabling the mode could have undone it.

I don't know whether, in general, you want to disable all effects of a
mode after downloading it. Modes that do want can use
FOO-unload-function like in your example.

> Otherwise I expect define-minor-mode could
> chuck some code in `foo-unload-hook' - if it presumes the feature symbol
> will match the load filename.

FOO-unload-hook is deprecated.

    Juanma

bug#5294: 23.1; unload-feature disable minor-mode

[Kevin Ryde]

Juanma Barranquero <lekktu@gmail.com> writes:
>
> I don't know whether, in general, you want to disable all effects of a
> mode after downloading it.

It seems unlikely a minor mode can do anything much good when its
functions have been unloaded.  Some "static" effects might be ok, but
anything active would presumably stick in an unchanging state or start
to error out, either a bit or badly.


> FOO-unload-hook is

... a flexible way for unrelated libraries, macros or bits of code to
undo things they know about, even different conditionalized parts of one
.el like in tramp-util.el.  A kind of inverse to eval-after-load,
difficult to arrange on a monolithic unload func.

bug#5294: 23.1; unload-feature disable minor-mode

[Juanma Barranquero]

On Sun, Jan 3, 2010 at 00:58, Kevin Ryde <user42@zip.com.au> wrote:

> It seems unlikely a minor mode can do anything much good when its
> functions have been unloaded.  Some "static" effects might be ok,

That is the point. The writer of the mode knows better. I'm not saying
that you're not right in this, only that there could be downsides.

>> FOO-unload-hook is
>
> ... a flexible way for unrelated libraries, macros or bits of code to
> undo things they know about, even different conditionalized parts of one
> .el like in tramp-util.el.

Yeah, well, that and also, it never really worked. You seem to think
that FOO-unload-hook is a hook run while executing `unload-feature'.
In fact, is a hook run *instead* of some other code. The very act of
defining FOO-unload-hook has unexpected consequences: `unlead-feature'
won't remove FOO functions from hooks, nor from `auto-mode-alist'. And
it was always so; that's why I pushed for introducing
FOO-unload-function.

> A kind of inverse to eval-after-load,
> difficult to arrange on a monolithic unload func.

In general, a package is the place where the knowledge about its
unloading should be concentrated, IMO. If you want to piggyback onto
it, there are several ways, like advising FOO-unload-function, if it
is defined, or `unload-feature', etc. There's nothing "difficult to
arrange".

If you want to have an inverse to eval-after-load, I'd suggest you
propose a new hook, unload-feature-functions (not FOO-specific).

    Juanma

bug#5294: 23.1; unload-feature disable minor-mode

[Kevin Ryde]

Juanma Barranquero <lekktu@gmail.com> writes:
>
> In fact, is a hook run *instead* of some other code.

Ah, I didn't know that.

> The very act of
> defining FOO-unload-hook has unexpected consequences: `unlead-feature'
> won't remove FOO functions from hooks, nor from `auto-mode-alist'. And
> it was always so; that's why I pushed for introducing
> FOO-unload-function.

Sounds like it could have been better to do those latter actions
irrespective of the hook.  Let only the FOO-unload-function say not to
do them, for the rare cases they're unwanted.

bug#5294: 23.1; unload-feature disable minor-mode

[Juanma Barranquero]

On Sun, Jan 3, 2010 at 02:05, Kevin Ryde <user42@zip.com.au> wrote:

> Sounds like it could have been better to do those latter actions
> irrespective of the hook. Let only the FOO-unload-function say not to
> do them, for the rare cases they're unwanted.

The now-obsolete functionality (for FOO-unload-hook, not
unload-feature in general) was confusing, badly designed and
ill-documented; changing it would have been backward incompatible, not
to mention fragile. It was better to add a new way. The current
FOO-feature-function was added because in most cases, it's the
package's responsibility to know how to unload itself. For the few
cases that do not, there's always advising; but perhaps
`unload-feature-functions' would be a good idea, and there's no worry
about compatibility.

    Juanma

bug#5294: 23.1; unload-feature disable minor-mode

[Kevin Ryde]

Juanma Barranquero <lekktu@gmail.com> writes:
>
> advising ... `unload-feature'

Sounds likely.  Though I wonder that an add-on might want to slip in
after the FOO-unload-function has reported whether the "normal" unload
actions should be performed.

> confusing ... not to mention fragile

Hmm, ah dear, yep.

> perhaps `unload-feature-functions'

One thing I was contemplating is whether defadvice might automatically
unload, and what advice.el or the defadvice macro might do to make that
happen.  I'll start another bug for that.

Mind you, I wonder if unload-feature is slightly doomed in general,
ie. unless a given package has thought rather carefully about the
consequences!  :-)

bug#5294: 23.1; unload-feature disable minor-mode

[Juanma Barranquero]

On Mon, Jan 4, 2010 at 00:04, Kevin Ryde <user42@zip.com.au> wrote:

> Though I wonder that an add-on might want to slip in
> after the FOO-unload-function has reported whether the "normal" unload
> actions should be performed.

You can use an around advice, and have almost complete control.

> Mind you, I wonder if unload-feature is slightly doomed in general,
> ie. unless a given package has thought rather carefully about the
> consequences!  :-)

Well, certainly unloading can have consequences; that's why I insist
that the best place to define what should do for any package is in the
package itself. Even an add-on to a package should just define its own
feature, and FOO-unload-function for itself, IMO. If you don't use the
FORCE argument, a package cannot be unloaded before unloading its
dependencies, so it is safe to unload the add-ons before.

    Juanma

bug#5294: 23.1; unload-feature disable minor-mode

[Kevin Ryde]

To, umm, get slightly back on-topic -- as an alternative for unloading
minor modes, if the mode is currently enabled in a buffer, or enabled
globally for a global, then unload-feature might refuse to unload
(except under FORCE) on that basis that in-use is a kind of dependency
on the feature's code etc.  The same might be applied to major modes,
ie. refuse to unload if in use.

Some generality could be had if there was a way that define-minor-mode
might tie-in a test that unload-feature would reach when considering
whether to unload.  define-minor-mode might like to run some code on the
actual unload too, to reverse some of `add-minor-mode', like removing
from minor-mode-list (unless perhaps autoloaded minor mode funcs could
stay there happily enough).

bug#5294: 23.1; unload-feature disable minor-mode

[Juanma Barranquero]

On Wed, Jan 6, 2010 at 00:31, Kevin Ryde <user42@zip.com.au> wrote:

> To, umm, get slightly back on-topic -- as an alternative for unloading
> minor modes, if the mode is currently enabled in a buffer, or enabled
> globally for a global, then unload-feature might refuse to unload
> (except under FORCE) on that basis that in-use is a kind of dependency
> on the feature's code etc.  The same might be applied to major modes,
> ie. refuse to unload if in use.

That would be too restrictive; there are instances in which just
deactivating the mode(s) and unloading the package is just what the
user expects. Also, FORCE already has a defined meaning: to bypass
checks for package dependencies, not safety checks for all kinds of
things a package may set up.

> Some generality could be had if there was a way that define-minor-mode
> might tie-in a test that unload-feature would reach when considering
> whether to unload.

How is passing that test (presumibly, either a form or a predicate
function) to define-minor-mode better than putting it into
FEATURE-unload-function?

> define-minor-mode might like to run some code on the
> actual unload too, to reverse some of `add-minor-mode', like removing
> from minor-mode-list (unless perhaps autoloaded minor mode funcs could
> stay there happily enough).

Which can be done from FEATURE-unload-function.

At first you talked about add-ons which would want to piggyback into
the unloading of a package, and I don't think that's a good idea, but
if a package author is really interested on it, it is doable with
advices, etc. But your examples in this message are all things that
can already be done on FEATURE-unload-function, for those packages
that really need it.

If you consider that going through the full buffer list, checking for
the mode and disabling it is too cumbersome, perhaps you can write a
helper function that could be called from the unload function...

    Juanma

bug#5294: 23.1; unload-feature disable minor-mode

[Kevin Ryde]

Juanma Barranquero <lekktu@gmail.com> writes:
>
> How is passing that test (presumibly, either a form or a predicate
> function) to define-minor-mode better than putting it into
> FEATURE-unload-function?

Principally that define-minor-mode knows what things it did and
therefore should be undone, or why they might not be undone at the
present time.  I think it would be rather repetitive to be obliged to
write a FOO-unload-function whenever making a minor mode.

Which, err, presumes that there may be standardized things that should
be considered and/or undone for unloading a mode.  I see you say there's
normally not -- where I say there could be a disable or a caution to
avoid likely breakage, removal from the minor modes menu, etc.

bug#5294: 23.1; unload-feature disable minor-mode

[Juanma Barranquero]

On Wed, Jan 6, 2010 at 02:01, Kevin Ryde <user42@zip.com.au> wrote:

> Principally that define-minor-mode knows what things it did and
> therefore should be undone, or why they might not be undone at the
> present time.

But that is defined in the same file as the unload-function...

> I think it would be rather repetitive to be obliged to
> write a FOO-unload-function whenever making a minor mode.

You want to write the same code, but put it in define-minor-mode. I
don't think define-minor-mode should worry itself with unloading.

> Which, err, presumes that there may be standardized things that should
> be considered and/or undone for unloading a mode.  I see you say there's
> normally not -- where I say there could be a disable or a caution to
> avoid likely breakage, removal from the minor modes menu, etc.

I do not say that there are not, I say that they are not easy to
generalize. Some are. For example, removing a minor-mode for the
minor-modes menu seems a good idea, and should be done by
unload-feature if it is not done already, because calling a
nonexistent function is an error.

    Juanma

bug#5294: 23.1; unload-feature disable minor-mode

[Lars Ingebrigtsen]

Juanma Barranquero <lekktu@gmail.com> writes:

> I do not say that there are not, I say that they are not easy to
> generalize. Some are. For example, removing a minor-mode for the
> minor-modes menu seems a good idea, and should be done by
> unload-feature if it is not done already, because calling a
> nonexistent function is an error.

(I'm going through old bug reports that unfortunately weren't resolved
at the time.)

I think the conclusion here was that there isn't much that can be done
in general here -- minor modes aren't special here; removing all
"running code" when unloading a feature isn't really possible.  So I
don't think there's anything actionable in this bug report, and I'm
therefore closing it.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no