GNU bug report logs - #70622
[PATCH] New window parameter 'cursor-type'

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 70622 in the body.
You can then email your comments to 70622 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] New window parameter 'cursor-type'
Date: Sun, 28 Apr 2024 08:27:53 +0200

[Message part 1 (text/plain, inline)]

Tags: patch

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.

[0001-New-window-parameter-cursor-type.patch (text/patch, attachment)]

Message #8 received at 70622 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: 70622 <at> debbugs.gnu.org
Subject: Re: bug#70622: [PATCH] New window parameter 'cursor-type'
Date: Sun, 28 Apr 2024 10:22:02 +0300

> 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" <bug-gnu-emacs <at> gnu.org>
> 
> 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.

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

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

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

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

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

Also, what about the cursor in mini-windows?

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?  If so, I'm not sure the patch ensures
such an update.  In particular, non-selected windows are not
guaranteed to be updated by redisplay cycles, unless redisplay is
told, either directly or indirectly, about the need to do so.  To test
this, bind to a simple key, like F8, a command that changes the
window-parameter, and see if the change takes effect as soon as you
expect.

Message #11 received at 70622 <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 70622 <at> debbugs.gnu.org
Subject: Re: bug#70622: [PATCH] New window parameter 'cursor-type'
Date: Sun, 28 Apr 2024 17:00:33 +0200

[Message part 1 (text/plain, inline)]

Hi,

Eli Zaretskii <eliz <at> gnu.org> 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" <bug-gnu-emacs <at> gnu.org>
>>
>> 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 <at> 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?

[v2-0001-New-window-parameter-cursor-type.patch (text/x-patch, attachment)]

Message #14 received at 70622 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Eshel Yaron <me <at> eshelyaron.com>, martin rudalics <rudalics <at> gmx.at>
Cc: 70622 <at> debbugs.gnu.org
Subject: Re: bug#70622: [PATCH] New window parameter 'cursor-type'
Date: Sun, 28 Apr 2024 18:29:30 +0300

> From: Eshel Yaron <me <at> eshelyaron.com>
> Cc: 70622 <at> debbugs.gnu.org
> Date: Sun, 28 Apr 2024 17:00:33 +0200
> 
> > 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".

We have the same problem in the frame parameter by this name, and we
solve it there without these complications.  Why should the window
parameter be different?  It's confusing and hard to remember.

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

Not just that, but it also means one cannot specify a different cursor
type for a window when it's selected and when not selected, unlike
with buffers.  Is this anomaly justified?

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

I tend to think option 1 is better, but I'm curious what others think.
Martin, WDYT?

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

I think it would be better to make sure the parameter takes effect
immediately.  Documenting the force-window-update thing should be
fallback, if the immediate effect is impossible.

>        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);
> +	    }

We don't use braces for a single-line block.

Message #17 received at 70622 <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: martin rudalics <rudalics <at> gmx.at>, 70622 <at> debbugs.gnu.org
Subject: Re: bug#70622: [PATCH] New window parameter 'cursor-type'
Date: Sun, 28 Apr 2024 21:05:59 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Eshel Yaron <me <at> eshelyaron.com>
>> Cc: 70622 <at> debbugs.gnu.org
>> Date: Sun, 28 Apr 2024 17:00:33 +0200
>>
>> > 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".
>
> We have the same problem in the frame parameter by this name, and we
> solve it there without these complications.  Why should the window
> parameter be different?

This is a little different, though.  Indeed, the frame parameter also
uses nil to denote "no cursor".  The variable, which overrides the frame
parameter, uses t to delegate to the frame parameter, so it supports all
values of the frame parameter (including nil), plus t.  But unlike a
variable, we can't make the window parameter default to t (or any other
non-nil value) as easily.

> It's confusing and hard to remember.

I went with this format because it lets you use all values of the
cursor-type variable in a uniform manner (just wrap it in a list).
Alternatively, we can use the exact same format for the window
parameter as we do for the variable, except we interpret a window
parameter nil value as "window parameter unset", not "no cursor", and
instead add another value for the window parameter, 'none, that'll
correspond to the nil value of the variable (so window parameter 'none
would say not to show the cursor).  This is in line with what we have
for the mode-line-format variable and window parameter, for example.
Does this alternative sound better?

>> > 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?
>
> Not just that, but it also means one cannot specify a different cursor
> type for a window when it's selected and when not selected, unlike
> with buffers.  Is this anomaly justified?

See below.

>> > 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?
>
> I tend to think option 1 is better, but I'm curious what others think.

All right.  I can implement that option instead, if that's deemed
preferable.  The way I see it, option 1 makes it easy to set the
cursor for a window and have it change based on whether or not the
window is selected, but it requires a bit more work to set the cursor
for that window once and for all if you don't want it to change based
on window selection.  Option 2 is the other way around: it's easier in
case you want to set the cursor type for a window regardless of
whether it is selected or not.  So there's a small tradeoff here.

> Martin, WDYT?
>

[...]

> I think it would be better to make sure the parameter takes effect
> immediately.  Documenting the force-window-update thing should be
> fallback, if the immediate effect is impossible.

OK, I'll look into it.

>>        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);
>> +	    }
>
> We don't use braces for a single-line block.

Thanks, I'll fix that in the next iteration of this patch.


Best,

Eshel

Message #20 received at 70622 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: rudalics <at> gmx.at, 70622 <at> debbugs.gnu.org
Subject: Re: bug#70622: [PATCH] New window parameter 'cursor-type'
Date: Mon, 29 Apr 2024 09:31:49 +0300

> From: Eshel Yaron <me <at> eshelyaron.com>
> Cc: martin rudalics <rudalics <at> gmx.at>,  70622 <at> debbugs.gnu.org
> Date: Sun, 28 Apr 2024 21:05:59 +0200
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> From: Eshel Yaron <me <at> eshelyaron.com>
> >> Cc: 70622 <at> debbugs.gnu.org
> >> Date: Sun, 28 Apr 2024 17:00:33 +0200
> >>
> >> > 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".
> >
> > We have the same problem in the frame parameter by this name, and we
> > solve it there without these complications.  Why should the window
> > parameter be different?
> 
> This is a little different, though.  Indeed, the frame parameter also
> uses nil to denote "no cursor".  The variable, which overrides the frame
> parameter, uses t to delegate to the frame parameter, so it supports all
> values of the frame parameter (including nil), plus t.  But unlike a
> variable, we can't make the window parameter default to t (or any other
> non-nil value) as easily.

I still don't think I understand the difficultly.  The "window
parameter not set" can be detected by testing whether cursor-type is
an element in the alist returned by window-parameters, couldn't it?

> > It's confusing and hard to remember.
> 
> I went with this format because it lets you use all values of the
> cursor-type variable in a uniform manner (just wrap it in a list).

It is confusing to have several parameters and variables with a
similar semantics that each require special quirks to get the same
effect.  We should try to make the forms of the values of such
variables and parameters be uniform.

> Alternatively, we can use the exact same format for the window
> parameter as we do for the variable, except we interpret a window
> parameter nil value as "window parameter unset", not "no cursor", and
> instead add another value for the window parameter, 'none, that'll
> correspond to the nil value of the variable (so window parameter 'none
> would say not to show the cursor).

That's possible, but doesn't the absence of cursor-type parameter from
window-parameters already allow to solve that?

> This is in line with what we have
> for the mode-line-format variable and window parameter, for example.

Where do we use this convention for window parameters?

Message #23 received at 70622 <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: rudalics <at> gmx.at, 70622 <at> debbugs.gnu.org
Subject: Re: bug#70622: [PATCH] New window parameter 'cursor-type'
Date: Mon, 29 Apr 2024 10:18:33 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Eshel Yaron <me <at> eshelyaron.com>
>> Cc: martin rudalics <rudalics <at> gmx.at>,  70622 <at> debbugs.gnu.org
>> Date: Sun, 28 Apr 2024 21:05:59 +0200
>>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>> >> From: Eshel Yaron <me <at> eshelyaron.com>
>> >> Cc: 70622 <at> debbugs.gnu.org
>> >> Date: Sun, 28 Apr 2024 17:00:33 +0200
>> >>
>> >> > 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".
>> >
>> > We have the same problem in the frame parameter by this name, and we
>> > solve it there without these complications.  Why should the window
>> > parameter be different?
>>
>> This is a little different, though.  Indeed, the frame parameter also
>> uses nil to denote "no cursor".  The variable, which overrides the frame
>> parameter, uses t to delegate to the frame parameter, so it supports all
>> values of the frame parameter (including nil), plus t.  But unlike a
>> variable, we can't make the window parameter default to t (or any other
>> non-nil value) as easily.
>
> I still don't think I understand the difficultly.  The "window
> parameter not set" can be detected by testing whether cursor-type is
> an element in the alist returned by window-parameters, couldn't it?

Perhaps I'm missing something, but I think it's not so simple.  We
can't/shouldn't interpret a value of nil differently from no alist entry
at all, since there's no way to remove alist entries (in order to unset
the window parameter) from lisp, right?

>> > It's confusing and hard to remember.
>>
>> I went with this format because it lets you use all values of the
>> cursor-type variable in a uniform manner (just wrap it in a list).
>
> It is confusing to have several parameters and variables with a
> similar semantics that each require special quirks to get the same
> effect.  We should try to make the forms of the values of such
> variables and parameters be uniform.

Agreed.  If we could use exactly the same format for the values of the
variable and window parameter that would be best.  But AFAIU we need
some encoding (special quirk) when translating from variable values (VV)
to window parameter values (WPV), due to the competing interpretations
for nil.  The first encoding I proposed is simple and uniform: cons to
go from VV to WPV, car to go the other way.  The alternative encoding I
proposed below, following mode-line-format, is trivial for most VV, but
not uniform, because one needs to translate nil to 'none when going from
VV to WPV.

>> Alternatively, we can use the exact same format for the window
>> parameter as we do for the variable, except we interpret a window
>> parameter nil value as "window parameter unset", not "no cursor", and
>> instead add another value for the window parameter, 'none, that'll
>> correspond to the nil value of the variable (so window parameter 'none
>> would say not to show the cursor).
>
> That's possible, but doesn't the absence of cursor-type parameter from
> window-parameters already allow to solve that?

See above.  Please let me know if I'm mistaken about the possibility of
differentiating a value of nil from the absence of an alist entry.

>> This is in line with what we have
>> for the mode-line-format variable and window parameter, for example.
>
> Where do we use this convention for window parameters?

See "(elisp) Window Parameters":

‘mode-line-format’
     This parameter replaces the value of the buffer-local variable
     ‘mode-line-format’ (*note Mode Line Basics::) of this window's
     buffer whenever this window is displayed.  The symbol ‘none’ means
     to suppress display of a mode line for this window.

Here the window parameter has the same set of possible values as the
variable, except the window parameter also has the value 'none, which
corresponds to the variable's nil value.  The nil value of the window
parameter means "window parameter not set".

Message #26 received at 70622 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: rudalics <at> gmx.at, 70622 <at> debbugs.gnu.org
Subject: Re: bug#70622: [PATCH] New window parameter 'cursor-type'
Date: Mon, 29 Apr 2024 12:08:08 +0300

> From: Eshel Yaron <me <at> eshelyaron.com>
> Cc: rudalics <at> gmx.at,  70622 <at> debbugs.gnu.org
> Date: Mon, 29 Apr 2024 10:18:33 +0200
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > I still don't think I understand the difficultly.  The "window
> > parameter not set" can be detected by testing whether cursor-type is
> > an element in the alist returned by window-parameters, couldn't it?
> 
> Perhaps I'm missing something, but I think it's not so simple.  We
> can't/shouldn't interpret a value of nil differently from no alist entry
> at all, since there's no way to remove alist entries (in order to unset
> the window parameter) from lisp, right?

Why is the ability to remove important?

> > It is confusing to have several parameters and variables with a
> > similar semantics that each require special quirks to get the same
> > effect.  We should try to make the forms of the values of such
> > variables and parameters be uniform.
> 
> Agreed.  If we could use exactly the same format for the values of the
> variable and window parameter that would be best.  But AFAIU we need
> some encoding (special quirk) when translating from variable values (VV)
> to window parameter values (WPV), due to the competing interpretations
> for nil.  The first encoding I proposed is simple and uniform: cons to
> go from VV to WPV, car to go the other way.  The alternative encoding I
> proposed below, following mode-line-format, is trivial for most VV, but
> not uniform, because one needs to translate nil to 'none when going from
> VV to WPV.
> 
> >> Alternatively, we can use the exact same format for the window
> >> parameter as we do for the variable, except we interpret a window
> >> parameter nil value as "window parameter unset", not "no cursor", and
> >> instead add another value for the window parameter, 'none, that'll
> >> correspond to the nil value of the variable (so window parameter 'none
> >> would say not to show the cursor).
> >
> > That's possible, but doesn't the absence of cursor-type parameter from
> > window-parameters already allow to solve that?
> 
> See above.  Please let me know if I'm mistaken about the possibility of
> differentiating a value of nil from the absence of an alist entry.
> 
> >> This is in line with what we have
> >> for the mode-line-format variable and window parameter, for example.
> >
> > Where do we use this convention for window parameters?
> 
> See "(elisp) Window Parameters":
> 
> ‘mode-line-format’
>      This parameter replaces the value of the buffer-local variable
>      ‘mode-line-format’ (*note Mode Line Basics::) of this window's
>      buffer whenever this window is displayed.  The symbol ‘none’ means
>      to suppress display of a mode line for this window.
> 
> Here the window parameter has the same set of possible values as the
> variable, except the window parameter also has the value 'none, which
> corresponds to the variable's nil value.  The nil value of the window
> parameter means "window parameter not set".

If this is unavoidable (which I'm not yet sure), we can use this.  But
let's first hear from Martin, maybe there are better ideas.

Message #29 received at 70622 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Eli Zaretskii <eliz <at> gnu.org>, Eshel Yaron <me <at> eshelyaron.com>
Cc: 70622 <at> debbugs.gnu.org
Subject: Re: bug#70622: [PATCH] New window parameter 'cursor-type'
Date: Mon, 29 Apr 2024 11:48:07 +0200

>> 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?
>
> I tend to think option 1 is better, but I'm curious what others think.
> Martin, WDYT?

I'd give 'cursor-in-non-selected-windows' precedence over the new window
parameter and not add another window parameter to override it.  IIUC
that's what a buffer-local value of 'cursor-in-non-selected-windows'
currently does to a 'cursor-type' frame parameter.

>> 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?
>
> I think it would be better to make sure the parameter takes effect
> immediately.  Documenting the force-window-update thing should be
> fallback, if the immediate effect is impossible.

I think 'set-window-parameter' should handle 'cursor-type' specially -
when it changes, it should set that window's redisplay flag.

martin

Message #32 received at 70622 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Eli Zaretskii <eliz <at> gnu.org>, Eshel Yaron <me <at> eshelyaron.com>
Cc: 70622 <at> debbugs.gnu.org
Subject: Re: bug#70622: [PATCH] New window parameter 'cursor-type'
Date: Mon, 29 Apr 2024 11:48:19 +0200

>> Here the window parameter has the same set of possible values as the
>> variable, except the window parameter also has the value 'none, which
>> corresponds to the variable's nil value.  The nil value of the window
>> parameter means "window parameter not set".
>
> If this is unavoidable (which I'm not yet sure), we can use this.  But
> let's first hear from Martin, maybe there are better ideas.

Since I'm responsible for the 'none' convention (which I dislike) I have
no better ideas.  Would it help to make cursor_type a slot in the window
structure and provide a 'set-window-cursor-type' function?

martin

Message #35 received at 70622 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: martin rudalics <rudalics <at> gmx.at>
Cc: me <at> eshelyaron.com, 70622 <at> debbugs.gnu.org
Subject: Re: bug#70622: [PATCH] New window parameter 'cursor-type'
Date: Mon, 29 Apr 2024 14:10:46 +0300

> Date: Mon, 29 Apr 2024 11:48:19 +0200
> Cc: 70622 <at> debbugs.gnu.org
> From: martin rudalics <rudalics <at> gmx.at>
> 
>  >> Here the window parameter has the same set of possible values as the
>  >> variable, except the window parameter also has the value 'none, which
>  >> corresponds to the variable's nil value.  The nil value of the window
>  >> parameter means "window parameter not set".
>  >
>  > If this is unavoidable (which I'm not yet sure), we can use this.  But
>  > let's first hear from Martin, maybe there are better ideas.
> 
> Since I'm responsible for the 'none' convention (which I dislike) I have
> no better ideas.  Would it help to make cursor_type a slot in the window
> structure and provide a 'set-window-cursor-type' function?

So that cursor-type is no longer a window-parameter?

Message #38 received at 70622 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: me <at> eshelyaron.com, 70622 <at> debbugs.gnu.org
Subject: Re: bug#70622: [PATCH] New window parameter 'cursor-type'
Date: Tue, 30 Apr 2024 11:03:44 +0200

> So that cursor-type is no longer a window-parameter?

Yes.  IIUC it would mean that we have to initialize the cursor_type slot
of a new window to t.  With the window parameter solution one would have
to add a new 'cursor-type' parameter with a value of t whenever making a
new window.  This would break with the convention that non-nil window
parameters are used for handling "special" behaviors only.

martin

Message #41 received at 70622 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: martin rudalics <rudalics <at> gmx.at>
Cc: me <at> eshelyaron.com, 70622 <at> debbugs.gnu.org
Subject: Re: bug#70622: [PATCH] New window parameter 'cursor-type'
Date: Tue, 30 Apr 2024 15:12:12 +0300

> Date: Tue, 30 Apr 2024 11:03:44 +0200
> Cc: me <at> eshelyaron.com, 70622 <at> debbugs.gnu.org
> From: martin rudalics <rudalics <at> gmx.at>
> 
>  > So that cursor-type is no longer a window-parameter?
> 
> Yes.  IIUC it would mean that we have to initialize the cursor_type slot
> of a new window to t.  With the window parameter solution one would have
> to add a new 'cursor-type' parameter with a value of t whenever making a
> new window.  This would break with the convention that non-nil window
> parameters are used for handling "special" behaviors only.

I'm fine with this approach, thanks.  It will also solve nicely the
problem of redisplaying the affected window ASAP.

Message #44 received at 70622 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: me <at> eshelyaron.com
Cc: rudalics <at> gmx.at, 70622 <at> debbugs.gnu.org
Subject: Re: bug#70622: [PATCH] New window parameter 'cursor-type'
Date: Thu, 09 May 2024 10:44:30 +0300

> Cc: me <at> eshelyaron.com, 70622 <at> debbugs.gnu.org
> Date: Tue, 30 Apr 2024 15:12:12 +0300
> From: Eli Zaretskii <eliz <at> gnu.org>
> 
> > Date: Tue, 30 Apr 2024 11:03:44 +0200
> > Cc: me <at> eshelyaron.com, 70622 <at> debbugs.gnu.org
> > From: martin rudalics <rudalics <at> gmx.at>
> > 
> >  > So that cursor-type is no longer a window-parameter?
> > 
> > Yes.  IIUC it would mean that we have to initialize the cursor_type slot
> > of a new window to t.  With the window parameter solution one would have
> > to add a new 'cursor-type' parameter with a value of t whenever making a
> > new window.  This would break with the convention that non-nil window
> > parameters are used for handling "special" behaviors only.
> 
> I'm fine with this approach, thanks.  It will also solve nicely the
> problem of redisplaying the affected window ASAP.

Eshel, could you please update your patch along these lines?

Thanks.

Message #47 received at 70622 <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: rudalics <at> gmx.at, 70622 <at> debbugs.gnu.org
Subject: Re: bug#70622: [PATCH] New window parameter 'cursor-type'
Date: Thu, 09 May 2024 12:56:54 +0200

[Message part 1 (text/plain, inline)]

Hi,

Eli Zaretskii <eliz <at> gnu.org> writes:

>> Cc: me <at> eshelyaron.com, 70622 <at> debbugs.gnu.org
>> Date: Tue, 30 Apr 2024 15:12:12 +0300
>> From: Eli Zaretskii <eliz <at> gnu.org>
>> 
>> > Date: Tue, 30 Apr 2024 11:03:44 +0200
>> > Cc: me <at> eshelyaron.com, 70622 <at> debbugs.gnu.org
>> > From: martin rudalics <rudalics <at> gmx.at>
>> > 
>> >  > So that cursor-type is no longer a window-parameter?
>> > 
>> > Yes.  IIUC it would mean that we have to initialize the cursor_type slot
>> > of a new window to t.  With the window parameter solution one would have
>> > to add a new 'cursor-type' parameter with a value of t whenever making a
>> > new window.  This would break with the convention that non-nil window
>> > parameters are used for handling "special" behaviors only.
>> 
>> I'm fine with this approach, thanks.  It will also solve nicely the
>> problem of redisplaying the affected window ASAP.
>
> Eshel, could you please update your patch along these lines?

Yes, how does the attached patch look?

[v3-0001-New-functions-set-window-cursor-type.patch (text/x-patch, attachment)]

Message #50 received at 70622 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: rudalics <at> gmx.at, 70622 <at> debbugs.gnu.org
Subject: Re: bug#70622: [PATCH] New window parameter 'cursor-type'
Date: Thu, 09 May 2024 14:25:37 +0300

> From: Eshel Yaron <me <at> eshelyaron.com>
> Cc: rudalics <at> gmx.at,  70622 <at> 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.

Message #53 received at 70622 <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: rudalics <at> gmx.at, 70622 <at> debbugs.gnu.org
Subject: Re: bug#70622: [PATCH] New window parameter 'cursor-type'
Date: Thu, 09 May 2024 16:19:31 +0200

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Eshel Yaron <me <at> eshelyaron.com>
>> Cc: rudalics <at> gmx.at,  70622 <at> 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:

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:

[v4-0001-New-functions-set-window-cursor-type.patch (text/x-patch, attachment)]

Message #56 received at 70622 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Eshel Yaron <me <at> eshelyaron.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 70622 <at> debbugs.gnu.org
Subject: Re: bug#70622: [PATCH] New window parameter 'cursor-type'
Date: Fri, 10 May 2024 10:58:58 +0200

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

I think Eshel is right.

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

If needed, the function can be given a second argument that allows to
retrieve either the value specified for this window or the effective
value used.

'set-window-cursor-type' like all other functions and parameters in this
context suffers from the problem that there's no common concept wrt
setting window/buffer/frame-local values.  This matters when a
buffer-local value changes or another buffer is shown in a window.  For
margins or fringes the PERSISTENT argument can be used to tell whether a
window-specified value shall survive subsequent 'set-window-buffer'
calls.  Setting the buffer variable for these doesn't take effect
immediately.

For 'set-window-cursor-type' we probably should say that any value it
sets is "persistent" (survives subsequent 'set-window-buffer' calls)
unless the value delegates the choice to the 'cursor-type' variable or
frame parameter and that it takes effect immediately.

And we _could_ tell that setting the 'cursor-type' variable takes effect
"pretty soon", that is when an associated window is redisplayed so it
does not have to wait until the next 'set-window-buffer' call, that it
can be overridden by 'set-window-cursor-type' in quite some ways, and
that 'set-window-buffer' won't use the buffer value when another way has
been specified via 'set-window-cursor-type'.  If I'm right, obviously.

martin

Message #59 received at 70622 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: rudalics <at> gmx.at, 70622 <at> debbugs.gnu.org
Subject: Re: bug#70622: [PATCH] New window parameter 'cursor-type'
Date: Fri, 10 May 2024 19:19:00 +0300

> From: Eshel Yaron <me <at> eshelyaron.com>
> Cc: rudalics <at> gmx.at,  70622 <at> debbugs.gnu.org
> Date: Thu, 09 May 2024 16:19:31 +0200
> 
> >> +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 :)

I know, but that's because there's no useful way of signaling an error
in the middle of redisplay.  Here we have an opportunity to help the
caller to catch any mistakes before the value is applied, so why not?

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

Martin suggested a way to have both, so I think we should take it.

Message #62 received at 70622 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: martin rudalics <rudalics <at> gmx.at>
Cc: me <at> eshelyaron.com, 70622 <at> debbugs.gnu.org
Subject: Re: bug#70622: [PATCH] New window parameter 'cursor-type'
Date: Fri, 10 May 2024 19:20:05 +0300

> Date: Fri, 10 May 2024 10:58:58 +0200
> Cc: 70622 <at> debbugs.gnu.org
> From: martin rudalics <rudalics <at> gmx.at>
> 
>  >>> +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?
> 
> I think Eshel is right.

Any reasons?

>  > 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?
> 
> If needed, the function can be given a second argument that allows to
> retrieve either the value specified for this window or the effective
> value used.

SGTM, thanks.

Message #65 received at 70622 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: me <at> eshelyaron.com, 70622 <at> debbugs.gnu.org
Subject: Re: bug#70622: [PATCH] New window parameter 'cursor-type'
Date: Sat, 11 May 2024 09:35:37 +0200

>>   >> 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?
>>
>> I think Eshel is right.
>
> Any reasons?

The doc-string of the variable 'cursor-type' states that

    cursor-type is a variable defined in ‘C source code’.

    Its value is t

    Cursor to use when this buffer is in the selected window.
    Values are interpreted as follows:

      t               use the cursor specified for the frame
      [...]
      ANYTHING ELSE   display a hollow box cursor

Suppose the buffer value is ANYTHING ELSE, someone wants to temporarily
change it for a specific window and later wants to reset it back to its
buffer value via 'set-window-cursor-type'.  Should Emacs signal an error
up front in that case?

martin

Message #68 received at 70622 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: martin rudalics <rudalics <at> gmx.at>
Cc: me <at> eshelyaron.com, 70622 <at> debbugs.gnu.org
Subject: Re: bug#70622: [PATCH] New window parameter 'cursor-type'
Date: Sat, 11 May 2024 11:34:46 +0300

> Date: Sat, 11 May 2024 09:35:37 +0200
> Cc: me <at> eshelyaron.com, 70622 <at> debbugs.gnu.org
> From: martin rudalics <rudalics <at> gmx.at>
> 
>  >>   >> 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?
>  >>
>  >> I think Eshel is right.
>  >
>  > Any reasons?
> 
> The doc-string of the variable 'cursor-type' states that
> 
>      cursor-type is a variable defined in ‘C source code’.
> 
>      Its value is t
> 
>      Cursor to use when this buffer is in the selected window.
>      Values are interpreted as follows:
> 
>        t               use the cursor specified for the frame
>        [...]
>        ANYTHING ELSE   display a hollow box cursor
> 
> Suppose the buffer value is ANYTHING ELSE, someone wants to temporarily
> change it for a specific window and later wants to reset it back to its
> buffer value via 'set-window-cursor-type'.  Should Emacs signal an error
> up front in that case?

I don't see why not.

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.

But if you are still unconvinced, I won't argue, although I still
don't understand your and Eshel's objections.

Message #71 received at 70622 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: me <at> eshelyaron.com, 70622 <at> debbugs.gnu.org
Subject: Re: bug#70622: [PATCH] New window parameter 'cursor-type'
Date: Sun, 12 May 2024 10:29:24 +0200

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

martin

Message #74 received at 70622 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: martin rudalics <rudalics <at> gmx.at>
Cc: me <at> eshelyaron.com, 70622 <at> debbugs.gnu.org
Subject: Re: bug#70622: [PATCH] New window parameter 'cursor-type'
Date: Sun, 12 May 2024 12:39:15 +0300

> Date: Sun, 12 May 2024 10:29:24 +0200
> Cc: me <at> eshelyaron.com, 70622 <at> debbugs.gnu.org
> From: martin rudalics <rudalics <at> 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.

Message #77 received at 70622 <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: martin rudalics <rudalics <at> gmx.at>, 70622 <at> debbugs.gnu.org
Subject: Re: bug#70622: [PATCH] New window parameter 'cursor-type'
Date: Sun, 12 May 2024 14:33:45 +0200

[Message part 1 (text/plain, inline)]

Hi,

Eli Zaretskii <eliz <at> gnu.org> writes:

>> Date: Sun, 12 May 2024 10:29:24 +0200
>> Cc: me <at> eshelyaron.com, 70622 <at> debbugs.gnu.org
>> From: martin rudalics <rudalics <at> 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:

[v5-0001-New-functions-set-window-cursor-type.patch (text/x-patch, attachment)]

Message #80 received at 70622 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Eshel Yaron <me <at> eshelyaron.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 70622 <at> debbugs.gnu.org
Subject: Re: bug#70622: [PATCH] New window parameter 'cursor-type'
Date: Mon, 13 May 2024 10:05:37 +0200

> Here's the updated patch:

One ambiguity:

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

"This setting" may refer to the "initial value".  I'd write "The value
set by this function".

martin

Message #83 received at 70622 <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: martin rudalics <rudalics <at> gmx.at>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 70622 <at> debbugs.gnu.org
Subject: Re: bug#70622: [PATCH] New window parameter 'cursor-type'
Date: Mon, 13 May 2024 14:13:53 +0200

[Message part 1 (text/plain, inline)]

Hi Martin,

martin rudalics <rudalics <at> gmx.at> writes:

>> Here's the updated patch:
>
> One ambiguity:
>
>> 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}.
>
> "This setting" may refer to the "initial value".  I'd write "The value
> set by this function".

Thanks, I've adopted your wording in the updated patch below.

[v6-0001-New-functions-set-window-cursor-type.patch (text/x-patch, attachment)]

Message #86 received at 70622 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: rudalics <at> gmx.at, 70622 <at> debbugs.gnu.org
Subject: Re: bug#70622: [PATCH] New window parameter 'cursor-type'
Date: Sat, 18 May 2024 12:43:17 +0300

> From: Eshel Yaron <me <at> eshelyaron.com>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  70622 <at> debbugs.gnu.org
> Date: Mon, 13 May 2024 14:13:53 +0200
> 
> Hi Martin,
> 
> martin rudalics <rudalics <at> gmx.at> writes:
> 
> >> Here's the updated patch:
> >
> > One ambiguity:
> >
> >> 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}.
> >
> > "This setting" may refer to the "initial value".  I'd write "The value
> > set by this function".
> 
> Thanks, I've adopted your wording in the updated patch below.

Feel free to install, and thanks.

Message #89 received at 70622 <at> debbugs.gnu.org (full text, mbox):

From: Eshel Yaron <me <at> eshelyaron.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: rudalics <at> gmx.at, 70622 <at> debbugs.gnu.org
Subject: Re: bug#70622: [PATCH] New window parameter 'cursor-type'
Date: Sat, 18 May 2024 15:45:29 +0200

close 70622 30.1
quit

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Eshel Yaron <me <at> eshelyaron.com>
>> Cc: Eli Zaretskii <eliz <at> gnu.org>,  70622 <at> debbugs.gnu.org
>> Date: Mon, 13 May 2024 14:13:53 +0200
>> 
>> Hi Martin,
>> 
>> martin rudalics <rudalics <at> gmx.at> writes:
>> 
>> >> Here's the updated patch:
>> >
>> > One ambiguity:
>> >
>> >> 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}.
>> >
>> > "This setting" may refer to the "initial value".  I'd write "The value
>> > set by this function".
>> 
>> Thanks, I've adopted your wording in the updated patch below.
>
> Feel free to install, and thanks.

Thanks, pushed to master (commit 3b890bf2bd2), and closing the bug.


Eshel

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.