Fix cursor colour #1910
Merged
Fix cursor colour #1910
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes emacs-evil#502, fixes emacs-evil#1835. Refreshing the cursor when changing the evil state is only needed (or almost only, see below) when the current buffer is displayed in the selected window. If this is not the case, and the buffer is only displayed or its window selected at some later point in time, then `evil-refresh-cursor` will be called as part of the `window-configuration-change-hook` or as advice to `select-window`. However, this introduces the following tiny (and maybe acceptable?) regression: When doing something like ```elisp (with-current-buffer (some-buffer-displayed-in-another-window) (some-evil-state-with-a-different-cursor-type)) ``` the cursor will not be refreshed in the other window before selecting it. The cursor colour should indeed not be refreshed, because it is defined for the whole frame; however, the cursor type should in principle be changed, as it is defined per buffer and also defines the shape of the cursor in non-selected windows. There exist different ways to also handle this case, but they mostly seem ugly or needlessly complicated to me. I think the most elegant way to fix this would involve implementing per-buffer cursor colors in Emacs (as suggested in https://debbugs.gnu.org/cgi/bugreport.cgi?bug=24153d). What do you think?
fnussbaum
force-pushed
the
fix-cursor-colour
branch
from
a2da991 to
8de83ab
Compare
|
Thanks for the PR @fnussbaum I think this is a net improvement. Hopefully we won't see a rush of complaints wrt the regression you mentioned. I'm happy to take the risk. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #502, fixes #1835. Refreshing the cursor when changing the evil state is
only needed (or almost only, see below) when the current buffer is displayed in the
selected window. If this is not the case, and the buffer is only displayed or
its window selected at some later point in time, then
evil-refresh-cursorwillbe called as part of the
window-configuration-change-hookor as advice toselect-window.However, this introduces the following tiny (and maybe acceptable?) regression:
When doing something like
(with-current-buffer some-buffer-displayed-in-another-window (some-evil-state-with-a-different-cursor-type))the cursor will not be refreshed in the other window before selecting it. The
cursor colour should indeed not be refreshed, because it is defined for the
whole frame; however, the cursor type should in principle be changed, as it is
defined per buffer and also defines the shape of the cursor in non-selected
windows.
There exist different ways to also handle this case, but they mostly seem ugly
or needlessly complicated to me. I think the most elegant way to fix this would
involve implementing per-buffer cursor colours in Emacs (as suggested in
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=24153d). What do you think?