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

[samer <samer@samertm.com>]

Hi friends, Charles, countrymen, and debbugs,

When eshell opens a visual command, it creates a buffer for the process,
switches to that buffer, and, if the process is still running, attaches
a sentinel to the process that kills the process when the sentinel dies.
If the process is not running, the visual command signals an error,
I assume because there is no good reason to show the term buffer for a
process that isn't running.

There is a race condition in the part that handles whether a command is
running: even short-lived commands, like "git --version", are running
when polled, and the error is never thrown. That means we've attached
the buffer-killing sentinel to these short-lived commands, so the buffer
they're attached to gets killed immediately afterwards.

To sum up everything wrong with the current behavior:

1. Killing the process's buffer when the process dies is not the desired
behavior.

2. Even if we wanted to kill the process's buffer for long running
processes but not short ones, there is *no way* to tell how long a
process will run. That means, there is no way to do this correctly.

The solution is simple: for every visual application, simply create the
term-mode buffer for it and switch to that buffer. This satisfies every
use case: if a process dies quickly with an error message, the user will
be able to read the error; if the process is running, it doesn't quit
(this is also the current behavior); if the process is long running and
dies, the user will be able to read the output of the process. I've
attached a patch that implements this behavior below.

There is a small issue with this patch: eshell does not start on a new
line when you execute a visual command, and so you need to press 'enter'
before entering a new command. If anyone has any insight into this, I'm
all ears, otherwise I can probably figure it out eventually.

This is a longer patch, and so it may need to wait for my papers to be
processed. I am not sure how long that will take, as it's been more than
a month since I first submitted my application.

-samer

Patch below:

Changes in 3e0d44a..b9f2247
2 files changed, 14 insertions(+), 37 deletions(-)
  ChangeLog              |    6 ++++++
  lisp/eshell/em-term.el |   45 
++++++++-------------------------------------

	Modified   ChangeLog
diff --git a/ChangeLog b/ChangeLog
index 36edfe6..d09380a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2015-01-19  Samer Masterson  <samer@samertm.com>
+
+	* eshell/em-term.el (eshell-exec-visual, eshell-term-sentinel):
+	Remove eshell-term-sentinel, show term-mode buffer regardless of
+	whether the process has died (bug#18108).
+
  2015-01-04  Paul Eggert  <eggert@cs.ucla.edu>

  	* INSTALL: Mention 'make WERROR_CFLAGS='.
	Modified   lisp/eshell/em-term.el
diff --git a/lisp/eshell/em-term.el b/lisp/eshell/em-term.el
index 4a6ac23..6870a6d 100644
--- a/lisp/eshell/em-term.el
+++ b/lisp/eshell/em-term.el
@@ -132,10 +132,6 @@ character to the invoked process."
    :type 'boolean
    :group 'eshell-term)

-;;; Internal Variables:
-
-(defvar eshell-parent-buffer)
-
  ;;; Functions:

  (defun eshell-term-initialize ()
@@ -171,39 +167,14 @@ allowed."
  					       (cdr args)))))
  	 (term-buf
  	  (generate-new-buffer
-	   (concat "*" (file-name-nondirectory program) "*")))
-	 (eshell-buf (current-buffer)))
-    (save-current-buffer
-      (switch-to-buffer term-buf)
-      (term-mode)
-      (set (make-local-variable 'term-term-name) eshell-term-name)
-      (make-local-variable 'eshell-parent-buffer)
-      (setq eshell-parent-buffer eshell-buf)
-      (term-exec term-buf program program nil args)
-      (let ((proc (get-buffer-process term-buf)))
-	(if (and proc (eq 'run (process-status proc)))
-	    (set-process-sentinel proc 'eshell-term-sentinel)
-	  (error "Failed to invoke visual command")))
-      (term-char-mode)
-      (if eshell-escape-control-x
-	  (term-set-escape-char ?\C-x))))
-  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))))
+	   (concat "*" (file-name-nondirectory program) "*"))))
+    (switch-to-buffer term-buf)
+    (term-mode)
+    (set (make-local-variable 'term-term-name) eshell-term-name)
+    (term-exec term-buf program program nil args)
+    (term-char-mode)
+    (if eshell-escape-control-x
+        (term-set-escape-char ?\C-x))))

  ;; jww (1999-09-17): The code below will allow Eshell to send input
  ;; characters directly to the currently running interactive process.