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

From: Eshel Yaron <[email-masked]>
Date: Sun, 28 Apr 2024 17:00:33 +0200

Hi,

Eli Zaretskii <[email-masked]> writes:

>> Date: Sun, 28 Apr 2024 08:27:53 +0200
>> From:  Eshel Yaron via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <[email-masked]>
>>
>> This patch makes it possible to set the cursor type in a specific window,
>> without affecting other windows that may be showing the same buffer.
>
> Thanks, I have a few comments and questions below.
>

Thanks for reviewing.

>> --- a/doc/lispref/windows.texi
>> +++ b/doc/lispref/windows.texi
>> @@ -6691,6 +6691,15 @@ Window Parameters
>>  window and should be used, with due care, exclusively by those
>>  applications.  It might be replaced by an improved solution in future
>>  versions of Emacs.
>> +
>> +@item cursor-type
>> +@vindex cursor-type@r{, a window parameter}
>> +If this parameter is set to a cons cell, its @sc{car} specifies the
>> +shape of the cursor in this window, using the same format as the
>> +buffer-local variable @code{cursor-type}.  @xref{Cursor Parameters}.
>> +Use this window parameter instead of the @code{cursor-type} variable or
>> +frame parameter when a buffer is displayed in multiple windows and you
>> +want to change the cursor for one window without affecting the others.
>>  @end table
>
> This doesn't say what happens when the buffer-local variable and the
> window parameter don't agree.

Right, I've made this more explicit in the updated patch below.

> Also, the "Cursor Parameters" node should mention this window-specific
> parameter, with a cross-reference.

Done.

>> +*** New window parameter 'cursor-type'.
>> +If this parameter is set to a cons cell, its 'car' specifies the shape
>> +of the window's cursor, using the same format as the buffer-local
>> +variable 'cursor-type'.
>
> Why only cons cells are supported?
>

We need a convenient way to specify both "window parameter not set" and
"window parameter set and says not to show a cursor".  Naturally, we
want to use nil for "window parameter not set", but then nil is what the
variable cursor-type uses for "don't to show a cursor".  So we embed the
space of possible values for the variable in a cons cell to clearly
disambiguate the variable's nil from the window parameter's nil.
Namely, if the window parameter is set to '(nil) that means not to show
a cursor, in contrast with a plain nil which means the window parameter
is unset.

>> +  win_cursor = window_parameter (w, Qcursor_type);
>> +  if (CONSP (win_cursor))
>>      {
>> -      cursor_type = FRAME_DESIRED_CURSOR (f);
>> -      *width = FRAME_CURSOR_WIDTH (f);
>> +      cursor_type = get_specified_cursor_type (XCAR (win_cursor), width);
>>      }
>
> Same question here.
>

See above.

> And I have a question: is this supposed to work for non-selected
> windows as well?  The documentation you added says nothing about that,
> but I wonder what was the intent?

Yes, it's supposed to work for non-selected windows as well.  Do you
think that's worth mentioning explicitly in the documentation?

> The reason I ask is that we have two buffer-local variables, not one,
> for both selected and non-selected windows, whereas your patch
> provides just one window parameter.  How will it interact with the
> buffer-local variables in both cases?

That's a really good point.  WRT cursor-in-non-selected-windows, I think
there are two viable options:

1. Give cursor-in-non-selected-windows precedence over the new window
   parameter, and add another window parameter to override
   cursor-in-non-selected-windows.
2. Give the new window parameter precedence also over
   cursor-in-non-selected-windows.

In the updated patch, I went with option 2, so if you set the
cursor-type window parameter, that overrides any buffer-local variable,
whether or not the window is selected.  I think that's sensible enough,
WDYT?

> Also, what about the cursor in mini-windows?

This applies also to the mini-window.  In the updated patch, I've
extended it to take effect also when the minibuffer is not selected but
cursor-in-echo-area is non-nil.

> In addition, what is supposed to happen when this new window-parameter
> is changed? is the cursor supposed to be redrawn in the new shape
> immediately, i.e. do you expect redisplay to happen right away to
> update the relevant window?

Not necessarily.  In cases where we want to ensure the cursor is redrawn
immediately in a non-selected window, we can use force-window-update
after setting the window parameter.  Should that be mentioned in the
documentation as well?

>From 0838e20f9d23b5be81ab6ff7460d0dc0d519754b Mon Sep 17 00:00:00 2001
From: Eshel Yaron <[email-masked]>
Date: Sat, 27 Apr 2024 20:47:34 +0200
Subject: [PATCH v2] New window parameter 'cursor-type'

* src/xdisp.c (get_window_cursor_type): Consult new window
parameter 'cursor-type'.
* doc/lispref/windows.texi (Window Parameters): Document it.
* doc/lispref/frames.texi (Cursor Parameters): Mention it.
* etc/NEWS: Announce it.  (Bug#70622)
---
 doc/lispref/frames.texi  |  5 +++--
 doc/lispref/windows.texi | 12 ++++++++++++
 etc/NEWS                 |  6 ++++++
 src/xdisp.c              | 37 +++++++++++++++++++++++++------------
 4 files changed, 46 insertions(+), 14 deletions(-)

diff --git a/doc/lispref/frames.texi b/doc/lispref/frames.texi
index cae93acae9f..b79879063fb 100644
--- a/doc/lispref/frames.texi
+++ b/doc/lispref/frames.texi
@@ -2341,8 +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
+The @code{cursor-type} frame parameter may be overridden by the window
+parameter @code{cursor-type} (@pxref{Definition of cursor-type window
+parameter}), and by the variables @code{cursor-type} and
 @code{cursor-in-non-selected-windows}:
 
 @defopt cursor-type
diff --git a/doc/lispref/windows.texi b/doc/lispref/windows.texi
index 104420235df..b66b81aca6e 100644
--- a/doc/lispref/windows.texi
+++ b/doc/lispref/windows.texi
@@ -6691,6 +6691,18 @@ Window Parameters
 window and should be used, with due care, exclusively by those
 applications.  It might be replaced by an improved solution in future
 versions of Emacs.
+
+@item cursor-type
+@vindex cursor-type@r{, a window parameter}
+@anchor{Definition of cursor-type window parameter}
+If this parameter is set to a cons cell, its @sc{car} specifies the
+shape of the cursor in this window, using the same format as the
+buffer-local variable @code{cursor-type}.  @xref{Cursor Parameters}.
+Use this window parameter instead of the @code{cursor-type} variable or
+frame parameter when a buffer is displayed in multiple windows and you
+want to change the cursor for one window without affecting the others.
+This window parameter takes precedence over the @code{cursor-type}
+variable as well as @code{cursor-in-non-selected-windows}.
 @end table

diff --git a/etc/NEWS b/etc/NEWS
index 9c356e64bde..e08998e4b6f 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -340,6 +340,12 @@ 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 window parameter 'cursor-type'.
+If this parameter is set to a cons cell, its 'car' specifies the shape
+of the window's cursor, using the same format as the buffer-local
+variable 'cursor-type'.
+
 ** Tab Bars and Tab Lines
 
 ---
diff --git a/src/xdisp.c b/src/xdisp.c
index 85802ec5083..9e4e145d93d 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -33605,7 +33605,7 @@ get_window_cursor_type (struct window *w, struct glyph *glyph, int *width,
   struct frame *f = XFRAME (w->frame);
   struct buffer *b = XBUFFER (w->contents);
   int cursor_type = DEFAULT_CURSOR;
-  Lisp_Object alt_cursor;
+  Lisp_Object alt_cursor, win_cursor;
   bool non_selected = false;
 
   *active_cursor = true;
@@ -33617,7 +33617,12 @@ 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)))
+	  win_cursor = window_parameter (w, Qcursor_type);
+	  if (CONSP (win_cursor))
+	    {
+	      return get_specified_cursor_type (XCAR (win_cursor), 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,22 +33649,30 @@ 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))
+  win_cursor = window_parameter (w, Qcursor_type);
+  if (CONSP (win_cursor))
     {
-      cursor_type = FRAME_DESIRED_CURSOR (f);
-      *width = FRAME_CURSOR_WIDTH (f);
+      cursor_type = get_specified_cursor_type (XCAR (win_cursor), width);
     }
   else
-    cursor_type = get_specified_cursor_type (BVAR (b, cursor_type), width);
+    {
+      /* 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);
+    }
 
   /* Use cursor-in-non-selected-windows instead
      for non-selected window or frame.  */
-  if (non_selected)
+  if (non_selected && !CONSP (win_cursor))
     {
       alt_cursor = BVAR (b, cursor_in_non_selected_windows);
       if (!EQ (Qt, alt_cursor))
-- 
2.44.0