bug#18108: [PATCH] 24.3.92 : eshell-visual-options fails with some output.

[Samer Masterson <samer@samertm.com>]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> No doubt.  I think the best way to try and make it work well in "both"
> cases is to try and make the behavior closer to what would happen inside
> an xterm (say).  E.g. one possibility is that when the process dies,
> rather than simply deleting the corresponding term buffer, we first copy
> the term buffer's contents to the Eshell's buffer, so as not to lose
> this process output.
> WDYT?

I agree, it's best to emulate xterm when we can. In this case, though,
visual programs are pretty different from "visual" commands in a normal
xterm. The solution you suggested comes close, but visual commands are
"asynchronous" because they don't block the eshell buffer that spawned
them. So it might be confusing if you spawn a visual program, switch
back to eshell and forget about the visual program, and then later kill
the visual program and have its buffer spit into your eshell. It seems a
bit too action-at-a-distance for me, and at the end of the day, it's
more complexity for little gain.

An ideal solution would be to detect when a command is trying to control
the screen and then automatically jump into term mode. I'm not sure how
hard that would be, though.

FWIW, I've been using this patch for the last couple days and it feels
very natural. I've attached a patch that adds a custom var to get the
old behavior, tho the new behavior is default. Let me know if that's the
right direction.

Thanks! I hope you're doing well.
-s

Patch below:


From c58d82ddb2a2b893f39a926a0de98ff8195a8cad Mon Sep 17 00:00:00 2001
From: Samer Masterson <samer@samertm.com>
Date: Mon, 4 May 2015 05:50:57 -0700
Subject: [PATCH] Fix bug#18108

* lisp/eshell/em-term.el (eshell-destroy-buffer-when-process-dies):
New custom to preserve previous behavior.

* lisp/eshell/em-term.el (eshell-term-sentinel): No-op by default,
only kills term if `eshell-destroy-buffer-when-process-dies' is
non-nil.
---
 etc/NEWS               |  5 +++++
 lisp/eshell/em-term.el | 39 +++++++++++++++++++++++++--------------
 2 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 1193055..e779b1b 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -632,6 +632,11 @@ command line's password prompt.
 *** The new built-in command `clear' can scroll window contents out of sight.
 If provided with an optional non-nil argument, the scrollback contents will be cleared.
 
+*** By default, eshell "visual" program buffers (as defined by
+`eshell-custom-program' and the `eshell-term' group) are no longer
+killed when their processes die.  For the old behavior, set
+`eshell-destroy-buffer-when-process-dies' to non-nil.
+
 ** Browse-url
 
 *** Support for the Conkeror web browser.
diff --git a/lisp/eshell/em-term.el b/lisp/eshell/em-term.el
index 4a6ac23..9ac2813 100644
--- a/lisp/eshell/em-term.el
+++ b/lisp/eshell/em-term.el
@@ -132,6 +132,13 @@ character to the invoked process."
   :type 'boolean
   :group 'eshell-term)
 
+(defcustom eshell-destroy-buffer-when-process-dies nil
+  "If non-nil, term buffers are destroyed after their processes die.
+WARNING: Setting this to non-nil may result in unexpected
+behavior for short-lived processes, see bug#18108."
+  :type 'boolean
+  :group 'eshell-term)
+
 ;;; Internal Variables:
 
 (defvar eshell-parent-buffer)
@@ -190,20 +197,24 @@ allowed."
   nil)
 
 ;; Process sentinels receive two arguments.
-(defun eshell-term-sentinel (proc _string)
-  "Destroy the buffer visiting PROC."
-  (let ((proc-buf (process-buffer proc)))
-    (when (and proc-buf (buffer-live-p proc-buf)
-	       (not (eq 'run (process-status proc)))
-	       (= (process-exit-status proc) 0))
-      (if (eq (current-buffer) proc-buf)
-	  (let ((buf (and (boundp 'eshell-parent-buffer)
-			  eshell-parent-buffer
-			  (buffer-live-p eshell-parent-buffer)
-			  eshell-parent-buffer)))
-	    (if buf
-		(switch-to-buffer buf))))
-      (kill-buffer proc-buf))))
+(defun eshell-term-sentinel (proc msg)
+  "Clean up the buffer visiting PROC.
+If `eshell-destroy-buffer-when-process-dies' is non-nil, destroy
+the buffer."
+  (term-sentinel proc msg) ;; First call the normal term sentinel.
+  (when eshell-destroy-buffer-when-process-dies
+    (let ((proc-buf (process-buffer proc)))
+      (when (and proc-buf (buffer-live-p proc-buf)
+                 (not (eq 'run (process-status proc)))
+                 (= (process-exit-status proc) 0))
+        (if (eq (current-buffer) proc-buf)
+            (let ((buf (and (boundp 'eshell-parent-buffer)
+                            eshell-parent-buffer
+                            (buffer-live-p eshell-parent-buffer)
+                            eshell-parent-buffer)))
+              (if buf
+                  (switch-to-buffer buf))))
+        (kill-buffer proc-buf)))))
 
 ;; jww (1999-09-17): The code below will allow Eshell to send input
 ;; characters directly to the currently running interactive process.