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

From: Eshel Yaron <[email-masked]>
Date: Thu, 09 May 2024 16:19:31 +0200

Eli Zaretskii <[email-masked]> writes:

>> From: Eshel Yaron <[email-masked]>
>> Cc: [email-masked],  [email-masked]
>> 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:

Thanks, I'm attaching another iteration of this patch below.

>>  @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.

Done.

>> --- 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.

Done.

>> ++++
>> +*** 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.

And done.

>> +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?

AFAICT there are no invalid values, since we take "any other value" to
mean the same as 'hollow' (see C-h v cursor-type), so I think not
validating anything should be perfectly valid :)

> 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?

Maybe, but that's a bit tricky because we need to consider whether or
not the window is selected and what cursor-in-non-selected-windows has
to say to figure out what the cursor will actually look like.  Currently
this function just provides Lisp with access to the window slot, so we
have all the needed information available in Lisp.  I suggest keeping
'window-cursor-type' simple and, if the need arises, providing a Lisp
function that calculates the effective cursor type for a given window.
WDYT?

>> --- 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.  */

Oh I missed that, thanks!

> So you must move this new Lisp_Object member to before
> mode_line_help_echo

Done, here's the updated patch:

>From c685d968962be8e16205bffb4cc1a953218d8e4f Mon Sep 17 00:00:00 2001
From: Eshel Yaron <[email-masked]>
Date: Sat, 27 Apr 2024 20:47:34 +0200
Subject: [PATCH v4] New functions '(set-)window-cursor-type'

* src/window.h (struct window): Add 'cursor_type' slot.
(wset_cursor_type): New inline function.
* src/xdisp.c (get_window_cursor_type): Consult 'cursor_type'.
* src/window.c (make_window): Initialize 'cursor_type' to t.
(Fset_window_cursor_type, Fwindow_cursor_type): New functions.
(syms_of_window): List their symbols.
* doc/lispref/windows.texi (Window Point): Document them.
* doc/lispref/frames.texi (Cursor Parameters): Mention
new 'set-window-cursor-type'.
* etc/NEWS: Announce new functions.  (Bug#70622)
---
 doc/lispref/frames.texi  |  6 +++---
 doc/lispref/windows.texi | 13 +++++++++++++
 etc/NEWS                 |  7 +++++++
 src/window.c             | 30 ++++++++++++++++++++++++++++++
 src/window.h             |  9 +++++++++
 src/xdisp.c              | 29 ++++++++++++++++++-----------
 6 files changed, 80 insertions(+), 14 deletions(-)

diff --git a/doc/lispref/frames.texi b/doc/lispref/frames.texi
index cae93acae9f..974def3de10 100644
--- a/doc/lispref/frames.texi
+++ b/doc/lispref/frames.texi
@@ -2341,9 +2341,9 @@ Cursor Parameters
 @end table
 
 @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} (@pxref{Window Point}), and by the
+variables @code{cursor-type} and @code{cursor-in-non-selected-windows}:
 
 @defopt cursor-type
 This buffer-local variable controls how the cursor looks in a selected
diff --git a/doc/lispref/windows.texi b/doc/lispref/windows.texi
index 104420235df..eb9a5b4efb1 100644
--- a/doc/lispref/windows.texi
+++ b/doc/lispref/windows.texi
@@ -5142,6 +5142,19 @@ 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.  @xref{Cursor
+Parameters}.  If @var{window} is @code{nil}, it means to set the cursor
+type for the selected window.
+@end defun
+
+@defun window-cursor-type &optional window
+This function returns the cursor type of @var{window}, defaulting to the
+selected window.
+@end defun
+
 @node Window Start and End
 @section The Window Start and End Positions
 @cindex window start position
diff --git a/etc/NEWS b/etc/NEWS
index 9c356e64bde..700c7d060b8 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -340,6 +340,13 @@ and 'window-state-get'.  Then later another new variable
 'window-state-put' to restore positions of window points
 according to the context stored in a window parameter.
 
++++
+*** 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.  Windows
+are always created with a 'window-cursor-type' of t, which means to
+consult the variable 'cursor-type' as before.
+
 ** Tab Bars and Tab Lines
 
 ---
diff --git a/src/window.c b/src/window.c
index cf12841bd51..e71f1b152a6 100644
--- a/src/window.c
+++ b/src/window.c
@@ -4425,6 +4425,7 @@ make_window (void)
   wset_old_pointm (w, Fmake_marker ());
   wset_vertical_scroll_bar_type (w, Qt);
   wset_horizontal_scroll_bar_type (w, Qt);
+  wset_cursor_type (w, Qt);
   /* These Lisp fields are marked specially so they're not set to nil by
      allocate_window.  */
   wset_prev_buffers (w, Qnil);
@@ -8050,6 +8051,33 @@ DEFUN ("window-fringes", Fwindow_fringes, Swindow_fringes,
 		w->fringes_persistent ? Qt : Qnil);
 }
 
+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);
+
+  /* Redisplay with updated cursor type.  */
+  wset_redisplay (w);
+
+  return type;
+}
+
+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;
+}
 
 
 /***********************************************************************
@@ -8976,4 +9004,6 @@ syms_of_window (void)
   defsubr (&Swindow_parameters);
   defsubr (&Swindow_parameter);
   defsubr (&Sset_window_parameter);
+  defsubr (&Swindow_cursor_type);
+  defsubr (&Sset_window_cursor_type);
 }
diff --git a/src/window.h b/src/window.h
index 19283725931..86932181252 100644
--- a/src/window.h
+++ b/src/window.h
@@ -205,6 +205,9 @@ #define WINDOW_H_INCLUDED
     /* An alist with parameters.  */
     Lisp_Object window_parameters;
 
+    /* `cursor-type' to use in this window.  */
+    Lisp_Object cursor_type;
+
     /* The help echo text for this window.  Qnil if there's none.  */
     Lisp_Object mode_line_help_echo;
 
@@ -542,6 +545,12 @@ wset_horizontal_scroll_bar_type (struct window *w, Lisp_Object val)
   w->horizontal_scroll_bar_type = val;
 }
 
+INLINE void
+wset_cursor_type (struct window *w, Lisp_Object val)
+{
+  w->cursor_type = val;
+}
+
 INLINE void
 wset_prev_buffers (struct window *w, Lisp_Object val)
 {
diff --git a/src/xdisp.c b/src/xdisp.c
index 85802ec5083..24d0c090326 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -33617,7 +33617,9 @@ get_window_cursor_type (struct window *w, struct glyph *glyph, int *width,
     {
       if (w == XWINDOW (echo_area_window))
 	{
-	  if (EQ (BVAR (b, cursor_type), Qt) || NILP (BVAR (b, cursor_type)))
+	  if (!EQ (Qt, w->cursor_type))
+	      return get_specified_cursor_type (w->cursor_type, width);
+	  else if (EQ (BVAR (b, cursor_type), Qt) || NILP (BVAR (b, cursor_type)))
 	    {
 	      *width = FRAME_CURSOR_WIDTH (f);
 	      return FRAME_DESIRED_CURSOR (f);
@@ -33644,18 +33646,23 @@ get_window_cursor_type (struct window *w, struct glyph *glyph, int *width,
       non_selected = true;
     }
 
-  /* Never display a cursor in a window in which cursor-type is nil.  */
-  if (NILP (BVAR (b, cursor_type)))
-    return NO_CURSOR;
-
-  /* Get the normal cursor type for this window.  */
-  if (EQ (BVAR (b, cursor_type), Qt))
+  if (!EQ (Qt, w->cursor_type))
+      cursor_type = get_specified_cursor_type (w->cursor_type, width);
+  else
     {
-      cursor_type = FRAME_DESIRED_CURSOR (f);
-      *width = FRAME_CURSOR_WIDTH (f);
+      /* Never display a cursor in a window in which cursor-type is nil.  */
+      if (NILP (BVAR (b, cursor_type)))
+	return NO_CURSOR;
+
+      /* Get the normal cursor type for this window.  */
+      if (EQ (BVAR (b, cursor_type), Qt))
+	{
+	  cursor_type = FRAME_DESIRED_CURSOR (f);
+	  *width = FRAME_CURSOR_WIDTH (f);
+	}
+      else
+	cursor_type = get_specified_cursor_type (BVAR (b, cursor_type), width);
     }
-  else
-    cursor_type = get_specified_cursor_type (BVAR (b, cursor_type), width);
 
   /* Use cursor-in-non-selected-windows instead
      for non-selected window or frame.  */
-- 
2.45.0