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

[Eshel Yaron via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

[-- Attachment #1: Type: text/plain, Size: 1971 bytes --]

Hi,

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Sun, 12 May 2024 10:29:24 +0200
>> Cc: me@eshelyaron.com, 70622@debbugs.gnu.org
>> From: martin rudalics <rudalics@gmx.at>
>>
>>  > The ANYTHING ELSE clause is simply defensive programming, something we
>>  > do in many other places.  It can easily hide mistakes, and I see no
>>  > reason not to help users find those mistakes.
>>
>> I'm not a fan of strong typing but I agree with you.  The problem I see
>> is that we don't care about catching mistakes when setting the variable
>> or the corresponding frame parameter.  If we want to help users finding
>> mistakes, we should care everywhere in a consistent fashion.
>>
>>  > But if you are still unconvinced, I won't argue, although I still
>>  > don't understand your and Eshel's objections.
>>
>> I only stated my personal view and will leave it to Eshel and you how
>> serve users best.
>
> Thanks.  Let's see what Eshel takes from this.

I think that given there are no strictly invalid cursor-type values,
designating some (most) values as invalid for set-window-cursor-type
reduces consistency for little practical gain.  OTOH, I don't think
there's any real harm to it either, so I added such a check in the
update patch below.

I've also extended the documentation of set-window-cursor-type to note
that this setting persists when you change the buffer of the window, as
Martin suggested.

I've added a FIXME comment about extending window-cursor-type with an
optional argument that asks for the effective cursor type, but I did not
implement it at this point.  I looked into it, but the implementation is
quite involved, it basically requires duplicating many of the careful
considerations of get_window_cursor_type, or refactoring it for reuse.
Since we do provide all the needed information for computing the
effective cursor in Lisp, and there's no concrete use case for doing
that ATM, I think we can leave it at for now.


Here's the updated patch:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v5-0001-New-functions-set-window-cursor-type.patch --]
[-- Type: text/x-patch, Size: 8468 bytes --]

From 5f257d8b0ed85be9ac638b36b6669cd96c70cada Mon Sep 17 00:00:00 2001
From: Eshel Yaron <me@eshelyaron.com>
Date: Sat, 27 Apr 2024 20:47:34 +0200
Subject: [PATCH v5] 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 | 18 +++++++++++++++
 etc/NEWS                 |  7 ++++++
 src/window.c             | 49 ++++++++++++++++++++++++++++++++++++++++
 src/window.h             |  9 ++++++++
 src/xdisp.c              | 29 +++++++++++++++---------
 6 files changed, 104 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..eac7e01946d 100644
--- a/doc/lispref/windows.texi
+++ b/doc/lispref/windows.texi
@@ -5142,6 +5142,24 @@ 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.
+
+The initial value for new windows is @code{t}, which says to respect the
+buffer-local value of @code{cursor-type}.  This setting persists across
+buffers shown in @var{window}, so @code{set-window-buffer} does not
+reset it.  @xref{Buffers and Windows}.
+@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 846bf759995..c93b04bae2d 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..03f359b932b 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,52 @@ 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.  The initial value
+for new windows is t, which says to respect the buffer-local value of
+`cursor-type'.
+
+WINDOW nil means use the selected window.  This setting persists across
+buffers shown in WINDOW, so `set-window-buffer' does not reset it.  */)
+  (Lisp_Object window, Lisp_Object type)
+{
+  struct window *w = decode_live_window (window);
+
+  if (!(NILP (type)
+	|| EQ (type, Qt)
+	|| EQ (type, Qbox)
+	|| EQ (type, Qhollow)
+	|| EQ (type, Qbar)
+	|| EQ (type, Qhbar)
+	|| (CONSP (type)
+	    && (EQ (XCAR (type), Qbox)
+		|| EQ (XCAR (type), Qbar)
+		|| EQ (XCAR (type), Qhbar))
+	    && INTEGERP (XCDR (type)))))
+    error ("Invalid cursor type");
+
+  wset_cursor_type (w, type);
+
+  /* Redisplay with updated cursor type.  */
+  wset_redisplay (w);
+
+  return type;
+}
+
+/* FIXME: Add a way to get the _effective_ cursor type, possibly by
+   extending this function with an additional optional argument.  */
+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;
+}
 
 \f
 /***********************************************************************
@@ -8976,4 +9023,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 4b7d7e02c68..c74247e43aa 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -33616,7 +33616,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);
@@ -33643,18 +33645,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