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

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.

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.