* 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 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 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 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.