Subject: bug#54136: 29.0.50; Eshell emits extra prompts when killing processes in some cases

From: Jim Porter <[email-masked]>
Date: Wed, 23 Feb 2022 21:16:04 -0800

On 2/23/2022 9:11 PM, Jim Porter wrote:
> (Patch forthcoming; I'm just getting a bug number first...)

Ok, here's the patch with some tests.
From 52530c208d1b06cfbf527d77425bbbae1666b345 Mon Sep 17 00:00:00 2001
From: Jim Porter <[email-masked]>
Date: Wed, 23 Feb 2022 20:43:38 -0800
Subject: [PATCH] Don't superfluously emit prompts when terminating processes
 in Eshell

* lisp/eshell/esh-proc.el (eshell-kill-process-function): Only reset
the prompt if PROC is writing to the terminal.
(eshell-sentinel): Only write the exit message if PROC is writing to
the terminal (bug#54136).

* test/lisp/eshell/esh-proc-tests.el (esh-proc-test/kill-pipeline)
(esh-proc-test/kill-pipeline-head)
(esh-proc-test/kill-background-process): New tests.
---
 lisp/eshell/esh-proc.el            | 16 ++++++++---
 test/lisp/eshell/esh-proc-tests.el | 46 ++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/lisp/eshell/esh-proc.el b/lisp/eshell/esh-proc.el
index d7d22d2a9e..70426ccaf2 100644
--- a/lisp/eshell/esh-proc.el
+++ b/lisp/eshell/esh-proc.el
@@ -102,6 +102,7 @@ eshell-process-list
   "A list of the current status of subprocesses.")
 
 (declare-function eshell-send-eof-to-process "esh-mode")
+(declare-function eshell-tail-process "esh-cmd")
 
 (defvar-keymap eshell-proc-mode-map
   "C-c M-i"  #'eshell-insert-process
@@ -119,7 +120,9 @@ eshell-kill-process-function
 PROC and STATUS to functions on the latter."
   ;; Was there till 24.1, but it is not optional.
   (remove-hook 'eshell-kill-hook #'eshell-reset-after-proc)
-  (eshell-reset-after-proc status)
+  ;; Only reset the prompt if this process is running interactively.
+  (when (eq proc (eshell-tail-process))
+    (eshell-reset-after-proc status))
   (run-hook-with-args 'eshell-kill-hook proc status))
 
 (define-minor-mode eshell-proc-mode
@@ -414,7 +417,7 @@ eshell-sentinel
   (when (buffer-live-p (process-buffer proc))
     (with-current-buffer (process-buffer proc)
       (unwind-protect
-	  (let* ((entry (assq proc eshell-process-list)))
+          (let ((entry (assq proc eshell-process-list)))
 ;	    (if (not entry)
 ;		(error "Sentinel called for unowned process `%s'"
 ;		       (process-name proc))
@@ -422,8 +425,13 @@ eshell-sentinel
 	      (unwind-protect
 		  (progn
 		    (unless (string= string "run")
-		      (unless (string-match "^\\(finished\\|exited\\)" string)
-			(eshell-insertion-filter proc string))
+                      ;; Write the exit message if the status is
+                      ;; abnormal and the process is already writing
+                      ;; to the terminal.
+                      (when (and (eq proc (eshell-tail-process))
+                                 (not (string-match "^\\(finished\\|exited\\)"
+                                                    string)))
+                        (funcall (process-filter proc) proc string))
                       (let ((handles (nth 1 entry))
                             (str (prog1 (nth 3 entry)
                                    (setf (nth 3 entry) nil)))
diff --git a/test/lisp/eshell/esh-proc-tests.el b/test/lisp/eshell/esh-proc-tests.el
index e7ea6c00d6..a8be0f8030 100644
--- a/test/lisp/eshell/esh-proc-tests.el
+++ b/test/lisp/eshell/esh-proc-tests.el
@@ -43,3 +43,49 @@ esh-proc-test/sigpipe-exits-process
     "y\n")
    (eshell-wait-for-subprocess t)
    (should (eq (process-list) nil))))
+
+(ert-deftest esh-proc-test/kill-pipeline ()
+  "Test that killing a pipeline of processes only emits a single
+prompt.  See bug#54136."
+  (skip-unless (and (executable-find "sh")
+                    (executable-find "echo")
+                    (executable-find "sleep")))
+  (with-temp-eshell
+   (eshell-insert-command
+    (concat "sh -c 'while true; do echo y; sleep 1; done' | "
+            "sh -c 'while true; do read NAME; done'"))
+   (let ((output-start (eshell-beginning-of-output)))
+     (eshell-kill-process)
+     (eshell-wait-for-subprocess t)
+     (should (equal (buffer-substring-no-properties
+                     output-start (eshell-end-of-output))
+                    "killed\n")))))
+
+(ert-deftest esh-proc-test/kill-pipeline-head ()
+  "Test that killing the first process in a pipeline doesn't
+write the exit status to the pipe.  See bug#54136."
+  (skip-unless (and (executable-find "sh")
+                    (executable-find "echo")
+                    (executable-find "sleep")))
+  (with-temp-eshell
+   (eshell-insert-command
+    (concat "sh -c 'while true; sleep 1; done' | "
+            "sh -c 'while read NAME; do echo =${NAME}=; done'"))
+   (let ((output-start (eshell-beginning-of-output)))
+     (kill-process (eshell-head-process))
+     (eshell-wait-for-subprocess t)
+     (should (equal (buffer-substring-no-properties
+                     output-start (eshell-end-of-output))
+                    "")))))
+
+(ert-deftest esh-proc-test/kill-background-process ()
+  "Test that killing a background process doesn't emit a new
+prompt.  See bug#54136."
+  (skip-unless (and (executable-find "sh")
+                    (executable-find "sleep")))
+  (with-temp-eshell
+   (eshell-insert-command "sh -c 'while true; do sleep 1; done' &")
+   (kill-process (caar eshell-process-list))
+   ;; Give `eshell-sentinel' a chance to run.
+   (sit-for 0.1)
+   (eshell-match-result "\\[sh\\] [[:digit:]]+\n")))
-- 
2.25.1