bug#18133: Suppressing asynchronous command output

bug#18133: Suppressing asynchronous command output

[Reuben Thomas]

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

When using an asychronous command, e.g. & from dired-mode, it would be nice
if it didn't pop up the buffer until output was received. Often, no output
is received, for example, when using an asynchronous command to start an
external viewer (here, it makes sense to start it asynchronously, as the
user doesn't want Emacs to block until the viewer exits).

This thread:
https://groups.google.com/forum/#!topic/gnu.emacs.help/xrs6ny67c_4
discusses the issue, and gives some workarounds and partial solutions; but
would there be any disadvantage to changing the behavior to pop up the
buffer when input arrives, and otherwise not do so?

-- 
http://rrt.sc3d.org

[-- Attachment #2: Type: text/html, Size: 884 bytes --]

bug#18133: Suppressing asynchronous command output

[Juri Linkov]

> When using an asychronous command, e.g. & from dired-mode, it would be nice
> if it didn't pop up the buffer until output was received. Often, no output
> is received, for example, when using an asynchronous command to start an
> external viewer (here, it makes sense to start it asynchronously, as the
> user doesn't want Emacs to block until the viewer exits).
>
> This thread:
> https://groups.google.com/forum/#!topic/gnu.emacs.help/xrs6ny67c_4
> discusses the issue, and gives some workarounds and partial solutions; but
> would there be any disadvantage to changing the behavior to pop up the
> buffer when input arrives, and otherwise not do so?

It's possible to display *Async Shell Command* only
when the command finishes with empty input using:

  (add-to-list 'display-buffer-alist '("\\*Async Shell Command\\*"
                                       display-buffer-no-window (nil)))

  (advice-add 'shell-command-sentinel :after
              (lambda (process signal)
                (when (and (string-match-p "\\*Async Shell Command\\*"
                                           (buffer-name (process-buffer process)))
                           (memq (process-status process) '(exit signal))
                           (not (zerop (buffer-size (process-buffer process)))))
                  (display-buffer (process-buffer process)))))

or when the first output is received:

  (add-to-list 'display-buffer-alist '("\\*Async Shell Command\\*"
                                       display-buffer-no-window (nil)))

  (advice-add 'comint-output-filter :after
              (lambda (process string)
                (when (and (string-match-p "\\*Async Shell Command\\*"
                                           (buffer-name (process-buffer process))))
                  (display-buffer (process-buffer process)))))

bug#18133: Suppressing asynchronous command output

[Reuben Thomas]

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

On 30 July 2014 00:47, Juri Linkov <juri@jurta.org> wrote:

> > When using an asychronous command, e.g. & from dired-mode, it would be
> nice
> > if it didn't pop up the buffer until output was received. Often, no
> output
> > is received, for example, when using an asynchronous command to start an
> > external viewer (here, it makes sense to start it asynchronously, as the
> > user doesn't want Emacs to block until the viewer exits).
> >
> > This thread:
> > https://groups.google.com/forum/#!topic/gnu.emacs.help/xrs6ny67c_4
> > discusses the issue, and gives some workarounds and partial solutions;
> but
> > would there be any disadvantage to changing the behavior to pop up the
> > buffer when input arrives, and otherwise not do so?
>
> It's possible to display *Async Shell Command* only
> when the command finishes with empty input using:
>
>   (add-to-list 'display-buffer-alist '("\\*Async Shell Command\\*"
>                                        display-buffer-no-window (nil)))
>
>   (advice-add 'shell-command-sentinel :after
>               (lambda (process signal)
>                 (when (and (string-match-p "\\*Async Shell Command\\*"
>                                            (buffer-name (process-buffer
> process)))
>                            (memq (process-status process) '(exit signal))
>                            (not (zerop (buffer-size (process-buffer
> process)))))
>                   (display-buffer (process-buffer process)))))
>
> or when the first output is received:
>
>   (add-to-list 'display-buffer-alist '("\\*Async Shell Command\\*"
>                                        display-buffer-no-window (nil)))
>
>   (advice-add 'comint-output-filter :after
>               (lambda (process string)
>                 (when (and (string-match-p "\\*Async Shell Command\\*"
>                                            (buffer-name (process-buffer
> process))))
>                   (display-buffer (process-buffer process)))))
>

Thanks very much. The second form seems like the "right" one: when running
a command asynchronously, output should be immediately visible.

Is this suitable also to be used as the basis of a patch to Emacs?

-- 
http://rrt.sc3d.org

[-- Attachment #2: Type: text/html, Size: 3141 bytes --]

bug#18133: Suppressing asynchronous command output

[Reuben Thomas]

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

On 30 July 2014 10:16, Reuben Thomas <rrt@sc3d.org> wrote:

>
>   (add-to-list 'display-buffer-alist '("\\*Async Shell Command\\*"
>>                                        display-buffer-no-window (nil)))
>>
>>   (advice-add 'comint-output-filter :after
>>               (lambda (process string)
>>                 (when (and (string-match-p "\\*Async Shell Command\\*"
>>                                            (buffer-name (process-buffer
>> process))))
>>                   (display-buffer (process-buffer process)))))
>>
>
> Thanks very much. The second form seems like the "right" one: when running
> a command asynchronously, output should be immediately visible.
>

Unfortunately, this code relies on features in the upcoming 24.4. I've
rewritten it to work in 24.3, and fixed a bug:

(add-to-list 'display-buffer-alist '("\\*Async Shell Command\\*"
                                     display-buffer-no-window
(allow-no-window . t)))

(defadvice comint-output-filter (after delay-ashell-sync-command-output
(process string))
  "Stop Async Shell Command output from appearing until there is some
output"
  (when (and (string-match-p "\\*Async Shell Command\\*"
                             (buffer-name (process-buffer process))))
    (display-buffer (process-buffer process))))

The argument to display-buffer-no-window now uses allow-no-window as it
should, and I've rewritten the call to advice-add as a defadvice.

-- 
http://rrt.sc3d.org

[-- Attachment #2: Type: text/html, Size: 2411 bytes --]

bug#18133: Suppressing asynchronous command output

[Juri Linkov]

> Unfortunately, this code relies on features in the upcoming 24.4. I've
> rewritten it to work in 24.3, and fixed a bug:
>
> (add-to-list 'display-buffer-alist '("\\*Async Shell Command\\*"
>                                      display-buffer-no-window
> (allow-no-window . t)))
>
> (defadvice comint-output-filter (after delay-ashell-sync-command-output
> (process string))
>   "Stop Async Shell Command output from appearing until there is some
> output"
>   (when (and (string-match-p "\\*Async Shell Command\\*"
>                              (buffer-name (process-buffer process))))
>     (display-buffer (process-buffer process))))
>
> The argument to display-buffer-no-window now uses allow-no-window as it
> should, and I've rewritten the call to advice-add as a defadvice.

In 24.4 there is no need to add (allow-no-window . t)
because `shell-command' already provides this alist in its call:

  (display-buffer buffer '(nil (allow-no-window . t)))

> Is this suitable also to be used as the basis of a patch to Emacs?

Using defadvice is undesirable for the core files, so
if this feature is useful enough then a new user customizable
variable could be added to optionally enable displaying
the output buffer when the first output is received
using code like in this defadvice.

bug#18133: Suppressing asynchronous command output

[Reuben Thomas]

[-- Attachment #1.1: Type: text/plain, Size: 430 bytes --]

Attached, a patch for this bug.

It makes hiding the output buffer until there is output the default
behavior; if the user wishes to get the old behavior back, then
display-buffer-alist can be customized to remove the new default element.

If this patch is acceptable, I can add a NEWS item and install it.

As a side effect, the process object is now made available to input and
output filter functions.

-- 
http://rrt.sc3d.org

[-- Attachment #1.2: Type: text/html, Size: 993 bytes --]

[-- Attachment #2: 0002-Delay-showing-Async-Shell-Command-buffer-until-outpu.patch --]
[-- Type: text/x-patch, Size: 6506 bytes --]

From c478dad4d4f25980f215c0f99af03520bd2643fa Mon Sep 17 00:00:00 2001
From: Reuben Thomas <rrt@sc3d.org>
Date: Mon, 19 Dec 2016 15:38:36 +0000
Subject: [PATCH 2/6] Delay showing Async Shell Command buffer until output
 (Bug#18133)

* lisp/comint.el (comint-input-filter): Add process argument.
(comint-output-filter-functions): Add `comint-make-buffer-visible' to
the list, document the second argument (the process).
(comint-send-input): Call input and output filter functions with
process argument.
(comint-output-filter): Call output filter functions with process
argument.
(comint-make-buffer-visible): New hook function, to make Async Shell
Command buffer visible.
(comint-postoutput-scroll-to-bottom):
(comint-watch-for-password-prompt): Add dummy process argument.
* lisp/window.el (display-buffer-alist): Add a default entry to hide
Async Shell Command output window.
* lisp/ansi-color.el (ansi-color-process-output): Add a dummy process
argument.
---
 lisp/ansi-color.el |  2 +-
 lisp/comint.el     | 31 ++++++++++++++++++++-----------
 lisp/window.el     |  3 ++-
 3 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/lisp/ansi-color.el b/lisp/ansi-color.el
index 788a7bd..ac3a910 100644
--- a/lisp/ansi-color.el
+++ b/lisp/ansi-color.el
@@ -207,7 +207,7 @@ ansi-color-for-comint-mode-filter
   (setq ansi-color-for-comint-mode 'filter))
 
 ;;;###autoload
-(defun ansi-color-process-output (ignored)
+(defun ansi-color-process-output (_string _process)
   "Maybe translate SGR control sequences of comint output into text properties.
 
 Depending on variable `ansi-color-for-comint-mode' the comint output is
diff --git a/lisp/comint.el b/lisp/comint.el
index b9c65b0..23b5d9a 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -395,16 +395,17 @@ comint-input-filter
 
 (defvar comint-input-filter-functions '()
   "Abnormal hook run before input is sent to the process.
-These functions get one argument, a string containing the text to send.")
+These functions get two arguments: a string containing the text to send,
+and the process.")
 
 ;;;###autoload
-(defvar comint-output-filter-functions '(ansi-color-process-output comint-postoutput-scroll-to-bottom comint-watch-for-password-prompt)
+(defvar comint-output-filter-functions '(comint-make-buffer-visible ansi-color-process-output comint-postoutput-scroll-to-bottom comint-watch-for-password-prompt)
   "Functions to call after output is inserted into the buffer.
 One possible function is `comint-postoutput-scroll-to-bottom'.
-These functions get one argument, a string containing the text as originally
-inserted.  Note that this might not be the same as the buffer contents between
-`comint-last-output-start' and the buffer's `process-mark', if other filter
-functions have already modified the buffer.
+These functions get two arguments: a string containing the text as originally
+inserted, and the process.  Note that the string might not be the same as the
+buffer contents between `comint-last-output-start' and the buffer's
+`process-mark', if other filter functions have already modified the buffer.
 
 See also `comint-preoutput-filter-functions'.
 
@@ -1834,7 +1835,8 @@ comint-send-input
 
         (run-hook-with-args 'comint-input-filter-functions
                             (if no-newline input
-                              (concat input "\n")))
+                              (concat input "\n"))
+                            proc)
 
         (let ((beg (marker-position pmark))
               (end (if no-newline (point) (1- (point)))))
@@ -1911,7 +1913,7 @@ comint-send-input
 
         ;; This used to call comint-output-filter-functions,
         ;; but that scrolled the buffer in undesirable ways.
-        (run-hook-with-args 'comint-output-filter-functions "")))))
+        (run-hook-with-args 'comint-output-filter-functions "" proc)))))
 
 (defvar comint-preoutput-filter-functions nil
   "List of functions to call before inserting Comint output into the buffer.
@@ -2069,7 +2071,7 @@ comint-output-filter
 
 	    ;; Run these hooks with point where the user had it.
 	    (goto-char saved-point)
-	    (run-hook-with-args 'comint-output-filter-functions string)
+	    (run-hook-with-args 'comint-output-filter-functions string process)
 	    (set-marker saved-point (point))
 
 	    (goto-char (process-mark process)) ; In case a filter moved it.
@@ -2111,6 +2113,13 @@ comint-output-filter
 	      (add-text-properties prompt-start (point) '(rear-nonsticky t)))
 	    (goto-char saved-point)))))))
 
+(defun comint-make-buffer-visible (_string process)
+  "Make the output buffer visible.
+Useful in `comint-output-filter-functions'."
+  (when (and (string-match-p "\\*Async Shell Command\\*"
+                             (buffer-name (process-buffer process))))
+    (display-buffer (process-buffer process))))
+
 (defun comint-preinput-scroll-to-bottom ()
   "Go to the end of buffer in all windows showing it.
 Movement occurs if point in the selected window is not after the process mark,
@@ -2139,7 +2148,7 @@ comint-preinput-scroll-to-bottom
 (defvar follow-mode)
 (declare-function follow-comint-scroll-to-bottom "follow" (&optional window))
 
-(defun comint-postoutput-scroll-to-bottom (_string)
+(defun comint-postoutput-scroll-to-bottom (_string _process)
   "Go to the end of buffer in some or all windows showing it.
 Do not scroll if the current line is the last line in the buffer.
 Depends on the value of `comint-move-point-for-output' and
@@ -2348,7 +2357,7 @@ send-invisible
 	    (message "Warning: text will be echoed")))
       (error "Buffer %s has no process" (current-buffer)))))
 
-(defun comint-watch-for-password-prompt (string)
+(defun comint-watch-for-password-prompt (string _process)
   "Prompt in the minibuffer for password and send without echoing.
 This function uses `send-invisible' to read and send a password to the buffer's
 process if STRING contains a password prompt defined by
diff --git a/lisp/window.el b/lisp/window.el
index fdb67ed..49d9c33 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -6776,7 +6776,8 @@ display-buffer-overriding-action
 See `display-buffer' for details.")
 (put 'display-buffer-overriding-action 'risky-local-variable t)
 
-(defcustom display-buffer-alist nil
+(defcustom display-buffer-alist '(("\\*Async Shell Command\\*"
+                                   display-buffer-no-window))
   "Alist of conditional actions for `display-buffer'.
 This is a list of elements (CONDITION . ACTION), where:
 
-- 
2.7.4

bug#18133: Suppressing asynchronous command output

[Eli Zaretskii]

> From: Reuben Thomas <rrt@sc3d.org>
> Date: Mon, 19 Dec 2016 15:48:00 +0000
> 
> Attached, a patch for this bug.
> 
> It makes hiding the output buffer until there is output the default behavior; if the user wishes to get the old
> behavior back, then display-buffer-alist can be customized to remove the new default element.

I'm okay with adding this feature, but why turn it on by default right
away?  The original discussion of this bug says nothing about making
this the default behavior.  Perhaps it would be better to let users
use it first, and then turn it on by default if there's popular
demand?

Thanks.

bug#18133: Suppressing asynchronous command output

[Reuben Thomas]

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

On 21 December 2016 at 17:55, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Reuben Thomas <rrt@sc3d.org>
> > Date: Mon, 19 Dec 2016 15:48:00 +0000
> >
> > Attached, a patch for this bug.
> >
> > It makes hiding the output buffer until there is output the default
> behavior; if the user wishes to get the old
> > behavior back, then display-buffer-alist can be customized to remove the
> new default element.
>
> I'm okay with adding this feature, but why turn it on by default right
> away?  The original discussion of this bug says nothing about making
> this the default behavior.  Perhaps it would be better to let users
> use it first, and then turn it on by default if there's popular
> demand?


Would a reasonable way to do this be to add a toggleable option to the
defcustom for display-buffer-alist, and document this in
async-shell-command's docstring?

-- 
http://rrt.sc3d.org

[-- Attachment #2: Type: text/html, Size: 1648 bytes --]

bug#18133: Suppressing asynchronous command output

[Eli Zaretskii]

> From: Reuben Thomas <rrt@sc3d.org>
> Date: Wed, 21 Dec 2016 22:44:33 +0000
> Cc: 18133@debbugs.gnu.org
> 
>  I'm okay with adding this feature, but why turn it on by default right
>  away? The original discussion of this bug says nothing about making
>  this the default behavior. Perhaps it would be better to let users
>  use it first, and then turn it on by default if there's popular
>  demand?
> 
> Would a reasonable way to do this be to add a toggleable option to the defcustom for display-buffer-alist, and
> document this in async-shell-command's docstring?

I think it could be.  Martin, WDYT?

bug#18133: Suppressing asynchronous command output

[martin rudalics]

 >> Would a reasonable way to do this be to add a toggleable option to the defcustom for display-buffer-alist, and
 >> document this in async-shell-command's docstring?
 >
 > I think it could be.  Martin, WDYT?

What is a toggleable option?  A new action function?  Or simply adding
‘display-buffer-no-window’ to the value menu?  Or something completely
different?

martin

bug#18133: Suppressing asynchronous command output

[Eli Zaretskii]

> Date: Thu, 22 Dec 2016 18:53:47 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: 18133@debbugs.gnu.org
> 
>  >> Would a reasonable way to do this be to add a toggleable option to the defcustom for display-buffer-alist, and
>  >> document this in async-shell-command's docstring?
>  >
>  > I think it could be.  Martin, WDYT?
> 
> What is a toggleable option?  A new action function?  Or simply adding
> ‘display-buffer-no-window’ to the value menu?  Or something completely
> different?

Perhaps Reuben could show what he had in mind.

bug#18133: Suppressing asynchronous command output

[Reuben Thomas]

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

On 22 December 2016 at 17:53, martin rudalics <rudalics@gmx.at> wrote:

> >> Would a reasonable way to do this be to add a toggleable option to the
> defcustom for display-buffer-alist, and
> >> document this in async-shell-command's docstring?
> >
> > I think it could be.  Martin, WDYT?
>
> What is a toggleable option?  A new action function?  Or simply adding
> ‘display-buffer-no-window’ to the value menu?  Or something completely
> different?


​I meant the second, just adding the (buffer-name-string .
display-buffer-no-window) pair to the value menu.

-- 
http://rrt.sc3d.org

[-- Attachment #2: Type: text/html, Size: 1214 bytes --]

bug#18133: Suppressing asynchronous command output

[martin rudalics]

 > ​I meant the second, just adding the (buffer-name-string .
 > display-buffer-no-window) pair to the value menu.

Which value menu?  ‘display-buffer-alist’ is a user option - its
original value must be nil, no code is allowed to add anything to it.
If an application urgently wants to override the user, it can use
‘display-buffer-overriding-action’.

Or do you just want to add ‘display-buffer-no-window’ to
‘display-buffer--action-function-custom-type’?

martin

bug#18133: Suppressing asynchronous command output

[Reuben Thomas]

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

On 22 December 2016 at 20:15, martin rudalics <rudalics@gmx.at> wrote:

> > ​I meant the second, just adding the (buffer-name-string .
> > display-buffer-no-window) pair to the value menu.
>
> Which value menu?  ‘display-buffer-alist’ is a user option - its
> original value must be nil, no code is allowed to add anything to it.
>

​That's what I meant, so I guess it will have to be something else.​

If an application urgently wants to override the user, it can use
> ‘display-buffer-overriding-action’.
>

​I'm not trying to override the user here, I'm just trying to avoid adding
another configuration option.​

But even if it were a configuration option, if code can't add anything to
display-buffer-alist, then how should this setting be implemented? (As you
can see, the current implementation relies on changing
display-buffer-alist.) I'm a bit confused, how is display-buffer-alist
different from, say, auto-mode-alist (which can be altered by the user, but
also is added to by code.)

Or do you just want to add ‘display-buffer-no-window’ to
> ‘display-buffer--action-function-custom-type’?
>

It would be nice if the user only had to change one thing to enable hiding
the async output buffer until there is output. If we only added the
function to display-buffer--action-function-custom-type, the user still has
to manually add the right buffer name pattern and the action to
display-buffer-alist.

-- 
http://rrt.sc3d.org

[-- Attachment #2: Type: text/html, Size: 2702 bytes --]

bug#18133: Suppressing asynchronous command output

[martin rudalics]

 > ​I'm not trying to override the user here, I'm just trying to avoid adding
 > another configuration option.​

You change long-standing behavior when you add such an option.  Did
people agree that the buffer should be preferably not displayed?  The
current status is that

   To run COMMAND without displaying the output
   in a window you can configure `display-buffer-alist' to use the action
   `display-buffer-no-window' for the buffer `*Async Shell Command*'.

With your proposal the buffer would not be displayed and the user would
have to delete the ‘display-buffer-alist’ entry to display the buffer.

But if people generally agree that the buffer should not be displayed by
default, you can simply do something like

(display-buffer buffer
		`(,(and (equal (buffer-name buffer) "*Async Shell Command*")
			'display-buffer-no-window)
		  (allow-no-window . t)))

in ‘shell-command’.  This has the same effect without cluttering
‘display-buffer-alist’.

 > It would be nice if the user only had to change one thing to enable hiding
 > the async output buffer until there is output.

But when you add the entry to ‘display-buffer-alist’ the buffer will
already be hidden without any user intervention.

martin

bug#18133: Suppressing asynchronous command output

[Reuben Thomas]

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

On 23 December 2016 at 18:59, martin rudalics <rudalics@gmx.at> wrote:

> > ​I'm not trying to override the user here, I'm just trying to avoid
> adding
> > another configuration option.​
>
> You change long-standing behavior when you add such an option.


​I haven't talked about adding an option.​


> Did
> ​ p​
> eople agree that the buffer should be preferably not displayed?  The
> current status is that
>
>   To run COMMAND without displaying the output
>   in a window you can configure `display-buffer-alist' to use the action
>   `display-buffer-no-window' for the buffer `*Async Shell Command*'.
>
> With your proposal the buffer would not be displayed and the user would
> have to delete the ‘display-buffer-alist’ entry to display the buffer.
>

​That's the case with my original patch. But I agreed with Eli (and you)
that the default behaviour should be the same as at present.​

​I'm just asking how to implement this, preferably without adding another
configuration option.​


> > It would be nice if the user only had to change one thing to enable
> hiding
> > the async output buffer until there is output.
>
> But when you add the entry to ‘display-buffer-alist’ the buffer will
> already be hidden without any user intervention.


​Again, you are describing the behaviour of my original patch. ​I then
suggested adding an option to display-buffer-alist's defcustom
specification. But you said that is not allowed (if I understood correctly).

I will try to outline the current position again:

1. I agree that the current default behaviour should remain as it is.

2. I would like to add the ability to easily turn off displaying the Async
Command output buffer until there is some output.

3. I suggested achieving this by i) adding an option to
display-buffer-alist, and ii) documenting this in shell-command.

4. You stated that it is not allowed for code to change
display-buffer-alist. I was puzzled by this, because other user variables
such as auto-mode-alist are changed by code as well as by the user. In any
case, I am not suggesting automatically changing display-buffer-alist;
rather I have suggested adding an option to the customization menu for
display-buffer-alist.

So, maybe you could give your opinion of my current suggestion (3),
ignoring my original patch?

Sorry if I have confused you, and I hope the above makes things clearer.

-- 
http://rrt.sc3d.org

[-- Attachment #2: Type: text/html, Size: 4522 bytes --]

bug#18133: Suppressing asynchronous command output

[martin rudalics]

 > ​Again, you are describing the behaviour of my original patch. ​I then
 > suggested adding an option to display-buffer-alist's defcustom
 > specification. But you said that is not allowed (if I understood correctly).

Yes.  But I still do not understand what that option would be.

 > I will try to outline the current position again:
 >
 > 1. I agree that the current default behaviour should remain as it is.
 >
 > 2. I would like to add the ability to easily turn off displaying the Async
 > Command output buffer until there is some output.

Isn't this much more than changing the way ‘display-buffer’ behaves?
IIUC, you want the buffer to pop up whenever some output arrives, the
user should be allowed to delete the window, and when new output arrives
the buffer should pop up again.  Correct?  Then this is a perfect
candidate for a minor mode where you would remove the ‘allow-no-window’
entry in every ‘display-buffer’ call when new output arrives.

 > 3. I suggested achieving this by i) adding an option to
 > display-buffer-alist, and ii) documenting this in shell-command.
 >
 > 4. You stated that it is not allowed for code to change
 > display-buffer-alist.

Earlier we had a number of options affecting the behavior of
‘display-buffer’ which still exist in some way like
‘same-window-buffer-names’, ‘display-buffer-reuse-frames’ or
‘pop-up-frames’.  These usually ended up being crowded by applications
that added their preferences and customizing these options became very
awkward, usually accompanied by alerts like "changed outside customize"
and similar annoyances.  That's why we disallow changing the value of
this option in our code.

In general, code should refrain from changing the value of user
customizable variables.  They are reserved for the user.

 > I was puzzled by this, because other user variables
 > such as auto-mode-alist

‘auto-mode-alist’ is not a user variable aka option.  Anyone is allowed
to set it.

 > are changed by code as well as by the user. In any
 > case, I am not suggesting automatically changing display-buffer-alist;
 > rather I have suggested adding an option to the customization menu for
 > display-buffer-alist.
 >
 > So, maybe you could give your opinion of my current suggestion (3),
 > ignoring my original patch?
 >
 > Sorry if I have confused you, and I hope the above makes things clearer.

I have not read your original patch.  But I'm still confused by how you
want to add something to ‘display-buffer-alist’ and at the same time to
not change the behavior of ‘display-buffer’ ;-)

martin

bug#18133: Suppressing asynchronous command output

[Reuben Thomas]

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

On 23 December 2016 at 19:55, martin rudalics <rudalics@gmx.at> wrote:

> ​​
>
> Isn't this much more than changing the way ‘display-buffer’ behaves?
> IIUC, you want the buffer to pop up whenever some output arrives, the
> user should be allowed to delete the window, and when new output arrives
> the buffer should pop up again.  Correct?


Here is the implementation I currently use (from Juri Linkov, message #8,
second block of code):

(advice-add 'comint-output-filter :after
            "Stop Async Shell Command output from appearing until there is
output."
            (lambda (process string)
              (when (and (string-match-p "\\*Async Shell Command\\*"
                                         (buffer-name (process-buffer
process))))
                (display-buffer (process-buffer process)))))

I think this does what you say: whenever some output arrives, the buffer
pops up. It does not involve changing the behaviour of display-buffer.

All my patch does is move the advice into a function suitable for
comint-output-filter-functions.

But I'm still confused by how you
> want to add something to ‘display-buffer-alist’ and at the same time to
> not change the behavior of ‘display-buffer’ ;-)

​
I suggested adding an option to display-buffer-alist's defcustom
specification, something like:

                :options (((regexp "\\*Async Shell Command\\*") (function
display-buffer-no-window)))

By default, this is not selected, so it does not change the default
behaviour.

I currently simply add an item to display-buffer-alist:

(add-to-list 'display-buffer-alist '("\\*Async Shell Command\\*"
                                     display-buffer-no-window))

-- 
http://rrt.sc3d.org

[-- Attachment #2: Type: text/html, Size: 3601 bytes --]

bug#18133: Suppressing asynchronous command output

[martin rudalics]

 > Here is the implementation I currently use (from Juri Linkov, message #8,
 > second block of code):
 >
 > (advice-add 'comint-output-filter :after
 >              "Stop Async Shell Command output from appearing until there is
 > output."
 >              (lambda (process string)
 >                (when (and (string-match-p "\\*Async Shell Command\\*"
 >                                           (buffer-name (process-buffer
 > process))))
 >                  (display-buffer (process-buffer process)))))
 >
 > I think this does what you say: whenever some output arrives, the buffer
 > pops up. It does not involve changing the behaviour of display-buffer.
 >
 > All my patch does is move the advice into a function suitable for
 > comint-output-filter-functions.

To turn this into a minor mode I would do two things:

(1) Add an entry to ‘display-buffer-overriding-action’ so that the
     buffer is not displayed right away.

(2) Get rid of the advice, maybe by doing the display at the end of
     ‘comint-output-filter-functions’.

 > But I'm still confused by how you
 >> >want to add something to ‘display-buffer-alist’ and at the same time to
 >> >not change the behavior of ‘display-buffer’;-)
 > ​
 > I suggested adding an option to display-buffer-alist's defcustom
 > specification, something like:
 >
 >                  :options (((regexp "\\*Async Shell Command\\*") (function
 > display-buffer-no-window)))
 >
 > By default, this is not selected, so it does not change the default
 > behaviour.

Please don't do such things.  Within a couple of months we would be back
where we were before the introduction of ‘display-buffer-alist’.

 > I currently simply add an item to display-buffer-alist:
 >
 > (add-to-list 'display-buffer-alist '("\\*Async Shell Command\\*"
 >                                       display-buffer-no-window))

If you now try to customize ‘display-buffer-alist’, you'll be told that
this option has been "CHANGED outside Customize".  You can avoid that by
doing

(customize-save-variable
  'display-buffer-alist
  (add-to-list
   'display-buffer-alist '("\\*Async Shell Command\\*"
			  display-buffer-no-window)))

instead.

martin

bug#18133: Suppressing asynchronous command output

[Reuben Thomas]

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

On 24 December 2016 at 09:16, martin rudalics <rudalics@gmx.at> wrote:

> > Here is the implementation I currently use (from Juri Linkov, message #8,
> > second block of code):
> >
> > (advice-add 'comint-output-filter :after
> >              "Stop Async Shell Command output from appearing until there
> is
> > output."
> >              (lambda (process string)
> >                (when (and (string-match-p "\\*Async Shell Command\\*"
> >                                           (buffer-name (process-buffer
> > process))))
> >                  (display-buffer (process-buffer process)))))
> >
> > I think this does what you say: whenever some output arrives, the buffer
> > pops up. It does not involve changing the behaviour of display-buffer.
> >
> > All my patch does is move the advice into a function suitable for
> > comint-output-filter-functions.
>
> To turn this into a minor mode I would do two things:
>

​Please, start from the patch I provided, and read the discussion in
previous messages; otherwise, you're partly rehashing old conversations.

In any case, adding a minor mode might be tidier than adding suggestions to
display-buffer-alist, but it's worse in terms of adding complexity to
Emacs. It's not worth it.

In my view, what I suggested is simply the correct behaviour, and should be
the default.

In Eli's view, we should not change the default unless it proves popular.

I am therefore seeking a way to make it easier to customize than at present
(as you can see, at present it requires 10 lines of code, which is 10 times
too much).

​Any suggestions?​ Eli?

If you now try to customize ‘display-buffer-alist’, you'll be told that
> this option has been "CHANGED outside Customize".  You can avoid that by
> doing
>
> (customize-save-variable
>  'display-buffer-alist
>  (add-to-list
>   'display-buffer-alist '("\\*Async Shell Command\\*"
>                           display-buffer-no-window)))
>
> instead.
>

​Thanks, I've made that change to my configuration.

-- 
http://rrt.sc3d.org

[-- Attachment #2: Type: text/html, Size: 3618 bytes --]

bug#18133: Suppressing asynchronous command output

[Eli Zaretskii]

> From: Reuben Thomas <rrt@sc3d.org>
> Date: Sat, 24 Dec 2016 11:11:37 +0000
> Cc: Eli Zaretskii <eliz@gnu.org>, 18133@debbugs.gnu.org
> 
>  To turn this into a minor mode I would do two things:
> 
> ​Please, start from the patch I provided, and read the discussion in previous messages; otherwise, you're
> partly rehashing old conversations.
> 
> In any case, adding a minor mode might be tidier than adding suggestions to display-buffer-alist, but it's worse
> in terms of adding complexity to Emacs. It's not worth it.
> 
> In my view, what I suggested is simply the correct behaviour, and should be the default.
> 
> In Eli's view, we should not change the default unless it proves popular.
> 
> I am therefore seeking a way to make it easier to customize than at present (as you can see, at present it
> requires 10 lines of code, which is 10 times too much).
> 
> ​Any suggestions?​ Eli?

Not sure why this went astray so much.  Let me try bringing it back on
track.

display-buffer-alist is a defcustom.  The issue at hand is to offer
users a way of customizing display-buffer-alist such that buffers
whose names match "*Async Shell Command*" will not be displayed unless
they have something to show the user.

Martin, what can we add to the display-buffer-alist's defcustom to
allow users easy customization to that effect?

Thanks.

bug#18133: Suppressing asynchronous command output

[martin rudalics]

 > display-buffer-alist is a defcustom.  The issue at hand is to offer
 > users a way of customizing display-buffer-alist such that buffers
 > whose names match "*Async Shell Command*" will not be displayed unless
 > they have something to show the user.
 >
 > Martin, what can we add to the display-buffer-alist's defcustom to
 > allow users easy customization to that effect?

Let me try to explain this once more: The idea of ‘display-buffer-alist’
was to provide an option with a default value of nil.  An option
entirely in the hands of the user, which means that no code is allowed
to change its value.  In the five years since its introduction, new
Emacs code has followed this convention.

As soon as we allow new code to change this as Reuben proposed, nothing
will prevent users from saying that they like a non-nil default value
for the entry in question and a few years later we'll be in the very
situation we had before, namely that of users' customizations dwarfed by
applications adding their own preferences.  We've been there and it was
a mess.

Applications have three ways to affect what ‘display-buffer’ does:

(1) Provide the function to use via an argument.  This can be overridden
     by ‘display-buffer-alist’.

(2) Let-bind ‘display-buffer-alist’ temporarily to a value of its
     preference.  This is a work-around for the case where the
     application does not call ‘display-buffer’ directly.

(3) Use ‘display-buffer-overriding-action’.  This overrides
     ‘display-buffer-alist’ completely.

Is it really so difficult to provide an extra option which allows to
have either of these trigger the wanted behavior?  Also taking into
account that the lazy pop-up behavior Reuben aims at cannot be obtained
by customizing an existing option anyway?

martin

bug#18133: Suppressing asynchronous command output

[Reuben Thomas]

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

On 24 December 2016 at 13:54, martin rudalics <rudalics@gmx.at> wrote:

>
> Applications have three ways to affect what ‘display-buffer’ does:
>
> (1) Provide the function to use via an argument.  This can be overridden
>     by ‘display-buffer-alist’.
>
> (2) Let-bind ‘display-buffer-alist’ temporarily to a value of its
>     preference.  This is a work-around for the case where the
>     application does not call ‘display-buffer’ directly.
>
> (3) Use ‘display-buffer-overriding-action’.  This overrides
>     ‘display-buffer-alist’ completely.
>

​Thanks for this clear summary (and thanks to Eli for helping out!).​


> Is it really so difficult to provide an extra option which allows to
> have either of these trigger the wanted behavior?


​Could you please make a concrete suggestion? I must admit that I don't
feel competent yet, and I've already tried and failed!
​
-- 
http://rrt.sc3d.org

[-- Attachment #2: Type: text/html, Size: 1758 bytes --]

bug#18133: Suppressing asynchronous command output

[Eli Zaretskii]

> Date: Sat, 24 Dec 2016 14:54:00 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: 18133@debbugs.gnu.org
> 
>  > display-buffer-alist is a defcustom.  The issue at hand is to offer
>  > users a way of customizing display-buffer-alist such that buffers
>  > whose names match "*Async Shell Command*" will not be displayed unless
>  > they have something to show the user.
>  >
>  > Martin, what can we add to the display-buffer-alist's defcustom to
>  > allow users easy customization to that effect?
> 
> Let me try to explain this once more: The idea of ‘display-buffer-alist’
> was to provide an option with a default value of nil.  An option
> entirely in the hands of the user, which means that no code is allowed
> to change its value.  In the five years since its introduction, new
> Emacs code has followed this convention.

I'm not talking about any code that would change the default value.
I'm talking about showing the users a non-default value, for them to
select if they want to, that would produce the effect desired here.

IOW, when the user clicks "Value menu", I would like them to see a
value which makes async shell buffers behave like Reuben wants.
That's all.

If you are saying that we must not show any value but nil in the value
menu, then my next question will be why is this variable a defcustom,
if users are not allowed to select non-default values for it.

> Applications have three ways to affect what ‘display-buffer’ does:

We are not talking about any applications, at least I wasn't.  I was
talking about providing another possible value, that'd be easy to
select without writing any Lisp, and which, when selected, will cause
the async shell output buffer be displayed only when there's some
material in it.

> Is it really so difficult to provide an extra option which allows to
> have either of these trigger the wanted behavior?  Also taking into
> account that the lazy pop-up behavior Reuben aims at cannot be obtained
> by customizing an existing option anyway?

AFAIU, customizing display-buffer-alist, an existing option, does in
fact produce the desired effect.

bug#18133: Suppressing asynchronous command output

[martin rudalics]

 > ​Could you please make a concrete suggestion? I must admit that I don't
 > feel competent yet, and I've already tried and failed!

(display-buffer
  buffer
  `(,(and async-shell-lazy-pop-up-mode
	 (equal (buffer-name buffer) "*Async Shell Command*")
	 'display-buffer-no-window)
    (allow-no-window . t)))

martin

bug#18133: Suppressing asynchronous command output

[martin rudalics]

 > I'm not talking about any code that would change the default value.
 > I'm talking about showing the users a non-default value, for them to
 > select if they want to, that would produce the effect desired here.
 >
 > IOW, when the user clicks "Value menu", I would like them to see a
 > value which makes async shell buffers behave like Reuben wants.
 > That's all.

If that's all, just add ‘display-buffer-no-window’ to
‘display-buffer--action-function-custom-type’ as I suggested before.
(BTW Juri should have done that when he added that function.)  But
Reuben replied

   It would be nice if the user only had to change one thing to enable hiding
   the async output buffer until there is output. If we only added the
   function to display-buffer--action-function-custom-type, the user still has
   to manually add the right buffer name pattern and the action to
   display-buffer-alist.

 > If you are saying that we must not show any value but nil in the value
 > menu, then my next question will be why is this variable a defcustom,
 > if users are not allowed to select non-default values for it.
 >
 >> Applications have three ways to affect what ‘display-buffer’ does:
 >
 > We are not talking about any applications, at least I wasn't.  I was
 > talking about providing another possible value, that'd be easy to
 > select without writing any Lisp, and which, when selected, will cause
 > the async shell output buffer be displayed only when there's some
 > material in it.

And how should ‘display-buffer’ know whether "there's some material" in
that buffer?

martin

bug#18133: Suppressing asynchronous command output

[Eli Zaretskii]

> Date: Sat, 24 Dec 2016 17:33:01 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: rrt@sc3d.org, 18133@debbugs.gnu.org
> 
>  > I'm not talking about any code that would change the default value.
>  > I'm talking about showing the users a non-default value, for them to
>  > select if they want to, that would produce the effect desired here.
>  >
>  > IOW, when the user clicks "Value menu", I would like them to see a
>  > value which makes async shell buffers behave like Reuben wants.
>  > That's all.
> 
> If that's all, just add ‘display-buffer-no-window’ to
> ‘display-buffer--action-function-custom-type’ as I suggested before.
> (BTW Juri should have done that when he added that function.)  But
> Reuben replied
> 
>    It would be nice if the user only had to change one thing to enable hiding
>    the async output buffer until there is output. If we only added the
>    function to display-buffer--action-function-custom-type, the user still has
>    to manually add the right buffer name pattern and the action to
>    display-buffer-alist.

Yes, I meant to add a value that would handle "*Async Shell Output*"
buffer like described above.

>  > We are not talking about any applications, at least I wasn't.  I was
>  > talking about providing another possible value, that'd be easy to
>  > select without writing any Lisp, and which, when selected, will cause
>  > the async shell output buffer be displayed only when there's some
>  > material in it.
> 
> And how should ‘display-buffer’ know whether "there's some material" in
> that buffer?

That's up to Reuben, I thought he had this figured out already.

bug#18133: Suppressing asynchronous command output

[martin rudalics]

 >>     It would be nice if the user only had to change one thing to enable hiding
 >>     the async output buffer until there is output. If we only added the
 >>     function to display-buffer--action-function-custom-type, the user still has
 >>     to manually add the right buffer name pattern and the action to
 >>     display-buffer-alist.
 >
 > Yes, I meant to add a value that would handle "*Async Shell Output*"
 > buffer like described above.

The type specification of ‘display-buffer-alist’ goes as:

   :type `(alist :key-type
		(choice :tag "Condition"
			regexp
			(function :tag "Matcher function"))
		:value-type ,display-buffer--action-custom-type)

This associates regexps/matcher functions with actions.  How, in such a
specification, can I splice in a buffer name associated with a key
_without_ assigning that pair to the default of ‘display-buffer-alist’?
Maybe I'm missing some detail of the customization interface.

 >> And how should ‘display-buffer’ know whether "there's some material" in
 >> that buffer?
 >
 > That's up to Reuben, I thought he had this figured out already.

Whatever he figures out, it will affect the decision whether to display
the buffer initially.  You can decide that via a matcher function in
‘display-buffer-alist’ (if the buffer name equals "..." and the buffer
is not empty display it, otherwise not) but then it would be easier to
add a new action function like ‘display-buffer-if-not-empty’.

martin

bug#18133: Suppressing asynchronous command output

[Reuben Thomas]

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

[Apologies, I've been working most of today and am working most of
tomorrow. I'll get back with a concrete patch that attempts to answer
Martin's questions as soon as I can.]

[-- Attachment #2: Type: text/html, Size: 299 bytes --]

bug#18133: Suppressing asynchronous command output

[Juri Linkov]

> Here is the implementation I currently use (from Juri Linkov, message #8,
> second block of code):
>
> (advice-add 'comint-output-filter :after
>             "Stop Async Shell Command output from appearing until there is
> output."
>             (lambda (process string)
>               (when (and (string-match-p "\\*Async Shell Command\\*"
>                                          (buffer-name (process-buffer
> process))))
>                 (display-buffer (process-buffer process)))))

This code is mostly intended for the users to put in their ~/.emacs.
For core implementation, we need a new customizable variable that you
could check in the filter, and display the buffer when necessary.  This is
like other options we already have, e.g. ‘comint-move-point-for-output’,
‘comint-scroll-show-maximum-output’ used from ‘comint-output-filter-functions’.

A good name would be ‘async-shell-command-display-buffer’ with a list of
options when to display the buffer: the first choice to implement now
would be just “on the first output received”, but later we could add
more options like “on every output received”, etc.

Then you could use the code Martin posted here with using such option
in the call to ‘display-buffer’, i.e. the first use of this option
in ‘display-buffer’ is not to display the buffer, and the second use
in the filter-function is to display it on first output.  Please note
that also you have to take care about not displaying the same buffer
many times in filter-function because it is called repeatedly on
small chunks of output, depending on the size of output.

PS: I'm not sure about using async-specific variables like
‘async-shell-command-display-buffer’ in comint-filters,
so maybe better would be add async-specific comint-filters
dynamically in ‘async-shell-command’?

bug#18133: Suppressing asynchronous command output

[Reuben Thomas]

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

On 24 December 2016 at 18:14, martin rudalics <rudalics@gmx.at> wrote:

> >>     It would be nice if the user only had to change one thing to enable
> hiding
> >>     the async output buffer until there is output. If we only added the
> >>     function to display-buffer--action-function-custom-type, the user
> still has
> >>     to manually add the right buffer name pattern and the action to
> >>     display-buffer-alist.
> >
> > Yes, I meant to add a value that would handle "*Async Shell Output*"
> > buffer like described above.
>
> The type specification of ‘display-buffer-alist’ goes as:
>
>   :type `(alist :key-type
>                 (choice :tag "Condition"
>                         regexp
>                         (function :tag "Matcher function"))
>                 :value-type ,display-buffer--action-custom-type)
>
> This associates regexps/matcher functions with actions.  How, in such a
> specification, can I splice in a buffer name associated with a key
> _without_ assigning that pair to the default of ‘display-buffer-alist’?
> Maybe I'm missing some detail of the customization interface.


​The detail I think you're missing is that when you add an ":options"
entry, that is a selectable option, off by default.​

>> And how should ‘display-buffer’ know whether "there's some material" in
> >> that buffer?
>

​The answer to that is, display-buffer does not know.


> > That's up to Reuben, I thought he had this figured out already.
>
> Whatever he figures out, it will affect the decision whether to display
> the buffer initially.


​Initially, the buffer is not displayed. Every time output is added to it,
it is displayed, by the filter function. (According to Juri's later
message, this is inefficient.)

-- 
http://rrt.sc3d.org

[-- Attachment #2: Type: text/html, Size: 3115 bytes --]

bug#18133: Suppressing asynchronous command output

[Reuben Thomas]

[-- Attachment #1.1: Type: text/plain, Size: 572 bytes --]

On 26 December 2016 at 23:27, Juri Linkov <juri@linkov.net> wrote:

[message cut]

Thanks, Juri, for helping again with this bug.

I attach an updated version of my patch, which uses an :options setting for
display-buffer-alist so that the current behaviour remains the default, and
runs a preoutput-filter-function which calls display-buffer only when
output is added to an empty buffer.

I believe this addresses the performance and behaviour issues, without the
need for another user option.

It is also rather shorter than the previous patch.

-- 
http://rrt.sc3d.org

[-- Attachment #1.2: Type: text/html, Size: 1556 bytes --]

[-- Attachment #2: 0001-Delay-showing-Async-Shell-Command-buffer-until-outpu.patch --]
[-- Type: text/x-patch, Size: 3970 bytes --]

From 935d50d93d37d2d4649673a5d52ec4efe4abce7b Mon Sep 17 00:00:00 2001
From: Reuben Thomas <rrt@sc3d.org>
Date: Mon, 19 Dec 2016 15:38:36 +0000
Subject: [PATCH] Delay showing Async Shell Command buffer until output
 (Bug#18133)

* lisp/comint.el (comint-input-filter): Add process argument.
(comint-output-filter-functions): Add `comint-make-buffer-visible' to
the list, document the second argument (the process).
(comint-send-input): Call input and output filter functions with
process argument.
(comint-output-filter): Call output filter functions with process
argument.
(comint-make-buffer-visible): New hook function, to make Async Shell
Command buffer visible.
(comint-postoutput-scroll-to-bottom):
(comint-watch-for-password-prompt): Add dummy process argument.
* lisp/window.el (display-buffer-alist): Add a default entry to hide
Async Shell Command output window.
* lisp/ansi-color.el (ansi-color-process-output): Add a dummy process
argument.
---
 lisp/comint.el | 22 ++++++++++++++++------
 lisp/window.el |  5 ++++-
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/lisp/comint.el b/lisp/comint.el
index b9c65b0..6fa2fc9 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -1913,12 +1913,12 @@ comint-send-input
         ;; but that scrolled the buffer in undesirable ways.
         (run-hook-with-args 'comint-output-filter-functions "")))))
 
-(defvar comint-preoutput-filter-functions nil
+(defvar comint-preoutput-filter-functions '(comint-make-newly-written-buffer-visible)
   "List of functions to call before inserting Comint output into the buffer.
-Each function gets one argument, a string containing the text received
-from the subprocess.  It should return the string to insert, perhaps
-the same string that was received, or perhaps a modified or transformed
-string.
+Each function gets two argument, a string containing the text received
+from the subprocess, and the process.  It should return the string to
+insert, perhaps the same string that was received, or perhaps a modified or
+transformed string.
 
 The functions on the list are called sequentially, and each one is
 given the string returned by the previous one.  The string returned by
@@ -2023,7 +2023,7 @@ comint-output-filter
 		(let ((functions
                        (default-value 'comint-preoutput-filter-functions)))
 		  (while (and functions string)
-		    (setq string (funcall (car functions) string))
+		    (setq string (funcall (car functions) string process))
 		    (setq functions (cdr functions))))
 	      (setq string (funcall (car functions) string)))
 	    (setq functions (cdr functions))))
@@ -2111,6 +2111,16 @@ comint-output-filter
 	      (add-text-properties prompt-start (point) '(rear-nonsticky t)))
 	    (goto-char saved-point)))))))
 
+(defun comint-make-newly-written-buffer-visible (string process)
+  "Make the output buffer visible when output is added to an empty buffer.
+Useful in `comint-preoutput-filter-functions'."
+  (let ((buffer (process-buffer process)))
+    (when (and (= 0 (buffer-size buffer))
+               (string-match-p "\\*Async Shell Command\\*"
+                               (buffer-name buffer)))
+      (display-buffer (process-buffer process))))
+  string)
+
 (defun comint-preinput-scroll-to-bottom ()
   "Go to the end of buffer in all windows showing it.
 Movement occurs if point in the selected window is not after the process mark,
diff --git a/lisp/window.el b/lisp/window.el
index fdb67ed..a49f4a3 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -6797,7 +6797,10 @@ display-buffer-alist
 		(choice :tag "Condition"
 			regexp
 			(function :tag "Matcher function"))
-		:value-type ,display-buffer--action-custom-type)
+		:value-type ,display-buffer--action-custom-type
+		:options (("\\*Async Shell Command\\*"
+                           (cons
+                            (const display-buffer-no-window) (const nil)))))
   :risky t
   :version "24.1"
   :group 'windows)
-- 
2.7.4

bug#18133: Suppressing asynchronous command output

[Reuben Thomas]

[-- Attachment #1.1: Type: text/plain, Size: 201 bytes --]

I've tweaked the patch to have the correct type for the regexp key in the
:options list. This doesn't change the functionality, but makes it
consistent with the declared type.

-- 
http://rrt.sc3d.org

[-- Attachment #1.2: Type: text/html, Size: 501 bytes --]

[-- Attachment #2: 0001-Delay-showing-Async-Shell-Command-buffer-until-outpu.patch --]
[-- Type: text/x-patch, Size: 3979 bytes --]

From 8aaf464ce69aba845b7356d4d192b7da252f18fd Mon Sep 17 00:00:00 2001
From: Reuben Thomas <rrt@sc3d.org>
Date: Mon, 19 Dec 2016 15:38:36 +0000
Subject: [PATCH] Delay showing Async Shell Command buffer until output
 (Bug#18133)

* lisp/comint.el (comint-input-filter): Add process argument.
(comint-output-filter-functions): Add `comint-make-buffer-visible' to
the list, document the second argument (the process).
(comint-send-input): Call input and output filter functions with
process argument.
(comint-output-filter): Call output filter functions with process
argument.
(comint-make-buffer-visible): New hook function, to make Async Shell
Command buffer visible.
(comint-postoutput-scroll-to-bottom):
(comint-watch-for-password-prompt): Add dummy process argument.
* lisp/window.el (display-buffer-alist): Add a default entry to hide
Async Shell Command output window.
* lisp/ansi-color.el (ansi-color-process-output): Add a dummy process
argument.
---
 lisp/comint.el | 22 ++++++++++++++++------
 lisp/window.el |  5 ++++-
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/lisp/comint.el b/lisp/comint.el
index b9c65b0..6fa2fc9 100644
--- a/lisp/comint.el
+++ b/lisp/comint.el
@@ -1913,12 +1913,12 @@ comint-send-input
         ;; but that scrolled the buffer in undesirable ways.
         (run-hook-with-args 'comint-output-filter-functions "")))))
 
-(defvar comint-preoutput-filter-functions nil
+(defvar comint-preoutput-filter-functions '(comint-make-newly-written-buffer-visible)
   "List of functions to call before inserting Comint output into the buffer.
-Each function gets one argument, a string containing the text received
-from the subprocess.  It should return the string to insert, perhaps
-the same string that was received, or perhaps a modified or transformed
-string.
+Each function gets two argument, a string containing the text received
+from the subprocess, and the process.  It should return the string to
+insert, perhaps the same string that was received, or perhaps a modified or
+transformed string.
 
 The functions on the list are called sequentially, and each one is
 given the string returned by the previous one.  The string returned by
@@ -2023,7 +2023,7 @@ comint-output-filter
 		(let ((functions
                        (default-value 'comint-preoutput-filter-functions)))
 		  (while (and functions string)
-		    (setq string (funcall (car functions) string))
+		    (setq string (funcall (car functions) string process))
 		    (setq functions (cdr functions))))
 	      (setq string (funcall (car functions) string)))
 	    (setq functions (cdr functions))))
@@ -2111,6 +2111,16 @@ comint-output-filter
 	      (add-text-properties prompt-start (point) '(rear-nonsticky t)))
 	    (goto-char saved-point)))))))
 
+(defun comint-make-newly-written-buffer-visible (string process)
+  "Make the output buffer visible when output is added to an empty buffer.
+Useful in `comint-preoutput-filter-functions'."
+  (let ((buffer (process-buffer process)))
+    (when (and (= 0 (buffer-size buffer))
+               (string-match-p "\\*Async Shell Command\\*"
+                               (buffer-name buffer)))
+      (display-buffer (process-buffer process))))
+  string)
+
 (defun comint-preinput-scroll-to-bottom ()
   "Go to the end of buffer in all windows showing it.
 Movement occurs if point in the selected window is not after the process mark,
diff --git a/lisp/window.el b/lisp/window.el
index fdb67ed..869ed4c 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -6797,7 +6797,10 @@ display-buffer-alist
 		(choice :tag "Condition"
 			regexp
 			(function :tag "Matcher function"))
-		:value-type ,display-buffer--action-custom-type)
+		:value-type ,display-buffer--action-custom-type
+		:options (((regexp "\\*Async Shell Command\\*")
+                           (cons
+                            (const display-buffer-no-window) (const nil)))))
   :risky t
   :version "24.1"
   :group 'windows)
-- 
2.7.4

bug#18133: Suppressing asynchronous command output

[Eli Zaretskii]

> From: Reuben Thomas <rrt@sc3d.org>
> Date: Tue, 27 Dec 2016 01:20:42 +0000
> Cc: martin rudalics <rudalics@gmx.at>, 18133@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org>
> 
> +(defun comint-make-newly-written-buffer-visible (string process)
> +  "Make the output buffer visible when output is added to an empty buffer.
> +Useful in `comint-preoutput-filter-functions'."
> +  (let ((buffer (process-buffer process)))
> +    (when (and (= 0 (buffer-size buffer))
> +               (string-match-p "\\*Async Shell Command\\*"
> +                               (buffer-name buffer)))
> +      (display-buffer (process-buffer process))))
> +  string)

Why are we hard-coding a certain buffer name in a function that is
supposed to be more general, judging by its name and doc string?

bug#18133: Suppressing asynchronous command output

[martin rudalics]

 >> Maybe I'm missing some detail of the customization interface.
 >
 >
 > ​The detail I think you're missing is that when you add an ":options"
 > entry, that is a selectable option, off by default.​

Right.  That's the one I was missing.

Thanks, martin

bug#18133: Suppressing asynchronous command output

[Reuben Thomas]

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

On 27 December 2016 at 06:23, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Reuben Thomas <rrt@sc3d.org>
> > Date: Tue, 27 Dec 2016 01:20:42 +0000
> > Cc: martin rudalics <rudalics@gmx.at>, 18133@debbugs.gnu.org, Eli
> Zaretskii <eliz@gnu.org>
> >
> > +(defun comint-make-newly-written-buffer-visible (string process)
> > +  "Make the output buffer visible when output is added to an empty
> buffer.
> > +Useful in `comint-preoutput-filter-functions'."
> > +  (let ((buffer (process-buffer process)))
> > +    (when (and (= 0 (buffer-size buffer))
> > +               (string-match-p "\\*Async Shell Command\\*"
> > +                               (buffer-name buffer)))
> > +      (display-buffer (process-buffer process))))
> > +  string)
>
> Why are we hard-coding a certain buffer name in a function that is
> supposed to be more general, judging by its name and doc string?
>

​I found it hard-coded several times in other places, so I hard-coded it
here. What do you suggest?

-- 
http://rrt.sc3d.org

[-- Attachment #2: Type: text/html, Size: 1857 bytes --]

bug#18133: Suppressing asynchronous command output

[Eli Zaretskii]

> From: Reuben Thomas <rrt@sc3d.org>
> Date: Tue, 27 Dec 2016 09:01:32 +0000
> Cc: Juri Linkov <juri@linkov.net>, martin rudalics <rudalics@gmx.at>, 18133@debbugs.gnu.org
> 
>  > +(defun comint-make-newly-written-buffer-visible (string process)
>  > + "Make the output buffer visible when output is added to an empty buffer.
>  > +Useful in `comint-preoutput-filter-functions'."
>  > + (let ((buffer (process-buffer process)))
>  > + (when (and (= 0 (buffer-size buffer))
>  > + (string-match-p "\\*Async Shell Command\\*"
>  > + (buffer-name buffer)))
>  > + (display-buffer (process-buffer process))))
>  > + string)
> 
>  Why are we hard-coding a certain buffer name in a function that is
>  supposed to be more general, judging by its name and doc string?
> 
> ​I found it hard-coded several times in other places, so I hard-coded it here. What do you suggest?

I don't see it hard-coded in any context such as this.

I think the regexp against which buffer names are matched in
comint-make-newly-written-buffer-visible should be customizable, with
"*Async Shell Command*" being (in) the default value.

And another question: where's the user option to turn this feature on
or off (including the default being off)?

Thanks.

bug#18133: Suppressing asynchronous command output

[Reuben Thomas]

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

On 27 December 2016 at 09:28, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Reuben Thomas <rrt@sc3d.org>
> > Date: Tue, 27 Dec 2016 09:01:32 +0000
> > Cc: Juri Linkov <juri@linkov.net>, martin rudalics <rudalics@gmx.at>,
> 18133@debbugs.gnu.org
> >
> >  > +(defun comint-make-newly-written-buffer-visible (string process)
> >  > + "Make the output buffer visible when output is added to an empty
> buffer.
> >  > +Useful in `comint-preoutput-filter-functions'."
> >  > + (let ((buffer (process-buffer process)))
> >  > + (when (and (= 0 (buffer-size buffer))
> >  > + (string-match-p "\\*Async Shell Command\\*"
> >  > + (buffer-name buffer)))
> >  > + (display-buffer (process-buffer process))))
> >  > + string)
> >
> >  Why are we hard-coding a certain buffer name in a function that is
> >  supposed to be more general, judging by its name and doc string?
> >
> > ​I found it hard-coded several times in other places, so I hard-coded it
> here. What do you suggest?
>
> I don't see it hard-coded in any context such as this.
>

​I'm not quite sure what you mean by "in any context such as this". I see
the string repeated in source code, and never assigned to a variable name.
This seems OK, since strings and symbols are pretty much interchangeable
when of this sort (e.g. not internationalised).​


> I think the regexp against which buffer names are matched in
> comint-make-newly-written-buffer-visible should be customizable, with
> "*Async Shell Command*" being (in) the default value.
>

This is already the case.​

And another question: where's the user option to turn this feature on
> or off (including the default being off)?
>

​M-x​ customize-variable RET display-buffer-alist RET

You can tick/untick the option for "*Async Shell Command*", and, when it is
ticked, the regexp can be edited.

-- 
http://rrt.sc3d.org

[-- Attachment #2: Type: text/html, Size: 3368 bytes --]

bug#18133: Suppressing asynchronous command output

[Eli Zaretskii]

> From: Reuben Thomas <rrt@sc3d.org>
> Date: Tue, 27 Dec 2016 22:21:11 +0000
> Cc: Juri Linkov <juri@linkov.net>, martin rudalics <rudalics@gmx.at>, 18133@debbugs.gnu.org
> 
>  > Why are we hard-coding a certain buffer name in a function that is
>  > supposed to be more general, judging by its name and doc string?
>  >
>  > ​I found it hard-coded several times in other places, so I hard-coded it here. What do you suggest?
> 
>  I don't see it hard-coded in any context such as this.
> 
> ​I'm not quite sure what you mean by "in any context such as this". I see the string repeated in source code,
> and never assigned to a variable name. This seems OK, since strings and symbols are pretty much
> interchangeable when of this sort (e.g. not internationalised).​

It appears:

  . in ibuf-ext.el, as one of several strings users can select
  . in tramp-adb.el as a buffer where the shell output should go
  . in tramp.el, likewise
  . in simple.el, likewise
  . in comments in some other files

By contrast, you were hard-coding it in a function that should provide
optional behavior for buffers that are not necessarily related to
shell output.  In that context, I think the user should be able to
instruct Emacs about the buffers which should exhibit this behavior.

>  I think the regexp against which buffer names are matched in
>  comint-make-newly-written-buffer-visible should be customizable, with
>  "*Async Shell Command*" being (in) the default value.
> 
> This is already the case.​

Maybe, but in that case I'm missing something here.

>  And another question: where's the user option to turn this feature on
>  or off (including the default being off)?
> 
> ​M-x​ customize-variable RET display-buffer-alist RET
> 
> You can tick/untick the option for "*Async Shell Command*", and, when it is ticked, the regexp can be edited.

But if I select that, what I get is that the buffer will not be
displayed at all, right?  What will trigger its display when it
becomes non-empty?  Sorry if I'm missing something obvious.

bug#18133: Suppressing asynchronous command output

[Reuben Thomas]

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

On 28 December 2016 at 16:01, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Reuben Thomas <rrt@sc3d.org>
> > Date: Tue, 27 Dec 2016 22:21:11 +0000
> > Cc: Juri Linkov <juri@linkov.net>, martin rudalics <rudalics@gmx.at>,
> 18133@debbugs.gnu.org
> >
> >  > Why are we hard-coding a certain buffer name in a function that is
> >  > supposed to be more general, judging by its name and doc string?
> >  >
> >  > ​I found it hard-coded several times in other places, so I hard-coded
> it here. What do you suggest?
> >
> >  I don't see it hard-coded in any context such as this.
> >
> > ​I'm not quite sure what you mean by "in any context such as this". I
> see the string repeated in source code,
> > and never assigned to a variable name. This seems OK, since strings and
> symbols are pretty much
> > interchangeable when of this sort (e.g. not internationalised).​
>
> It appears:
>
>   . in ibuf-ext.el, as one of several strings users can select
>   . in tramp-adb.el as a buffer where the shell output should go
>   . in tramp.el, likewise
>   . in simple.el, likewise
>   . in comments in some other files
>
> By contrast, you were hard-coding it in a function that should provide
> optional behavior for buffers that are not necessarily related to
> shell output.  In that context, I think the user should be able to
> instruct Emacs about the buffers which should exhibit this behavior.
>

​As I said, the name is editable in M-x customize-variable.​

>  And another question: where's the user option to turn this feature on
> >  or off (including the default being off)?
> >
> > ​M-x​ customize-variable RET display-buffer-alist RET
> >
> > You can tick/untick the option for "*Async Shell Command*", and, when it
> is ticked, the regexp can be edited.
>
> But if I select that, what I get is that the buffer will not be
> displayed at all, right?  What will trigger its display when it
> becomes non-empty?  Sorry if I'm missing something obvious.
>

​​The buffer will be displayed by comint-make-newly-written-buffer-visible,
which I've added to the default value of comint-preoutput-filter-functions.
At present the buffer name is hard coded there, so this will only work for
"*Async Shell Command*".

So, to allow the user to be able to change the name, I suppose another user
option would need to be introduced.

However, that is beyond the scope of this bug, which is simply to change
the behaviour for asynchronous uses of shell-command.

-- 
http://rrt.sc3d.org

[-- Attachment #2: Type: text/html, Size: 3991 bytes --]

bug#18133: Suppressing asynchronous command output

[Eli Zaretskii]

> From: Reuben Thomas <rrt@sc3d.org>
> Date: Wed, 28 Dec 2016 20:58:40 +0000
> Cc: Juri Linkov <juri@linkov.net>, martin rudalics <rudalics@gmx.at>, 18133@debbugs.gnu.org
> 
>  It appears:
> 
>  . in ibuf-ext.el, as one of several strings users can select
>  . in tramp-adb.el as a buffer where the shell output should go
>  . in tramp.el, likewise
>  . in simple.el, likewise
>  . in comments in some other files
> 
>  By contrast, you were hard-coding it in a function that should provide
>  optional behavior for buffers that are not necessarily related to
>  shell output. In that context, I think the user should be able to
>  instruct Emacs about the buffers which should exhibit this behavior.
> 
> ​As I said, the name is editable in M-x customize-variable.​

Yes, but AFAIU, that doesn't affect the function about which I was
commenting.

>  > You can tick/untick the option for "*Async Shell Command*", and, when it is ticked, the regexp can be
>  edited.
> 
>  But if I select that, what I get is that the buffer will not be
>  displayed at all, right? What will trigger its display when it
>  becomes non-empty? Sorry if I'm missing something obvious.
> 
> ​​The buffer will be displayed by comint-make-newly-written-buffer-visible, which I've added to the default value
> of comint-preoutput-filter-functions. At present the buffer name is hard coded there, so this will only work for
> "*Async Shell Command*".
> 
> So, to allow the user to be able to change the name, I suppose another user option would need to be
> introduced.
> 
> However, that is beyond the scope of this bug, which is simply to change the behaviour for asynchronous
> uses of shell-command.

The name comint-make-newly-written-buffer-visible doesn't imply that
it only handles that single buffer.  We could, of course, change the
name so that it did, but IMO making the function customizable wrt
buffers it handles would be a much better way.

bug#18133: Suppressing asynchronous command output

[Juri Linkov]

> The buffer will be displayed by comint-make-newly-written-buffer-visible,
> which I've added to the default value of comint-preoutput-filter-functions.
> At present the buffer name is hard coded there, so this will only work for
> "*Async Shell Command*".

Maybe you could construct a lambda in ‘shell-command’
containing the buffer name dynamically bound to the value of
(or output-buffer "*Async Shell Command*"), then set this lambda
to the process-filter, as we already do in ‘shell-command’ with

(set-process-filter proc 'comint-output-filter)

i.e. something like

(set-process-filter proc `(lambda (process string)
                           (when ...
                            (display-buffer ,(or output-buffer "*Async Shell Command*")))))

> So, to allow the user to be able to change the name, I suppose another user
> option would need to be introduced.

If the above solution will work, then we'll need a new customizable variable
like ‘async-shell-command-display-buffer’.  And also ‘display-buffer’ in
‘shell-command’ will need to be adjusted in the way recommended by Martin.

bug#18133: Suppressing asynchronous command output

[Reuben Thomas]

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

On 29 December 2016 at 23:08, Juri Linkov <juri@linkov.net> wrote:

> > The buffer will be displayed by comint-make-newly-written-
> buffer-visible,
> > which I've added to the default value of comint-preoutput-filter-
> functions.
> > At present the buffer name is hard coded there, so this will only work
> for
> > "*Async Shell Command*".
>
> Maybe you could construct a lambda in ‘shell-command’
> containing the buffer name dynamically bound to the value of
> (or output-buffer "*Async Shell Command*"), then set this lambda
> to the process-filter, as we already do in ‘shell-command’ with
>
> (set-process-filter proc 'comint-output-filter)
>
> i.e. something like
>
> (set-process-filter proc `(lambda (process string)
>                            (when ...
>                             (display-buffer ,(or output-buffer "*Async
> Shell Command*")))))
>
> > So, to allow the user to be able to change the name, I suppose another
> user
> > option would need to be introduced.
>
> If the above solution will work, then we'll need a new customizable
> variable
> like ‘async-shell-command-display-buffer’.  And also ‘display-buffer’ in
> ‘shell-command’ will need to be adjusted in the way recommended by Martin.
>

​I tried to integrate these changes with Eli's feedback, but I got stuck.

"The way recommended by Martin" involves a new minor mode,
async-shell-lazy-pop-up-mode, which I tried to avoid; Eli didn't seem to
support its addition, either.

I'm not sure what the variable async-shell-command-display-buffer is
supposed to contain. (It does not seem to be the name of the buffer to be
matched.)

I am unclear what goes in the ellipsis after "when" in the sample code
above; it seems to imply a test for whether the buffer should be displayed,
but I already handled that in my patch with the preoutput-filter function.

So, the above seems to be an alternative suggestion to Eli's, rather than a
complementary one, as I first thought.

It would be good if you experts could agree on an approach that I could
quickly implement. I'm out of my depth here as far as design goes, and
while I'm learning a bit, I'm not sure it's a good use of the total amount
of time we seem to be expending between us!

-- 
http://rrt.sc3d.org

[-- Attachment #2: Type: text/html, Size: 3688 bytes --]

bug#18133: Suppressing asynchronous command output

[Eli Zaretskii]

> From: Reuben Thomas <rrt@sc3d.org>
> Date: Fri, 30 Dec 2016 18:28:46 +0000
> Cc: Eli Zaretskii <eliz@gnu.org>, martin rudalics <rudalics@gmx.at>, 18133@debbugs.gnu.org
> 
> It would be good if you experts could agree on an approach that I could quickly implement. I'm out of my depth
> here as far as design goes, and while I'm learning a bit, I'm not sure it's a good use of the total amount of time
> we seem to be expending between us!

I'm okay with your last variant, if you change
comint-make-newly-written-buffer-visible to make the list of buffer
names customizable.

Thanks.

bug#18133: Suppressing asynchronous command output

[Reuben Thomas]

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

On 30 December 2016 at 20:50, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Reuben Thomas <rrt@sc3d.org>
> > Date: Fri, 30 Dec 2016 18:28:46 +0000
> > Cc: Eli Zaretskii <eliz@gnu.org>, martin rudalics <rudalics@gmx.at>,
> 18133@debbugs.gnu.org
> >
> > It would be good if you experts could agree on an approach that I could
> quickly implement. I'm out of my depth
> > here as far as design goes, and while I'm learning a bit, I'm not sure
> it's a good use of the total amount of time
> > we seem to be expending between us!
>
> I'm okay with your last variant, if you change
> comint-make-newly-written-buffer-visible to make the list of buffer
> names customizable.
>

​Great, thanks. Juri, Martin, is that OK?

-- 
http://rrt.sc3d.org

[-- Attachment #2: Type: text/html, Size: 1564 bytes --]

bug#18133: Suppressing asynchronous command output

[Juri Linkov]

> "The way recommended by Martin" involves a new minor mode,
> async-shell-lazy-pop-up-mode, which I tried to avoid; Eli didn't seem to
> support its addition, either.

No, not a new minor mode, I meant https://debbugs.gnu.org/18133#47
i.e. in ‘shell-command’ instead of

(display-buffer buffer '(nil (allow-no-window . t)))

we could use

(display-buffer buffer '(display-buffer-no-window (allow-no-window . t)))

when this feature is enabled by the new customizable variable
‘async-shell-command-display-buffer’.

> I'm not sure what the variable async-shell-command-display-buffer is
> supposed to contain. (It does not seem to be the name of the buffer to be
> matched.)

We can't hard-code the name of the buffer because ‘shell-command’
uses the arg ‘output-buffer’, and only if it's nil then by default
"*Async Shell Command*".  So ‘async-shell-command-display-buffer’
shouldn't define the name of the buffer.  Instead, it could be
boolean to enable this feature (or a choice of options for more
future related features like in ‘async-shell-command-buffer’).

> I am unclear what goes in the ellipsis after "when" in the sample code
> above; it seems to imply a test for whether the buffer should be displayed,
> but I already handled that in my patch with the preoutput-filter function.

In the ellipsis goes the code that you already wrote for the filter:

(set-process-filter
 proc
 `(lambda (process string)
    (when (and (= 0 (buffer-size (process-buffer process)))
               (eq (buffer-name (process-buffer process))
                  ,(or output-buffer "*Async Shell Command*")))
      (display-buffer (process-buffer process)))))

i.e. it will handle exactly the same buffer that was provided as an arg
‘output-buffer’ to ‘async-shell-command’.

bug#18133: Suppressing asynchronous command output

[Reuben Thomas]

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

On 30 December 2016 at 22:56, Juri Linkov <juri@linkov.net> wrote:

[snip]

Thanks, Juri; can you converge with Eli, and then I'll have a coherent
proposal I can program?

[-- Attachment #2: Type: text/html, Size: 605 bytes --]

bug#18133: Suppressing asynchronous command output

[Eli Zaretskii]

> From: Reuben Thomas <rrt@sc3d.org>
> Date: Sat, 31 Dec 2016 00:19:21 +0000
> Cc: Eli Zaretskii <eliz@gnu.org>, martin rudalics <rudalics@gmx.at>, 18133@debbugs.gnu.org
> 
> Thanks, Juri; can you converge with Eli, and then I'll have a coherent proposal I can program?

I'm okay with Juri's approach as well.

bug#18133: Suppressing asynchronous command output

[Reuben Thomas]

[-- Attachment #1.1: Type: text/plain, Size: 967 bytes --]

On 31 December 2016 at 08:41, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Reuben Thomas <rrt@sc3d.org>
> > Date: Sat, 31 Dec 2016 00:19:21 +0000
> > Cc: Eli Zaretskii <eliz@gnu.org>, martin rudalics <rudalics@gmx.at>,
> 18133@debbugs.gnu.org
> >
> > Thanks, Juri; can you converge with Eli, and then I'll have a coherent
> proposal I can program?
>
> I'm okay with Juri's approach as well.
>

​Here is a patch that attempts to implement Juri's approach.​

​However, at the moment it doesn't work. I think the problem is to do with
combining process filters. As suggested in the Emacs manual, I use
add-function to combine the new process filter with the default
comint-output-filter. But no matter what I try, the original filter does
not seem to be run any more, even if I pass a trivial function to
add-function. I've studied the documentation, and I can't see what I'm
doing wrong.

-- 
https://rrt.sc3d.org <http://rrt.sc3d.org>

[-- Attachment #1.2: Type: text/html, Size: 1731 bytes --]

[-- Attachment #2: 0001-Allow-async-command-output-buffer-to-be-shown-only-o.patch --]
[-- Type: text/x-patch, Size: 2749 bytes --]

From ee79ead9f2f132e85806c75f30ec861bb094f387 Mon Sep 17 00:00:00 2001
From: Reuben Thomas <rrt@sc3d.org>
Date: Wed, 28 Jun 2017 22:40:33 +0100
Subject: [PATCH] Allow async command output buffer to be shown only on output

* lisp/simple.el (async-shell-command-display-buffer): Add
defcustom.
(shell-command): Use the new defcustom to determine whether to show
the buffer immediately, or add a process filter that shows it only
when there is some output.

Thanks to Juri Linkov and Eli Zaretskii for advice and guidance.
---
 lisp/simple.el | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/lisp/simple.el b/lisp/simple.el
index a5565ab..2f2887e 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -3271,6 +3271,17 @@ async-shell-command-buffer
   :group 'shell
   :version "24.3")
 
+(defcustom async-shell-command-display-buffer t
+  "Whether to display the command buffer immediately.
+If t, display the buffer immediately; if nil, wait until there
+is output."
+  :type '(choice (const :tag "Display buffer immediately"
+			t)
+		 (const :tag "Display buffer on output"
+			nil))
+  :group 'shell
+  :version "26.1")
+
 (defun shell-command--save-pos-or-erase ()
   "Store a buffer position or erase the buffer.
 See `shell-command-dont-erase-buffer'."
@@ -3517,7 +3528,6 @@ shell-command
 		    (setq buffer (get-buffer-create
 				  (or output-buffer "*Async Shell Command*"))))))
 		(with-current-buffer buffer
-		  (display-buffer buffer '(nil (allow-no-window . t)))
                   (shell-command--save-pos-or-erase)
 		  (setq default-directory directory)
 		  (setq proc (start-process "Shell" buffer shell-file-name
@@ -3528,7 +3538,16 @@ shell-command
 		  ;; Use the comint filter for proper handling of carriage motion
 		  ;; (see `comint-inhibit-carriage-motion'),.
 		  (set-process-filter proc 'comint-output-filter)
-		  ))
+                  (if async-shell-command-display-buffer
+                      (display-buffer buffer '(nil (allow-no-window . t)))
+                    (add-function :before (process-filter proc)
+                                  `(lambda (process string)
+                                     (when (and (= 0 (buffer-size (process-buffer process)))
+                                                (eq (buffer-name (process-buffer process))
+                                                    ,(or output-buffer "*Async Shell Command*")))
+                                       (display-buffer (process-buffer process))))
+                                  ))
+                  ))
 	    ;; Otherwise, command is executed synchronously.
 	    (shell-command-on-region (point) (point) command
 				     output-buffer nil error-buffer)))))))
-- 
2.7.4

bug#18133: Suppressing asynchronous command output

[Reuben Thomas]

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

On 28 June 2017 at 22:45, Reuben Thomas <rrt@sc3d.org> wrote:

>
> On 31 December 2016 at 08:41, Eli Zaretskii <eliz@gnu.org> wrote:
>
>> > From: Reuben Thomas <rrt@sc3d.org>
>> > Date: Sat, 31 Dec 2016 00:19:21 +0000
>> > Cc: Eli Zaretskii <eliz@gnu.org>, martin rudalics <rudalics@gmx.at>,
>> 18133@debbugs.gnu.org
>> >
>> > Thanks, Juri; can you converge with Eli, and then I'll have a coherent
>> proposal I can program?
>>
>> I'm okay with Juri's approach as well.
>>
>
> ​Here is a patch that attempts to implement Juri's approach.​
>
> ​However, at the moment it doesn't work. I think the problem is to do with
> combining process filters. As suggested in the Emacs manual, I use
> add-function to combine the new process filter with the default
> comint-output-filter. But no matter what I try, the original filter does
> not seem to be run any more, even if I pass a trivial function to
> add-function. I've studied the documentation, and I can't see what I'm
> doing wrong.
>
​
S
​orry, my diagnosis above is nonsense. The output is indeed being inserted
in the buffer; the problem is with the display of the buffer when
async-shell-command-display-buffer is set to nil. I will have another look.
​

-- 
https://rrt.sc3d.org <http://rrt.sc3d.org>

[-- Attachment #2: Type: text/html, Size: 2520 bytes --]

bug#18133: Suppressing asynchronous command output

[Reuben Thomas]

[-- Attachment #1.1: Type: text/plain, Size: 206 bytes --]

​Here's an update to the previous patch; the problem seemed to be the use
of eq for comparing strings rather than string=.​

It seems to work now.

-- 
https://rrt.sc3d.org <http://rrt.sc3d.org>

[-- Attachment #1.2: Type: text/html, Size: 515 bytes --]

[-- Attachment #2: 0001-Allow-async-command-output-buffer-to-be-shown-only-o.patch --]
[-- Type: text/x-patch, Size: 2754 bytes --]

From a82dcca70d3aa527dee3c03bee3bdfcda03a1bb9 Mon Sep 17 00:00:00 2001
From: Reuben Thomas <rrt@sc3d.org>
Date: Wed, 28 Jun 2017 22:40:33 +0100
Subject: [PATCH] Allow async command output buffer to be shown only on output

* lisp/simple.el (async-shell-command-display-buffer): Add
defcustom.
(shell-command): Use the new defcustom to determine whether to show
the buffer immediately, or add a process filter that shows it only
when there is some output.

Thanks to Juri Linkov and Eli Zaretskii for advice and guidance.
---
 lisp/simple.el | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/lisp/simple.el b/lisp/simple.el
index a5565ab..62422f1 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -3271,6 +3271,17 @@ async-shell-command-buffer
   :group 'shell
   :version "24.3")
 
+(defcustom async-shell-command-display-buffer t
+  "Whether to display the command buffer immediately.
+If t, display the buffer immediately; if nil, wait until there
+is output."
+  :type '(choice (const :tag "Display buffer immediately"
+			t)
+		 (const :tag "Display buffer on output"
+			nil))
+  :group 'shell
+  :version "26.1")
+
 (defun shell-command--save-pos-or-erase ()
   "Store a buffer position or erase the buffer.
 See `shell-command-dont-erase-buffer'."
@@ -3517,7 +3528,6 @@ shell-command
 		    (setq buffer (get-buffer-create
 				  (or output-buffer "*Async Shell Command*"))))))
 		(with-current-buffer buffer
-		  (display-buffer buffer '(nil (allow-no-window . t)))
                   (shell-command--save-pos-or-erase)
 		  (setq default-directory directory)
 		  (setq proc (start-process "Shell" buffer shell-file-name
@@ -3528,7 +3538,16 @@ shell-command
 		  ;; Use the comint filter for proper handling of carriage motion
 		  ;; (see `comint-inhibit-carriage-motion'),.
 		  (set-process-filter proc 'comint-output-filter)
-		  ))
+                  (if async-shell-command-display-buffer
+                      (display-buffer buffer '(nil (allow-no-window . t)))
+                    (add-function :before (process-filter proc)
+                                  `(lambda (process string)
+                                     (when (and (= 0 (buffer-size (process-buffer process)))
+                                                (string= (buffer-name (process-buffer process))
+                                                    ,(or output-buffer "*Async Shell Command*")))
+                                       (display-buffer (process-buffer process))))
+                                  ))
+                  ))
 	    ;; Otherwise, command is executed synchronously.
 	    (shell-command-on-region (point) (point) command
 				     output-buffer nil error-buffer)))))))
-- 
2.7.4

bug#18133: Suppressing asynchronous command output

[Reuben Thomas]

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

On 28 June 2017 at 23:05, Reuben Thomas <rrt@sc3d.org> wrote:

> ​Here's an update to the previous patch; the problem seemed to be the use
> of eq for comparing strings rather than string=.​
>
> It seems to work now.
>

​Ping! Any comments before I install this patch?

-- 
https://rrt.sc3d.org <http://rrt.sc3d.org>

[-- Attachment #2: Type: text/html, Size: 951 bytes --]

bug#18133: Suppressing asynchronous command output

[Eli Zaretskii]

> From: Reuben Thomas <rrt@sc3d.org>
> Date: Mon, 7 Aug 2017 13:31:56 +0100
> Cc: Juri Linkov <juri@linkov.net>, martin rudalics <rudalics@gmx.at>, 18133@debbugs.gnu.org
> 
> On 28 June 2017 at 23:05, Reuben Thomas <rrt@sc3d.org> wrote:
> 
>  ​Here's an update to the previous patch; the problem seemed to be the use of eq for comparing strings
>  rather than string=.​
> 
>  It seems to work now.
> 
> ​Ping! Any comments before I install this patch?

Sorry for the delay.  Please go ahead, but I think this also needs
suitable changes for NEWS and the manual.

bug#18133: Suppressing asynchronous command output

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>>  ​Here's an update to the previous patch; the problem seemed to be
>> the use of eq for comparing strings
>>  rather than string=.​
>> 
>>  It seems to work now.
>> 
>> ​Ping! Any comments before I install this patch?
>
> Sorry for the delay.  Please go ahead, but I think this also needs
> suitable changes for NEWS and the manual.

This was a kinda long thread, and I've only loosely skimmed it.  But it
seems like the proposed patch was applied, and it seems to be
documented, so I'm guessing this bug report was just left open by
mistake?  So I'm closing it now.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#18133: Suppressing asynchronous command output

[Reuben Thomas via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

On Fri, 18 Sep 2020 at 14:40, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Eli Zaretskii <eliz@gnu.org> writes:
>
> >>  Here's an update to the previous patch; the problem seemed to be
> >> the use of eq for comparing strings
> >>  rather than string=.
> >>
> >>  It seems to work now.
> >>
> >> Ping! Any comments before I install this patch?
> >
> > Sorry for the delay.  Please go ahead, but I think this also needs
> > suitable changes for NEWS and the manual.
>
> This was a kinda long thread, and I've only loosely skimmed it.  But it
> seems like the proposed patch was applied, and it seems to be
> documented, so I'm guessing this bug report was just left open by
> mistake?  So I'm closing it now.
>

Thanks! Sorry, I guess I should've closed the bug when I installed the
patch (and I just confirmed that I did indeed install it).

-- 
https://rrt.sc3d.org

[-- Attachment #2: Type: text/html, Size: 1586 bytes --]

bug#18133: Suppressing asynchronous command output

[Lars Ingebrigtsen]

Reuben Thomas <rrt@sc3d.org> writes:

> Thanks! Sorry, I guess I should've closed the bug when I installed the
> patch (and I just confirmed that I did indeed install it).

No problem.  :-)  Thanks for checking.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no