master 17bf6a829c 3/3: Simplify handling of /dev/null redirection in Eshell

[Jim Porter]

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

    Simplify handling of /dev/null redirection in Eshell
    
    This also fixes an issue where "echo hi > foo > /dev/null" didn't
    write to the file "foo".  (Note that users can still use their
    system's null device name when redirecting; Eshell doesn't need to do
    anything special to support that.)
    
    * lisp/eshell/esh-io.el (eshell-virtual-targets): Add "/dev/null".
    (eshell-set-output-handle): Handle 'eshell-null-device'.
    
    * test/lisp/eshell/esh-io-tests.el
    (esh-io-test/redirect-subcommands/dev-null)
    (esh-io-test/virtual/dev-null, esh-io-test/virtual/dev-null/multiple):
    New tests.
---
 lisp/eshell/esh-io.el            | 54 ++++++++++++++++++++--------------------
 test/lisp/eshell/esh-io-tests.el | 33 ++++++++++++++++++++++--
 2 files changed, 58 insertions(+), 29 deletions(-)

diff --git a/lisp/eshell/esh-io.el b/lisp/eshell/esh-io.el
index 58084db28a..f2bc87374c 100644
--- a/lisp/eshell/esh-io.el
+++ b/lisp/eshell/esh-io.el
@@ -116,16 +116,22 @@ from executing while Emacs is redisplaying."
   :group 'eshell-io)
 
 (defcustom eshell-virtual-targets
-  '(("/dev/eshell" eshell-interactive-print nil)
+  '(;; The literal string "/dev/null" is intentional here.  It just
+    ;; provides compatibility so that users can redirect to
+    ;; "/dev/null" no matter the actual value of `null-device'.
+    ("/dev/null" (lambda (_mode) (throw 'eshell-null-device t)) t)
+    ("/dev/eshell" eshell-interactive-print nil)
     ("/dev/kill" (lambda (mode)
-                  (if (eq mode 'overwrite)
-                      (kill-new ""))
-                  'eshell-kill-append) t)
+                   (when (eq mode 'overwrite)
+                     (kill-new ""))
+                   #'eshell-kill-append)
+     t)
     ("/dev/clip" (lambda (mode)
-                  (if (eq mode 'overwrite)
-                      (let ((select-enable-clipboard t))
-                        (kill-new "")))
-                  'eshell-clipboard-append) t))
+                   (when (eq mode 'overwrite)
+                     (let ((select-enable-clipboard t))
+                       (kill-new "")))
+                   #'eshell-clipboard-append)
+     t))
   "Map virtual devices name to Emacs Lisp functions.
 If the user specifies any of the filenames above as a redirection
 target, the function in the second element will be called.
@@ -138,10 +144,8 @@ function.
 
 The output function is then called repeatedly with single strings,
 which represents successive pieces of the output of the command, until nil
-is passed, meaning EOF.
-
-NOTE: /dev/null is handled specially as a virtual target, and should
-not be added to this variable."
+is passed, meaning EOF."
+  :version "30.1"
   :type '(repeat
          (list (string :tag "Target")
                function
@@ -357,21 +361,17 @@ the value already set in `eshell-last-command-result'."
   "Set handle INDEX for the current HANDLES to point to TARGET using MODE.
 If HANDLES is nil, use `eshell-current-handles'."
   (when target
-    (let ((handles (or handles eshell-current-handles)))
-      (if (and (stringp target)
-               (string= target (null-device)))
-          (aset handles index nil)
-        (let* ((where (eshell-get-target target mode))
-               (handle (or (aref handles index)
-                           (aset handles index (list nil nil 1))))
-               (current (car handle))
-               (defaultp (cadr handle)))
-          (if (not defaultp)
-              (unless (member where current)
-                (setq current (append current (list where))))
-            (setq current (list where)))
-          (setcar handle current)
-          (setcar (cdr handle) nil))))))
+    (let* ((handles (or handles eshell-current-handles))
+           (handle (or (aref handles index)
+                       (aset handles index (list nil nil 1))))
+           (defaultp (cadr handle))
+           (current (unless defaultp (car handle))))
+      (catch 'eshell-null-device
+        (let ((where (eshell-get-target target mode)))
+          (unless (member where current)
+            (setq current (append current (list where))))))
+      (setcar handle current)
+      (setcar (cdr handle) nil))))
 
 (defun eshell-copy-output-handle (index index-to-copy &optional handles)
   "Copy the handle INDEX-TO-COPY to INDEX for the current HANDLES.
diff --git a/test/lisp/eshell/esh-io-tests.el b/test/lisp/eshell/esh-io-tests.el
index ccf8ac1b9a..9a3c14f365 100644
--- a/test/lisp/eshell/esh-io-tests.el
+++ b/test/lisp/eshell/esh-io-tests.el
@@ -166,6 +166,17 @@ ensure only its statement is redirected."
       (should (equal (buffer-string) "bar")))
     (should (equal (buffer-string) "foobaz"))))
 
+(ert-deftest esh-io-test/redirect-subcommands/dev-null ()
+  "Check that redirecting subcommands applies to all subcommands.
+Include a redirect to /dev/null to ensure it only applies to its
+statement."
+  (eshell-with-temp-buffer bufname "old"
+    (with-temp-eshell
+     (eshell-insert-command
+      (format "{echo foo; echo bar > /dev/null; echo baz} > #<%s>"
+              bufname)))
+    (should (equal (buffer-string) "foobaz"))))
+
 (ert-deftest esh-io-test/redirect-subcommands/interpolated ()
   "Check that redirecting interpolated subcommands applies to all subcommands."
   (eshell-with-temp-buffer bufname "old"
@@ -302,12 +313,30 @@ stdout originally pointed (the terminal)."
 
 ;; Virtual targets
 
-(ert-deftest esh-io-test/virtual-dev-eshell ()
+(ert-deftest esh-io-test/virtual/dev-null ()
+  "Check that redirecting to /dev/null works."
+  (with-temp-eshell
+   (eshell-match-command-output "echo hi > /dev/null" "\\`\\'")))
+
+(ert-deftest esh-io-test/virtual/dev-null/multiple ()
+  "Check that redirecting to /dev/null works alongside other redirections."
+  (eshell-with-temp-buffer bufname "old"
+    (with-temp-eshell
+     (eshell-match-command-output
+      (format "echo new > /dev/null > #<%s>" bufname) "\\`\\'"))
+    (should (equal (buffer-string) "new")))
+  (eshell-with-temp-buffer bufname "old"
+    (with-temp-eshell
+     (eshell-match-command-output
+      (format "echo new > #<%s> > /dev/null" bufname) "\\`\\'"))
+    (should (equal (buffer-string) "new"))))
+
+(ert-deftest esh-io-test/virtual/dev-eshell ()
   "Check that redirecting to /dev/eshell works."
   (with-temp-eshell
    (eshell-match-command-output "echo hi > /dev/eshell" "hi")))
 
-(ert-deftest esh-io-test/virtual-dev-kill ()
+(ert-deftest esh-io-test/virtual/dev-kill ()
   "Check that redirecting to /dev/kill works."
   (with-temp-eshell
    (eshell-insert-command "echo one > /dev/kill")