[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Redundant (harmful) duplication of run-hooks in define-globalized-mi
Re: Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch-2]
Sun, 3 Feb 2013 22:14:27 +0000
On Fri, Feb 01, 2013 at 06:16:37PM -0500, Stefan Monnier wrote:
> > Is it OK to commit this change to emacs-24 now, and close bug#11152?
> No, I think this is good for trunk but not for emacs-24.
> I think that for emacs-24 we need a less invasive change.
Any less invasive change would surely be a better change, if we could
come up with one, so would also be better for the trunk. Maybe we just
leave this bug to 24.4/25.1 to fix.
> For that we should probably go back and try and figure out why the hook
> is run both times. As mentioned, the code tries to avoid running it
> twice by checking the value of `major-mode', so it appears that in the
> problem case, `major-mode' changes between the two hooks. Can you try
> and figure out if that's indeed the case and why?
The MODE-major-mode check was in place long before the current issues
arose in 2010-04-25 in the emacs-devel thread "globalized minor modes -
priority over mode hook?". I've bzr blame'd it back to before revision
49780.1.32 from 2006.
MODE-major-mode's original purpose seems to be to prevent the needless
disabling and re-enabling of the minor mode. It doesn't appear to be
there to detect a mode changing between two run-hook calls. I haven't
found any evidence of any such "problem case".
The first revision where MODE-enable-in-buffers gets invoked twice is
100071, this one:
committer: Stefan Monnier <address@hidden>
branch nick: trunk
timestamp: Wed 2010-04-28 11:18:37 -0400
Make it possible to locally disable a globally enabled mode.
* simple.el (fundamental-mode): Run fundamental-mode-hook.
* emacs-lisp/derived.el (define-derived-mode): Use fundamental-mode
rather than kill-all-local-variables so it runs fundamental-mode-hook.
* emacs-lisp/easy-mmode.el (define-globalized-minor-mode):
Use fundamental-mode-hook to run MODE-enable-in-buffers earlier, so
that subsequent hooks get a chance to disable it.
, with this change:
*** lisp/emacs-lisp/easy-mmode.el 2010-04-27 18:14:16 +0000
--- lisp/emacs-lisp/easy-mmode.el 2010-04-28 15:18:37 +0000
*** 338,346 ****
--- 338,348 ----
+ (add-hook 'fundamental-mode-hook ',MODE-enable-in-buffers)
(add-hook 'find-file-hook ',MODE-check-buffers)
(add-hook 'change-major-mode-hook ',MODE-cmhh))
+ (remove-hook 'fundamental-mode-hook ',MODE-enable-in-buffers)
(remove-hook 'find-file-hook ',MODE-check-buffers)
(remove-hook 'change-major-mode-hook ',MODE-cmhh))
. fundamental-mode-hook was later suppressed and replaced by
Stefan, have you any recollection at all of why you left
MODE-enable-in-buffers also in after-change-major-mode-hook?
Alan Mackenzie (Nuremberg, Germany).