From: martin rudalics <rudalics@gmx.at>
To: Eli Zaretskii <eliz@gnu.org>
Cc: emacs-devel@gnu.org
Subject: Re: master b3dd0ce: Provide new option `delete-window-set-selected'
Date: Thu, 10 Jun 2021 17:01:59 +0200 [thread overview]
Message-ID: <d3e602f3-292c-f4db-79a8-bbfb8e248e02@gmx.at> (raw)
In-Reply-To: <83a6nycdz7.fsf@gnu.org>
> A few comments about this changeset:
Appreciated!
> Why a variable that is a user option is documented only in the ELisp
> manual? I think it should also be mentioned in the "Change Window"
> node of the user manual, where we describe "C-x 0" and "C-x 1".
In a restricted sense I suppose because we nowhere describe terms like
"frame selected window" or "most recently used window" there. So in the
C-x 0 paragraph something like "The option @code{delete-window..} allows
to choose which window becomes the new selected window instead." should
do what you have in mind here. OK?
>> + Note that a window with a non-@code{nil}
>> +@code{no-other-window} parameter is never chosen.
>
> Doesn't this depend on some argument to the relevant functions?
Yes. I have added such an argument to `get-mru-window' and friends.
But see my last remark below.
>> @cindex largest window
>> -@defun get-largest-window &optional all-frames dedicated not-selected
>> +@defun get-largest-window &optional all-frames dedicated not-selected no-other
>> This function returns the window with the largest area (height times
>> -width). The optional argument @var{all-frames} specifies the windows to
>> -search, and has the same meaning as in @code{next-window}.
>> -
>> -A minibuffer window is never a candidate. A dedicated window
>> -(@pxref{Dedicated Windows}) is never a candidate unless the optional
>> -argument @var{dedicated} is non-@code{nil}. The selected window is not
>> -a candidate if the optional argument @var{not-selected} is
>> -non-@code{nil}. If the optional argument @var{not-selected} is
>> -non-@code{nil} and the selected window is the only candidate, this
>> -function returns @code{nil}.
>> -
>> -If there are two candidate windows of the same size, this function
>> -prefers the one that comes first in the cyclic ordering of windows,
>> -starting from the selected window.
>> +width). If there are two candidate windows of the same size, it prefers
>> +the one that comes first in the cyclic ordering of windows, starting
>> +from the selected window. The meaning of the arguments is the same as
>> +for @code{get-lru-window}.
>> @end defun
>
> This hunk loses the information about minibuffer windows and dedicated
> windows, at least. it also seems to lose the information about when
> the selected window isn't a candidate. Why?
This information is now provided by the sentence "The meaning of the
arguments is the same as for ‘get-lru-window’." just as with the
preceding `get-mru-window'.
>> +(defun window-at-pos (x y &optional frame no-other)
>> + "Return live window at coordinates X, Y on specified FRAME.
>
> A better name for this function is window-at-x-y, IMO. "Pos" can have
> ambiguous interpretations, see above.
So `window-at-x-y' be it.
>> +X and Y are counted in pixels from an origin at 0, 0 of FRAME's
>> +native frame. A coordinate on an edge shared by two windows is
>> +attributed to the window on the right (or below). Return nil if
>
> If you say "FRAME-relative coordinates", doesn't that tell the same
> story, just much more succinctly and clearly?
It does.
>> +(defcustom delete-window-set-selected 'mru
>
> Wouldn't the name delete-window-choose-selected be better?
I'll go for that unless Juri has a better idea.
> The doc
> string says that much:
>
>> + "How to choose a frame's selected window after window deletion.
>
> Or maybe delete-selected-window-choose-replacement?
>
>> + ;; If `delete-window-internal' selected a window with a
>> + ;; non-nil 'no-other-window' parameter as its frame's
>> + ;; selected window, try to choose another one.
>> + (catch 'found
>> + (walk-window-tree
>> + (lambda (other)
>> + (unless (window-parameter other 'no-other-window)
>> + (set-frame-selected-window frame other)
>> + (throw 'found t)))
>> + frame))))
>
> That "try" means that we could sometimes fail, and return a window
> with a non-nil 'no-other-window' parameter, right? If so, the doc
> strings, which say such a window will _never_ be returned, are
> inaccurate, right?
Functions like `get-mru-window' never return such a window provided
their NO-OTHER argument has been set. The snippet you cite is from
`delete-window' which, in that case, leaves the window selected by
`delete-window-internal' alone and these doc strings are right. But
I'll amend the documentation of the new option appropriately.
Thanks, martin
next prev parent reply other threads:[~2021-06-10 15:01 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-10 8:37 master b3dd0ce: Provide new option `delete-window-set-selected' Eli Zaretskii
2021-06-10 15:01 ` martin rudalics [this message]
2021-06-10 15:29 ` Eli Zaretskii
2021-06-11 8:07 ` martin rudalics
2021-06-11 11:56 ` Eli Zaretskii
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d3e602f3-292c-f4db-79a8-bbfb8e248e02@gmx.at \
--to=rudalics@gmx.at \
--cc=eliz@gnu.org \
--cc=emacs-devel@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).