From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Mathieu Boespflug Newsgroups: gmane.emacs.bugs Subject: bug#12259: Add delete-trailing-whitespace to list of safe eval forms Date: Wed, 22 Aug 2012 12:27:47 -0400 Message-ID: <87r4qyao3g.fsf@santiago.tweag.net> References: <87r4r1e7i1.fsf@santiago.tweag.net> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: ger.gmane.org 1345654186 18041 80.91.229.3 (22 Aug 2012 16:49:46 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Wed, 22 Aug 2012 16:49:46 +0000 (UTC) Cc: 12259@debbugs.gnu.org To: Stefan Monnier Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Wed Aug 22 18:49:46 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 1T4E7k-0004O5-Sk for geb-bug-gnu-emacs@m.gmane.org; Wed, 22 Aug 2012 18:49:45 +0200 Original-Received: from localhost ([::1]:43645 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T4E7j-0001XT-BX for geb-bug-gnu-emacs@m.gmane.org; Wed, 22 Aug 2012 12:49:43 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:33656) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T4E7f-0001Wb-2l for bug-gnu-emacs@gnu.org; Wed, 22 Aug 2012 12:49:41 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T4E7d-0008SF-U3 for bug-gnu-emacs@gnu.org; Wed, 22 Aug 2012 12:49:39 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:34093) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T4E7d-0008SB-Qw for bug-gnu-emacs@gnu.org; Wed, 22 Aug 2012 12:49:37 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.72) (envelope-from ) id 1T4E83-0003kN-23 for bug-gnu-emacs@gnu.org; Wed, 22 Aug 2012 12:50:03 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Mathieu Boespflug Original-Sender: debbugs-submit-bounces@debbugs.gnu.org Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 22 Aug 2012 16:50:02 +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.134565415914334 (code B ref 12259); Wed, 22 Aug 2012 16:50:02 +0000 Original-Received: (at 12259) by debbugs.gnu.org; 22 Aug 2012 16:49:19 +0000 Original-Received: from localhost ([127.0.0.1]:43634 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1T4E7K-0003j8-Lg for submit@debbugs.gnu.org; Wed, 22 Aug 2012 12:49:19 -0400 Original-Received: from mail-yx0-f172.google.com ([209.85.213.172]:52631) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from <0xbadcode@gmail.com>) id 1T4Dpg-0003He-8b for 12259@debbugs.gnu.org; Wed, 22 Aug 2012 12:31:05 -0400 Original-Received: by yenm5 with SMTP id m5so836697yen.3 for <12259@debbugs.gnu.org>; Wed, 22 Aug 2012 09:30:38 -0700 (PDT) Original-Received: by 10.50.186.130 with SMTP id fk2mr2839065igc.60.1345653033041; Wed, 22 Aug 2012 09:30:33 -0700 (PDT) Original-Received: from santiago.tweag.net ([216.252.64.250]) by mx.google.com with ESMTPS id ua5sm3322208igb.10.2012.08.22.09.30.31 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 22 Aug 2012 09:30:32 -0700 (PDT) In-Reply-To: (Stefan Monnier's message of "Wed, 22 Aug 2012 10:36:29 -0400") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux) X-Mailman-Approved-At: Wed, 22 Aug 2012 12:49:17 -0400 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:63400 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. This is undesirable because .dir-locals.el is often used by free software projects to enforce a common set of guidelines and style for editing code. Any changes to hooks should therefore ideally be directory local, so as to apply only to those files that are part of some particular free software repository. > IOW I think we should only add the before-save-hook version but not > the others (and I guess the same holds for time-stamp, tho we'll > probably keep the other ones for time-stamp for backward-compatibility > reasons). (See above.) >> 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. >> That way we wouldn't have to write patches such as this one for every >> new sensible stock function that people want to have executed on >> file saves. > > 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. 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. -- Mathieu