GNU bug report logs - #18108
24.3.92 : eshell-visual-options fails with some output.

Previous Next

Full log

View this message in rfc822 format

From: Samer Masterson <samer <at> samertm.com>
To: 18108 <at> debbugs.gnu.org
Subject: bug#18108: [PATCH] 24.3.92 : eshell-visual-options fails with some output.
Date: Sun, 05 Apr 2015 21:05:43 -0700

Hi,

Pinging the list with a well-formed patch. Info below (quoted from my reply to 18108):

 > 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.

And the patch is below.

-samer

ec2e573b4644ffdfa7e035aeb66f55e847b7c991 HEAD 18108
Author: Samer Masterson <samer <at> samertm.com>
Date:   Sun Apr 5 20:59:37 2015 -0700

    Fixes 18108.
    
    * eshell/em-term.el (eshell-exec-visual): Show term-mode buffer
    regardless of whether the process has died (bug#18108).
    (eshell-term-sentinel, eshell-parent-buffer): Now unused, remove.

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

	Modified   lisp/ChangeLog
diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index b2d431c..a06ad01 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,9 @@
+2015-04-06  Samer Masterson  <samer <at> samertm.com>
+
+	* eshell/em-term.el (eshell-exec-visual): Show term-mode buffer
+	regardless of whether the process has died (bug#18108).
+	(eshell-term-sentinel, eshell-parent-buffer): Now unused, remove.
+
 2015-03-27  Wolfgang Jenkner  <wjenkner <at> inode.at>
 
 	* font-lock.el (font-lock--remove-face-from-text-property): New
	Modified   lisp/eshell/em-term.el
diff --git a/lisp/eshell/em-term.el b/lisp/eshell/em-term.el
index 4a6ac23..7f98ee6 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.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.