[PATCH] emacs: show: stop stderr appearing in buffer

[PATCH] emacs: show: stop stderr appearing in buffer

[Mark Walters]

In emacs 24.3+ the stdout/stderr from externally displaying an
attachment gets inserted into the show buffer. This is caused by
changes in mm-display-external in mm-decode.el.

Ideally, we would put this output in the notmuch errors buffer but the
handler is called asynchronously so we don't know when the output will
appear. Thus if we put it straight into the errors buffer it could get
interleaved with other errors, otoh we can't easily tell when we
have got all the error output so can't wait until the process is complete.

This patch just takes the simplest approach and discards the error
output (which means the behaviour is the same as it was with emacs pre
24.3).
---
As I say above it would be nice to capture the error output but it
does not appear to be simple. This is probably worth applying in the
meantime as we definitely don't want the stderr/stdout appearing in
the show buffer.

The bug is easy to reproduce: make a script which outputs to stderr or
stdout and call it on any part with ". o scriptname" (in fact even
". o ls" will do!)

Best wishes

Mark






 emacs/notmuch-show.el |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 904b98e..42438ba 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -2044,9 +2044,12 @@ caller is responsible for killing this buffer as appropriate."
 This ensures that the temporary buffer created for the mm-handle
 is destroyed when FN returns."
   (let ((handle (notmuch-show-current-part-handle)))
-    (unwind-protect
-	(funcall fn handle)
-      (kill-buffer (mm-handle-buffer handle)))))
+    ;; emacs 24.3+ puts stdout/stderr into the calling buffer so we
+    ;; call it from a temp-buffer.
+    (with-temp-buffer
+      (unwind-protect
+	  (funcall fn handle)
+	(kill-buffer (mm-handle-buffer handle))))))
 
 (defun notmuch-show-part-button-default (&optional button)
   (interactive)
-- 
1.7.9.1

Re: [PATCH] emacs: show: stop stderr appearing in buffer

[David Bremner]

> Ideally, we would put this output in the notmuch errors buffer but the
> handler is called asynchronously so we don't know when the output will
> appear. Thus if we put it straight into the errors buffer it could get
> interleaved with other errors, otoh we can't easily tell when we
> have got all the error output so can't wait until the process is complete.

Hi Mark;

I think your patch is OK, but would it be much harder to created a named
buffer like *notmuch-view-$message-d* ? (using e.g. the code from
notmuch-show). I might make debugging easier.

Of course those buffers would accumulate, along with show, search and
pick buffers...

Or we could push this as is, and add some debugging facility later like
a variable notmuch-view-errors-buffer.

d

Re: [PATCH] emacs: show: stop stderr appearing in buffer

[Mark Walters]

Hi

On Tue, 10 Sep 2013, David Bremner <david@tethera.net> wrote:
>> Ideally, we would put this output in the notmuch errors buffer but the
>> handler is called asynchronously so we don't know when the output will
>> appear. Thus if we put it straight into the errors buffer it could get
>> interleaved with other errors, otoh we can't easily tell when we
>> have got all the error output so can't wait until the process is complete.
>
> Hi Mark;
>
> I think your patch is OK, but would it be much harder to created a named
> buffer like *notmuch-view-$message-d* ? (using e.g. the code from
> notmuch-show). I might make debugging easier.

Yes this is easy. There are several possibilities and I am not sure
which is best (some are clearly bad but are worth mentioning anyway).

1) have a single buffer for part errors; this would accumulate stuff and
output seems to get interleaved so this is probably useless.

2) have a buffer for each part viewer as you describe.

3) have a buffer for each part viewer but start its name with a space so
it doesn't show up in buffer lists but is findable (maybe)

4) stick with just the temp buffer approach

Also, we could have it togglable with some sort of debug flag. In some
senses 3 is nice but you would probably end up with 10's or even
hundreds of hidden buffers which seems bad. In 2 you see them so you
probably kill them as you go but I think they would be pretty
annoying. A key difference from the accumulated show/search/pick buffers
is that, at some point, you did want to see those buffers.

Since all these approaches are easy to implement it is really up to us
which we want.

Any thoughts?

Mark


>
> Of course those buffers would accumulate, along with show, search and
> pick buffers...
>
> Or we could push this as is, and add some debugging facility later like
> a variable notmuch-view-errors-buffer.
>
> d

Re: [PATCH] emacs: show: stop stderr appearing in buffer

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:


> 3) have a buffer for each part viewer but start its name with a space so
> it doesn't show up in buffer lists but is findable (maybe)

This, toggleable with a debug flag sounds good to me; if the debug flag
is nil, use a temp buffer. Or equivalently delete the named buffer if
debug flag is nil.

d

Re: [PATCH] emacs: show: stop stderr appearing in buffer

[Tomi Ollila]

On Thu, Sep 12 2013, Mark Walters <markwalters1009@gmail.com> wrote:

> Hi
>
> On Tue, 10 Sep 2013, David Bremner <david@tethera.net> wrote:
>>> Ideally, we would put this output in the notmuch errors buffer but the
>>> handler is called asynchronously so we don't know when the output will
>>> appear. Thus if we put it straight into the errors buffer it could get
>>> interleaved with other errors, otoh we can't easily tell when we
>>> have got all the error output so can't wait until the process is complete.
>>
>> Hi Mark;
>>
>> I think your patch is OK, but would it be much harder to created a named
>> buffer like *notmuch-view-$message-d* ? (using e.g. the code from
>> notmuch-show). I might make debugging easier.
>
> Yes this is easy. There are several possibilities and I am not sure
> which is best (some are clearly bad but are worth mentioning anyway).
>
> 1) have a single buffer for part errors; this would accumulate stuff and
> output seems to get interleaved so this is probably useless.
>
> 2) have a buffer for each part viewer as you describe.
>
> 3) have a buffer for each part viewer but start its name with a space so
> it doesn't show up in buffer lists but is findable (maybe)
>
> 4) stick with just the temp buffer approach


Maybe check whether the temp buffer is empty. if not, use
(buffer-string) & (notmuch-logged-error) to append the message
to the *Notmuch errors* buffer... just that notmuch-logged-error
signals an error which we may not want to do...

We could unify to "*Notmuch Messages*" and have more functions to 
append data there... somewhat analogous to current *Messages* buffer
just that that one has so much noise...

Tomi


>
> Also, we could have it togglable with some sort of debug flag. In some
> senses 3 is nice but you would probably end up with 10's or even
> hundreds of hidden buffers which seems bad. In 2 you see them so you
> probably kill them as you go but I think they would be pretty
> annoying. A key difference from the accumulated show/search/pick buffers
> is that, at some point, you did want to see those buffers.
>
> Since all these approaches are easy to implement it is really up to us
> which we want.
>
> Any thoughts?
>
> Mark
>
>
>>
>> Of course those buffers would accumulate, along with show, search and
>> pick buffers...
>>
>> Or we could push this as is, and add some debugging facility later like
>> a variable notmuch-view-errors-buffer.
>>
>> d
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] emacs: show: stop stderr appearing in buffer

[Mark Walters]

On Thu, 12 Sep 2013, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Thu, Sep 12 2013, Mark Walters <markwalters1009@gmail.com> wrote:
>
>> Hi
>>
>> On Tue, 10 Sep 2013, David Bremner <david@tethera.net> wrote:
>>>> Ideally, we would put this output in the notmuch errors buffer but the
>>>> handler is called asynchronously so we don't know when the output will
>>>> appear. Thus if we put it straight into the errors buffer it could get
>>>> interleaved with other errors, otoh we can't easily tell when we
>>>> have got all the error output so can't wait until the process is complete.
>>>
>>> Hi Mark;
>>>
>>> I think your patch is OK, but would it be much harder to created a named
>>> buffer like *notmuch-view-$message-d* ? (using e.g. the code from
>>> notmuch-show). I might make debugging easier.
>>
>> Yes this is easy. There are several possibilities and I am not sure
>> which is best (some are clearly bad but are worth mentioning anyway).
>>
>> 1) have a single buffer for part errors; this would accumulate stuff and
>> output seems to get interleaved so this is probably useless.
>>
>> 2) have a buffer for each part viewer as you describe.
>>
>> 3) have a buffer for each part viewer but start its name with a space so
>> it doesn't show up in buffer lists but is findable (maybe)
>>
>> 4) stick with just the temp buffer approach
>
>
> Maybe check whether the temp buffer is empty. if not, use
> (buffer-string) & (notmuch-logged-error) to append the message
> to the *Notmuch errors* buffer... just that notmuch-logged-error
> signals an error which we may not want to do...

The problem is that the external part viewer is run asynchronously. So
when we get to the decision what to do the buffer has not received any
output yet even when the first thing the viewer does is output
something.

A further complication is that in some cases the viewer might not
output things until it is closed and that could be arbitrarily later.

Best wishes

Mark


>
> We could unify to "*Notmuch Messages*" and have more functions to 
> append data there... somewhat analogous to current *Messages* buffer
> just that that one has so much noise...
>
> Tomi
>
>
>>
>> Also, we could have it togglable with some sort of debug flag. In some
>> senses 3 is nice but you would probably end up with 10's or even
>> hundreds of hidden buffers which seems bad. In 2 you see them so you
>> probably kill them as you go but I think they would be pretty
>> annoying. A key difference from the accumulated show/search/pick buffers
>> is that, at some point, you did want to see those buffers.
>>
>> Since all these approaches are easy to implement it is really up to us
>> which we want.
>>
>> Any thoughts?
>>
>> Mark
>>
>>
>>>
>>> Of course those buffers would accumulate, along with show, search and
>>> pick buffers...
>>>
>>> Or we could push this as is, and add some debugging facility later like
>>> a variable notmuch-view-errors-buffer.
>>>
>>> d
>> _______________________________________________
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] emacs: show: stop stderr appearing in buffer

[Tomi Ollila]

On Thu, Sep 12 2013, Mark Walters <markwalters1009@gmail.com> wrote:

> On Thu, 12 Sep 2013, Tomi Ollila <tomi.ollila@iki.fi> wrote:
>> On Thu, Sep 12 2013, Mark Walters <markwalters1009@gmail.com> wrote:
>>
>>> Hi
>>>
>>> On Tue, 10 Sep 2013, David Bremner <david@tethera.net> wrote:
>>>>> Ideally, we would put this output in the notmuch errors buffer but the
>>>>> handler is called asynchronously so we don't know when the output will
>>>>> appear. Thus if we put it straight into the errors buffer it could get
>>>>> interleaved with other errors, otoh we can't easily tell when we
>>>>> have got all the error output so can't wait until the process is complete.
>>>>
>>>> Hi Mark;
>>>>
>>>> I think your patch is OK, but would it be much harder to created a named
>>>> buffer like *notmuch-view-$message-d* ? (using e.g. the code from
>>>> notmuch-show). I might make debugging easier.
>>>
>>> Yes this is easy. There are several possibilities and I am not sure
>>> which is best (some are clearly bad but are worth mentioning anyway).
>>>
>>> 1) have a single buffer for part errors; this would accumulate stuff and
>>> output seems to get interleaved so this is probably useless.
>>>
>>> 2) have a buffer for each part viewer as you describe.
>>>
>>> 3) have a buffer for each part viewer but start its name with a space so
>>> it doesn't show up in buffer lists but is findable (maybe)
>>>
>>> 4) stick with just the temp buffer approach
>>
>>
>> Maybe check whether the temp buffer is empty. if not, use
>> (buffer-string) & (notmuch-logged-error) to append the message
>> to the *Notmuch errors* buffer... just that notmuch-logged-error
>> signals an error which we may not want to do...
>
> The problem is that the external part viewer is run asynchronously. So
> when we get to the decision what to do the buffer has not received any
> output yet even when the first thing the viewer does is output
> something.
>
> A further complication is that in some cases the viewer might not
> output things until it is closed and that could be arbitrarily later.

I may not have understood completely -- but the temp buffer has lifetime...
We could store the timestamp when executing function starts and just
before the temp buffer is about to be wiped out, do the emptiness check
and if not empty use the stored timestamp & current time in the message
to be written to aid the human reader who may want to see the message...

>
> Best wishes
>
> Mark

Tomi


>
>
>>
>> We could unify to "*Notmuch Messages*" and have more functions to 
>> append data there... somewhat analogous to current *Messages* buffer
>> just that that one has so much noise...
>>
>> Tomi
>>
>>
>>>
>>> Also, we could have it togglable with some sort of debug flag. In some
>>> senses 3 is nice but you would probably end up with 10's or even
>>> hundreds of hidden buffers which seems bad. In 2 you see them so you
>>> probably kill them as you go but I think they would be pretty
>>> annoying. A key difference from the accumulated show/search/pick buffers
>>> is that, at some point, you did want to see those buffers.
>>>
>>> Since all these approaches are easy to implement it is really up to us
>>> which we want.
>>>
>>> Any thoughts?
>>>
>>> Mark
>>>
>>>
>>>>
>>>> Of course those buffers would accumulate, along with show, search and
>>>> pick buffers...
>>>>
>>>> Or we could push this as is, and add some debugging facility later like
>>>> a variable notmuch-view-errors-buffer.
>>>>
>>>> d
>>> _______________________________________________
>>> notmuch mailing list
>>> notmuch@notmuchmail.org
>>> http://notmuchmail.org/mailman/listinfo/notmuch
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] emacs: show: stop stderr appearing in buffer

[Austin Clements]

Quoth Mark Walters on Sep 12 at 10:33 am:
> 
> Hi
> 
> On Tue, 10 Sep 2013, David Bremner <david@tethera.net> wrote:
> >> Ideally, we would put this output in the notmuch errors buffer but the
> >> handler is called asynchronously so we don't know when the output will
> >> appear. Thus if we put it straight into the errors buffer it could get
> >> interleaved with other errors, otoh we can't easily tell when we
> >> have got all the error output so can't wait until the process is complete.
> >
> > Hi Mark;
> >
> > I think your patch is OK, but would it be much harder to created a named
> > buffer like *notmuch-view-$message-d* ? (using e.g. the code from
> > notmuch-show). I might make debugging easier.
> 
> Yes this is easy. There are several possibilities and I am not sure
> which is best (some are clearly bad but are worth mentioning anyway).
> 
> 1) have a single buffer for part errors; this would accumulate stuff and
> output seems to get interleaved so this is probably useless.
> 
> 2) have a buffer for each part viewer as you describe.
> 
> 3) have a buffer for each part viewer but start its name with a space so
> it doesn't show up in buffer lists but is findable (maybe)

3.5) Say something in the echo area when a viewer terminates with
output, so it doesn't interrupt the user if they're doing something,
but the output buffer is still discoverable.  Maybe bind C-c ` to show
the most recently reported output buffer, like what (la)tex-mode and
others do, and mention this binding in the echo area message.

> 4) stick with just the temp buffer approach
> 
> Also, we could have it togglable with some sort of debug flag. In some
> senses 3 is nice but you would probably end up with 10's or even
> hundreds of hidden buffers which seems bad. In 2 you see them so you
> probably kill them as you go but I think they would be pretty
> annoying. A key difference from the accumulated show/search/pick buffers
> is that, at some point, you did want to see those buffers.

3.5.1) Don't create a buffer until the command has output (or, easier
to implement: create the buffer, but kill it on exit if there was no
output).  When starting a new command, kill output buffers from
no-longer-running viewers that have never been visited (using
buffer-display-count or buffer-display-time).

> Since all these approaches are easy to implement it is really up to us
> which we want.
> 
> Any thoughts?
> 
> Mark
> 
> 
> >
> > Of course those buffers would accumulate, along with show, search and
> > pick buffers...
> >
> > Or we could push this as is, and add some debugging facility later like
> > a variable notmuch-view-errors-buffer.
> >
> > d

Re: [PATCH] emacs: show: stop stderr appearing in buffer

[Mark Walters]

On Thu, 12 Sep 2013, Austin Clements <amdragon@MIT.EDU> wrote:
> Quoth Mark Walters on Sep 12 at 10:33 am:
>> 
>> Hi
>> 
>> On Tue, 10 Sep 2013, David Bremner <david@tethera.net> wrote:
>> >> Ideally, we would put this output in the notmuch errors buffer but the
>> >> handler is called asynchronously so we don't know when the output will
>> >> appear. Thus if we put it straight into the errors buffer it could get
>> >> interleaved with other errors, otoh we can't easily tell when we
>> >> have got all the error output so can't wait until the process is complete.
>> >
>> > Hi Mark;
>> >
>> > I think your patch is OK, but would it be much harder to created a named
>> > buffer like *notmuch-view-$message-d* ? (using e.g. the code from
>> > notmuch-show). I might make debugging easier.
>> 
>> Yes this is easy. There are several possibilities and I am not sure
>> which is best (some are clearly bad but are worth mentioning anyway).
>> 
>> 1) have a single buffer for part errors; this would accumulate stuff and
>> output seems to get interleaved so this is probably useless.
>> 
>> 2) have a buffer for each part viewer as you describe.
>> 
>> 3) have a buffer for each part viewer but start its name with a space so
>> it doesn't show up in buffer lists but is findable (maybe)
>
> 3.5) Say something in the echo area when a viewer terminates with
> output, so it doesn't interrupt the user if they're doing something,
> but the output buffer is still discoverable.  Maybe bind C-c ` to show
> the most recently reported output buffer, like what (la)tex-mode and
> others do, and mention this binding in the echo area message.

The key problem here is that I don't know how to tell when the viewer
terminates. The viewer is run asynchronously and the sentinel for that
process puts the output in the buffer that called mm-display-external
(provided that that buffer is still alive).

Moreover, the code in mm-display-external seems to do some strange point
moving:

(when (buffer-live-p outbuf)
  (with-current-buffer outbuf
    (let ((buffer-read-only nil)
	  (point (point)))
      (forward-line 2)
      (mm-insert-inline
       handle (with-current-buffer buffer
		(buffer-string)))
      (goto-char point))))

which seems to put this batch of error/output into the buffer two lines
into the previous batch of error/output. 

>
>> 4) stick with just the temp buffer approach
>> 
>> Also, we could have it togglable with some sort of debug flag. In some
>> senses 3 is nice but you would probably end up with 10's or even
>> hundreds of hidden buffers which seems bad. In 2 you see them so you
>> probably kill them as you go but I think they would be pretty
>> annoying. A key difference from the accumulated show/search/pick buffers
>> is that, at some point, you did want to see those buffers.
>
> 3.5.1) Don't create a buffer until the command has output (or, easier
> to implement: create the buffer, but kill it on exit if there was no
> output).  When starting a new command, kill output buffers from
> no-longer-running viewers that have never been visited (using
> buffer-display-count or buffer-display-time).

Again this relies on being able to tell when a viewer has finished. If I
can do that then I think I could just put the output as a block in
*notmuch-errors*

Best wishes

Mark
>
>> Since all these approaches are easy to implement it is really up to us
>> which we want.
>> 
>> Any thoughts?
>> 
>> Mark
>> 
>> 
>> >
>> > Of course those buffers would accumulate, along with show, search and
>> > pick buffers...
>> >
>> > Or we could push this as is, and add some debugging facility later like
>> > a variable notmuch-view-errors-buffer.
>> >
>> > d

Re: [PATCH] emacs: show: stop stderr appearing in buffer

[Mark Walters]

On Thu, 12 Sep 2013, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Thu, Sep 12 2013, Mark Walters <markwalters1009@gmail.com> wrote:
>
>> On Thu, 12 Sep 2013, Tomi Ollila <tomi.ollila@iki.fi> wrote:
>>> On Thu, Sep 12 2013, Mark Walters <markwalters1009@gmail.com> wrote:
>>>
>>>> Hi
>>>>
>>>> On Tue, 10 Sep 2013, David Bremner <david@tethera.net> wrote:
>>>>>> Ideally, we would put this output in the notmuch errors buffer but the
>>>>>> handler is called asynchronously so we don't know when the output will
>>>>>> appear. Thus if we put it straight into the errors buffer it could get
>>>>>> interleaved with other errors, otoh we can't easily tell when we
>>>>>> have got all the error output so can't wait until the process is complete.
>>>>>
>>>>> Hi Mark;
>>>>>
>>>>> I think your patch is OK, but would it be much harder to created a named
>>>>> buffer like *notmuch-view-$message-d* ? (using e.g. the code from
>>>>> notmuch-show). I might make debugging easier.
>>>>
>>>> Yes this is easy. There are several possibilities and I am not sure
>>>> which is best (some are clearly bad but are worth mentioning anyway).
>>>>
>>>> 1) have a single buffer for part errors; this would accumulate stuff and
>>>> output seems to get interleaved so this is probably useless.
>>>>
>>>> 2) have a buffer for each part viewer as you describe.
>>>>
>>>> 3) have a buffer for each part viewer but start its name with a space so
>>>> it doesn't show up in buffer lists but is findable (maybe)
>>>>
>>>> 4) stick with just the temp buffer approach
>>>
>>>
>>> Maybe check whether the temp buffer is empty. if not, use
>>> (buffer-string) & (notmuch-logged-error) to append the message
>>> to the *Notmuch errors* buffer... just that notmuch-logged-error
>>> signals an error which we may not want to do...
>>
>> The problem is that the external part viewer is run asynchronously. So
>> when we get to the decision what to do the buffer has not received any
>> output yet even when the first thing the viewer does is output
>> something.
>>
>> A further complication is that in some cases the viewer might not
>> output things until it is closed and that could be arbitrarily later.
>
> I may not have understood completely -- but the temp buffer has lifetime...
> We could store the timestamp when executing function starts and just
> before the temp buffer is about to be wiped out, do the emptiness check
> and if not empty use the stored timestamp & current time in the message
> to be written to aid the human reader who may want to see the message...

The temp buffer is killed as soon as mm-display-external returns which
is almost instantly as it starts the external viewer asynchronously. The
sentinel for the external viewer (called when the external viewer exits)
sees the calling buffer is dead and just drops the error/output.

So it is not really that the error goes into the temp buffer: it just
doesn't go into the (quite likely still existing) show buffer.

Best wishes

Mark





>
>>
>> Best wishes
>>
>> Mark
>
> Tomi
>
>
>>
>>
>>>
>>> We could unify to "*Notmuch Messages*" and have more functions to 
>>> append data there... somewhat analogous to current *Messages* buffer
>>> just that that one has so much noise...
>>>
>>> Tomi
>>>
>>>
>>>>
>>>> Also, we could have it togglable with some sort of debug flag. In some
>>>> senses 3 is nice but you would probably end up with 10's or even
>>>> hundreds of hidden buffers which seems bad. In 2 you see them so you
>>>> probably kill them as you go but I think they would be pretty
>>>> annoying. A key difference from the accumulated show/search/pick buffers
>>>> is that, at some point, you did want to see those buffers.
>>>>
>>>> Since all these approaches are easy to implement it is really up to us
>>>> which we want.
>>>>
>>>> Any thoughts?
>>>>
>>>> Mark
>>>>
>>>>
>>>>>
>>>>> Of course those buffers would accumulate, along with show, search and
>>>>> pick buffers...
>>>>>
>>>>> Or we could push this as is, and add some debugging facility later like
>>>>> a variable notmuch-view-errors-buffer.
>>>>>
>>>>> d
>>>> _______________________________________________
>>>> notmuch mailing list
>>>> notmuch@notmuchmail.org
>>>> http://notmuchmail.org/mailman/listinfo/notmuch
>> _______________________________________________
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] emacs: show: stop stderr appearing in buffer

[Tomi Ollila]

On Thu, Sep 12 2013, Mark Walters <markwalters1009@gmail.com> wrote:

> On Thu, 12 Sep 2013, Tomi Ollila <tomi.ollila@iki.fi> wrote:
>> On Thu, Sep 12 2013, Mark Walters <markwalters1009@gmail.com> wrote:
>>
>>> On Thu, 12 Sep 2013, Tomi Ollila <tomi.ollila@iki.fi> wrote:
>>>>>
>>>>> Yes this is easy. There are several possibilities and I am not sure
>>>>> which is best (some are clearly bad but are worth mentioning anyway).
>>>>>
>>>>> 1) have a single buffer for part errors; this would accumulate stuff and
>>>>> output seems to get interleaved so this is probably useless.

Actually this option would be better than 4 -- we get some output and we
could test whether this (temporary) solution work. It is also better than
the current when we get output into show buffer. Then -- probably --
sometime aroung 2017-2018 if we've got enough bug reports an be also
annoyed ourselves about it (and no-one else have sent tolerable patches
to fix it) we revisit some other fixes.

Simplest would be just create the buffer (like *notmuch Async* ;) and
use that. A bit more complex would be to output the start date of
async operation that is started (and maybe create mechanism to keep
this (these) message buffers to a certain upper size).


>>>>> 2) have a buffer for each part viewer as you describe.
>>>>>
>>>>> 3) have a buffer for each part viewer but start its name with a space so
>>>>> it doesn't show up in buffer lists but is findable (maybe)
>>>>>
>>>>> 4) stick with just the temp buffer approach
>>>>
>>>>
>>>> Maybe check whether the temp buffer is empty. if not, use
>>>> (buffer-string) & (notmuch-logged-error) to append the message
>>>> to the *Notmuch errors* buffer... just that notmuch-logged-error
>>>> signals an error which we may not want to do...
>>>
>>> The problem is that the external part viewer is run asynchronously. So
>>> when we get to the decision what to do the buffer has not received any
>>> output yet even when the first thing the viewer does is output
>>> something.
>>>
>>> A further complication is that in some cases the viewer might not
>>> output things until it is closed and that could be arbitrarily later.
>>
>> I may not have understood completely -- but the temp buffer has lifetime...
>> We could store the timestamp when executing function starts and just
>> before the temp buffer is about to be wiped out, do the emptiness check
>> and if not empty use the stored timestamp & current time in the message
>> to be written to aid the human reader who may want to see the message...
>
> The temp buffer is killed as soon as mm-display-external returns which
> is almost instantly as it starts the external viewer asynchronously. The
> sentinel for the external viewer (called when the external viewer exits)
> sees the calling buffer is dead and just drops the error/output.
>
> So it is not really that the error goes into the temp buffer: it just
> doesn't go into the (quite likely still existing) show buffer.

Ack. It seems I did not (even) read your original commit message >;)

>
> Best wishes
>
> Mark
>

Tomi

[PATCH] emacs: show: stop stderr appearing in buffer

[Mark Walters]

In emacs 24.3+ the stdout/stderr from externally displaying an
attachment gets inserted into the show buffer. This is caused by
changes in mm-display-external in mm-decode.el.

Ideally, we would put this output in the notmuch errors buffer but the
handler is called asynchronously so we don't know when the output will
appear. Thus if we put it straight into the errors buffer it could get
interleaved with other errors. Also we can't easily tell when we
have got all the error output so can't wait until the process is complete.

One solution would be to create a new buffer for the stderr of each
attachment viewed. Again, since we can't tell when the process has
finished, we can't close these buffers automatically so this will
leave lots of buffers around.

Thus we add a debug variable notmuch-show-attachment-debug: it this is
non-nil we create a new buffer for each viewer; if this variable is
nil we just use a temp buffer which means all error output is
discarded (this is the same behaviour as with emacs pre 24.3).
---

This is a new version of
id:1378502198-7980-1-git-send-email-markwalters1009@gmail.com I don't
think there was complete consensus on the right thing to do here but
most people wanted at least the option of seeing the stderr/stdout.

This keeps the default behaviour the same as before, but adds a debug
variable to put the stderr/stdout in its own buffer.

Note: I always use emacs 23 so this is not heavily tested.

Best wishes

Mark


 emacs/notmuch-show.el |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index f00273a..f9a3344 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -159,6 +159,8 @@ indentation."
 (make-variable-buffer-local 'notmuch-show-indent-content)
 (put 'notmuch-show-indent-content 'permanent-local t)
 
+(defvar notmuch-show-attachment-debug nil)
+
 (defcustom notmuch-show-stash-mlarchive-link-alist
   '(("Gmane" . "http://mid.gmane.org/")
     ("MARC" . "http://marc.info/?i=")
@@ -2089,8 +2091,16 @@ caller is responsible for killing this buffer as appropriate."
 This ensures that the temporary buffer created for the mm-handle
 is destroyed when FN returns."
   (let ((handle (notmuch-show-current-part-handle)))
+    ;; emacs 24.3+ puts stdout/stderr into the calling buffer so we
+    ;; call it from a temp-buffer, unless
+    ;; notmuch-show-attachment-debug is non-nil in which case we put
+    ;; it in " *notmuch-part*".
     (unwind-protect
-	(funcall fn handle)
+	(if notmuch-show-attachment-debug
+	    (with-current-buffer (generate-new-buffer " *notmuch-part*")
+	      (funcall fn handle))
+	  (with-temp-buffer
+	    (funcall fn handle)))
       (kill-buffer (mm-handle-buffer handle)))))
 
 (defun notmuch-show-part-button-default (&optional button)
-- 
1.7.9.1

Re: [PATCH] emacs: show: stop stderr appearing in buffer

[Tomi Ollila]

On Wed, Nov 13 2013, Mark Walters <markwalters1009@gmail.com> wrote:

> In emacs 24.3+ the stdout/stderr from externally displaying an
> attachment gets inserted into the show buffer. This is caused by
> changes in mm-display-external in mm-decode.el.
>
> Ideally, we would put this output in the notmuch errors buffer but the
> handler is called asynchronously so we don't know when the output will
> appear. Thus if we put it straight into the errors buffer it could get
> interleaved with other errors. Also we can't easily tell when we
> have got all the error output so can't wait until the process is complete.
>
> One solution would be to create a new buffer for the stderr of each
> attachment viewed. Again, since we can't tell when the process has
> finished, we can't close these buffers automatically so this will
> leave lots of buffers around.
>
> Thus we add a debug variable notmuch-show-attachment-debug: it this is
> non-nil we create a new buffer for each viewer; if this variable is
> nil we just use a temp buffer which means all error output is
> discarded (this is the same behaviour as with emacs pre 24.3).
> ---

This patch looks tolerable to me. +1

Tomi

>
> This is a new version of
> id:1378502198-7980-1-git-send-email-markwalters1009@gmail.com I don't
> think there was complete consensus on the right thing to do here but
> most people wanted at least the option of seeing the stderr/stdout.
>
> This keeps the default behaviour the same as before, but adds a debug
> variable to put the stderr/stdout in its own buffer.
>
> Note: I always use emacs 23 so this is not heavily tested.
>
> Best wishes
>
> Mark
>
>
>  emacs/notmuch-show.el |   12 +++++++++++-
>  1 files changed, 11 insertions(+), 1 deletions(-)
>
> diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
> index f00273a..f9a3344 100644
> --- a/emacs/notmuch-show.el
> +++ b/emacs/notmuch-show.el
> @@ -159,6 +159,8 @@ indentation."
>  (make-variable-buffer-local 'notmuch-show-indent-content)
>  (put 'notmuch-show-indent-content 'permanent-local t)
>  
> +(defvar notmuch-show-attachment-debug nil)
> +
>  (defcustom notmuch-show-stash-mlarchive-link-alist
>    '(("Gmane" . "http://mid.gmane.org/")
>      ("MARC" . "http://marc.info/?i=")
> @@ -2089,8 +2091,16 @@ caller is responsible for killing this buffer as appropriate."
>  This ensures that the temporary buffer created for the mm-handle
>  is destroyed when FN returns."
>    (let ((handle (notmuch-show-current-part-handle)))
> +    ;; emacs 24.3+ puts stdout/stderr into the calling buffer so we
> +    ;; call it from a temp-buffer, unless
> +    ;; notmuch-show-attachment-debug is non-nil in which case we put
> +    ;; it in " *notmuch-part*".
>      (unwind-protect
> -	(funcall fn handle)
> +	(if notmuch-show-attachment-debug
> +	    (with-current-buffer (generate-new-buffer " *notmuch-part*")
> +	      (funcall fn handle))
> +	  (with-temp-buffer
> +	    (funcall fn handle)))
>        (kill-buffer (mm-handle-buffer handle)))))
>  
>  (defun notmuch-show-part-button-default (&optional button)
> -- 
> 1.7.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] emacs: show: stop stderr appearing in buffer

[David Bremner]

Mark Walters <markwalters1009@gmail.com> writes:

> Thus we add a debug variable notmuch-show-attachment-debug: it this is
> non-nil we create a new buffer for each viewer; if this variable is
> nil we just use a temp buffer which means all error output is
> discarded (this is the same behaviour as with emacs pre 24.3).

This seems to work fine, but I couldn't find the temp buffer name until
I looked at the source. How about a docstring?

d

[PATCH v2] emacs: show: stop stderr appearing in buffer

[Mark Walters]

In emacs 24.3+ the stdout/stderr from externally displaying an
attachment gets inserted into the show buffer. This is caused by
changes in mm-display-external in mm-decode.el.

Ideally, we would put this output in the notmuch errors buffer but the
handler is called asynchronously so we don't know when the output will
appear. Thus if we put it straight into the errors buffer it could get
interleaved with other errors. Also we can't easily tell when we
have got all the error output so can't wait until the process is complete.

One solution would be to create a new buffer for the stderr of each
attachment viewed. Again, since we can't tell when the process has
finished, we can't close these buffers automatically so this will
leave lots of buffers around.

Thus we add a debug variable notmuch-show-attachment-debug: it this is
non-nil we create a new buffer for each viewer; if this variable is
nil we just use a temp buffer which means all error output is
discarded (this is the same behaviour as with emacs pre 24.3).
---

This version just adds a docstring to the debug variable describing
what it does and where the debug data is stored (ie what the buffer
name is).

Best wishes

Mark

 emacs/notmuch-show.el |   19 ++++++++++++++++++-
 1 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index f00273a..784644c 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -159,6 +159,15 @@ indentation."
 (make-variable-buffer-local 'notmuch-show-indent-content)
 (put 'notmuch-show-indent-content 'permanent-local t)
 
+(defvar notmuch-show-attachment-debug nil
+  "If t log stdout and stderr from attachment handlers
+
+When set to nil (the default) stdout and stderr from attachment
+handlers is discarded. When set to t the stdout and stderr from
+each attachment handler is logged in buffers with names beginning
+\" *notmuch-part*\". This option requires emacs version at least
+24.3 to work.")
+
 (defcustom notmuch-show-stash-mlarchive-link-alist
   '(("Gmane" . "http://mid.gmane.org/")
     ("MARC" . "http://marc.info/?i=")
@@ -2089,8 +2098,16 @@ caller is responsible for killing this buffer as appropriate."
 This ensures that the temporary buffer created for the mm-handle
 is destroyed when FN returns."
   (let ((handle (notmuch-show-current-part-handle)))
+    ;; emacs 24.3+ puts stdout/stderr into the calling buffer so we
+    ;; call it from a temp-buffer, unless
+    ;; notmuch-show-attachment-debug is non-nil in which case we put
+    ;; it in " *notmuch-part*".
     (unwind-protect
-	(funcall fn handle)
+	(if notmuch-show-attachment-debug
+	    (with-current-buffer (generate-new-buffer " *notmuch-part*")
+	      (funcall fn handle))
+	  (with-temp-buffer
+	    (funcall fn handle)))
       (kill-buffer (mm-handle-buffer handle)))))
 
 (defun notmuch-show-part-button-default (&optional button)
-- 
1.7.9.1