bug#347: C mode asks twice about local variables

[Alan Mackenzie]

Hi, Stefan!

On Mon, Jun 09, 2008 at 11:18:28AM -0400, Stefan Monnier wrote:
> >> > Yes.  This needs fixing, somehow.

> >> > The way this happens is that in a C file's local variables list, there
> >> > are two "special" variables, e.g. `c-file-style'.

> >> > When 

> >> >     c-basic-offset: 11
> >> >     c-file-style: "k&r"

> >> > occurs in the local variable list, this triggers a hook function
> >> > which calls (c-set-style "k&r").  The hook is
> >> > hack-local-variables-hook.  The problem is that this c-set-style call
> >> > will overwrite the explicit value for c-basic-offset.  The explicit
> >> > value MUST take precedence here.

> >> Can you try and call `c-file-style' with some extra "don't override"
> >> (when called from that hack-local-variables-hook) argument so that any
> >> variable that already has a buffer-local binding will not be
> >> overridden?

> Any comment about this suggestion?

YUCK!!!  ;-)

I don't know off-hand what the significance of buffer locality is in CC
Mode styles.  But trying to get a "don't override" parameter to work
would be horrendous.  hack-local-variables is actually called after the
language hook, isn't it?  By that time, the CC Mode style will already
have been set, and the pertinent variables will already be buffer local.
At least I think so.

> >> > My solution was to call hack-local-variables a second time from
> >> > within the hook function, first having deleted any occurrences of
> >> > `mode', `c-file-style' etc. from the Local Variables.  This kludge
> >> > worked reasonably well until the handling of
> >> > safe/dangerous-local-variables was changed for Emacs 22.

> >> How 'bout wrapping the call inside (let ((enable-local-variables
> >> :safe))?

> > I've been thinking this over.  It's not the right solution.  For a start,
> > the second call to `hack-local-variables' is in itself a kludge.

> I'd tend to agree, but "kludge" fixes the problem, it's still better
> than the current "same kludge + problem".

> > More importantly, the value ":safe" is non-portable (to earlier Emacsen,
> > or to the other one),

> Non-portability might indeed be a problem (tho, it's obviously "your"
> problem rather than mine), but introducing a new hook in Emacs-23 won't
> help you there, so it's not relevant to this discussion.

What I had in mind was using an (if (boundp 'before-hack-local-variables)
...) to separate out new strategy from old.

> > and doesn't feel at all safe.

> But I don't see why you don't consider it safe: it is really meant to be
> safe, so if there's any doubt about it, we should very much address it.

At some stage, maybe, perhaps, another option, single:, will come into
being.  This will mean "prompt for each non-safe variable individually".
Explicitly setting `enable-local-variable' in CC Mode will couple CC Mode
very strongly to files.el.  That's not a good thing.

> > (defvar before-hack-local-variables-hook nil
> [...]
> > What do you think?

> I'm not thrilled.  You might convince me at some point, but the kludge
> doesn't look nearly as bad when compared to this hook.

Actually, it's a lot simpler than I expectied.  We can use the already
read (as in lisp read) version of variables' values and we needn't
separate out the "-*-" values from the "Local Variables:" values.  In
that case, the entire change to `hack-local-variables' would be:

(i) Rename `result' to `hack-local-variables-alist'.  This variable
  contains the spec of all local variables as an alist, and would be
  available to hook functions for manipulation.
(ii) Insert this single line of code:

      ;; We've read all the local variables.  Now, return whether the
      ;; mode is specified (if MODE-ONLY is non-nil), or set the
      ;; variables (if MODE-ONLY is nil.)
      (if mode-only
          result
        (run-hooks 'before-hack-local-variables-hook) ; <============= NEW LINE
        (dolist (ignored ignored-local-variables)
          (setq hack-local-variables-list
                (assq-delete-all ignored hack-local-variables-alist)))



The new hook function in CC Mode (which would supersede the old one for
Emacs 23 (?or 22.3?)) would be this:

(defun c-before-hack-hook ()
  "Doc string."
  (let ((stile (cdr (assq 'c-file-style hack-local-variables-list)))
        (offsets (cdr (assq 'c-file-offsets hack-local-variables-list))))
    (when stile
      (or (stringp stile) (error "...."))
      (c-set-style stile))
    (when offsets
      (mapc
          (lambda (langentry)
            (let ((langelem (car langentry))
                  (offset (cdr langentry)))
              (c-set-offset langelem offset)))
          offsets))))

This would replace ~90 lines of existing code (once Emacs 22 has fallen
into desuetude).

Oh, and the hook would need documenting in the Elisp manual, but I can
manage that.

Hey, this is so easy and obviously the right thing.  Let's do it!

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).