From e83d7d446dcb417ba73b35ef9c8f4d46f76016ce Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 25 Jan 2024 17:54:13 -0800
Subject: [PATCH] Improve handling of local variable settings in Eshell

This ensures that these commands work the same as normal commands,
aside from making environment variable settings local to that command.
Among other things, this means that "VAR=value cd dir/" now changes
the directory correctly.

* lisp/eshell/esh-var.el (eshell-in-local-scope-p)
(eshell-local-variable-bindings): New variables.
(eshell-var-initialize, eshell-set-variable): Use
'eshell-local-variable-bindings'.
(eshell-handle-local-variables): Don't use 'eshell-as-subcommand'.

* test/lisp/eshell/esh-var-tests.el (esh-var-test/local-variables/cd):
New test.

(cherry picked from commit 65829b27ca4898ff0905a8124980243977a1382f)
---
 lisp/eshell/esh-var.el            | 57 +++++++++++++++++--------------
 test/lisp/eshell/esh-var-tests.el |  8 +++++
 2 files changed, 39 insertions(+), 26 deletions(-)

diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index ae0b18cd13a..1d90fbdd8ee 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -255,6 +255,20 @@ copied (a.k.a. \"exported\") to the environment of created subprocesses."
 (defvar-keymap eshell-var-mode-map
   "C-c M-v" #'eshell-insert-envvar)
 
+;;; Internal Variables:
+
+(defvar eshell-in-local-scope-p nil
+  "Non-nil if the current command has a local variable scope.
+This is set to t in `eshell-local-variable-bindings' (which see).")
+
+(defvar eshell-local-variable-bindings
+  '((eshell-in-local-scope-p t)
+    (process-environment (eshell-copy-environment))
+    (eshell-variable-aliases-list eshell-variable-aliases-list)
+    (eshell-path-env-list eshell-path-env-list)
+    (comint-pager comint-pager))
+  "A list of `let' bindings for local variable (and subcommand) environments.")
+
 ;;; Functions:
 
 (define-minor-mode eshell-var-mode
@@ -271,12 +285,8 @@ copied (a.k.a. \"exported\") to the environment of created subprocesses."
     (setq-local process-environment (eshell-copy-environment)))
   (make-local-variable 'comint-pager)
   (setq-local eshell-subcommand-bindings
-              (append
-               '((process-environment (eshell-copy-environment))
-                 (eshell-variable-aliases-list eshell-variable-aliases-list)
-                 (eshell-path-env-list eshell-path-env-list)
-                 (comint-pager comint-pager))
-               eshell-subcommand-bindings))
+              (append eshell-local-variable-bindings
+                      eshell-subcommand-bindings))
 
   (setq-local eshell-special-chars-inside-quoting
        (append eshell-special-chars-inside-quoting '(?$)))
@@ -296,30 +306,25 @@ copied (a.k.a. \"exported\") to the environment of created subprocesses."
 
 (defun eshell-handle-local-variables ()
   "Allow for the syntax `VAR=val <command> <args>'."
-  ;; Eshell handles local variable settings (e.g. 'CFLAGS=-O2 make')
-  ;; by making the whole command into a subcommand, and calling
-  ;; `eshell-set-variable' immediately before the command is invoked.
-  ;; This means that 'FOO=x cd bar' won't work exactly as expected,
-  ;; but that is by no means a typical use of local environment
-  ;; variables.
+  ;; Handle local variable settings by let-binding the entries in
+  ;; `eshell-local-variable-bindings' and calling `eshell-set-variable'
+  ;; for each variable before the command is invoked.
   (let ((setvar "\\`\\([A-Za-z_][A-Za-z0-9_]*\\)=\\(.*\\)\\'")
         (command eshell-last-command-name)
         (args eshell-last-arguments))
     (when (and (stringp command) (string-match setvar command))
       (throw 'eshell-replace-command
-             `(eshell-as-subcommand
-               (progn
-                 ,@(let (locals)
-                     (while (and (stringp command)
-                                 (string-match setvar command))
-                       (push `(eshell-set-variable
-                               ,(match-string 1 command)
-                               ,(match-string 2 command))
-                             locals)
-                       (setq command (pop args)))
-                     (nreverse locals))
-                 (eshell-named-command ,command ,(list 'quote args)))
-              )))))
+             `(let ,eshell-local-variable-bindings
+                ,@(let (locals)
+                    (while (and (stringp command)
+                                (string-match setvar command))
+                      (push `(eshell-set-variable
+                              ,(match-string 1 command)
+                              ,(match-string 2 command))
+                            locals)
+                      (setq command (pop args)))
+                    (nreverse locals))
+                 (eshell-named-command ,command ,(list 'quote args)))))))
 
 (defun eshell-interpolate-variable ()
   "Parse a variable interpolation.
@@ -709,7 +714,7 @@ to a Lisp variable)."
          ((functionp target)
           (funcall target nil value))
          ((null target)
-          (unless eshell-in-subcommand-p
+          (unless eshell-in-local-scope-p
             (error "Variable `%s' is not settable" (eshell-stringify name)))
           (push `(,name ,(lambda () value) t t)
                 eshell-variable-aliases-list)
diff --git a/test/lisp/eshell/esh-var-tests.el b/test/lisp/eshell/esh-var-tests.el
index 39c278a6277..bb3d18abf6d 100644
--- a/test/lisp/eshell/esh-var-tests.el
+++ b/test/lisp/eshell/esh-var-tests.el
@@ -653,6 +653,14 @@ nil, use FUNCTION instead."
                                 "VAR=hello\n")
    (should (equal (getenv "VAR") "value"))))
 
+(ert-deftest esh-var-test/local-variables/cd ()
+  "Test that \"VAR=value cd DIR\" properly changes the directory."
+  (let ((parent-directory (file-name-directory
+                           (directory-file-name default-directory))))
+    (with-temp-eshell
+     (eshell-insert-command "VAR=hello cd ..")
+     (should (equal default-directory parent-directory)))))
+
 
 ;; Variable aliases
 
-- 
2.39.5