bug#11328: 24.1.50; Comment in `dired-copy-file-recursive' code

[Thierry Volpiatto <thierry.volpiatto@gmail.com>]

"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