GNU bug report logs - #8613
"mode:" for minor-mode breaks set-visited-file-name

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 8613 in the body.
You can then email your comments to 8613 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #3 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: submit <at> debbugs.gnu.org
Subject: "mode:" for minor-mode breaks set-visited-file-name
Date: Tue, 03 May 2011 19:00:14 -0400

Package: emacs
Version: 23.3

The fact that "mode:" is allowed to indicate both major and minor modes
in file local variables is problematic. Things would be much simpler if
a different convention were used for minor modes.

There is bug#2355 (specifying a minor-mode in the first line breaks
major-mode detection); there is bug#5239 (major "mode:" specifications
only work right if they are at the start of the local variables list. It
might be nice to automatically move "mode:" to the front of the local
variables list, so that people don't have to worry about this; but since
"mode:" is also used for minor modes, and these probably need to come
AFTER setting of their relevant variables, this cannot be done.); there
is also bug#8586 (directory-local variables and files with "mode:"
cookies; I cannot see how to solve this given that "mode:" might mean
either a major or minor mode).

Here's another issue, affecting set-visited-file-name, caused by the
fact that the hack-local-variables MODE-ONLY feature cannot work right,
because all it does is check for a "mode:" cookie, and it has no way to
know if this is specifying a major-mode or a minor-mode:

>| foo.el
emacs -Q foo.el
M-x set-visited-file-name RET foo.f90 RET
  -> buffer switches to f90 mode

cat >| foo2.el <<EOF
;; Local Variables:
;; mode: outline-minor
;; End:
EOF

emacs -Q foo2.el
M-x set-visited-file-name RET foo2.f90 RET
  -> buffer stays in emacs-lisp-mode

Message #6 received at 8613 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Glenn Morris <rgm <at> gnu.org>
Cc: 8613 <at> debbugs.gnu.org
Subject: Re: bug#8613: "mode:" for minor-mode breaks set-visited-file-name
Date: Tue, 03 May 2011 21:38:31 -0300

> The fact that "mode:" is allowed to indicate both major and minor modes
> in file local variables is problematic. Things would be much simpler if
> a different convention were used for minor modes.

Agreed,


        Stefan

Message #9 received at 8613 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 8613 <at> debbugs.gnu.org
Subject: Re: bug#8613: "mode:" for minor-mode breaks set-visited-file-name
Date: Tue, 03 May 2011 22:08:17 -0400

Stefan Monnier wrote:

>> The fact that "mode:" is allowed to indicate both major and minor modes
>> in file local variables is problematic. Things would be much simpler if
>> a different convention were used for minor modes.
>
> Agreed,

So; shall we try to come up with a new convention for enabling
minor-modes via file-local variables, and deprecate using mode: for that?

All "mode:" does is funcall FOO-mode [1], which is nothing that can't be
done with :eval, except the latter will prompt for confirmation. Rather
than adding eg "minor-mode:", maybe safe-local-eval-forms could be
extended with a regexp element matching something like "([a-z]+-mode)".

[1] Well, it also ignores unknown modes.

Message #12 received at 8613 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Glenn Morris <rgm <at> gnu.org>
Cc: 8613 <at> debbugs.gnu.org
Subject: Re: bug#8613: "mode:" for minor-mode breaks set-visited-file-name
Date: Wed, 04 May 2011 09:52:42 -0300

>>> The fact that "mode:" is allowed to indicate both major and minor modes
>>> in file local variables is problematic. Things would be much simpler if
>>> a different convention were used for minor modes.
> So; shall we try to come up with a new convention for enabling
> minor-modes via file-local variables, and deprecate using mode: for that?

Yes, please.

> All "mode:" does is funcall FOO-mode [1], which is nothing that can't be
> done with :eval, except the latter will prompt for confirmation.  Rather
> than adding eg "minor-mode:", maybe safe-local-eval-forms could be
> extended with a regexp element matching something like "([a-z]+-mode)".

Not a bad idea, tho I'd much rather add
a (run-hook-with-args-until-success 'safe-local-eval-functions) instead
rather than rely on regexps.

Another option is to let define-minor-mode place
a `safe-local-eval-function' property on the minor-mode's symbol.


        Stefan

Message #15 received at 8613 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 8613 <at> debbugs.gnu.org
Subject: Re: bug#8613: "mode:" for minor-mode breaks set-visited-file-name
Date: Wed, 04 May 2011 22:20:00 -0400

Stefan Monnier wrote:

> Not a bad idea, tho I'd much rather add
> a (run-hook-with-args-until-success 'safe-local-eval-functions) instead
> rather than rely on regexps.

Regexps were the only way I could think of that did not require changes
to the definition of each minor mode, however...

> Another option is to let define-minor-mode place
> a `safe-local-eval-function' property on the minor-mode's symbol.

... that's nice and simple (I didn't know `safe-local-eval-function'
existed); but I had to tweak make-autoload to get it to work right with
autoloaded minor-modes (eg outline-minor).


*** lisp/emacs-lisp/autoload.el      2011-05-04 15:38:41 +0000
--- lisp/emacs-lisp/autoload.el      2011-05-05 02:14:30 +0000
***************
*** 190,195 ****
--- 190,197 ----
             (if (member ',file loads) nil
               (put ',groupname 'custom-loads (cons ',file loads))))))
  
+      ((eq car 'put) form)
+ 
       ;; nil here indicates that this is not a special autoload form.
       (t nil))))
  

*** lisp/emacs-lisp/easy-mmode.el	2011-01-25 04:08:28 +0000
--- lisp/emacs-lisp/easy-mmode.el	2011-05-05 00:44:36 +0000
***************
*** 115,120 ****
--- 115,122 ----
  :lighter SPEC	Same as the LIGHTER argument.
  :keymap MAP	Same as the KEYMAP argument.
  :require SYM	Same as in `defcustom'.
+ :safe PROP	Set the MODE function's `safe-local-eval-function' property
+ 		to PROP (default t).
  :variable PLACE	The location (as can be used with `setf') to use instead
  		of the variable MODE to store the state of the mode.  PLACE
  		can also be of the form (GET . SET) where GET is an expression
***************
*** 156,161 ****
--- 158,164 ----
           (setter nil)            ;The function (if any) to set the mode var.
           (modefun mode)          ;The minor mode function name we're defining.
  	 (require t)
+ 	 (safe t)
  	 (hook (intern (concat mode-name "-hook")))
  	 (hook-on (intern (concat mode-name "-on-hook")))
  	 (hook-off (intern (concat mode-name "-off-hook")))
***************
*** 174,179 ****
--- 177,183 ----
  	(:group (setq group (nconc group (list :group (pop body)))))
  	(:type (setq type (list :type (pop body))))
  	(:require (setq require (pop body)))
+ 	(:safe (setq safe (pop body)))
  	(:keymap (setq keymap (pop body)))
          (:variable (setq variable (pop body))
           (if (not (functionp (cdr-safe variable)))
***************
*** 264,269 ****
--- 268,275 ----
  	 ;; Return the new setting.
  	 ,mode)
  
+        (put ',modefun 'safe-local-eval-function ,safe)
+ 
         ;; Autoloading a define-minor-mode autoloads everything
         ;; up-to-here.
         :autoload-end

Message #18 received at 8613 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Glenn Morris <rgm <at> gnu.org>
Cc: 8613 <at> debbugs.gnu.org
Subject: Re: bug#8613: "mode:" for minor-mode breaks set-visited-file-name
Date: Thu, 05 May 2011 09:40:20 -0300

>> Not a bad idea, tho I'd much rather add
>> a (run-hook-with-args-until-success 'safe-local-eval-functions)
>> instead rather than rely on regexps.
> Regexps were the only way I could think of that did not require changes
> to the definition of each minor mode, however...

With a safe-local-eval-functions, you can do

(add-hook 'safe-local-eval-functions
          (lambda (form)
            (and (null (cdr form))
                 (symbolp (car form))
                 (string-match "-mode\\'" (symbol-name (car form))))))

>> Another option is to let define-minor-mode place
>> a `safe-local-eval-function' property on the minor-mode's symbol.

> ... that's nice and simple (I didn't know `safe-local-eval-function'
> existed); but I had to tweak make-autoload to get it to work right with
> autoloaded minor-modes (eg outline-minor).


> *** lisp/emacs-lisp/autoload.el      2011-05-04 15:38:41 +0000
> --- lisp/emacs-lisp/autoload.el      2011-05-05 02:14:30 +0000
> ***************
> *** 190,195 ****
> --- 190,197 ----
>              (if (member ',file loads) nil
>                (put ',groupname 'custom-loads (cons ',file loads))))))
>   
> +      ((eq car 'put) form)
> + 
>        ;; nil here indicates that this is not a special autoload form.
>        (t nil))))

Hmm... that will affect the (push (nth 1 form) autoloads-done).
Not sure if it matters.

> *** lisp/emacs-lisp/easy-mmode.el	2011-01-25 04:08:28 +0000
> --- lisp/emacs-lisp/easy-mmode.el	2011-05-05 00:44:36 +0000
> ***************
> *** 115,120 ****
> --- 115,122 ----
>   :lighter SPEC	Same as the LIGHTER argument.
>   :keymap MAP	Same as the KEYMAP argument.
>   :require SYM	Same as in `defcustom'.
> + :safe PROP	Set the MODE function's `safe-local-eval-function' property
> + 		to PROP (default t).

Not sure if we need it (currently all the -mode functions are presumed
safe when called without argument, as in the "mode:" cookie).
But if we do need it, it's better to reverse the meaning since we
should usually aim to make ":prop nil" behave the same as when
it's absent.


        Stefan

Message #21 received at 8613 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 8613 <at> debbugs.gnu.org
Subject: Re: bug#8613: "mode:" for minor-mode breaks set-visited-file-name
Date: Mon, 09 May 2011 18:05:25 -0400

Stefan Monnier wrote:

>>> Not a bad idea, tho I'd much rather add
>>> a (run-hook-with-args-until-success 'safe-local-eval-functions)
>>> instead rather than rely on regexps.
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
[...]
> With a safe-local-eval-functions, you can do
>
> (add-hook 'safe-local-eval-functions
>           (lambda (form)
>             (and (null (cdr form))
>                  (symbolp (car form))
>                  (string-match "-mode\\'" (symbol-name (car form))))))
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Does not compute.

Anyway, that would work, tho' perhaps it could just be hard-coded in
hack-one-local-variable-eval-safep, since the safety of mode: is not
customizable at present, and having hooks with non-empty defaults is
sub-optimal.

> Hmm... that will affect the (push (nth 1 form) autoloads-done).
> Not sure if it matters.

FWIW, I checked loaddefs.el without and without this change, and saw
only the expected differences.

>> + :safe PROP	Set the MODE function's `safe-local-eval-function' property
>> + 		to PROP (default t).
>
> Not sure if we need it (currently all the -mode functions are presumed
> safe when called without argument, as in the "mode:" cookie).
> But if we do need it, it's better to reverse the meaning since we
> should usually aim to make ":prop nil" behave the same as when
> it's absent.

The only reason I could see needing :safe as an argument was if someone
had some hypothetical minor-mode that for some reason was not safe. In
which case, passing ":safe nil" seems like the simplest thing to me,
rather than eg ":safe 'no", or inverting the whole thing to use :risky
instead of :safe.

But, there's no option to do this with mode:, so this is not needed to
replace mode: for minor-modes.

Message #24 received at 8613 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Glenn Morris <rgm <at> gnu.org>
Cc: 8613 <at> debbugs.gnu.org
Subject: Re: bug#8613: "mode:" for minor-mode breaks set-visited-file-name
Date: Mon, 09 May 2011 19:44:46 -0300

>> (string-match "-mode\\'" (symbol-name (car form))))))
>                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Does not compute.

Not sure why it wouldn't compute, but I'll trust you on that one.

> Anyway, that would work, tho' perhaps it could just be hard-coded in
> hack-one-local-variable-eval-safep, since the safety of mode: is not
> customizable at present, and having hooks with non-empty defaults is
> sub-optimal.

Agreed.  As mentioned recently, I'm fine with a non-nil hook as long as
it's preloaded and not defcustom.  But I think safe-local-eval-functions
would be a likely defcustom, so it should be nil.

> The only reason I could see needing :safe as an argument was if someone
> had some hypothetical minor-mode that for some reason was not safe. In
> which case, passing ":safe nil" seems like the simplest thing to me,
> rather than eg ":safe 'no", or inverting the whole thing to use :risky
> instead of :safe.

I prefer :risky.  It's always handy to be able to do things like
:foo (plist-get <bar> :foo), so ":foo nil" should behave the same as
its absence.

> But, there's no option to do this with mode:, so this is not needed to
> replace mode: for minor-modes.

Agreed.  If/when we need it we can add a :risky.  In the mean time if
someone really needs it she can add a (put 'mode
'safe-local-eval-function nil) after the define-minor-mode.


        Stefan

Message #27 received at 8613 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 8613 <at> debbugs.gnu.org
Subject: Re: bug#8613: "mode:" for minor-mode breaks set-visited-file-name
Date: Mon, 09 May 2011 19:41:16 -0400

Do you have a preference for a solution?
Here's the hack-one-local-variable-eval-safep version:

*** lisp/files.el	2011-04-24 00:24:30 +0000
--- lisp/files.el	2011-05-09 23:22:07 +0000
***************
*** 3327,3332 ****
--- 3327,3336 ----
        ;; Certain functions can be allowed with safe arguments
        ;; or can specify verification functions to try.
        (and (symbolp (car exp))
+ 	   ;; Allow (minor)-modes calls with no arguments.
+ 	   ;; This obsoletes the use of "mode:" for such things.  (Bug#8613)
+ 	   (or (and (null (cdr exp))
+ 		    (string-match "-mode\\'" (symbol-name (car exp))))
  	       (let ((prop (get (car exp) 'safe-local-eval-function)))
  		 (cond ((eq prop t)
  			(let ((ok t))
***************
*** 3341,3347 ****
  		      (dolist (function prop)
  			(if (funcall function exp)
  			    (setq ok t)))
! 		      ok)))))))
  
  (defun hack-one-local-variable (var val)
    "Set local variable VAR with value VAL.
--- 3345,3351 ----
  			  (dolist (function prop)
  			    (if (funcall function exp)
  				(setq ok t)))
! 			  ok))))))))
  
  (defun hack-one-local-variable (var val)
    "Set local variable VAR with value VAL.

Message #30 received at 8613 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Glenn Morris <rgm <at> gnu.org>
Cc: 8613 <at> debbugs.gnu.org
Subject: Re: bug#8613: "mode:" for minor-mode breaks set-visited-file-name
Date: Mon, 09 May 2011 22:09:29 -0300

> Do you have a preference for a solution?

Either is fine.


        Stefan

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.