* Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode
@ 2013-01-13 19:28 Alan Mackenzie
2013-01-13 20:48 ` Dmitry Gutov
2013-01-14 16:30 ` Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch] Alan Mackenzie
0 siblings, 2 replies; 22+ messages in thread
From: Alan Mackenzie @ 2013-01-13 19:28 UTC (permalink / raw
To: emacs-devel
Hi, Emacs.
The situation here is the direct cause of bug #11152, where in CC Mode,
doc comments whose fontification is specified in a mode hook don't get
fontified properly, or at all. It would be good to fix this bug for
Emacs 24.3.
In define-globalized-minor-mode L72-75, the newly defined -enable-
function is added to both the following hooks:
change-major-mode-after-body-hook
after-change-major-mode-hook
. (These hooks are run before and after the major mode hook.)
It seems the hacker who formulated this macro was undecided whether to
run the -enable- function before or after the mode hooks, so decided
upon both as a compromise. This isn't harmless.
In particular, running `global-font-lock-mode-enable-in-buffers' before
the objc-mode-hook causes a one-time font-lock-keywords initialisation
to happen before a critical initialisation has been performed by that
hook.
Can some means be found so that `font-lock-mode' isn't called twice?
(For that matter, `global-hi-lock-mode-enable-in-buffers' doesn't need
calling twice, either.) Such a means might be the addition of another
parameter to define-globalized-minor-mode specifying when the -enable-
call should take place.
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode
2013-01-13 19:28 Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode Alan Mackenzie
@ 2013-01-13 20:48 ` Dmitry Gutov
2013-01-14 16:30 ` Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch] Alan Mackenzie
1 sibling, 0 replies; 22+ messages in thread
From: Dmitry Gutov @ 2013-01-13 20:48 UTC (permalink / raw
To: Alan Mackenzie; +Cc: emacs-devel
Alan Mackenzie <acm@muc.de> writes:
> The situation here is the direct cause of bug #11152, where in CC Mode,
> doc comments whose fontification is specified in a mode hook don't get
> fontified properly, or at all. It would be good to fix this bug for
> Emacs 24.3.
>
> In define-globalized-minor-mode L72-75, the newly defined -enable-
> function is added to both the following hooks:
> change-major-mode-after-body-hook
> after-change-major-mode-hook
> . (These hooks are run before and after the major mode hook.)
>
> It seems the hacker who formulated this macro was undecided whether to
> run the -enable- function before or after the mode hooks, so decided
> upon both as a compromise. This isn't harmless.
>
> In particular, running `global-font-lock-mode-enable-in-buffers' before
> the objc-mode-hook causes a one-time font-lock-keywords initialisation
> to happen before a critical initialisation has been performed by that
> hook.
Sounds similar to http://debbugs.gnu.org/cgi/bugreport.cgi?bug=11929
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch]
2013-01-13 19:28 Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode Alan Mackenzie
2013-01-13 20:48 ` Dmitry Gutov
@ 2013-01-14 16:30 ` Alan Mackenzie
2013-01-14 16:52 ` emacs24/auctex bug Camm Maguire
2013-01-15 2:12 ` Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch] Stefan Monnier
1 sibling, 2 replies; 22+ messages in thread
From: Alan Mackenzie @ 2013-01-14 16:30 UTC (permalink / raw
To: emacs-devel
Hi, Emacs.
On Sun, Jan 13, 2013 at 07:28:54PM +0000, Alan Mackenzie wrote:
> The situation here is the direct cause of bug #11152, where in CC Mode,
> doc comments whose fontification is specified in a mode hook don't get
> fontified properly, or at all. It would be good to fix this bug for
> Emacs 24.3.
> In define-globalized-minor-mode L72-75, the newly defined -enable-
> function is added to both the following hooks:
> change-major-mode-after-body-hook
> after-change-major-mode-hook
> . (These hooks are run before and after the major mode hook.)
> It seems the hacker who formulated this macro was undecided whether to
> run the -enable- function before or after the mode hooks, so decided
> upon both as a compromise. This isn't harmless.
> In particular, running `global-font-lock-mode-enable-in-buffers' before
> the objc-mode-hook causes a one-time font-lock-keywords initialisation
> to happen before a critical initialisation has been performed by that
> hook.
> Can some means be found so that `font-lock-mode' isn't called twice?
> (For that matter, `global-hi-lock-mode-enable-in-buffers' doesn't need
> calling twice, either.) Such a means might be the addition of another
> parameter to define-globalized-minor-mode specifying when the -enable-
> call should take place.
I propose to amend define-globalized-minor-mode such that the generated
-enable- function gets added to precisely one of the following hooks, not
both:
change-major-mode-after-body-hook
after-change-major-mode-hook
, the latter one by default. To change this default, the new
keyword-parameter :call-before-hook is introduced. I think this
amendment should go into Emacs 24.3 (it "solves" bug #11152).
There are 6 invocations of define-globalized-minor-mode in Emacs,
1 global-visual-line-mode
2 global-hi-lock-mode
3 global-font-lock-mode
4 global-cwarn-mode
5 global-linum-mode
6 global-highlight-changes-mode
. 2 and 3 appear to work fine with the amended macro. I haven't tested
the others.
Here is the patch to the .el file (another will be needed for NEWS):
=== modified file 'lisp/emacs-lisp/easy-mmode.el'
*** lisp/emacs-lisp/easy-mmode.el 2013-01-01 09:11:05 +0000
--- lisp/emacs-lisp/easy-mmode.el 2013-01-14 15:50:50 +0000
***************
*** 329,335 ****
and that should try to turn MODE on if applicable for that buffer.
KEYS is a list of CL-style keyword arguments. As the minor mode
defined by this function is always global, any :global keyword is
! ignored. Other keywords have the same meaning as in `define-minor-mode',
which see. In particular, :group specifies the custom group.
The most useful keywords are those that are passed on to the
`defcustom'. It normally makes no sense to pass the :lighter
--- 329,338 ----
and that should try to turn MODE on if applicable for that buffer.
KEYS is a list of CL-style keyword arguments. As the minor mode
defined by this function is always global, any :global keyword is
! ignored. If :call-before-hook is present with a non-nil argument, TURN-ON
! will be called before running the major mode's hook, otherwise it will be
! called afterwards.
! Other keywords have the same meaning as in `define-minor-mode',
which see. In particular, :group specifies the custom group.
The most useful keywords are those that are passed on to the
`defcustom'. It normally makes no sense to pass the :lighter
***************
*** 346,351 ****
--- 349,355 ----
(pretty-name (easy-mmode-pretty-mode-name mode))
(pretty-global-name (easy-mmode-pretty-mode-name global-mode))
(group nil)
+ (call-before-hook nil)
(extra-keywords nil)
(MODE-buffers (intern (concat global-mode-name "-buffers")))
(MODE-enable-in-buffers
***************
*** 362,367 ****
--- 366,372 ----
(pcase keyw
(`:group (setq group (nconc group (list :group (pop keys)))))
(`:global (setq keys (cdr keys)))
+ (`:call-before-hook (setq call-before-hook (pop keys)))
(_ (push keyw extra-keywords) (push (pop keys) extra-keywords))))
(unless group
***************
*** 394,402 ****
;; Setup hook to handle future mode changes and new buffers.
(if ,global-mode
(progn
! (add-hook 'after-change-major-mode-hook
! ',MODE-enable-in-buffers)
! (add-hook 'change-major-mode-after-body-hook
',MODE-enable-in-buffers)
(add-hook 'find-file-hook ',MODE-check-buffers)
(add-hook 'change-major-mode-hook ',MODE-cmhh))
--- 399,407 ----
;; Setup hook to handle future mode changes and new buffers.
(if ,global-mode
(progn
! (add-hook ',(if call-before-hook
! 'change-major-mode-after-body-hook
! 'after-change-major-mode-hook)
',MODE-enable-in-buffers)
(add-hook 'find-file-hook ',MODE-check-buffers)
(add-hook 'change-major-mode-hook ',MODE-cmhh))
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply [flat|nested] 22+ messages in thread
* emacs24/auctex bug
2013-01-14 16:30 ` Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch] Alan Mackenzie
@ 2013-01-14 16:52 ` Camm Maguire
2013-01-14 23:58 ` Xue Fuqiao
2013-01-15 2:12 ` Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch] Stefan Monnier
1 sibling, 1 reply; 22+ messages in thread
From: Camm Maguire @ 2013-01-14 16:52 UTC (permalink / raw
To: emacs-devel
Greetings! When I try to open a .bib file in emacs24 with auctex
loaded, I hang with this backtrace. Suggestions?
=============================================================================
Debugger entered--Lisp error: (quit)
dbus-call-method-non-blocking(:session "org.gnome.evince.Daemon" "/org/gnome/evince/Daemon" "org.freedesktop.DBus.Introspectable" "Introspect")
byte-code("\b\203\b.\305\202 .\306 \n\v\f\307%\207" [noninteractive bus service path dbus-interface-introspectable dbus-call-method dbus-call-method-non-blocking "Introspect"] 6)
dbus-introspect(:session "org.gnome.evince.Daemon" "/org/gnome/evince/Daemon")
byte-code("\304\305!.r\bq\210\306\216\307 \n\v#c\210\310ed\"+\207" [temp-buffer bus service path generate-new-buffer " *temp*" ((byte-code "\301\b!\203\n.\302\b!\210\301\207" [temp-buffer buffer-name kill-buffer] 2)) dbus-introspect xml-parse-region] 4)
dbus-introspect-xml(:session "org.gnome.evince.Daemon" "/org/gnome/evince/Daemon")
dbus-introspect-get-interface(:session "org.gnome.evince.Daemon" "/org/gnome/evince/Daemon" "org.gnome.evince.Daemon")
dbus-introspect-get-method(:session "org.gnome.evince.Daemon" "/org/gnome/evince/Daemon" "org.gnome.evince.Daemon" "FindDocument")
TeX-evince-dbus-p(:forward)
byte-code("\b\301=\203\b.\302\207\303\304\305\306\307\310\311!\203.\0\312\202+.\313\314\315\316\317\320!\"\203&.\321\202'.\322D\323BBD\324BBBBB\207" [system-type windows-nt (("Yap" ("yap -1" (mode-io-correlate " -s %n%b") " %o")) ("dvips and start" "dvips %d -o && start \"\" %f") ("start" "start \"\" %o")) ("xdvi" ("%(o?)xdvi" (mode-io-correlate " -sourceposition \"%n %b\" -editor \"%cS\"") ((paper-a4 paper-portrait) " -paper a4") ((paper-a4 paper-landscape) " -paper a4r") ((paper-a5 paper-portrait) " -paper a5") ((paper-a5 paper-landscape) " -paper a5r") (paper-b5 " -paper b5") (paper-letter " -paper us") (paper-legal " -paper legal") (paper-executive " -paper 7.25x10.5in") " %d")) ("dvips and gv" "%(o?)dvips %d -o && gv %f") ("gv" "gv %o") ("xpdf" ("xpdf -remote %s -raise %o" (mode-io-correlate " %(outpage)"))) "Evince" TeX-evince-dbus-p :forward TeX-evince-sync-view "evince" mode-io-correlate string-match "--page-index" shell-command-to-string "evince --help" " -i %(outpage)" " -p %(outpage)" (" %o") (("Okular" ("okular --unique %o" (mode-io-correlate "#src:%n%b"))) ("xdg-open" "xdg-open %o"))] 11)
(defvar TeX-view-program-list-builtin (byte-code "\b\301=\203\b.\302\207\303\304\305\306\307\310\311!\203.\0\312\202+.\313\314\315\316\317\320!\"\203&.\321\202'.\322D\323BBD\324BBBBB\207" [system-type windows-nt (("Yap" ("yap -1" (mode-io-correlate " -s %n%b") " %o")) ("dvips and start" "dvips %d -o && start \"\" %f") ("start" "start \"\" %o")) ("xdvi" ("%(o?)xdvi" (mode-io-correlate " -sourceposition \"%n %b\" -editor \"%cS\"") ((paper-a4 paper-portrait) " -paper a4") ((paper-a4 paper-landscape) " -paper a4r") ((paper-a5 paper-portrait) " -paper a5") ((paper-a5 paper-landscape) " -paper a5r") (paper-b5 " -paper b5") (paper-letter " -paper us") (paper-legal " -paper legal") (paper-executive " -paper 7.25x10.5in") " %d")) ("dvips and gv" "%(o?)dvips %d -o && gv %f") ("gv" "gv %o") ("xpdf" ("xpdf -remote %s -raise %o" (mode-io-correlate " %(outpage)"))) "Evince" TeX-evince-dbus-p :forward TeX-evince-sync-view "evince" mode-io-correlate string-match "--page-index" shell-command-to-string "evince --help" " -i %(outpage)" " -p %(outpage)" (" %o") (("Okular" ("okular --unique %o" (mode-io-correlate "#src:%n%b"))) ("xdg-open" "xdg-open %o"))] 11) ("/usr/share/emacs24/site-lisp/auctex/tex.elc" . 30407))
require(tex)
byte-code("\300\301!\210\300\302!\207" [require tex tex-style] 2)
BibTeX-auto-store()
run-hooks(change-major-mode-after-body-hook bibtex-mode-hook)
apply(run-hooks (change-major-mode-after-body-hook bibtex-mode-hook))
run-mode-hooks(bibtex-mode-hook)
bibtex-mode()
=============================================================================
--
Camm Maguire camm@maguirefamily.org
==========================================================================
"The earth is but one country, and mankind its citizens." -- Baha'u'llah
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: emacs24/auctex bug
2013-01-14 16:52 ` emacs24/auctex bug Camm Maguire
@ 2013-01-14 23:58 ` Xue Fuqiao
0 siblings, 0 replies; 22+ messages in thread
From: Xue Fuqiao @ 2013-01-14 23:58 UTC (permalink / raw
To: Camm Maguire; +Cc: emacs-devel
On Mon, 14 Jan 2013 11:52:55 -0500
Camm Maguire <camm@maguirefamily.org> wrote:
> Greetings! When I try to open a .bib file in emacs24 with auctex
> loaded, I hang with this backtrace.
You can post it in:
auctex@gnu.org
bug-auctex@gnu.org
help-gnu-emacs@gnu.org
--
Best regards, Xue Fuqiao.
http://www.emacswiki.org/emacs/XueFuqiao
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch]
2013-01-14 16:30 ` Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch] Alan Mackenzie
2013-01-14 16:52 ` emacs24/auctex bug Camm Maguire
@ 2013-01-15 2:12 ` Stefan Monnier
2013-01-15 14:08 ` Alan Mackenzie
1 sibling, 1 reply; 22+ messages in thread
From: Stefan Monnier @ 2013-01-15 2:12 UTC (permalink / raw
To: Alan Mackenzie; +Cc: emacs-devel
> ! (add-hook 'after-change-major-mode-hook
> ! ',MODE-enable-in-buffers)
> ! (add-hook 'change-major-mode-after-body-hook
> ',MODE-enable-in-buffers)
> (add-hook 'find-file-hook ',MODE-check-buffers)
> (add-hook 'change-major-mode-hook ',MODE-cmhh))
> --- 399,407 ----
> ;; Setup hook to handle future mode changes and new buffers.
> (if ,global-mode
> (progn
> ! (add-hook ',(if call-before-hook
> ! 'change-major-mode-after-body-hook
> ! 'after-change-major-mode-hook)
I think we first need to understand why we currently use both.
Also, the MODE-enable-in-buffers function should enable the mode only
once, normally, even if called via both hooks.
So if you see a problem, maybe it's because (eq ,MODE-major-mode
major-mode) is not true the second time around, i.e. because
`major-mode' has changed between the two hooks. Is that the case?
If so, why?
Stefan
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch]
2013-01-15 2:12 ` Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch] Stefan Monnier
@ 2013-01-15 14:08 ` Alan Mackenzie
2013-01-17 13:17 ` João Távora
0 siblings, 1 reply; 22+ messages in thread
From: Alan Mackenzie @ 2013-01-15 14:08 UTC (permalink / raw
To: Stefan Monnier; +Cc: Chong Yidong, emacs-devel
Hi, Stefan.
On Mon, Jan 14, 2013 at 09:12:15PM -0500, Stefan Monnier wrote:
> > ! (add-hook 'after-change-major-mode-hook
> > ! ',MODE-enable-in-buffers)
> > ! (add-hook 'change-major-mode-after-body-hook
> > ',MODE-enable-in-buffers)
> > (add-hook 'find-file-hook ',MODE-check-buffers)
> > (add-hook 'change-major-mode-hook ',MODE-cmhh))
> > --- 399,407 ----
> > ;; Setup hook to handle future mode changes and new buffers.
> > (if ,global-mode
> > (progn
> > ! (add-hook ',(if call-before-hook
> > ! 'change-major-mode-after-body-hook
> > ! 'after-change-major-mode-hook)
> I think we first need to understand why we currently use both.
This is difficult. The change was in revision 106202. Previously,
MODE-enable-in-buffers was simply add-hooked into
after-change-major-mode-hook. The relevant ChangeLog entry runs:
+ 2011-10-27 Chong Yidong <cyd@gnu.org>
+
+ * subr.el (change-major-mode-after-body-hook): New hook.
+ (run-mode-hooks): Run it.
+
+ * emacs-lisp/easy-mmode.el (define-globalized-minor-mode): Use
+ change-major-mode-before-body-hook.
+
+ * simple.el (fundamental-mode):
+ * emacs-lisp/derived.el (define-derived-mode): Revert 2010-04-28
+ change introducing fundamental-mode-hook.
+
The new hook change-major-mode-after-body-hook came into existence after
a naming competition in the thread starting at:
From: Christoph Scholtes <cschol2112@googlemail.com>
To: emacs-devel@gnu.org
Subject: Fundamental mode vs. special mode
Date: Sat, 22 Oct 2011 14:13:03 -0600
Message-ID: <86sjmkvl80.fsf@googlemail.com>
That thread mainly discussed the propriety of fundamental-mode-hook, and
prompted Yidong to remove it, putting change-major-mode-after-body-hook
in its place. Here is his email proposing this:
> I propose to remove the fundamental-mode-hook variable introduced in
> that change, and introduce `run-mode-hooks-hook'[*], which
> `run-mode-hooks' runs before all the other hooks. Then minor modes
> can use `run-mode-hooks-hook', and the "turning off global modes"
> feature will be available to non-derived modes.
[*] renamed to `change-major-mode-after-body-hook' before
implementation.
> Also, the MODE-enable-in-buffers function should enable the mode only
> once, normally, even if called via both hooks.
I think that's true. My problem is that font-lock-mode is being called
too soon, before the mode has been fully set up by the major mode hook.
That first call of f-l-m sets up font-lock-keywords to include entries
for fontifying "doc comments". A buffer local variable specifies what
sort of doc comments.
It has to be admitted that the CC Mode code here isn't totally correct;
a complete solution to this bug probably involves making
font-lock-keywords buffer local when doc comments are specified. I
don't want to make this change (which would be somewhat involved) before
Emacs 24.3. ;-( Postponing `font-lock-mode' until after the mode hook
seems a safer bet at the moment.
> So if you see a problem, maybe it's because (eq ,MODE-major-mode
> major-mode) is not true the second time around, i.e. because
> `major-mode' has changed between the two hooks. Is that the case?
> If so, why?
> Stefan
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch]
2013-01-15 14:08 ` Alan Mackenzie
@ 2013-01-17 13:17 ` João Távora
2013-01-17 17:51 ` Alan Mackenzie
0 siblings, 1 reply; 22+ messages in thread
From: João Távora @ 2013-01-17 13:17 UTC (permalink / raw
To: Alan Mackenzie; +Cc: Chong Yidong, Stefan Monnier, emacs-devel
On Tue, Jan 15, 2013 at 2:08 PM, Alan Mackenzie <acm@muc.de> wrote:
> I think that's true. My problem is that font-lock-mode is being called
> too soon, before the mode has been fully set up by the major mode hook.
Sorry to hijack, but I have a similiar problem in autopair and yasnippet. My
problem is that I sometimes want the minor modes never to be called at all
depending on the major-mode setup.
While the major-mode hook has a chance to disable them with (xxx-mode -1),
there's no way to use the major-mode's hook to prevent a globalized minor mode
from ever being activated in the buffer.
This is a inneficient and cumbersome for these cases:
* autopair-minor-mode crashes the system when activated with sldb-minor-mode, I
use autopair-mode-on for this and hardcode the sldb reference there. It also
does a lot of useless setup of keybindings etc...
* yas-minor-mode jit-loads snippets for the major mode when invoked. If its
turned off immediately after, it loaded them too soon.
before emacs 24, I used to use xxx-dont-turn-on variables set in major mode hook
and read in the xxx-mode-on function, but I agree that isn't particularly pretty
too.
Thanks,
--
João Távora
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch]
2013-01-17 13:17 ` João Távora
@ 2013-01-17 17:51 ` Alan Mackenzie
2013-01-17 18:31 ` Stefan Monnier
0 siblings, 1 reply; 22+ messages in thread
From: Alan Mackenzie @ 2013-01-17 17:51 UTC (permalink / raw
To: João Távora; +Cc: Chong Yidong, Stefan Monnier, emacs-devel
Hi, João.
On Thu, Jan 17, 2013 at 01:17:03PM +0000, João Távora wrote:
> On Tue, Jan 15, 2013 at 2:08 PM, Alan Mackenzie <acm@muc.de> wrote:
> > I think that's true. My problem is that font-lock-mode is being called
> > too soon, before the mode has been fully set up by the major mode hook.
> Sorry to hijack, but I have a similiar problem in autopair and yasnippet. My
> problem is that I sometimes want the minor modes never to be called at all
> depending on the major-mode setup.
OK, so I'm not the only person with this problem. :-)
> While the major-mode hook has a chance to disable them with (xxx-mode -1),
> there's no way to use the major-mode's hook to prevent a globalized minor mode
> from ever being activated in the buffer.
> This is a inneficient and cumbersome for these cases:
> * autopair-minor-mode crashes the system when activated with sldb-minor-mode, I
> use autopair-mode-on for this and hardcode the sldb reference there. It also
> does a lot of useless setup of keybindings etc...
> * yas-minor-mode jit-loads snippets for the major mode when invoked. If its
> turned off immediately after, it loaded them too soon.
> before emacs 24, I used to use xxx-dont-turn-on variables set in major mode hook
> and read in the xxx-mode-on function, but I agree that isn't particularly pretty
> too.
Maybe making a buffer local copy of global-xxxx-mode would do the trick,
but that's anything but pretty.
I'm still trying to come up with a reason somebody might want to turn on
the minor mode _before_ the major mode's hook has been run.
> Thanks,
> --
> João Távora
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch]
2013-01-17 17:51 ` Alan Mackenzie
@ 2013-01-17 18:31 ` Stefan Monnier
2013-01-18 12:07 ` João Távora
` (2 more replies)
0 siblings, 3 replies; 22+ messages in thread
From: Stefan Monnier @ 2013-01-17 18:31 UTC (permalink / raw
To: Alan Mackenzie; +Cc: Chong Yidong, João Távora, emacs-devel
> I'm still trying to come up with a reason somebody might want to turn on
> the minor mode _before_ the major mode's hook has been run.
The issue is how to locally turn off the globalized minor mode from
a major-mode hook. If the globalized mode is enabled after the
major-mode hook, you basically can't do it without resorting to
ugly gymnastics.
I'm not completely sure what happens (and what should happen instead) in
your scenario, but for the case described by João, where you don't just
want to disable it in some modes, but you want to prevent it from ever
being enabled in such modes, obviously the current approach
fails similarly.
A simple solution is to activate the minor mode late, and provide
a `disable-foo-mode' variable, that can be set in the major mode hook.
But it's kind of ugly because it requires another var and the user needs
to know about that magic var.
Maybe we could do it this way:
for a globalized minor mode `foo', before running the major-mode hook, a magic
`disable-foo-mode' is set to nil. A function is added to `foo-mode-hook'
which sets this var to t when foo-mode is disabled. And after running
the major-mode hook, the globalized minor mode code checks the magic var
to decide whether to enable the minor mode.
This way, the user doesn't need to know about the extra magic var; she
can just call (foo-mode -1) in her major-mode hook as she can now, but
the minor-mode is really only activated late.
It's also kind of ugly because of all the magic, but it's the best I can
come up with so far.
Stefan
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch]
2013-01-17 18:31 ` Stefan Monnier
@ 2013-01-18 12:07 ` João Távora
2013-01-18 17:09 ` Alan Mackenzie
2013-01-31 11:04 ` Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch-2] Alan Mackenzie
2 siblings, 0 replies; 22+ messages in thread
From: João Távora @ 2013-01-18 12:07 UTC (permalink / raw
To: Stefan Monnier; +Cc: Alan Mackenzie, Chong Yidong, emacs-devel
[-- Attachment #1: Type: text/plain, Size: 789 bytes --]
On Thu, Jan 17, 2013 at 6:31 PM, Stefan Monnier <monnier@iro.umontreal.ca>
wrote:
> It's also kind of ugly because of all the magic, but it's the best I can
> come up with so far.
I think the approach is quite good (was actually thinking along those
lines on my way to work this morning). It's not uglier than the existing
(defvar ,MODE-major-mode nil)
(make-variable-buffer-local ',MODE-major-mode)
I wonder if the existing MODE-major-mode and the disable-MODE you
proposed could somehow take advantage of lexical binding so they
wouldn't be visible to the user, but maybe this makes no sense...
reading that section of easy-mmode.el always makes me delirious :-)
Also couldn't ,MODE-major-mode contain both semantics and be renamed or
something?
--
João Távora
[-- Attachment #2: Type: text/html, Size: 1146 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch]
2013-01-17 18:31 ` Stefan Monnier
2013-01-18 12:07 ` João Távora
@ 2013-01-18 17:09 ` Alan Mackenzie
2013-01-31 11:04 ` Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch-2] Alan Mackenzie
2 siblings, 0 replies; 22+ messages in thread
From: Alan Mackenzie @ 2013-01-18 17:09 UTC (permalink / raw
To: Stefan Monnier; +Cc: Chong Yidong, João Távora, emacs-devel
Hello, Stefan.
On Thu, Jan 17, 2013 at 01:31:32PM -0500, Stefan Monnier wrote:
> > I'm still trying to come up with a reason somebody might want to turn on
> > the minor mode _before_ the major mode's hook has been run.
> The issue is how to locally turn off the globalized minor mode from
> a major-mode hook. If the globalized mode is enabled after the
> major-mode hook, you basically can't do it without resorting to
> ugly gymnastics.
Ah, OK.
> I'm not completely sure what happens (and what should happen instead) in
> your scenario, but for the case described by João, where you don't just
> want to disable it in some modes, but you want to prevent it from ever
> being enabled in such modes, obviously the current approach
> fails similarly.
> A simple solution is to activate the minor mode late, and provide
> a `disable-foo-mode' variable, that can be set in the major mode hook.
> But it's kind of ugly because it requires another var and the user needs
> to know about that magic var.
Not enabling a globally-enabled mode in particular buffers is essentially
ugly, so whatever solution we come up with will be ugly too.
> Maybe we could do it this way:
> for a globalized minor mode `foo', before running the major-mode hook, a magic
> `disable-foo-mode' is set to nil. A function is added to `foo-mode-hook'
> which sets this var to t when foo-mode is disabled. And after running
> the major-mode hook, the globalized minor mode code checks the magic var
> to decide whether to enable the minor mode.
!!!!!!!!!!!!!!!
> This way, the user doesn't need to know about the extra magic var; she
> can just call (foo-mode -1) in her major-mode hook as she can now, but
> the minor-mode is really only activated late.
> It's also kind of ugly because of all the magic, but it's the best I can
> come up with so far.
I can't come up with anything better at the moment, either, but I can't
help feeling that there's something more fundamental about global minor
modes that we're missing.
> Stefan
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch-2]
2013-01-17 18:31 ` Stefan Monnier
2013-01-18 12:07 ` João Távora
2013-01-18 17:09 ` Alan Mackenzie
@ 2013-01-31 11:04 ` Alan Mackenzie
2013-01-31 14:38 ` Stefan Monnier
2 siblings, 1 reply; 22+ messages in thread
From: Alan Mackenzie @ 2013-01-31 11:04 UTC (permalink / raw
To: Stefan Monnier; +Cc: Chong Yidong, emacs-devel, João Távora, Leo Liu
Hi, Stefan.
On Thu, Jan 17, 2013 at 01:31:32PM -0500, Stefan Monnier wrote:
[Note: this discussion is relevant to bug#11152.]
> The issue is how to locally turn off the globalized minor mode from
> a major-mode hook. If the globalized mode is enabled after the
> major-mode hook, you basically can't do it without resorting to
> ugly gymnastics.
> I'm not completely sure what happens (and what should happen instead) in
> your scenario, but for the case described by João, where you don't just
> want to disable it in some modes, but you want to prevent it from ever
> being enabled in such modes, obviously the current approach
> fails similarly.
> A simple solution is to activate the minor mode late, and provide
> a `disable-foo-mode' variable, that can be set in the major mode hook.
> But it's kind of ugly because it requires another var and the user needs
> to know about that magic var.
> Maybe we could do it this way:
> for a globalized minor mode `foo', before running the major-mode hook, a magic
> `disable-foo-mode' is set to nil. A function is added to `foo-mode-hook'
> which sets this var to t when foo-mode is disabled. And after running
> the major-mode hook, the globalized minor mode code checks the magic var
> to decide whether to enable the minor mode.
OK, I've bitten the bullet. Please see my patch below, which implements
this precisely. It seems to work, in that:
(i) It fixes #11152 - the new global-font-lock-mode calls font-lock-mode
_after_ the major mode hook.
(ii) Should the major mode hook do (font-lock-mode -1), this isn't
overridden by global mode.
> This way, the user doesn't need to know about the extra magic var; she
> can just call (foo-mode -1) in her major-mode hook as she can now, but
> the minor-mode is really only activated late.
> It's also kind of ugly because of all the magic, but it's the best I can
> come up with so far.
Here's the patch, based on easy-mmode.el in the emacs-24 branch:
=== modified file 'lisp/emacs-lisp/easy-mmode.el'
*** lisp/emacs-lisp/easy-mmode.el 2013-01-01 09:11:05 +0000
--- lisp/emacs-lisp/easy-mmode.el 2013-01-31 10:13:51 +0000
***************
*** 340,348 ****
enabled, then disabling and reenabling MODE should make MODE work
correctly with the current major mode. This is important to
prevent problems with derived modes, that is, major modes that
! call another major mode in their body."
(declare (doc-string 2))
(let* ((global-mode-name (symbol-name global-mode))
(pretty-name (easy-mmode-pretty-mode-name mode))
(pretty-global-name (easy-mmode-pretty-mode-name global-mode))
(group nil)
--- 340,353 ----
enabled, then disabling and reenabling MODE should make MODE work
correctly with the current major mode. This is important to
prevent problems with derived modes, that is, major modes that
! call another major mode in their body.
!
! MODE is actually turned on when a major mode is initialized, just
! after running the major mode's hook. However, MODE is not turned
! on, should the hook explicitly disable it."
(declare (doc-string 2))
(let* ((global-mode-name (symbol-name global-mode))
+ (mode-name (symbol-name mode))
(pretty-name (easy-mmode-pretty-mode-name mode))
(pretty-global-name (easy-mmode-pretty-mode-name global-mode))
(group nil)
***************
*** 353,358 ****
--- 358,369 ----
(MODE-check-buffers
(intern (concat global-mode-name "-check-buffers")))
(MODE-cmhh (intern (concat global-mode-name "-cmhh")))
+ (MODE-cancel-disable
+ (intern (concat global-mode-name "-cancel-disable")))
+ (MODE-disable-in-buffer
+ (intern (concat global-mode-name "-disable-in-buffer")))
+ (minor-MODE-hook (intern (concat mode-name "-hook")))
+ (disable-MODE (intern (concat "disable-" mode-name)))
(MODE-major-mode (intern (concat (symbol-name mode) "-major-mode")))
keyw)
***************
*** 397,403 ****
(add-hook 'after-change-major-mode-hook
',MODE-enable-in-buffers)
(add-hook 'change-major-mode-after-body-hook
! ',MODE-enable-in-buffers)
(add-hook 'find-file-hook ',MODE-check-buffers)
(add-hook 'change-major-mode-hook ',MODE-cmhh))
(remove-hook 'after-change-major-mode-hook ',MODE-enable-in-buffers)
--- 408,414 ----
(add-hook 'after-change-major-mode-hook
',MODE-enable-in-buffers)
(add-hook 'change-major-mode-after-body-hook
! ',MODE-cancel-disable)
(add-hook 'find-file-hook ',MODE-check-buffers)
(add-hook 'change-major-mode-hook ',MODE-cmhh))
(remove-hook 'after-change-major-mode-hook ',MODE-enable-in-buffers)
***************
*** 415,420 ****
--- 426,435 ----
;; up-to-here.
:autoload-end
+ ;; A function which checks whether MODE has been disabled in the major
+ ;; mode hook which has just been run.
+ (add-hook ',minor-MODE-hook ',MODE-disable-in-buffer)
+
;; List of buffers left to process.
(defvar ,MODE-buffers nil)
***************
*** 423,436 ****
(dolist (buf ,MODE-buffers)
(when (buffer-live-p buf)
(with-current-buffer buf
! (unless (eq ,MODE-major-mode major-mode)
! (if ,mode
! (progn
! (,mode -1)
! (,turn-on)
! (setq ,MODE-major-mode major-mode))
! (,turn-on)
! (setq ,MODE-major-mode major-mode)))))))
(put ',MODE-enable-in-buffers 'definition-name ',global-mode)
(defun ,MODE-check-buffers ()
--- 438,452 ----
(dolist (buf ,MODE-buffers)
(when (buffer-live-p buf)
(with-current-buffer buf
! (if ,disable-MODE
! (if ,mode (,mode -1))
! (unless (eq ,MODE-major-mode major-mode)
! (if ,mode
! (progn
! (,mode -1)
! (,turn-on))
! (,turn-on))))
! (setq ,MODE-major-mode major-mode)))))
(put ',MODE-enable-in-buffers 'definition-name ',global-mode)
(defun ,MODE-check-buffers ()
***************
*** 443,449 ****
(defun ,MODE-cmhh ()
(add-to-list ',MODE-buffers (current-buffer))
(add-hook 'post-command-hook ',MODE-check-buffers))
! (put ',MODE-cmhh 'definition-name ',global-mode))))
;;;
;;; easy-mmode-defmap
--- 459,473 ----
(defun ,MODE-cmhh ()
(add-to-list ',MODE-buffers (current-buffer))
(add-hook 'post-command-hook ',MODE-check-buffers))
! (put ',MODE-cmhh 'definition-name ',global-mode)
! (defvar ,disable-MODE nil)
! (defun ,MODE-cancel-disable ()
! (setq ,disable-MODE nil))
! (put ',MODE-cancel-disable 'definition-name ',global-mode)
! (defun ,MODE-disable-in-buffer ()
! (unless ,mode
! (setq ,disable-MODE t)))
! (put ',MODE-disable-in-buffer 'definition-name ',global-mode))))
;;;
;;; easy-mmode-defmap
> Stefan
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch-2]
2013-01-31 11:04 ` Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch-2] Alan Mackenzie
@ 2013-01-31 14:38 ` Stefan Monnier
2013-02-01 15:44 ` Alan Mackenzie
0 siblings, 1 reply; 22+ messages in thread
From: Stefan Monnier @ 2013-01-31 14:38 UTC (permalink / raw
To: Alan Mackenzie; +Cc: Leo Liu, Chong Yidong, João Távora, emacs-devel
> + (MODE-cancel-disable
> + (intern (concat global-mode-name "-cancel-disable")))
> + (MODE-disable-in-buffer
> + (intern (concat global-mode-name "-disable-in-buffer")))
If you defvar-local the MODE-disable-in-buffer, then you shouldn't need
MODE-cancel-disable since kill-all-local-variables will have reset
MODE-disable-in-buffer to nil already.
Stefan
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch-2]
2013-01-31 14:38 ` Stefan Monnier
@ 2013-02-01 15:44 ` Alan Mackenzie
2013-02-01 16:28 ` Stefan Monnier
0 siblings, 1 reply; 22+ messages in thread
From: Alan Mackenzie @ 2013-02-01 15:44 UTC (permalink / raw
To: Stefan Monnier; +Cc: Chong Yidong, emacs-devel, Leo Liu, João Távora
Hi, Stefan.
On Thu, Jan 31, 2013 at 09:38:31AM -0500, Stefan Monnier wrote:
> > + (MODE-cancel-disable
> > + (intern (concat global-mode-name "-cancel-disable")))
> > + (MODE-disable-in-buffer
> > + (intern (concat global-mode-name "-disable-in-buffer")))
> If you defvar-local the MODE-disable-in-buffer, .....
That would be disable-MODE I think you meant. Yes, I've now made
disable-MODE buffer local. It has to be buffer local, considering the
way that MODE-enable-in-buffers checks all buffers each time it runs.
> .... then you shouldn't need MODE-cancel-disable since
> kill-all-local-variables will have reset MODE-disable-in-buffer to nil
> already.
I'm not so sure about this. These complicated structures of macros and
hooks and generated functions are making my head hurt. ;-( Are you
sure there are no ways of invoking this which won't bypass
kill-all-local-variables? (That's a real question, not a rhetorical
one.)
Another thought. As I've coded it up, the disable-MODE flag, once it
becomes t, stays t (upto the next major mode change). However, running
global-MODE still enables MODE on this buffer. Should we worry about
this? I don't think we need to.
[Amended patch not included.]
> Stefan
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch-2]
2013-02-01 15:44 ` Alan Mackenzie
@ 2013-02-01 16:28 ` Stefan Monnier
2013-02-01 19:53 ` Alan Mackenzie
0 siblings, 1 reply; 22+ messages in thread
From: Stefan Monnier @ 2013-02-01 16:28 UTC (permalink / raw
To: Alan Mackenzie; +Cc: Chong Yidong, emacs-devel, Leo Liu, João Távora
> I'm not so sure about this. These complicated structures of macros and
> hooks and generated functions are making my head hurt. ;-( Are you
> sure there are no ways of invoking this which won't bypass
> kill-all-local-variables? (That's a real question, not a rhetorical
> one.)
Any major mode will have to call kill-all-local-variables, so it's
pretty sure.
Stefan
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch-2]
2013-02-01 16:28 ` Stefan Monnier
@ 2013-02-01 19:53 ` Alan Mackenzie
2013-02-01 20:09 ` Achim Gratz
2013-02-01 23:16 ` Stefan Monnier
0 siblings, 2 replies; 22+ messages in thread
From: Alan Mackenzie @ 2013-02-01 19:53 UTC (permalink / raw
To: Stefan Monnier; +Cc: João Távora, Chong Yidong, Leo Liu, emacs-devel
Hi, Stefan.
On Fri, Feb 01, 2013 at 11:28:00AM -0500, Stefan Monnier wrote:
> > I'm not so sure about this. These complicated structures of macros and
> > hooks and generated functions are making my head hurt. ;-( Are you
> > sure there are no ways of invoking this which won't bypass
> > kill-all-local-variables? (That's a real question, not a rhetorical
> > one.)
> Any major mode will have to call kill-all-local-variables, so it's
> pretty sure.
OK, I see it now. I've removed that function, replacing it with a
comment about kill-all-local-variables attending to disable-MODE.
I've also tidied up the new bit of doc-string I added,
Is it OK to commit this change to emacs-24 now, and close bug#11152?
Here's the patch:
=== modified file 'lisp/emacs-lisp/easy-mmode.el'
*** lisp/emacs-lisp/easy-mmode.el 2013-01-01 09:11:05 +0000
--- lisp/emacs-lisp/easy-mmode.el 2013-02-01 19:13:42 +0000
***************
*** 340,348 ****
enabled, then disabling and reenabling MODE should make MODE work
correctly with the current major mode. This is important to
prevent problems with derived modes, that is, major modes that
! call another major mode in their body."
(declare (doc-string 2))
(let* ((global-mode-name (symbol-name global-mode))
(pretty-name (easy-mmode-pretty-mode-name mode))
(pretty-global-name (easy-mmode-pretty-mode-name global-mode))
(group nil)
--- 340,353 ----
enabled, then disabling and reenabling MODE should make MODE work
correctly with the current major mode. This is important to
prevent problems with derived modes, that is, major modes that
! call another major mode in their body.
!
! When a major mode is initialized, MODE is actually turned on just
! after running the major mode's hook. However, MODE is not turned
! on if the hook has expllicitly disabled it."
(declare (doc-string 2))
(let* ((global-mode-name (symbol-name global-mode))
+ (mode-name (symbol-name mode))
(pretty-name (easy-mmode-pretty-mode-name mode))
(pretty-global-name (easy-mmode-pretty-mode-name global-mode))
(group nil)
***************
*** 353,358 ****
--- 358,367 ----
(MODE-check-buffers
(intern (concat global-mode-name "-check-buffers")))
(MODE-cmhh (intern (concat global-mode-name "-cmhh")))
+ (MODE-disable-in-buffer
+ (intern (concat global-mode-name "-disable-in-buffer")))
+ (minor-MODE-hook (intern (concat mode-name "-hook")))
+ (disable-MODE (intern (concat "disable-" mode-name)))
(MODE-major-mode (intern (concat (symbol-name mode) "-major-mode")))
keyw)
***************
*** 396,403 ****
(progn
(add-hook 'after-change-major-mode-hook
',MODE-enable-in-buffers)
- (add-hook 'change-major-mode-after-body-hook
- ',MODE-enable-in-buffers)
(add-hook 'find-file-hook ',MODE-check-buffers)
(add-hook 'change-major-mode-hook ',MODE-cmhh))
(remove-hook 'after-change-major-mode-hook ',MODE-enable-in-buffers)
--- 405,410 ----
***************
*** 415,420 ****
--- 422,431 ----
;; up-to-here.
:autoload-end
+ ;; A function which checks whether MODE has been disabled in the major
+ ;; mode hook which has just been run.
+ (add-hook ',minor-MODE-hook ',MODE-disable-in-buffer)
+
;; List of buffers left to process.
(defvar ,MODE-buffers nil)
***************
*** 423,436 ****
(dolist (buf ,MODE-buffers)
(when (buffer-live-p buf)
(with-current-buffer buf
! (unless (eq ,MODE-major-mode major-mode)
! (if ,mode
! (progn
! (,mode -1)
! (,turn-on)
! (setq ,MODE-major-mode major-mode))
! (,turn-on)
! (setq ,MODE-major-mode major-mode)))))))
(put ',MODE-enable-in-buffers 'definition-name ',global-mode)
(defun ,MODE-check-buffers ()
--- 434,448 ----
(dolist (buf ,MODE-buffers)
(when (buffer-live-p buf)
(with-current-buffer buf
! (if ,disable-MODE
! (if ,mode (,mode -1))
! (unless (eq ,MODE-major-mode major-mode)
! (if ,mode
! (progn
! (,mode -1)
! (,turn-on))
! (,turn-on))))
! (setq ,MODE-major-mode major-mode)))))
(put ',MODE-enable-in-buffers 'definition-name ',global-mode)
(defun ,MODE-check-buffers ()
***************
*** 443,449 ****
(defun ,MODE-cmhh ()
(add-to-list ',MODE-buffers (current-buffer))
(add-hook 'post-command-hook ',MODE-check-buffers))
! (put ',MODE-cmhh 'definition-name ',global-mode))))
;;;
;;; easy-mmode-defmap
--- 455,468 ----
(defun ,MODE-cmhh ()
(add-to-list ',MODE-buffers (current-buffer))
(add-hook 'post-command-hook ',MODE-check-buffers))
! (put ',MODE-cmhh 'definition-name ',global-mode)
! ;; disable-MODE is set in MODE-disable-in-buffer and cleared by
! ;; kill-all-local-variables.
! (defvar-local ,disable-MODE nil)
! (defun ,MODE-disable-in-buffer ()
! (unless ,mode
! (setq ,disable-MODE t)))
! (put ',MODE-disable-in-buffer 'definition-name ',global-mode))))
;;;
;;; easy-mmode-defmap
> Stefan
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch-2]
2013-02-01 19:53 ` Alan Mackenzie
@ 2013-02-01 20:09 ` Achim Gratz
2013-02-01 20:15 ` Alan Mackenzie
2013-02-01 23:17 ` Stefan Monnier
2013-02-01 23:16 ` Stefan Monnier
1 sibling, 2 replies; 22+ messages in thread
From: Achim Gratz @ 2013-02-01 20:09 UTC (permalink / raw
To: emacs-devel
Alan Mackenzie writes:
> ! on if the hook has expllicitly disabled it."
Should be "explicitly".
Regards,
Achim.
--
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+
Factory and User Sound Singles for Waldorf Q+, Q and microQ:
http://Synth.Stromeko.net/Downloads.html#WaldorfSounds
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch-2]
2013-02-01 20:09 ` Achim Gratz
@ 2013-02-01 20:15 ` Alan Mackenzie
2013-02-01 23:17 ` Stefan Monnier
1 sibling, 0 replies; 22+ messages in thread
From: Alan Mackenzie @ 2013-02-01 20:15 UTC (permalink / raw
To: Achim Gratz; +Cc: emacs-devel
Hello, Achim.
On Fri, Feb 01, 2013 at 09:09:34PM +0100, Achim Gratz wrote:
> Alan Mackenzie writes:
> > ! on if the hook has expllicitly disabled it."
> Should be "explicitly".
Yes indeed! Now corrected. Thanks!
> Regards,
> Achim.
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch-2]
2013-02-01 19:53 ` Alan Mackenzie
2013-02-01 20:09 ` Achim Gratz
@ 2013-02-01 23:16 ` Stefan Monnier
2013-02-03 22:14 ` Alan Mackenzie
1 sibling, 1 reply; 22+ messages in thread
From: Stefan Monnier @ 2013-02-01 23:16 UTC (permalink / raw
To: Alan Mackenzie; +Cc: João Távora, Chong Yidong, Leo Liu, emacs-devel
> 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.
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?
Stefan
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch-2]
2013-02-01 20:09 ` Achim Gratz
2013-02-01 20:15 ` Alan Mackenzie
@ 2013-02-01 23:17 ` Stefan Monnier
1 sibling, 0 replies; 22+ messages in thread
From: Stefan Monnier @ 2013-02-01 23:17 UTC (permalink / raw
To: Achim Gratz; +Cc: emacs-devel
>> ! on if the hook has expllicitly disabled it."
> Should be "explicitly".
You mean "exppllicittly"?
Stefan
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch-2]
2013-02-01 23:16 ` Stefan Monnier
@ 2013-02-03 22:14 ` Alan Mackenzie
0 siblings, 0 replies; 22+ messages in thread
From: Alan Mackenzie @ 2013-02-03 22:14 UTC (permalink / raw
To: Stefan Monnier; +Cc: João Távora, Chong Yidong, Leo Liu, emacs-devel
Hi, Stefan.
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:
revno: 100071
committer: Stefan Monnier <monnier@iro.umontreal.ca>
branch nick: trunk
timestamp: Wed 2010-04-28 11:18:37 -0400
message:
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 ----
(progn
(add-hook 'after-change-major-mode-hook
',MODE-enable-in-buffers)
+ (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 'after-change-major-mode-hook ',MODE-enable-in-buffers)
+ (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
change-major-mode-after-body-hook.
Stefan, have you any recollection at all of why you left
MODE-enable-in-buffers also in after-change-major-mode-hook?
> Stefan
--
Alan Mackenzie (Nuremberg, Germany).
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2013-02-03 22:14 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-13 19:28 Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode Alan Mackenzie
2013-01-13 20:48 ` Dmitry Gutov
2013-01-14 16:30 ` Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch] Alan Mackenzie
2013-01-14 16:52 ` emacs24/auctex bug Camm Maguire
2013-01-14 23:58 ` Xue Fuqiao
2013-01-15 2:12 ` Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch] Stefan Monnier
2013-01-15 14:08 ` Alan Mackenzie
2013-01-17 13:17 ` João Távora
2013-01-17 17:51 ` Alan Mackenzie
2013-01-17 18:31 ` Stefan Monnier
2013-01-18 12:07 ` João Távora
2013-01-18 17:09 ` Alan Mackenzie
2013-01-31 11:04 ` Redundant (harmful) duplication of run-hooks in define-globalized-minor-mode [patch-2] Alan Mackenzie
2013-01-31 14:38 ` Stefan Monnier
2013-02-01 15:44 ` Alan Mackenzie
2013-02-01 16:28 ` Stefan Monnier
2013-02-01 19:53 ` Alan Mackenzie
2013-02-01 20:09 ` Achim Gratz
2013-02-01 20:15 ` Alan Mackenzie
2013-02-01 23:17 ` Stefan Monnier
2013-02-01 23:16 ` Stefan Monnier
2013-02-03 22:14 ` Alan Mackenzie
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.