Hi, Eli Zaretskii 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" >> >> 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?