bug#343: C mode asks twice about local variables

bug#343: C mode asks twice about local variables

[Glenn Morris]

This applies in Emacs 22.2 and CVS trunk.

emacs -Q lib-src/etags.c

    The local variables list in etags.c
    contains values that may not be safe (*).

    Do you want to apply it?  You can type
    y  -- to apply the local variables list.
    n  -- to ignore the local variables list.
    !  -- to apply the local variables list, and permanently mark these
    values (*) as safe (in the future, they will be set automatically.)

    indent-tabs-mode: t
    tab-width: 8
    fill-column: 79
  * c-font-lock-extra-types: ("FILE" "bool" "language" "linebuffer" "fdesc" "node" "regexp")
    c-file-style: "gnu"


If I answer `y', nothing happens. I have to press `y' a second time.

Re: C mode asks twice about local variables

[Alan Mackenzie]

Hi, Glenn,

On Sat, May 31, 2008 at 06:51:35PM -0400, Glenn Morris wrote:

> This applies in Emacs 22.2 and CVS trunk.

> emacs -Q lib-src/etags.c

>     The local variables list in etags.c
>     contains values that may not be safe (*).

>     Do you want to apply it?  You can type
>     y  -- to apply the local variables list.
>     n  -- to ignore the local variables list.
>     !  -- to apply the local variables list, and permanently mark these
>     values (*) as safe (in the future, they will be set automatically.)

>     indent-tabs-mode: t
>     tab-width: 8
>     fill-column: 79
>   * c-font-lock-extra-types: ("FILE" "bool" "language" "linebuffer" "fdesc" "node" "regexp")
>     c-file-style: "gnu"


> If I answer `y', nothing happens. I have to press `y' a second time.

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.

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.

Simply marking all pertinent variables as "safe" is not the right fix,
because they aren't: some can contain arbitrary functions.

Emacs doesn't really have the requisite hooks here:
`before-hack-local-variables-hook' and `after-hack-local-variables-hook'
would let the problem be fixed (rather than kludged), but it kind of
feels like overkill.

If Emacs were not to rebind the do-you-want-to-apply-it variable inside
hack-local-variables, that would allow the kludge to work.

Suggestions and helpful comments would be appreciated here.

-- 
Alan Mackenzie (Nuremberg, Germany).

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

bug#347: C mode asks twice about local variables

[Stefan Monnier]

> 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?

> 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))?

> If Emacs were not to rebind the do-you-want-to-apply-it variable inside
> hack-local-variables, that would allow the kludge to work.

I don't know what you're referring to here.


        Stefan

Re: bug#347: C mode asks twice about local variables

[Alan Mackenzie]

Hi, Stefan and Glenn,

On Tue, Jun 03, 2008 at 02:40:29AM -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?

> > 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.

More importantly, the value ":safe" is non-portable (to earlier Emacsen,
or to the other one), and doesn't feel at all safe.  I'd have to emulate,
to some extent, the processing of `enable-local-variables' inside CC Mode
code, and this variable gives the distinct feeling of not yet having
reached its final definition.  And I'd almost certainly get this wrong,
either causing lots of bug reports for the failure of LV handling or
leaving a security hole for unsafe variables.

The right thing is to increase the granularity of control over the Local
Variables handling.  In particular, to have a new hook called just after
parsing the local variables, but before setting them.  I think it should
look like this:

#########################################################################
(defvar before-hack-local-variables-hook nil
  "Normal hook run before setting a file's local variables.
This hook is called only when there is at least one local
variable to set.

At the time of call, the content of the \"-*-\" line at the top
of the buffer and of any \"Local Variables\" section have been
parsed, respectively in the two dynamically bound variables
`hack-variables-on-prop-line' and `hack-variables-in-list'.  Each
of these is an alist with elements of the form (VAR . VALUE), VAR
being the name of the variable to set and VALUE the raw string
representation of its value.  The elements are in the same order
as in the buffer.

A hook function may change the value of either of these alists.

Point is at the start of the line with \"Local Variables\", or at
the beginning of buffer if there is no such line.")
#########################################################################

What do you think?

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php

Re: bug#347: C mode asks twice about local variables

[Stefan Monnier]

>> > 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?

>> > 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.

> 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.

> (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.


        Stefan

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php

Re: 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).

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php

Re: bug#347: C mode asks twice about local variables

[Stefan Monnier]

[ Moving this over to emacs-devel. ]

>> >> 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.

Could you ask the CC mode maintainer, maybe?

> 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.

I see, yes, it might be messy or even impossible.

>> 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.

I'm sure we can come up with a good test to decide whether :safe
will do the right thing.

> 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!

It's going in the right direction but I'm still not completely satisfied.
How 'bout something like the following:

Some variables can be flagged as being special, in that when they apper
in the file-local list of settings, they "get set" by calling
a function.  E.g. `mode' is globally special and "setting it" will
actually call the corresponding mode.

Then `c-mode' can define `c-file-style' and `c-file-offsets' as being
special so that setting them actually calls a function of yours
(probably a function that could also be used for the :set in defcustom,
BTW).

Of course, the interesting bit is that hack-local-variables will be
careful to reorder the file-local settings such that special variables
are set first.


        Stefan

Re: bug#347: C mode asks twice about local variables

[Glenn Morris]

Stefan Monnier wrote:

> Some variables can be flagged as being special, in that when they apper
> in the file-local list of settings, they "get set" by calling
> a function.
[...]
> (probably a function that could also be used for the :set in defcustom,
[...]
> Of course, the interesting bit is that hack-local-variables will be
> careful to reorder the file-local settings such that special variables
> are set first.

Sounds like hack-local-variables should respect custom's :set-after
and :set parameters. Then what works for custom should Just Work for
hack-local-variables too.

Re: bug#347: C mode asks twice about local variables

[Stefan Monnier]

>> Some variables can be flagged as being special, in that when they apper
>> in the file-local list of settings, they "get set" by calling
>> a function.
> [...]
>> (probably a function that could also be used for the :set in defcustom,
> [...]
>> Of course, the interesting bit is that hack-local-variables will be
>> careful to reorder the file-local settings such that special variables
>> are set first.

> Sounds like hack-local-variables should respect custom's :set-after
> and :set parameters. Then what works for custom should Just Work for
> hack-local-variables too.

In theory, that's also what I thought at first.  But since defcustom
works well for global settings but poorly for buffer-local settings, you
can't assume that all :set and :set-after functions will work properly
when called for buffer-local settings.


        Stefan

Re: bug#347: C mode asks twice about local variables

[Alan Mackenzie]

'Evening, Stefan and Glenn!

On Wed, Jun 11, 2008 at 10:41:38AM -0400, Stefan Monnier wrote:
> > I don't know off-hand what the significance of buffer locality is in
> > CC Mode styles.

> Could you ask the CC mode maintainer, maybe?

He was as confused about it as I was.

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

[ .... ]

> > 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!

> It's going in the right direction but I'm still not completely
> satisfied.  How 'bout something like the following:

> Some variables can be flagged as being special, in that when they apper
> in the file-local list of settings, they "get set" by calling
> a function.  E.g. `mode' is globally special and "setting it" will
> actually call the corresponding mode.

> Then `c-mode' can define `c-file-style' and `c-file-offsets' as being
> special so that setting them actually calls a function of yours
> (probably a function that could also be used for the :set in defcustom,
> BTW).

> Of course, the interesting bit is that hack-local-variables will be
> careful to reorder the file-local settings such that special variables
> are set first.

Why, in general, should "special" variables always be set first?  I
can't feel any enthusiasm for this approach.

To verify my feelings about the `before-hack-local-variables-hook'
approach, I have done a trial implementation of it.  It was a little
more complicated that I'd expected - I had to refactor a bit, replacing
`hack-local-variables-apply' with `hack-local-variables-filter' (which
removes unwanted variables from the hack-list rather than setting the
rest) and moving the setting of the hack-variables into h-l-v itself.

The change to cc-mode.el was straightforward.

See what you think.  Here is the trial patch to the code:



2008-06-15  Alan Mackenzie  <acm@muc.de>

	* progmodes/cc-mode.el (c-before-hack-hook): New function
	(Top Level): Install c-before-hack-hook on
	before-hack-local-variables-hook, rather than
	c-postprocess-file-styles on hack-local-variables-hook.

	* files.el (hack-local-variables-alist): New variable.
	(before-hack-local-variables-hook): New hook.
	(hack-local-variables-filter): Refactored version of
	Hack-local-variables-apply.
	(hack-local-variables): Call `before-hack-local-variables-hook'.



Index: cc-mode.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/progmodes/cc-mode.el,v
retrieving revision 1.76
diff -c -r1.76 cc-mode.el
*** cc-mode.el	26 May 2008 06:57:40 -0000	1.76
--- cc-mode.el	15 Jun 2008 21:06:32 -0000
***************
*** 656,661 ****
--- 656,681 ----
        (and (cdr rfn)
  	   (setq require-final-newline mode-require-final-newline)))))
  
+ (defun c-before-hack-hook ()
+   "Set the CC Mode style and \"offsets\" when in the buffer's local variables.
+ They are set only when, respectively, the pseudo variables
+ `c-file-style' and `c-file-offsets' are present in the list.
+ 
+ This function is called from the hook `before-hack-local-variables-hook'."
+   (when c-buffer-is-cc-mode
+     (let ((stile (cdr (assq 'c-file-style hack-local-variables-alist)))
+ 	  (offsets (cdr (assq 'c-file-offsets hack-local-variables-alist))))
+       (when stile
+ 	(or (stringp stile) (error "c-file-style is not a string"))
+ 	(c-set-style stile))
+       (when offsets
+ 	(mapc
+ 	 (lambda (langentry)
+ 	   (let ((langelem (car langentry))
+ 		 (offset (cdr langentry)))
+ 	     (c-set-offset langelem offset)))
+ 	 offsets)))))
+ 
  (defun c-remove-any-local-eval-or-mode-variables ()
    ;; If the buffer specifies `mode' or `eval' in its File Local Variable list
    ;; or on the first line, remove all occurrences.  See
***************
*** 747,753 ****
  	    (hack-local-variables))
  	  nil))))
  
! (add-hook 'hack-local-variables-hook 'c-postprocess-file-styles)
  
  (defmacro c-run-mode-hooks (&rest hooks)
    ;; Emacs 21.1 has introduced a system with delayed mode hooks that
--- 767,775 ----
  	    (hack-local-variables))
  	  nil))))
  
! (if (boundp 'before-hack-local-variables-hook)
!     (add-hook 'before-hack-local-variables-hook 'c-before-hack-hook)
!   (add-hook 'hack-local-variables-hook 'c-postprocess-file-styles))
  
  (defmacro c-run-mode-hooks (&rest hooks)
    ;; Emacs 21.1 has introduced a system with delayed mode hooks that




Index: files.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/files.el,v
retrieving revision 1.985
diff -c -r1.985 files.el
*** files.el	11 Jun 2008 01:47:47 -0000	1.985
--- files.el	15 Jun 2008 20:46:17 -0000
***************
*** 2514,2519 ****
--- 2514,2539 ----
    '(ignored-local-variables safe-local-variable-values)
    "Variables to be ignored in a file's local variable spec.")
  
+ (defvar hack-local-variables-alist nil
+   "Alist of (VAR . VALUE) pairs read from a buffer's local variables.
+ VAR, a symbol, is a variable to be set, and VALUE (unevaluated) is
+ the value it will get set to.
+ 
+ This alist contains the settings from both the \"-*-\" line at
+ the top of the buffer and the \"Local Variables\:\" section near
+ the bottom of the buffer.  The settings are in the same order as
+ in the buffer.")
+ 
+ (defvar before-hack-local-variables-hook nil
+   "Normal hook run before setting a file's local variables.
+ It is called after the checks for unsafe and risky variables are
+ done.  It is called only when there is at least one local
+ variable to set.
+ 
+ The details of the local variables are in the variable
+ `hack-local-variables-alist'; a hook function may change the
+ contents of this alist.")
+ 
  (defvar hack-local-variables-hook nil
    "Normal hook run after processing a file's local variables specs.
  Major modes can use this to examine user-specified local variables
***************
*** 2777,2798 ****
  	  mode-specified
  	result))))
  
! (defun hack-local-variables-apply (result project)
!   "Apply an alist of local variable settings.
! RESULT is the alist.
! Will query the user when necessary."
    (dolist (ignored ignored-local-variables)
!     (setq result (assq-delete-all ignored result)))
    (if (null enable-local-eval)
!       (setq result (assq-delete-all 'eval result)))
!   (when result
!     (setq result (nreverse result))
      ;; Find those variables that we may want to save to
      ;; `safe-local-variable-values'.
      (let (risky-vars unsafe-vars)
!       (dolist (elt result)
  	(let ((var (car elt))
  	      (val (cdr elt)))
  	  ;; Don't query about the fake variables.
  	  (or (memq var '(mode unibyte coding))
  	      (and (eq var 'eval)
--- 2797,2830 ----
  	  mode-specified
  	result))))
  
! (defun hack-local-variables-filter (variables project)
!   "Remove risky \(etc.) local variables from VARIABLES.
! These are determined from the options `enable-local-variables',
! `enable-local-eval', `ignored-local-variables' and possibly the
! result of querying the user.
! 
! VARIABLES is an alist, each element of which has the form (VAR
! . VALUE), VAR being a variable to set (a symbol), VALUE being its
! \(unevaluted) value.  This format is the same as
! `hack-local-variables-alist''s.  This function might modify
! VARIABLES's list structure.
! 
! PROJECT is .... ?????
! 
! The function's result is VARIABLES with all rejected variables
! removed.  This may well be nil."
    (dolist (ignored ignored-local-variables)
!     (setq variables (assq-delete-all ignored variables)))
    (if (null enable-local-eval)
!       (setq variables (assq-delete-all 'eval variables)))
!   (when variables
      ;; Find those variables that we may want to save to
      ;; `safe-local-variable-values'.
      (let (risky-vars unsafe-vars)
!       (dolist (elt variables)
  	(let ((var (car elt))
  	      (val (cdr elt)))
+ 	  ;; Scan the variables, collecting risky and unsafe ones.
  	  ;; Don't query about the fake variables.
  	  (or (memq var '(mode unibyte coding))
  	      (and (eq var 'eval)
***************
*** 2803,2815 ****
  	      (and (risky-local-variable-p var val)
  		   (push elt risky-vars))
  	      (push elt unsafe-vars))))
        (if (eq enable-local-variables :safe)
  	  ;; If caller wants only the safe variables,
! 	  ;; install only them.
! 	  (dolist (elt result)
! 	    (unless (or (member elt unsafe-vars)
! 			(member elt risky-vars))
! 	      (hack-one-local-variable (car elt) (cdr elt))))
  	;; Query, except in the case where all are known safe
  	;; if the user wants no query in that case.
  	(if (or (and (eq enable-local-variables t)
--- 2835,2848 ----
  	      (and (risky-local-variable-p var val)
  		   (push elt risky-vars))
  	      (push elt unsafe-vars))))
+ 
        (if (eq enable-local-variables :safe)
  	  ;; If caller wants only the safe variables,
! 	  ;; expunge the list of the rest.
! 	  (dolist (elt variables)
! 	    (if (or (member elt unsafe-vars)
! 		    (member elt risky-vars))
! 	      (setq variables (assq-delete-all (car elt) variables))))
  	;; Query, except in the case where all are known safe
  	;; if the user wants no query in that case.
  	(if (or (and (eq enable-local-variables t)
***************
*** 2817,2825 ****
  		     (null risky-vars))
  		(eq enable-local-variables :all)
  		(hack-local-variables-confirm
! 		 result unsafe-vars risky-vars project))
! 	    (dolist (elt result)
! 	      (hack-one-local-variable (car elt) (cdr elt))))))))
  
  (defun hack-local-variables (&optional mode-only)
    "Parse and put into effect this buffer's local variables spec.
--- 2850,2858 ----
  		     (null risky-vars))
  		(eq enable-local-variables :all)
  		(hack-local-variables-confirm
! 		 variables unsafe-vars risky-vars project))
! 	    variables
! 	  )))))
  
  (defun hack-local-variables (&optional mode-only)
    "Parse and put into effect this buffer's local variables spec.
***************
*** 2827,2835 ****
  is specified, returning t if it is specified."
    (let ((enable-local-variables
  	 (and local-enable-local-variables enable-local-variables))
! 	result)
      (when (or mode-only enable-local-variables)
!       (setq result (hack-local-variables-prop-line mode-only))
        ;; Look for "Local variables:" line in last page.
        (save-excursion
  	(goto-char (point-max))
--- 2860,2868 ----
  is specified, returning t if it is specified."
    (let ((enable-local-variables
  	 (and local-enable-local-variables enable-local-variables))
! 	result hack-local-variables-alist)
      (when (or mode-only enable-local-variables)
!       (setq hack-local-variables-alist (hack-local-variables-prop-line mode-only))
        ;; Look for "Local variables:" line in last page.
        (save-excursion
  	(goto-char (point-max))
***************
*** 2906,2921 ****
  			    (push (cons (if (eq var 'eval)
  					    'eval
  					  (indirect-variable var))
! 					val) result)
  			  (error nil)))))
  		  (forward-line 1)))))))
  
        ;; 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
! 	(hack-local-variables-apply result nil)
  	(run-hooks 'hack-local-variables-hook)))))
  
  (defun safe-local-variable-p (sym val)
--- 2939,2959 ----
  			    (push (cons (if (eq var 'eval)
  					    'eval
  					  (indirect-variable var))
! 					val) hack-local-variables-alist)
  			  (error nil)))))
  		  (forward-line 1)))))))
+       (setq hack-local-variables-alist (nreverse hack-local-variables-alist))
  
        ;; 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
! 	(setq hack-local-variables-alist
! 	      (hack-local-variables-filter hack-local-variables-alist nil))
! 	(run-hooks 'before-hack-local-variables-hook)
! 	(dolist (elt hack-local-variables-alist)
! 	  (hack-one-local-variable (car elt) (cdr elt)))
  	(run-hooks 'hack-local-variables-hook)))))
  
  (defun safe-local-variable-p (sym val)

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: bug#347: C mode asks twice about local variables

[Alan Mackenzie]

Hi, Stefan!

On Sun, Jun 15, 2008 at 10:04:12PM +0000, Alan Mackenzie wrote:
> 'Evening, Stefan and Glenn!

[ Followed by a patch to cc-mode.el and files.el which would introduce a
new hook, before-hack-local-variables-hook, with this ChangeLog entry.]

> 2008-06-15  Alan Mackenzie  <acm@muc.de>

> 	* progmodes/cc-mode.el (c-before-hack-hook): New function
> 	(Top Level): Install c-before-hack-hook on
> 	before-hack-local-variables-hook, rather than
> 	c-postprocess-file-styles on hack-local-variables-hook.

> 	* files.el (hack-local-variables-alist): New variable.
> 	(before-hack-local-variables-hook): New hook.
> 	(hack-local-variables-filter): Refactored version of
> 	Hack-local-variables-apply.
> 	(hack-local-variables): Call `before-hack-local-variables-hook'.

You haven't got round to replying yet!  We have a feature freeze in a
very few days.

This patch would solve the problem of the double question.  Shall I
install this (and write some Docu)?

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: bug#347: C mode asks twice about local variables

[Chong Yidong]

Alan Mackenzie <acm@muc.de> writes:

>> 2008-06-15  Alan Mackenzie  <acm@muc.de>
>
>> 	* progmodes/cc-mode.el (c-before-hack-hook): New function
>> 	(Top Level): Install c-before-hack-hook on
>> 	before-hack-local-variables-hook, rather than
>> 	c-postprocess-file-styles on hack-local-variables-hook.
>
>> 	* files.el (hack-local-variables-alist): New variable.
>> 	(before-hack-local-variables-hook): New hook.
>> 	(hack-local-variables-filter): Refactored version of
>> 	Hack-local-variables-apply.
>> 	(hack-local-variables): Call `before-hack-local-variables-hook'.

Looks OK to me, except for two things:

- hack-local-variables-alist, before-hack-local-variables-hook, and
  c-before-hack-hook need to be marked as risky local vars.

- how bout renaming hack-local-variables-alist to
  file-local-variables-alist (which is a better description), making it
  buffer-local, and setting its buffer value instead of let-binding it?
  Then it would provide a lasting record what vars were set in this
  buffer using the hack-local-variables mechanism, which might be a
  useful thing to have.

If no one has any other objections in a couple of days, I'd go ahead and
check it in.  Please add a note to NEWS as well.

Re: bug#347: C mode asks twice about local variables

[Chong Yidong]

Chong Yidong <cyd@stupidchicken.com> writes:

> - hack-local-variables-alist, before-hack-local-variables-hook, and
>   c-before-hack-hook need to be marked as risky local vars.

Come to think of it, maybe marking hack-local-variables-alist as risky
may not be enough.  It may be better to make the local vars code handle
it specially by ignoring it.

Re: bug#347: C mode asks twice about local variables

[Johan Bockgård]

Chong Yidong <cyd@stupidchicken.com> writes:

> - hack-local-variables-alist, before-hack-local-variables-hook, and
>   c-before-hack-hook need to be marked as risky local vars.

Names ending in -hook are automatically considered risky.

-- 
Johan Bockgård

Re: bug#347: C mode asks twice about local variables

[Chong Yidong]

Hi Alan,

>> 2008-06-15  Alan Mackenzie  <acm@muc.de>
>
>> 	* progmodes/cc-mode.el (c-before-hack-hook): New function
>> 	(Top Level): Install c-before-hack-hook on
>> 	before-hack-local-variables-hook, rather than
>> 	c-postprocess-file-styles on hack-local-variables-hook.
>
>> 	* files.el (hack-local-variables-alist): New variable.
>> 	(before-hack-local-variables-hook): New hook.
>> 	(hack-local-variables-filter): Refactored version of
>> 	Hack-local-variables-apply.
>> 	(hack-local-variables): Call `before-hack-local-variables-hook'.

I've checked in a modified version of this patch.  Could you test it out
and see if it DTRT?

Thanks!

bug#343: marked as done (C mode asks twice about local variables)

[Emacs bug Tracking System]

[-- Attachment #1: Type: text/plain, Size: 843 bytes --]


Your message dated Thu, 31 Jul 2008 10:14:42 -0400
with message-id <871w1aywbx.fsf@stupidchicken.com>
and subject line Re: C mode asks twice about local variables
has caused the Emacs bug report #343,
regarding C mode asks twice about local variables
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact don@donarmstrong.com
immediately.)


-- 
343: http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=343
Emacs Bug Tracking System
Contact don@donarmstrong.com with problems

[-- Attachment #2: Type: message/rfc822, Size: 2708 bytes --]

From: Glenn Morris <rgm@gnu.org>
To: bug-gnu-emacs@gnu.org
Cc: bug-cc-mode@gnu.org
Subject: C mode asks twice about local variables
Date: Sat, 31 May 2008 18:51:35 -0400
Message-ID: <s8abi6ccjs.fsf@fencepost.gnu.org>


This applies in Emacs 22.2 and CVS trunk.

emacs -Q lib-src/etags.c

    The local variables list in etags.c
    contains values that may not be safe (*).

    Do you want to apply it?  You can type
    y  -- to apply the local variables list.
    n  -- to ignore the local variables list.
    !  -- to apply the local variables list, and permanently mark these
    values (*) as safe (in the future, they will be set automatically.)

    indent-tabs-mode: t
    tab-width: 8
    fill-column: 79
  * c-font-lock-extra-types: ("FILE" "bool" "language" "linebuffer" "fdesc" "node" "regexp")
    c-file-style: "gnu"


If I answer `y', nothing happens. I have to press `y' a second time.




[-- Attachment #3: Type: message/rfc822, Size: 1154 bytes --]

From: Chong Yidong <cyd@stupidchicken.com>
To: 343-done@emacsbugs.donarmstrong.com, 347-done@emacsbugs.donarmstrong.com
Subject: Re: C mode asks twice about local variables
Date: Thu, 31 Jul 2008 10:14:42 -0400
Message-ID: <871w1aywbx.fsf@stupidchicken.com>

I believe this is now fixed in the trunk.  We'll leave it unfixed in the
branch, since the fix involves the hack-local-variables code and needs
to be tested thoroughly.

bug#347: marked as done (C mode asks twice about local variables)

[Emacs bug Tracking System]

[-- Attachment #1: Type: text/plain, Size: 843 bytes --]


Your message dated Thu, 31 Jul 2008 10:14:42 -0400
with message-id <871w1aywbx.fsf@stupidchicken.com>
and subject line Re: C mode asks twice about local variables
has caused the Emacs bug report #343,
regarding C mode asks twice about local variables
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact don@donarmstrong.com
immediately.)


-- 
343: http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=343
Emacs Bug Tracking System
Contact don@donarmstrong.com with problems

[-- Attachment #2: Type: message/rfc822, Size: 4343 bytes --]

From: Alan Mackenzie <acm@muc.de>
To: Glenn Morris <rgm@gnu.org>
Cc: bug-gnu-emacs@gnu.org, bug-cc-mode@gnu.org
Subject: Re: C mode asks twice about local variables
Date: Sun, 1 Jun 2008 17:21:44 +0000
Message-ID: <20080601172143.GA5899@muc.de>

Hi, Glenn,

On Sat, May 31, 2008 at 06:51:35PM -0400, Glenn Morris wrote:

> This applies in Emacs 22.2 and CVS trunk.

> emacs -Q lib-src/etags.c

>     The local variables list in etags.c
>     contains values that may not be safe (*).

>     Do you want to apply it?  You can type
>     y  -- to apply the local variables list.
>     n  -- to ignore the local variables list.
>     !  -- to apply the local variables list, and permanently mark these
>     values (*) as safe (in the future, they will be set automatically.)

>     indent-tabs-mode: t
>     tab-width: 8
>     fill-column: 79
>   * c-font-lock-extra-types: ("FILE" "bool" "language" "linebuffer" "fdesc" "node" "regexp")
>     c-file-style: "gnu"


> If I answer `y', nothing happens. I have to press `y' a second time.

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.

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.

Simply marking all pertinent variables as "safe" is not the right fix,
because they aren't: some can contain arbitrary functions.

Emacs doesn't really have the requisite hooks here:
`before-hack-local-variables-hook' and `after-hack-local-variables-hook'
would let the problem be fixed (rather than kludged), but it kind of
feels like overkill.

If Emacs were not to rebind the do-you-want-to-apply-it variable inside
hack-local-variables, that would allow the kludge to work.

Suggestions and helpful comments would be appreciated here.

-- 
Alan Mackenzie (Nuremberg, Germany).




[-- Attachment #3: Type: message/rfc822, Size: 1154 bytes --]

From: Chong Yidong <cyd@stupidchicken.com>
To: 343-done@emacsbugs.donarmstrong.com, 347-done@emacsbugs.donarmstrong.com
Subject: Re: C mode asks twice about local variables
Date: Thu, 31 Jul 2008 10:14:42 -0400
Message-ID: <871w1aywbx.fsf@stupidchicken.com>

I believe this is now fixed in the trunk.  We'll leave it unfixed in the
branch, since the fix involves the hack-local-variables code and needs
to be tested thoroughly.