From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: "Drew Adams" Newsgroups: gmane.emacs.bugs Subject: bug#11328: 24.1.50; Comment in `dired-copy-file-recursive' code Date: Wed, 25 Apr 2012 14:51:48 -0700 Message-ID: References: <0CC212AF2EA740A0B8FE5EEF91077A2D@us.oracle.com><9DC04CC6E710430F90675022247AF8C1@us.oracle.com><6D7414BA5657418E9D2AB4D0AA0E71A3@us.oracle.com><87y5pk53ra.fsf@spindle.srvr.nix> <87ty07hcv7.fsf@gmail.com> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit X-Trace: dough.gmane.org 1335390739 13814 80.91.229.3 (25 Apr 2012 21:52:19 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Wed, 25 Apr 2012 21:52:19 +0000 (UTC) To: "'Thierry Volpiatto'" , <11328@debbugs.gnu.org> Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Wed Apr 25 23:52:18 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 1SNA8G-00073x-JK for geb-bug-gnu-emacs@m.gmane.org; Wed, 25 Apr 2012 23:52:16 +0200 Original-Received: from localhost ([::1]:38974 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SNA8G-00043z-1c for geb-bug-gnu-emacs@m.gmane.org; Wed, 25 Apr 2012 17:52:16 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:35310) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SNA8A-0003oY-OR for bug-gnu-emacs@gnu.org; Wed, 25 Apr 2012 17:52:12 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SNA88-0000BL-Ct for bug-gnu-emacs@gnu.org; Wed, 25 Apr 2012 17:52:10 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:52136) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SNA88-0000BC-9S for bug-gnu-emacs@gnu.org; Wed, 25 Apr 2012 17:52:08 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.72) (envelope-from ) id 1SNA90-0003lw-8p for bug-gnu-emacs@gnu.org; Wed, 25 Apr 2012 17:53:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: "Drew Adams" Original-Sender: debbugs-submit-bounces@debbugs.gnu.org Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 25 Apr 2012 21:53:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 11328 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 11328-submit@debbugs.gnu.org id=B11328.133539078014493 (code B ref 11328); Wed, 25 Apr 2012 21:53:02 +0000 Original-Received: (at 11328) by debbugs.gnu.org; 25 Apr 2012 21:53:00 +0000 Original-Received: from localhost ([127.0.0.1]:53170 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SNA8x-0003lg-FO for submit@debbugs.gnu.org; Wed, 25 Apr 2012 17:53:00 -0400 Original-Received: from acsinet15.oracle.com ([141.146.126.227]:27036) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SNA8u-0003lR-9U for 11328@debbugs.gnu.org; Wed, 25 Apr 2012 17:52:57 -0400 Original-Received: from ucsinet22.oracle.com (ucsinet22.oracle.com [156.151.31.94]) by acsinet15.oracle.com (Sentrion-MTA-4.2.2/Sentrion-MTA-4.2.2) with ESMTP id q3PLpp3l018566 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 25 Apr 2012 21:51:52 GMT Original-Received: from acsmt357.oracle.com (acsmt357.oracle.com [141.146.40.157]) by ucsinet22.oracle.com (8.14.4+Sun/8.14.4) with ESMTP id q3PLponN022872 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 25 Apr 2012 21:51:51 GMT Original-Received: from abhmt108.oracle.com (abhmt108.oracle.com [141.146.116.60]) by acsmt357.oracle.com (8.12.11.20060308/8.12.11) with ESMTP id q3PLpore026720; Wed, 25 Apr 2012 16:51:50 -0500 Original-Received: from dradamslap1 (/130.35.178.194) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Wed, 25 Apr 2012 14:51:50 -0700 X-Mailer: Microsoft Office Outlook 11 In-Reply-To: <87ty07hcv7.fsf@gmail.com> Thread-Index: Ac0jE2uWYXbLAAo6TEu1+EFCaFNRcgAACUCg X-MimeOLE: Produced By Microsoft MimeOLE V6.00.2900.6157 X-Source-IP: ucsinet22.oracle.com [156.151.31.94] 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:59490 Archived-At: Hi Thierry, > about fluid variables in dired code, particularly in > `dired-create-files', I would say instead of fluid "obscure", as they > are variables that must be in any calls of `dired-create-files' > within a lambda used as argument of this function. > I think it would be better to have explicit arg in the caller and have > also the lambda's there. The code would be easier to read and > understand and no need to have obscure comments such as "fluid > variable...etc". That the use of those vars can make the code obscure is one thing. But what makes the vars potentially problematic is that they are free. If there is to be a comment in the code to draw attention to this then the comment should say that the vars are _free_ here or there. A comment should not just say that they are "obscure" (I know you did not suggest that). What you suggest is one approach to eliminating the free occurrences. I'm not sure that's really needed or is the best approach. I have no opinion about that - I don't really care much one way or the other. To know what the best approach is someone would need to spend a bit of time with the code. There are also some other approaches, if we did want to eliminate those free occurrences: The code (e.g. callers) could just use either (a) lexical scoping (`lexical-let' or file-level) to capture the variable plus its value within the lambda closure, or (b) backquote with quote+comma to capture only the value (i.e., a pseudo-closure: no var at all, just the value). E.g., in the NAME-CONSTRUCTOR arg that is passed by `dired-do-create-files' to `dired-create-files', the code could use this, substituting TARGET's value instead of leaving TARGET as a free var in the lambda: `(lambda (from) (expand-file-name (file-name-nondirectory from) ',target)) instead of: (lambda (from) (expand-file-name (file-name-nondirectory from) target)) Or it could just use the latter if TARGET were lexically bound with the right value. In that case the lambda would form a closure. That's an easy one. There is also the more convoluted case of `d-do-copy', which calls `d-create-files', which binds `d-overwrite-confirmed' around its funcall of the FILE-CREATOR arg, which is `d-copy-file' in this case, which calls `d-handle-overwrite' (without passing `d-overwrite-confirmed'), which uses `d-overwrite-confirmed'. Maybe that's what you had in mind. First thing about that one is that the funcall actually passes `d-overwrite-confirmed' as an arg to `d-copy-file', in addition to binding it for use by `d-handle-overwrite'. It would be simpler to just add it as a parameter for `d-handle-overwrite' and then let `d-copy-file' and others pass it along explicitly to that function. Second thing is that the value of `overwrite-backup-query', which var is free in `d-handle-overwrite', is never even used anywhere. That var is bound in `d-create-files' presumably only because `d-query', to which it is passed, expects a variable (which it sets - in this case uselessly). There is plenty of such convoluted stuff in the Dired code. No doubt some of it could be simplified, but the cleanup would have to be careful and be sure not to change any behavior. And some changes will likely affect 3rd-party code (e.g. Dired+). One reason for such complicated code (a guess) is that someone at one point tried to define some generic code (macros, functions), and other code was written to use that code, and then later someone needed slightly different generic functions (e.g. additional parameters), but instead of modifying the generic functions (because there was already consuming code that expected the existing interfaces) they fudged other ways of getting the additional info to the recipients. Such is life. Other than that, the same ideas above apply. Suppose that there is some good reason not to add a parameter to `d-handle-overwrite' in order to pass the `d-overwrite-confirmed' value. Or suppose that `d-copy-file' did not accept a third arg, and that it were only `d-handle-overwrite' that needed the `d-overwrite-confirmed' value. In such cases, to get the `d-overwrite-confirmed' value to the innards of such functions that do not accept it as an arg, instead of funcalling FILE-CREATOR directly, `d-create-files' could funcall a lambda that encapsulates the value of `d-overwrite-confirmed' and calls FILE-CREATOR. That too would explicitly reflect the fact that the file creator function depends on that value. Or if `d-copy-file' & compagnie are used only for `d-create-files', then put their defuns inside it lexically (nested defuns) and use lexical scoping for the file. And so on. There are different ways to eliminate the free vars or wrap them together with their values in a closure. And perhaps the code could anyway be simplified in other ways, which might obviate any such need. Dunno. I haven't bothered to look closely at it (I don't care enough). Again, if someone does that, I really hope they are careful. Or we can just live with the free vars, in which case a comment doesn't hurt. But it should say "free", not "fluid", IMO. ;-) - Drew