From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.bugs Subject: bug#12259: Add delete-trailing-whitespace to list of safe eval forms Date: Thu, 23 Aug 2012 08:19:41 -0400 Message-ID: References: <87r4r1e7i1.fsf@santiago.tweag.net> <87r4qyao3g.fsf@santiago.tweag.net> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: ger.gmane.org 1345724443 20908 80.91.229.3 (23 Aug 2012 12:20:43 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Thu, 23 Aug 2012 12:20:43 +0000 (UTC) Cc: 12259@debbugs.gnu.org To: Mathieu Boespflug Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Thu Aug 23 14:20:43 2012 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1T4WOv-0002QM-EY for geb-bug-gnu-emacs@m.gmane.org; Thu, 23 Aug 2012 14:20:41 +0200 Original-Received: from localhost ([::1]:53567 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T4WOt-0000nv-O9 for geb-bug-gnu-emacs@m.gmane.org; Thu, 23 Aug 2012 08:20:39 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:46808) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T4WOn-0000nD-JD for bug-gnu-emacs@gnu.org; Thu, 23 Aug 2012 08:20:37 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T4WOm-0005Ik-65 for bug-gnu-emacs@gnu.org; Thu, 23 Aug 2012 08:20:33 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:35180) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T4WOm-0005If-2q for bug-gnu-emacs@gnu.org; Thu, 23 Aug 2012 08:20:32 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.72) (envelope-from ) id 1T4WPF-0008DB-TA for bug-gnu-emacs@gnu.org; Thu, 23 Aug 2012 08:21:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Stefan Monnier Original-Sender: debbugs-submit-bounces@debbugs.gnu.org Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 23 Aug 2012 12:21:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 12259 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 12259-submit@debbugs.gnu.org id=B12259.134572441531509 (code B ref 12259); Thu, 23 Aug 2012 12:21:01 +0000 Original-Received: (at 12259) by debbugs.gnu.org; 23 Aug 2012 12:20:15 +0000 Original-Received: from localhost ([127.0.0.1]:44726 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1T4WOU-0008C8-PL for submit@debbugs.gnu.org; Thu, 23 Aug 2012 08:20:15 -0400 Original-Received: from ironport2-out.teksavvy.com ([206.248.154.182]:45969) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1T4WOT-0008C2-0H for 12259@debbugs.gnu.org; Thu, 23 Aug 2012 08:20:13 -0400 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: Ai0FAG6Zu09FpZ+X/2dsb2JhbABEsEiDSYEIghUBAQQBViMFCwsOJhIUGA0kLoduBboJixsHhSIDozOBWIMFgToJ X-IronPort-AV: E=Sophos;i="4.75,637,1330923600"; d="scan'208";a="196507532" Original-Received: from 69-165-159-151.dsl.teksavvy.com (HELO pastel.home) ([69.165.159.151]) by ironport2-out.teksavvy.com with ESMTP/TLS/ADH-AES256-SHA; 23 Aug 2012 08:19:42 -0400 Original-Received: by pastel.home (Postfix, from userid 20848) id CD76058ADD; Thu, 23 Aug 2012 08:19:41 -0400 (EDT) In-Reply-To: <87r4qyao3g.fsf@santiago.tweag.net> (Mathieu Boespflug's message of "Wed, 22 Aug 2012 12:27:47 -0400") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.2.50 (gnu/linux) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.13 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) X-Received-From: 140.186.70.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:63424 Archived-At: >>> I have attached a patch at the end of this email that considers eval >>> forms that add 'delete-trailing-whitespace to various hooks safe by >>> default. >> Actually, I wonder whether we want to accept/encourage those uses >> instead of (add-hook 'before-save-hook 'delete-trailing-whitespace). > The problem with the method above is that before-save-hook isn't made > a buffer-local variable by hack-local-variables. Therefore, using > (add-hook 'before-save-hook 'delete-trailing-whitespace) causes > delete-trailing-whitespace to be run even for buffers that are not in > the directory hierarchy of .dir-locals.el. Indeed, you have to use (add-hook 'before-save-hook 'delete-trailing-whitespace nil t) With very few (historical) exceptions, all hooks are neither global-only nor buffer-local-only, so the "nil t" args should always be used for buffer-local settings. So we have a bug in the current setting of safe-local-eval-forms. >>> But ideally this patch would be superseded by adding a mechanism that >>> allows .dir-locals.el to add predefined functions to hooks (at least >>> buffer local ones) without having to use eval. >> Why? > Because using eval for the purposes of adding new functions to hooks > feels overkill, and causes several problems. The > affecting-hooks-that-are-not-buffer-local problem is one of them. > Another problem is that there are many equivalent ways of modifying > a hook (using add-hook, using setq, etc), so adding new entries to > safe-local-eval-forms would never catch them all. setq is a wrong way to modify a hook, and safe-local-eval-forms does not need to catch them all, only to allow the ones that are known safe and that are useful. That fact that using eval is overkill doesn't matter, since safe-local-eval-forms restricts this "overkill power" to something very much less powerful. The shape of the setting has to be ": ", so for adding a function to a hook, it could be "add-hook: (write-file-functions time-stamp)", but that's not terribly more convenient than "eval: (add-hook 'write-file-functions 'time-stamp)" while having the disadvantage that eval re-uses an existing syntax. Now, admittedly, because of the `local' argument, the choice is really between add-hook: (write-file-functions time-stamp) and eval: (add-hook 'write-file-functions 'time-stamp nil t) or eval: (add-local-hook 'write-file-functions 'time-stamp) I much prefer one of the last two since it is familiar to Elisp coders, and for those for whom it's not familiar, it's a useful syntax to learn since they can also use it in their .emacs. >> You don't have to write patches like this one. You can just customize >> safe-local-eval-forms. There is a problem, indeed, tho: if you >> customize this var and we later add things to it, you'll keep using your >> customized version and won't benefit from the expanded list. >> So we should keep the default value of safe-local-eval-forms as nil, and >> allow things like those add-hook some other way (e.g. a new var). > ... and that's the third problem caused by using eval to set hooks. No, the same problem would appear with a special "add-hook" setting, since we'd need a new safe-local-add-hooks which would suffer from the same complications. > Besides, customizing safe-local-eval-forms isn't a great solution in the > scenario discussed above: the whole point for a free software project to > have a .dir-locals.el at the root of the repo is so that none of the > (potentially hundreds of) developers of that project need to fiddle with > customize manually. There's no clearly safe subset of Elisp, so we're limited to listing a few known safe cases which we know are used. Note that adding an element to safe-local-eval-forms is a lot easier than changing your .emacs so that the files of project X (and only those files) are opened with the right settings, so the use of .dir-local.el is still very useful even if it has to use an eval form that's not in the default value of safe-local-eval-forms. I've just installed the patch below in the emacs-24 branch. Stefan === modified file 'lisp/files.el' --- lisp/files.el 2012-08-15 16:29:11 +0000 +++ lisp/files.el 2012-08-23 12:15:31 +0000 @@ -2837,7 +2837,8 @@ ;; This should be here at least as long as Emacs supports write-file-hooks. '((add-hook 'write-file-hooks 'time-stamp) (add-hook 'write-file-functions 'time-stamp) - (add-hook 'before-save-hook 'time-stamp)) + (add-hook 'before-save-hook 'time-stamp nil t) + (add-hook 'before-save-hook 'delete-trailing-whitespace nil t)) "Expressions that are considered safe in an `eval:' local variable. Add expressions to this list if you want Emacs to evaluate them, when they appear in an `eval' local variable specification, without first