Support arbitrary Eshell arguments inside special references

* lisp/eshell/esh-arg.el (eshell-current-argument-plain): New variable.
(eshell-parse-special-reference): Use 'eshell-parse-arguments'.
(eshell-get-buffer): New function.
(eshell-insert-buffer-name): Properly quote the buffer name.

* lisp/eshell/esh-proc.el (eshell-read-process-name): Move to "Special
references" section.
(eshell-insert-process): Properly quote the process name.

* lisp/eshell/em-extpipe.el (eshell-parse-external-pipeline):
* lisp/eshell/esh-io.el (eshell-parse-redirection): Don't do anything
when 'eshell-argument-plain' is non-nil.

* test/lisp/eshell/esh-arg-tests.el
(esh-arg-test/special-reference/quoted)
(esh-arg-test/special-reference/var-expansion): New tests.
(esh-arg-test/special-reference/special): Rename to...
(esh-arg-test/special-reference/special-characters): ... this.

* test/lisp/eshell/em-extpipe-tests.el (em-extpipe-tests--deftest):
Properly quote the buffer name.
(em-extpipe-test-4, em-extpipe-test-7): Use 'eshell-get-buffer'.

6 files changed, 178 insertions, 144 deletions

diff --git a/lisp/eshell/em-extpipe.el b/lisp/eshell/em-extpipe.el
index 5c9a0a85934..0d5c217f5f0 100644
--- a/lisp/eshell/em-extpipe.el
+++ b/lisp/eshell/em-extpipe.el
@@ -118,86 +118,87 @@ as though it were Eshell syntax."
   ;; other members of `eshell-parse-argument-hook'.  We must avoid
   ;; misinterpreting a quoted `*|', `*<' or `*>' as indicating an
   ;; external pipeline, hence the structure of the loop in `findbeg1'.
-  (cl-flet
-      ((findbeg1 (pat &optional go (bound (point-max)))
-         (let* ((start (point))
-                (result
-                 (catch 'found
-                   (while (> bound (point))
-                     (let* ((found
-                             (save-excursion
-                               (re-search-forward
-                                "\\(?:#?'\\|\"\\|\\\\\\)" bound t)))
-                            (next (or (and found (match-beginning 0))
-                                      bound)))
-                       (if (re-search-forward pat next t)
-                           (throw 'found (match-beginning 1))
-                         (goto-char next)
-                         (while (eshell-extpipe--or-with-catch
-                                 (eshell-parse-lisp-argument)
-                                 (eshell-parse-backslash)
-                                 (eshell-parse-double-quote)
-                                 (eshell-parse-literal-quote)))
-                         ;; Guard against an infinite loop if none of
-                         ;; the parsers moved us forward.
-                         (unless (or (> (point) next) (eobp))
-                           (forward-char 1))))))))
-           (goto-char (if (and result go) (match-end 0) start))
-           result)))
-    (unless (or eshell-current-argument eshell-current-quoted)
-      (let ((beg (point)) end
-            (next-marked (findbeg1 "\\(?:\\=\\|\\s-\\)\\(\\*[|<>]\\)"))
-            (next-unmarked
-             (or (findbeg1 "\\(?:\\=\\|[^*]\\|\\S-\\*\\)\\(|\\)")
-                 (point-max))))
-        (when (and next-marked (> next-unmarked next-marked)
-                   (or (> next-marked (point))
-                       (looking-back "\\`\\|\\s-" nil)))
-          ;; Skip to the final segment of the external pipeline.
-          (while (findbeg1 "\\(?:\\=\\|\\s-\\)\\(\\*|\\)" t))
-          ;; Find output redirections.
-          (while (findbeg1
-                  "\\([0-9]?>+&?[0-9]?\\s-*\\S-\\)" t next-unmarked)
-            ;; Is the output redirection Eshell-specific?  We have our
-            ;; own logic, rather than calling `eshell-parse-argument',
-            ;; to avoid specifying here all the possible cars of
-            ;; parsed special references -- `get-buffer-create' etc.
-            (forward-char -1)
-            (let ((this-end
-                   (save-match-data
-                     (cond ((looking-at "#<")
-                            (forward-char 1)
-                            (1+ (eshell-find-delimiter ?\< ?\>)))
-                           ((and (looking-at "/\\S-+")
-                                 (assoc (match-string 0)
-                                        eshell-virtual-targets))
-                            (match-end 0))))))
-              (cond ((and this-end end)
-                     (goto-char this-end))
-                    (this-end
-                     (goto-char this-end)
-                     (setq end (match-beginning 0)))
-                    (t
-                     (setq end nil)))))
-          ;; We've moved past all Eshell-specific output redirections
-          ;; we could find.  If there is only whitespace left, then
-          ;; `end' is right before redirections we should exclude;
-          ;; otherwise, we must include everything.
-          (unless (and end (skip-syntax-forward "\s" next-unmarked)
-                       (= next-unmarked (point)))
-            (setq end next-unmarked))
-          (let ((cmd (string-trim
-                      (buffer-substring-no-properties beg end))))
-            (goto-char end)
-            ;; We must now drop the asterisks, unless quoted/escaped.
-            (with-temp-buffer
-              (insert cmd)
-              (goto-char (point-min))
-              (cl-loop
-               for next = (findbeg1 "\\(?:\\=\\|\\s-\\)\\(\\*[|<>]\\)" t)
-               while next do (forward-char -2) (delete-char 1))
-              (eshell-finish-arg
-               `(eshell-external-pipeline ,(buffer-string))))))))))
+  (unless eshell-current-argument-plain
+    (cl-flet
+        ((findbeg1 (pat &optional go (bound (point-max)))
+           (let* ((start (point))
+                  (result
+                   (catch 'found
+                     (while (> bound (point))
+                       (let* ((found
+                               (save-excursion
+                                 (re-search-forward
+                                  "\\(?:#?'\\|\"\\|\\\\\\)" bound t)))
+                              (next (or (and found (match-beginning 0))
+                                        bound)))
+                         (if (re-search-forward pat next t)
+                             (throw 'found (match-beginning 1))
+                           (goto-char next)
+                           (while (eshell-extpipe--or-with-catch
+                                   (eshell-parse-lisp-argument)
+                                   (eshell-parse-backslash)
+                                   (eshell-parse-double-quote)
+                                   (eshell-parse-literal-quote)))
+                           ;; Guard against an infinite loop if none of
+                           ;; the parsers moved us forward.
+                           (unless (or (> (point) next) (eobp))
+                             (forward-char 1))))))))
+             (goto-char (if (and result go) (match-end 0) start))
+             result)))
+      (unless (or eshell-current-argument eshell-current-quoted)
+        (let ((beg (point)) end
+              (next-marked (findbeg1 "\\(?:\\=\\|\\s-\\)\\(\\*[|<>]\\)"))
+              (next-unmarked
+               (or (findbeg1 "\\(?:\\=\\|[^*]\\|\\S-\\*\\)\\(|\\)")
+                   (point-max))))
+          (when (and next-marked (> next-unmarked next-marked)
+                     (or (> next-marked (point))
+                         (looking-back "\\`\\|\\s-" nil)))
+            ;; Skip to the final segment of the external pipeline.
+            (while (findbeg1 "\\(?:\\=\\|\\s-\\)\\(\\*|\\)" t))
+            ;; Find output redirections.
+            (while (findbeg1
+                    "\\([0-9]?>+&?[0-9]?\\s-*\\S-\\)" t next-unmarked)
+              ;; Is the output redirection Eshell-specific?  We have our
+              ;; own logic, rather than calling `eshell-parse-argument',
+              ;; to avoid specifying here all the possible cars of
+              ;; parsed special references -- `get-buffer-create' etc.
+              (forward-char -1)
+              (let ((this-end
+                     (save-match-data
+                       (cond ((looking-at "#<")
+                              (forward-char 1)
+                              (1+ (eshell-find-delimiter ?\< ?\>)))
+                             ((and (looking-at "/\\S-+")
+                                   (assoc (match-string 0)
+                                          eshell-virtual-targets))
+                              (match-end 0))))))
+                (cond ((and this-end end)
+                       (goto-char this-end))
+                      (this-end
+                       (goto-char this-end)
+                       (setq end (match-beginning 0)))
+                      (t
+                       (setq end nil)))))
+            ;; We've moved past all Eshell-specific output redirections
+            ;; we could find.  If there is only whitespace left, then
+            ;; `end' is right before redirections we should exclude;
+            ;; otherwise, we must include everything.
+            (unless (and end (skip-syntax-forward "\s" next-unmarked)
+                         (= next-unmarked (point)))
+              (setq end next-unmarked))
+            (let ((cmd (string-trim
+                        (buffer-substring-no-properties beg end))))
+              (goto-char end)
+              ;; We must now drop the asterisks, unless quoted/escaped.
+              (with-temp-buffer
+                (insert cmd)
+                (goto-char (point-min))
+                (cl-loop
+                 for next = (findbeg1 "\\(?:\\=\\|\\s-\\)\\(\\*[|<>]\\)" t)
+                 while next do (forward-char -2) (delete-char 1))
+                (eshell-finish-arg
+                 `(eshell-external-pipeline ,(buffer-string)))))))))))
 
 (defun eshell-rewrite-external-pipeline (terms)
   "Rewrite an external pipeline in TERMS as parsed by
diff --git a/lisp/eshell/esh-arg.el b/lisp/eshell/esh-arg.el
index e7b5eef11db..c3d3347e888 100644
--- a/lisp/eshell/esh-arg.el
+++ b/lisp/eshell/esh-arg.el
@@ -49,6 +49,8 @@ yield the values intended."
 (defvar eshell-arg-listified nil)
 (defvar eshell-nested-argument nil)
 (defvar eshell-current-quoted nil)
+(defvar eshell-current-argument-plain nil
+  "If non-nil, the current argument is \"plain\", and not part of a command.")
 (defvar eshell-inside-quote-regexp nil)
 (defvar eshell-outside-quote-regexp nil)
 
@@ -184,11 +186,6 @@ treated as a literal character."
     (add-hook 'pcomplete-try-first-hook
               #'eshell-complete-special-reference nil t)))
 
-(defun eshell-insert-buffer-name (buffer-name)
-  "Insert BUFFER-NAME into the current buffer at point."
-  (interactive "BName of buffer: ")
-  (insert-and-inherit "#<buffer " buffer-name ">"))
-
 (defsubst eshell-escape-arg (string)
   "Return STRING with the `escaped' property on it."
   (if (stringp string)
@@ -505,42 +502,6 @@ leaves point where it was."
         (goto-char bound)
         (apply #'concat (nreverse strings))))))
 
-(defun eshell-parse-special-reference ()
-  "Parse a special syntax reference, of the form `#<args>'.
-
-args           := `type' `whitespace' `arbitrary-args' | `arbitrary-args'
-type           := \"buffer\" or \"process\"
-arbitrary-args := any string of characters.
-
-If the form has no `type', the syntax is parsed as if `type' were
-\"buffer\"."
-  (when (and (not eshell-current-argument)
-             (not eshell-current-quoted)
-             (looking-at (rx "#<" (? (group (or "buffer" "process"))
-                                     space))))
-    (let ((here (point)))
-      (goto-char (match-end 0)) ;; Go to the end of the match.
-      (let ((buffer-p (if (match-beginning 1)
-                          (equal (match-string 1) "buffer")
-                        t)) ; With no type keyword, assume we want a buffer.
-            (end (eshell-find-delimiter ?\< ?\>)))
-        (when (not end)
-          (when (match-beginning 1)
-            (goto-char (match-beginning 1)))
-          (throw 'eshell-incomplete "#<"))
-        (if (eshell-arg-delimiter (1+ end))
-            (prog1
-                (list (if buffer-p #'get-buffer-create #'get-process)
-                      ;; FIXME: We should probably parse this as a
-                      ;; real Eshell argument so that we get the
-                      ;; benefits of quoting, variable-expansion, etc.
-                      (string-trim-right
-                       (replace-regexp-in-string
-                        (rx "\\" (group anychar)) "\\1"
-                        (buffer-substring-no-properties (point) end))))
-              (goto-char (1+ end)))
-          (ignore (goto-char here)))))))
-
 (defun eshell-parse-delimiter ()
   "Parse an argument delimiter, which is essentially a command operator."
   ;; this `eshell-operator' keyword gets parsed out by
@@ -591,7 +552,38 @@ If no argument requested a splice, return nil."
     (when splicep
       grouped-args)))
 
-;;;_* Special ref completion
+;;; Special references
+
+(defun eshell-parse-special-reference ()
+  "Parse a special syntax reference, of the form `#<args>'.
+
+args           := `type' `whitespace' `arbitrary-args' | `arbitrary-args'
+type           := \"buffer\" or \"process\"
+arbitrary-args := any number of Eshell arguments
+
+If the form has no `type', the syntax is parsed as if `type' were
+\"buffer\"."
+  (when (and (not eshell-current-argument)
+             (not eshell-current-quoted)
+             (looking-at (rx "#<" (? (group (or "buffer" "process"))
+                                     space))))
+    (let ((here (point)))
+      (goto-char (match-end 0)) ;; Go to the end of the match.
+      (let ((buffer-p (if (match-beginning 1)
+                          (equal (match-string 1) "buffer")
+                        t)) ; With no type keyword, assume we want a buffer.
+            (end (eshell-find-delimiter ?\< ?\>)))
+        (when (not end)
+          (when (match-beginning 1)
+            (goto-char (match-beginning 1)))
+          (throw 'eshell-incomplete "#<"))
+        (if (eshell-arg-delimiter (1+ end))
+            (prog1
+                (cons (if buffer-p #'eshell-get-buffer #'get-process)
+                      (let ((eshell-current-argument-plain t))
+                        (eshell-parse-arguments (point) end)))
+              (goto-char (1+ end)))
+          (ignore (goto-char here)))))))
 
 (defun eshell-complete-special-reference ()
   "If there is a special reference, complete it."
@@ -627,5 +619,16 @@ If no argument requested a splice, return nil."
         (throw 'pcomplete-completions
                (all-completions pcomplete-stub all-results))))))
 
+(defun eshell-get-buffer (buffer-or-name)
+  "Return the buffer specified by BUFFER-OR-NAME, creating a new one if needed.
+This is equivalent to `get-buffer-create', but only accepts a
+single argument."
+  (get-buffer-create buffer-or-name))
+
+(defun eshell-insert-buffer-name (buffer-name)
+  "Insert BUFFER-NAME into the current buffer at point."
+  (interactive "BName of buffer: ")
+  (insert-and-inherit "#<buffer " (eshell-quote-argument buffer-name) ">"))
+
 (provide 'esh-arg)
 ;;; esh-arg.el ends here
diff --git a/lisp/eshell/esh-io.el b/lisp/eshell/esh-io.el
index d0f1e04e925..c29b96dd711 100644
--- a/lisp/eshell/esh-io.el
+++ b/lisp/eshell/esh-io.el
@@ -196,7 +196,8 @@ describing the mode, e.g. for using with `eshell-get-target'.")
 
 (defun eshell-parse-redirection ()
   "Parse an output redirection, such as `2>' or `>&'."
-  (when (not eshell-current-quoted)
+  (unless (or eshell-current-quoted
+              eshell-current-argument-plain)
     (cond
      ;; Copying a handle (e.g. `2>&1').
      ((looking-at (rx (? (group digit))
diff --git a/lisp/eshell/esh-proc.el b/lisp/eshell/esh-proc.el
index 3c946c22bdc..ea5896461b4 100644
--- a/lisp/eshell/esh-proc.el
+++ b/lisp/eshell/esh-proc.el
@@ -227,23 +227,6 @@ and signal names."
 
 (put 'eshell/kill 'eshell-no-numeric-conversions t)
 
-(defun eshell-read-process-name (prompt)
-  "Read the name of a process from the minibuffer, using completion.
-The prompt will be set to PROMPT."
-  (completing-read prompt
-                   (mapcar
-                    (lambda (proc)
-                      (cons (process-name proc) t))
-                    (process-list))
-                   nil t))
-
-(defun eshell-insert-process (process)
-  "Insert the name of PROCESS into the current buffer at point."
-  (interactive
-   (list (get-process
-          (eshell-read-process-name "Name of process: "))))
-  (insert-and-inherit "#<process " (process-name process) ">"))
-
 (defsubst eshell-record-process-object (object)
   "Record OBJECT as now running."
   (when (and eshell-subjob-messages
@@ -695,5 +678,26 @@ everything."
 ;    ;; `eshell-resume-eval'.
 ;    (eshell--reset-after-signal "continue\n")))
 
+;;; Special references
+
+(defun eshell-read-process-name (prompt)
+  "Read the name of a process from the minibuffer, using completion.
+The prompt will be set to PROMPT."
+  (completing-read prompt
+                   (mapcar
+                    (lambda (proc)
+                      (cons (process-name proc) t))
+                    (process-list))
+                   nil t))
+
+(defun eshell-insert-process (process)
+  "Insert the name of PROCESS into the current buffer at point."
+  (interactive
+   (list (get-process
+          (eshell-read-process-name "Name of process: "))))
+  (insert-and-inherit "#<process "
+                      (eshell-quote-argument (process-name process))
+                      ">"))
+
 (provide 'esh-proc)
 ;;; esh-proc.el ends here
diff --git a/test/lisp/eshell/em-extpipe-tests.el b/test/lisp/eshell/em-extpipe-tests.el
index bdffcd9b320..6984ec2de59 100644
--- a/test/lisp/eshell/em-extpipe-tests.el
+++ b/test/lisp/eshell/em-extpipe-tests.el
@@ -55,7 +55,9 @@
                              "temp\\([^>]\\|\\'\\)" temp
                              (string-replace
                               "#<buffer temp>"
-                              (concat "#<buffer " (buffer-name temp-buffer) ">")
+                              (format "#<buffer %s>"
+                                      (eshell-quote-argument
+                                       (buffer-name temp-buffer)))
                               input))))
                        ,@body)
                    (when (buffer-name temp-buffer)
@@ -110,7 +112,7 @@
    '(progn
       (ignore
        (eshell-set-output-handle 1 'overwrite
-                                 (get-buffer-create "temp")))
+                                 (eshell-get-buffer "temp")))
       (eshell-named-command "sh"
                             (list "-c" "echo \"bar\" | rev"))))
   (with-substitute-for-temp
@@ -133,7 +135,7 @@
    '(progn
       (ignore
        (eshell-set-output-handle 1 'overwrite
-                                 (get-buffer-create "quux")))
+                                 (eshell-get-buffer "quux")))
       (ignore
        (eshell-set-output-handle 1 'append
                                  (get-process "other")))
diff --git a/test/lisp/eshell/esh-arg-tests.el b/test/lisp/eshell/esh-arg-tests.el
index c883db3907f..0e07d107562 100644
--- a/test/lisp/eshell/esh-arg-tests.el
+++ b/test/lisp/eshell/esh-arg-tests.el
@@ -118,7 +118,30 @@ treated literally, as a backslash and a newline."
      (format "echo #<buffer %s>" (buffer-name))
      (current-buffer))))
 
-(ert-deftest esh-arg-test/special-reference/special ()
+(ert-deftest esh-arg-test/special-reference/quoted ()
+  "Test that '#<buffer \"foo bar\">' refers to the buffer \"foo bar\"."
+  (with-temp-buffer
+    (rename-buffer "foo bar" t)
+    (eshell-command-result-equal
+     (format "echo #<buffer \"%s\">" (buffer-name))
+     (current-buffer))
+    (eshell-command-result-equal
+     (format "echo #<buffer '%s'>" (buffer-name))
+     (current-buffer))))
+
+(ert-deftest esh-arg-test/special-reference/var-expansion ()
+  "Test that variable expansion inside special references works."
+  (with-temp-buffer
+    (rename-buffer "my-buffer" t)
+    (let ((eshell-test-value (buffer-name)))
+      (eshell-command-result-equal
+       "echo #<buffer $eshell-test-value>"
+       (current-buffer))
+      (eshell-command-result-equal
+       "echo #<buffer \"$eshell-test-value\">"
+       (current-buffer)))))
+
+(ert-deftest esh-arg-test/special-reference/special-characters ()
   "Test that \"#<...>\" works correctly when escaping special characters."
   (with-temp-buffer
     (rename-buffer "<my buffer>" t)