bug#65110: eshell variable triggers error

[Jim Porter <jporterbugs@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 569 bytes --]

On 8/6/2023 12:20 PM, Jim Porter wrote:
> The I/O refcounting code in Eshell is pretty tricky. I have a WIP patch 
> that will hopefully make it less error-prone, but I'll worry about that 
> later...

Happily, this issue *wasn't* a bug in the I/O refcounting code. The 
failed assertion (which is new in Emacs 30 and exists for exactly this 
reason) just revealed a bug in another part of Eshell. Fix attached 
(plus fixing another issue mentioned in the code comments) with some 
regression tests.

Let me know if this works for you, or if you see any further issues.

[-- Attachment #2: 0001-Fix-listing-of-directory-contents-after-cd-in-Eshell.patch --]
[-- Type: text/plain, Size: 2961 bytes --]

From b6460d4a47fa048b06872d89721578401793ec98 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 6 Aug 2023 13:34:18 -0700
Subject: [PATCH] Fix listing of directory contents after "cd" in Eshell

* lisp/eshell/em-dirs.el (eshell/cd): Ensure we don't close the I/O
handles prematurely.  Additionally, don't clobber the "cd" command's
last-command info.

* test/lisp/eshell/em-dirs-tests.el (em-dirs-test/cd):
(em-dirs-test/cd/list-files-after-cd): New tests (bug#65110).
---
 lisp/eshell/em-dirs.el            | 10 +++++++---
 test/lisp/eshell/em-dirs-tests.el | 23 +++++++++++++++++++++++
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/lisp/eshell/em-dirs.el b/lisp/eshell/em-dirs.el
index 5284df9ab59..640d3676750 100644
--- a/lisp/eshell/em-dirs.el
+++ b/lisp/eshell/em-dirs.el
@@ -429,9 +429,13 @@ eshell/cd
 	  (and eshell-cd-shows-directory
 	       (eshell-printn result)))
 	(run-hooks 'eshell-directory-change-hook)
-	(if eshell-list-files-after-cd
-	    ;; Let-bind eshell-last-command around this?
-	    (eshell-plain-command "ls" (cdr args)))
+        (when eshell-list-files-after-cd
+          ;; Call "ls", but don't update the last-command information.
+          (let ((eshell-last-command-name)
+                (eshell-last-command-status)
+                (eshell-last-arguments))
+            (eshell-protect
+             (eshell-plain-command "ls" (cdr args)))))
 	nil))))
 
 (put 'eshell/cd 'eshell-no-numeric-conversions t)
diff --git a/test/lisp/eshell/em-dirs-tests.el b/test/lisp/eshell/em-dirs-tests.el
index d30b3d7d73f..9864b72ba78 100644
--- a/test/lisp/eshell/em-dirs-tests.el
+++ b/test/lisp/eshell/em-dirs-tests.el
@@ -99,4 +99,27 @@ em-dirs-test/directory-ring-var-indices
      (eshell-match-command-output "echo $-[1][/ 1 3]"
                                   "(\"some\" \"here\")\n"))))
 
+(ert-deftest em-dirs-test/cd ()
+  "Test that changing directories with `cd' works."
+  (ert-with-temp-directory tmpdir
+    (write-region "text" nil (expand-file-name "file.txt" tmpdir))
+    (with-temp-eshell
+     (eshell-match-command-output (format "cd '%s'" tmpdir)
+                                  "\\`\\'")
+     (should (equal default-directory tmpdir)))))
+
+(ert-deftest em-dirs-test/cd/list-files-after-cd ()
+  "Test that listing files after `cd' works."
+  (let ((eshell-list-files-after-cd t))
+    (ert-with-temp-directory tmpdir
+      (write-region "text" nil (expand-file-name "file.txt" tmpdir))
+      (with-temp-eshell
+       (eshell-match-command-output (format "cd '%s'" tmpdir)
+                                    "file.txt\n")
+       (should (equal default-directory tmpdir))
+       ;; Make sure we didn't update the last-command information when
+       ;; running "ls".
+       (should (equal eshell-last-command-name "#<function eshell/cd>"))
+       (should (equal eshell-last-arguments (list tmpdir)))))))
+
 ;; em-dirs-tests.el ends here
-- 
2.25.1