Subject: bug#23403: 25.1.50; cua-mode does not interact with cursor-face and themes well

From: Mauro Aranda <[email-masked]>
Date: Mon, 17 Mar 2025 07:10:28 -0300

Mauro Aranda <[email-masked]> writes:

 > "Jeremy T. Braun" <[email-masked]> writes:
 >
 >> I've had issues where themes couldn't change cursor colors.  I finally
 >> figured out the culprit, cua-mode.  Details on what I noticed and how I
 >> found it follow.
 >>
 >>
 >>
 >> I believe the "correct" way for themes to interact with the cursor color
 >> is to modify the :background attribute of the face named cursor.  See:
 >>
 > 
https://www.gnu.org/software/emacs/manual/html_node/emacs/Cursor-Display.html
 >>
 >> cua-mode defines a variable cua-normal-cursor-color.  By default this is
 >> set to one of initial-cursor-color, cursor-color from initial-frame-alis
 >> or default-frame-alist, the frame-parameter 'cursor-color, or "red" if
 >> those are all not true.
 >>
 >> When cua-enable-cursor-indications is enabled, cua--update-indications
 >> is run as a part of cua--post-command-handler, which is added to
 >> post-command-hook.
 >>
 >> When run, cua-enable-cursor-indications typically calls set-cursor-color
 >> to one of the cua-*-cursor-color variables.
 >>
 >> The result?  Themes can change the cursor-color (via the face or via
 >> set-cursor-color, but after they are loaded/run, the post-command-hook
 >> eventually runs cua--update-indications, which overwrites the theme's
 >> new "normal" cursor color with whatever was previously stored in
 >> cua-normal-cursor-color (for "normal" buffers, anyway).
 >>
 >> I first noticed this because I run emacs in --daemon mode a lot.  When
 >> emacs is in --daemon mode, I think that .emacs/init.el is run in the
 >> frame/context of the terminal or background emacs process. When my
 >> init.el is run, (frame-parameter nil 'cursor-color) returns "white",
 >> which cua-normal-cursor-color is set to.  When I switch between the
 >> background-mode = dark/light color themes linked below, the cursor
 >> always stays white, and it's illegible in the "light" theme.
 >> https://github.com/sellout/emacs-color-theme-solarized
 >>
 >> When I launch emacs without --daemon, my init.el loads the theme first,
 >> which sets frame-parameter.cursor-color to #839496, which initializes
 >> cua-normal-cursor-color, which is legible in both dark and light modes.
 >
 > IIUC, the issue here can be reproduced in a couple of ways. Here's one:
 > emacs -Q
 > M-x cua-mode
 > M-x customize-set-variable RET cua-enable-cursor-indications RET y
 > M-x load-theme RET deeper-blue
 >
 > Notice that the cursor is black.  Compare with:
 > emacs -Q
 > M-x load-theme RET deeper-blue
 > M-x cua-mode
 > M-x customize-set-variable RET cua-enable-cursor-indications RET y
 >
 > Now the cursor is green.  If you want, load any other theme, and see
 > that the cursor color doesn't change: it stays green.
 >
 >> I'm not sure how "best" to fix this.  Options I see:
 >> * Change documentation on custom themes, requiring that in addition to
 >>   modifying the cursor face, they should also modify cua-*-cursor-color.
 >
 > Themes have pretty much no choice, since it seems to me that for a good
 > user experience, they need to make sure all cua-*-cursor-color options
 > interact well with their settings.
 >
 >> * Change cua-mode to be aware that themes may modify the cursor face,
 >>   and either update cua-normal-cursor-face to reflect the cursor face's
 >>   :background attribute, or maybe have cua--update-indications use
 >>   something like:
 >>     (or cua-normal-cursor-face (grab the :background attribute of
 >>     cursor-face))
 >>   And then the user can set/customize cua-normal-cursor-face to nil to
 >>   allow the theme to control it.
 >
 > This idea seems like an improvement.  Allow nil for this option, so that
 > it CUA understands it as "don't mess with the cursor color".
 >
 >
 > Opinions?

Here's a patch.  Question is, do we change the default value too?
From 34fc680db3f40ede391b87370fe210af71ce0a98 Mon Sep 17 00:00:00 2001
From: Mauro Aranda <[email-masked]>
Date: Sun, 16 Mar 2025 10:07:50 -0300
Subject: [PATCH] Respect customized cursor face in cua-mode

* lisp/emulation/cua-base.el (cua-normal-cursor-color): Accept
nil as value.  Update version.  (Bug#23403)
* test/lisp/emulation/cua-base-tests.el: New file.
---
 lisp/emulation/cua-base.el            | 11 +++++---
 test/lisp/emulation/cua-base-tests.el | 36 +++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 3 deletions(-)
 create mode 100644 test/lisp/emulation/cua-base-tests.el

diff --git a/lisp/emulation/cua-base.el b/lisp/emulation/cua-base.el
index baee1a6965f..0b710320606 100644
--- a/lisp/emulation/cua-base.el
+++ b/lisp/emulation/cua-base.el
@@ -481,9 +481,12 @@ cua-normal-cursor-color
 If the value is a COLOR name, then only the `cursor-color' attribute will be
 affected.  If the value is a cursor TYPE (one of: box, hollow, bar, or hbar),
 then only the `cursor-type' property will be affected.  If the value is
-a cons (TYPE . COLOR), then both properties are affected."
+a cons (TYPE . COLOR), then both properties are affected.
+
+A value of nil means to respect the cursor's background property."
   :initialize #'custom-initialize-default
   :type '(choice
+          (const :tag "Use cursor's background property" nil)
 	  (color :tag "Color")
 	  (choice :tag "Type"
 		  (const :tag "Filled box" box)
@@ -496,7 +499,8 @@ cua-normal-cursor-color
 			(const :tag "Vertical bar" bar)
 			(const :tag "Horizontal bar" hbar)
 			(const :tag "Hollow box" hollow))
-		(color :tag "Color"))))
+                (color :tag "Color")))
+  :version "31.1")
 
 (defcustom cua-read-only-cursor-color "darkgreen"
   "Cursor color used in read-only buffers, if non-nil.
@@ -1087,7 +1091,8 @@ cua--update-indications
 	    cua-read-only-cursor-color)
 	   ((and cua-overwrite-cursor-color overwrite-mode)
 	    cua-overwrite-cursor-color)
-	   (t cua-normal-cursor-color)))
+	   (t (or cua-normal-cursor-color
+                  (face-attribute 'cursor :background)))))
 	 (color (if (consp cursor) (cdr cursor) cursor))
 	 (type (if (consp cursor) (car cursor) cursor)))
     (if (and color
diff --git a/test/lisp/emulation/cua-base-tests.el b/test/lisp/emulation/cua-base-tests.el
new file mode 100644
index 00000000000..444907a0a24
--- /dev/null
+++ b/test/lisp/emulation/cua-base-tests.el
@@ -0,0 +1,36 @@
+;;; cua-base-tests.el --- Tests for cua-base.el      -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2025 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+;;
+;; GNU Emacs is free software: you can redistribute it and/or
+;; modify it under the terms of the GNU General Public License as
+;; published by the Free Software Foundation, either version 3 of the
+;; License, or (at your option) any later version.
+;;
+;; GNU Emacs is distributed in the hope that it will be useful, but
+;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+;; General Public License for more details.
+;;
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Code:
+(require 'ert)
+(require 'cua-base)
+
+(ert-deftest cua-base-test-normal-cursor-color ()
+  "Test that cua respects a customized/themed cursor color."
+  (load-theme 'deeper-blue t)
+  (let ((color (face-attribute 'cursor :background)))
+    (disable-theme 'deeper-blue)
+    (cua-mode)
+    (setq cua-enable-cursor-indications t)
+    (setq cua-normal-cursor-color nil)
+    (enable-theme 'deeper-blue)
+    (should (string= (frame-parameter nil 'cursor-color) color))))
+
+(provide 'cua-base-tests)
+;;; cua-base-tests.el ends here
-- 
2.34.1