Re: master a710f8a: * lisp/comint.el (comint-history-isearch-setup): Check if process is live.

Re: master a710f8a: * lisp/comint.el (comint-history-isearch-setup): Check if process is live.

[Leo Liu]

On 2018-02-06 16:39 -0500, Juri Linkov wrote:
> branch: master
> commit a710f8aa61ca73054109dc4f926d1ac6aabdd849
> Author: Juri Linkov <juri@linkov.net>
> Commit: Juri Linkov <juri@linkov.net>
>
>     * lisp/comint.el (comint-history-isearch-setup): Check if process is live.
>     
>     Don't activate comint-history isearch when shell prompt is empty
>     like in all *Async Shell Command* buffers. (Bug#30187)
> ---
>  lisp/comint.el | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/lisp/comint.el b/lisp/comint.el
> index 8dba317..b4fbfc8 100644
> --- a/lisp/comint.el
> +++ b/lisp/comint.el
> @@ -1448,10 +1448,17 @@ If nil, Isearch operates on the whole comint buffer."
>  (defun comint-history-isearch-setup ()
>    "Set up a comint for using Isearch to search the input history.
>  Intended to be added to `isearch-mode-hook' in `comint-mode'."
> -  (when (or (eq comint-history-isearch t)
> -	    (and (eq comint-history-isearch 'dwim)
> -		 ;; Point is at command line.
> -		 (comint-after-pmark-p)))
> +  (when (and (get-buffer-process (current-buffer))
> +	     (or (eq comint-history-isearch t)
> +		 (and (eq comint-history-isearch 'dwim)
> +		      ;; Point is at command line.
> +		      (comint-after-pmark-p)
> +		      ;; Prompt is not empty like in Async Shell Command buffers
> +		      (not (eq (save-excursion
> +				 (goto-char (comint-line-beginning-position))
> +				 (forward-line 0)
> +				 (point))
> +			       (comint-line-beginning-position))))))
>      (setq isearch-message-prefix-add "history ")
>      (setq-local isearch-search-fun-function
>                  #'comint-history-isearch-search)
>

Changing comint-history-isearch-setup this way seems too pervasive. I
have a use case where (get-buffer-process (current-buffer)) is always
nil. Is there another way to work around the issue in *Async Shell
Command*? Thanks.

Leo

bug#30187: master a710f8a: * lisp/comint.el (comint-history-isearch-setup): Check if process is live.

[Juri Linkov]

> Changing comint-history-isearch-setup this way seems too pervasive. I
> have a use case where (get-buffer-process (current-buffer)) is always
> nil. Is there another way to work around the issue in *Async Shell
> Command*? Thanks.

Before this fix the search was broken in *Async Shell Command*
and in inactive shells.

For example, try to set comint-history-isearch to ‘dwim’
and type ‘C-r’ in a *Async Shell Command* buffer.  It fails with

  Lisp error: (wrong-type-argument processp nil)
  process-mark(nil)
  comint-after-pmark-p()
  comint-history-isearch-setup()
  ...

Or even when comint-history-isearch is nil by default, run shell ‘M-x shell’,
then exit it, and after “Process shell finished” type ‘M-r’ and any letter
to search for it:

  Lisp error: (wrong-type-argument processp nil)
  process-mark(nil)
  comint-delete-input()
  comint-goto-input(nil)
  comint-history-isearch-pop-state
  ...

In these cases the history can't be searched because there is no active shell.
I wonder what use case do you need in inactive shells without a prompt,
so it's impossible to search in the history.  How you used to search
through the shell history without failing in ‘comint-goto-input’ like
in the backtrace above?

bug#30187: master a710f8a: * lisp/comint.el (comint-history-isearch-setup): Check if process is live.

[Leo Liu]

On 2018-02-07 23:20 +0200, Juri Linkov wrote:
> Before this fix the search was broken in *Async Shell Command*
> and in inactive shells.

Thanks. I tried the following examples and saw their failures.

>
> For example, try to set comint-history-isearch to ‘dwim’
> and type ‘C-r’ in a *Async Shell Command* buffer.  It fails with
>
>   Lisp error: (wrong-type-argument processp nil)
>   process-mark(nil)
>   comint-after-pmark-p()
>   comint-history-isearch-setup()
>   ...

It seems to make a lot of sense to have comint-after-pmark-p return nil
instead. WDYT?

> Or even when comint-history-isearch is nil by default, run shell ‘M-x shell’,
> then exit it, and after “Process shell finished” type ‘M-r’ and any letter
> to search for it:
>
>   Lisp error: (wrong-type-argument processp nil)
>   process-mark(nil)
>   comint-delete-input()
>   comint-goto-input(nil)
>   comint-history-isearch-pop-state
>   ...
>
> In these cases the history can't be searched because there is no active shell.
> I wonder what use case do you need in inactive shells without a prompt,
> so it's impossible to search in the history.  How you used to search
> through the shell history without failing in ‘comint-goto-input’ like
> in the backtrace above?

My use case is using a function (or in Erlang's lingo a light-weight
process) to communicate with a remote Erlang shell. So there is no
process as comint/emacs understands.

Leo

bug#30187: master a710f8a: * lisp/comint.el (comint-history-isearch-setup): Check if process is live.

[Juri Linkov]

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

>> For example, try to set comint-history-isearch to ‘dwim’
>> and type ‘C-r’ in a *Async Shell Command* buffer.  It fails with
>>
>>   Lisp error: (wrong-type-argument processp nil)
>>   process-mark(nil)
>>   comint-after-pmark-p()
>>   comint-history-isearch-setup()
>>   ...
>
> It seems to make a lot of sense to have comint-after-pmark-p return nil
> instead. WDYT?

Good idea.  The patch at the end of this message also always checks
if the prompt is empty at the end of the shell buffer to exclude
Async Shell Command buffers and inactive shells.

>> Or even when comint-history-isearch is nil by default, run shell ‘M-x shell’,
>> then exit it, and after “Process shell finished” type ‘M-r’ and any letter
>> to search for it:
>>
>>   Lisp error: (wrong-type-argument processp nil)
>>   process-mark(nil)
>>   comint-delete-input()
>>   comint-goto-input(nil)
>>   comint-history-isearch-pop-state
>>   ...
>>
>> In these cases the history can't be searched because there is no active shell.
>> I wonder what use case do you need in inactive shells without a prompt,
>> so it's impossible to search in the history.  How you used to search
>> through the shell history without failing in ‘comint-goto-input’ like
>> in the backtrace above?
>
> My use case is using a function (or in Erlang's lingo a light-weight
> process) to communicate with a remote Erlang shell. So there is no
> process as comint/emacs understands.

Nice, good to know, as these days I completely switched to Elixir.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: comint-after-pmark-p.patch --]
[-- Type: text/x-diff, Size: 1810 bytes --]

diff --git a/lisp/comint.el b/lisp/comint.el
index b4fbfc8..22fbe06 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -1448,17 +1448,18 @@ comint-history-isearch-message-overlay
 (defun comint-history-isearch-setup ()
   "Set up a comint for using Isearch to search the input history.
 Intended to be added to `isearch-mode-hook' in `comint-mode'."
-  (when (and (get-buffer-process (current-buffer))
+  (when (and
+             ;; Prompt is not empty like in Async Shell Command buffers
+             ;; or in inactive shells
+             (not (eq (save-excursion
+		        (goto-char (comint-line-beginning-position))
+		        (forward-line 0)
+		        (point))
+		      (comint-line-beginning-position)))
 	     (or (eq comint-history-isearch t)
 		 (and (eq comint-history-isearch 'dwim)
 		      ;; Point is at command line.
-		      (comint-after-pmark-p)
-		      ;; Prompt is not empty like in Async Shell Command buffers
-		      (not (eq (save-excursion
-				 (goto-char (comint-line-beginning-position))
-				 (forward-line 0)
-				 (point))
-			       (comint-line-beginning-position))))))
+		      (comint-after-pmark-p))))
     (setq isearch-message-prefix-add "history ")
     (setq-local isearch-search-fun-function
                 #'comint-history-isearch-search)
@@ -2288,8 +2289,10 @@ comint-skip-prompt
 
 (defun comint-after-pmark-p ()
   "Return t if point is after the process output marker."
-  (let ((pmark (process-mark (get-buffer-process (current-buffer)))))
-    (<= (marker-position pmark) (point))))
+  (let ((process (get-buffer-process (current-buffer))))
+    (when process
+      (let ((pmark (process-mark process)))
+        (<= (marker-position pmark) (point))))))
 
 (defun comint-simple-send (proc string)
   "Default function for sending to PROC input STRING.

bug#30187: master a710f8a: * lisp/comint.el (comint-history-isearch-setup): Check if process is live.

[Leo Liu]

On 2018-02-08 23:29 +0200, Juri Linkov wrote:
> Good idea.  The patch at the end of this message also always checks
> if the prompt is empty at the end of the shell buffer to exclude
> Async Shell Command buffers and inactive shells.

Thanks for taking this on. The patch looks good.

>> My use case is using a function (or in Erlang's lingo a light-weight
>> process) to communicate with a remote Erlang shell. So there is no
>> process as comint/emacs understands.
>
> Nice, good to know, as these days I completely switched to Elixir.

Good to know too. The BEAM is underappreciated.

Leo

bug#30187: master a710f8a: * lisp/comint.el (comint-history-isearch-setup): Check if process is live.

[Juri Linkov]

>> Good idea.  The patch at the end of this message also always checks
>> if the prompt is empty at the end of the shell buffer to exclude
>> Async Shell Command buffers and inactive shells.
>
> Thanks for taking this on. The patch looks good.

This is now pushed to master.  Thanks for the feedback.