RFC: add .dir-locals.el

RFC: add .dir-locals.el

[Tom Tromey]

I'd appreciate comments on this patch.

This adds a .dir-locals.el to gdb.  This will cause Emacs to
automatically pick up the correct settings for various variables, so
that users won't have to configure Emacs for editing gdb.

These settings won't affect any files outside of gdb.

In the absence of comments I plan to check it in.

Tom

ChangeLog:
2012-03-26  Tom Tromey  <tromey@redhat.com>

	* .dir-locals.el: New file.

Index: .dir-locals.el
===================================================================
RCS file: .dir-locals.el
diff -N .dir-locals.el
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ .dir-locals.el	26 Mar 2012 18:50:07 -0000
@@ -0,0 +1,8 @@
+(
+ (tcl-mode . ((tcl-indent-level . 4)
+	      (tcl-continued-indent-level . 4)
+	      (indent-tabs-mode . t)))
+ (nil . ((bug-reference-url-format . "http://sourceware.org/bugzilla/show_bug.cgi?id=%s")))
+ (c-mode . ((c-file-style . "GNU")
+	    (indent-tabs-mode . t)))
+)

Re: RFC: add .dir-locals.el

[Joel Brobecker]

> ChangeLog:
> 2012-03-26  Tom Tromey  <tromey@redhat.com>
> 
> 	* .dir-locals.el: New file.

You don't say which directory this is going to be in, and I couldn't
determine that from the patch itself, but I don't see how this could
be a problem, even for those who are using Emacs and have added some
local settings for debugging GDB (they should have selected the same
ones).

I think it should have a copyright header, though....

-- 
Joel

Re: RFC: add .dir-locals.el

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> You don't say which directory this is going to be in, and I couldn't
Joel> determine that from the patch itself,

Oops, sorry.  The new file would be src/gdb/.dir-locals.el.

Joel> I think it should have a copyright header, though....

Will do.

Tom

Re: RFC: add .dir-locals.el

[Tom Tromey]

>>>>> "Joel" == Tom Tromey <tromey@redhat.com> writes:

Joel> I think it should have a copyright header, though....

Tom> Will do.

Here is what I am checking in.

Tom

2012-03-28  Tom Tromey  <tromey@redhat.com>

	* .dir-locals.el: New file.

Index: .dir-locals.el
===================================================================
RCS file: .dir-locals.el
diff -N .dir-locals.el
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ .dir-locals.el	28 Mar 2012 17:35:04 -0000
@@ -0,0 +1,24 @@
+;; Emacs settings.
+;; Copyright (C) 2012 Free Software Foundation, Inc.
+
+;; This program is free software; you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation; either version 3 of the License, or
+;; (at your option) any later version.
+
+;; This program is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+(
+ (tcl-mode . ((tcl-indent-level . 4)
+	      (tcl-continued-indent-level . 4)
+	      (indent-tabs-mode . t)))
+ (nil . ((bug-reference-url-format . "http://sourceware.org/bugzilla/show_bug.cgi?id=%s")))
+ (c-mode . ((c-file-style . "GNU")
+	    (indent-tabs-mode . t)))
+)

Re: RFC: add .dir-locals.el

[Yao Qi]

On 03/27/2012 02:52 AM, Tom Tromey wrote:
> This adds a .dir-locals.el to gdb.  This will cause Emacs to
> automatically pick up the correct settings for various variables, so
> that users won't have to configure Emacs for editing gdb.
> 
> These settings won't affect any files outside of gdb.

Do you think it is useful to add some code here to check code style or
check ARI rules?

The patch below shows two ARI checks we can do in emacs.  The benefit
is that code is checked automatically during writing, instead of being
pointed out by reviewers.

With this change, when emacs opens gdb files for the first time, it
prompts to ask apply risky local variables.  Not sure GDB contributors
using emacs are comfortable with this or not.

I have other emacs lisp snippets to do other checks, and personally, I
find they are quite useful.  If this is acceptable, I'll push them out
one by one.

-- 
Yao (齐尧) 

	* gdb/.dir-locals.el: Check improper functions and include
	files.

diff --git a/gdb/.dir-locals.el b/gdb/.dir-locals.el
index 8eca9e0..c41bb1a 100644
--- a/gdb/.dir-locals.el
+++ b/gdb/.dir-locals.el
@@ -20,5 +20,16 @@
              (indent-tabs-mode . t)))
  (nil . ((bug-reference-url-format . "http://sourceware.org/bugzilla/show_bug.cgi?id=%s")))
  (c-mode . ((c-file-style . "GNU")
-           (indent-tabs-mode . t)))
+           (indent-tabs-mode . t)
+           (eval . (progn
+                     ;; Don't use these functions.  Their alternatives are better.
+                     ;; This list of functions are from ARI rules.
+                     (font-lock-add-keywords
+                      nil
+                      '(("\\<\\(\\(xasprintf\\|abort\\|vasprintf\\|strerror\\|strdup\\|asprintf\\|sprintf\\)[ ]*\(\\)" 1 font-lock-warning-face t)))
+                     ;; Don't include these files.
+                     (font-lock-add-keywords
+                      nil
+                      '(("\\<include[ ]*\\(<\\(stat\\|dirent\\|wait\\|sys/wait\\|assert\\)\\.h>\\)" 1 font-lock-warning-face t)))
+                     ))))
 )

Re: RFC: add .dir-locals.el

[Joel Brobecker]

> Do you think it is useful to add some code here to check code style or
> check ARI rules?

Perhaps I should think about doing the same for vim...

-- 
Joel

Re: RFC: add .dir-locals.el

[Tom Tromey]

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> Do you think it is useful to add some code here to check code style or
Yao> check ARI rules?

Let's try it.
If people don't like it, we can always move the code out of
.dir-locals.el and into some new .el that developers can optionally load
somehow.

Yao> With this change, when emacs opens gdb files for the first time, it
Yao> prompts to ask apply risky local variables.  Not sure GDB contributors
Yao> using emacs are comfortable with this or not.

I don't think it is a big deal.

Yao> I have other emacs lisp snippets to do other checks, and personally, I
Yao> find they are quite useful.  If this is acceptable, I'll push them out
Yao> one by one.

Sounds fantastic.

Tom

Re: RFC: add .dir-locals.el

[Tom Tromey]

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> 	* gdb/.dir-locals.el: Check improper functions and include
Yao> 	files.

I wanted to explicitly mention -- ok.
Please check it in.

Tom

Re: RFC: add .dir-locals.el

[Eli Zaretskii]

> Date: Thu, 12 Apr 2012 07:32:56 +0800
> From: Yao Qi <yao@codesourcery.com>
> CC: <gdb-patches@sourceware.org>
> 
> With this change, when emacs opens gdb files for the first time, it
> prompts to ask apply risky local variables.  Not sure GDB contributors
> using emacs are comfortable with this or not.

Can't you make them not risky conditioned on specific types of values,
like strings etc.?

Re: RFC: add .dir-locals.el

[Tom Tromey]

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Thu, 12 Apr 2012 07:32:56 +0800
>> From: Yao Qi <yao@codesourcery.com>
>> CC: <gdb-patches@sourceware.org>
>> 
>> With this change, when emacs opens gdb files for the first time, it
>> prompts to ask apply risky local variables.  Not sure GDB contributors
>> using emacs are comfortable with this or not.

Eli> Can't you make them not risky conditioned on specific types of values,
Eli> like strings etc.?

I don't see how that applies here.

You can set a property on a symbol to make it safe in some situations.
This would normally be done via a patch to Emacs, but one can also do it
temporarily in .emacs or whatever.

In this case though, what symbol would you use?
AFAIK you have to call font-lock-add-keywords to change font-locking.
There is no local variable you can set.

Tom

Re: RFC: add .dir-locals.el

[Eli Zaretskii]

> From: Tom Tromey <tromey@redhat.com>
> Cc: Yao Qi <yao@codesourcery.com>, gdb-patches@sourceware.org
> Date: Thu, 12 Apr 2012 14:13:43 -0600
> 
> >>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Date: Thu, 12 Apr 2012 07:32:56 +0800
> >> From: Yao Qi <yao@codesourcery.com>
> >> CC: <gdb-patches@sourceware.org>
> >> 
> >> With this change, when emacs opens gdb files for the first time, it
> >> prompts to ask apply risky local variables.  Not sure GDB contributors
> >> using emacs are comfortable with this or not.
> 
> Eli> Can't you make them not risky conditioned on specific types of values,
> Eli> like strings etc.?
> 
> I don't see how that applies here.
> 
> You can set a property on a symbol to make it safe in some situations.
> This would normally be done via a patch to Emacs, but one can also do it
> temporarily in .emacs or whatever.
> 
> In this case though, what symbol would you use?
> AFAIK you have to call font-lock-add-keywords to change font-locking.
> There is no local variable you can set.

What variables cause the warning?

Re: RFC: add .dir-locals.el

[Tom Tromey]

Eli> What variables cause the warning?

The patch adds an 'eval' clause to .dir-locals.el.

Tom

Re: RFC: add .dir-locals.el

[Eli Zaretskii]

> From: Tom Tromey <tromey@redhat.com>
> Cc: yao@codesourcery.com, gdb-patches@sourceware.org
> Date: Fri, 13 Apr 2012 07:51:31 -0600
> 
> Eli> What variables cause the warning?
> 
> The patch adds an 'eval' clause to .dir-locals.el.

'eval' is a hard case, and using 'progn' after it is even harder.  I
couldn't find any way to disable the warning, except set
enable-local-eval to t, which security-minded people might be
unwilling to use.

I would suggest to ask on emacs-devel, maybe there's some trick that
some wizard would come up with.

Re: RFC: add .dir-locals.el

[Eli Zaretskii]

> From: Tom Tromey <tromey@redhat.com>
> Cc: Yao Qi <yao@codesourcery.com>, gdb-patches@sourceware.org
> Date: Thu, 12 Apr 2012 14:13:43 -0600
> 
> You can set a property on a symbol to make it safe in some situations.
> This would normally be done via a patch to Emacs, but one can also do it
> temporarily in .emacs or whatever.
> 
> In this case though, what symbol would you use?
> AFAIK you have to call font-lock-add-keywords to change font-locking.
> There is no local variable you can set.

As you discovered, there's safe-local-eval-forms, although it was only
introduced in Emacs 24.

Re: RFC: add .dir-locals.el

[Yao Qi]

On 04/13/2012 03:06 AM, Eli Zaretskii wrote:
> Can't you make them not risky conditioned on specific types of values,
> like strings etc.?

Could you elaborate a little please?  I find that emacs prompts to ask
apply risky local variables when opening *every* file in gdb, which is
boring to me.  I'll think of how to "make them not risky", but I have no
clue so far.

-- 
Yao (齐尧)

Re: RFC: add .dir-locals.el

[Eli Zaretskii]

> Date: Fri, 13 Apr 2012 17:10:50 +0800
> From: Yao Qi <yao@codesourcery.com>
> CC: <tromey@redhat.com>, <gdb-patches@sourceware.org>
> 
> On 04/13/2012 03:06 AM, Eli Zaretskii wrote:
> > Can't you make them not risky conditioned on specific types of values,
> > like strings etc.?
> 
> Could you elaborate a little please?  I find that emacs prompts to ask
> apply risky local variables when opening *every* file in gdb, which is
> boring to me.  I'll think of how to "make them not risky", but I have no
> clue so far.

Can you show the actual warnings that state the names of the variables
and the values they are being set to?  (Sorry, I cannot easily check
this myself where I'm typing this.)

Re: RFC: add .dir-locals.el

[Yao Qi]

On 04/13/2012 08:32 PM, Eli Zaretskii wrote:
> Can you show the actual warnings that state the names of the variables
> and the values they are being set to?  (Sorry, I cannot easily check
> this myself where I'm typing this.)

"The local variables list in /home/yao/SourceCode/gnu/gdb/gdb
contains variables that are risk (**).

Do you want to apply it?  You can type
y -- to apply the local variables list.
n -- to ignore the local variables list.

   (bug-reference-url-format .
"http://sourceware.org/bugzilla/show_bug.cgi?id=%s")
   (c-file-style . "GNU")
   (indent-tabs-mode . t)
** (eval : (progn (font-lock-add-keywords nil
'(("\\<\\(\\(xasprintf\\|abort\\|vasprintf\\|strerror\\|strdup\\|asprintf\\|sprintf\\)[
]*\(\\)" 1 font-lock-warning-face t))) (font-lock-add-keywords nil
'(("\\<include[
]*\\(<\\(stat\\|dirent\\|wait\\|sys/wait\\|assert\\)\\.h>\\)" 1
font-lock-warning-face t)))))"

-- 
Yao (齐尧)

Re: RFC: add .dir-locals.el

[Pedro Alves]

On 04/13/2012 02:50 PM, Yao Qi wrote:

> On 04/13/2012 08:32 PM, Eli Zaretskii wrote:
>> > Can you show the actual warnings that state the names of the variables
>> > and the values they are being set to?  (Sorry, I cannot easily check
>> > this myself where I'm typing this.)
> "The local variables list in /home/yao/SourceCode/gnu/gdb/gdb
> contains variables that are risk (**).
> 
> Do you want to apply it?  You can type
> y -- to apply the local variables list.
> n -- to ignore the local variables list.


I just tried this, and found it very annoying.  :-(  Any way to turn it off?
I'd rather be alerted by the ARI by email once in a blue moon, than having to
say "yes" to emacs 200 times per day.  :-/

-- 
Pedro Alves

Re: RFC: add .dir-locals.el

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> I just tried this, and found it very annoying.  :-( Any way to
Pedro> turn it off?  I'd rather be alerted by the ARI by email once in a
Pedro> blue moon, than having to say "yes" to emacs 200 times per day.
Pedro> :-/

You could add the forms to safe-local-eval-forms.

Alternatively we could just not put this patch in.

Tom

Re: RFC: add .dir-locals.el

[Eli Zaretskii]

> Date: Fri, 13 Apr 2012 15:04:17 +0100
> From: Pedro Alves <palves@redhat.com>
> CC: Eli Zaretskii <eliz@gnu.org>, tromey@redhat.com,        gdb-patches@sourceware.org
> 
> I just tried this, and found it very annoying.  :-(  Any way to turn it off?

Yes, set enable-local-eval to t in your .emacs (untested).

But I don't like to ask all users do this, I'd like to find a better
solution.