Make window-list return windows for all frames

Make window-list return windows for all frames

[Arthur Miller]

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

While working on info.el I needed a function to return info windows on all
frames. I was a bit surprised to discover there is no built-in option in
window-list or elsewhere as far as I can see to get all windows for all frames,
or a filtered list containing interesting windows for all frames. I ended up
with this:

#+begin_src emacs-lisp
(defun window-list-by-mode (mode &optional all-frames)
  "Return list of windows displaying buffers whose major mode is MODE.

If ALL-FRAMES is non-nil, consider all frames, otherwise just selected
frame."
  (mapcar
   (lambda (window) (cons (prin1-to-string window) window))
   (cl-remove-if-not
    (lambda (window)
      (with-current-buffer (window-buffer window)
        (eq major-mode mode)))
    (if all-frames
        (apply #'nconc (mapcar (lambda (f) (window-list f)) (frame-list)))
      (window-list)))))
#+end_src

Anyway, looking at window.c, I can see that window_candidate_p actually does
consider windows on all frames, but 't option is never passed through from
window-list. To save gymnastics via apply and nconc and doing the double work, I
have patched window-list function to let 't option further and documented the
change. It seems to me that it works as intended. I have tested myself, I hope I
haven't missed some possible case, but I'm a little suspisious: what was the
reason why this wasn't documented and used? I mean, it is already there, but
cut-off for some reason?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Return-window-list-for-all-frames.patch --]
[-- Type: text/x-patch, Size: 2512 bytes --]

From 1123400eaebdbe68dab527f9fe4397050f112890 Mon Sep 17 00:00:00 2001
From: Arthur Miller <arthur.miller@live.com>
Date: Thu, 15 Jun 2023 14:17:08 +0200
Subject: [PATCH] Return window-list for all frames

Add option to window-list to return windows for all frames.
* src/window.c Modify to return all windows when FRAME is 't.
* doc/elisp/window.text Document the change.
---
 doc/lispref/windows.texi | 3 ++-
 src/window.c             | 8 +++++---
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/doc/lispref/windows.texi b/doc/lispref/windows.texi
index 0196ed0e813..bbb60403a86 100644
--- a/doc/lispref/windows.texi
+++ b/doc/lispref/windows.texi
@@ -227,7 +227,8 @@ Windows and Frames
 @defun window-list &optional frame minibuffer window
 This function returns a list of all live windows owned by the specified
 @var{frame}.  If @var{frame} is omitted or @code{nil}, it defaults to
-the selected frame (@pxref{Input Focus}).
+the selected frame (@pxref{Input Focus}).  If @var{frame} is @code{t},
+the list of windows for all frames is returned.
 
 The optional argument @var{minibuffer} specifies whether to include the
 minibuffer window (@pxref{Minibuffer Windows}) in that list.  If
diff --git a/src/window.c b/src/window.c
index e22fea0bb7a..7d1fa5deef6 100644
--- a/src/window.c
+++ b/src/window.c
@@ -2989,9 +2989,10 @@ window_list_1 (Lisp_Object window, Lisp_Object minibuf, Lisp_Object all_frames)
 }
 
 
-DEFUN ("window-list", Fwindow_list, Swindow_list, 0, 3, 0,
-       doc: /* Return a list of windows on FRAME, starting with WINDOW.
+DEFUN ("window-list", Fwindow_list, Swindow_list, 0, 3, 0, doc
+       : /* Return a list of windows on FRAME, starting with WINDOW.
 FRAME nil or omitted means use the selected frame.
+FRAME t means return list of live windows for all frames.
 WINDOW nil or omitted means use the window selected within FRAME.
 MINIBUF t means include the minibuffer window, even if it isn't active.
 MINIBUF nil or omitted means include the minibuffer window only
@@ -3002,10 +3003,11 @@ DEFUN ("window-list", Fwindow_list, Swindow_list, 0, 3, 0,
   if (NILP (window))
     window = FRAMEP (frame) ? XFRAME (frame)->selected_window : selected_window;
   CHECK_WINDOW (window);
+
   if (NILP (frame))
     frame = selected_frame;
 
-  if (!EQ (frame, XWINDOW (window)->frame))
+  if (!EQ (frame, XWINDOW (window)->frame) && !(BASE_EQ (frame, Qt)))
     error ("Window is on a different frame");
 
   return window_list_1 (window, minibuf, frame);
-- 
2.40.0

Re: Make window-list return windows for all frames

[Eli Zaretskii]

> From: Arthur Miller <arthur.miller@live.com>
> Date: Thu, 15 Jun 2023 14:41:55 +0200
> 
> While working on info.el I needed a function to return info windows on all
> frames. I was a bit surprised to discover there is no built-in option in
> window-list or elsewhere as far as I can see to get all windows for all frames,
> or a filtered list containing interesting windows for all frames. I ended up
> with this:

Please describe in more details why you needed the list of all the
windows.  E.g., why walk-windows won't do whatever you needed to do?

> --- a/doc/lispref/windows.texi
> +++ b/doc/lispref/windows.texi
> @@ -227,7 +227,8 @@ Windows and Frames
>  @defun window-list &optional frame minibuffer window
>  This function returns a list of all live windows owned by the specified
>  @var{frame}.  If @var{frame} is omitted or @code{nil}, it defaults to
> -the selected frame (@pxref{Input Focus}).
> +the selected frame (@pxref{Input Focus}).  If @var{frame} is @code{t},
> +the list of windows for all frames is returned.

Passive voice alert!

Re: Make window-list return windows for all frames

[Gregory Heytings]

>
> I was a bit surprised to discover there is no built-in option in 
> window-list or elsewhere as far as I can see to get all windows for all 
> frames,
>

Doesn't (window-list-1 nil nil t) do what you want?

Re: Make window-list return windows for all frames

[Po Lu]

Arthur Miller <arthur.miller@live.com> writes:

>  @var{frame}.  If @var{frame} is omitted or @code{nil}, it defaults to
> -the selected frame (@pxref{Input Focus}).
> +the selected frame (@pxref{Input Focus}).  If @var{frame} is @code{t},
> +the list of windows for all frames is returned.

``If FRAME is T, return a list of live windows on all frames.''

> -       doc: /* Return a list of windows on FRAME, starting with WINDOW.
> +DEFUN ("window-list", Fwindow_list, Swindow_list, 0, 3, 0, doc
> +       : /* Return a list of windows on FRAME, starting with WINDOW.
>  FRAME nil or omitted means use the selected frame.

Don't wrap `doc' like this.  It probably interferes with make-docfile.

Thanks for working on Emacs.

Re: Make window-list return windows for all frames

[Arthur Miller]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Arthur Miller <arthur.miller@live.com>
>> Date: Thu, 15 Jun 2023 14:41:55 +0200
>> 
>> While working on info.el I needed a function to return info windows on all
>> frames. I was a bit surprised to discover there is no built-in option in
>> window-list or elsewhere as far as I can see to get all windows for all frames,
>> or a filtered list containing interesting windows for all frames. I ended up
>> with this:
>
> Please describe in more details why you needed the list of all the

I need list of all windows with displaing buffers in info-mode, so I just get
all windows and cull any non-info windows.

>   E.g., why walk-windows won't do whatever you needed to do?

I didn't know about it :). Well yes, it does indeed. It is a bit ugly, since I
have to use let and push, but I can save on one mapping to print names for
completing read:

(defun window-list-by-mode (mode &optional all-frames)
  (let ((window-list))
    (walk-windows
     (lambda (w)
       (with-current-buffer (window-buffer w)
         (when (eq major-mode mode)
           (push (cons (prin1-to-string w) w) window-list))))
     nil all-frames)
    window-list))

I would still argue that letting 't option be available through window list is
basically free and resembles more get-buffer-window and it's all-frames argument
and is overall more elegant and less repetitive, would be something like this:

As Gregory points out, there is window-list-1 which basically is same thing as
window-list but with arguments in different order. I was a bit blind, but it
feels a bit asymetric and strange not allow window-list have same semmantic for
frame parameter as in other functions, when it is already there, just one
comparison short :).

>> --- a/doc/lispref/windows.texi
>> +++ b/doc/lispref/windows.texi
>> @@ -227,7 +227,8 @@ Windows and Frames
>>  @defun window-list &optional frame minibuffer window
>>  This function returns a list of all live windows owned by the specified
>>  @var{frame}.  If @var{frame} is omitted or @code{nil}, it defaults to
>> -the selected frame (@pxref{Input Focus}).
>> +the selected frame (@pxref{Input Focus}).  If @var{frame} is @code{t},
>> +the list of windows for all frames is returned.
>
> Passive voice alert!

Re: Make window-list return windows for all frames

[Arthur Miller]

Gregory Heytings <gregory@heytings.org> writes:

>>
>> I was a bit surprised to discover there is no built-in option in window-list
>> or elsewhere as far as I can see to get all windows for all frames,
>>
>
> Doesn't (window-list-1 nil nil t) do what you want?

Indeed it does, I was a bit blind there. I usually consider those functions
with names ending in -1 as internal to implementation, so I haven't even looked
there, so I missed that one is actually exposed to lisp.

Now question is, as pointed in answer to Eli, why should window-list not have
same semantics as the rest, since both get-buffer-window, and window-list-1, it
makes the API a bit assymetric and arcane, when its already there and costs
basically nothing. 

Re: Make window-list return windows for all frames

[Gregory Heytings]

>>> I was a bit surprised to discover there is no built-in option in 
>>> window-list or elsewhere as far as I can see to get all windows for 
>>> all frames,
>>
>> Doesn't (window-list-1 nil nil t) do what you want?
>
> Indeed it does, I was a bit blind there. I usually consider those 
> functions with names ending in -1 as internal to implementation, so I 
> haven't even looked there, so I missed that one is actually exposed to 
> lisp.
>
> Now question is, as pointed in answer to Eli, why should window-list not 
> have same semantics as the rest, since both get-buffer-window, and 
> window-list-1, it makes the API a bit assymetric and arcane, when its 
> already there and costs basically nothing.
>

My understanding is that window-list considers windows on a given frame, 
and window-list-1 considers windows on all frames (unless it is asked to 
consider only a given frame).

If you want to allow the argument t to window-list to mean "all frames", 
you probably also want to allow the symbol 'visible and the number 0, 
which are accepted by window-list-1... and in that case window-list and 
window-list-1 become basically the same function.

Re: Make window-list return windows for all frames

[Eli Zaretskii]

> From: Arthur Miller <arthur.miller@live.com>
> Cc: emacs-devel@gnu.org
> Date: Thu, 15 Jun 2023 16:01:51 +0200
> 
> >   E.g., why walk-windows won't do whatever you needed to do?
> 
> I didn't know about it :). Well yes, it does indeed. It is a bit ugly, since I
> have to use let and push, but I can save on one mapping to print names for
> completing read:

If you are going to loop over the windows, walk-windows is the right
API, I think.

RE: [External] : Re: Make window-list return windows for all frames

[Drew Adams]

> (defun window-list-by-mode (mode &optional all-frames)
>   (let ((window-list))
>     (walk-windows
>      (lambda (w)
>        (with-current-buffer (window-buffer w)
>          (when (eq major-mode mode)
>            (push (cons (prin1-to-string w) w) window-list))))
>      nil all-frames)
>     window-list))

It's also possible to filter the buffers first.
 
(defun windows-with-mode (mode &optional minibuf all-frames)
  (let ((wins  ()))
    (dolist (buf  (seq-filter
                   (lambda (buf)
                     (with-current-buffer buf
                       (eq major-mode mode)))
                   (buffer-list)))
      (setq wins  (nconc (get-buffer-window-list
                          buf minibuf all-frames)
                         wins)))
    wins))

(windows-with-mode 'Info-mode nil 'visible)

But it's no doubt faster to filter the `window-list',
just as you've done, because there are typically many
buffers that are not displayed.

Re: Make window-list return windows for all frames

[Arthur Miller]

Gregory Heytings <gregory@heytings.org> writes:

>>>> I was a bit surprised to discover there is no built-in option in window-list
>>>> or elsewhere as far as I can see to get all windows for all frames,
>>>
>>> Doesn't (window-list-1 nil nil t) do what you want?
>>
>> Indeed it does, I was a bit blind there. I usually consider those functions
>> with names ending in -1 as internal to implementation, so I haven't even
>> looked there, so I missed that one is actually exposed to lisp.
>>
>> Now question is, as pointed in answer to Eli, why should window-list not have
>> same semantics as the rest, since both get-buffer-window, and window-list-1,
>> it makes the API a bit assymetric and arcane, when its already there and costs
>> basically nothing.
>>
>
> My understanding is that window-list considers windows on a given frame, and
> window-list-1 considers windows on all frames (unless it is asked to consider
> only a given frame).

Yes, that is what they do, but they invert the meaning of 'frame'. I don't see
why it would be a crime if window-list also returned all visible windows if
asked, as window-list-1 can be also inverted to do the thing on just one frame.

> If you want to allow the argument t to window-list to mean "all frames", you
> probably also want to allow the symbol 'visible and the number 0, which are
> accepted by window-list-1... and in that case window-list and window-list-1
> become basically the same function.

Basically I do see them as the same thing just slightly different entrance.

y immidiate thought is why we even have two of those, why we couldn't do with
just one from the beginning; but they are there now, so it ain't gonna change.

I don't see a reason why you would have to pass 0 and 'visible' just to make
'frame' argument more in line with other places with similar functionality.

Re: Make window-list return windows for all frames

[Arthur Miller]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Arthur Miller <arthur.miller@live.com>
>> Cc: emacs-devel@gnu.org
>> Date: Thu, 15 Jun 2023 16:01:51 +0200
>> 
>> >   E.g., why walk-windows won't do whatever you needed to do?
>> 
>> I didn't know about it :). Well yes, it does indeed. It is a bit ugly, since I
>> have to use let and push, but I can save on one mapping to print names for
>> completing read:
>
> If you are going to loop over the windows, walk-windows is the right
> API, I think.

We don't always want to loop over windows to do something with them,
sometimes we just need the list to feed into something else, get the length for
the number of live windows.

It is also about the API, perhaps it is just me, but I expected window-list to
take 't for the frame and consder all frames, because of get-buffer-window doing
so. I thought it was a pattern, but it obviously is not. Probably just me, but I
see no reason to forbid window-list to act so, since the functionality is there,
for free.

Anyway, it was just a suggestion. I understand it is now a very popular one :).

Re: Make window-list return windows for all frames

[Gregory Heytings]

>
> I don't see why it would be a crime if window-list also returned all 
> visible windows if asked, as window-list-1 can be also inverted to do 
> the thing on just one frame.
>

It wouldn't be a crime, for sure ;-)  It's just that the pattern to 
consider all frames uses window-list-1 (see window.el).

Re: [External] : Re: Make window-list return windows for all frames

[Arthur Miller]

Drew Adams <drew.adams@oracle.com> writes:

>> (defun window-list-by-mode (mode &optional all-frames)
>>   (let ((window-list))
>>     (walk-windows
>>      (lambda (w)
>>        (with-current-buffer (window-buffer w)
>>          (when (eq major-mode mode)
>>            (push (cons (prin1-to-string w) w) window-list))))
>>      nil all-frames)
>>     window-list))
>
> It's also possible to filter the buffers first.
>  
> (defun windows-with-mode (mode &optional minibuf all-frames)
>   (let ((wins  ()))
>     (dolist (buf  (seq-filter
>                    (lambda (buf)
>                      (with-current-buffer buf
>                        (eq major-mode mode)))
>                    (buffer-list)))
>       (setq wins  (nconc (get-buffer-window-list
>                           buf minibuf all-frames)
>                          wins)))
>     wins))
>
> (windows-with-mode 'Info-mode nil 'visible)
>
> But it's no doubt faster to filter the `window-list',
> just as you've done, because there are typically many
> buffers that are not displayed.

Indeed, but as you say probably slower and seems even less elegant than the
niether-so-elegant let-wrap over walk-windows.

Personally I would prefer more functional interface to it, something like map,
but that takes a predicate to filter stuff out; lighter then cl-remove-if and
cl-remove-if-not (no :key and that stuff, no need to pull in cl-seq) and
more elisp like-ish. Something like:

(cull predicate list)

For example (cull #'oddp '(1 2 3 4 5)) => (2 4)

We can do it in Lisp:

(defun cull (predicate list)
  "Remove elements from LIST that satisfy PREDICATE."
  (remq 'nil (mapcar (lambda (elt)
               (if (funcall predicate elt) nil elt)) list)))

(cull (lambda (w)
        (with-selected-window w
          (not (eq major-mode 'Info-mode))))
        (window-list-1 nil nil t))

=> (#<window 95 on *info*> #<window 102 on *info*<1>>)

I would prefer it though implemented in C so we don't have to do the iteration
through the list twice, but considering the last experience, I guess I should
not suggest a patch :).

Re: Make window-list return windows for all frames

[Arthur Miller]

Gregory Heytings <gregory@heytings.org> writes:

>>
>> I don't see why it would be a crime if window-list also returned all visible
>> windows if asked, as window-list-1 can be also inverted to do the thing on
>> just one frame.
>>
>
> It wouldn't be a crime, for sure ;-)  It's just that the pattern to consider all
> frames uses window-list-1 (see window.el).

Well at least after the discussion, I understand that it wasn't a bug that 't
option was left out from window-list :). As said in my first mail; I was
suspicios what it wasn't implemented the way I patched from the beginning, since
whomever implemented window_candidate_p had to be aware of it.

Also passing the 't to the original version, does end up in somewhat cryptic
message that the window is on a different frame. Perhaps that error message
could be somewhat more clear, but since it has been so for many years, I guess
it is not as important, otherwise other people would have screamed long time
ago.

Re: Make window-list return windows for all frames

[Arthur Miller]

Po Lu <luangruo@yahoo.com> writes:

> Arthur Miller <arthur.miller@live.com> writes:
>
>>  @var{frame}.  If @var{frame} is omitted or @code{nil}, it defaults to
>> -the selected frame (@pxref{Input Focus}).
>> +the selected frame (@pxref{Input Focus}).  If @var{frame} is @code{t},
>> +the list of windows for all frames is returned.
>
> ``If FRAME is T, return a list of live windows on all frames.''
>
>> -       doc: /* Return a list of windows on FRAME, starting with WINDOW.
>> +DEFUN ("window-list", Fwindow_list, Swindow_list, 0, 3, 0, doc
>> +       : /* Return a list of windows on FRAME, starting with WINDOW.
>>  FRAME nil or omitted means use the selected frame.
>
> Don't wrap `doc' like this.  It probably interferes with make-docfile.

Interesting. I haven't noticed; just accidental;

I have just added one line below, in-between two already existing lines.

Re: Make window-list return windows for all frames

[Gregory Heytings]

>
> It is also about the API, perhaps it is just me, but I expected 
> window-list to take 't for the frame and consider all frames, because of 
> get-buffer-window doing so.
>

The problem I see is that twelve window-related functions (next-window, 
previous-window, window-list-1, get-buffer-window, walk-windows, 
get-window-with-predicate, get-lru-window, get-mru-window, 
get-largest-window, get-buffer-window-list, one-window-p, 
delete-windows-on) already accept t for their ALL-FRAMES argument, 
together with the symbol 'visible, the number 0, and a frame.  Why would 
window-list only accept t?

Re: Make window-list return windows for all frames

[Arthur Miller]

Gregory Heytings <gregory@heytings.org> writes:

>>
>> It is also about the API, perhaps it is just me, but I expected window-list to
>> take 't for the frame and consider all frames, because of get-buffer-window
>> doing so.
>>
>
> The problem I see is that twelve window-related functions (next-window,
> previous-window, window-list-1, get-buffer-window, walk-windows,
> get-window-with-predicate, get-lru-window, get-mru-window, get-largest-window,
> get-buffer-window-list, one-window-p, delete-windows-on) already accept t for
> their ALL-FRAMES argument, together with the symbol 'visible, the number 0, and
> a frame.  Why would window-list only accept t?

Then I will agree with you :). Allow frame/all-frames to mean the same in all
places and thus easier to learn/use.

Yes, window-list and window-list-1 becaomes the same, just with arguments in
different order. Could even remove c version of window-list, and implement it as
a call to window-list-1. Less code, less maintanance.

Re: Make window-list return windows for all frames

[Eli Zaretskii]

> From: Arthur Miller <arthur.miller@live.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel@gnu.org
> Date: Fri, 16 Jun 2023 12:03:53 +0200
> 
> Gregory Heytings <gregory@heytings.org> writes:
> 
> >>
> >> It is also about the API, perhaps it is just me, but I expected window-list to
> >> take 't for the frame and consider all frames, because of get-buffer-window
> >> doing so.
> >>
> >
> > The problem I see is that twelve window-related functions (next-window,
> > previous-window, window-list-1, get-buffer-window, walk-windows,
> > get-window-with-predicate, get-lru-window, get-mru-window, get-largest-window,
> > get-buffer-window-list, one-window-p, delete-windows-on) already accept t for
> > their ALL-FRAMES argument, together with the symbol 'visible, the number 0, and
> > a frame.  Why would window-list only accept t?
> 
> Then I will agree with you :). Allow frame/all-frames to mean the same in all
> places and thus easier to learn/use.
> 
> Yes, window-list and window-list-1 becaomes the same, just with arguments in
> different order. Could even remove c version of window-list, and implement it as
> a call to window-list-1. Less code, less maintanance.

Martin, are there any reason we'd not want window-list to accept t as
the FRAME argument?

Re: Make window-list return windows for all frames

[martin rudalics]

 >> Yes, window-list and window-list-1 becaomes the same, just with arguments in
 >> different order. Could even remove c version of window-list, and implement it as
 >> a call to window-list-1. Less code, less maintanance.
 >
 > Martin, are there any reason we'd not want window-list to accept t as
 > the FRAME argument?

None on my side.  The current implementation of 'window-list' simply
throws an error for (window-list t).  Back then I made 'window-list-1' a
separate function because with

(defvar frame-1 (selected-frame))
(defvar window-2 (frame-root-window (make-frame)))

calling

(window-list frame-1 nil window-2)

complains while

(window-list-1 window-2 nil frame-1)

doesn't.  People were picky about such issues in the past.

martin

Re: Make window-list return windows for all frames

[Eli Zaretskii]

> Date: Fri, 16 Jun 2023 16:21:54 +0200
> Cc: gregory@heytings.org, emacs-devel@gnu.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  >> Yes, window-list and window-list-1 becaomes the same, just with arguments in
>  >> different order. Could even remove c version of window-list, and implement it as
>  >> a call to window-list-1. Less code, less maintanance.
>  >
>  > Martin, are there any reason we'd not want window-list to accept t as
>  > the FRAME argument?
> 
> None on my side.  The current implementation of 'window-list' simply
> throws an error for (window-list t).  Back then I made 'window-list-1' a
> separate function because with
> 
> (defvar frame-1 (selected-frame))
> (defvar window-2 (frame-root-window (make-frame)))
> 
> calling
> 
> (window-list frame-1 nil window-2)
> 
> complains while
> 
> (window-list-1 window-2 nil frame-1)
> 
> doesn't.  People were picky about such issues in the past.

Thanks.  then I guess we can install (on master) the patch which makes
window-list accept t as the FRAME argument.

Re: Make window-list return windows for all frames

[martin rudalics]

 > Thanks.  then I guess we can install (on master) the patch which makes
 > window-list accept t as the FRAME argument.

I haven't seen the patch but I suppose that all it has to do is to skip
the

   if (!EQ (frame, XWINDOW (window)->frame))
     error ("Window is on a different frame");

check when FRAME is not a live frame.

martin

Re: Make window-list return windows for all frames

[Eli Zaretskii]

> Date: Sat, 17 Jun 2023 11:39:43 +0200
> Cc: arthur.miller@live.com, gregory@heytings.org, emacs-devel@gnu.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  > Thanks.  then I guess we can install (on master) the patch which makes
>  > window-list accept t as the FRAME argument.
> 
> I haven't seen the patch but I suppose that all it has to do is to skip
> the
> 
>    if (!EQ (frame, XWINDOW (window)->frame))
>      error ("Window is on a different frame");
> 
> check when FRAME is not a live frame.

That, and a suitable change to the doc string, yes.

Re: Make window-list return windows for all frames

[Arthur Miller]

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Sat, 17 Jun 2023 11:39:43 +0200
>> Cc: arthur.miller@live.com, gregory@heytings.org, emacs-devel@gnu.org
>> From: martin rudalics <rudalics@gmx.at>
>> 
>>  > Thanks.  then I guess we can install (on master) the patch which makes
>>  > window-list accept t as the FRAME argument.
>> 
>> I haven't seen the patch but I suppose that all it has to do is to skip
>> the
>> 
>>    if (!EQ (frame, XWINDOW (window)->frame))
>>      error ("Window is on a different frame");
>> 
>> check when FRAME is not a live frame.
>
> That, and a suitable change to the doc string, yes.
That was basically what patch did, minus that I seem to messed up with some
formatting as pointed by Po (thanks Po).

I have got idea to make a few ert tests too, so I'll resend patch when done.

Thanks for the input from all, it was cool to hear a bit of history from Martin,
and I learned quite about window function by Gregory :).

Re: Make window-list return windows for all frames

[Arthur Miller]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Sat, 17 Jun 2023 11:39:43 +0200
>> Cc: arthur.miller@live.com, gregory@heytings.org, emacs-devel@gnu.org
>> From: martin rudalics <rudalics@gmx.at>
>> 
>>  > Thanks.  then I guess we can install (on master) the patch which makes
>>  > window-list accept t as the FRAME argument.
>> 
>> I haven't seen the patch but I suppose that all it has to do is to skip
>> the
>> 
>>    if (!EQ (frame, XWINDOW (window)->frame))
>>      error ("Window is on a different frame");
>> 
>> check when FRAME is not a live frame.
>
> That, and a suitable change to the doc string, yes.

Hello,

can you please see if this is a suitable change?

I seem to have done one extra empty line cleanup this time; is that OK or are
there supposed to be two empty lines?


A side question (for anyone):

When runing ert in my normal Emacs with my configuration, I get this (top of)
stack trace:

(debugger-make-xrefs)
  (let ((debugger-previous-backtrace nil)) (debugger-make-xrefs))
  (save-restriction (narrow-to-region begin (point)) (let ((debugger-previous-backtrace nil)) (debugger-make-xrefs)))
  (save-excursion (save-restriction (narrow-to-region begin (point)) (let ((debugger-previous-backtrace nil)) (debugger-make-xrefs))))
  ert--make-xrefs-region(175 216)
  ....

But when I run ert in emacs -Q, it works fine, so it is obviously something in
my setup. Where to look for what is wrong, any advice on top of the head? Why
would xref be involved here? Any idea?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Allow-t-for-FRAME-in-window-list.patch --]
[-- Type: text/x-patch, Size: 3446 bytes --]

From 2277a0e80a89b5f34812e2c8e781bd3a7c81c392 Mon Sep 17 00:00:00 2001
From: Arthur Miller <arthur.miller@live.com>
Date: Mon, 19 Jun 2023 08:52:23 +0200
Subject: [PATCH] Allow t for FRAME in window-list

* src/window.c (window-list):
Let window-list pass t as FRAME parameter further to window_list_1.
* test/src/window-tests.el: New file.
* test/src/window-tests.el (window-tests-window-list):
Few tests to test that 't for all-frames works as in window-list-1.
---
 src/window.c             |  3 ++-
 test/src/window-tests.el | 45 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 1 deletion(-)
 create mode 100644 test/src/window-tests.el

diff --git a/src/window.c b/src/window.c
index f4e09f49eae..87a80cacadc 100644
--- a/src/window.c
+++ b/src/window.c
@@ -2992,6 +2992,7 @@ window_list_1 (Lisp_Object window, Lisp_Object minibuf, Lisp_Object all_frames)
 DEFUN ("window-list", Fwindow_list, Swindow_list, 0, 3, 0,
        doc: /* Return a list of windows on FRAME, starting with WINDOW.
 FRAME nil or omitted means use the selected frame.
+FRAME t means consider all windows on all existing frames.
 WINDOW nil or omitted means use the window selected within FRAME.
 MINIBUF t means include the minibuffer window, even if it isn't active.
 MINIBUF nil or omitted means include the minibuffer window only
@@ -3005,7 +3006,7 @@ DEFUN ("window-list", Fwindow_list, Swindow_list, 0, 3, 0,
   if (NILP (frame))
     frame = selected_frame;
 
-  if (!EQ (frame, XWINDOW (window)->frame))
+  if (!EQ (frame, XWINDOW (window)->frame) && !(BASE_EQ (frame, Qt)))
     error ("Window is on a different frame");
 
   return window_list_1 (window, minibuf, frame);
diff --git a/test/src/window-tests.el b/test/src/window-tests.el
new file mode 100644
index 00000000000..b062318dd25
--- /dev/null
+++ b/test/src/window-tests.el
@@ -0,0 +1,45 @@
+;;; window-tests.el --- Tests for src/window.c  -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2023  Arthur Miller
+
+;; Author: Arthur Miller <arthur.miller@live.com>
+;; Keywords:
+
+;; This program is free software; you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; This program is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;;
+
+;;; Code:
+
+(require 'ert)
+
+(ert-deftest window-tests-window-list ()
+  (let ((windows-orig (window-list-1 nil nil t))
+        (test-frame (make-frame '(visibility . nil))))
+    (with-selected-frame test-frame
+      (split-window-right)
+      (should (= (length (window-list)) 2))
+      (should (= (length (window-list t))
+                 (+ (length windows-orig) 2)))
+      (should (= (length (window-list))
+                 (length (window-list-1
+                          nil nil (selected-frame)))))
+      (should (= (length (window-list t))
+                 (length (window-list-1 nil nil t))))
+      (delete-frame))))
+
+(provide 'window-tests)
+;;; window-tests.el ends here
-- 
2.41.0

Re: Make window-list return windows for all frames

[martin rudalics]

 > can you please see if this is a suitable change?

 > +  if (!EQ (frame, XWINDOW (window)->frame) && !(BASE_EQ (frame, Qt)))

I would instead write

   if (FRAMEP (frame) && !EQ (XWINDOW (window)->frame, frame))

and get the first part of what you earlier described as

 > Yes, window-list and window-list-1 becaomes the same, just with arguments in
 > different order. Could even remove c version of window-list, and implement it as
 > a call to window-list-1. Less code, less maintanance.

for free.  (If you also want to "remove c version of window-list", you
should teach run_window_configuration_change_hook to call window_list_1
first).

martin

Re: Make window-list return windows for all frames

[Arthur Miller]

martin rudalics <rudalics@gmx.at> writes:

>> can you please see if this is a suitable change?
>
>> +  if (!EQ (frame, XWINDOW (window)->frame) && !(BASE_EQ (frame, Qt)))
>
> I would instead write
>
>   if (FRAMEP (frame) && !EQ (XWINDOW (window)->frame, frame))
>
> and get the first part of what you earlier described as
>
>> Yes, window-list and window-list-1 becaomes the same, just with arguments in
>> different order. Could even remove c version of window-list, and implement it as
>> a call to window-list-1. Less code, less maintanance.
>
> for free.  (If you also want to "remove c version of window-list", you
> should teach run_window_configuration_change_hook to call window_list_1
> first).

Cool; thank you, that was more than what I hoped to learn. 

As I understand this has being long time in Emacs and nobody else has complained
about the api and using this, so after a second thought I guess it is better
not to fix what is not broken, so I'll take my idea about consistent frame
parameter naming back. 

Sorry for a bit late response, I was doing the Info thing, meant to write a bit
earlier. Thank you very much for the input and for the history. Cool to learn.

> martin

Best regards