master 65829b27ca4: Improve handling of local variable settings in Eshell

[Jim Porter]

branch: master
commit 65829b27ca4898ff0905a8124980243977a1382f
Author: Jim Porter <jporterbugs@gmail.com>
Commit: Jim Porter <jporterbugs@gmail.com>

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