all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Jim Porter <jporterbugs@gmail.com>
To: 54136@debbugs.gnu.org
Subject: bug#54136: 29.0.50; Eshell emits extra prompts when killing processes in some cases
Date: Wed, 23 Feb 2022 21:16:04 -0800	[thread overview]
Message-ID: <1157b5f3-a393-1c2d-7f7d-2c01d4e7f1e3@gmail.com> (raw)
In-Reply-To: <1bfc8b00-0062-70b6-6492-01cf3947e55b@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 141 bytes --]

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.

[-- Attachment #2: 0001-Don-t-superfluously-emit-prompts-when-terminating-pr.patch --]
[-- Type: text/plain, Size: 5244 bytes --]

From 52530c208d1b06cfbf527d77425bbbae1666b345 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
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


  reply	other threads:[~2022-02-24  5:16 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-24  5:11 bug#54136: 29.0.50; Eshell emits extra prompts when killing processes in some cases Jim Porter
2022-02-24  5:16 ` Jim Porter [this message]
2022-02-24  9:35   ` Lars Ingebrigtsen
2022-02-24 11:03     ` Eli Zaretskii
2022-02-24 18:55       ` Jim Porter
2022-02-24 20:03         ` Eli Zaretskii
2022-02-24 20:07           ` Lars Ingebrigtsen
2022-02-25  1:04             ` Jim Porter
2022-02-25  2:18               ` Lars Ingebrigtsen
2022-02-25  7:09               ` Eli Zaretskii
2022-02-25 18:31                 ` Jim Porter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1157b5f3-a393-1c2d-7f7d-2c01d4e7f1e3@gmail.com \
    --to=jporterbugs@gmail.com \
    --cc=54136@debbugs.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.