From: Thierry Volpiatto <thierry.volpiatto@gmail.com>
To: "Drew Adams" <drew.adams@oracle.com>
Cc: 11328@debbugs.gnu.org
Subject: bug#11328: 24.1.50; Comment in `dired-copy-file-recursive' code
Date: Thu, 26 Apr 2012 07:48:01 +0200 [thread overview]
Message-ID: <87ipgngi26.fsf@gmail.com> (raw)
In-Reply-To: <E8562764D90E47AEB6908D050EBF9FD7@us.oracle.com> (Drew Adams's message of "Wed, 25 Apr 2012 14:51:48 -0700")
"Drew Adams" <drew.adams@oracle.com> 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
next prev parent reply other threads:[~2012-04-26 5:48 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-04-24 17:37 bug#11328: 24.1.50; Comment in `dired-copy-file-recursive' code Drew Adams
2012-04-24 17:49 ` Drew Adams
2012-04-24 18:22 ` Drew Adams
2012-04-25 13:40 ` Nix
2012-04-25 16:26 ` Drew Adams
2012-04-25 18:42 ` Thierry Volpiatto
2012-04-25 21:51 ` Drew Adams
2012-04-26 5:48 ` Thierry Volpiatto [this message]
2012-04-26 14:09 ` Drew Adams
2012-04-26 15:35 ` Drew Adams
2012-04-26 18:38 ` Thierry Volpiatto
2012-04-24 18:54 ` Stefan Monnier
2014-02-09 4:32 ` Lars Ingebrigtsen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87ipgngi26.fsf@gmail.com \
--to=thierry.volpiatto@gmail.com \
--cc=11328@debbugs.gnu.org \
--cc=drew.adams@oracle.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.