From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Thierry Volpiatto Newsgroups: gmane.emacs.bugs Subject: bug#11328: 24.1.50; Comment in `dired-copy-file-recursive' code Date: Thu, 26 Apr 2012 07:48:01 +0200 Message-ID: <87ipgngi26.fsf@gmail.com> 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 X-Trace: dough.gmane.org 1335419362 24015 80.91.229.3 (26 Apr 2012 05:49:22 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Thu, 26 Apr 2012 05:49:22 +0000 (UTC) Cc: 11328@debbugs.gnu.org To: "Drew Adams" Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Thu Apr 26 07:49:20 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 1SNHZn-0001bY-I6 for geb-bug-gnu-emacs@m.gmane.org; Thu, 26 Apr 2012 07:49:11 +0200 Original-Received: from localhost ([::1]:42714 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SNHZn-0003tp-0n for geb-bug-gnu-emacs@m.gmane.org; Thu, 26 Apr 2012 01:49:11 -0400 Original-Received: from eggs.gnu.org ([208.118.235.92]:44893) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SNHZk-0003tb-5b for bug-gnu-emacs@gnu.org; Thu, 26 Apr 2012 01:49:09 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SNHZh-0008HM-Uy for bug-gnu-emacs@gnu.org; Thu, 26 Apr 2012 01:49:07 -0400 Original-Received: from debbugs.gnu.org ([140.186.70.43]:52348) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SNHZh-0008HH-Ol for bug-gnu-emacs@gnu.org; Thu, 26 Apr 2012 01:49:05 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.72) (envelope-from ) id 1SNHab-0000Rp-Ms for bug-gnu-emacs@gnu.org; Thu, 26 Apr 2012 01:50:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Thierry Volpiatto Original-Sender: debbugs-submit-bounces@debbugs.gnu.org Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 26 Apr 2012 05:50:01 +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.13354193521656 (code B ref 11328); Thu, 26 Apr 2012 05:50:01 +0000 Original-Received: (at 11328) by debbugs.gnu.org; 26 Apr 2012 05:49:12 +0000 Original-Received: from localhost ([127.0.0.1]:53382 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SNHZn-0000Qf-PC for submit@debbugs.gnu.org; Thu, 26 Apr 2012 01:49:12 -0400 Original-Received: from mail-wg0-f42.google.com ([74.125.82.42]:51111) by debbugs.gnu.org with esmtp (Exim 4.72) (envelope-from ) id 1SNHZk-0000QN-DW for 11328@debbugs.gnu.org; Thu, 26 Apr 2012 01:49:10 -0400 Original-Received: by wgbds11 with SMTP id ds11so5282435wgb.3 for <11328@debbugs.gnu.org>; Wed, 25 Apr 2012 22:48:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version:content-type; bh=BKIKb7+Vi0RjBqVeIEk7cU0iAHKP4UeYRh4J/VcgaOc=; b=ayUtFniUq6/4KS/La55uh6IIQTFr33UCjP07VVgX1FhIzLjPABiPyRinFBSQveGewN uUCcbYIyBVw1OjzpGTqVf/9nbtVQ0S2e1x8bwglBSxsD/DHNFBYRjynCUj/W6d4G2usG ka4tBCklqtn7P63U5uOQBBADWEpqzKQebVXZq1Nzt7Bv0bPuTeeY6U71B/lzVGZ/yP7S hBXzaYxxKA+b142850PMbNZxKCPSmoAcN8t05iEBTJ9fQ0BPjot11OGXgCy//cgZUFnD j/mKubSB812Rd20cIu2X3iMX86CQZVCSAAW3SstsIkyNQgLqP4/IJqe4qlZp7E9A2gCv 28gQ== Original-Received: by 10.216.133.139 with SMTP id q11mr3318686wei.44.1335419285522; Wed, 25 Apr 2012 22:48:05 -0700 (PDT) Original-Received: from thierry-MM061 (lbe83-2-78-243-104-167.fbx.proxad.net. [78.243.104.167]) by mx.google.com with ESMTPS id gg2sm7375678wib.7.2012.04.25.22.48.03 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 25 Apr 2012 22:48:04 -0700 (PDT) In-Reply-To: (Drew Adams's message of "Wed, 25 Apr 2012 14:51:48 -0700") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1.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:59499 Archived-At: "Drew Adams" writes: > 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. Yes me too now, but if I remember the first time I use `dired-create-files' in my code, it took me some time to figure how to use this. This is why I say it would be nice to clarify the use of `dired-create-files'. Just take example of TARGET, that could be an argument of `dired-create-files' instead of using NAME-CONSTRUCTOR. Actually you must give TARGET to d-c-files within a lambda (NAME-CONSTRUCTOR): --8<---------------cut here---------------start------------->8--- (dired-create-files fn ; the file-creator a function e.g `dired-copy-file' (symbol-name action) files ; A list of files to apply file-creator on. (if (file-directory-p target) ; A lambda form that handle the special arg TARGET. #'(lambda (from) (expand-file-name (file-name-nondirectory from) target)) #'(lambda (from) target)) marker) --8<---------------cut here---------------end--------------->8--- It would be more clear to call d-c-files like this: --8<---------------cut here---------------start------------->8--- (dired-create-files fn ; the file-creator a function e.g `dired-copy-file' (symbol-name action) files ; A list of files to apply file-creator on. ;; The `if' form above containing the lambda is now in `dired-create-files' ;; give it TARGET to handle. target marker) --8<---------------cut here---------------end--------------->8--- Of course using a lambda as arg like it is done actually seem more flexible, but I don't think it could be used in many differents way. i.e in others way than the one in `dired-do-create-files'. > 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)) This would be even more complex to understand how to use d-c-files. > 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+). Same here (helm), but this would not be difficult to fix. > 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. Agree, it is the problem with such obscure code, unexpected behavior, but in this case I don't think it is too scary to simplify it. > 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. ;-) Never understood what "fluid" mean, but we can't say also they are really "free" (even if they are sort of free) because they are (will be) all let-bounded in some places. i.e they just not figure in the code but they must be apported latter by the caller. -- Thierry Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997