Subject: Re: bug#54470: 29.0.50; [PATCH] Add documentation/tests for Eshell argument expansion

From: Jim Porter <[email-masked]>
Date: Fri, 15 Apr 2022 21:57:02 -0700

On 4/15/2022 5:56 AM, Eli Zaretskii wrote:
>> Cc: [email-masked]
>> From: Jim Porter <[email-masked]>
>> Date: Fri, 1 Apr 2022 22:10:07 -0700
>>
>>> @kbd is right for things that are meant specifically and only as
>>> keyboard input.
>>
>> Thanks for the explanation.
>>
>> Having thought this over further, I think Eli's suggestion to use @samp
>> makes sense for both the globs and the predicates/modifiers, so I've
>> updated my patches to do this.
> 
> Thanks.  I was about to install these, but then I saw that some of
> the tests you added fail on my system:
[snip]

Thanks for testing. I think this is because `eshell-file-attributes' 
calls `expand-file-name' on the FILE argument, which prepends a drive 
letter on MS Windows. That makes my code in `eshell-partial-let-func' 
(in em-pred-tests.el) fail to identify the fake files.

Can you try the attached patch to see if the tests pass? If it works, 
I'll fold it into the previous patches and resubmit them. (It works for 
me on an MS Windows system, but I don't have build tools on it, so I 
just used the binary release of 28.1 with some of the bits copied from 
my patches to test it out.)

There are a few other ways I could fix this, but this seemed like the 
best. Now, if `eshell-file-attributes' calls `file-attributes', it 
always forwards the FILE argument unchanged, so the wrapping is more 
"transparent" in that case. (Note: I'm not sure `eshell-file-attributes' 
is even necessary anymore; maybe Tramp handles that for us? I haven't 
tested this enough to be confident we can remove it though...)
diff --git a/lisp/eshell/esh-util.el b/lisp/eshell/esh-util.el
index 8089d4d74b..3da712c719 100644
--- a/lisp/eshell/esh-util.el
+++ b/lisp/eshell/esh-util.el
@@ -592,11 +592,11 @@ eshell-file-attributes
 The optional argument ID-FORMAT specifies the preferred uid and
 gid format.  Valid values are `string' and `integer', defaulting to
 `integer'.  See `file-attributes'."
-  (let* ((file (expand-file-name file))
+  (let* ((expanded-file (expand-file-name file))
 	 entry)
-    (if (string-equal (file-remote-p file 'method) "ftp")
-	(let ((base (file-name-nondirectory file))
-	      (dir (file-name-directory file)))
+    (if (string-equal (file-remote-p expanded-file 'method) "ftp")
+	(let ((base (file-name-nondirectory expanded-file))
+	      (dir (file-name-directory expanded-file)))
 	  (if (string-equal "" base) (setq base "."))
 	  (unless entry
 	    (setq entry (eshell-parse-ange-ls dir))