call-process and incremental display of output

call-process and incremental display of output

[Florian Weimer]

I'm trying to show some progress information while downloading email for
Gnus using an external program.

I think this reproduces the core issue:

(defun fw/get-new-mail ()
  (interactive)
  (let* ((buffer (get-buffer-create "*mbsync*"))
	 (status (with-current-buffer buffer
		   (delete-region (point-min) (point-max))
		   (call-process "bash" nil (list buffer t) t
				 "-c" "
for x in {1..5} ; do
  date
  sleep 1
done
"))))
    (unless (= 0 status)
      (switch-to-buffer buffer)
      (error "mbsync exit with status %d" status))))

When I run this using ‘M-x fw/get-new-mail RET’, the buffer is not
displayed, even though I passed t for the display argument.  The output
from the shell does land in the buffer, which is possible to confirm by
putting a line “exit 1” after the “done“.

Any idea what is necessary to get incremental output?

I'm running GNU Emacs 25.3.1 (x86_64-redhat-linux-gnu, GTK+ Version
3.22.26) of 2018-06-27 (which is usual the Fedora 27 build).

Thanks,
Florian

Re: call-process and incremental display of output

[Michael Albinus]

Florian Weimer <fweimer@redhat.com> writes:

> I'm trying to show some progress information while downloading email for
> Gnus using an external program.
>
> Any idea what is necessary to get incremental output?

Use `start-process'.

> Thanks,
> Florian

Best regards, Michael.

Re: call-process and incremental display of output

[Florian Weimer]

* Michael Albinus:

> Florian Weimer <fweimer@redhat.com> writes:
>
>> I'm trying to show some progress information while downloading email for
>> Gnus using an external program.
>>
>> Any idea what is necessary to get incremental output?
>
> Use `start-process'.

That's not synchronous, so it won't work for fetching email in a
callback from Gnus.

Thanks,
Florian

Re: call-process and incremental display of output

[Michael Albinus]

Florian Weimer <fweimer@redhat.com> writes:

>>> I'm trying to show some progress information while downloading email for
>>> Gnus using an external program.
>>>
>>> Any idea what is necessary to get incremental output?
>>
>> Use `start-process'.
>
> That's not synchronous, so it won't work for fetching email in a
> callback from Gnus.

You could write some code around, which returns to Gnus once the async
process has finished.

> Thanks,
> Florian

Best regards, Michael.

Re: call-process and incremental display of output

[Stefan Monnier]

> (defun fw/get-new-mail ()
>   (interactive)
>   (let* ((buffer (get-buffer-create "*mbsync*"))
> 	 (status (with-current-buffer buffer
> 		   (delete-region (point-min) (point-max))
> 		   (call-process "bash" nil (list buffer t) t
> 				 "-c" "
> for x in {1..5} ; do
>   date
>   sleep 1
> done
> "))))
>     (unless (= 0 status)
>       (switch-to-buffer buffer)
>       (error "mbsync exit with status %d" status))))
>
> When I run this using ‘M-x fw/get-new-mail RET’, the buffer is not
> displayed, even though I passed t for the display argument.

The argument to call-process controls whether redisplay will take place
while the process is running, so you indeed need to set it to t in your
case, but it doesn't affect which buffer is shown in which window, and
you only display the buffer in the switch-to-buffer which is performed
after call-process is over.

IOW, just move your switch-to-buffer (which you should also change to
pop-to-buffer or something like that if you want your code to be robust)
to before the call to call-process.


        Stefan

Re: call-process and incremental display of output

[Florian Weimer]

* Stefan Monnier:

>> (defun fw/get-new-mail ()
>>   (interactive)
>>   (let* ((buffer (get-buffer-create "*mbsync*"))
>> 	 (status (with-current-buffer buffer
>> 		   (delete-region (point-min) (point-max))
>> 		   (call-process "bash" nil (list buffer t) t
>> 				 "-c" "
>> for x in {1..5} ; do
>>   date
>>   sleep 1
>> done
>> "))))
>>     (unless (= 0 status)
>>       (switch-to-buffer buffer)
>>       (error "mbsync exit with status %d" status))))
>>
>> When I run this using ‘M-x fw/get-new-mail RET’, the buffer is not
>> displayed, even though I passed t for the display argument.
>
> The argument to call-process controls whether redisplay will take place
> while the process is running, so you indeed need to set it to t in your
> case, but it doesn't affect which buffer is shown in which window, and
> you only display the buffer in the switch-to-buffer which is performed
> after call-process is over.

Ahh, that explains it.

> IOW, just move your switch-to-buffer (which you should also change to
> pop-to-buffer or something like that if you want your code to be robust)
> to before the call to call-process.

I see.  Further questions: How can I restore the window configuration
after the process terminates?  Is there something similar to
save-excursion?

How can I make the displayed buffer to scroll to the end?

I probably shuld rethink this approach and just launch an xterm or
something. 8-)

Thanks,
Florian

Re: call-process and incremental display of output

[tomas]

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

On Wed, Oct 17, 2018 at 10:32:04AM +0200, Florian Weimer wrote:
> * Stefan Monnier:
> 
> >> (defun fw/get-new-mail ()

[...]

> I see.  Further questions: How can I restore the window configuration
> after the process terminates?  Is there something similar to
> save-excursion?

save-window-excursion

> How can I make the displayed buffer to scroll to the end?

  (pop-to buffer "foo")
  (goto-char (point-max))

Don't wrap this in save-excursion :-)

Cheers
-- t

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: call-process and incremental display of output

[Florian Weimer]

* > On Wed, Oct 17, 2018 at 10:32:04AM +0200, Florian Weimer wrote:
>> * Stefan Monnier:
>> 
>> >> (defun fw/get-new-mail ()
>
> [...]
>
>> I see.  Further questions: How can I restore the window configuration
>> after the process terminates?  Is there something similar to
>> save-excursion?
>
> save-window-excursion

Oops, I could have found this myself.

>> How can I make the displayed buffer to scroll to the end?
>
>   (pop-to buffer "foo")
>   (goto-char (point-max))
>
> Don't wrap this in save-excursion :-)

Nice, I now have got this:

(defun fw/get-new-mail ()
  (interactive)
  (let* ((buffer (get-buffer-create "*mbsync*"))
	 (status (save-window-excursion
		   (pop-to-buffer buffer)
		   (delete-region (point-min) (point-max))
		   (goto-char (point-max))
		   (call-process "mbsync" nil (list buffer t) t
				 "-V" "redhat-incoming"))))
    (unless (= 0 status)
      (switch-to-buffer buffer)
      (error "mbsync exit with status %d" status))))

It's not very Emacs-like, but at least I have something to watch while
mail is downloaded.  Thanks!

Florian

Re: call-process and incremental display of output

[tomas]

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

On Wed, Oct 17, 2018 at 11:34:14AM +0200, Florian Weimer wrote:
> * > On Wed, Oct 17, 2018 at 10:32:04AM +0200, Florian Weimer wrote:
> >> * Stefan Monnier:
> >> 
> >> >> (defun fw/get-new-mail ()
> >
> > [...]
> >
> >> I see.  Further questions: How can I restore the window configuration
> >> after the process terminates?  Is there something similar to
> >> save-excursion?
> >
> > save-window-excursion
> 
> Oops, I could have found this myself.

To be fair, Emacs is *huge*. I've always been fascinated by how
difficult it is (sometimes) to find things in Emacs, a piece of
software which really goes out of its way to aid discoverability.

> It's not very Emacs-like, but at least I have something to watch while
> mail is downloaded.  Thanks!

Thanks for the code snippet!

Cheers
-- tomás

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: call-process and incremental display of output

[Stefan Monnier]

> I see.  Further questions: How can I restore the window configuration
> after the process terminates?  Is there something similar to
> save-excursion?

Depends if you care about whether the buffer might be displayed in
another frame (as would typically be the case in my config).

If you don't care about other people's configs and you only use a single
frame, there's save-window-excursion (but for configs like mine, every
use of save-window-excursion is generally a source of problems).
A simpler solution to "undo" a pop-to-buffer is to (bury-buffer).

> How can I make the displayed buffer to scroll to the end?

You might like to (setq-local window-point-insertion-type t) in your
buffer (make sure you do it before the buffer is displayed in a window).


        Stefan

Re: call-process and incremental display of output

[John Shahid]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> You might like to (setq-local window-point-insertion-type t) in your
> buffer (make sure you do it before the buffer is displayed in a window).

I'm curious why `save-excursion' doesn't respect the value of
`window-point-insertion-type' ?  I was wondering about this for some
time now.  The behavior I want is to always append the output to the end
of the buffer but not annoy the user if they decide to move the point
somewhere else to look at previous output.  In this case the point
should stay where it is.  This sounds exactly like what markers are
supposed to do.  Unfortunately the marker created by `save-excursion'
always has a `nil' insertion type.  This means I have to maintain my own
marker in an `unwind-protect' similar to what the compilation mode does.
Are there any objections to changing `save-excursion' behavior ?

Re: call-process and incremental display of output

[Stefan Monnier]

>> You might like to (setq-local window-point-insertion-type t) in your
>> buffer (make sure you do it before the buffer is displayed in a window).
> I'm curious why `save-excursion' doesn't respect the value of
> `window-point-insertion-type' ?  I was wondering about this for some
> time now.  The behavior I want is to always append the output to the end
> of the buffer but not annoy the user if they decide to move the point
> somewhere else to look at previous output.  In this case the point
> should stay where it is.  This sounds exactly like what markers are
> supposed to do.  Unfortunately the marker created by `save-excursion'
> always has a `nil' insertion type.  This means I have to maintain my own
> marker in an `unwind-protect' similar to what the compilation mode does.
> Are there any objections to changing `save-excursion' behavior ?

I lost you: where do you save-excursion?


        Stefan

Re: call-process and incremental display of output

[John Shahid]

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

>>> You might like to (setq-local window-point-insertion-type t) in your
>>> buffer (make sure you do it before the buffer is displayed in a window).
>> I'm curious why `save-excursion' doesn't respect the value of
>> `window-point-insertion-type' ?  I was wondering about this for some
>> time now.  The behavior I want is to always append the output to the end
>> of the buffer but not annoy the user if they decide to move the point
>> somewhere else to look at previous output.  In this case the point
>> should stay where it is.  This sounds exactly like what markers are
>> supposed to do.  Unfortunately the marker created by `save-excursion'
>> always has a `nil' insertion type.  This means I have to maintain my own
>> marker in an `unwind-protect' similar to what the compilation mode does.
>> Are there any objections to changing `save-excursion' behavior ?
>
> I lost you: where do you save-excursion?

Sorry, I should have explained a little bit what I'm trying to do.  I
spin up a curl process that connects to our CI server and display the
log output of a build.  The server stream the log output as a sequence
of json messages that have to be parsed and appended to the buffer.  I
used to do that with the following snippet of code:

    (with-current-buffer log-buffer
      (save-excursion
        (goto-char (point-max))
        (insert log-line)))

This caused the data to be inserted at the end of the buffer, but I
constantly had to scroll to the end of the buffer in order to follow the
output.

I was aware of this problem for a while but never got chance to tackle
it.  Then, I saw your earlier message about
`window-point-insertion-type' and I decided to try it.  That didn't work
and I think the reason is `save-excursion' creates a marker with
insertion type set to `nil'.

In my earlier message, I was trying to propose making `save-excursion'
set the marker insertion type to `t' if `window-point-insertion-type' is
set to t.

FYI, what I currently have is something like this:

    (let ((pos (copy-marker (point) t)))
      (ignore-errors
        (goto-char (point-max))
        (insert payload))
      (goto-char pos))

Re: call-process and incremental display of output

[Stefan Monnier]

> spin up a curl process that connects to our CI server and display the
> log output of a build.  The server stream the log output as a sequence
> of json messages that have to be parsed and appended to the buffer.  I
> used to do that with the following snippet of code:
>
>     (with-current-buffer log-buffer
>       (save-excursion
>         (goto-char (point-max))
>         (insert log-line)))

Ah, this one.

> FYI, what I currently have is something like this:
>
>     (let ((pos (copy-marker (point) t)))
>       (ignore-errors
>         (goto-char (point-max))
>         (insert payload))
>       (goto-char pos))

Yes, many/most process filters end up doing that (tho others use
insert-before-markers instead, which comes with other problems).

Given how pervasively save-excursion is used, I think changing its
behavior is very risky.  Admittedly, it didn't prevent me from changing
it by dropping the mark handling from it.
And window-point-insertion-type is used fairly rarely, so maybe the
impact would not be quite as widespread as it seems.

Anyway, it's luckily not my call to make ;-)

I'd suggest you try running with such a change for some months, trying
to use a variety of packages and see if you bump into problems.


        Stefan

Re: call-process and incremental display of output

[John Shahid]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> spin up a curl process that connects to our CI server and display the
>> log output of a build.  The server stream the log output as a sequence
>> of json messages that have to be parsed and appended to the buffer.  I
>> used to do that with the following snippet of code:
>>
>>     (with-current-buffer log-buffer
>>       (save-excursion
>>         (goto-char (point-max))
>>         (insert log-line)))
>
> Ah, this one.
>
>> FYI, what I currently have is something like this:
>>
>>     (let ((pos (copy-marker (point) t)))
>>       (ignore-errors
>>         (goto-char (point-max))
>>         (insert payload))
>>       (goto-char pos))
>
> Yes, many/most process filters end up doing that (tho others use
> insert-before-markers instead, which comes with other problems).
>
> Given how pervasively save-excursion is used, I think changing its
> behavior is very risky.  Admittedly, it didn't prevent me from changing
> it by dropping the mark handling from it.
> And window-point-insertion-type is used fairly rarely, so maybe the
> impact would not be quite as widespread as it seems.
>
> Anyway, it's luckily not my call to make ;-)
>
> I'd suggest you try running with such a change for some months, trying
> to use a variety of packages and see if you bump into problems.
>

Sounds good.  I'll make the change locally and test it for a month or
two and report back.

Thanks,

js

Re: call-process and incremental display of output

[John Shahid]

FYI, I have been using this patch without any noticeable issues.  Not
sure if we should merge it.

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> Sounds good.  I'll make the change locally and test it for a month or
>> two and report back.
>
> FWIW, I'm running with the following change now:
>
>     diff --git a/src/editfns.c b/src/editfns.c
>     index e995b38a44..db95a8a20a 100644
>     --- a/src/editfns.c
>     +++ b/src/editfns.c
>     @@ -782,6 +782,12 @@ save_excursion_save (union specbinding *pdl)
>      {
>        eassert (pdl->unwind_excursion.kind == SPECPDL_UNWIND_EXCURSION);
>        pdl->unwind_excursion.marker = Fpoint_marker ();
>     +  /* Suggested by John Shahid <jvshahid@gmail.com> in the "call-process and
>     +   * incremental display of output" thread of help-gnu-emacs.
>     +   * This matches the manually-created behavior of compile.el's process filter
>     +   * and probably others like comint as well.  */
>     +  XMARKER (pdl->unwind_excursion.marker)->insertion_type
>     +    = !NILP (Vwindow_point_insertion_type);
>        /* Selected window if current buffer is shown in it, nil otherwise.  */
>        pdl->unwind_excursion.window
>          = (EQ (XWINDOW (selected_window)->contents, Fcurrent_buffer ())
>
> I haven't double checked that it does what I think it does, to be
> honest, but at least after a mere 48h of normal use I haven't noticed
> anything weird yet.
>
>
>         Stefan

Re: call-process and incremental display of output

[John Shahid]

Ping.  Not sure if my previous email was lost in the ether.  FWIW, I am
still using that patch and didn't run into any issues.  It works as I
expected, i.e. when point is at eob, new text is inserted before the
point.

John Shahid <jvshahid@gmail.com> writes:

> FYI, I have been using this patch without any noticeable issues.  Not
> sure if we should merge it.
>
> Stefan Monnier <monnier@IRO.UMontreal.CA> writes:
>
>>> Sounds good.  I'll make the change locally and test it for a month or
>>> two and report back.
>>
>> FWIW, I'm running with the following change now:
>>
>>     diff --git a/src/editfns.c b/src/editfns.c
>>     index e995b38a44..db95a8a20a 100644
>>     --- a/src/editfns.c
>>     +++ b/src/editfns.c
>>     @@ -782,6 +782,12 @@ save_excursion_save (union specbinding *pdl)
>>      {
>>        eassert (pdl->unwind_excursion.kind == SPECPDL_UNWIND_EXCURSION);
>>        pdl->unwind_excursion.marker = Fpoint_marker ();
>>     +  /* Suggested by John Shahid <jvshahid@gmail.com> in the "call-process and
>>     +   * incremental display of output" thread of help-gnu-emacs.
>>     +   * This matches the manually-created behavior of compile.el's process filter
>>     +   * and probably others like comint as well.  */
>>     +  XMARKER (pdl->unwind_excursion.marker)->insertion_type
>>     +    = !NILP (Vwindow_point_insertion_type);
>>        /* Selected window if current buffer is shown in it, nil otherwise.  */
>>        pdl->unwind_excursion.window
>>          = (EQ (XWINDOW (selected_window)->contents, Fcurrent_buffer ())
>>
>> I haven't double checked that it does what I think it does, to be
>> honest, but at least after a mere 48h of normal use I haven't noticed
>> anything weird yet.
>>
>>
>>         Stefan

Re: call-process and incremental display of output

[Stefan Monnier]

> Ping.  Not sure if my previous email was lost in the ether.  FWIW, I am
> still using that patch and didn't run into any issues.  It works as I
> expected, i.e. when point is at eob, new text is inserted before the
> point.

This should go to emacs-devel.

FWIW, I noticed at least the following incompatibility with this patch:

    emacs -Q
    M-x ielm
    M-x electric-pair-mode
    (

This should leave you with "()" and point between the two parens, but
instead point is left after the closing paren.

This is because the behavior of the electric-pair thingy is equivalent to

    (insert "(")
    (save-excursion
      (insert ")"))

which is indeed affected by this patch.


        Stefan


> John Shahid <jvshahid@gmail.com> writes:
>
>> FYI, I have been using this patch without any noticeable issues.  Not
>> sure if we should merge it.
>>
>> Stefan Monnier <monnier@IRO.UMontreal.CA> writes:
>>
>>>> Sounds good.  I'll make the change locally and test it for a month or
>>>> two and report back.
>>>
>>> FWIW, I'm running with the following change now:
>>>
>>>     diff --git a/src/editfns.c b/src/editfns.c
>>>     index e995b38a44..db95a8a20a 100644
>>>     --- a/src/editfns.c
>>>     +++ b/src/editfns.c
>>>     @@ -782,6 +782,12 @@ save_excursion_save (union specbinding *pdl)
>>>      {
>>>        eassert (pdl->unwind_excursion.kind == SPECPDL_UNWIND_EXCURSION);
>>>        pdl->unwind_excursion.marker = Fpoint_marker ();
>>>     +  /* Suggested by John Shahid <jvshahid@gmail.com> in the "call-process and
>>>     +   * incremental display of output" thread of help-gnu-emacs.
>>>     +   * This matches the manually-created behavior of compile.el's process filter
>>>     +   * and probably others like comint as well.  */
>>>     +  XMARKER (pdl->unwind_excursion.marker)->insertion_type
>>>     +    = !NILP (Vwindow_point_insertion_type);
>>>        /* Selected window if current buffer is shown in it, nil otherwise.  */
>>>        pdl->unwind_excursion.window
>>>          = (EQ (XWINDOW (selected_window)->contents, Fcurrent_buffer ())
>>>
>>> I haven't double checked that it does what I think it does, to be
>>> honest, but at least after a mere 48h of normal use I haven't noticed
>>> anything weird yet.
>>>
>>>
>>>         Stefan

Re: call-process and incremental display of output

[John Shahid]

[ -help-gnu-emacs and +emacs-devel ]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> Ping.  Not sure if my previous email was lost in the ether.  FWIW, I am
>> still using that patch and didn't run into any issues.  It works as I
>> expected, i.e. when point is at eob, new text is inserted before the
>> point.
>
> This should go to emacs-devel.

Good point.  I didn't realize this thread was CC'ing help-gnu-emacs.  I
think there is enough context below for those who haven't been following
the original thread.

> FWIW, I noticed at least the following incompatibility with this patch:
>
>     emacs -Q
>     M-x ielm
>     M-x electric-pair-mode
>     (
>
> This should leave you with "()" and point between the two parens, but
> instead point is left after the closing paren.
>
> This is because the behavior of the electric-pair thingy is equivalent to
>
>     (insert "(")
>     (save-excursion
>       (insert ")"))
>
> which is indeed affected by this patch.

This will affect all modes derived from comint modes and all the minor
modes that could be enabled in those buffers.  That isn't great.  Maybe
the type of marker used in `save-excursion' should be controlled by a
different and/or new variable, which isn't great either.  I don't like
the idea of introducing more variables/customizations.

Unfortunately I don't have anymore ideas to share ATM.

JS

>         Stefan
>
>
>> John Shahid <jvshahid@gmail.com> writes:
>>
>>> FYI, I have been using this patch without any noticeable issues.  Not
>>> sure if we should merge it.
>>>
>>> Stefan Monnier <monnier@IRO.UMontreal.CA> writes:
>>>
>>>>> Sounds good.  I'll make the change locally and test it for a month or
>>>>> two and report back.
>>>>
>>>> FWIW, I'm running with the following change now:
>>>>
>>>>     diff --git a/src/editfns.c b/src/editfns.c
>>>>     index e995b38a44..db95a8a20a 100644
>>>>     --- a/src/editfns.c
>>>>     +++ b/src/editfns.c
>>>>     @@ -782,6 +782,12 @@ save_excursion_save (union specbinding *pdl)
>>>>      {
>>>>        eassert (pdl->unwind_excursion.kind == SPECPDL_UNWIND_EXCURSION);
>>>>        pdl->unwind_excursion.marker = Fpoint_marker ();
>>>>     +  /* Suggested by John Shahid <jvshahid@gmail.com> in the "call-process and
>>>>     +   * incremental display of output" thread of help-gnu-emacs.
>>>>     +   * This matches the manually-created behavior of compile.el's process filter
>>>>     +   * and probably others like comint as well.  */
>>>>     +  XMARKER (pdl->unwind_excursion.marker)->insertion_type
>>>>     +    = !NILP (Vwindow_point_insertion_type);
>>>>        /* Selected window if current buffer is shown in it, nil otherwise.  */
>>>>        pdl->unwind_excursion.window
>>>>          = (EQ (XWINDOW (selected_window)->contents, Fcurrent_buffer ())
>>>>
>>>> I haven't double checked that it does what I think it does, to be
>>>> honest, but at least after a mere 48h of normal use I haven't noticed
>>>> anything weird yet.
>>>>
>>>>
>>>>         Stefan

Re: call-process and incremental display of output

[Stefan Monnier]

> This will affect all modes derived from comint modes and all the minor
> modes that could be enabled in those buffers.  That isn't great.  Maybe
> the type of marker used in `save-excursion' should be controlled by a
> different and/or new variable, which isn't great either.  I don't like
> the idea of introducing more variables/customizations.
> Unfortunately I don't have anymore ideas to share ATM.

Maybe we should just add a `save-excursion-with-insertion-type` macro.


        Stefan