master bc25d76650a: Fix documented Eshell behavior of ignoring leading nils in commands

[Jim Porter]

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

    Fix documented Eshell behavior of ignoring leading nils in commands
    
    * lisp/eshell/esh-var.el (eshell-handle-local-variables): Simplify,
    and move leading-nil handling to...
    * lisp/eshell/esh-cmd.el (eshell-named-command): ... here.
    
    * test/lisp/eshell/esh-cmd-tests.el (esh-cmd-test/skip-leading-nils):
    * test/lisp/eshell/esh-var-tests.el
    (esh-var-test/local-variables/skip-nil): New tests.
    
    * doc/misc/eshell.texi (Expansion): Document this behavior.
---
 doc/misc/eshell.texi              | 12 ++++++--
 lisp/eshell/esh-cmd.el            | 22 ++++++++++-----
 lisp/eshell/esh-var.el            | 59 +++++++++++++++------------------------
 test/lisp/eshell/esh-cmd-tests.el |  6 ++++
 test/lisp/eshell/esh-var-tests.el |  8 ++++++
 5 files changed, 61 insertions(+), 46 deletions(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 0ec90d0c159..8b3eb72aa66 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1385,9 +1385,15 @@ Concatenate the string representation of each value.
 
 @node Dollars Expansion
 @section Dollars Expansion
-Eshell has different @code{$} expansion syntax from other shells.  There
-are some similarities, but don't let these lull you into a false sense
-of familiarity.
+Like in many other shells, you can use @code{$} expansions to insert
+various values into your Eshell invocations.  While Eshell's @code{$}
+expansion syntax has some similarities to the syntax from other
+shells, there are also many differences.  Don't let these similarities
+lull you into a false sense of familiarity.
+
+When using command form (@pxref{Invocation}), Eshell will ignore any
+leading nil values, so if @var{foo} is @code{nil}, @samp{$@var{foo}
+echo hello} is equivalent to @samp{echo hello}.
 
 @table @code
 
diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index 169d66bc127..dc210ff74f9 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -1286,16 +1286,24 @@ have been replaced by constants."
 COMMAND may result in an alias being executed, or a plain command."
   (unless eshell-allow-commands
     (signal 'eshell-commands-forbidden '(named)))
+  ;; Strip off any leading nil values.  This can only happen if a
+  ;; variable evaluates to nil, such as "$var x", where `var' is nil.
+  ;; In that case, the command name becomes `x', for compatibility
+  ;; with most regular shells (the difference is that they do an
+  ;; interpolation pass before the argument parsing pass, but Eshell
+  ;; does both at the same time).
+  (while (and (not command) args)
+    (setq command (pop args)))
   (setq eshell-last-arguments args
-       eshell-last-command-name (eshell-stringify command))
+        eshell-last-command-name (eshell-stringify command))
   (run-hook-with-args 'eshell-prepare-command-hook)
   (cl-assert (stringp eshell-last-command-name))
-  (if eshell-last-command-name
-      (or (run-hook-with-args-until-success
-          'eshell-named-command-hook eshell-last-command-name
-          eshell-last-arguments)
-         (eshell-plain-command eshell-last-command-name
-                               eshell-last-arguments))))
+  (when eshell-last-command-name
+    (or (run-hook-with-args-until-success
+         'eshell-named-command-hook eshell-last-command-name
+         eshell-last-arguments)
+        (eshell-plain-command eshell-last-command-name
+                              eshell-last-arguments))))
 
 (defalias 'eshell-named-command* 'eshell-named-command)
 
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 711c35f8527..d484aa406e1 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -296,43 +296,30 @@ copied (a.k.a. \"exported\") to the environment of 
created subprocesses."
 
 (defun eshell-handle-local-variables ()
   "Allow for the syntax `VAR=val <command> <args>'."
-  ;; strip off any null commands, which can only happen if a variable
-  ;; evaluates to nil, such as "$var x", where `var' is nil.  The
-  ;; command name in that case becomes `x', for compatibility with
-  ;; most regular shells (the difference is that they do an
-  ;; interpolation pass before the argument parsing pass, but Eshell
-  ;; does both at the same time).
-  (while (and (not eshell-last-command-name)
-             eshell-last-arguments)
-    (setq eshell-last-command-name (car eshell-last-arguments)
-         eshell-last-arguments (cdr eshell-last-arguments)))
+  ;; 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.
   (let ((setvar "\\`\\([A-Za-z_][A-Za-z0-9_]*\\)=\\(.*\\)\\'")
-       (command (eshell-stringify eshell-last-command-name))
-       (args eshell-last-arguments))
-    ;; local variable settings (such as 'CFLAGS=-O2 make') are handled
-    ;; by making the whole command into a subcommand, and calling
-    ;; setenv immediately before the command is invoked.  This means
-    ;; that 'BLAH=x cd blah' won't work exactly as expected, but that
-    ;; is by no means a typical use of local environment variables.
-    (if (and command (string-match setvar command))
-       (throw
-        'eshell-replace-command
-        (list
-         'eshell-as-subcommand
-         (append
-          (list 'progn)
-          (let ((l (list t)))
-            (while (string-match setvar command)
-              (nconc
-               l (list
-                   (list 'eshell-set-variable
-                         (match-string 1 command)
-                         (match-string 2 command))))
-              (setq command (eshell-stringify (car args))
-                    args (cdr args)))
-            (cdr l))
-          (list (list 'eshell-named-command
-                      command (list 'quote args)))))))))
+        (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)))
+              )))))
 
 (defun eshell-interpolate-variable ()
   "Parse a variable interpolation.
diff --git a/test/lisp/eshell/esh-cmd-tests.el 
b/test/lisp/eshell/esh-cmd-tests.el
index d625b8a6a5d..7c384471e93 100644
--- a/test/lisp/eshell/esh-cmd-tests.el
+++ b/test/lisp/eshell/esh-cmd-tests.el
@@ -80,6 +80,12 @@ e.g. \"{(+ 1 2)} 3\" => 3"
    (eshell-match-command-output "echo ${echo $value}"
                                 "hello\n")))
 
+(ert-deftest esh-cmd-test/skip-leading-nils ()
+  "Test that Eshell skips leading nil arguments for named commands."
+  (eshell-command-result-equal "$eshell-test-value echo hello" "hello")
+  (eshell-command-result-equal
+   "$eshell-test-value $eshell-test-value echo hello" "hello"))
+
 (ert-deftest esh-cmd-test/let-rebinds-after-defer ()
   "Test that let-bound values are properly updated after `eshell-defer'.
 When inside a `let' block in an Eshell command form, we need to
diff --git a/test/lisp/eshell/esh-var-tests.el 
b/test/lisp/eshell/esh-var-tests.el
index ff646f5f977..83c0f480627 100644
--- a/test/lisp/eshell/esh-var-tests.el
+++ b/test/lisp/eshell/esh-var-tests.el
@@ -645,6 +645,14 @@ nil, use FUNCTION instead."
    (eshell-match-command-output "VAR=hello env" "VAR=hello\n")
    (should (equal (getenv "VAR") "value"))))
 
+(ert-deftest esh-var-test/local-variables/skip-nil ()
+  "Test that Eshell skips leading nil arguments after local variable setting."
+  (with-temp-eshell
+   (push "VAR=value" process-environment)
+   (eshell-match-command-output "VAR=hello $eshell-test-value env"
+                                "VAR=hello\n")
+   (should (equal (getenv "VAR") "value"))))
+
 
 ;; Variable aliases