GNU bug report logs - #9820
24.0.90; Behaviour of add-file-local-variable

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 9820 in the body.
You can then email your comments to 9820 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jambunathan K <kjambunathan <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.0.90; Behaviour of add-file-local-variable
Date: Fri, 21 Oct 2011 10:41:30 +0530

[Message part 1 (text/plain, inline)]

24.0.90; Behaviour of add-file-local-variable

Transcripts of exchanges on emacs-devel.
http://lists.gnu.org/archive/html/emacs-devel/2011-10/msg00850.html
http://lists.gnu.org/archive/html/emacs-devel/2011-10/msg00867.html

Additional note: All other file-local related commands may have to be
audited for the requested behaviour.

[Message part 2 (text/plain, inline)]

From: Jambunathan K <kjambunathan <at> gmail.com>
Subject: Re: Behaviour of add-file-local-variable?
Newsgroups: gmane.emacs.devel
To: Juri Linkov <juri <at> jurta.org>
Cc: emacs-devel <at> gnu.org
Date: Thu, 20 Oct 2011 09:51:36 +0530 (1 day, 45 minutes, 50 seconds ago)
Message-ID: <81fwioqomn.fsf <at> gmail.com>
Mail-Followup-To: Juri Linkov <juri <at> jurta.org>, emacs-devel <at> gnu.org

Juri Linkov <juri <at> jurta.org> writes:

>> Should add-file-local-variable also set the variable immediately rather
>> than merely updating the file footer?

> While keeping text in the Local Variables section in sync with actual
> values looks like the right thing to do, it raises related questions
> that are more difficult to decide:

If a user adds a file local variable and intends that it take
immediately what will he do? I believe he is more likely to revert the
file. Does "act-as-though-the-file-were-reverted policy" still very
difficult to deal with while applying file local variables?

Even if you insist on retaining the existing behaviour, at the minimum I
need some warning that the local variable has not kicked in. This could
be handled by

1. Updating the docstring of the above commands
2. Prompting the user whether he wants to "apply" the changes (in much
   the same way that a user is warned of risky or non-safe variables)
3. Provide a prefix modifier to the commands

(1) is the easiest way to tackle it.

Anyways, I surprised by the current behaviour and it took sometime to
figure out what's happening.

> Should `M-x add-file-local-variable RET mode RET' also change current mode?
>
> Should `M-x add-file-local-variable RET coding RET' also change the
> current buffer coding?
>
> Should `M-x add-file-local-variable RET eval RET' evaluate the added expression?
>
> Should `delete-file-local-variable' return the previous buffer-local value
> or revert to the global value?
>
> Should `add-dir-local-variable' after modifying .dir-locals.el
> also update values in all visited file buffers in all subdirectories?

-- 

[Message part 3 (text/plain, inline)]

From: Nix <nix <at> esperi.org.uk>
Subject: Re: Behaviour of add-file-local-variable?
Newsgroups: gmane.emacs.devel
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Juri Linkov <juri <at> jurta.org>, emacs-devel <at> gnu.org
Date: Fri, 21 Oct 2011 01:01:10 +0100 (5 hours, 5 minutes, 8 seconds ago)
Message-ID: <87y5wfmcvt.fsf <at> spindle.srvr.nix>

On 20 Oct 2011, Stefan Monnier stated:
> I think that add-file-local-variable could simply check if the
> variable's value is already the new one and if not output a message
> along the lines "Done; revisit the file to make it take effect" (tho
> "take effect" doesn't sound right: please native speakers fix it for
> me).

Your instincts mislead you: it's right. Perhaps a more explicit "Revisit
file to make this change take effect" would be better? (But that's
style, not grammar.)

-- 
NULL && (void)

[Message part 4 (text/plain, inline)]

In GNU Emacs 24.0.90.1 (i386-mingw-nt5.1.2600)
 of 2011-10-19 on MARVIN
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (4.6) --no-opt --cflags -I"C:/Program Files (x86)/GnuWin32/include" -ID:/devel/emacs/libXpm-3.5.8/include -ID:/devel/emacs/libXpm-3.5.8/src -ID:/devel/emacs/gnutls-2.10.5-x86/include --ldflags -LD:/devel/emacs/gnutls-2.10.5-x86/lib'

Message #8 received at 9820 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Jambunathan K <kjambunathan <at> gmail.com>
Cc: 9820 <at> debbugs.gnu.org
Subject: Re: bug#9820: 24.0.90; Behaviour of add-file-local-variable
Date: Fri, 21 Oct 2011 17:06:10 +0300

> Transcripts of exchanges on emacs-devel.
> http://lists.gnu.org/archive/html/emacs-devel/2011-10/msg00850.html
> http://lists.gnu.org/archive/html/emacs-devel/2011-10/msg00867.html

Thanks for filing the bug report based on that discussion.
As suggested, the following patch implements displaying a message.

> Additional note: All other file-local related commands may have to be
> audited for the requested behaviour.

It displays a message only in `add-file-local-variable' and
`add-file-local-variable-prop-line'.  I have no opinion
what other commands might do.

=== modified file 'lisp/files-x.el'
--- lisp/files-x.el	2011-04-19 13:44:55 +0000
+++ lisp/files-x.el	2011-10-21 14:04:22 +0000
@@ -214,7 +214,11 @@ (defun add-file-local-variable (variable
   (interactive
    (let ((variable (read-file-local-variable "Add file-local variable")))
      (list variable (read-file-local-variable-value variable))))
-  (modify-file-local-variable variable value 'add-or-replace))
+  (modify-file-local-variable variable value 'add-or-replace)
+  (when (and (called-interactively-p 'interactive)
+	     (symbolp variable) (boundp variable)
+	     (not (equal (symbol-value variable) value)))
+    (message "Revisit file to make this change take effect")))
 
 ;;;###autoload
 (defun delete-file-local-variable (variable)
@@ -335,7 +339,11 @@ (defun add-file-local-variable-prop-line
   (interactive
    (let ((variable (read-file-local-variable "Add -*- file-local variable")))
      (list variable (read-file-local-variable-value variable))))
-  (modify-file-local-variable-prop-line variable value 'add-or-replace))
+  (modify-file-local-variable-prop-line variable value 'add-or-replace)
+  (when (and (called-interactively-p 'interactive)
+	     (symbolp variable) (boundp variable)
+	     (not (equal (symbol-value variable) value)))
+    (message "Revisit file to make this change take effect")))
 
 ;;;###autoload
 (defun delete-file-local-variable-prop-line (variable)

Message #11 received at 9820 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juri Linkov <juri <at> jurta.org>
Cc: Jambunathan K <kjambunathan <at> gmail.com>, 9820 <at> debbugs.gnu.org
Subject: Re: bug#9820: 24.0.90; Behaviour of add-file-local-variable
Date: Fri, 21 Oct 2011 13:39:51 -0400

> === modified file 'lisp/files-x.el'
> --- lisp/files-x.el	2011-04-19 13:44:55 +0000
> +++ lisp/files-x.el	2011-10-21 14:04:22 +0000
> @@ -214,7 +214,11 @@ (defun add-file-local-variable (variable
>    (interactive
>     (let ((variable (read-file-local-variable "Add file-local variable")))
>       (list variable (read-file-local-variable-value variable))))
> -  (modify-file-local-variable variable value 'add-or-replace))
> +  (modify-file-local-variable variable value 'add-or-replace)
> +  (when (and (called-interactively-p 'interactive)
> +	     (symbolp variable) (boundp variable)
> +	     (not (equal (symbol-value variable) value)))
> +    (message "Revisit file to make this change take effect")))

Please pass use an optional `interactive' arg instead
of called-interactively-p.
You might even want to pass that arg down to modify-file-local-variable
so as to avoid duplicating the test and message.


        Stefan

Message #14 received at 9820 <at> debbugs.gnu.org (full text, mbox):

From: Jambunathan K <kjambunathan <at> gmail.com>
To: Juri Linkov <juri <at> jurta.org>
Cc: 9820 <at> debbugs.gnu.org
Subject: Re: bug#9820: 24.0.90; Behaviour of add-file-local-variable
Date: Sat, 22 Oct 2011 10:27:53 +0530

>> Additional note: All other file-local related commands may have to be
>> audited for the requested behaviour.
>
> It displays a message only in `add-file-local-variable' and
> `add-file-local-variable-prop-line'.  I have no opinion
> what other commands might do.

Would you like to give me a new patch with the warning message pushed to
modify-* routines (as Stefan suggested).

This way a (message ...) can be issued as soon as any modifications -
add-or-replace or delete - happen on the buffer.  I think the extra
intelligence of checking for new value against the old value could be
dropped altogether. The message is merely meant as a warning, so being
paranoid and issuing a warning ALWAYS will also serve the purpose well.

> === modified file 'lisp/files-x.el'
> --- lisp/files-x.el	2011-04-19 13:44:55 +0000
> +++ lisp/files-x.el	2011-10-21 14:04:22 +0000
> @@ -214,7 +214,11 @@ (defun add-file-local-variable (variable
>    (interactive
>     (let ((variable (read-file-local-variable "Add file-local variable")))
>       (list variable (read-file-local-variable-value variable))))
> -  (modify-file-local-variable variable value 'add-or-replace))
> +  (modify-file-local-variable variable value 'add-or-replace)
> +  (when (and (called-interactively-p 'interactive)
> +	     (symbolp variable) (boundp variable)
> +	     (not (equal (symbol-value variable) value)))
> +    (message "Revisit file to make this change take effect")))
>  
>  ;;;###autoload
>  (defun delete-file-local-variable (variable)
> @@ -335,7 +339,11 @@ (defun add-file-local-variable-prop-line
>    (interactive
>     (let ((variable (read-file-local-variable "Add -*- file-local variable")))
>       (list variable (read-file-local-variable-value variable))))
> -  (modify-file-local-variable-prop-line variable value 'add-or-replace))
> +  (modify-file-local-variable-prop-line variable value 'add-or-replace)
> +  (when (and (called-interactively-p 'interactive)
> +	     (symbolp variable) (boundp variable)
> +	     (not (equal (symbol-value variable) value)))
> +    (message "Revisit file to make this change take effect")))
>  
>  ;;;###autoload
>  (defun delete-file-local-variable-prop-line (variable)

Message #17 received at 9820 <at> debbugs.gnu.org (full text, mbox):

From: Jambunathan K <kjambunathan <at> gmail.com>
To: Juri Linkov <juri <at> jurta.org>
Cc: 9820 <at> debbugs.gnu.org
Subject: Re: bug#9820: 24.0.90; Behaviour of add-file-local-variable
Date: Sat, 22 Oct 2011 11:18:54 +0530

> The message is merely meant as a warning, so being paranoid and
> issuing a warning ALWAYS will also serve the purpose well.

How about - 

"Local Variables section updated.  To update actual variable values,
revisit the file"

The message is bit longer. However, it captures the spirit of these
commands.

Jambunathan K.

Message #20 received at 9820 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Jambunathan K <kjambunathan <at> gmail.com>
Cc: 9820 <at> debbugs.gnu.org
Subject: Re: bug#9820: 24.0.90; Behaviour of add-file-local-variable
Date: Sat, 22 Oct 2011 18:35:47 +0300

> How about -
> "Local Variables section updated.

That's exactly what these commands purposely do,
and users see as a result, so there is no need
to reiterate the obvious fact in the message.

> To update actual variable values, revisit the file"

If it's more clear, we could change:

  "Revisit file to make this change take effect"

to

  "Revisit file to update actual variable values"

> This way a (message ...) can be issued as soon as any modifications -
> add-or-replace or delete - happen on the buffer.  I think the extra
> intelligence of checking for new value against the old value could be
> dropped altogether. The message is merely meant as a warning, so being
> paranoid and issuing a warning ALWAYS will also serve the purpose well.

After moving this message to modify-file-local-variable,
checking for new value against the old value really makes less sense
because it's unclear what to do for the `delete' operations,
so I removed this check below.

> Would you like to give me a new patch with the warning message pushed to
> modify-* routines (as Stefan suggested).

Please try a new patch:

=== modified file 'lisp/files-x.el'
--- lisp/files-x.el	2011-04-19 13:44:55 +0000
+++ lisp/files-x.el	2011-10-22 15:34:12 +0000
@@ -115,7 +115,7 @@ (defun read-file-local-variable-mode ()
      ((and (stringp mode) (fboundp (intern mode))) (intern mode))
      (t mode))))
 
-(defun modify-file-local-variable (variable value op)
+(defun modify-file-local-variable (variable value op &optional interactive)
   "Modify file-local VARIABLE in Local Variables depending on operation OP.
 
 If OP is `add-or-replace' then delete all existing settings of
@@ -198,10 +198,13 @@ (defun modify-file-local-variable (varia
 	   ((eq variable 'mode) (goto-char beg))
 	   ((null replaced-pos) (goto-char end))
 	   (replaced-pos (goto-char replaced-pos)))
-	  (insert (format "%s%S: %S%s\n" prefix variable value suffix)))))))
+	  (insert (format "%s%S: %S%s\n" prefix variable value suffix)))
+
+	(when interactive
+	  (message "Revisit file to make this change take effect"))))))
 
 ;;;###autoload
-(defun add-file-local-variable (variable value)
+(defun add-file-local-variable (variable value &optional interactive)
   "Add file-local VARIABLE with its VALUE to the Local Variables list.
 
 This command deletes all existing settings of VARIABLE (except `mode'
@@ -213,17 +216,17 @@ (defun add-file-local-variable (variable
 `Local Variables:' and the last line containing the string `End:'."
   (interactive
    (let ((variable (read-file-local-variable "Add file-local variable")))
-     (list variable (read-file-local-variable-value variable))))
-  (modify-file-local-variable variable value 'add-or-replace))
+     (list variable (read-file-local-variable-value variable) t)))
+  (modify-file-local-variable variable value 'add-or-replace interactive))
 
 ;;;###autoload
-(defun delete-file-local-variable (variable)
+(defun delete-file-local-variable (variable &optional interactive)
   "Delete all settings of file-local VARIABLE from the Local Variables list."
   (interactive
-   (list (read-file-local-variable "Delete file-local variable")))
-  (modify-file-local-variable variable nil 'delete))
+   (list (read-file-local-variable "Delete file-local variable") t))
+  (modify-file-local-variable variable nil 'delete interactive))
 
-(defun modify-file-local-variable-prop-line (variable value op)
+(defun modify-file-local-variable-prop-line (variable value op &optional interactive)
   "Modify file-local VARIABLE in the -*- line depending on operation OP.
 
 If OP is `add-or-replace' then delete all existing settings of
@@ -320,10 +323,13 @@ (defun modify-file-local-variable-prop-l
 	      (insert ";"))
 	  (unless (eq (char-before) ?\s) (insert " "))
 	  (insert (format "%S: %S;" variable value))
-	  (unless (eq (char-after) ?\s) (insert " "))))))))
+	  (unless (eq (char-after) ?\s) (insert " "))))))
+
+    	(when interactive
+	  (message "Revisit file to make this change take effect"))))
 
 ;;;###autoload
-(defun add-file-local-variable-prop-line (variable value)
+(defun add-file-local-variable-prop-line (variable value &optional interactive)
   "Add file-local VARIABLE with its VALUE to the -*- line.
 
 This command deletes all existing settings of VARIABLE (except `mode'
@@ -334,15 +340,15 @@ (defun add-file-local-variable-prop-line
 then this function adds it."
   (interactive
    (let ((variable (read-file-local-variable "Add -*- file-local variable")))
-     (list variable (read-file-local-variable-value variable))))
-  (modify-file-local-variable-prop-line variable value 'add-or-replace))
+     (list variable (read-file-local-variable-value variable) t)))
+  (modify-file-local-variable-prop-line variable value 'add-or-replace interactive))
 
 ;;;###autoload
-(defun delete-file-local-variable-prop-line (variable)
+(defun delete-file-local-variable-prop-line (variable &optional interactive)
   "Delete all settings of file-local VARIABLE from the -*- line."
   (interactive
-   (list (read-file-local-variable "Delete -*- file-local variable")))
-  (modify-file-local-variable-prop-line variable nil 'delete))
+   (list (read-file-local-variable "Delete -*- file-local variable") t))
+  (modify-file-local-variable-prop-line variable nil 'delete interactive))
 
 (defvar auto-insert) ; from autoinsert.el
 

Message #23 received at 9820 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juri Linkov <juri <at> jurta.org>
Cc: Jambunathan K <kjambunathan <at> gmail.com>, 9820 <at> debbugs.gnu.org
Subject: Re: bug#9820: 24.0.90; Behaviour of add-file-local-variable
Date: Sun, 23 Oct 2011 13:59:14 -0400

> After moving this message to modify-file-local-variable,
> checking for new value against the old value really makes less sense
> because it's unclear what to do for the `delete' operations,
> so I removed this check below.

I think it's important to silence the message when we can know that it's
not useful (i.e. the variable already has the right value).  For `delete',
we can test local-variable-p.
BTW, I think the message is not yet as good as it should be.  E.g. we
could either tell the exact command that needs to be used (e.g. M-x
revert-buffer), or go the other way and only point out the current
situation without indicating how to "fix" it, something like "the change
hasn't yet taken effect".


        Stefan

Message #26 received at 9820 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Jambunathan K <kjambunathan <at> gmail.com>, 9820 <at> debbugs.gnu.org
Subject: Re: bug#9820: 24.0.90; Behaviour of add-file-local-variable
Date: Sun, 16 Jun 2013 01:58:07 +0300

This is an old request that I'd like to close now, one way or another.

> BTW, I think the message is not yet as good as it should be.  E.g. we
> could either tell the exact command that needs to be used (e.g. M-x
> revert-buffer), or go the other way and only point out the current
> situation without indicating how to "fix" it, something like "the change
> hasn't yet taken effect".

In a similar situation, Dired displays this message:

  (message "%s" (substitute-command-keys
                 "Directory has changed on disk; type \\[revert-buffer] to update Dired"))

So for `add-file-local-variable' it could be:

  (message "%s" (substitute-command-keys
                 "For this change to take effect revisit file using \\[revert-buffer]"))

The following patch displays this message.

=== modified file 'lisp/files-x.el'
--- lisp/files-x.el	2013-06-15 22:44:38 +0000
+++ lisp/files-x.el	2013-06-15 22:57:03 +0000
@@ -115,7 +115,7 @@ (defun read-file-local-variable-mode ()
      ((and (stringp mode) (fboundp (intern mode))) (intern mode))
      (t mode))))
 
-(defun modify-file-local-variable (variable value op)
+(defun modify-file-local-variable (variable value op &optional interactive)
   "Modify file-local VARIABLE in Local Variables depending on operation OP.
 
 If OP is `add-or-replace' then delete all existing settings of
@@ -198,10 +198,17 @@ (defun modify-file-local-variable (varia
 	   ((eq variable 'mode) (goto-char beg))
 	   ((null replaced-pos) (goto-char end))
 	   (replaced-pos (goto-char replaced-pos)))
-	  (insert (format "%s%S: %S%s\n" prefix variable value suffix)))))))
+	  (insert (format "%s%S: %S%s\n" prefix variable value suffix)))
+
+	(when (and interactive
+		   (not (and (symbolp variable)
+			     (boundp variable)
+			     (equal (symbol-value variable) value))))
+	  (message "%s" (substitute-command-keys
+			 "For this change to take effect revisit file using \\[revert-buffer]"))))))))
 
 ;;;###autoload
-(defun add-file-local-variable (variable value)
+(defun add-file-local-variable (variable value &optional interactive)
   "Add file-local VARIABLE with its VALUE to the Local Variables list.
 
 This command deletes all existing settings of VARIABLE (except `mode'
@@ -213,17 +220,17 @@ (defun add-file-local-variable (variable
 `Local Variables:' and the last line containing the string `End:'."
   (interactive
    (let ((variable (read-file-local-variable "Add file-local variable")))
-     (list variable (read-file-local-variable-value variable))))
-  (modify-file-local-variable variable value 'add-or-replace))
+     (list variable (read-file-local-variable-value variable) t)))
+  (modify-file-local-variable variable value 'add-or-replace interactive))
 
 ;;;###autoload
-(defun delete-file-local-variable (variable)
+(defun delete-file-local-variable (variable &optional interactive)
   "Delete all settings of file-local VARIABLE from the Local Variables list."
   (interactive
-   (list (read-file-local-variable "Delete file-local variable")))
-  (modify-file-local-variable variable nil 'delete))
+   (list (read-file-local-variable "Delete file-local variable") t))
+  (modify-file-local-variable variable nil 'delete interactive))
 
-(defun modify-file-local-variable-prop-line (variable value op)
+(defun modify-file-local-variable-prop-line (variable value op &optional interactive)
   "Modify file-local VARIABLE in the -*- line depending on operation OP.
 
 If OP is `add-or-replace' then delete all existing settings of
@@ -341,10 +348,17 @@ (defun modify-file-local-variable-prop-l
 	      (insert ";"))
 	  (unless (eq (char-before) ?\s) (insert " "))
 	  (insert (format "%S: %S;" variable value))
-	  (unless (eq (char-after) ?\s) (insert " "))))))))
+	  (unless (eq (char-after) ?\s) (insert " "))))))
+
+    (when (and interactive
+	       (not (and (symbolp variable)
+			 (boundp variable)
+			 (equal (symbol-value variable) value))))
+      (message "%s" (substitute-command-keys
+		     "For this change to take effect revisit file using \\[revert-buffer]")))))
 
 ;;;###autoload
-(defun add-file-local-variable-prop-line (variable value)
+(defun add-file-local-variable-prop-line (variable value &optional interactive)
   "Add file-local VARIABLE with its VALUE to the -*- line.
 
 This command deletes all existing settings of VARIABLE (except `mode'
@@ -355,15 +369,15 @@ (defun add-file-local-variable-prop-line
 then this function adds it."
   (interactive
    (let ((variable (read-file-local-variable "Add -*- file-local variable")))
-     (list variable (read-file-local-variable-value variable))))
-  (modify-file-local-variable-prop-line variable value 'add-or-replace))
+     (list variable (read-file-local-variable-value variable) t)))
+  (modify-file-local-variable-prop-line variable value 'add-or-replace interactive))
 
 ;;;###autoload
-(defun delete-file-local-variable-prop-line (variable)
+(defun delete-file-local-variable-prop-line (variable &optional interactive)
   "Delete all settings of file-local VARIABLE from the -*- line."
   (interactive
-   (list (read-file-local-variable "Delete -*- file-local variable")))
-  (modify-file-local-variable-prop-line variable nil 'delete))
+   (list (read-file-local-variable "Delete -*- file-local variable") t))
+  (modify-file-local-variable-prop-line variable nil 'delete interactive))
 
 (defvar auto-insert) ; from autoinsert.el
 

Message #29 received at 9820 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juri Linkov <juri <at> jurta.org>
Cc: Jambunathan K <kjambunathan <at> gmail.com>, 9820 <at> debbugs.gnu.org
Subject: Re: bug#9820: 24.0.90; Behaviour of add-file-local-variable
Date: Sat, 15 Jun 2013 20:50:15 -0400

> -	  (insert (format "%s%S: %S%s\n" prefix variable value suffix)))))))
> +	  (insert (format "%s%S: %S%s\n" prefix variable value suffix)))
> +
> +	(when (and interactive
> +		   (not (and (symbolp variable)
> +			     (boundp variable)
> +			     (equal (symbol-value variable) value))))
> +	  (message "%s" (substitute-command-keys
> +			 "For this change to take effect revisit file using \\[revert-buffer]"))))))))

Looks good, thanks.

> -	  (unless (eq (char-after) ?\s) (insert " "))))))))
> +	  (unless (eq (char-after) ?\s) (insert " "))))))
> +
> +    (when (and interactive
> +	       (not (and (symbolp variable)
> +			 (boundp variable)
> +			 (equal (symbol-value variable) value))))
> +      (message "%s" (substitute-command-keys
> +		     "For this change to take effect revisit file using \\[revert-buffer]")))))

But please eliminate this code duplication.


        Stefan

Message #32 received at 9820 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Jambunathan K <kjambunathan <at> gmail.com>, 9820 <at> debbugs.gnu.org
Subject: Re: bug#9820: 24.0.90; Behaviour of add-file-local-variable
Date: Tue, 18 Jun 2013 01:35:50 +0300

> But please eliminate this code duplication.

Code duplication is eliminated by adding a new function
`modify-file-local-variable-message' that now also performs
more checks to decide when to display the message:

=== modified file 'lisp/files-x.el'
--- lisp/files-x.el	2013-06-15 22:44:38 +0000
+++ lisp/files-x.el	2013-06-17 22:34:33 +0000
@@ -115,7 +115,38 @@ (defun read-file-local-variable-mode ()
      ((and (stringp mode) (fboundp (intern mode))) (intern mode))
      (t mode))))
 
-(defun modify-file-local-variable (variable value op)
+(defun modify-file-local-variable-message (variable value op)
+  (let* ((not-value (make-symbol ""))
+	 (old-value (cond ((eq variable 'mode)
+			   major-mode)
+			  ((eq variable 'coding)
+			   buffer-file-coding-system)
+			  (t (if (and (symbolp variable)
+				      (boundp variable))
+				 (symbol-value variable)
+			       not-value))))
+	 (new-value (if (eq op 'delete)
+			(cond ((eq variable 'mode)
+			       (default-value 'major-mode))
+			      ((eq variable 'coding)
+			       (default-value 'buffer-file-coding-system))
+			      (t (if (and (symbolp variable)
+					  (default-boundp variable))
+				     (default-value variable)
+				   not-value)))
+		      (cond ((eq variable 'mode)
+			     (let ((string (format "%S" value)))
+			       (if (string-match-p "-mode\\'" string)
+				   value
+				 (intern (concat string "-mode")))))
+			    (t value)))))
+    (when (or (eq old-value not-value)
+	      (eq new-value not-value)
+	      (not (equal old-value new-value)))
+      (message "%s" (substitute-command-keys
+		     "For this change to take effect revisit file using \\[revert-buffer]")))))
+
+(defun modify-file-local-variable (variable value op &optional interactive)
   "Modify file-local VARIABLE in Local Variables depending on operation OP.
 
 If OP is `add-or-replace' then delete all existing settings of
@@ -198,10 +229,13 @@ (defun modify-file-local-variable (varia
 	   ((eq variable 'mode) (goto-char beg))
 	   ((null replaced-pos) (goto-char end))
 	   (replaced-pos (goto-char replaced-pos)))
-	  (insert (format "%s%S: %S%s\n" prefix variable value suffix)))))))
+	  (insert (format "%s%S: %S%s\n" prefix variable value suffix))))
+
+      (when interactive
+	(modify-file-local-variable-message variable value op)))))
 
 ;;;###autoload
-(defun add-file-local-variable (variable value)
+(defun add-file-local-variable (variable value &optional interactive)
   "Add file-local VARIABLE with its VALUE to the Local Variables list.
 
 This command deletes all existing settings of VARIABLE (except `mode'
@@ -213,17 +247,17 @@ (defun add-file-local-variable (variable
 `Local Variables:' and the last line containing the string `End:'."
   (interactive
    (let ((variable (read-file-local-variable "Add file-local variable")))
-     (list variable (read-file-local-variable-value variable))))
-  (modify-file-local-variable variable value 'add-or-replace))
+     (list variable (read-file-local-variable-value variable) t)))
+  (modify-file-local-variable variable value 'add-or-replace interactive))
 
 ;;;###autoload
-(defun delete-file-local-variable (variable)
+(defun delete-file-local-variable (variable &optional interactive)
   "Delete all settings of file-local VARIABLE from the Local Variables list."
   (interactive
-   (list (read-file-local-variable "Delete file-local variable")))
-  (modify-file-local-variable variable nil 'delete))
+   (list (read-file-local-variable "Delete file-local variable") t))
+  (modify-file-local-variable variable nil 'delete interactive))
 
-(defun modify-file-local-variable-prop-line (variable value op)
+(defun modify-file-local-variable-prop-line (variable value op &optional interactive)
   "Modify file-local VARIABLE in the -*- line depending on operation OP.
 
 If OP is `add-or-replace' then delete all existing settings of
@@ -337,10 +371,13 @@ (defun modify-file-local-variable-prop-l
 	      (insert ";"))
 	  (unless (eq (char-before) ?\s) (insert " "))
 	  (insert (format "%S: %S;" variable value))
-	  (unless (eq (char-after) ?\s) (insert " "))))))))
+	  (unless (eq (char-after) ?\s) (insert " ")))))
+
+      (when interactive
+	(modify-file-local-variable-message variable value op)))))
 
 ;;;###autoload
-(defun add-file-local-variable-prop-line (variable value)
+(defun add-file-local-variable-prop-line (variable value &optional interactive)
   "Add file-local VARIABLE with its VALUE to the -*- line.
 
 This command deletes all existing settings of VARIABLE (except `mode'
@@ -355,15 +394,15 @@ (defun add-file-local-variable-prop-line
 then this function adds it."
   (interactive
    (let ((variable (read-file-local-variable "Add -*- file-local variable")))
-     (list variable (read-file-local-variable-value variable))))
-  (modify-file-local-variable-prop-line variable value 'add-or-replace))
+     (list variable (read-file-local-variable-value variable) t)))
+  (modify-file-local-variable-prop-line variable value 'add-or-replace interactive))
 
 ;;;###autoload
-(defun delete-file-local-variable-prop-line (variable)
+(defun delete-file-local-variable-prop-line (variable &optional interactive)
   "Delete all settings of file-local VARIABLE from the -*- line."
   (interactive
-   (list (read-file-local-variable "Delete -*- file-local variable")))
-  (modify-file-local-variable-prop-line variable nil 'delete))
+   (list (read-file-local-variable "Delete -*- file-local variable") t))
+  (modify-file-local-variable-prop-line variable nil 'delete interactive))
 
 (defvar auto-insert) ; from autoinsert.el
 

Message #35 received at 9820 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juri Linkov <juri <at> jurta.org>
Cc: Jambunathan K <kjambunathan <at> gmail.com>, 9820 <at> debbugs.gnu.org
Subject: Re: bug#9820: 24.0.90; Behaviour of add-file-local-variable
Date: Mon, 17 Jun 2013 20:53:31 -0400

>> But please eliminate this code duplication.
> Code duplication is eliminated by adding a new function
> `modify-file-local-variable-message' that now also performs
> more checks to decide when to display the message:

Great, thanks,


        Stefan

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.