bug#13594: 24.2.92; [PATCH] compilation-start doesn't consider nil OUTWIN

bug#13594: 24.2.92; [PATCH] compilation-start doesn't consider nil OUTWIN

[Leo Liu]

In building ggtags on top of compile.el, I have found one annoyance that
is I need to popup and immediately delete the compile window if there is
one or zero match, which is visually disturbing.

While finding my way to fix this annoyance, I discovered
compilation-start does not consider the case when OUTWIN is nil (which
is legitimate per the doc-string of `display-buffer').

        (setq outwin (display-buffer outbuf))

When OUTWIN is nil, all the subsequent window operations will be acting
on the wrong window.

(proposed patch attached)

Leo

diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index f383e02b..13ef425f 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -1616,7 +1616,7 @@ (defun compilation-start (command &optional mode name-function highlight-regexp)
       (set-buffer-modified-p nil))
     ;; Pop up the compilation buffer.
     ;; http://lists.gnu.org/archive/html/emacs-devel/2007-11/msg01638.html
-    (setq outwin (display-buffer outbuf))
+    (setq outwin (display-buffer outbuf)) ; Note: OUTWIN may be nil
     (with-current-buffer outbuf
       (let ((process-environment
 	     (append
@@ -1638,7 +1638,7 @@ (defun compilation-start (command &optional mode name-function highlight-regexp)
 	     (list command mode name-function highlight-regexp))
 	(set (make-local-variable 'revert-buffer-function)
 	     'compilation-revert-buffer)
-	(set-window-start outwin (point-min))
+	(and outwin (set-window-start outwin (point-min)))
 
 	;; Position point as the user will see it.
 	(let ((desired-visible-point
@@ -1647,15 +1647,16 @@ (defun compilation-start (command &optional mode name-function highlight-regexp)
 		   (point-max)
 		 ;; Normally put it at the top.
 		 (point-min))))
-	  (if (eq outwin (selected-window))
-	      (goto-char desired-visible-point)
-	    (set-window-point outwin desired-visible-point)))
+	  (when outwin
+	    (if (eq outwin (selected-window))
+		(goto-char desired-visible-point)
+	      (set-window-point outwin desired-visible-point))))
 
 	;; The setup function is called before compilation-set-window-height
 	;; so it can set the compilation-window-height buffer locally.
 	(if compilation-process-setup-function
 	    (funcall compilation-process-setup-function))
-	(compilation-set-window-height outwin)
+	(and outwin (compilation-set-window-height outwin))
 	;; Start the compilation.
 	(if (fboundp 'start-process)
 	    (let ((proc

bug#13594: 24.2.92; [PATCH] compilation-start doesn't consider nil OUTWIN

[Leo Liu]

On 2013-01-31 18:43 +0800, Leo Liu wrote:
> (proposed patch attached)

All three instances of display-buffer in compile fails to account for
nil value. The following patch fix them all.


 lisp/progmodes/compile.el | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

	Modified lisp/progmodes/compile.el
diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index f383e02b..09a2d9a2 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -1616,7 +1616,7 @@ (defun compilation-start (command &optional mode name-function highlight-regexp)
       (set-buffer-modified-p nil))
     ;; Pop up the compilation buffer.
     ;; http://lists.gnu.org/archive/html/emacs-devel/2007-11/msg01638.html
-    (setq outwin (display-buffer outbuf))
+    (setq outwin (display-buffer outbuf)) ; Note: OUTWIN may be nil
     (with-current-buffer outbuf
       (let ((process-environment
 	     (append
@@ -1638,7 +1638,7 @@ (defun compilation-start (command &optional mode name-function highlight-regexp)
 	     (list command mode name-function highlight-regexp))
 	(set (make-local-variable 'revert-buffer-function)
 	     'compilation-revert-buffer)
-	(set-window-start outwin (point-min))
+	(and outwin (set-window-start outwin (point-min)))
 
 	;; Position point as the user will see it.
 	(let ((desired-visible-point
@@ -1647,15 +1647,16 @@ (defun compilation-start (command &optional mode name-function highlight-regexp)
 		   (point-max)
 		 ;; Normally put it at the top.
 		 (point-min))))
-	  (if (eq outwin (selected-window))
-	      (goto-char desired-visible-point)
-	    (set-window-point outwin desired-visible-point)))
+	  (when outwin
+	    (if (eq outwin (selected-window))
+		(goto-char desired-visible-point)
+	      (set-window-point outwin desired-visible-point))))
 
 	;; The setup function is called before compilation-set-window-height
 	;; so it can set the compilation-window-height buffer locally.
 	(if compilation-process-setup-function
 	    (funcall compilation-process-setup-function))
-	(compilation-set-window-height outwin)
+	(and outwin (compilation-set-window-height outwin))
 	;; Start the compilation.
 	(if (fboundp 'start-process)
 	    (let ((proc
@@ -2490,11 +2491,12 @@ (defun compilation-goto-locus (msg mk end-mk)
 	      (display-buffer (marker-buffer msg))))
 	 (highlight-regexp (with-current-buffer (marker-buffer msg)
 			     ;; also do this while we change buffer
-			     (compilation-set-window w msg)
+			     (and w (compilation-set-window w msg))
 			     compilation-highlight-regexp)))
     ;; Ideally, the window-size should be passed to `display-buffer'
     ;; so it's only used when creating a new window.
-    (unless pre-existing (compilation-set-window-height w))
+    (when (and (not pre-existing) w)
+      (compilation-set-window-height w))
 
     (if from-compilation-buffer
         ;; If the compilation buffer window was selected,
@@ -2605,9 +2607,9 @@ (defun compilation-find-file (marker filename directory &rest formats)
     (while (null buffer)    ;Repeat until the user selects an existing file.
       ;; The file doesn't exist.  Ask the user where to find it.
       (save-excursion            ;This save-excursion is probably not right.
-        (let ((pop-up-windows t))
-          (compilation-set-window (display-buffer (marker-buffer marker))
-                                  marker)
+        (let* ((pop-up-windows t)
+	       (w (display-buffer (marker-buffer marker))))
+          (and w (compilation-set-window w marker))
           (let* ((name (read-file-name
                         (format "Find this %s in (default %s): "
                                 compilation-error filename)

bug#13594: 24.2.92; [PATCH] compilation-start doesn't consider nil OUTWIN

[Stefan Monnier]

> -    (setq outwin (display-buffer outbuf))
> +    (setq outwin (display-buffer outbuf)) ; Note: OUTWIN may be nil

`display-buffer' says:

   Return the window chosen for displaying BUFFER-OR-NAME,
   or nil if no such window is found.

which mostly means that it only returns nil if it couldn't find any
window to display the buffer, which is exceedingly rare.  Most/all
callers assume that display-buffer will display the buffer somewhere
(they sometimes disregard to return value, but when they don't they
almost systematically assume the return value is non-nil).

If you look further in the docstring you'll also see:

   Then it calls each function in the combined function list in turn,
   passing the buffer as the first argument and the combined alist as
   the second argument, until one of the functions returns non-nil.

Which again hints at the fact that nil is not treated as a valid return
value of display-buffer except when everything else failed.

IOW returning nil from display-buffer will inevitably bump into bugs.
Generally if you don't want to display a buffer, the best way to do that
is by not calling display-buffer.

So maybe the best approach is to extend compile.el such that it can be
told to do its job without displaying the compilation buffer.  If you do
that, try to take into account that this usage pattern could be useful
in other contexts than ggtags.el, for example users might prefer to not
see the compilation buffer and just be moved to the first error
automatically, and have C-x ` output the error message in the echo area
(maybe, accompanied by the number of errors left).


        Stefan

bug#13594: 24.2.92; [PATCH] compilation-start doesn't consider nil OUTWIN

[Leo Liu]

On 2013-01-31 23:14 +0800, Stefan Monnier wrote:
> So maybe the best approach is to extend compile.el such that it can be
> told to do its job without displaying the compilation buffer.  If you do
> that, try to take into account that this usage pattern could be useful
> in other contexts than ggtags.el, for example users might prefer to not
> see the compilation buffer and just be moved to the first error
> automatically, and have C-x ` output the error message in the echo area
> (maybe, accompanied by the number of errors left).

Thanks, Stefan, for the detailed info. I will provide such patch in the
weekend.

Leo

bug#13594: 24.2.92; [PATCH] compilation-start doesn't consider nil OUTWIN

[Leo Liu]

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

Hi Stefan,

On 2013-01-31 23:14 +0800, Stefan Monnier wrote:
> So maybe the best approach is to extend compile.el such that it can be
> told to do its job without displaying the compilation buffer.  If you do
> that, try to take into account that this usage pattern could be useful
> in other contexts than ggtags.el, for example users might prefer to not
> see the compilation buffer and just be moved to the first error
> automatically, and have C-x ` output the error message in the echo area
> (maybe, accompanied by the number of errors left).

The attached two patches (against emacs-24) implement:

1. A new variable compilation-dont-display-buffer to prevent calling
   display-buffer.

2. Display a message when calling compilation-next-error like this:
   Error: 2/623

Thanks for comments.

Leo


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-New-variable-compilation-dont-display-buffer.patch --]
[-- Type: text/x-patch, Size: 4665 bytes --]

From 021d45a602ef63bfd14a98a6d622cc107f75e2e7 Mon Sep 17 00:00:00 2001
From: Leo Liu <sdl.web@gmail.com>
Date: Tue, 5 Feb 2013 15:05:18 +0800
Subject: [PATCH 1/2] New variable compilation-dont-display-buffer

---
 lisp/progmodes/compile.el | 34 +++++++++++++++++++++++-----------
 1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index f383e02b..8b5e3a81 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -58,6 +58,12 @@ (defcustom compilation-start-hook nil
   :group 'compilation)
 
 ;;;###autoload
+(defcustom compilation-dont-display-buffer nil
+  "Non-nil to inhibit display the compilation buffer."
+  :type 'boolean
+  :group 'compilation)
+
+;;;###autoload
 (defcustom compilation-window-height nil
   "Number of lines in a compilation window.  If nil, use Emacs default."
   :type '(choice (const :tag "Default" nil)
@@ -1616,7 +1622,8 @@ (defun compilation-start (command &optional mode name-function highlight-regexp)
       (set-buffer-modified-p nil))
     ;; Pop up the compilation buffer.
     ;; http://lists.gnu.org/archive/html/emacs-devel/2007-11/msg01638.html
-    (setq outwin (display-buffer outbuf))
+    (unless (buffer-local-value 'compilation-dont-display-buffer outbuf)
+      (setq outwin (display-buffer outbuf)))
     (with-current-buffer outbuf
       (let ((process-environment
 	     (append
@@ -1638,7 +1645,7 @@ (defun compilation-start (command &optional mode name-function highlight-regexp)
 	     (list command mode name-function highlight-regexp))
 	(set (make-local-variable 'revert-buffer-function)
 	     'compilation-revert-buffer)
-	(set-window-start outwin (point-min))
+	(and outwin (set-window-start outwin (point-min)))
 
 	;; Position point as the user will see it.
 	(let ((desired-visible-point
@@ -1647,15 +1654,16 @@ (defun compilation-start (command &optional mode name-function highlight-regexp)
 		   (point-max)
 		 ;; Normally put it at the top.
 		 (point-min))))
-	  (if (eq outwin (selected-window))
-	      (goto-char desired-visible-point)
-	    (set-window-point outwin desired-visible-point)))
+	  (when outwin
+	    (if (eq outwin (selected-window))
+		(goto-char desired-visible-point)
+	      (set-window-point outwin desired-visible-point))))
 
 	;; The setup function is called before compilation-set-window-height
 	;; so it can set the compilation-window-height buffer locally.
 	(if compilation-process-setup-function
 	    (funcall compilation-process-setup-function))
-	(compilation-set-window-height outwin)
+	(and outwin (compilation-set-window-height outwin))
 	;; Start the compilation.
 	(if (fboundp 'start-process)
 	    (let ((proc
@@ -1977,6 +1985,7 @@ (defmacro define-compilation-mode (mode name doc &rest body)
 		   compilation-scroll-output
 		   compilation-search-path
 		   compilation-skip-threshold
+		   compilation-dont-display-buffer
 		   compilation-window-height))
        ,@body)))
 
@@ -2487,14 +2496,16 @@ (defun compilation-goto-locus (msg mk end-mk)
                 ;; the error location if the two buffers are in two
                 ;; different frames.  So don't do it if it's not necessary.
                 pre-existing
-	      (display-buffer (marker-buffer msg))))
+	      (unless compilation-dont-display-buffer
+		(display-buffer (marker-buffer msg)))))
 	 (highlight-regexp (with-current-buffer (marker-buffer msg)
 			     ;; also do this while we change buffer
-			     (compilation-set-window w msg)
+			     (and w (compilation-set-window w msg))
 			     compilation-highlight-regexp)))
     ;; Ideally, the window-size should be passed to `display-buffer'
     ;; so it's only used when creating a new window.
-    (unless pre-existing (compilation-set-window-height w))
+    (unless pre-existing
+      (and w (compilation-set-window-height w)))
 
     (if from-compilation-buffer
         ;; If the compilation buffer window was selected,
@@ -2606,8 +2617,9 @@ (defun compilation-find-file (marker filename directory &rest formats)
       ;; The file doesn't exist.  Ask the user where to find it.
       (save-excursion            ;This save-excursion is probably not right.
         (let ((pop-up-windows t))
-          (compilation-set-window (display-buffer (marker-buffer marker))
-                                  marker)
+          (unless compilation-dont-display-buffer
+	    (compilation-set-window (display-buffer (marker-buffer marker))
+				    marker))
           (let* ((name (read-file-name
                         (format "Find this %s in (default %s): "
                                 compilation-error filename)
-- 
1.8.1.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Display-current-error-rank-total.patch --]
[-- Type: text/x-patch, Size: 2641 bytes --]

From 014240765570257fd5c8499d78d1eae8149df756 Mon Sep 17 00:00:00 2001
From: Leo Liu <sdl.web@gmail.com>
Date: Tue, 5 Feb 2013 18:48:28 +0800
Subject: [PATCH 2/2] Display current error rank/total

---
 lisp/progmodes/compile.el | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index 8b5e3a81..0c3265ce 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -136,7 +136,7 @@ (defvar compilation-error "error"
 (defvar compilation-arguments nil
   "Arguments that were given to `compilation-start'.")
 
-(defvar compilation-num-errors-found)
+(defvar compilation-num-errors-found 0)
 
 ;; If you make any changes to `compilation-error-regexp-alist-alist',
 ;; be sure to run the ERT test in test/automated/compile-tests.el.
@@ -895,9 +895,9 @@ (cl-defstruct (compilation--message
             (:constructor nil)
             (:copier nil)
             ;; (:type list)                ;Old representation.
-            (:constructor compilation--make-message (loc type end-loc))
+            (:constructor compilation--make-message (loc type end-loc &optional rank))
             (:conc-name compilation--message->))
-  loc type end-loc)
+  loc type end-loc rank)
 
 (defvar compilation--previous-directory-cache nil
   "A pair (POS . RES) caching the result of previous directory search.
@@ -1181,8 +1181,11 @@ (defun compilation-internal-error-properties (file line end-line col end-col typ
                                               end-marker))))
 
     ;; Must start with face
+    (setq-local compilation-num-errors-found
+		(1+ compilation-num-errors-found))
     `(font-lock-face ,compilation-message-face
-      compilation-message ,(compilation--make-message loc type end-loc)
+      compilation-message ,(compilation--make-message
+			    loc type end-loc compilation-num-errors-found)
       help-echo ,(if col
                      "mouse-2: visit this file, line and column"
                    (if line
@@ -2281,8 +2284,12 @@ (defun compilation-next-error (n &optional different-file pt)
 	(compilation-loop < previous-single-property-change 1+
 			  "Moved back before first %s" (point-min))))
     (goto-char pt)
-    (or msg
-	(error "No %s here" compilation-error))))
+    (when msg
+      (message "%s: %s of %s"
+	       (capitalize compilation-error)
+	       (compilation--message->rank msg)
+	       compilation-num-errors-found))
+    (or msg (error "No %s here" compilation-error))))
 
 (defun compilation-previous-error (n)
   "Move point to the previous error in the compilation buffer.
-- 
1.8.1.1

bug#13594: 24.2.92; [PATCH] compilation-start doesn't consider nil OUTWIN

[Leo Liu]

On 2013-02-05 18:58 +0800, Leo Liu wrote:
> 2. Display a message when calling compilation-next-error like this:
>    Error: 2/623

Sadly this second patch doesn't account for the fact that some
compilation messages are removed later on by font-lock.

Leo

bug#13594: 24.2.92; [PATCH] compilation-start doesn't consider nil OUTWIN

[Juri Linkov]

> 1. A new variable compilation-dont-display-buffer to prevent calling
>    display-buffer.

Some users of `async-shell-command' want the same feature of
not displaying the output buffer.  From the old discussion
I got the impression that such customization should be possible
by customizing `display-buffer-alist' and associating
the buffer name (such as *compilation* or *Async Shell Command*)
with an inaction to skip window display.

(But it still need to check `outwin' for null values in your patch
for the case when `display-buffer' will return nil.)

> 2. Display a message when calling compilation-next-error like this:
>    Error: 2/623

I think it would be nice to display the same in the mode line.

> Sadly this second patch doesn't account for the fact that some
> compilation messages are removed later on by font-lock.

`grep' used to remove parts of grep/compilation messages,
but now there shouldn't be such a problem in grep's font-lock.
Do you have a test case that would demonstrate this problem
in compilation's font-lock?

bug#13594: 24.2.92; [PATCH] compilation-start doesn't consider nil OUTWIN

[Leo Liu]

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

On 2013-02-06 07:25 +0800, Juri Linkov wrote:
> Some users of `async-shell-command' want the same feature of
> not displaying the output buffer.  From the old discussion
> I got the impression that such customization should be possible
> by customizing `display-buffer-alist' and associating
> the buffer name (such as *compilation* or *Async Shell Command*)
> with an inaction to skip window display.

Would be nice to see how to do it using display-buffer-alist. From my
understanding and as Stefan suggested, the best thing to do is not to
call display buffer if the intention is not to display it.

>> 2. Display a message when calling compilation-next-error like this:
>>    Error: 2/623
>
> I think it would be nice to display the same in the mode line.

Yes I agree but let's get the count of errors right first.

>> Sadly this second patch doesn't account for the fact that some
>> compilation messages are removed later on by font-lock.
>
> `grep' used to remove parts of grep/compilation messages,
> but now there shouldn't be such a problem in grep's font-lock.
> Do you have a test case that would demonstrate this problem
> in compilation's font-lock?

I mean the 'compilation-message' property. In my second patch, the count
is based on each compilation-message properties inserted but it is
removed in, for example, these lines:

        Grep started at Wed Feb  6 09:11:06
        Grep finished (matches found) at Wed Feb  6 09:11:06

Here is the latest patches in one diff:
http://bpaste.net/show/npDaKs5Wvuk2AxSWgOiV

The total error count is still incorrect because (compilation-next-error
-1 nil (point-max)) doesn't find me the last real error but:

        Grep finished (matches found) at Wed Feb  6 09:11:06


[-- Attachment #2: bug13594.png --]
[-- Type: image/png, Size: 52384 bytes --]

[-- Attachment #3: Type: text/plain, Size: 5 bytes --]


Leo

bug#13594: 24.2.92; [PATCH] compilation-start doesn't consider nil OUTWIN

[Juri Linkov]

> Would be nice to see how to do it using display-buffer-alist.

(add-to-list 'display-buffer-alist '("\\*compilation\\*" ignore-t (nil)))

where `ignore-t' is like existing `ignore' but returns t instead of nil:

(defun ignore-t (&rest _ignore)
  (interactive)
  t)

Maybe such trivial function already exists but with another name?

bug#13594: 24.2.92; [PATCH] compilation-start doesn't consider nil OUTWIN

[Stefan Monnier]

> (add-to-list 'display-buffer-alist '("\\*compilation\\*" ignore-t (nil)))
> where `ignore-t' is like existing `ignore' but returns t instead of nil:
> (defun ignore-t (&rest _) t)

Problem is that the functions in display-buffer-alist are supposed to
return either the window they used or nil (to mean that display-buffer
should try the next candidate function).

So returning t is incorrect and can/will lead to bugs further down where
the caller does not expect a t value (most callers of display-buffer
don't expect a nil return value either).


        Stefan

bug#13594: 24.2.92; [PATCH] compilation-start doesn't consider nil OUTWIN

[Juri Linkov]

>> (add-to-list 'display-buffer-alist '("\\*compilation\\*" ignore-t (nil)))
>> where `ignore-t' is like existing `ignore' but returns t instead of nil:
>> (defun ignore-t (&rest _) t)
>
> Problem is that the functions in display-buffer-alist are supposed to
> return either the window they used or nil (to mean that display-buffer
> should try the next candidate function).
>
> So returning t is incorrect and can/will lead to bugs further down where
> the caller does not expect a t value (most callers of display-buffer
> don't expect a nil return value either).

Since there are more buffer names that users might want to not display
(e.g. "*Async Shell Command*") it makes sense to improve the
buffer-displaying framework with a new feature that would allow the user
to associate a buffer name with an inaction for which `display-buffer'
will return nil (since nil is a valid return value it's a bug when callers
of `display-buffer' don't check for nil, these callers should be fixed).

bug#13594: 24.2.92; [PATCH] compilation-start doesn't consider nil OUTWIN

[Stefan Monnier]

> will return nil (since nil is a valid return value it's a bug when callers
> of `display-buffer' don't check for nil, these callers should be fixed).

It's a valid return value, but only when display-buffer is *unable to
display the buffer*.  It is *very* rare to be unable to do so.  E.g. it
will never happen unless at least one of pop-up-frame-function or
display-buffer-fallback-action is changed.

So it's no wonder that most callers don't bother checking for a nil
return value.

Maybe a way around that is to use a special window that's never
displayed.  But that might introduce more trouble than it's trying
to solve.


        Stefan

bug#13594: 24.2.92; [PATCH] compilation-start doesn't consider nil OUTWIN

[Juri Linkov]

> It's a valid return value, but only when display-buffer is *unable to
> display the buffer*.  It is *very* rare to be unable to do so.  E.g. it
> will never happen unless at least one of pop-up-frame-function or
> display-buffer-fallback-action is changed.

Trying to do (setq display-buffer-fallback-action nil)
and `M-x compile RET' goes to re-arrange the wrong window
(that displays "*scratch*" in `emacs -Q') because the nil WINDOW arg
of `set-window-start' defaults to the selected window,  so yes,
a nil value is not a good thing to return from display-buffer.

> Maybe a way around that is to use a special window that's never
> displayed.  But that might introduce more trouble than it's trying
> to solve.

Trying to use an internal window:

(add-to-list 'display-buffer-alist '("\\*compilation\\*" display-buffer-ignore (nil)))
(defun display-buffer-ignore (&rest _ignore) (frame-root-window))

fails with `(wrong-type-argument window-live-p #<window 0x286f258>)',
so this is not possible to do without changes to the window framework
to add a new window type for live hidden windows.

bug#13594: 24.2.92; [PATCH] compilation-start doesn't consider nil OUTWIN

[martin rudalics]

 > It's a valid return value, but only when display-buffer is *unable to
 > display the buffer*.  It is *very* rare to be unable to do so.  E.g. it
 > will never happen unless at least one of pop-up-frame-function or
 > display-buffer-fallback-action is changed.

To make it happen you have to make all existing windows dedicated and
display some other buffer, inhibit popping up new windows, and inhibit
popping up new frames.

martin

bug#13594: 24.2.92; [PATCH] compilation-start doesn't consider nil OUTWIN

[Stefan Monnier]

>> Maybe a way around that is to use a special window that's never
>> displayed.  But that might introduce more trouble than it's trying
>> to solve.
> Trying to use an internal window:

No, I really meant a window that's not displayed, not the root-window.
E.g. a window on a separate frame with the frame marked invisible.


        Stefan

bug#13594: 24.2.92; [PATCH] compilation-start doesn't consider nil OUTWIN

[martin rudalics]

 > No, I really meant a window that's not displayed, not the root-window.
 > E.g. a window on a separate frame with the frame marked invisible.

I'm currently investigating this idea when fitting a frame to a buffer
before the frame is displayed at all.  But I wouldn't recommend it in
the context of the present thread - any calls to `display-buffer' should
provide some visual feedback.

In principle, `display-buffer' could always return a window, reusing a
dedicated one or making a new frame, if necessary.  OTOH, the while loop
in `display-buffer' could easily give up when one of the functions it
calls returns a special value, say 'skip, and return nil in that case.

The problem is how to make callers of `display-buffer' (and more
seriously `pop-to-buffer' and `switch-to-buffer') aware of the fact that
no window was found, maybe deliberately.

I think the only correct solution is to have the caller add an alist
entry (or an extra argument to `display-buffer') say 'may-fail, whose
semantics are:

- If 'may-fail is non-nil, allow returning a nil value - I, the caller,
   already know how to handle that.

- If 'may-fail is nil or doesn't exist, return some window at any cost.

martin

bug#13594: 24.2.92; [PATCH] compilation-start doesn't consider nil OUTWIN

[Juri Linkov]

> I think the only correct solution is to have the caller add an alist
> entry (or an extra argument to `display-buffer') say 'may-fail, whose
> semantics are:
>
> - If 'may-fail is non-nil, allow returning a nil value - I, the caller,
>   already know how to handle that.
>
> - If 'may-fail is nil or doesn't exist, return some window at any cost.

But such some window at any cost should be harmless when the caller goes
to use the returned window e.g. to get its buffer with `window-buffer', etc.
This is why using a hidden window is better, despite its invisibility
the window will be still correct in all its properties.

bug#13594: 24.2.92; [PATCH] compilation-start doesn't consider nil OUTWIN

[martin rudalics]

 > But such some window at any cost should be harmless when the caller goes
 > to use the returned window e.g. to get its buffer with `window-buffer', etc.

Returning the selected window is harmless.

 > This is why using a hidden window is better, despite its invisibility
 > the window will be still correct in all its properties.

Deleting the last visible Emacs frame and subsequently switching off
your machine is not entirely harmless.

martin

bug#13594: 24.2.92; [PATCH] compilation-start doesn't consider nil OUTWIN

[Juri Linkov]

>> But such some window at any cost should be harmless when the caller goes
>> to use the returned window e.g. to get its buffer with `window-buffer', etc.
>
> Returning the selected window is harmless.

Only if it displays the requested buffer.

Do you think it is possible to implement a special "hidden" window type
so callers could operate on it invisibly to the user?

bug#13594: 24.2.92; [PATCH] compilation-start doesn't consider nil OUTWIN

[martin rudalics]

 >> Returning the selected window is harmless.
 >
 > Only if it displays the requested buffer.

We could make it do that.  That is, even for (inhibit-same-window . t)
and the case where the selected window is dedicated, we could display it
there if we don't have another choice.

 > Do you think it is possible to implement a special "hidden" window type
 > so callers could operate on it invisibly to the user?

Not really.  If you call `with-selected-window' on such a window and get
stuck in it, the display routines would probably have to find their way
out of such a situation.

We could, as Stefan proposed, use the root window of an invisible frame
for this.  But as I mentioned earlier, this has the disadvantage that
you have to somehow clean up that frame when you delete the last visible
frame.  Officially, `delete-frame' should handle this case but at least
here on Windows XP it certainly doesn't.  And I'm quite sure that we can
find at least one window manager where sampling the visibility of frames
is not 100% reliable.

martin

bug#13594: 24.2.92; [PATCH] compilation-start doesn't consider nil OUTWIN

[Leo Liu]

On 2013-02-12 01:31 +0800, martin rudalics wrote:
>> Do you think it is possible to implement a special "hidden" window type
>> so callers could operate on it invisibly to the user?
>
> Not really.  If you call `with-selected-window' on such a window and get
> stuck in it, the display routines would probably have to find their way
> out of such a situation.
>
> We could, as Stefan proposed, use the root window of an invisible frame
> for this.  But as I mentioned earlier, this has the disadvantage that
> you have to somehow clean up that frame when you delete the last visible
> frame.  Officially, `delete-frame' should handle this case but at least
> here on Windows XP it certainly doesn't.  And I'm quite sure that we can
> find at least one window manager where sampling the visibility of frames
> is not 100% reliable.
>
> martin

What happens if we just return a minibuffer window to display-buffer?
(still ugly as hell)

Leo

bug#13594: 24.2.92; [PATCH] compilation-start doesn't consider nil OUTWIN

[Leo Liu]

On 2013-02-12 01:55 +0800, Leo Liu wrote:
> What happens if we just return a minibuffer window to display-buffer?
> (still ugly as hell)

Have we reached an impasse? Is it advisable to go back to what Stefan
originally suggested?

Cheers,
Leo

bug#13594: 24.2.92; [PATCH] compilation-start doesn't consider nil OUTWIN

[Stefan Monnier]

>> What happens if we just return a minibuffer window to display-buffer?
>> (still ugly as hell)
> Have we reached an impasse? Is it advisable to go back to what Stefan
> originally suggested?

Basically, as it currently stands "don't display at all" can be done in
2 ways:
- make it work only for those callers that are prepared for it: for this
  case, we could simply define a t return value (returned from an
  ACTION) to mean "not displayed".  Ideally, display-buffer would return
  nil in this case, but returning t is OK as well.
- make it work everywhere: then we need display-buffer to return
  a "dummy" window (i.e. an object that behaves like a window object
  but that is not displayed).

I'm beginning to think the best choice is the first option: it requires
no changes right away, and we can progressively change callers so that
they can handle the "not displayed" case.


        Stefan

bug#13594: 24.2.92; [PATCH] compilation-start doesn't consider nil OUTWIN

[Leo Liu]

On 2013-02-14 22:15 +0800, Stefan Monnier wrote:
> Basically, as it currently stands "don't display at all" can be done in
> 2 ways:
> - make it work only for those callers that are prepared for it: for this
>   case, we could simply define a t return value (returned from an
>   ACTION) to mean "not displayed".  Ideally, display-buffer would return
>   nil in this case, but returning t is OK as well.
> - make it work everywhere: then we need display-buffer to return
>   a "dummy" window (i.e. an object that behaves like a window object
>   but that is not displayed).
>
> I'm beginning to think the best choice is the first option: it requires
> no changes right away, and we can progressively change callers so that
> they can handle the "not displayed" case.

Does this roughly follow your idea? i.e. document display-buffer to
allow non-window return value (I also get rid of mentioning nil). In
compile.el, handle non-window return value. The attached patch does
these two. (I'll prepare a thorough patch tomorrow.).

diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index b82afc67..8edfe487 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -1638,7 +1638,8 @@ (defun compilation-start (command &optional mode name-function highlight-regexp)
 	     (list command mode name-function highlight-regexp))
 	(set (make-local-variable 'revert-buffer-function)
 	     'compilation-revert-buffer)
-	(set-window-start outwin (point-min))
+	(when (windowp outwin)
+	  (set-window-start outwin (point-min)))
 
 	;; Position point as the user will see it.
 	(let ((desired-visible-point
@@ -1647,15 +1648,18 @@ (defun compilation-start (command &optional mode name-function highlight-regexp)
 		   (point-max)
 		 ;; Normally put it at the top.
 		 (point-min))))
-	  (if (eq outwin (selected-window))
-	      (goto-char desired-visible-point)
-	    (set-window-point outwin desired-visible-point)))
+	  (cond
+	   ((windowp outwin)
+	    (set-window-point outwin desired-visible-point))
+	   ((eq outwin (selected-window))
+	    (goto-char desired-visible-point))))
 
 	;; The setup function is called before compilation-set-window-height
 	;; so it can set the compilation-window-height buffer locally.
 	(if compilation-process-setup-function
 	    (funcall compilation-process-setup-function))
-	(compilation-set-window-height outwin)
+	(and (windowp outwin)
+	     (compilation-set-window-height outwin))
 	;; Start the compilation.
 	(if (fboundp 'start-process)
 	    (let ((proc
diff --git a/lisp/window.el b/lisp/window.el
index 63d75f60..e96c8c60 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -5368,7 +5368,8 @@ (defun display-buffer (buffer-or-name &optional action frame)
   "Display BUFFER-OR-NAME in some window, without selecting it.
 BUFFER-OR-NAME must be a buffer or the name of an existing
 buffer.  Return the window chosen for displaying BUFFER-OR-NAME,
-or nil if no such window is found.
+or a non-nil value to mean one of the actions forbids displaying
+the buffer.
 
 Optional argument ACTION, if non-nil, should specify a display
 action.  Its form is described below.
@@ -5394,7 +5395,9 @@ (defun display-buffer (buffer-or-name &optional action frame)
 `display-buffer-fallback-action' (in order).  Then it calls each
 function in the combined function list in turn, passing the
 buffer as the first argument and the combined alist as the second
-argument, until one of the functions returns non-nil.
+argument, until one of the functions returns non-nil.  If the
+value is not a window object, the search stops and no buffer is
+displayed.
 
 If ACTION is nil, the function list and the alist are built using
 only the other variables mentioned above.

bug#13594: 24.2.92; [PATCH] compilation-start doesn't consider nil OUTWIN

[Stefan Monnier]

>> Basically, as it currently stands "don't display at all" can be done in
>> 2 ways:
>> - make it work only for those callers that are prepared for it: for this
>> case, we could simply define a t return value (returned from an
>> ACTION) to mean "not displayed".  Ideally, display-buffer would return
>> nil in this case, but returning t is OK as well.
>> - make it work everywhere: then we need display-buffer to return
>> a "dummy" window (i.e. an object that behaves like a window object
>> but that is not displayed).
>> 
>> I'm beginning to think the best choice is the first option: it requires
>> no changes right away, and we can progressively change callers so that
>> they can handle the "not displayed" case.

> Does this roughly follow your idea? i.e. document display-buffer to
> allow non-window return value (I also get rid of mentioning nil). In
> compile.el, handle non-window return value. The attached patch does
> these two. (I'll prepare a thorough patch tomorrow.).

It only does one half: change a few callers to handle a non-window
return value.  But the other half is important: the non-window return
value should only be possible if the caller announces that it's ready to
handle it.


        Stefan


> diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
> index b82afc67..8edfe487 100644
> --- a/lisp/progmodes/compile.el
> +++ b/lisp/progmodes/compile.el
> @@ -1638,7 +1638,8 @@ (defun compilation-start (command &optional mode name-function highlight-regexp)
>  	     (list command mode name-function highlight-regexp))
>  	(set (make-local-variable 'revert-buffer-function)
>  	     'compilation-revert-buffer)
> -	(set-window-start outwin (point-min))
> +	(when (windowp outwin)
> +	  (set-window-start outwin (point-min)))
 
>  	;; Position point as the user will see it.
>  	(let ((desired-visible-point
> @@ -1647,15 +1648,18 @@ (defun compilation-start (command &optional mode name-function highlight-regexp)
>  		   (point-max)
>  		 ;; Normally put it at the top.
>  		 (point-min))))
> -	  (if (eq outwin (selected-window))
> -	      (goto-char desired-visible-point)
> -	    (set-window-point outwin desired-visible-point)))
> +	  (cond
> +	   ((windowp outwin)
> +	    (set-window-point outwin desired-visible-point))
> +	   ((eq outwin (selected-window))
> +	    (goto-char desired-visible-point))))
 
>  	;; The setup function is called before compilation-set-window-height
>  	;; so it can set the compilation-window-height buffer locally.
>  	(if compilation-process-setup-function
>  	    (funcall compilation-process-setup-function))
> -	(compilation-set-window-height outwin)
> +	(and (windowp outwin)
> +	     (compilation-set-window-height outwin))
>  	;; Start the compilation.
>  	(if (fboundp 'start-process)
>  	    (let ((proc
> diff --git a/lisp/window.el b/lisp/window.el
> index 63d75f60..e96c8c60 100644
> --- a/lisp/window.el
> +++ b/lisp/window.el
> @@ -5368,7 +5368,8 @@ (defun display-buffer (buffer-or-name &optional action frame)
>    "Display BUFFER-OR-NAME in some window, without selecting it.
>  BUFFER-OR-NAME must be a buffer or the name of an existing
>  buffer.  Return the window chosen for displaying BUFFER-OR-NAME,
> -or nil if no such window is found.
> +or a non-nil value to mean one of the actions forbids displaying
> +the buffer.
 
>  Optional argument ACTION, if non-nil, should specify a display
>  action.  Its form is described below.
> @@ -5394,7 +5395,9 @@ (defun display-buffer (buffer-or-name &optional action frame)
>  `display-buffer-fallback-action' (in order).  Then it calls each
>  function in the combined function list in turn, passing the
>  buffer as the first argument and the combined alist as the second
> -argument, until one of the functions returns non-nil.
> +argument, until one of the functions returns non-nil.  If the
> +value is not a window object, the search stops and no buffer is
> +displayed.
 
>  If ACTION is nil, the function list and the alist are built using
>  only the other variables mentioned above.

bug#13594: 24.2.92; [PATCH] compilation-start doesn't consider nil OUTWIN

[Leo Liu]

On 2013-03-20 11:12 +0800, Stefan Monnier wrote:
> It only does one half: change a few callers to handle a non-window
> return value.  But the other half is important: the non-window return
> value should only be possible if the caller announces that it's ready to
> handle it.

So add to display-buffer an optional arg like the following?

(display-buffer BUFFER-OR-NAME &optional ACTION FRAME HANDLE-NONWINDOW)

display-buffer currently does not checking; it just passes back the
non-nil value to the caller. WDYT?

Leo

bug#13594: 24.2.92; [PATCH] compilation-start doesn't consider nil OUTWIN

[Stefan Monnier]

>> It only does one half: change a few callers to handle a non-window
>> return value.  But the other half is important: the non-window return
>> value should only be possible if the caller announces that it's ready to
>> handle it.
> So add to display-buffer an optional arg like the following?
> (display-buffer BUFFER-OR-NAME &optional ACTION FRAME HANDLE-NONWINDOW)

That's one possibility, yes.  Tho adding an argument doesn't sound
much fun.  So I'd prefer if it could be passed via ACTION.

> display-buffer currently does not checking; it just passes back the
> non-nil value to the caller. WDYT?

That's OK.  The whole point is that the code (e.g. in
display-buffer-alist) that returns the non-window value needs to be able
to decide whether it's safe to do so.


        Stefan

bug#13594: 24.2.92; [PATCH] compilation-start doesn't consider nil OUTWIN

[Leo Liu]

On 2013-03-20 20:51 +0800, Stefan Monnier wrote:
> That's one possibility, yes.  Tho adding an argument doesn't sound
> much fun.  So I'd prefer if it could be passed via ACTION.

I have read this bug thread again. I am confused by this decision. Why
do we want to tell display-buffer to do nothing (via ACTION or extra
arg) when we can choose not to call it? Isn't not calling it better?

Leo

bug#13594: 24.2.92; [PATCH] compilation-start doesn't consider nil OUTWIN

[martin rudalics]

 >> That's one possibility, yes.  Tho adding an argument doesn't sound
 >> much fun.  So I'd prefer if it could be passed via ACTION.
 >
 > I have read this bug thread again. I am confused by this decision. Why
 > do we want to tell display-buffer to do nothing (via ACTION or extra
 > arg) when we can choose not to call it? Isn't not calling it better?

Stefan proposes to handle the case where the application wants to
display the buffer but `display-buffer' is not able to do so.  Ever
since, all callers of `display-buffer' I know of silently assumed that
the latter would display the buffer and return its window despite the
fact that it was possible that the buffer would not get displayed.

Stefan's proposal would correct that misunderstanding although now these
might result from the fact that if no argument is "passed via ACTION",
`display-buffer' could use the selected window, harming an application's
assumptions provided via setting inhibit-same-window to non-nil.  We at
least would have to document such fact but it's an "incompatible change"
nevertheless.  I think this is a minor evil and should be fairly rare as
well.

Now if an application and/or user do not want to display the buffer in
the first case, `display-buffer' should not be called.  If, however, the
call cannot be avoided, we should add a second (user) action which
avoids entering the loop in `display-buffer' and returns nil immediately
provided the first action was set (that is, the calling application at
least pretends to know how to handle a nil return value).

martin

bug#13594: Planning Emacs-24.4

[Stefan Monnier]

>> So, it's getting time for Emacs-24.4.
>> I'd like to freeze the code before the end of this year (I'm thinking of
>> mid-december).
>> If you have some code you'd like to see included in Emacs-24.4, then
>> please install it "now", and if it's not ready yet, then tell me, so we
>> can see how to accommodate it.
> I hope http://debbugs.gnu.org/13594 get fixed this time.

IIRC, the way to fix this is:
- document a parameter that can be passed via ACTION which basically
  declares "the caller is prepared to handle a non-window return value
  if display-buffer decides that the buffer should not be displayed".
- document that if this new parameter is set, the display-buffer-alist
  rules can simply return t if they want the buffer to not be displayed.
- change compile.el to pass this new parameter and then to handle
  a t return value of display-buffer.

Can you take care of that?  The first two are just documentation
changes, and the third is basically the patch you originally submitted
in that bug-report, except that outwin will be t rather than nil (tho
I recommend to just test whether it's `windowp').


        Stefan

bug#13594: Planning Emacs-24.4

[Leo Liu]

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

Implemented as requested. I have also briefly tested with ggtags and it
worked. Comments welcome. Thanks.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 13594.diff --]
[-- Type: text/x-patch, Size: 4924 bytes --]

=== modified file 'lisp/progmodes/compile.el'
--- lisp/progmodes/compile.el	2013-10-23 16:25:56 +0000
+++ lisp/progmodes/compile.el	2013-11-18 08:37:19 +0000
@@ -1632,7 +1632,7 @@
       (set-buffer-modified-p nil))
     ;; Pop up the compilation buffer.
     ;; http://lists.gnu.org/archive/html/emacs-devel/2007-11/msg01638.html
-    (setq outwin (display-buffer outbuf))
+    (setq outwin (display-buffer outbuf '(ignore (may-fail . t))))
     (with-current-buffer outbuf
       (let ((process-environment
 	     (append
@@ -1654,7 +1654,8 @@
 	     (list command mode name-function highlight-regexp))
 	(set (make-local-variable 'revert-buffer-function)
 	     'compilation-revert-buffer)
-	(set-window-start outwin (point-min))
+	(when (windowp outwin)
+	  (set-window-start outwin (point-min)))
 
 	;; Position point as the user will see it.
 	(let ((desired-visible-point
@@ -1663,15 +1664,16 @@
 		   (point-max)
 		 ;; Normally put it at the top.
 		 (point-min))))
-	  (if (eq outwin (selected-window))
-	      (goto-char desired-visible-point)
+	  (goto-char desired-visible-point)
+	  (when (eq outwin (selected-window))
 	    (set-window-point outwin desired-visible-point)))
 
 	;; The setup function is called before compilation-set-window-height
 	;; so it can set the compilation-window-height buffer locally.
 	(if compilation-process-setup-function
 	    (funcall compilation-process-setup-function))
-	(compilation-set-window-height outwin)
+	(when (windowp outwin)
+	  (compilation-set-window-height outwin))
 	;; Start the compilation.
 	(if (fboundp 'start-process)
 	    (let ((proc
@@ -2513,14 +2515,17 @@
                 ;; the error location if the two buffers are in two
                 ;; different frames.  So don't do it if it's not necessary.
                 pre-existing
-	      (display-buffer (marker-buffer msg))))
+	      (display-buffer (marker-buffer msg) '(ignore (may-fail . t)))))
 	 (highlight-regexp (with-current-buffer (marker-buffer msg)
 			     ;; also do this while we change buffer
-			     (compilation-set-window w msg)
+			     (goto-char (marker-position msg))
+			     (when (windowp w)
+			       (compilation-set-window w msg))
 			     compilation-highlight-regexp)))
     ;; Ideally, the window-size should be passed to `display-buffer'
     ;; so it's only used when creating a new window.
-    (unless pre-existing (compilation-set-window-height w))
+    (when (and (not pre-existing) (windowp w))
+      (compilation-set-window-height w))
 
     (if from-compilation-buffer
         ;; If the compilation buffer window was selected,
@@ -2631,9 +2636,12 @@
     (while (null buffer)    ;Repeat until the user selects an existing file.
       ;; The file doesn't exist.  Ask the user where to find it.
       (save-excursion            ;This save-excursion is probably not right.
-        (let ((pop-up-windows t))
-          (compilation-set-window (display-buffer (marker-buffer marker))
-                                  marker)
+        (let ((w (let ((pop-up-windows t))
+		   (display-buffer (marker-buffer marker)
+				   '(ignore (may-fail . t))))))
+          (with-current-buffer (marker-buffer marker)
+	    (goto-char marker)
+	    (and (windowp w) (compilation-set-window w marker)))
           (let* ((name (read-file-name
                         (format "Find this %s in (default %s): "
                                 compilation-error filename)

=== modified file 'lisp/window.el'
--- lisp/window.el	2013-11-12 07:25:14 +0000
+++ lisp/window.el	2013-11-18 08:37:19 +0000
@@ -5355,7 +5355,10 @@
  ACTION is a cons cell (FUNCTION . ALIST), where FUNCTION is a
   function or a list of functions.  Each such function should
   accept two arguments: a buffer to display and an alist of the
-  same form as ALIST.  See `display-buffer' for details.
+  same form as ALIST.  If (may-fail . t) is in ALIST, the caller
+  is prepared to handle non-window return value and FUNCTION can
+  safely return t to suppress displaying the window.  See
+  `display-buffer' for details.
 
 `display-buffer' scans this alist until it either finds a
 matching regular expression or the function specified by a
@@ -5439,9 +5442,12 @@
 ALIST is an arbitrary association list (alist).
 
 Each such FUNCTION should accept two arguments: the buffer to
-display and an alist.  Based on those arguments, it should either
-display the buffer and return the window, or return nil if unable
-to display the buffer.
+display and an alist.  Based on those arguments, it should
+display the buffer and return the window.  It may return nil if
+unable to display the buffer (which is rare and most callers
+don't check this value).  If the caller is prepared to handle
+non-window return value it should pass (may-fail . t) as an
+element of the ALIST.
 
 The `display-buffer' function builds a function list and an alist
 by combining the functions and alists specified in

bug#13594: Planning Emacs-24.4

[Leo Liu]

On 2013-11-18 16:41 +0800, Leo Liu wrote:
> +	  (when (eq outwin (selected-window))

Fixed locally as:

(when (and (windowp outwin)
           (not (eq outwin (selected-window))))
  ...)

bug#13594: Planning Emacs-24.4

[Andreas Schwab]

Leo Liu <sdl.web@gmail.com> writes:

> (when (and (windowp outwin)
>            (not (eq outwin (selected-window))))

No need to test for windowp.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

bug#13594: Planning Emacs-24.4

[Leo Liu]

On 2013-11-18 18:00 +0800, Andreas Schwab wrote:
> No need to test for windowp.

We permit return value t so must test windowp.

Leo

bug#13594: Planning Emacs-24.4

[Andreas Schwab]

Leo Liu <sdl.web@gmail.com> writes:

> On 2013-11-18 18:00 +0800, Andreas Schwab wrote:
>> No need to test for windowp.
>
> We permit return value t so must test windowp.

Which return value?

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

bug#13594: Planning Emacs-24.4

[Leo Liu]

On 2013-11-18 18:26 +0800, Andreas Schwab wrote:
> Which return value?
>
> Andreas.

display-buffer can return a non-nil non-window value (typically t).

See the documentation change to window.el in the patch.

Leo

bug#13594: Planning Emacs-24.4

[Andreas Schwab]

Leo Liu <sdl.web@gmail.com> writes:

> display-buffer can return a non-nil non-window value (typically t).

How is that relevant? t can never be eq selected-window.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

bug#13594: Planning Emacs-24.4

[martin rudalics]

 > Implemented as requested. I have also briefly tested with ggtags and it
 > worked. Comments welcome. Thanks.

Why didn't you implement the may-fail part?  IIUC `display-buffer'
should *not* return nil unless may-fail has been set.  So if may-fail is
not set, `display-buffer' should either create a new window, a new
frame, or reuse some window at any cost.

And there's no use for `display-buffer' returning t.

martin

bug#13594: Planning Emacs-24.4

[Leo Liu]

On 2013-11-18 18:38 +0800, Andreas Schwab wrote:
> How is that relevant? t can never be eq selected-window.
>
> Andreas.

Exactly. We want to call set-window-point when outwin is a window but
not the selected one.

Leo

bug#13594: Planning Emacs-24.4

[Leo Liu]

On 2013-11-18 18:46 +0800, martin rudalics wrote:
> Why didn't you implement the may-fail part?  IIUC `display-buffer'
> should *not* return nil unless may-fail has been set.  So if may-fail is
> not set, `display-buffer' should either create a new window, a new
> frame, or reuse some window at any cost.
>
> And there's no use for `display-buffer' returning t.
>
> martin

t was just a value to stop display-buffer searching down the action
list. this seems enough to manage display-buffer via
display-buffer-alist or the like.

But I'll wait for Stefan to explain what his intention is.

Leo

bug#13594: Planning Emacs-24.4

[Andreas Schwab]

Leo Liu <sdl.web@gmail.com> writes:

> Exactly. We want to call set-window-point when outwin is a window but
> not the selected one.

Thanks, I now see my error.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

bug#13594: Planning Emacs-24.4

[Leo Liu]

On 2013-11-18 19:25 +0800, Andreas Schwab wrote:
> Thanks, I now see my error.
>
> Andreas.

Sorry, I probably shouldn't omit set-window-point.

Leo

bug#13594: Planning Emacs-24.4

[martin rudalics]

 > t was just a value to stop display-buffer searching down the action
 > list. this seems enough to manage display-buffer via
 > display-buffer-alist or the like.

It is not.  Returning t when calling `display-buffer' without may-fail
set doesn't improve anything.  And if my-fail is set, we can return nil
because the caller is prepared for it.  And you still don't handle the
case where a user doesn't want to see the buffer in the first place.

So please

(1) Provide two actions designators may-fail and do-fail, say.

(2) When may-fail is set and no window is found, return nil.  When
     may-fail is not set, return the most suitable window.

(3) When may-fail and do-fail are both set, break the

	(while (and functions (not window))
	  (setq window (funcall (car functions) buffer alist)
	  	functions (cdr functions)))

     loop in `display-buffer' (for example, by having the function called
     return 'fail) and return nil.

martin

bug#13594: Planning Emacs-24.4

[Stefan Monnier]

>> Implemented as requested. I have also briefly tested with ggtags and it
>> worked.  Comments welcome.  Thanks.

Thank you.  Two comments:
1- You can/should use nil instead of `ignore'.
2- I'm not sure `may-fail' is the right name.  After all, it's not
   really a failure, but rather a conscious (and successful) decision to
   not display the buffer.  I don't have a good counter-proposition, tho
   ("no-display-ok" is what comes to my mind, but I don't like it too
   much either).

> Why didn't you implement the may-fail part?  IIUC `display-buffer'
> should *not* return nil unless may-fail has been set.  So if may-fail is
> not set, `display-buffer' should either create a new window, a new
> frame, or reuse some window at any cost.

AFAICT, display-buffer does already try pretty hard.  I think that if
display-buffer returns nil in a context where may-fail is nil, it's not
a bug in display-buffer but in some of the ACTIONS, and I see no reason
why `display-buffer' should try and cover up the problem.

> And there's no use for `display-buffer' returning t.

You mean, it should return nil (and never t) if the return value is not
a window?
I guess it would indeed be cleaner, yes,


        Stefan

bug#13594: Planning Emacs-24.4

[Leo Liu]

On 2013-11-18 21:19 +0800, martin rudalics wrote:
> (1) Provide two actions designators may-fail and do-fail, say.
>
> (2) When may-fail is set and no window is found, return nil.  When
>     may-fail is not set, return the most suitable window.
>
> (3) When may-fail and do-fail are both set, break the
>
> 	(while (and functions (not window))
> 	  (setq window (funcall (car functions) buffer alist)
> 	  	functions (cdr functions)))
>
>     loop in `display-buffer' (for example, by having the function called
>     return 'fail) and return nil.
>
> martin

Hi Martin,

I wonder if at this point you can take over fixing this bug. I am
unfamiliar with windowing. So I might not be able to propose a fix in
perspective.

BTW I use this action to suppress displaying the compilation window and
it works.

  (cons (lambda (buf action)
          (and (assq 'may-fail (cdr action))
               (with-current-buffer buf
                 (derived-mode-p 'ggtags-global-mode))))
        (list (lambda (&rest _) t)))

The only thing I want is a way to tell compile.el not to pop up the
window.

Leo

bug#13594: Planning Emacs-24.4

[martin rudalics]

 > I wonder if at this point you can take over fixing this bug. I am
 > unfamiliar with windowing. So I might not be able to propose a fix in
 > perspective.

I thought you intially wanted to fix the problem that a caller cannot
handle a nil return value from `display-buffer'.  But Stefan says that
it's not necessary to do that and I agree with him.  So your fix is OK
but I'd just make the last line of `display-buffer' something like

(when (windowp window) window)

and you don't have to talk about a return value of t.  Returning t sounds
not very intuitive when a function fails to accomplish something.

martin

bug#13594: Planning Emacs-24.4

[martin rudalics]

> AFAICT, display-buffer does already try pretty hard.  I think that if
> display-buffer returns nil in a context where may-fail is nil, it's not
> a bug in display-buffer but in some of the ACTIONS, and I see no reason
> why `display-buffer' should try and cover up the problem.

Fully agreed.  I just misunderstood the intentions of the patch.

martin

bug#13594: Planning Emacs-24.4

[Leo Liu]

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

On 2013-11-18 23:20 +0800, martin rudalics wrote:
> I thought you intially wanted to fix the problem that a caller cannot
> handle a nil return value from `display-buffer'.  But Stefan says that
> it's not necessary to do that and I agree with him.  So your fix is OK
> but I'd just make the last line of `display-buffer' something like
>
> (when (windowp window) window)
>
> and you don't have to talk about a return value of t.  Returning t sounds
> not very intuitive when a function fails to accomplish something.

OK, thank you for the quick response. I have re-worked the patch based
on Stefan's and your comments. Please review it. Thanks.

Leo


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 13594.diff --]
[-- Type: text/x-patch, Size: 5187 bytes --]

=== modified file 'lisp/progmodes/compile.el'
--- lisp/progmodes/compile.el	2013-10-23 16:25:56 +0000
+++ lisp/progmodes/compile.el	2013-11-18 15:43:29 +0000
@@ -1632,7 +1632,7 @@
       (set-buffer-modified-p nil))
     ;; Pop up the compilation buffer.
     ;; http://lists.gnu.org/archive/html/emacs-devel/2007-11/msg01638.html
-    (setq outwin (display-buffer outbuf))
+    (setq outwin (display-buffer outbuf '(nil (non-window-ok . t))))
     (with-current-buffer outbuf
       (let ((process-environment
 	     (append
@@ -1654,7 +1654,7 @@
 	     (list command mode name-function highlight-regexp))
 	(set (make-local-variable 'revert-buffer-function)
 	     'compilation-revert-buffer)
-	(set-window-start outwin (point-min))
+	(and outwin (set-window-start outwin (point-min)))
 
 	;; Position point as the user will see it.
 	(let ((desired-visible-point
@@ -1663,15 +1663,15 @@
 		   (point-max)
 		 ;; Normally put it at the top.
 		 (point-min))))
-	  (if (eq outwin (selected-window))
-	      (goto-char desired-visible-point)
+	  (goto-char desired-visible-point)
+	  (when (and outwin (not (eq outwin (selected-window))))
 	    (set-window-point outwin desired-visible-point)))
 
 	;; The setup function is called before compilation-set-window-height
 	;; so it can set the compilation-window-height buffer locally.
 	(if compilation-process-setup-function
 	    (funcall compilation-process-setup-function))
-	(compilation-set-window-height outwin)
+	(and outwin (compilation-set-window-height outwin))
 	;; Start the compilation.
 	(if (fboundp 'start-process)
 	    (let ((proc
@@ -2513,14 +2513,16 @@
                 ;; the error location if the two buffers are in two
                 ;; different frames.  So don't do it if it's not necessary.
                 pre-existing
-	      (display-buffer (marker-buffer msg))))
+	      (display-buffer (marker-buffer msg) '(nil (non-window-ok . t)))))
 	 (highlight-regexp (with-current-buffer (marker-buffer msg)
 			     ;; also do this while we change buffer
-			     (compilation-set-window w msg)
+			     (goto-char (marker-position msg))
+			     (and w (compilation-set-window w msg))
 			     compilation-highlight-regexp)))
     ;; Ideally, the window-size should be passed to `display-buffer'
     ;; so it's only used when creating a new window.
-    (unless pre-existing (compilation-set-window-height w))
+    (when (and (not pre-existing) w)
+      (compilation-set-window-height w))
 
     (if from-compilation-buffer
         ;; If the compilation buffer window was selected,
@@ -2631,9 +2633,12 @@
     (while (null buffer)    ;Repeat until the user selects an existing file.
       ;; The file doesn't exist.  Ask the user where to find it.
       (save-excursion            ;This save-excursion is probably not right.
-        (let ((pop-up-windows t))
-          (compilation-set-window (display-buffer (marker-buffer marker))
-                                  marker)
+        (let ((w (let ((pop-up-windows t))
+		   (display-buffer (marker-buffer marker)
+				   '(nil (non-window-ok . t))))))
+          (with-current-buffer (marker-buffer marker)
+	    (goto-char marker)
+	    (and w (compilation-set-window w marker)))
           (let* ((name (read-file-name
                         (format "Find this %s in (default %s): "
                                 compilation-error filename)

=== modified file 'lisp/window.el'
--- lisp/window.el	2013-11-12 07:25:14 +0000
+++ lisp/window.el	2013-11-18 15:43:29 +0000
@@ -5355,7 +5355,10 @@
  ACTION is a cons cell (FUNCTION . ALIST), where FUNCTION is a
   function or a list of functions.  Each such function should
   accept two arguments: a buffer to display and an alist of the
-  same form as ALIST.  See `display-buffer' for details.
+  same form as ALIST.  If (non-window-ok . t) is in ALIST, the
+  caller is prepared to handle non-window value and FUNCTION can
+  safely return non-window value to suppress displaying the
+  window.  See `display-buffer' for details.
 
 `display-buffer' scans this alist until it either finds a
 matching regular expression or the function specified by a
@@ -5439,9 +5442,12 @@
 ALIST is an arbitrary association list (alist).
 
 Each such FUNCTION should accept two arguments: the buffer to
-display and an alist.  Based on those arguments, it should either
-display the buffer and return the window, or return nil if unable
-to display the buffer.
+display and an alist.  Based on those arguments, it should
+display the buffer and return the window.  It may return nil if
+unable to display the buffer (which is rare and most callers
+don't check this value).  If the caller is prepared to handle
+non-window value it should pass (non-window-ok . t) as an element
+of the ALIST.
 
 The `display-buffer' function builds a function list and an alist
 by combining the functions and alists specified in
@@ -5542,7 +5548,7 @@
 	(while (and functions (not window))
 	  (setq window (funcall (car functions) buffer alist)
 	  	functions (cdr functions)))
-	window))))
+	(and (windowp window) window)))))
 
 (defun display-buffer-other-frame (buffer)
   "Display buffer BUFFER preferably in another frame.

bug#13594: Planning Emacs-24.4

[Stefan Monnier]

>> t was just a value to stop display-buffer searching down the action
>> list. this seems enough to manage display-buffer via
>> display-buffer-alist or the like.
> It is not.

In which sense is it not?

> Returning t when calling `display-buffer' without may-fail
> set doesn't improve anything.

Indeed, an FUNCTION that returns t when may-fail is not set is in error.
But this has never happened so far, so I'm not sure it's terribly
important to check it.

> And if my-fail is set, we can return nil
> because the caller is prepared for it.

t is the return value of the FUNCTION.  It can't be nil, since nil already
means "try the next FUNCTION".  `display-buffer' can then take this
t return value and turn it into a nil.

> And you still don't handle the case where a user doesn't want to see
> the buffer in the first place.

I think his code does handle it (by having the FUNCTION return t without
displaying the buffer).

> (1) Provide two actions designators may-fail and do-fail, say.

What would `do-fail' mean?  Is that a FUNCTION or an element of the ALIST?

> (2) When may-fail is set and no window is found, return nil.

We already do that (regardless of `may-fail', actually).  But "no window
is found" can pretty much never happen.

>     When may-fail is not set, return the most suitable window.

If no window is found, there is no "most suitable window".

> (3) When may-fail and do-fail are both set, break the
> 	(while (and functions (not window))
> 	  (setq window (funcall (car functions) buffer alist)
> 	  	functions (cdr functions)))
>     loop in `display-buffer' (for example, by having the function called
>     return 'fail) and return nil.

That sounds cumbersome, compared to the simple solution he uses now of
stopping when FUNCTION returns t (which the code already does, by virtue
of stopping as soon as FUNCTION returns non-nil).


        Stefan

bug#13594: Planning Emacs-24.4

[Stefan Monnier]

> OK, thank you for the quick response. I have re-worked the patch based
> on Stefan's and your comments. Please review it. Thanks.

Fine by me, thank you (I don't like `non-window-ok' either, but again
I have nothing better to suggest for replacement).


        Stefan

bug#13594: Planning Emacs-24.4

[Juri Linkov]

>> OK, thank you for the quick response. I have re-worked the patch based
>> on Stefan's and your comments. Please review it. Thanks.
>
> Fine by me, thank you (I don't like `non-window-ok' either, but again
> I have nothing better to suggest for replacement).

A good name depends on how the users will customize it.

Currently display-buffer supports these actions:

 `display-buffer-same-window'
 `display-buffer-reuse-window'
 `display-buffer-pop-up-frame'
 `display-buffer-pop-up-window'
 `display-buffer-use-some-window'

Then a new action could have a name:

 `display-buffer-no-window'

The existing ALIST entries are:

 `inhibit-same-window'
 `inhibit-switch-frame'

Then a new property could be:

 `allow-no-window'

And the new action `display-buffer-no-window' could check
if `allow-no-window' is non-nil then it will return t
(for which `display-buffer' will return nil to the caller).

bug#13594: Planning Emacs-24.4

[Leo Liu]

On 2013-11-19 08:33 +0800, Stefan Monnier wrote:
> Fine by me, thank you (I don't like `non-window-ok' either, but again
> I have nothing better to suggest for replacement).
>
>
>         Stefan

I have committed the change use the name no-display-ok. I consider this
bug fixed and will close it in next days. Thank you for the help.

On 2013-11-19 08:54 +0800, Juri Linkov wrote:
[snipped 6 lines]
> A good name depends on how the users will customize it.
>
> Currently display-buffer supports these actions:
>
>  `display-buffer-same-window'
>  `display-buffer-reuse-window'
>  `display-buffer-pop-up-frame'
>  `display-buffer-pop-up-window'
>  `display-buffer-use-some-window'
>
> Then a new action could have a name:
>
>  `display-buffer-no-window'
>
> The existing ALIST entries are:
>
>  `inhibit-same-window'
>  `inhibit-switch-frame'
>
> Then a new property could be:
>
>  `allow-no-window'
>
> And the new action `display-buffer-no-window' could check
> if `allow-no-window' is non-nil then it will return t
> (for which `display-buffer' will return nil to the caller).

Hi Juri,

I am not too sure about those things so I will stop messing with them.
But I am happy to accommodate whatever changes made in this area.

Thanks,
Leo

bug#13594: Planning Emacs-24.4

[Stefan Monnier]

> Then a new property could be:
>  `allow-no-window'

I"m not fond of it, but it's maybe the least bad so far.


        Stefan

bug#13594: Planning Emacs-24.4

[martin rudalics]

 >>> t was just a value to stop display-buffer searching down the action
 >>> list. this seems enough to manage display-buffer via
 >>> display-buffer-alist or the like.
 >> It is not.
 >
 > In which sense is it not?

To handle the case where the application is not able to correctly deal
with a nil return value.

 >> Returning t when calling `display-buffer' without may-fail
 >> set doesn't improve anything.
 >
 > Indeed, an FUNCTION that returns t when may-fail is not set is in error.
 > But this has never happened so far, so I'm not sure it's terribly
 > important to check it.

So far t was not a valid return value for `display-buffer' which was
supposed to "Return the window chosen for displaying BUFFER-OR-NAME, or
nil if no such window is found.".

 >> And if my-fail is set, we can return nil
 >> because the caller is prepared for it.
 >
 > t is the return value of the FUNCTION.  It can't be nil, since nil already
 > means "try the next FUNCTION".  `display-buffer' can then take this
 > t return value and turn it into a nil.

That's what Leo implemented now.  In fact, this handles the erroneous
case where a user action could return a non-nil, non-window value and
cause `display-buffer' to return it.

 >> And you still don't handle the case where a user doesn't want to see
 >> the buffer in the first place.
 >
 > I think his code does handle it (by having the FUNCTION return t without
 > displaying the buffer).

I meant that he didn't explicitly write a function like
`display-buffer-no-window' as sketched by Juri.

 >> (1) Provide two actions designators may-fail and do-fail, say.
 >
 > What would `do-fail' mean?  Is that a FUNCTION or an element of the ALIST?

ALIST elements.  `may-fail' stands for "the application is capable of
handling a non-window return value".  `do-fail' means `display-buffer'
should return nil when and if this ALIST element gets handled (thus was
not overridden by another action).  For example, an application could
propose to not display a buffer by default but let the user override it.

 >> (2) When may-fail is set and no window is found, return nil.
 >
 > We already do that (regardless of `may-fail', actually).  But "no window
 > is found" can pretty much never happen.

I agreed earlier that we need not handle this case.

 >>     When may-fail is not set, return the most suitable window.
 >
 > If no window is found, there is no "most suitable window".

Emacs 23 had

		  (or (get-lru-window frame-to-use)
		      (let ((window (get-buffer-window buffer 'visible)))
			(unless (and not-this-window
				     (eq window (selected-window)))
			  window))
		      (get-largest-window 'visible)
		      (let ((window (get-buffer-window buffer 0)))
			(unless (and not-this-window
				     (eq window (selected-window)))
			  window))
		      (get-largest-window 0)
		      (frame-selected-window (funcall pop-up-frame-function))))

which made it really hard for a user to prevent finding a "most suitable
window" ;-)

 >
 >> (3) When may-fail and do-fail are both set, break the
 >> 	(while (and functions (not window))
 >> 	  (setq window (funcall (car functions) buffer alist)
 >> 	  	functions (cdr functions)))
 >>     loop in `display-buffer' (for example, by having the function called
 >>     return 'fail) and return nil.
 >
 > That sounds cumbersome, compared to the simple solution he uses now of
 > stopping when FUNCTION returns t (which the code already does, by virtue
 > of stopping as soon as FUNCTION returns non-nil).

It would have just meant that a function like `display-buffer-no-window'
would have to return 'fail in order to confirm that it is aware of the
semantics of a thing like no-display-ok.

martin

bug#13594: Planning Emacs-24.4

[martin rudalics]

 > I have committed the change use the name no-display-ok.

Thanks.  Please consider adding a NEWS entry and describing the feature
in the Elisp manual.

martin

bug#13594: Planning Emacs-24.4

[Juri Linkov]

> It would have just meant that a function like `display-buffer-no-window'
> would have to return 'fail in order to confirm that it is aware of the
> semantics of a thing like no-display-ok.

Do you think `display-buffer-no-window' should return 'fail instead of t
when the user overrides the default action?

Then it could be defined as:

(defun display-buffer-no-window (buffer alist)
  (when (cdr (assq 'allow-no-window alist))
    'fail))

And the user customization:

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

bug#13594: Planning Emacs-24.4

[Leo Liu]

On 2013-11-19 15:42 +0800, martin rudalics wrote:
> Thanks.  Please consider adding a NEWS entry and describing the feature
> in the Elisp manual.

Done.

Leo

bug#13594: Planning Emacs-24.4

[Stefan Monnier]

> Do you think `display-buffer-no-window' should return 'fail instead of t
> when the user overrides the default action?

I don't think it really matters (and IIUC the current code doesn't care
abut the distinction).

> Then it could be defined as:
> (defun display-buffer-no-window (buffer alist)
>   (when (cdr (assq 'allow-no-window alist))
>     'fail))
> And the user customization:
> (add-to-list 'display-buffer-alist '("\\*compilation\\*"
>  display-buffer-no-window (nil)))

Sounds fine.


        Stefan

bug#13594: Planning Emacs-24.4

[martin rudalics]

>> Thanks.  Please consider adding a NEWS entry and describing the feature
>> in the Elisp manual.
> 
> Done.

Thanks, martin

bug#13594: Planning Emacs-24.4

[martin rudalics]

 > Do you think `display-buffer-no-window' should return 'fail instead of t
 > when the user overrides the default action?

It certainly would indicate the intentions more clearly.

 > Then it could be defined as:
 >
 > (defun display-buffer-no-window (buffer alist)
 >   (when (cdr (assq 'allow-no-window alist))
 >     'fail))
 >
 > And the user customization:
 >
 > (add-to-list 'display-buffer-alist '("\\*compilation\\*" display-buffer-no-window (nil)))

Exactly.

martin

bug#13594: Planning Emacs-24.4

[Juri Linkov]

>> Then it could be defined as:
>> (defun display-buffer-no-window (buffer alist)
>>   (when (cdr (assq 'allow-no-window alist))
>>     'fail))
>> And the user customization:
>> (add-to-list 'display-buffer-alist '("\\*compilation\\*"
>>  display-buffer-no-window (nil)))
>
> Sounds fine.

This patch adds a new action to window.el and also adds support
for not displaying a window for the buffer `*Async Shell Command*'
of `async-shell-command' that will be possible to customize with

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

=== modified file 'lisp/window.el'
--- lisp/window.el	2013-11-20 02:44:38 +0000
+++ lisp/window.el	2013-11-21 00:30:00 +0000
@@ -5501,6 +5501,9 @@ (defun display-buffer (buffer-or-name &o
     argument - a new window.  The function is supposed to adjust
     the width of the window; its return value is ignored.
 
+ `allow-no-window' -- A non-nil value allows the user to override
+    the default action and not display the buffer in a window.
+
 The ACTION argument to `display-buffer' can also have a non-nil
 and non-list value.  This means to display the buffer in a window
 other than the selected one, even if it is already displayed in
@@ -5830,6 +5833,16 @@ (defun display-buffer-use-some-window (b
 	(unless (cdr (assq 'inhibit-switch-frame alist))
 	  (window--maybe-raise-frame (window-frame window)))))))
 
+(defun display-buffer-no-window (buffer alist)
+  "Display BUFFER in no window.
+If ALIST has a non-nil `allow-no-window' entry, then don't display
+a window at all.  This makes possible to override the default action
+and avoid displaying the buffer.  It is assumed that when the caller
+specifies a non-nil `allow-no-window' then it can handle a nil value
+returned from `display-buffer' in this case."
+  (when (cdr (assq 'allow-no-window alist))
+    'fail))
+
 ;;; Display + selection commands:
 (defun pop-to-buffer (buffer &optional action norecord)
   "Select buffer BUFFER in some window, preferably a different one.

=== modified file 'lisp/simple.el'
--- lisp/simple.el	2013-10-30 02:45:53 +0000
+++ lisp/simple.el	2013-11-21 00:30:04 +0000
@@ -2820,7 +2820,7 @@ (defun shell-command (command &optional
 		  ;; which comint sometimes adds for prompts.
 		  (let ((inhibit-read-only t))
 		    (erase-buffer))
-		  (display-buffer buffer)
+		  (display-buffer buffer '(nil (allow-no-window . t)))
 		  (setq default-directory directory)
 		  (setq proc (start-process "Shell" buffer shell-file-name
 					    shell-command-switch command))

bug#13594: Planning Emacs-24.4

[Leo Liu]

On 2013-11-21 08:30 +0800, Juri Linkov wrote:
> This patch adds a new action to window.el and also adds support
> for not displaying a window for the buffer `*Async Shell Command*'
> of `async-shell-command' that will be possible to customize with

What's the status of this bug? Should I close it or is there something
pending?

Thanks,
Leo

bug#13594: Planning Emacs-24.4

[Juri Linkov]

>> This patch adds a new action to window.el and also adds support
>> for not displaying a window for the buffer `*Async Shell Command*'
>> of `async-shell-command' that will be possible to customize with
>
> What's the status of this bug? Should I close it or is there something
> pending?

I installed the patch.  Please have a look and close this bug
if everything is fixed.

bug#13594: Planning Emacs-24.4

[Leo Liu]

Fixed in 24.4

On 2013-12-03 09:19 +0800, Juri Linkov wrote:
> I installed the patch.  Please have a look and close this bug
> if everything is fixed.

Thanks for the improvement.

Leo

bug#13594: Planning Emacs-24.4

[martin rudalics]

> I installed the patch.  Please have a look and close this bug
> if everything is fixed.

Thanks, martin