Subject: bug#64619: [PATCH] Add toggle-window-dedicated command

From: Eli Zaretskii <[email-masked]>
Date: Sun, 20 Aug 2023 08:57:26 +0300

> From: [email-masked]
> Date: Sat, 19 Aug 2023 20:02:35 +0000 (UTC)
> Cc: Spencer Baugh <[email-masked]>, Eli Zaretskii <[email-masked]>,
> 	martin rudalics <[email-masked]>, Drew Adams <[email-masked]>,
> 	[email-masked]
> 
> OK, added a NEWS entry and information in the manual.

Thanks, a few comments, mainly to the documentation, below.

> +  Sometimes, a window is ``dedicated'' to its current buffer.

Please add here a cross-reference to where dedicated windows are
described in the ELisp manual.  (This is our standard style of
documentation: whenever we mention a feature or terminology explained
elsewhere, we add a cross-reference there, unless one is already
available nearby.)

I would also add a @cindex entry here:

  @cindex dedicated window

> +@code{display-buffer} will avoid reusing dedicated windows most of the
> +time.  This is indicated by a ``d'' in the mode line.

And here there should be a cross-reference to where the mode line is
described.

>                                                        A window can
> +also be strongly dedicated, which prevents any changes to what buffer
> +that window displays;

"...prevents any changes to the buffer displayed in the window" is
easier to comprehend.

> +Usually, dedicated windows are used to display specialized buffers,
> +but dedication can sometimes be useful to interactively control
> +@code{display-buffer}'s window choices.

This text is not very useful.  How about adding an example or two for
when the user may wish to make a window dedicated, or make a dedicated
window not dedicated?  Then this command will make much more sense.

> +
> +@kindex C-x w d
> +@findex toggle-window-dedicated
> +  Toggle whether the current window is dedicated to the current
                        ^^^^^^^
"selected", not "current".  Or maybe "currently-selected".

We should also make changed in the section the describes the mode
line, to include this indication there.

> ++++
> +** 'd' in the mode line now indicates that the window is dedicated.
> +Windows have always been able to be dedicated to a specific buffer;
> +see 'window-dedicated-p'.  Now the mode line indicates the dedicated
> +status of a window, with 'd' appearing in the mode line if a window is
> +dedicated and 'D' if the window is strongly dedicated.

This should tell where on the mode line this indication will be shown.

> +(defun mode-line-window-control ()
> +  "Compute mode line construct for window dedicated state.
> +Value is used for `mode-line-window-dedicated', which see."
> +  (cond
> +   ((eq (window-dedicated-p) t)
> +   '(:propertize
> +     " D"
> +     help-echo "Window is strongly dedicated to current buffer"
> +     mouse-face 'mode-line-highlight))
> +   ((window-dedicated-p)
> +   '(:propertize
> +     " d"
> +     help-echo "Window is dedicated to current buffer"
> +     mouse-face 'mode-line-highlight))
> +   (t "")))

Why not allow toggling the state by a mouse click, like we do with the
buffer-writable indication?

And the tooltip text is too long, I think.  I suggest to use shorter
text:

   Window dedicated to its buffer
   Window strongly dedicated to its buffer

> @@ -675,6 +696,7 @@ mode-line-end-spaces
>  	            'mode-line-modified
>  	            'mode-line-remote)
>                'display '(min-width (5.0)))
> +             'mode-line-window-dedicated
>  	     'mode-line-frame-identification
>  	     'mode-line-buffer-identification
>  	     "   "

Why not add this to the group with the min-width property (and enlarge
that to 6.0)?  That way, we prevent annoying horizontal movement of
the rest of the mode-line display when toggling the state.

> +(defun toggle-window-dedicated (&optional window flag interactive)
> +  "Toggle whether WINDOW is dedicated.
> +
> +See `set-window-dedicated-p' for more details.  WINDOW defaults
> +to the currently selected window.  FLAG defaults to
> +`dedicated' (weak dedication) or `t' (strong dedication) with a
> +prefix argument.  If INTERACTIVE is non-nil, will print a message
> +about the dedication status of the window afterwards."

This is a command, so the doc string must be detailed enough and
user-friendly.  Sending the user to read a doc string of another
function is acceptable only if that other function is also a command
(so that the doc string doesn't include too many technicalities), and
if the description is very complicated in its user-facing parts.  I
don't think this is the case.  My suggestion is to use the following
doc string instead:

    Toggle whether WINDOW is dedicated to its current buffer.

  WINDOW must be a live window and defaults to the selected one.
  If FLAG is t (interactively, the prefix argument), make the window
  \"strongly\" dedicated to its buffer.  FLAG defaults to a non-nil,
  non-t value, and is passed to `set-window-dedicated-p', which see.
  If INTERACTIVE is non-nil, print a message describing the dedication
  status of WINDOW, after toggling it.  Interactively, this argument is
  always non-nil.

  When a window is dedicated to its buffer, `display-buffer' will avoid
  displaying another buffer in it, if possible.  When a window is
  strongly dedicated to its buffer, changing the buffer shown in the
  window will usually signal an error.

  See the info node `(elisp)Dedicated Windows' for more details.