* bug#8613: "mode:" for minor-mode breaks set-visited-file-name
@ 2011-05-03 23:00 Glenn Morris
2011-05-04 0:38 ` Stefan Monnier
0 siblings, 1 reply; 10+ messages in thread
From: Glenn Morris @ 2011-05-03 23:00 UTC (permalink / raw)
To: 8613
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#8613: "mode:" for minor-mode breaks set-visited-file-name
2011-05-03 23:00 bug#8613: "mode:" for minor-mode breaks set-visited-file-name Glenn Morris
@ 2011-05-04 0:38 ` Stefan Monnier
2011-05-04 2:08 ` Glenn Morris
0 siblings, 1 reply; 10+ messages in thread
From: Stefan Monnier @ 2011-05-04 0:38 UTC (permalink / raw)
To: Glenn Morris; +Cc: 8613
> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#8613: "mode:" for minor-mode breaks set-visited-file-name
2011-05-04 0:38 ` Stefan Monnier
@ 2011-05-04 2:08 ` Glenn Morris
2011-05-04 12:52 ` Stefan Monnier
0 siblings, 1 reply; 10+ messages in thread
From: Glenn Morris @ 2011-05-04 2:08 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 8613
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#8613: "mode:" for minor-mode breaks set-visited-file-name
2011-05-04 2:08 ` Glenn Morris
@ 2011-05-04 12:52 ` Stefan Monnier
2011-05-05 2:20 ` Glenn Morris
0 siblings, 1 reply; 10+ messages in thread
From: Stefan Monnier @ 2011-05-04 12:52 UTC (permalink / raw)
To: Glenn Morris; +Cc: 8613
>>> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#8613: "mode:" for minor-mode breaks set-visited-file-name
2011-05-04 12:52 ` Stefan Monnier
@ 2011-05-05 2:20 ` Glenn Morris
2011-05-05 12:40 ` Stefan Monnier
0 siblings, 1 reply; 10+ messages in thread
From: Glenn Morris @ 2011-05-05 2:20 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 8613
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#8613: "mode:" for minor-mode breaks set-visited-file-name
2011-05-05 2:20 ` Glenn Morris
@ 2011-05-05 12:40 ` Stefan Monnier
2011-05-09 22:05 ` Glenn Morris
0 siblings, 1 reply; 10+ messages in thread
From: Stefan Monnier @ 2011-05-05 12:40 UTC (permalink / raw)
To: Glenn Morris; +Cc: 8613
>> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#8613: "mode:" for minor-mode breaks set-visited-file-name
2011-05-05 12:40 ` Stefan Monnier
@ 2011-05-09 22:05 ` Glenn Morris
2011-05-09 22:44 ` Stefan Monnier
0 siblings, 1 reply; 10+ messages in thread
From: Glenn Morris @ 2011-05-09 22:05 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 8613
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#8613: "mode:" for minor-mode breaks set-visited-file-name
2011-05-09 22:05 ` Glenn Morris
@ 2011-05-09 22:44 ` Stefan Monnier
2011-05-09 23:41 ` Glenn Morris
0 siblings, 1 reply; 10+ messages in thread
From: Stefan Monnier @ 2011-05-09 22:44 UTC (permalink / raw)
To: Glenn Morris; +Cc: 8613
>> (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
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#8613: "mode:" for minor-mode breaks set-visited-file-name
2011-05-09 22:44 ` Stefan Monnier
@ 2011-05-09 23:41 ` Glenn Morris
2011-05-10 1:09 ` Stefan Monnier
0 siblings, 1 reply; 10+ messages in thread
From: Glenn Morris @ 2011-05-09 23:41 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 8613
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.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#8613: "mode:" for minor-mode breaks set-visited-file-name
2011-05-09 23:41 ` Glenn Morris
@ 2011-05-10 1:09 ` Stefan Monnier
0 siblings, 0 replies; 10+ messages in thread
From: Stefan Monnier @ 2011-05-10 1:09 UTC (permalink / raw)
To: Glenn Morris; +Cc: 8613
> Do you have a preference for a solution?
Either is fine.
Stefan
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-05-10 1:09 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-05-03 23:00 bug#8613: "mode:" for minor-mode breaks set-visited-file-name Glenn Morris
2011-05-04 0:38 ` Stefan Monnier
2011-05-04 2:08 ` Glenn Morris
2011-05-04 12:52 ` Stefan Monnier
2011-05-05 2:20 ` Glenn Morris
2011-05-05 12:40 ` Stefan Monnier
2011-05-09 22:05 ` Glenn Morris
2011-05-09 22:44 ` Stefan Monnier
2011-05-09 23:41 ` Glenn Morris
2011-05-10 1:09 ` Stefan Monnier
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).