GNU bug report logs - #11328
24.1.50; Comment in `dired-copy-file-recursive' code

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 11328 in the body.
You can then email your comments to 11328 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "Drew Adams" <drew.adams <at> oracle.com>
To: <bug-gnu-emacs <at> gnu.org>
Subject: 24.1.50; Comment in `dired-copy-file-recursive' code
Date: Tue, 24 Apr 2012 10:37:09 -0700

Just a nit.  But if you are going to add unnecessary comments to the
code that describe only what everyone can see the code does, then at
least get them right.  Otherwise you mislead readers.
 
This comment is incorrect: "Not a directory".  What is actually true at
that point is the following;
 
a. RECURSIVE is nil
b. RECURSIVE is non-nil and this is not a directory
c. this is a directory, RECURSIVE is non-nil and not `always',
   and the user replied `n'
 
(Similarly, the comment "This is a directory", though true, does not
convey the real meaning.  It is a directory AND it should be copied
recursively.)
 
It is a bad habit to add such comments to the code.  Comments should
generally be used when it is not obvious what the code does or why.
 
In GNU Emacs 24.1.50.1 (i386-mingw-nt5.1.2600)
 of 2012-04-23 on MARVIN
Bzr revision: 108006
agustin.martin <at> hispalinux.es-20120423103325-xmra3329elgzhmpc
Windowing system distributor `Microsoft Corp.', version 5.1.2600
Configured using:
 `configure --with-gcc (4.6) --no-opt --enable-checking --cflags
 -ID:/devel/emacs/libs/libXpm-3.5.8/include
 -ID:/devel/emacs/libs/libXpm-3.5.8/src
 -ID:/devel/emacs/libs/libpng-dev_1.4.3-1/include
 -ID:/devel/emacs/libs/zlib-dev_1.2.5-2/include
 -ID:/devel/emacs/libs/giflib-4.1.4-1/include
 -ID:/devel/emacs/libs/jpeg-6b-4/include
 -ID:/devel/emacs/libs/tiff-3.8.2-1/include
 -ID:/devel/emacs/libs/gnutls-3.0.9/include
 -ID:/devel/emacs/libs/libiconv-1.13.1-1-dev/include
 -ID:/devel/emacs/libs/libxml2-2.7.8/include/libxml2'
 

Message #8 received at 11328 <at> debbugs.gnu.org (full text, mbox):

From: "Drew Adams" <drew.adams <at> oracle.com>
To: <11328 <at> debbugs.gnu.org>
Subject: RE: bug#11328: 24.1.50; Comment in `dired-copy-file-recursive' code
Date: Tue, 24 Apr 2012 10:49:50 -0700

Oh, and since there is no doc string it might be a good idea to use better
parameter names.  There is a reason that for `copy-file' and
`make-symbolic-link' the parameter is called OK-IF-ALREADY-EXISTS and not simply
OK-FLAG.  (Same problem for `dired-copy-file'.)  You can guess the meaning from
OK-IF-ALREADY-EXISTS.

You are not helping readers of the code if they need to examine it all closely
to determine what a given parameter does.  In that case, you might as well name
the parameters X1, X2, X3, X4, X5, and X6.

And it's generally a good idea to use the same parameter names when you just
pass the arguments to another function and they have the same meaning.  If you
just pass PRESERVE-TIME to `copy-directory' and `copy-file', then use the same
name they use: KEEP-TIME.  (Or change all three and any other functions related
so they use the same names.)

Message #11 received at 11328 <at> debbugs.gnu.org (full text, mbox):

From: "Drew Adams" <drew.adams <at> oracle.com>
To: <11328 <at> debbugs.gnu.org>
Subject: RE: bug#11328: 24.1.50; Comment in `dired-copy-file-recursive' code
Date: Tue, 24 Apr 2012 11:22:04 -0700

While on the subject of comments in this file, you might want to change "fluid
variable" everywhere to "free variable".  This is perhaps a translation problem.
(AFAIK, in English fluid variables have only to do with fluid mechanics.)

Message #14 received at 11328 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: "Drew Adams" <drew.adams <at> oracle.com>
Cc: 11328 <at> debbugs.gnu.org
Subject: Re: bug#11328: 24.1.50; Comment in `dired-copy-file-recursive' code
Date: Tue, 24 Apr 2012 14:54:55 -0400

I really appreciate your nit-picks (I know the word sounds negative,
but to improve code-quality, it's really what is needed).  But please,
please, pretty please, get yourself access to the Bzr so you can fix
those things yourself.
You'll be happier, we'll be happier, the world will smile, birds will
sing.


        Stefan


>>>>> "Drew" == Drew Adams <drew.adams <at> oracle.com> writes:

> Just a nit.  But if you are going to add unnecessary comments to the
> code that describe only what everyone can see the code does, then at
> least get them right.  Otherwise you mislead readers.
 
> This comment is incorrect: "Not a directory".  What is actually true at
> that point is the following;
 
> a. RECURSIVE is nil
> b. RECURSIVE is non-nil and this is not a directory
> c. this is a directory, RECURSIVE is non-nil and not `always',
>    and the user replied `n'
 
> (Similarly, the comment "This is a directory", though true, does not
> convey the real meaning.  It is a directory AND it should be copied
> recursively.)
 
> It is a bad habit to add such comments to the code.  Comments should
> generally be used when it is not obvious what the code does or why.
 
> In GNU Emacs 24.1.50.1 (i386-mingw-nt5.1.2600)
>  of 2012-04-23 on MARVIN
> Bzr revision: 108006
> agustin.martin <at> hispalinux.es-20120423103325-xmra3329elgzhmpc
> Windowing system distributor `Microsoft Corp.', version 5.1.2600
> Configured using:
>  `configure --with-gcc (4.6) --no-opt --enable-checking --cflags
>  -ID:/devel/emacs/libs/libXpm-3.5.8/include
>  -ID:/devel/emacs/libs/libXpm-3.5.8/src
>  -ID:/devel/emacs/libs/libpng-dev_1.4.3-1/include
>  -ID:/devel/emacs/libs/zlib-dev_1.2.5-2/include
>  -ID:/devel/emacs/libs/giflib-4.1.4-1/include
>  -ID:/devel/emacs/libs/jpeg-6b-4/include
>  -ID:/devel/emacs/libs/tiff-3.8.2-1/include
>  -ID:/devel/emacs/libs/gnutls-3.0.9/include
>  -ID:/devel/emacs/libs/libiconv-1.13.1-1-dev/include
>  -ID:/devel/emacs/libs/libxml2-2.7.8/include/libxml2'
 

Message #17 received at 11328 <at> debbugs.gnu.org (full text, mbox):

From: Nix <nix <at> esperi.org.uk>
To: "Drew Adams" <drew.adams <at> oracle.com>
Cc: 11328 <at> debbugs.gnu.org
Subject: Re: bug#11328: 24.1.50; Comment in `dired-copy-file-recursive' code
Date: Wed, 25 Apr 2012 14:40:09 +0100

On 24 Apr 2012, Drew Adams stated:

> While on the subject of comments in this file, you might want to change "fluid
> variable" everywhere to "free variable".  This is perhaps a translation problem.
> (AFAIK, in English fluid variables have only to do with fluid mechanics.)

Fluid variables are a Scheme thing (more, generally, a
functional-programming thing). "Dynamically-scoped free variable" would
be a reasonable thing to replace it with. If this were a docstring or a
string in the manual, I'd agree that either this needs replacing, or
fluids need a definition -- but this is a code comment, and finding out
what a fluid is once you see that is a matter of ten seconds looking in,
say, the Guile manual.

-- 
NULL && (void)

Message #20 received at 11328 <at> debbugs.gnu.org (full text, mbox):

From: "Drew Adams" <drew.adams <at> oracle.com>
To: "'Nix'" <nix <at> esperi.org.uk>
Cc: 11328 <at> debbugs.gnu.org
Subject: RE: bug#11328: 24.1.50; Comment in `dired-copy-file-recursive' code
Date: Wed, 25 Apr 2012 09:26:18 -0700

> Fluid variables are a Scheme thing (more, generally, a
> functional-programming thing).  "Dynamically-scoped free 
> variable" would be a reasonable thing to replace it with.

Bof!

Googling just a bit suggests that a fluid variable is more or less a dynamically
scoped variable.  That does not at all make it a _free_ variable.  The variable
occurrences noted in this code were free - no matter how scoped.  Free in lambda
calculi, free if dynamically scoped, free if lexically scoped.  Free variables.
And that is presumably why the comments were inserted: to point out to readers
that the occurrences are not bound locally.

> finding out what a fluid is once you see that is a matter of
> ten seconds looking in, say, the Guile manual.

No, it is not a matter of ten seconds.  And the term "fluid variable" does not
even appear in the Guile manual.

The closest the manual comes, AFAICT, is this: "Fluids can also be used to
simulate the desirable effects of dynamically scoped variables."  NB: "also" and
"simulate" and only certain "effects of".

IOW, the Guile manual does not even say that a fluid is a dynamically scoped
variable, let alone a d-s _free_ variable.  A fluid is said to be an object that
can store one value per dynamic state.  The claim wrt dynamic scoping is only
that, among its various uses, a fluid can simulate some of the effects of a
dynamically scoped variable.

(The Guile manual, BTW, distinguishes fluid vars from "parameters", which it
says are "like dynamically bound variables in other Lisp dialects".  The
distinction being apparently that parameters are per-thread.)

Anyway, this is not Guile - or Scheme of any sort.

In the same vein, here is a quote from Emacs Wiki, presumably written by a
Schemite.  It indicates a similar parochialism:

  "Scheme was the first language to introduce lexical binding. ...
   Variables subject to dynamic binding are usually referred to as
   `fluid variables' or `parameters' on these systems."

(http://www.emacswiki.org/emacs/DynamicBindingVsLexicalBinding#Scheme)

In the Beginning there was The Scheme...  First to introduce lexical binding,
indeed!

But even if dynamically bound variables are "usually" referred to as "fluid
variables" among Scheme enthusiasts, that is no reason to suppose that Computer
Science or programming practice in general refers to them that way.  And anyway,
the code comments I commented on are about _free_ variable occurrences.  They do
not simply point out variables that are dynamically scoped.

FWIW, the `librep' manual says that "fluid variables" aka fluids are another
"method of implementing dynamically scoped variables."  The sole purpose of such
an object, it claims, is "to provide a location from which dynamic bindings may
be created."

(And yet librep's claim to fame is that it has improved on Emacs Lisp in this
way: It "was originally inspired by Emacs Lisp.  However one of the main
deficiencies of elisp--the reliance on dynamic scope--has been removed."
Removed ... and then implemented, it seems, using fluids.)

FWIW, here's a Joulist characterization of fluid variables, which claims that
Scheme has _no_ fluid variables:

 "A fluid variable is a symbol and cell that is established as part
  of the current environment.  When a fluid variable (symbol) is
  referenced the current nest of environments is searched for a
  fluid variable with the matching symbol. The first such match
  provides the cell that satisfies the reference.  This really means
  that each continuation embodies a set of fluid bindings.  Many uses
  of continuations, for which they were reified, are incompatible
  with fluid variables.  One such use is time-slicing.
  Scheme has no fluid variables."

http://www.cap-lore.com/Languages/Global.html

The same article mentions global vars, static class vars, "fluid variables in
Common Lisp", and dynamically scoped vars as being "related".  I personally have
not seen "fluid variable" used wrt Common Lisp, which AFAIK calls it a "special
variable".

In short, I do not see where, even in the limited world of Scheme or Guile, a
free variable occurrence is a "fluid variable" or vice versa.  No, I am no
expert on SkemeSpeke.  But I can usually recognize a free variable when I see
one.

Mea culpa: I googled for "fluid variable" before filing the bug, and it seemed
that the term was essentially limited to fluid mechanics.  So I guessed
(wrongly) that its use here might have been a mistranslation from some other
natural language to English.  I did not guess that it was simply the _use_ of
another language, unnatural, and not a translation at all.

So let me translate my bug report: time to translate to English.  These are
_free_ variable occurrences.

Message #23 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#11328: 24.1.50; Comment in `dired-copy-file-recursive' code
Date: Wed, 25 Apr 2012 20:42:36 +0200

Hi Drew,
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".

-- 
  Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 

Message #26 received at 11328 <at> debbugs.gnu.org (full text, mbox):

From: "Drew Adams" <drew.adams <at> oracle.com>
To: "'Thierry Volpiatto'" <thierry.volpiatto <at> gmail.com>,
	<11328 <at> debbugs.gnu.org>
Subject: RE: bug#11328: 24.1.50; Comment in `dired-copy-file-recursive' code
Date: Wed, 25 Apr 2012 14:51:48 -0700

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

Message #29 received at 11328 <at> debbugs.gnu.org (full text, mbox):

From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
To: "Drew Adams" <drew.adams <at> oracle.com>
Cc: 11328 <at> debbugs.gnu.org
Subject: Re: bug#11328: 24.1.50; Comment in `dired-copy-file-recursive' code
Date: Thu, 26 Apr 2012 07:48:01 +0200

"Drew Adams" <drew.adams <at> 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 

Message #32 received at 11328 <at> debbugs.gnu.org (full text, mbox):

From: "Drew Adams" <drew.adams <at> oracle.com>
To: "'Thierry Volpiatto'" <thierry.volpiatto <at> gmail.com>
Cc: 11328 <at> debbugs.gnu.org
Subject: RE: bug#11328: 24.1.50; Comment in `dired-copy-file-recursive' code
Date: Thu, 26 Apr 2012 07:09:06 -0700

> 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.

Look up the definition of free variable - see the lambda calculus.

It is always about free _occurrences_ of a variable.  Freedom of a variable (not
a great word for it, admittedly) is relative to a given context.

Whether a variable is bound in some outer lambda is not the question.  What
matters is whether it is free in some other (e.g. inner) lambda.

X is a free variable in the inner lambda form here, though it is bound in the
outer one, i.e., in the overall expression.

(funcall
 (funcall
   (lambda (x) (lambda (y) (+ x y)))
   42) ; Fn that increments by 42.
 3) ; Applied to 3 gives 45.

That is equivalent to this:

(let ((x 42))
  (funcall (lambda (y) (+ x y)) 3)

(It's perhaps clearer if you remove all the `funcall' atoms.  That will give you
Lisp 1 syntax ~= lambda calculus syntax.)

Message #35 received at 11328 <at> debbugs.gnu.org (full text, mbox):

From: "Drew Adams" <drew.adams <at> oracle.com>
To: "'Thierry Volpiatto'" <thierry.volpiatto <at> gmail.com>
Cc: 11328 <at> debbugs.gnu.org
Subject: RE: bug#11328: 24.1.50; Comment in `dired-copy-file-recursive' code
Date: Thu, 26 Apr 2012 08:35:49 -0700

> Just take example of TARGET, that could be an argument of
> `dired-create-files' instead of using NAME-CONSTRUCTOR.
> [Currently] you must give TARGET to d-c-files within a lambda
> (NAME-CONSTRUCTOR)
> It would be more clear to call d-c-files like this:
> 
> (dired-create-files
>  fn
>  (symbol-name action) 
>  files
>  ;;      The `if' form above containing the lambda 
>  ;;      is now in `dired-create-files'
>  target  ; Give it TARGET to handle.        
>  marker)

As I explained, in the particular case of NAME-CONSTRUCTOR and TARGET the caller
does not in fact ever need (make use of) the _variable_ TARGET.  All it needs is
its value, i.e., the value at the time and place that the lambda is constructed,
in `d-do-create-files'.

So there is no need to include the _variable_ itself in the lambda form.  It is
sufficient to use its value there.  That is clearer to read and is cleaner code.

And there is no need either to pass TARGET as an additional argument to
`d-create-files'.

> > 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))
>
> This would be even more complex to understand how to use d-c-files.

I don't think so.  It has nothing to do with how to use `d-c-files'.

Do you find this clearer?

(lambda (from)
  (expand-file-name (file-name-nondirectory from)
                    "/foo/bar"))

I assume so.  No TARGET variable there.  I've just substituted its current value
at the time the lambda form was constructed (i.e., in `d-do-create-files') -
let's assume "/foo/bar" in this call to `d-do-create-files'.

How about this?

(list 'lambda (list 'from)
  (list 'expand-file-name (list 'file-name-nondirectory 'from))
  (symbol-value 'target))

Those three are all the same thing (assuming TARGET is "/foo/bar" in
`d-do-create-files').

The point is that the lambda form need not contain the (free) variable TARGET at
all.  It is enough that it use the variable's _value_.

And Occam's razor says that if it need not then it should not - just get the
value at lambda construction time/place and plug it in.  The caller of the
lambda need never bother with the variable at all, in any context.

Again, however, this is the simple case - NAME-CONSTRUCTOR.  In other cases the
caller does in fact use the variable itself, looking it up in some particular
context to get its value there, or perhaps even setting it.

But in this simple case the caller does not need the variable - its value at the
time and place of the lambda construction is all that is needed.  And the code
is clearer if we make that explicit.

No sense letting a reader mistakenly think that the caller might somehow use the
variable TARGET.  In fact, it takes a bit of looking at the code to realize
this.  Far better to make it clear to readers from the outset.

> > 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.

In that case, we would be encapsulating the variable's binding at the lambda
construction place (not time, however, since binding is lexical).

That's overkill, but it amounts to the same thing.  The only difference is that
the variable value would be looked up when the lambda is _used_, not when it was
constructed.  But that use-time lookup just returns the value that was in play
at the time of the lambda construction.  So same effect in the end.

HTH - Drew

Message #38 received at 11328 <at> debbugs.gnu.org (full text, mbox):

From: Thierry Volpiatto <thierry.volpiatto <at> gmail.com>
To: "Drew Adams" <drew.adams <at> oracle.com>
Cc: 11328 <at> debbugs.gnu.org
Subject: Re: bug#11328: 24.1.50; Comment in `dired-copy-file-recursive' code
Date: Thu, 26 Apr 2012 20:38:35 +0200

"Drew Adams" <drew.adams <at> oracle.com> writes:

> As I explained, in the particular case of NAME-CONSTRUCTOR and TARGET the caller
> does not in fact ever need (make use of) the _variable_ TARGET.  All it needs is
> its value, i.e., the value at the time and place that the lambda is constructed,
> in `d-do-create-files'.
>
> So there is no need to include the _variable_ itself in the lambda form.  It is
> sufficient to use its value there.  That is clearer to read and is cleaner code.
Yes that is what i wanted to explain.

> And there is no need either to pass TARGET as an additional argument to
> `d-create-files'.
Of course there is no need to do this, it would change nothing, just
make the code clearer to use and read.

What can be done also is leave `dired-create-files' as it is and modify
`dired-do-create-files' to have a generic function usable anywhere
(actually it is usable only in dired) and easy to use; It is what I do here.


-- 
  Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997 

Message #41 received at 11328 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: "Drew Adams" <drew.adams <at> oracle.com>
Cc: 11328 <at> debbugs.gnu.org
Subject: Re: bug#11328: 24.1.50; Comment in `dired-copy-file-recursive' code
Date: Sat, 08 Feb 2014 20:32:28 -0800

"Drew Adams" <drew.adams <at> oracle.com> writes:

> Just a nit.  But if you are going to add unnecessary comments to the
> code that describe only what everyone can see the code does, then at
> least get them right.  Otherwise you mislead readers.
>
> This comment is incorrect: "Not a directory".  What is actually true at
> that point is the following;

Fixed on trunk.

As for the "fluid variable" thing, I think that's probably left as is,
or the code should be rewritten.  Closing.

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.