bug#70622: [PATCH] New window parameter 'cursor-type'

[Eli Zaretskii <eliz@gnu.org>]

> From: Eshel Yaron <me@eshelyaron.com>
> Cc: rudalics@gmx.at,  70622@debbugs.gnu.org
> Date: Thu, 09 May 2024 12:56:54 +0200
> 
> > Eshel, could you please update your patch along these lines?
> 
> Yes, how does the attached patch look?

Looks good, with a few nits:

>  @vindex cursor-type
> -The @code{cursor-type} frame parameter may be overridden by the
> -variables @code{cursor-type} and
> -@code{cursor-in-non-selected-windows}:
> +The @code{cursor-type} frame parameter may be overridden by
> +@code{set-window-cursor-type}, and by the variables @code{cursor-type}
> +and @code{cursor-in-non-selected-windows}:

Please add here a cross-reference to where set-window-cursor-type is
described.

> --- a/doc/lispref/windows.texi
> +++ b/doc/lispref/windows.texi
> @@ -5142,6 +5142,18 @@ Window Point
>  so @code{window-point} will stay behind text inserted there.
>  @end defvar
>  
> +@defun set-window-cursor-type window type
> +This function sets the cursor shape for @var{window}.  This setting
> +takes precedence over the @code{cursor-type} variable, and @var{type}
> +has the same format as the value of that variable.

And here, please add a cross-reference to where the values accepted by
cursor-type are described.

> ++++
> +*** New functions 'set-window-cursor-type' and 'window-cursor-type'.
> +'set-window-cursor-type' sets a per-window cursor type, and
> +'window-cursor-type' queries this setting for a given window.

Here, we should tell what is the default value and its meaning.

> +DEFUN ("set-window-cursor-type", Fset_window_cursor_type,
> +       Sset_window_cursor_type, 2, 2, 0,
> +       doc: /* Set the `cursor-type' of WINDOW to TYPE.
> +
> +This setting takes precedence over the variable `cursor-type', and TYPE
> +has the same format as the value of that variable.  WINDOW nil means use
> +the selected window.  */)
> +  (Lisp_Object window, Lisp_Object type)
> +{
> +  struct window *w = decode_live_window (window);
> +
> +  wset_cursor_type (w, type);

Shouldn't we validate the value of TYPE before plugging it into the
window?  I know we will validate it at display time, but maybe it's a
good idea to do that here as well, and signal an error up front?
Martin, WDYT?

> +DEFUN ("window-cursor-type", Fwindow_cursor_type, Swindow_cursor_type,
> +       0, 1, 0,
> +       doc: /* Return the `cursor-type' of WINDOW.
> +WINDOW must be a live window and defaults to the selected one.  */)
> +  (Lisp_Object window)
> +{
> +  return decode_live_window (window)->cursor_type;
> +}

This will AFAIU return t if the value is Qt.  Should we return the
value actually in effect in that case?

> --- a/src/window.h
> +++ b/src/window.h
> @@ -323,6 +323,9 @@ #define WINDOW_H_INCLUDED
>         as the normal may yield a matrix which is too small.  */
>      int nrows_scale_factor, ncols_scale_factor;
>  
> +    /* `cursor-type' to use in this window.  */
> +    Lisp_Object cursor_type;
> +

The comment after the mode_line_help_echo member of 'struct window'
says:

    /* No Lisp data may follow this point; mode_line_help_echo must be
       the last Lisp member.  */

So you must move this new Lisp_Object member to before
mode_line_help_echo (or move it just after it, and update the comment
and allocate_window accordingly).  This is because GC needs to know
where the Lisp part of a window object ends.

Thanks.