[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: master b3dd0ce: Provide new option `delete-window-set-selected'
From: |
Eli Zaretskii |
Subject: |
Re: master b3dd0ce: Provide new option `delete-window-set-selected' |
Date: |
Thu, 10 Jun 2021 11:37:16 +0300 |
A few comments about this changeset:
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".
> +Possible choices are @code{mru} (the default) to select the most
> +recently used window on that frame and @code{pos} to choose the window
^
There should be a comma before "and" there.
> +at the position of point of the previously selected window.
This part:
[...] 'pos' to choose the window at the position of point of the
previously selected window [...]
is confusing: when you say "position of point", the most likely
interpretation is the buffer position of point, which makes the
meaning of the value nonsensical. I understand that you meant the
frame-relative coordinates of point instead, so the text (here and in
the doc string) should IMO be amended to say that.
> + If this
> +option is @code{nil}, it means to choose the frame's first window
> +instead.
This text doesn't explain what does "the first window" mean in this
context.
> + 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?
> @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?
> +(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.
> +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?
> +(defcustom delete-window-set-selected 'mru
Wouldn't the name delete-window-choose-selected be better? 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?
Thanks.
- Re: master b3dd0ce: Provide new option `delete-window-set-selected',
Eli Zaretskii <=