bug#4148: mode: for a minor mode should turn on, not toggle the mode

bug#4148: mode: for a minor mode should turn on, not toggle the mode

[Dan Nicolaescu]

A well known long standing problem:
using 
mode: SOME_MINOR_MODE

in a local variable section (or in .dir-locals.el) toggles the minor
mode, it should turn it on.

So if the user sets a hook to turn on the minor mode in her emacs, when
using a file that sets the same minor mode, the minor mode ends up being
disable.

bug#4148: mode: for a minor mode should turn on, not toggle the mode

[Chong Yidong]

> A well known long standing problem:
> using 
> mode: SOME_MINOR_MODE

> in a local variable section (or in .dir-locals.el) toggles the minor
> mode, it should turn it on.

> So if the user sets a hook to turn on the minor mode in her emacs, when
> using a file that sets the same minor mode, the minor mode ends up being
> disable.

Does this patch DTRT?

*** trunk/lisp/files.el.~1.1066.~	2009-08-14 19:37:09.000000000 -0400
--- trunk/lisp/files.el	2009-08-15 13:37:24.000000000 -0400
***************
*** 3189,3195 ****
  				     "-mode"))))
  	   (unless (eq (indirect-function mode)
  		       (indirect-function major-mode))
! 	     (funcall mode))))
  	((eq var 'eval)
  	 (save-excursion (eval val)))
  	(t
--- 3189,3199 ----
  				     "-mode"))))
  	   (unless (eq (indirect-function mode)
  		       (indirect-function major-mode))
! 	     (if (memq mode minor-mode-list)
! 		 ;; For a minor mode, enable unconditionally instead
! 		 ;; of toggling (since the mode may already be on).
! 		 (funcall mode 1)
! 	       (funcall mode)))))
  	((eq var 'eval)
  	 (save-excursion (eval val)))
  	(t

bug#4148: mode: for a minor mode should turn on, not toggle the mode

[Dan Nicolaescu]

Chong Yidong <cyd@stupidchicken.com> writes:

  > > A well known long standing problem:
  > > using 
  > > mode: SOME_MINOR_MODE
  > 
  > > in a local variable section (or in .dir-locals.el) toggles the minor
  > > mode, it should turn it on.
  > 
  > > So if the user sets a hook to turn on the minor mode in her emacs, when
  > > using a file that sets the same minor mode, the minor mode ends up being
  > > disable.
  > 
  > Does this patch DTRT?
  > 
  > *** trunk/lisp/files.el.~1.1066.~	2009-08-14 19:37:09.000000000 -0400
  > --- trunk/lisp/files.el	2009-08-15 13:37:24.000000000 -0400
  > ***************
  > *** 3189,3195 ****
  >   				     "-mode"))))
  >   	   (unless (eq (indirect-function mode)
  >   		       (indirect-function major-mode))
  > ! 	     (funcall mode))))
  >   	((eq var 'eval)
  >   	 (save-excursion (eval val)))
  >   	(t
  > --- 3189,3199 ----
  >   				     "-mode"))))
  >   	   (unless (eq (indirect-function mode)
  >   		       (indirect-function major-mode))
  > ! 	     (if (memq mode minor-mode-list)
  > ! 		 ;; For a minor mode, enable unconditionally instead
  > ! 		 ;; of toggling (since the mode may already be on).
  > ! 		 (funcall mode 1)
  > ! 	       (funcall mode)))))
  >   	((eq var 'eval)
  >   	 (save-excursion (eval val)))
  >   	(t

Thanks, it seems to work fine.

After this gets checked in, we can re-add (mode . bug-reference) to emacs/.dir-locals.el

Any chance this patch can get onto the next release branch?

bug#4148: mode: for a minor mode should turn on, not toggle the mode

[Chong Yidong]

Dan Nicolaescu <dann@ics.uci.edu> writes:

> Thanks, it seems to work fine.
>
> After this gets checked in, we can re-add (mode . bug-reference) to
> emacs/.dir-locals.el
>
> Any chance this patch can get onto the next release branch?

I'm having trouble coming up with a test-case for testing this patch.
Do you have one?

bug#4148: mode: for a minor mode should turn on, not toggle the mode

[Dan Nicolaescu]

Chong Yidong <cyd@stupidchicken.com> writes:

  > Dan Nicolaescu <dann@ics.uci.edu> writes:
  > 
  > > Thanks, it seems to work fine.
  > >
  > > After this gets checked in, we can re-add (mode . bug-reference) to
  > > emacs/.dir-locals.el
  > >
  > > Any chance this patch can get onto the next release branch?
  > 
  > I'm having trouble coming up with a test-case for testing this patch.
  > Do you have one?

Here's one:

$ cat foo.el
(add-hook 'change-log-mode-hook (lambda () (bug-reference-mode 1)))

$ cat emacs/.dir-locals.el

((nil . ((tab-width . 8)
         (fill-column . 70)))
 (c-mode . ((c-file-style . "GNU")))
 (text-mode . ((mode . auto-fill)))
 (change-log-mode . ((add-log-time-zone-rule . t)
                     (fill-column . 74)
                     (bug-reference-url-format
                      . "http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=%s")
                     (mode . bug-reference))))

;; arch-tag: fb93c160-e9fe-4184-aad1-e4f5daa11cbd


$ emacs -Q -l foo.el
C-x C-f emacs/lisp/ChangeLog

C-h v bug-reference-mode RET

before your patch bug-reference-mode is nil, after it's t