bug#55426: [PATCH] Add option to kill a shell buffer when the process ends

bug#55426: [PATCH] Add option to kill a shell buffer when the process ends

[Philip Kaludercic]

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

Tags: patch


The below patch adds an option that has a *shell* buffer automatically
killed when a shell process terminates.  I find this useful, because it
helps to avoid re-activating old shell buffers in the wrong
`default-directory'.  Eshell has a "similar" behaviour (setting aside
that it doesn't have an inferior process), but term doesn't.  If this
patch is merged, perhaps something similar for term could be done too?`

In GNU Emacs 29.0.50 (build 12, x86_64-pc-linux-gnu, GTK+ Version 2.24.33, cairo version 1.16.0)
 of 2022-05-11 built on icterid
Repository revision: 474241f356c638bfd8d4eecb7138e3af76a8c036
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Debian GNU/Linux 11 (bullseye)

Configured using:
 'configure --with-tree-sitter'


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-option-to-kill-a-shell-buffer-when-the-process-e.patch --]
[-- Type: text/patch, Size: 1461 bytes --]

From 21962d1e55db25c3e31a7efc722f91667bcd2319 Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk@posteo.net>
Date: Sun, 15 May 2022 02:09:46 +0200
Subject: [PATCH] Add option to kill a shell buffer when the process ends

* shell.el (shell-kill-buffer-on-quit): Add new option.
(shell): Respect `shell-kill-buffer-on-quit'.
---
 lisp/shell.el | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/lisp/shell.el b/lisp/shell.el
index 47887433d9..d7d0c487a0 100644
--- a/lisp/shell.el
+++ b/lisp/shell.el
@@ -331,6 +331,12 @@ shell-has-auto-cd
   :group 'shell-directories
   :version "28.1")
 
+(defcustom shell-kill-buffer-on-quit t
+  "Kill shell buffer after the process terminates."
+  :type 'boolean
+  :group 'shell
+  :version "29.1")
+
 (defvar shell-mode-map
   (let ((map (make-sparse-keymap)))
     (define-key map "\C-c\C-f" 'shell-forward-command)
@@ -818,6 +824,16 @@ shell
           (with-temp-buffer
             (insert-file-contents startfile)
             (buffer-string)))))))
+  (when shell-kill-buffer-on-quit
+    (let* ((buffer (current-buffer))
+           (sentinel (process-sentinel (get-buffer-process buffer))))
+      (set-process-sentinel
+       (get-buffer-process (current-buffer))
+       (lambda (proc event)
+         (unless (buffer-live-p proc)
+           (kill-buffer buffer))
+         (when sentinel
+           (funcall sentinel proc event))))))
   buffer)
 
 ;;; Directory tracking
-- 
2.30.2

bug#55426: [PATCH] Add option to kill a shell buffer when the process ends

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Date: Sun, 15 May 2022 08:13:49 +0000
> 
> The below patch adds an option that has a *shell* buffer automatically
> killed when a shell process terminates.  I find this useful, because it
> helps to avoid re-activating old shell buffers in the wrong
> `default-directory'.

Thanks, a few comments below.

> +(defcustom shell-kill-buffer-on-quit t

I think a better name is shell-kill-shell-buffer-on-exit.

> +  "Kill shell buffer after the process terminates."

"Kill the shell buffer when the shell process terminates."

> +  (when shell-kill-buffer-on-quit
> +    (let* ((buffer (current-buffer))
> +           (sentinel (process-sentinel (get-buffer-process buffer))))
> +      (set-process-sentinel
> +       (get-buffer-process (current-buffer))
> +       (lambda (proc event)
> +         (unless (buffer-live-p proc)
> +           (kill-buffer buffer))
> +         (when sentinel
> +           (funcall sentinel proc event))))))

Shouldn't we call the previous sentinel before killing the buffer?
That sentinel could not be prepared to the buffer being dead.

Also, can you modify the code to call get-buffer-process only once?

> If this patch is merged, perhaps something similar for term could be
> done too?

I'm not sure, but I don't use term frequently enough for my opinion to
matter.

bug#55426: [PATCH] Add option to kill a shell buffer when the process ends

[Eli Zaretskii]

> Cc: 55426@debbugs.gnu.org
> Date: Sun, 15 May 2022 11:30:17 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > From: Philip Kaludercic <philipk@posteo.net>
> > Date: Sun, 15 May 2022 08:13:49 +0000
> > 
> > The below patch adds an option that has a *shell* buffer automatically
> > killed when a shell process terminates.  I find this useful, because it
> > helps to avoid re-activating old shell buffers in the wrong
> > `default-directory'.
> 
> Thanks, a few comments below.

Oh, and one more: I think this change warrants a NEWS entry.

bug#55426: [PATCH] Add option to kill a shell buffer when the process ends

[Philip Kaludercic]

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


Thank you for your comments, here is an improved version:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-option-to-kill-a-shell-buffer-when-the-process-e.patch --]
[-- Type: text/x-diff, Size: 2051 bytes --]

From 7337f6862cdb6954498282b9c9d22f6b2cd3457d Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk@posteo.net>
Date: Sun, 15 May 2022 02:09:46 +0200
Subject: [PATCH] Add option to kill a shell buffer when the process ends

* shell.el (shell-kill-buffer-on-quit): Add new option (bug#55426).
(shell): Respect 'shell-kill-buffer-on-quit'.
* NEWS: Mention 'shell-kill-buffer-on-quit'.
---
 etc/NEWS      |  7 +++++++
 lisp/shell.el | 17 +++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/etc/NEWS b/etc/NEWS
index a0164bbf3f..9ef83189dd 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1544,6 +1544,13 @@ values passed as a single token, such as '-oVALUE' or
 'eshell-eval-using-options' macro.  See "Defining new built-in
 commands" in the "(eshell) Built-ins" node of the Eshell manual.
 
+** Shell
+
+---
+*** New user option 'shell-kill-buffer-on-exit'
+Enabling this open will automatically kill a *shell* buffer as soon as
+the shell session terminates.
+
 ** Calc
 
 +++
diff --git a/lisp/shell.el b/lisp/shell.el
index 47887433d9..7000d38255 100644
--- a/lisp/shell.el
+++ b/lisp/shell.el
@@ -331,6 +331,12 @@ shell-has-auto-cd
   :group 'shell-directories
   :version "28.1")
 
+(defcustom shell-kill-buffer-on-exit t
+  "Kill a shell buffer after the shell process terminates."
+  :type 'boolean
+  :group 'shell
+  :version "29.1")
+
 (defvar shell-mode-map
   (let ((map (make-sparse-keymap)))
     (define-key map "\C-c\C-f" 'shell-forward-command)
@@ -818,6 +824,17 @@ shell
           (with-temp-buffer
             (insert-file-contents startfile)
             (buffer-string)))))))
+  (when shell-kill-buffer-on-exit
+    (let* ((buffer (current-buffer))
+           (process (get-buffer-process buffer))
+           (sentinel (process-sentinel process)))
+      (set-process-sentinel
+       process
+       (lambda (proc event)
+         (when sentinel
+           (funcall sentinel proc event))
+         (unless (buffer-live-p proc)
+           (kill-buffer buffer))))))
   buffer)
 
 ;;; Directory tracking
-- 
2.30.2


[-- Attachment #3: Type: text/plain, Size: 2192 bytes --]


Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Date: Sun, 15 May 2022 08:13:49 +0000
>> 
>> The below patch adds an option that has a *shell* buffer automatically
>> killed when a shell process terminates.  I find this useful, because it
>> helps to avoid re-activating old shell buffers in the wrong
>> `default-directory'.
>
> Thanks, a few comments below.
>
>> +(defcustom shell-kill-buffer-on-quit t
>
> I think a better name is shell-kill-shell-buffer-on-exit.
>
>> +  "Kill shell buffer after the process terminates."
>
> "Kill the shell buffer when the shell process terminates."

        ^
        I replaced this with "a", to avoid confusion as to what specific
        shell buffer is meant (not only *shell*, but any shell-mode buffer).  

>> +  (when shell-kill-buffer-on-quit
>> +    (let* ((buffer (current-buffer))
>> +           (sentinel (process-sentinel (get-buffer-process buffer))))
>> +      (set-process-sentinel
>> +       (get-buffer-process (current-buffer))
>> +       (lambda (proc event)
>> +         (unless (buffer-live-p proc)
>> +           (kill-buffer buffer))
>> +         (when sentinel
>> +           (funcall sentinel proc event))))))
>
> Shouldn't we call the previous sentinel before killing the buffer?
> That sentinel could not be prepared to the buffer being dead.
>
> Also, can you modify the code to call get-buffer-process only once?
>
>> If this patch is merged, perhaps something similar for term could be
>> done too?
>
> I'm not sure, but I don't use term frequently enough for my opinion to
> matter.

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: 55426@debbugs.gnu.org
>> Date: Sun, 15 May 2022 11:30:17 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>> 
>> > From: Philip Kaludercic <philipk@posteo.net>
>> > Date: Sun, 15 May 2022 08:13:49 +0000
>> > 
>> > The below patch adds an option that has a *shell* buffer automatically
>> > killed when a shell process terminates.  I find this useful, because it
>> > helps to avoid re-activating old shell buffers in the wrong
>> > `default-directory'.
>> 
>> Thanks, a few comments below.
>
> Oh, and one more: I think this change warrants a NEWS entry.

bug#55426: [PATCH] Add option to kill a shell buffer when the process ends

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: 55426@debbugs.gnu.org
> Date: Sun, 15 May 2022 11:06:42 +0000
> 
> Thank you for your comments, here is an improved version:

Thanks.  One nit:

> +** Shell
> +
> +---
> +*** New user option 'shell-kill-buffer-on-exit'
> +Enabling this open will automatically kill a *shell* buffer as soon as
                 ^^^^
Seems like a leftover from a different version?

bug#55426: [PATCH] Add option to kill a shell buffer when the process ends

[Andreas Schwab]

On Mai 15 2022, Philip Kaludercic wrote:

> diff --git a/lisp/shell.el b/lisp/shell.el
> index 47887433d9..7000d38255 100644
> --- a/lisp/shell.el
> +++ b/lisp/shell.el
> @@ -331,6 +331,12 @@ shell-has-auto-cd
>    :group 'shell-directories
>    :version "28.1")
>  
> +(defcustom shell-kill-buffer-on-exit t

That should default to nil.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

bug#55426: [PATCH] Add option to kill a shell buffer when the process ends

[Philip Kaludercic]

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


Fixed both here:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-option-to-kill-a-shell-buffer-when-the-process-e.patch --]
[-- Type: text/x-diff, Size: 2048 bytes --]

From 6db6675c1f931b0eed6606227ecd4919a7052afd Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk@posteo.net>
Date: Sun, 15 May 2022 02:09:46 +0200
Subject: [PATCH] Add option to kill a shell buffer when the process ends

* shell.el (shell-kill-buffer-on-quit): Add new option (bug#55426).
(shell): Respect 'shell-kill-buffer-on-quit'.
* NEWS: Mention 'shell-kill-buffer-on-quit'.
---
 etc/NEWS      |  7 +++++++
 lisp/shell.el | 17 +++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/etc/NEWS b/etc/NEWS
index a0164bbf3f..27772dc54e 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1544,6 +1544,13 @@ values passed as a single token, such as '-oVALUE' or
 'eshell-eval-using-options' macro.  See "Defining new built-in
 commands" in the "(eshell) Built-ins" node of the Eshell manual.
 
+** Shell
+
+---
+*** New user option 'shell-kill-buffer-on-exit'
+Enabling this will automatically kill a *shell* buffer as soon as the
+shell session terminates.
+
 ** Calc
 
 +++
diff --git a/lisp/shell.el b/lisp/shell.el
index 47887433d9..4e65fccf9e 100644
--- a/lisp/shell.el
+++ b/lisp/shell.el
@@ -331,6 +331,12 @@ shell-has-auto-cd
   :group 'shell-directories
   :version "28.1")
 
+(defcustom shell-kill-buffer-on-exit nil
+  "Kill a shell buffer after the shell process terminates."
+  :type 'boolean
+  :group 'shell
+  :version "29.1")
+
 (defvar shell-mode-map
   (let ((map (make-sparse-keymap)))
     (define-key map "\C-c\C-f" 'shell-forward-command)
@@ -818,6 +824,17 @@ shell
           (with-temp-buffer
             (insert-file-contents startfile)
             (buffer-string)))))))
+  (when shell-kill-buffer-on-exit
+    (let* ((buffer (current-buffer))
+           (process (get-buffer-process buffer))
+           (sentinel (process-sentinel process)))
+      (set-process-sentinel
+       process
+       (lambda (proc event)
+         (when sentinel
+           (funcall sentinel proc event))
+         (unless (buffer-live-p proc)
+           (kill-buffer buffer))))))
   buffer)
 
 ;;; Directory tracking
-- 
2.30.2


[-- Attachment #3: Type: text/plain, Size: 895 bytes --]


Should it push it?

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Cc: 55426@debbugs.gnu.org
>> Date: Sun, 15 May 2022 11:06:42 +0000
>> 
>> Thank you for your comments, here is an improved version:
>
> Thanks.  One nit:
>
>> +** Shell
>> +
>> +---
>> +*** New user option 'shell-kill-buffer-on-exit'
>> +Enabling this open will automatically kill a *shell* buffer as soon as
>                  ^^^^
> Seems like a leftover from a different version?

Andreas Schwab <schwab@linux-m68k.org> writes:

> On Mai 15 2022, Philip Kaludercic wrote:
>
>> diff --git a/lisp/shell.el b/lisp/shell.el
>> index 47887433d9..7000d38255 100644
>> --- a/lisp/shell.el
>> +++ b/lisp/shell.el
>> @@ -331,6 +331,12 @@ shell-has-auto-cd
>>    :group 'shell-directories
>>    :version "28.1")
>>  
>> +(defcustom shell-kill-buffer-on-exit t
>
> That should default to nil.

bug#55426: [PATCH] Add option to kill a shell buffer when the process ends

[Philip Kaludercic]

So is the patch fine?  May I push it?

Philip Kaludercic <philipk@posteo.net> writes:

> Fixed both here:
>
>>From 6db6675c1f931b0eed6606227ecd4919a7052afd Mon Sep 17 00:00:00 2001
> From: Philip Kaludercic <philipk@posteo.net>
> Date: Sun, 15 May 2022 02:09:46 +0200
> Subject: [PATCH] Add option to kill a shell buffer when the process ends
>
> * shell.el (shell-kill-buffer-on-quit): Add new option (bug#55426).
> (shell): Respect 'shell-kill-buffer-on-quit'.
> * NEWS: Mention 'shell-kill-buffer-on-quit'.
> ---
>  etc/NEWS      |  7 +++++++
>  lisp/shell.el | 17 +++++++++++++++++
>  2 files changed, 24 insertions(+)
>
> diff --git a/etc/NEWS b/etc/NEWS
> index a0164bbf3f..27772dc54e 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -1544,6 +1544,13 @@ values passed as a single token, such as '-oVALUE' or
>  'eshell-eval-using-options' macro.  See "Defining new built-in
>  commands" in the "(eshell) Built-ins" node of the Eshell manual.
>  
> +** Shell
> +
> +---
> +*** New user option 'shell-kill-buffer-on-exit'
> +Enabling this will automatically kill a *shell* buffer as soon as the
> +shell session terminates.
> +
>  ** Calc
>  
>  +++
> diff --git a/lisp/shell.el b/lisp/shell.el
> index 47887433d9..4e65fccf9e 100644
> --- a/lisp/shell.el
> +++ b/lisp/shell.el
> @@ -331,6 +331,12 @@ shell-has-auto-cd
>    :group 'shell-directories
>    :version "28.1")
>  
> +(defcustom shell-kill-buffer-on-exit nil
> +  "Kill a shell buffer after the shell process terminates."
> +  :type 'boolean
> +  :group 'shell
> +  :version "29.1")
> +
>  (defvar shell-mode-map
>    (let ((map (make-sparse-keymap)))
>      (define-key map "\C-c\C-f" 'shell-forward-command)
> @@ -818,6 +824,17 @@ shell
>            (with-temp-buffer
>              (insert-file-contents startfile)
>              (buffer-string)))))))
> +  (when shell-kill-buffer-on-exit
> +    (let* ((buffer (current-buffer))
> +           (process (get-buffer-process buffer))
> +           (sentinel (process-sentinel process)))
> +      (set-process-sentinel
> +       process
> +       (lambda (proc event)
> +         (when sentinel
> +           (funcall sentinel proc event))
> +         (unless (buffer-live-p proc)
> +           (kill-buffer buffer))))))
>    buffer)
>  
>  ;;; Directory tracking

bug#55426: [PATCH] Add option to kill a shell buffer when the process ends

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: 55426@debbugs.gnu.org
> Date: Sun, 15 May 2022 11:51:53 +0000
> 
> +*** New user option 'shell-kill-buffer-on-exit'

This should end with a period, to be a full sentence.

Otherwise, LGTM, thanks.

bug#55426: [PATCH] Add option to kill a shell buffer when the process ends

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: 55426@debbugs.gnu.org
> Date: Wed, 18 May 2022 23:13:05 +0000
> 
> 
> So is the patch fine?  May I push it?

Sorry for the delay.  I've posted a response just now with a small
correction; feel free to install after fixing that nit.

Thanks.

bug#55426: [PATCH] Add option to kill a shell buffer when the process ends

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Cc: 55426@debbugs.gnu.org
>> Date: Sun, 15 May 2022 11:51:53 +0000
>> 
>> +*** New user option 'shell-kill-buffer-on-exit'
>
> This should end with a period, to be a full sentence.
>
> Otherwise, LGTM, thanks.

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Cc: 55426@debbugs.gnu.org
>> Date: Wed, 18 May 2022 23:13:05 +0000
>> 
>> 
>> So is the patch fine?  May I push it?
>
> Sorry for the delay.  I've posted a response just now with a small
> correction; feel free to install after fixing that nit.

Done.

> Thanks.

Thank you for your help.