bug#15577: 24.3; dir-local variables not applied when switching major-mode

[Phil Sainty <psainty@orcon.net.nz>]

My gut feeling is that retaining (across a major mode change)
the values of local variables which were set *interactively*
probably isn't a practical thing to try to do?

For starters we would have to flag variables as having been
set interactively, otherwise there would be no way of telling
which local values should be kept and which should be cleared.
(e.g. if a major mode hook sets a local variable, we're not
going to want to retain that when we change major modes.)

But even knowing that a value was set interactively still
isn't a *guarantee* that it should remain unchanged for the
new major mode?

What seems much simpler is to always hack-local-variables
after a major mode change, and simply work out a solution to
the associated problem with local 'mode' variables.

If we do that, then all of the local variables which are
*automatically* set for a given mode will be set correctly,
which is all people actually need, most of the time. That seems
to me like a "good enough" approach (and one which should be
safer).


At present `normal-mode' runs:
(report-errors "File local-variables error: %s"
  (hack-local-variables))

My first thought was to just move that into `run-mode-hooks',
but that relies upon the major mode function running its mode
hook, which I presume isn't guaranteed (and maybe this is exactly
why the code *does* run in `normal-mode'?)

It's unclear to me whether this is a real concern, though -- can we
assume that all major modes *will* call `kill-all-local-variables'
and `run-mode-hooks'? Certainly all the non-derived major modes
I've looked at in the Emacs lisp files take care of these steps
manually.

(Well... I suppose for any modes which don't do this housekeeping,
there's actually nothing we can do to help in the situation when
they are called interactively. So as long as we ensure that things
work when `normal-mode' runs, and also when any *well-behaved*
major mode is called interactively, we'll be doing everything that
we can reasonably do.)

So... how about this as a basic approach:

* Define a new buffer-local variable to track whether local vars
  have been hacked (nil by default). This will obviously get reset
  in any given buffer when a mode calls `kill-all-local-variables';
  and to take care of the case when `set-auto-mode' calls a
  'badly-behaved' mode (one which does not kill local vars), we
  can also let-bind the variable to nil in normal-mode.

* `normal-mode' then tests this variable before calling
  `hack-local-variables', only calling the function if the
  variable is nil. Most of the time, however, it will be
  non-nil, because...

* `run-mode-hooks' calls `hack-local-variables' after it has
  run `after-change-major-mode-hook'. This is therefore the way
  that the vast majority of major modes will set the local vars,
  (but the code in `normal-mode' will still account for
  exceptions).

See diffs below (from 24.3, and untested) for a possible
implementation.

This approach seems a little cleaner than the one used in the
Stack Overflow workaround, as this way we are more consistent
in *where* we call `hack-local-variables' for a given mode (the
other approach would either call it after the mode change OR in
normal-mode, depending on whether or not the mode was called
interactively.)



That leaves the problem of local 'mode' variables making it
impossible to interactively call a different mode (because we
immediately switch back to the mode specified by the local var).

The code has me slightly confused here: `hack-one-local-variable'
calls the mode function when there is a local mode variable, but
my initial thought is that we only want to use a local mode var
when running `set-auto-mode'? Yet set-auto-mode has already set
the mode -- using the (hack-local-variables MODE-ONLY) look-up --
*before* we get to `hack-one-local-variable', so should the latter
function actually ignore 'mode' vars instead? If we did that, then
the problem would go away. I suspect I'm missing something here?

Ah, okay; `autoload-find-file' is an example of code which could
need a mode var to be processed directly by `hack-one-local-variable'.

How about if we provide a new variable which determines whether
or not local mode variables will be applied? Or maybe it should be
an argument to `hack-local-variables'? By default we enable the
behaviour, so there's no change for any code which hacks local
variables directly; but in `normal-mode' and `run-mode-hooks'
we explicitly disable it, so that mode variables are ignored.

(n.b. The example code below does not implement this suggestion.)


-Phil





diff -u -L /usr/local/share/emacs/24.3/lisp/files.el.gz -L \#\<buffer\
files.el.gz\> /tmp/jka-com2287tTC /tmp/buffer-content-2287HoO
--- /usr/local/share/emacs/24.3/lisp/files.el.gz
+++ #<buffer files.el.gz>
@@ -2172,13 +2172,17 @@
 in that case, this function acts as if `enable-local-variables' were t."
   (interactive)
   (funcall (or (default-value 'major-mode) 'fundamental-mode))
-  (let ((enable-local-variables (or (not find-file)
enable-local-variables)))
+  (let ((enable-local-variables (or (not find-file)
enable-local-variables))
+        (local-variables-hacked nil))
     ;; FIXME this is less efficient than it could be, since both
     ;; s-a-m and h-l-v may parse the same regions, looking for "mode:".
     (report-errors "File mode specification error: %s"
       (set-auto-mode))
-    (report-errors "File local-variables error: %s"
-      (hack-local-variables)))
+    ;; Most major modes will hack local vars via `run-mode-hooks';
+    ;; this catches the exceptions:
+    (unless local-variables-hacked
+      (report-errors "File local-variables error: %s"
+        (hack-local-variables))))
   ;; Turn font lock off and on, to make sure it takes account of
   ;; whatever file local variables are relevant to it.
   (when (and font-lock-mode
@@ -3140,6 +3144,11 @@
 		   (assq-delete-all (car elt) file-local-variables-alist)))
 	   (push elt file-local-variables-alist)))))

+(defvar local-variables-hacked nil
+  "Buffer-local variable which indicates whether `hack-local-variables'
+has run for the current buffer.")
+(make-variable-buffer-local 'local-variables-hacked)
+
 (defun hack-local-variables (&optional mode-only)
   "Parse and put into effect this buffer's local variables spec.
 Uses `hack-local-variables-apply' to apply the variables.
@@ -3277,7 +3286,8 @@
       (if mode-only result
 	;; Otherwise, set the variables.
 	(hack-local-variables-filter result nil)
-	(hack-local-variables-apply)))))
+	(hack-local-variables-apply)
+        (setq local-variables-hacked t)))))

 (defun hack-local-variables-apply ()
   "Apply the elements of `file-local-variables-alist'.





diff -u -L /usr/local/share/emacs/24.3/lisp/subr.el.gz -L \#\<buffer\
subr.el.gz\> /tmp/jka-com2287GnH /tmp/buffer-content-2287g7T
--- /usr/local/share/emacs/24.3/lisp/subr.el.gz
+++ #<buffer subr.el.gz>
@@ -1588,7 +1588,9 @@
     (setq hooks (nconc (nreverse delayed-mode-hooks) hooks))
     (setq delayed-mode-hooks nil)
     (apply 'run-hooks (cons 'change-major-mode-after-body-hook hooks))
-    (run-hooks 'after-change-major-mode-hook)))
+    (run-hooks 'after-change-major-mode-hook)
+    (report-errors "File Local-variables error: %s"
+      (hack-local-variables))))

 (defmacro delay-mode-hooks (&rest body)
   "Execute BODY, but delay any `run-mode-hooks'.